[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
