[PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands
The block migration is considered obsolete and has been deprecated in 8.2. Remove the migrate command option that enables it. This only affects the QMP and HMP commands, the feature can still be accessed by setting the migration 'block' capability. The whole feature will be removed in a future patch. Deprecation commit 8846b5bfca ("migration: migrate 'blk' command option is deprecated."). Signed-off-by: Fabiano Rosas --- .gitlab-ci.d/buildtest.yml | 2 +- docs/about/deprecated.rst| 9 -- docs/about/removed-features.rst | 14 +++ migration/migration-hmp-cmds.c | 9 +- migration/migration.c| 31 ++- migration/options.c | 3 +- qapi/migration.json | 8 -- tests/qemu-iotests/183 | 147 --- tests/qemu-iotests/183.out | 66 -- tests/qemu-iotests/common.filter | 7 -- 10 files changed, 22 insertions(+), 274 deletions(-) delete mode 100755 tests/qemu-iotests/183 delete mode 100644 tests/qemu-iotests/183.out diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index 6394b8f41e..3d975f8c34 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -342,7 +342,7 @@ build-tcg-disabled: - cd tests/qemu-iotests/ - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048 052 063 077 086 101 104 106 113 148 150 151 152 157 159 160 163 -170 171 183 184 192 194 208 221 226 227 236 253 277 image-fleecing +170 171 184 192 194 208 221 226 227 236 253 277 image-fleecing - ./check -qcow2 028 051 056 057 058 065 068 082 085 091 095 096 102 122 124 132 139 142 144 145 151 152 155 157 165 194 196 200 202 208 209 216 218 227 234 246 247 248 250 254 255 257 258 diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index ebe53821ed..d739358bb1 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -468,15 +468,6 @@ option). Migration - -``blk`` migrate command option (since 8.2) -'' - -Use blockdev-mirror with NBD instead. - -As an intermediate step the ``blk`` functionality can be achieved by -setting the ``block`` migration capability to ``true``. But this -capability is also deprecated. - block migration (since 8.2) ''' diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 7da4b3df14..a491c0 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -627,6 +627,13 @@ Use blockdev-mirror with NBD instead. See "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst for a detailed explanation. +``migrate`` command option ``blk`` (removed in 9.1) +''' + +Use blockdev-mirror with NBD instead. See "QMP invocation for live +storage migration with ``blockdev-mirror`` + NBD" in +docs/interop/live-block-operations.rst for a detailed explanation. + Human Monitor Protocol (HMP) commands - @@ -694,6 +701,13 @@ Use blockdev-mirror with NBD instead. See "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst for a detailed explanation. +``migrate`` command ``-b`` option (removed in 9.1) +'' + +Use blockdev-mirror with NBD instead. See "QMP invocation for live +storage migration with ``blockdev-mirror`` + NBD" in +docs/interop/live-block-operations.rst for a detailed explanation. + Host Architectures -- diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index f49f061be1..5ab204d91d 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -758,26 +758,19 @@ static void hmp_migrate_status_cb(void *opaque) void hmp_migrate(Monitor *mon, const QDict *qdict) { bool detach = qdict_get_try_bool(qdict, "detach", false); -bool blk = qdict_get_try_bool(qdict, "blk", false); bool resume = qdict_get_try_bool(qdict, "resume", false); const char *uri = qdict_get_str(qdict, "uri"); Error *err = NULL; g_autoptr(MigrationChannelList) caps = NULL; g_autoptr(MigrationChannel) channel = NULL; -if (blk) { -warn_report("option '-b' is deprecated;" -" use blockdev-mirror with NBD instead"); -} - if (!migrate_uri_parse(uri, &channel, &err)) { hmp_handle_error(mon, err); return; } QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel)); -qmp_migrate(NULL, true, caps, !!blk, blk, false, false, -true, resume, &err); +qmp_migrate(NULL, true, caps, false, false, true, resume, &err); if (hmp_handle_error(mon, err)) { return; } diff --git a/migration/migration.c b/migr
Re: [PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands
On Fri, Apr 26, 2024 at 10:14:05AM -0300, Fabiano Rosas wrote: > @@ -2003,21 +1997,7 @@ static bool migrate_prepare(MigrationState *s, bool > blk, bool resume, > } > } > > -if (blk) { > -if (migrate_colo()) { > -error_setg(errp, "No disk migration is required in COLO mode"); > -return false; > -} > -if (migrate_block()) { > -error_setg(errp, "Command options are incompatible with " > - "current migration capabilities"); > -return false; > -} > -if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, errp)) { > -return false; > -} > -s->must_remove_block_options = true; > -} > +s->must_remove_block_options = true; Can we drop this var too? Perhaps with block_cleanup_parameters()? -- Peter Xu ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands
Peter Xu writes: > On Fri, Apr 26, 2024 at 10:14:05AM -0300, Fabiano Rosas wrote: >> @@ -2003,21 +1997,7 @@ static bool migrate_prepare(MigrationState *s, bool >> blk, bool resume, >> } >> } >> >> -if (blk) { >> -if (migrate_colo()) { >> -error_setg(errp, "No disk migration is required in COLO mode"); >> -return false; >> -} >> -if (migrate_block()) { >> -error_setg(errp, "Command options are incompatible with " >> - "current migration capabilities"); >> -return false; >> -} >> -if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, errp)) { >> -return false; >> -} >> -s->must_remove_block_options = true; >> -} >> +s->must_remove_block_options = true; > > Can we drop this var too? Perhaps with block_cleanup_parameters()? Yes, Markus mentioned it in v1 already. Take a look there. There's several other declarations I missed. v3 is coming soon. ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands
On Mon, Apr 29, 2024 at 02:18:57PM -0300, Fabiano Rosas wrote: > Peter Xu writes: > > > On Fri, Apr 26, 2024 at 10:14:05AM -0300, Fabiano Rosas wrote: > >> @@ -2003,21 +1997,7 @@ static bool migrate_prepare(MigrationState *s, bool > >> blk, bool resume, > >> } > >> } > >> > >> -if (blk) { > >> -if (migrate_colo()) { > >> -error_setg(errp, "No disk migration is required in COLO > >> mode"); > >> -return false; > >> -} > >> -if (migrate_block()) { > >> -error_setg(errp, "Command options are incompatible with " > >> - "current migration capabilities"); > >> -return false; > >> -} > >> -if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, errp)) { > >> -return false; > >> -} > >> -s->must_remove_block_options = true; > >> -} > >> +s->must_remove_block_options = true; > > > > Can we drop this var too? Perhaps with block_cleanup_parameters()? > > Yes, Markus mentioned it in v1 already. Take a look there. There's > several other declarations I missed. v3 is coming soon. Right, noticed that it's removed actually in the next patch. But iiuc it can already been removed in this patch. If we want to remove it in the next, logically we should set must_remove_block_options=false here, though.. So maybe easier to just drop it here. -- Peter Xu ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands
Peter Xu writes: > On Mon, Apr 29, 2024 at 02:18:57PM -0300, Fabiano Rosas wrote: >> Peter Xu writes: >> >> > On Fri, Apr 26, 2024 at 10:14:05AM -0300, Fabiano Rosas wrote: >> >> @@ -2003,21 +1997,7 @@ static bool migrate_prepare(MigrationState *s, >> >> bool blk, bool resume, >> >> } >> >> } >> >> >> >> -if (blk) { >> >> -if (migrate_colo()) { >> >> -error_setg(errp, "No disk migration is required in COLO >> >> mode"); >> >> -return false; >> >> -} >> >> -if (migrate_block()) { >> >> -error_setg(errp, "Command options are incompatible with " >> >> - "current migration capabilities"); >> >> -return false; >> >> -} >> >> -if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, errp)) { >> >> -return false; >> >> -} >> >> -s->must_remove_block_options = true; >> >> -} >> >> +s->must_remove_block_options = true; >> > >> > Can we drop this var too? Perhaps with block_cleanup_parameters()? >> >> Yes, Markus mentioned it in v1 already. Take a look there. There's >> several other declarations I missed. v3 is coming soon. > > Right, noticed that it's removed actually in the next patch. > > But iiuc it can already been removed in this patch. If we want to remove > it in the next, logically we should set must_remove_block_options=false > here, though.. So maybe easier to just drop it here. Ah I see what you mean. I thought you're just asking for the removal overall. But block_cleanup_parameters sets the block capability to false and the whole block migration only goes away in the next patch. I think we need to keep this as true to preserve behavior. In theory, after this patch people could still use the block migration just fine by setting the cap. ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands
On Mon, Apr 29, 2024 at 03:35:02PM -0300, Fabiano Rosas wrote: > Peter Xu writes: > > > On Mon, Apr 29, 2024 at 02:18:57PM -0300, Fabiano Rosas wrote: > >> Peter Xu writes: > >> > >> > On Fri, Apr 26, 2024 at 10:14:05AM -0300, Fabiano Rosas wrote: > >> >> @@ -2003,21 +1997,7 @@ static bool migrate_prepare(MigrationState *s, > >> >> bool blk, bool resume, > >> >> } > >> >> } > >> >> > >> >> -if (blk) { [1] > >> >> -if (migrate_colo()) { > >> >> -error_setg(errp, "No disk migration is required in COLO > >> >> mode"); > >> >> -return false; > >> >> -} > >> >> -if (migrate_block()) { > >> >> -error_setg(errp, "Command options are incompatible with " > >> >> - "current migration capabilities"); > >> >> -return false; > >> >> -} > >> >> -if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, errp)) { > >> >> -return false; > >> >> -} > >> >> -s->must_remove_block_options = true; > >> >> -} > >> >> +s->must_remove_block_options = true; > >> > > >> > Can we drop this var too? Perhaps with block_cleanup_parameters()? > >> > >> Yes, Markus mentioned it in v1 already. Take a look there. There's > >> several other declarations I missed. v3 is coming soon. > > > > Right, noticed that it's removed actually in the next patch. > > > > But iiuc it can already been removed in this patch. If we want to remove > > it in the next, logically we should set must_remove_block_options=false > > here, though.. So maybe easier to just drop it here. > > Ah I see what you mean. I thought you're just asking for the removal > overall. > > But block_cleanup_parameters sets the block capability to false and the > whole block migration only goes away in the next patch. I think we need > to keep this as true to preserve behavior. In theory, after this patch > people could still use the block migration just fine by setting the cap. I'm reading this patch the same as "blk==false" always above [1]. In that case, only if we keep must_remove_block_options=false could we maintain the behavior? Otherwise at the end of migration (or cancellations) QEMU can wrongly clear MIGRATION_CAPABILITY_BLOCK bit if the user set it manually? (But hey, we're discussing whoever applies this patch only without the rest.. definitely not a huge deal :) -- Peter Xu ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org