[libvirt] [Qemu-devel] [PATCH 1/2] qapi: deprecate drive-mirror and drive-backup

[Peter Krempa]

On Wed, Aug 14, 2019 at 15:22:15 -0400, John Snow wrote:
> 
> 
> On 8/14/19 6:07 AM, Vladimir Sementsov-Ogievskiy wrote:
> > It's hard and not necessary to maintain outdated versions of these
> > commands.
> > 
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov at virtuozzo.com>
> > ---
> >  qemu-deprecated.texi  |  4 ++++
> >  qapi/block-core.json  |  4 ++++
> >  qapi/transaction.json |  2 +-
> >  blockdev.c            | 10 ++++++++++
> >  4 files changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> > index fff07bb2a3..2753fafd0b 100644
> > --- a/qemu-deprecated.texi
> > +++ b/qemu-deprecated.texi
> > @@ -179,6 +179,10 @@ and accurate ``query-qmp-schema'' command.
> >  Character devices creating sockets in client mode should not specify
> >  the 'wait' field, which is only applicable to sockets in server mode
> >  
> > + at subsection drive-mirror, drive-backup and drive-backup transaction action (since 4.2)
> > +
> > +Use blockdev-mirror and blockdev-backup instead.
> > +
> >  @section Human Monitor Protocol (HMP) commands
> >  
> >  @subsection The hub_id parameter of 'hostfwd_add' / 'hostfwd_remove' (since 3.1)

[...]

> > @@ -3831,6 +3838,9 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
> >      const char *format = arg->format;
> >      int ret;
> >  
> > +    warn_report("drive-mirror command is deprecated and will disappear in "
> > +                "future. Use blockdev-mirror instead");
> > +
> >      bs = qmp_get_root_bs(arg->device, errp);
> >      if (!bs) {
> >          return;
> > 
> 
> Hm!
> 
> I wonder if this is ever-so-slightly too soon for our friends over at
> the libvirt project.
> 
> I don't think they have fully moved away from the non-blockdev
> interfaces *just yet*, and I might encourage seeing the first full
> libvirt release that does support and use it before we start the
> deprecation clock.
> 
> (Juuuust in case.)
> 
> That's just me being very, very cautious though.
> 
> Peter Krempa, how do you feel about this?

Thanks for the heads up!

Currently libvirt does not use 'drive-backup' at all so that one can be
deprecated immediately.

In case of 'drive-mirror' the situation is a bit more complex:

Libvirt uses 'drive-mirror' currently in the following places

1) virDomainBlockCopy API
With blockdev integration enabled this will go away. Pathces are being
reviewed:

https://www.redhat.com/archives/libvir-list/2019-August/msg00295.html

2) VM migration with non-shared storage
Currently uses 'drive-mirror' in most cases but there is pre-existing
integration for blockdev-mirror for nbd+tls. I need to make sure that
the blockdev version will be used unconditionally once the integration
is enabled. This is a TODO.

There is also one gotcha. In case when an 'sd' card device is used for
the VM, libvirt disables all of blockdev, because SD cards can't be
expressed with blockdev. There's too many code paths which would need
checking to be worth it. To be fair, I'm not even sure when a sd card
can be emulated by qemu as all of my basic tests failed and I did not
care more.

For libvirt to enable blockdev there's one more part missing and that's
snapshot integration. I'm currently testing patches to integrate it with
external snapshots, which should be posted soon.

I also found a bug in qemu, which prevents creation of internal
snapshots when -blockdev is used:

When savevm HMP command is used (via QMP->HMP bridge) qemu invokes
save_snapshot(), which calls bdrv_all_can_snapshot(). That function uses
bdrv_next() to iterate all nodes which correspond to a block backend
first, but then also iterates any other node which is monitor-owned.

Since with blockdev all nodes including the ones for the 'file' protocol
are monitor-owned, and 'file' does not support snapshots that check
fails. A simple hack of skipping the second part in bdrv_next() allows
to do a snapshot actually. Kevin told me that the idea is that also
non-attached nodes should be considered for internal snapshod which is
okay in my opinion, but given how the snapshot works for the files
attached to backeds (and also in pre-blockdev use) only the top level of
a chain should ever be considered for snapshot.

So the summary is, that I'm pretty hopeful that we should be able to get
rid of all reasonable uses of drive-mirror very soon after I finish
snapshot integration. The only question is how much
we care about SD card users being able to do a drive-mirror in the
future.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190815/56ab350b/attachment-0001.sig>