Message ID | 20170503104257.5127-1-dgilbert@redhat.com |
---|---|
State | New |
Headers | show |
"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. Later, Juan.
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 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
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.
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 (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
* 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 > Later, Juan. -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
"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.
* 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
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!
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