Re: -drive if=none: can't we make this the default?

[Kevin Wolf <kwolf@redhat.com>]

Am 01.11.2023 um 12:21 hat Peter Maydell geschrieben:
> On Tue, 31 Oct 2023 at 18:45, Kevin Wolf <kwolf@redhat.com> wrote:
> > Am 16.10.2023 um 13:58 hat Michael Tokarev geschrieben:
> > > Almost everyone mentions -blockdev as a replacement for -drive.
> >
> > More specifically for -drive if=none. I honestly don't know many common
> > use cases for that one.
> 
> One use case for it is "create a drive with a qcow2 backend to use
> for -snapshot snapshots, but don't plug it into anything". See
> https://translatedcode.wordpress.com/2015/07/06/tricks-for-debugging-qemu-savevm-snapshots/
> I dunno whether that counts as "common", though :-)

Ok, I was already wondering what good -snapshot was for an image that
isn't even used, but what the article describes is actually not using
-snapshot, but internal snapshots with savevm/loadvm, i.e. using the
image to store the VM state.

This actually makes a lot of sense for if=none, as one of the few cases
where "none" accurately tells what device it will be used with.

> > For management tools, -blockdev is indeed what should be used, and that
> > things are more explicit there is actually a feature, not a bug, for
> > management tools.
> >
> > As a human user, in the common case where I don't care about the
> > details, I don't want to type up an explicit -device. if=virtio gives me
> > more directly what I want.
> 
> I can never remember if if=virtio is going to give me virtio-pci or
> virtio-mmio, so my scripts all explicitly create a drive with "-drive if=none"
> and a virtio-blk-pci device with "-device". I don't much mind being a
> bit long-winded in a script file.

This makes me think that if=virtio-pci/virtio-mmio would make sense.
Maybe even more generally if=<qdev-device-name> as long as it is a block
device and therefore has a 'drive' property?

if=virtio already gets translated into a -device option internally
anyway, so doing this for more device names shouldn't be that bad.

> > So you only need it when you want to specify one of the more exotic
> > -device options, which shouldn't happen that often. Well, it doesn't for
> > me anyway, other people may have other use cases. Is that your case? If
> > so, which options do you usually want to give to -device?
> >
> > > But I have to remind several issues with it:
> > >
> > > 1. While documentation has improved a lot, -blockdev is still mostly unknown
> > >    to the masses.
> >
> > And for manual human use, that's okay anyway - you probably don't want
> > to use it. But if you're writing scripts or even advanced management
> > software, then you should use it.
> >
> > (Of course, in complex cases you may have to use it manually anyway
> > because -drive has some limitations, but that should be the absolute
> > exception.)
> >
> > > 2. -blockdev is just too verbose, one have to specify a lot of parameters just to
> > >    do a simple thing which is solved with an extra parameter with -drive.
> > >    Including various backing stores/chains for qcow2 files - this is terrible for
> > >    using things manually from command line
> >
> > You don't have to specify the backing chain explicitly if you're happy
> > with the default options with which the backing files are opened.
> >
> > -blockdev options are typically a bit longer than -drive ones, but not
> > extremely. The separate -device that if=none gives you is already a
> > similar amount of extra typing.
> >
> > -drive if=virtio,file=test.qcow2
> > -drive if=none,file=test.qcow2,id=disk -device virtio-blk,drive=disk
> > -blockdev qcow2,file.driver=file,file.filename=test.qcow2,node-name=disk -device virtio-blk,drive=disk
> 
> How did -blockdev end up with a different syntax for specifying the
> ID of the drive (i.e. "node-name=foo" rather than "id=foo")
> than everything else uses?

I don't remember the details, but I believe this is historical baggage
from -drive, which already used "id" for the BlockBackend (i.e. the
whole block tree attached to a device), and then "node-name" was added
for the BlockDriverState (the individual nodes in the tree).

When later -blockdev came around and only defined nodes rather than
whole trees, "node-name" was already there and doing the right thing.

> > > 3. -blockdev does not work with -snapshot
> > >
> > > 4. Something else I forgot while typing all the above :)
> > >
> > > In my view, -blockdev is not a substitute for -drive, not at all, and it is
> > > very user-unfriendly.  This is why -drive seems to be a good trade-off between
> > > things like -hda (which is just too simplistic) and -blockdev which which is
> > > way too verbose and lacks some automatic sugar like -snapshot.
> >
> > I would agree with that, but if=none already feels a bit unfriendly.
> >
> > Honestly, I would like to throw away the existing -drive and replace it
> > with one that has a simpler implementation (as a wrapper around
> > -blockdev) and I would be happy if it gained some additional options for
> > passing through things to -device so that you don't have to bother with
> > if=none even in the more complex cases any more.
> >
> > It would be pure syntactic sugar with a similar compatibility promise as
> > in HMP (we won't break it just for fun, but we'll also not stop making
> > sensible changes just because they make things look a bit different).
> 
> You really need to make -blockdev work with -snapshot first, though.
> Pretty much none of my use cases will ever switch over to it until
> that happens.

-snapshot doesn't really make sense for -blockdev, because -blockdev
defines a single node and -snapshot implies creating a temporary overlay
which brings in two additional nodes. But the new -drive should of
course still support that. It would just translate into multiple
-blockdevs.

> Also, you can't arbitrarily change the command-line compat
> requirements because of how you've chosen to (re-)implement an
> option. That doesn't mean the current syntax is set in stone, but
> I'm pretty sure the command line isn't at the HMP "we can change
> it without deprecation" level of compat promises.

That's why we haven't done it yet, but I do think we need to change the
compat requirements for -drive before we can move on and improve its
state. Of course, there is a need for a stable interface for management
tools, but for defining block device backends, that's -blockdev and not
-drive.

The problem with -drive is that it has grown organically for so long
that nobody really understands what it's doing in detail. I can do a 90%
(or more) compatible reimplementation of it, but the problem is that I
can't tell what the remaining 10% consist of, so I can't explicitly
deprecate that functionality before doing the rewrite.

The other option would be introducing another high level option like
-disk and deprecating -drive wholesale, but I don't think that would
actually improve things for anyone. It would make the transition process
more standard, but also much more painful because it breaks the 90%,
too.

Kevin