Re: [PATCH] multifd: Add colo support

[Juan Quintela]

Lukas Straub <lukasstraub2@web.de> wrote:
> On Thu, 11 May 2023 11:52:55 +0200
> Juan Quintela <quintela@redhat.com> wrote:
>
>> Lukas Straub <lukasstraub2@web.de> wrote:
>> > On Tue,  9 May 2023 20:15:28 +0200
>> > Juan Quintela <quintela@redhat.com> wrote:
>> >  
>> >> From: Lukas Straub <lukasstraub2@web.de>
>> >> 
>> >> Like in the normal ram_load() path, put the received pages into the
>> >> colo cache and mark the pages in the bitmap so that they will be
>> >> flushed to the guest later.
>> >> 
>> >> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
>> >> 
>> >> ---
>> >> 
>> >> Hi Lukas
>> >> 
>> >> What about this instead of your other three patches?  I think it is
>> >> clearer, and I don't think that we are going to have anything else
>> >> that is going to hook there anytime soon.
>> >> 
>> >> Notice that I put CONFIG_COLO waiting for Vladimir changes to get in
>> >> before I merge this.
>> >> 
>> >> Notice also that I "lost" the line:
>> >> 
>> >>   p->host = p->block->host;
>> >> 
>> >> In the error case.  But in that case we are aborting the migration, so
>> >> we don't care.
>> >> 
>> >> Can you check if it works for you?
>> >> Here it compiles, so it must be perfect.
>> >> 
>> >> Thanks, Juan.  
>> >
>> > This way is okay for now. Though I will send a patch.  
>> 
>> 
>> If it works for you, you can add a review-by and I will integrate in
>> next PULL.
>
> Nack, There are quite few things wrong at a glance. I will cook up my
> own patch, similar to this one.

Perfect.  Thanks.
>> >> @@ -23,7 +23,7 @@ softmmu_ss.add(files(
>> >>    'migration.c',
>> >>    'multifd.c',
>> >>    'multifd-zlib.c',
>> >> -  'multifd-zlib.c',
>
> ???

Patch sent upstream to fix it.  Introduced (by me) when I rebased/merged
your compression code changes.

>> >> diff --git a/migration/multifd-colo.h b/migration/multifd-colo.h
>> >> new file mode 100644
>> >> index 0000000000..1636c617fc
>> >> --- /dev/null
>> >> +++ b/migration/multifd-colo.h
>> >> @@ -0,0 +1,24 @@
>> >> +/*
>> >> + * multifd colo header
>> >> + *
>> >> + * Copyright (c) Lukas Straub <lukasstraub2@web.de>
>> >> + *
>> >> + * This work is licensed under the terms of the GNU GPL, version 2 or 
>> >> later.
>> >> + * See the COPYING file in the top-level directory.
>> >> + */
>> >> +
>> >> +#ifndef QEMU_MIGRATION_MULTIFD_COLO_H
>> >> +#define QEMU_MIGRATION_MULTIFD_COLO_H
>> >> +
>> >> +#ifndef CONFIG_COLO
>
> Should be CONFIG_REPLICATION

Aha, thanks.

Will wait for your patch.

Later, Juan.

>> >> +
>> >> +void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p);
>> >> +void multifd_colo_process_recv_pages(MultiFDRecvParams *p);
>> >> +
>> >> +#else
>> >> +
>> >> +static inline void multifd_colo_prepare_recv_pages(MultiFDRecvParams *p) 
>> >> {}
>> >> +static inline void multifd_colo_process_recv_pages(MultiFDRecvParams *p) 
>> >> {}
>> >> +
>> >> +#endif /* CONFIG_COLO */
>> >> +#endif  
>>