Re: [PATCH v7 0/4] Migration deprecated parts

[Juan Quintela]

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(-)