docs/about/deprecated.rst | 35 +++++++++++++ qapi/migration.json | 93 ++++++++++++++++++++++++++-------- migration/block.c | 3 ++ migration/migration-hmp-cmds.c | 10 ++++ migration/migration.c | 10 ++++ migration/options.c | 22 +++++++- tests/qemu-iotests/183.out | 2 + 7 files changed, 152 insertions(+), 23 deletions(-)
Based on: Message-ID: <20231018100651.32674-1-quintela@redhat.com>
[PULL 00/11] Migration 20231018 patches
And here we are, at v7:
- drop black line at the end of deprecated.rst
- change qemu-iotest output due to warnings for deprecation.
The only real change is the output of the qemu-iotest. That is the
reason why I maintained the reviewed-by. But will be happy if anyone
of the block people ack the changes.
Thanks, Juan.
On this v6:
- Fixed Markus comments
- 1st patch is reviewed
- dropped the RFC ones.
Later, Juan.
On this v5:
- Rebased on top of last migration pull requesnt:
- address markus comments. Basically we recommend always
blockdev-mirror + NBD. In deprecated.rst we also put the posiblity
of using block-incremental and block, but we state that they are
also deprecated.
I know, I know, I deprecated them in the following patch.
- Dropped the removal of block-migration and block-incremental I am
only interested in showing why I want to remove the -b/-i options.
Please review.
Later, Juan.
On this v4:
- addressed all markus comments.
- rebased on latest.
- improve formatting of migration.json
- print block migration status when needed.
- patches 7-10 are not mean to merge, they just show why we want to
deprecate block migration and remove its support.
- Patch 7 just drop support for -i/-b and qmp equivalents.
- Patch 8 shows all the helpers and convolutions we need to have to
support that -i and -d.
- patch 9 drops block-incremental migration support.
- patch 9 drops block migration support.
Please review.
Thanks, Juan.
On this v3:
- Rebase on top of upstream.
- Changed v8.1 to 8.2 (I left the reviewed by anyways)
- missing the block deprecation code, please.
Please, review.
Later, Juan.
On this v2:
- dropped -incoming <uri> deprecation
Paolo came with a better solution using keyvalues.
- skipped field is already ready for next pull request, so dropped.
- dropped the RFC bits, nermal PATCH.
- Assessed all the review comments.
- Added indentation of migration.json.
- Used the documentation pointer to substitute block migration.
Please review.
[v1]
Hi this series describe the migration parts that have to be deprecated.
- It is an rfc because I doubt that I did the deprecation process right. Hello Markus O:-)
- skipped field: It is older than me, I have never know what it stands
for. As far as I know it has always been zero.
- inc/blk migrate command options. They are only used by block
migration (that I deprecate on the following patch). And they are really bad.
grep must_remove_block_options.
- block migration. block jobs, whatever they are called this week are
way more flexible. Current code works, but we broke it here and
there, and really nobody has stand up to maintain it. It is quite
contained and can be left there. Is anyone really using it?
- old compression method. It don't work. See last try from Lukas to
make a test that works reliabely. I failed with the same task years
ago. It is really slow, and if compression is good for you, multifd
+ zlib is going to perform/compress way more.
I don't know what to do with this code, really.
* Remove it for this release? It don't work, and haven't work
reliabely in quite a few time.
* Deprecate it and remove in another couple of releases, i.e. normal
deprecation.
* Ideas?
- -incoming <uri>
if you need to set parameters (multifd cames to mind, and preempt has
the same problem), you really needs to use defer. So what should we do here?
This part is not urget, because management apps have a working
option that are already using "defer", and the code simplifacation
if we remove it is not so big. So we can leave it until 9.0 or
whatever we think fit.
What do you think?
Later, Juan.
Juan Quintela (4):
migration: migrate 'inc' command option is deprecated.
migration: migrate 'blk' command option is deprecated.
migration: Deprecate block migration
migration: Deprecate old compression method
docs/about/deprecated.rst | 35 +++++++++++++
qapi/migration.json | 93 ++++++++++++++++++++++++++--------
migration/block.c | 3 ++
migration/migration-hmp-cmds.c | 10 ++++
migration/migration.c | 10 ++++
migration/options.c | 22 +++++++-
tests/qemu-iotests/183.out | 2 +
7 files changed, 152 insertions(+), 23 deletions(-)
--
2.41.0
Juan Quintela <quintela@redhat.com> wrote:
> Based on: Message-ID: <20231018100651.32674-1-quintela@redhat.com>
> [PULL 00/11] Migration 20231018 patches
>
> And here we are, at v7:
> - drop black line at the end of deprecated.rst
> - change qemu-iotest output due to warnings for deprecation.
>
> The only real change is the output of the qemu-iotest. That is the
> reason why I maintained the reviewed-by. But will be happy if anyone
> of the block people ack the changes.
I forgot to include the link to the CI of the previous failure.
https://gitlab.com/juan.quintela/qemu/-/jobs/5314070229
tput mismatch (see /builds/juan.quintela/qemu/build/tests/qemu-iotests/scratch/raw-file-183/183.out.bad)
--- /builds/juan.quintela/qemu/tests/qemu-iotests/183.out
+++ /builds/juan.quintela/qemu/build/tests/qemu-iotests/scratch/raw-file-183/183.out.bad
@@ -28,6 +28,8 @@
{ 'execute': 'migrate',
'arguments': { 'uri': 'unix:SOCK_DIR/migrate', 'blk': true } }
+warning: parameter 'blk is deprecated; use blockdev-mirror with NBD instead
+warning: block migration is deprecated; use blockdev-mirror with NBD instead
{"return": {}}
{ 'execute': 'query-status' }
{"return": {"status": "postmigrate", "singlestep": false, "running":
false}}
>
> Thanks, Juan.
>
> On this v6:
> - Fixed Markus comments
> - 1st patch is reviewed
> - dropped the RFC ones.
>
> Later, Juan.
>
> On this v5:
> - Rebased on top of last migration pull requesnt:
>
> - address markus comments. Basically we recommend always
> blockdev-mirror + NBD. In deprecated.rst we also put the posiblity
> of using block-incremental and block, but we state that they are
> also deprecated.
> I know, I know, I deprecated them in the following patch.
>
> - Dropped the removal of block-migration and block-incremental I am
> only interested in showing why I want to remove the -b/-i options.
>
> Please review.
>
> Later, Juan.
>
> On this v4:
> - addressed all markus comments.
> - rebased on latest.
> - improve formatting of migration.json
> - print block migration status when needed.
> - patches 7-10 are not mean to merge, they just show why we want to
> deprecate block migration and remove its support.
> - Patch 7 just drop support for -i/-b and qmp equivalents.
> - Patch 8 shows all the helpers and convolutions we need to have to
> support that -i and -d.
> - patch 9 drops block-incremental migration support.
> - patch 9 drops block migration support.
>
> Please review.
>
> Thanks, Juan.
>
> On this v3:
>
> - Rebase on top of upstream.
> - Changed v8.1 to 8.2 (I left the reviewed by anyways)
> - missing the block deprecation code, please.
>
> Please, review.
>
> Later, Juan.
>
> On this v2:
>
> - dropped -incoming <uri> deprecation
> Paolo came with a better solution using keyvalues.
>
> - skipped field is already ready for next pull request, so dropped.
>
> - dropped the RFC bits, nermal PATCH.
>
> - Assessed all the review comments.
>
> - Added indentation of migration.json.
>
> - Used the documentation pointer to substitute block migration.
>
> Please review.
>
> [v1]
> Hi this series describe the migration parts that have to be deprecated.
>
> - It is an rfc because I doubt that I did the deprecation process right. Hello Markus O:-)
>
> - skipped field: It is older than me, I have never know what it stands
> for. As far as I know it has always been zero.
>
> - inc/blk migrate command options. They are only used by block
> migration (that I deprecate on the following patch). And they are really bad.
> grep must_remove_block_options.
>
> - block migration. block jobs, whatever they are called this week are
> way more flexible. Current code works, but we broke it here and
> there, and really nobody has stand up to maintain it. It is quite
> contained and can be left there. Is anyone really using it?
>
> - old compression method. It don't work. See last try from Lukas to
> make a test that works reliabely. I failed with the same task years
> ago. It is really slow, and if compression is good for you, multifd
> + zlib is going to perform/compress way more.
>
> I don't know what to do with this code, really.
>
> * Remove it for this release? It don't work, and haven't work
> reliabely in quite a few time.
>
> * Deprecate it and remove in another couple of releases, i.e. normal
> deprecation.
>
> * Ideas?
>
> - -incoming <uri>
>
> if you need to set parameters (multifd cames to mind, and preempt has
> the same problem), you really needs to use defer. So what should we do here?
>
> This part is not urget, because management apps have a working
> option that are already using "defer", and the code simplifacation
> if we remove it is not so big. So we can leave it until 9.0 or
> whatever we think fit.
>
> What do you think?
>
> Later, Juan.
>
> Juan Quintela (4):
> migration: migrate 'inc' command option is deprecated.
> migration: migrate 'blk' command option is deprecated.
> migration: Deprecate block migration
> migration: Deprecate old compression method
>
> docs/about/deprecated.rst | 35 +++++++++++++
> qapi/migration.json | 93 ++++++++++++++++++++++++++--------
> migration/block.c | 3 ++
> migration/migration-hmp-cmds.c | 10 ++++
> migration/migration.c | 10 ++++
> migration/options.c | 22 +++++++-
> tests/qemu-iotests/183.out | 2 +
> 7 files changed, 152 insertions(+), 23 deletions(-)
On Wed, Oct 18, 2023 at 12:38:10PM +0200, Juan Quintela wrote:
> Juan Quintela <quintela@redhat.com> wrote:
> > Based on: Message-ID: <20231018100651.32674-1-quintela@redhat.com>
> > [PULL 00/11] Migration 20231018 patches
> >
> > And here we are, at v7:
> > - drop black line at the end of deprecated.rst
> > - change qemu-iotest output due to warnings for deprecation.
> >
> > The only real change is the output of the qemu-iotest. That is the
> > reason why I maintained the reviewed-by. But will be happy if anyone
> > of the block people ack the changes.
>
> I forgot to include the link to the CI of the previous failure.
>
> https://gitlab.com/juan.quintela/qemu/-/jobs/5314070229
>
> tput mismatch (see /builds/juan.quintela/qemu/build/tests/qemu-iotests/scratch/raw-file-183/183.out.bad)
> --- /builds/juan.quintela/qemu/tests/qemu-iotests/183.out
> +++ /builds/juan.quintela/qemu/build/tests/qemu-iotests/scratch/raw-file-183/183.out.bad
> @@ -28,6 +28,8 @@
> { 'execute': 'migrate',
> 'arguments': { 'uri': 'unix:SOCK_DIR/migrate', 'blk': true } }
> +warning: parameter 'blk is deprecated; use blockdev-mirror with NBD instead
> +warning: block migration is deprecated; use blockdev-mirror with NBD instead
> {"return": {}}
> { 'execute': 'query-status' }
> {"return": {"status": "postmigrate", "singlestep": false, "running":
> false}}
IIUC, the JSON bits are being written on stdout, and the warnings
are being written on stderr. The interleaving of the data is
potentially going to be non-deterministic in the .out file.
Generally you'd want a filter in the iotests that culls the
warning: lines to avoid this mixing of stdout/err streams.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Oct 18, 2023 at 12:38:10PM +0200, Juan Quintela wrote:
>> Juan Quintela <quintela@redhat.com> wrote:
>> > Based on: Message-ID: <20231018100651.32674-1-quintela@redhat.com>
>> > [PULL 00/11] Migration 20231018 patches
>> >
>> > And here we are, at v7:
>> > - drop black line at the end of deprecated.rst
>> > - change qemu-iotest output due to warnings for deprecation.
>> >
>> > The only real change is the output of the qemu-iotest. That is the
>> > reason why I maintained the reviewed-by. But will be happy if anyone
>> > of the block people ack the changes.
>>
>> I forgot to include the link to the CI of the previous failure.
>>
>> https://gitlab.com/juan.quintela/qemu/-/jobs/5314070229
>>
>> tput mismatch (see /builds/juan.quintela/qemu/build/tests/qemu-iotests/scratch/raw-file-183/183.out.bad)
>> --- /builds/juan.quintela/qemu/tests/qemu-iotests/183.out
>> +++ /builds/juan.quintela/qemu/build/tests/qemu-iotests/scratch/raw-file-183/183.out.bad
>> @@ -28,6 +28,8 @@
>> { 'execute': 'migrate',
>> 'arguments': { 'uri': 'unix:SOCK_DIR/migrate', 'blk': true } }
>> +warning: parameter 'blk is deprecated; use blockdev-mirror with NBD instead
>> +warning: block migration is deprecated; use blockdev-mirror with NBD instead
>> {"return": {}}
>> { 'execute': 'query-status' }
>> {"return": {"status": "postmigrate", "singlestep": false, "running":
>> false}}
>
> IIUC, the JSON bits are being written on stdout, and the warnings
> are being written on stderr. The interleaving of the data is
> potentially going to be non-deterministic in the .out file.
> Generally you'd want a filter in the iotests that culls the
> warning: lines to avoid this mixing of stdout/err streams.
Thanks.
So here I am, going to v8 to create filters.
Later, Juan.
© 2016 - 2024 Red Hat, Inc.