The Postcopy preempt capability requires to be set before incoming
starts, so change the postcopy tests to start with deferred incoming and
call migrate-incoming after the cap has been set.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 tests/qtest/migration-test.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index b99b49a314..31ce3d959c 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1158,10 +1158,11 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
                                     QTestState **to_ptr,
                                     MigrateCommon *args)
 {
-    g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+    g_autofree char *uri = NULL;
     QTestState *from, *to;
+    QDict *rsp;
 
-    if (test_migrate_start(&from, &to, uri, &args->start)) {
+    if (test_migrate_start(&from, &to, "defer", &args->start)) {
         return -1;
     }
 
@@ -1180,9 +1181,14 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
 
     migrate_ensure_non_converge(from);
 
+    rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
+                           "  'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
+    qobject_unref(rsp);
+
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
 
+    uri = migrate_get_socket_address(to, "socket-address");
     migrate_qmp(from, uri, "{}");
 
     wait_for_migration_pass(from);
-- 
2.27.0

On Tue, May 30, 2023 at 05:02:59PM +0800, Wei Wang wrote:
> The Postcopy preempt capability requires to be set before incoming
> starts, so change the postcopy tests to start with deferred incoming and
> call migrate-incoming after the cap has been set.
> 
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>

Hmm.. so we used to do socket_start_incoming_migration_internal() before
setting the right num for the preempt test, then I'm curious why it wasn't
failing before this patch when trying to connect with the preempt channel..

Wei, do you know?

> ---
>  tests/qtest/migration-test.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index b99b49a314..31ce3d959c 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1158,10 +1158,11 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>                                      QTestState **to_ptr,
>                                      MigrateCommon *args)
>  {
> -    g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> +    g_autofree char *uri = NULL;
>      QTestState *from, *to;
> +    QDict *rsp;
>  
> -    if (test_migrate_start(&from, &to, uri, &args->start)) {
> +    if (test_migrate_start(&from, &to, "defer", &args->start)) {
>          return -1;
>      }
>  
> @@ -1180,9 +1181,14 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>  
>      migrate_ensure_non_converge(from);
>  
> +    rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
> +                           "  'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
> +    qobject_unref(rsp);
> +
>      /* Wait for the first serial output from the source */
>      wait_for_serial("src_serial");
>  
> +    uri = migrate_get_socket_address(to, "socket-address");
>      migrate_qmp(from, uri, "{}");
>  
>      wait_for_migration_pass(from);
> -- 
> 2.27.0
> 

-- 
Peter Xu

On Tuesday, May 30, 2023 10:41 PM, Peter Xu wrote:
> On Tue, May 30, 2023 at 05:02:59PM +0800, Wei Wang wrote:
> > The Postcopy preempt capability requires to be set before incoming
> > starts, so change the postcopy tests to start with deferred incoming
> > and call migrate-incoming after the cap has been set.
> >
> > Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> 
> Hmm.. so we used to do socket_start_incoming_migration_internal() before
> setting the right num for the preempt test, then I'm curious why it wasn't
> failing before this patch when trying to connect with the preempt channel..
> 
> Wei, do you know?

I think there are two reasons:
#1 "backlog" specifies the number of pending connections. As long as the server
accepts the connections faster than the clients side connecting, connection
will succeed. For the preempt test, it uses only 2 channels, so very likely to not have
pending connections. (This is easier to trigger for the multifd case, e.g. use a much
larger number like 16).
#2 per my tests (on kernel 6.2), the number of pending connections allowed is actually
"backlog + 1", which is 2 in this case. Adding more pending connections will then be
dropped. I'm not sure if " backlog + 1" is true for older versions of kernel, though.

On Wed, May 31, 2023 at 10:41:37AM +0000, Wang, Wei W wrote:
> On Tuesday, May 30, 2023 10:41 PM, Peter Xu wrote:
> > On Tue, May 30, 2023 at 05:02:59PM +0800, Wei Wang wrote:
> > > The Postcopy preempt capability requires to be set before incoming
> > > starts, so change the postcopy tests to start with deferred incoming
> > > and call migrate-incoming after the cap has been set.
> > >
> > > Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> > 
> > Hmm.. so we used to do socket_start_incoming_migration_internal() before
> > setting the right num for the preempt test, then I'm curious why it wasn't
> > failing before this patch when trying to connect with the preempt channel..
> > 
> > Wei, do you know?
> 
> I think there are two reasons:
> #1 "backlog" specifies the number of pending connections. As long as the server
> accepts the connections faster than the clients side connecting, connection
> will succeed. For the preempt test, it uses only 2 channels, so very likely to not have
> pending connections. (This is easier to trigger for the multifd case, e.g. use a much
> larger number like 16).
> #2 per my tests (on kernel 6.2), the number of pending connections allowed is actually
> "backlog + 1", which is 2 in this case. Adding more pending connections will then be
> dropped. I'm not sure if " backlog + 1" is true for older versions of kernel, though.

Interesting to know, thanks.

If there'll be a new version, please consider adding some of those into the
commit message.

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

On Wednesday, May 31, 2023 8:58 PM, Peter Xu wrote:
> > > Hmm.. so we used to do socket_start_incoming_migration_internal()
> > > before setting the right num for the preempt test, then I'm curious
> > > why it wasn't failing before this patch when trying to connect with the
> preempt channel..
> > >
> > > Wei, do you know?
> >
> > I think there are two reasons:
> > #1 "backlog" specifies the number of pending connections. As long as
> > the server accepts the connections faster than the clients side
> > connecting, connection will succeed. For the preempt test, it uses
> > only 2 channels, so very likely to not have pending connections. (This
> > is easier to trigger for the multifd case, e.g. use a much larger number like
> 16).
> > #2 per my tests (on kernel 6.2), the number of pending connections
> > allowed is actually "backlog + 1", which is 2 in this case. Adding
> > more pending connections will then be dropped. I'm not sure if " backlog +
> 1" is true for older versions of kernel, though.
> 
> Interesting to know, thanks.
> 
> If there'll be a new version, please consider adding some of those into the
> commit message.

OK, will resend with commit update.
Plan to wait a bit in case there would be other feedbacks.

> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> --
> Peter Xu