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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Kashyap Chamarthy (kchamart@redhat.com) wrote:
> On Wed, May 03, 2017 at 11:42:57AM +0100, Dr. David Alan Gilbert (git) wrote:
> 
> A small comment inline, in the 'ifndef' section.
> 
> > 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>
> > ---
> >  configure               | 11 +++++++++++
> >  migration/Makefile.objs |  2 +-
> >  migration/migration.c   | 12 ++++++++++++
> >  vl.c                    |  2 ++
> >  4 files changed, 26 insertions(+), 1 deletion(-)
> > 
> > diff --git a/configure b/configure
> > index 48a9370cc6..69eed5fb8d 100755
> > --- a/configure
> > +++ b/configure
> > @@ -316,6 +316,7 @@ vte=""
> >  virglrenderer=""
> >  tpm="yes"
> >  libssh2=""
> > +live_block_migration="yes"
> >  numa=""
> >  tcmalloc="no"
> >  jemalloc="no"
> > @@ -1168,6 +1169,10 @@ for opt do
> >    ;;
> >    --enable-libssh2) libssh2="yes"
> >    ;;
> > +  --disable-live-block-migration) live_block_migration="no"
> > +  ;;
> > +  --enable-live-block-migration) live_block_migration="yes"
> > +  ;;
> >    --disable-numa) numa="no"
> >    ;;
> >    --enable-numa) numa="yes"
> > @@ -1400,6 +1405,7 @@ disabled with --disable-FEATURE, default is enabled if available:
> >    libnfs          nfs support
> >    smartcard       smartcard support (libcacard)
> >    libusb          libusb (for usb passthrough)
> > +  live-block-migration   Block migration in the main migration stream
> >    usb-redir       usb network redirection support
> >    lzo             support of lzo compression library
> >    snappy          support of snappy compression library
> > @@ -5210,6 +5216,7 @@ echo "TPM support       $tpm"
> >  echo "libssh2 support   $libssh2"
> >  echo "TPM passthrough   $tpm_passthrough"
> >  echo "QOM debugging     $qom_cast_debug"
> > +echo "Live block migration $live_block_migration"
> >  echo "lzo support       $lzo"
> >  echo "snappy support    $snappy"
> >  echo "bzip2 support     $bzip2"
> > @@ -5776,6 +5783,10 @@ if test "$libssh2" = "yes" ; then
> >    echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak
> >  fi
> >  
> > +if test "$live_block_migration" = "yes" ; then
> > +  echo "CONFIG_LIVE_BLOCK_MIGRATION=y" >> $config_host_mak
> > +fi
> > +
> >  # USB host support
> >  if test "$libusb" = "yes"; then
> >    echo "HOST_USB=libusb legacy" >> $config_host_mak
> > diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> > index 480dd493a9..200b5e0c67 100644
> > --- a/migration/Makefile.objs
> > +++ b/migration/Makefile.objs
> > @@ -9,5 +9,5 @@ common-obj-y += qjson.o
> >  
> >  common-obj-$(CONFIG_RDMA) += rdma.o
> >  
> > -common-obj-y += block.o
> > +common-obj-$(CONFIG_LIVE_BLOCK_MIGRATION) += block.o
> >  
> > 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
> >  
> >          if (cpu_throttle_active()) {
> >              info->has_cpu_throttle_percentage = true;
> > @@ -720,6 +722,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));
> > @@ -727,6 +730,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >              info->disk->remaining = blk_mig_bytes_remaining();
> >              info->disk->total = blk_mig_bytes_total();
> >          }
> > +#endif
> >  
> >          get_xbzrle_cache_stats(info);
> >          break;
> > @@ -1222,6 +1226,14 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >      params.blk = has_blk && blk;
> >      params.shared = has_inc && inc;
> >  
> > +#ifndef CONFIG_LIVE_BLOCK_MIGRATION
> > +    if (params.blk || params.shared) {
> > +        error_setg(errp, "QEMU compiled without old-style block migration. "
> > +                         "Use drive_mirror+NBD.");
> 
> Is it worth spelling out briefly what the "old-style block migration"
> is?  Something like:
> 
>     "QEMU compiled without old-style (i.e. QMP `migrate` with
>     "inc":true) block migration. Use `drive-mirror`+NBD")
>  
> But I also wonder if it's needlessly wordy, so your call to incorporate
> it or not.

I was trying to find a short way to say it, and that's the closest I got to.

> I spelled out the QMP equivalent (as opposed to HMP: 'migrate -b')
> because, that's what users of higher layers (libvirt, OpenStack etc) see
> in their QMP interactions with QEMU, when the old-style approach is
> used:
> 
>     {"execute":"migrate","arguments":{{"detach":true,"blk":false,"inc":true,"uri":"fd:migrate"}

But would openstack users even know about QMP?

Dave

> 
> [...]
> 
> -- 
> /kashyap
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK