Re: [Qemu-devel] [PATCH] block migration: Allow compile time disable

[Juan Quintela <quintela@redhat.com>]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
>> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> >
>> > Many users now prefer to use drive_mirror over NBD as an
>> > alternative to the older migrate -b option; drive_mirror is
>> > more complex to setup but gives you more options (e.g. only
>> > migrating some of the disks if some of them are shared).
>> >
>> > Allow the large chunk of block migration code to be compiled
>> > out for those who don't use it.
>> >
>> > Based on a downstream-patch we've had for a while by Jeff Cody.
>> >
>> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> 
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> 
>> > diff --git a/migration/migration.c b/migration/migration.c
>> > index 353f2728cf..ffce72aabc 100644
>> > --- a/migration/migration.c
>> > +++ b/migration/migration.c
>> > @@ -692,6 +692,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>> >  
>> >          populate_ram_info(info, s);
>> >  
>> > +#ifdef CONFIG_LIVE_BLOCK_MIGRATION
>> >          if (blk_mig_active()) {
>> >              info->has_disk = true;
>> >              info->disk = g_malloc0(sizeof(*info->disk));
>> > @@ -699,6 +700,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>> >              info->disk->remaining = blk_mig_bytes_remaining();
>> >              info->disk->total = blk_mig_bytes_total();
>> >          }
>> > +#endif
>> 
>> I think it can be cleaner to move this inside block.c and just export a
>> function that is empty in case it is compiled out, but that it is just
>> me, so you got the revieweb by anyways.
>> 
>> >          }
>> >      }
>> >  
>> > +#ifdef CONFIG_LIVE_BLOCK_MIGRATION
>> >      blk_mig_init();
>> > +#endif
>> >      ram_mig_init();
>> >  
>> >      /* If the currently selected machine wishes to override the units-per-bus
>> 
>> Same here, I preffer the ifdef in the header than here.
>
> OK, done I've now got the block.h header having:
> #ifdef CONFIG_LIVE_BLOCK_MIGRATION
> void blk_mig_init(void);
> int blk_mig_active(void);
> uint64_t blk_mig_bytes_transferred(void);
> uint64_t blk_mig_bytes_remaining(void);
> uint64_t blk_mig_bytes_total(void);
>
> #else
> static inline void blk_mig_init(void) { };
> static inline int blk_mig_active(void)
> {
>     return false;
> }
> static inline uint64_t blk_mig_bytes_transferred(void)
> {
>     return 0;
> }
>
> static inline uint64_t blk_mig_bytes_remaining(void)
> {
>     return 0;
> }
>
> static inline uint64_t blk_mig_bytes_total(void)
> {
>     return 0;
> }
> #endif /* CONFIG_LIVE_BLOCK_MIGRATION */
>
> Dave

good for me.  Thanks.