[Qemu-devel] [PULL 5/5] multifd: Use number of channels as listen backlog

Juan Quintela posted 5 patches 6 years, 2 months ago
Maintainers: Michael Roth <mdroth@linux.vnet.ibm.com>, Stefan Berger <stefanb@linux.ibm.com>, Kevin Wolf <kwolf@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Eric Blake <eblake@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Fam Zheng <fam@euphon.net>, Juan Quintela <quintela@redhat.com>, Max Reitz <mreitz@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
[Qemu-devel] [PULL 5/5] multifd: Use number of channels as listen backlog
Posted by Juan Quintela 6 years, 2 months ago
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/socket.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/migration/socket.c b/migration/socket.c
index e63f5e1612..97c9efde59 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -178,10 +178,15 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
 {
     QIONetListener *listener = qio_net_listener_new();
     size_t i;
+    int num = 1;
 
     qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_net_listener_open_sync(listener, saddr, 1, errp) < 0) {
+    if (migrate_use_multifd()) {
+        num = migrate_multifd_channels();
+    }
+
+    if (qio_net_listener_open_sync(listener, saddr, num, errp) < 0) {
         object_unref(OBJECT(listener));
         return;
     }
-- 
2.21.0


Re: [Qemu-devel] [PULL 5/5] multifd: Use number of channels as listen backlog
Posted by Wei Yang 6 years, 1 month ago
On Wed, Sep 04, 2019 at 08:29:15AM +0200, Juan Quintela wrote:
>Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>Signed-off-by: Juan Quintela <quintela@redhat.com>
>---
> migration/socket.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/migration/socket.c b/migration/socket.c
>index e63f5e1612..97c9efde59 100644
>--- a/migration/socket.c
>+++ b/migration/socket.c
>@@ -178,10 +178,15 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
> {
>     QIONetListener *listener = qio_net_listener_new();
>     size_t i;
>+    int num = 1;
> 
>     qio_net_listener_set_name(listener, "migration-socket-listener");
> 
>-    if (qio_net_listener_open_sync(listener, saddr, 1, errp) < 0) {
>+    if (migrate_use_multifd()) {
>+        num = migrate_multifd_channels();
>+    }
>+
>+    if (qio_net_listener_open_sync(listener, saddr, num, errp) < 0) {
>         object_unref(OBJECT(listener));
>         return;
>     }

My confusion is this function is called at the beginning of the program, which
means we didn't set multifd on or change the multifd channel parameter.

They are the default value at this point.

Am I right?

>-- 
>2.21.0
>

-- 
Wei Yang
Help you, Help me

Re: [PULL 5/5] multifd: Use number of channels as listen backlog
Posted by Juan Quintela 6 years, 1 month ago
Wei Yang <richardw.yang@linux.intel.com> wrote:
> On Wed, Sep 04, 2019 at 08:29:15AM +0200, Juan Quintela wrote:
>>Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>Signed-off-by: Juan Quintela <quintela@redhat.com>
>>---
>> migration/socket.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>>diff --git a/migration/socket.c b/migration/socket.c
>>index e63f5e1612..97c9efde59 100644
>>--- a/migration/socket.c
>>+++ b/migration/socket.c
>>@@ -178,10 +178,15 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
>> {
>>     QIONetListener *listener = qio_net_listener_new();
>>     size_t i;
>>+    int num = 1;
>> 
>>     qio_net_listener_set_name(listener, "migration-socket-listener");
>> 
>>-    if (qio_net_listener_open_sync(listener, saddr, 1, errp) < 0) {
>>+    if (migrate_use_multifd()) {
>>+        num = migrate_multifd_channels();
>>+    }
>>+
>>+    if (qio_net_listener_open_sync(listener, saddr, num, errp) < 0) {
>>         object_unref(OBJECT(listener));
>>         return;
>>     }
>
> My confusion is this function is called at the beginning of the program, which
> means we didn't set multifd on or change the multifd channel parameter.
>
> They are the default value at this point.
>
> Am I right?

Hi

good catch!

You are right.  The fix worked for me because I always use on the
command line:

--global migration.multifd-channels=10

or whatever number I want to avoid typing.  I can only see two
solutions:
- increase the number always
- require "defer" when using multifd to be able to setup parameters.

Any other good ideas?

Thanks, Juan.

PD.  I was having problem reproducing this issue because I use the
command line for the parameter.

Re: [PULL 5/5] multifd: Use number of channels as listen backlog
Posted by Wei Yang 6 years, 1 month ago
On Fri, Oct 11, 2019 at 12:40:03PM +0200, Juan Quintela wrote:
>Wei Yang <richardw.yang@linux.intel.com> wrote:
>> On Wed, Sep 04, 2019 at 08:29:15AM +0200, Juan Quintela wrote:
>>>Reviewed-by: Daniel P. Berrang?? <berrange@redhat.com>
>>>Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>---
>>> migration/socket.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>>diff --git a/migration/socket.c b/migration/socket.c
>>>index e63f5e1612..97c9efde59 100644
>>>--- a/migration/socket.c
>>>+++ b/migration/socket.c
>>>@@ -178,10 +178,15 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
>>> {
>>>     QIONetListener *listener = qio_net_listener_new();
>>>     size_t i;
>>>+    int num = 1;
>>> 
>>>     qio_net_listener_set_name(listener, "migration-socket-listener");
>>> 
>>>-    if (qio_net_listener_open_sync(listener, saddr, 1, errp) < 0) {
>>>+    if (migrate_use_multifd()) {
>>>+        num = migrate_multifd_channels();
>>>+    }
>>>+
>>>+    if (qio_net_listener_open_sync(listener, saddr, num, errp) < 0) {
>>>         object_unref(OBJECT(listener));
>>>         return;
>>>     }
>>
>> My confusion is this function is called at the beginning of the program, which
>> means we didn't set multifd on or change the multifd channel parameter.
>>
>> They are the default value at this point.
>>
>> Am I right?
>
>Hi
>
>good catch!
>
>You are right.  The fix worked for me because I always use on the
>command line:
>
>--global migration.multifd-channels=10
>
>or whatever number I want to avoid typing.  I can only see two
>solutions:
>- increase the number always

You mean change default value? Then which one should we choose?

>- require "defer" when using multifd to be able to setup parameters.
>
>Any other good ideas?

Would you mind explaining more about "defer"? How this works?

>
>Thanks, Juan.
>
>PD.  I was having problem reproducing this issue because I use the
>command line for the parameter.

-- 
Wei Yang
Help you, Help me