qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 8/8] multifd: rest of zlib compression


From: Juan Quintela
Subject: Re: [Qemu-devel] [PATCH v3 8/8] multifd: rest of zlib compression
Date: Tue, 11 Jun 2019 18:54:49 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)

Wei Yang <address@hidden> wrote:
> On Wed, May 15, 2019 at 02:15:44PM +0200, Juan Quintela wrote:
>>This is still a work in progress, but get everything sent as expected
>>and it is faster than the code that is already there.
>
> Generally, I prefer to merge this one with previous one.

Done, sir O:-)

For the WIP part, it was easier to have the bits that didn't change and
the ones that I was working with.

>>@@ -1145,7 +1239,11 @@ static void *multifd_send_thread(void *opaque)
>>     /* initial packet */
>>     p->num_packets = 1;
>> 
>>-    multifd_send_state->ops = &multifd_none_ops;
>>+    if (migrate_use_multifd_zlib()) {
>>+        multifd_send_state->ops = &multifd_zlib_ops;
>>+    } else {
>>+        multifd_send_state->ops = &multifd_none_ops;
>>+    }
>
> Again, to manipulate a global variable in each thread is not a good idea.

Fixed.

> This would be better to use an array to assign ops instead of *if*. In case
> you would have several compress methods, the code would be difficult to read.

it is going to end:

   if (migrate_use_multifd_zlib()) {
       multifd_send_state->ops = &multifd_zlib_ops;
   if (migrate_use_multifd_zstd()) {
       multifd_send_state->ops = &multifd_zstd_ops;
   } else {
       multifd_send_state->ops = &multifd_none_ops;
   }

We can use:

multifd_send_state->ops = multifd_ops[migrate_multifd_method(void)];

About what is easier to read .....  it depends on taste.

Will change anyways.

Thanks, Juan.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]