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

Dr. David Alan Gilbert (git) posted 1 patch 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170503104257.5127-1-dgilbert@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
configure               | 11 +++++++++++
migration/Makefile.objs |  2 +-
migration/migration.c   | 12 ++++++++++++
vl.c                    |  2 ++
4 files changed, 26 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] block migration: Allow compile time disable
Posted by Dr. David Alan Gilbert (git) 6 years, 11 months ago
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.");
+        return;
+    }
+#endif
+
     if (migration_is_setup_or_active(s->state) ||
         s->state == MIGRATION_STATUS_CANCELLING ||
         s->state == MIGRATION_STATUS_COLO) {
diff --git a/vl.c b/vl.c
index f46e070e0d..d0fc99a41d 100644
--- a/vl.c
+++ b/vl.c
@@ -4471,7 +4471,9 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+#ifdef CONFIG_LIVE_BLOCK_MIGRATION
     blk_mig_init();
+#endif
     ram_mig_init();
 
     /* If the currently selected machine wishes to override the units-per-bus
-- 
2.12.2


Re: [Qemu-devel] [PATCH] block migration: Allow compile time disable
Posted by Kashyap Chamarthy 6 years, 11 months ago
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 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"}


[...]

-- 
/kashyap

Re: [Qemu-devel] [PATCH] block migration: Allow compile time disable
Posted by Dr. David Alan Gilbert 6 years, 11 months ago
* 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

Re: [Qemu-devel] [PATCH] block migration: Allow compile time disable
Posted by Kashyap Chamarthy 6 years, 11 months ago
On Thu, May 04, 2017 at 11:25:25AM +0100, Dr. David Alan Gilbert wrote:
> * Kashyap Chamarthy (kchamart@redhat.com) wrote:
> > On Wed, May 03, 2017 at 11:42:57AM +0100, Dr. David Alan Gilbert (git) wrote:

[...]

> > > +#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.

Fair enough.

> > 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?

No, they're not expected to know of it.  Only developers digging into
Nova Vir driver issues that trickle down to QEMU normally know about it.

-- 
/kashyap

Re: [Qemu-devel] [PATCH] block migration: Allow compile time disable
Posted by Dr. David Alan Gilbert 6 years, 11 months ago
* 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 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"}

OK, I've changed this to:

(qemu) migrate -b "exec:cat /dev/null"
QEMU compiled without old-style (blk/-b, inc/-i) block migration
Use drive_mirror+NBD instead.

(The second line being the hint).

Dave

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

Re: [Qemu-devel] [PATCH] block migration: Allow compile time disable
Posted by Eric Blake 6 years, 11 months ago
On 05/03/2017 05:42 AM, Dr. David Alan Gilbert (git) 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>
> ---
> @@ -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.");

error_setg() should not be used with '.' (it should be a single phrase,
here you are trying to stuff in two sentences).  error_append_hint() can
be used to supply the advice about using drive_mirror+NBD as the
alternative.

Otherwise this looks reasonable to me.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


Re: [Qemu-devel] [PATCH] block migration: Allow compile time disable
Posted by Dr. David Alan Gilbert 6 years, 11 months ago
* Eric Blake (eblake@redhat.com) wrote:
> On 05/03/2017 05:42 AM, Dr. David Alan Gilbert (git) 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>
> > ---
> > @@ -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.");
> 
> error_setg() should not be used with '.' (it should be a single phrase,
> here you are trying to stuff in two sentences).  error_append_hint() can
> be used to supply the advice about using drive_mirror+NBD as the
> alternative.
> 
> Otherwise this looks reasonable to me.

Done as:
        error_setg(errp, "QEMU compiled without old-style block migration");
        error_append_hint(errp, "Use drive_mirror+NBD.\n");

(All the places I can see seem to have . and \n in append_hint)

Dave

> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 
> 



--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] block migration: Allow compile time disable
Posted by Eric Blake 6 years, 11 months ago
On 05/15/2017 07:13 AM, Dr. David Alan Gilbert wrote:

>>> +#ifndef CONFIG_LIVE_BLOCK_MIGRATION
>>> +    if (params.blk || params.shared) {
>>> +        error_setg(errp, "QEMU compiled without old-style block migration. "
>>> +                         "Use drive_mirror+NBD.");
>>
>> error_setg() should not be used with '.' (it should be a single phrase,
>> here you are trying to stuff in two sentences).  error_append_hint() can
>> be used to supply the advice about using drive_mirror+NBD as the
>> alternative.
>>
>> Otherwise this looks reasonable to me.
> 
> Done as:
>         error_setg(errp, "QEMU compiled without old-style block migration");
>         error_append_hint(errp, "Use drive_mirror+NBD.\n");
> 
> (All the places I can see seem to have . and \n in append_hint)

That's correct. The hint has to supply its own newline, because we have
places where we construct the hint through several append calls in a row
(only the last one adds the newline).  Looks good to me!

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org