Insufficiently documented deprecated command arguments

Insufficiently documented deprecated command arguments

[Markus Armbruster]

I went through the QAPI schema looking for deprecated stuff not
mentioned in qemu-deprecated.texi.  Here's what I found:

    Commit b33945cfff "block: Accept device model name for
    blockdev-open/close-tray" (v2.8.0) deprecated blockdev-open-tray,
    blockdev-close-tray argument @device.

    Commit 70e2cb3bd7 "block: Accept device model name for
    blockdev-change-medium" (v2.8.0) deprecated blockdev-change-medium
    argument @device.

    Commit 7a9877a026 "block: Accept device model name for
    block_set_io_throttle" (v2.8.0) deprecated block_set_io_throttle
    argument @device.

    Commit c01c214b69 "block: remove all encryption handling APIs"
    (v2.10.0) deprecated query-named-block-nodes result field
    encryption_key_missing and query-block result field
    inserted.encryption_key_missing.

    Commit c42e8742f5 "block: Use JSON null instead of "" to disable
    backing file" (v2.10.0) deprecated blockdev-add empty string
    argument @backing.

    These were missed in commit eb22aeca65 "docs: document deprecation
    policy & deprecated features in appendix" (v2.10.0).

    Commit 3c605f4074 "commit: Add top-node/base-node options" (v3.1.0)
    deprecated block-commit arguments @base and @top.

I can update qemu-deprecated.texi for these.

Now my question: I wonder whether we want to remove any of them right
away.

Re: Insufficiently documented deprecated command arguments

[Peter Krempa]

On Wed, Dec 11, 2019 at 09:12:41 +0100, Markus Armbruster wrote:
> I went through the QAPI schema looking for deprecated stuff not
> mentioned in qemu-deprecated.texi.  Here's what I found:
> 
>     Commit b33945cfff "block: Accept device model name for
>     blockdev-open/close-tray" (v2.8.0) deprecated blockdev-open-tray,
>     blockdev-close-tray argument @device.

Libvirt uses these commands only in 'blockdev' mode. We use the qom
name/path [1] as the value for @id field. We never used @device.

>     Commit 70e2cb3bd7 "block: Accept device model name for
>     blockdev-change-medium" (v2.8.0) deprecated blockdev-change-medium
>     argument @device.

Same as above.

>     Commit 7a9877a026 "block: Accept device model name for
>     block_set_io_throttle" (v2.8.0) deprecated block_set_io_throttle
>     argument @device.

This one is more complex. The command is used both in 'blockdev' and in
'drive' mode:

In 'drive' mode we pass the alias of the 'drive' as the @device
argument.

In 'blockdev' mode we pass the qom name as @id

>     Commit c01c214b69 "block: remove all encryption handling APIs"
>     (v2.10.0) deprecated query-named-block-nodes result field
>     encryption_key_missing and query-block result field
>     inserted.encryption_key_missing.

We don't extract or use the 'encryption_key_missing' at all.


>     Commit c42e8742f5 "block: Use JSON null instead of "" to disable
>     backing file" (v2.10.0) deprecated blockdev-add empty string
>     argument @backing.

This is used in 'blockdev' mode only and we always pass the JSON null or
a node name string.

>     These were missed in commit eb22aeca65 "docs: document deprecation
>     policy & deprecated features in appendix" (v2.10.0).
> 
>     Commit 3c605f4074 "commit: Add top-node/base-node options" (v3.1.0)
>     deprecated block-commit arguments @base and @top.

This command also has two modes:

In 'drive' mode we pass in path strings as @base and @ top.

In 'blockdev' mode we pass in nodenames as @base-node and @top-node.

Starting from qemu-4.2 libvirt uses 'blockdev' mode for VMs unless an SD
card is configured as we didn't convert to the '-device' approach for
those as AFAIK not everything is possible to be converted.

> I can update qemu-deprecated.texi for these.
> 
> Now my question: I wonder whether we want to remove any of them right
> away.

Feel free to delete any unused ones. I'm afraid that -drive mode will
need to be supported for a while until we can convert the sd-cards too.


[1]: The qom name used by libvirt is generated is generated here:

https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_alias.c;h=93bdcb7548757de547b2ccb01d0a2af16d5a7cc6;hb=HEAD#l224

Re: Insufficiently documented deprecated command arguments

[Kevin Wolf]

Am 11.12.2019 um 10:33 hat Peter Krempa geschrieben:
> On Wed, Dec 11, 2019 at 09:12:41 +0100, Markus Armbruster wrote:
> >     Commit 7a9877a026 "block: Accept device model name for
> >     block_set_io_throttle" (v2.8.0) deprecated block_set_io_throttle
> >     argument @device.
> 
> This one is more complex. The command is used both in 'blockdev' and in
> 'drive' mode:
> 
> In 'drive' mode we pass the alias of the 'drive' as the @device
> argument.
> 
> In 'blockdev' mode we pass the qom name as @id

Any reason you couldn't use the QOM name even in 'drive' mode for any
QEMU version that has the @id option?

> >     Commit c42e8742f5 "block: Use JSON null instead of "" to disable
> >     backing file" (v2.10.0) deprecated blockdev-add empty string
> >     argument @backing.
> 
> This is used in 'blockdev' mode only and we always pass the JSON null or
> a node name string.

Here the thing to consider might be that JSON null isn't easy to use on
the command line for manual users.

> >     These were missed in commit eb22aeca65 "docs: document deprecation
> >     policy & deprecated features in appendix" (v2.10.0).
> > 
> >     Commit 3c605f4074 "commit: Add top-node/base-node options" (v3.1.0)
> >     deprecated block-commit arguments @base and @top.
> 
> This command also has two modes:
> 
> In 'drive' mode we pass in path strings as @base and @ top.
> 
> In 'blockdev' mode we pass in nodenames as @base-node and @top-node.
> 
> Starting from qemu-4.2 libvirt uses 'blockdev' mode for VMs unless an SD
> card is configured as we didn't convert to the '-device' approach for
> those as AFAIK not everything is possible to be converted.

Hm... I guess in 'drive' mode, you stil don't assign node names, so you
actually have to rely on paths?

Kevin

Re: Insufficiently documented deprecated command arguments

[Peter Krempa]

On Wed, Dec 11, 2019 at 11:14:39 +0100, Kevin Wolf wrote:
> Am 11.12.2019 um 10:33 hat Peter Krempa geschrieben:
> > On Wed, Dec 11, 2019 at 09:12:41 +0100, Markus Armbruster wrote:
> > >     Commit 7a9877a026 "block: Accept device model name for
> > >     block_set_io_throttle" (v2.8.0) deprecated block_set_io_throttle
> > >     argument @device.
> > 
> > This one is more complex. The command is used both in 'blockdev' and in
> > 'drive' mode:
> > 
> > In 'drive' mode we pass the alias of the 'drive' as the @device
> > argument.
> > 
> > In 'blockdev' mode we pass the qom name as @id
> 
> Any reason you couldn't use the QOM name even in 'drive' mode for any
> QEMU version that has the @id option?

Honestly, I didn't want to change the existing implementation at all.

If it will help I can change that as it will be pretty easy to do.

> 
> > >     Commit c42e8742f5 "block: Use JSON null instead of "" to disable
> > >     backing file" (v2.10.0) deprecated blockdev-add empty string
> > >     argument @backing.
> > 
> > This is used in 'blockdev' mode only and we always pass the JSON null or
> > a node name string.
> 
> Here the thing to consider might be that JSON null isn't easy to use on
> the command line for manual users.
> 
> > >     These were missed in commit eb22aeca65 "docs: document deprecation
> > >     policy & deprecated features in appendix" (v2.10.0).
> > > 
> > >     Commit 3c605f4074 "commit: Add top-node/base-node options" (v3.1.0)
> > >     deprecated block-commit arguments @base and @top.
> > 
> > This command also has two modes:
> > 
> > In 'drive' mode we pass in path strings as @base and @ top.
> > 
> > In 'blockdev' mode we pass in nodenames as @base-node and @top-node.
> > 
> > Starting from qemu-4.2 libvirt uses 'blockdev' mode for VMs unless an SD
> > card is configured as we didn't convert to the '-device' approach for
> > those as AFAIK not everything is possible to be converted.
> 
> Hm... I guess in 'drive' mode, you stil don't assign node names, so you
> actually have to rely on paths?

Well, in some specific cases we could detect the node names
auto-assigned by qemu and use them instead of paths, but in my opinion
it's not worth the effort and extra code.

Re: Insufficiently documented deprecated command arguments

[Kevin Wolf]

Am 11.12.2019 um 11:51 hat Peter Krempa geschrieben:
> On Wed, Dec 11, 2019 at 11:14:39 +0100, Kevin Wolf wrote:
> > Am 11.12.2019 um 10:33 hat Peter Krempa geschrieben:
> > > On Wed, Dec 11, 2019 at 09:12:41 +0100, Markus Armbruster wrote:
> > > >     Commit 7a9877a026 "block: Accept device model name for
> > > >     block_set_io_throttle" (v2.8.0) deprecated block_set_io_throttle
> > > >     argument @device.
> > > 
> > > This one is more complex. The command is used both in 'blockdev' and in
> > > 'drive' mode:
> > > 
> > > In 'drive' mode we pass the alias of the 'drive' as the @device
> > > argument.
> > > 
> > > In 'blockdev' mode we pass the qom name as @id
> > 
> > Any reason you couldn't use the QOM name even in 'drive' mode for any
> > QEMU version that has the @id option?
> 
> Honestly, I didn't want to change the existing implementation at all.
> 
> If it will help I can change that as it will be pretty easy to do.

You can keep it for until after the blockdev dust has settled. It would
"just" be to address the deprecation since QEMU 2.8, not because of any
functional change.

> > 
> > > >     Commit c42e8742f5 "block: Use JSON null instead of "" to disable
> > > >     backing file" (v2.10.0) deprecated blockdev-add empty string
> > > >     argument @backing.
> > > 
> > > This is used in 'blockdev' mode only and we always pass the JSON null or
> > > a node name string.
> > 
> > Here the thing to consider might be that JSON null isn't easy to use on
> > the command line for manual users.
> > 
> > > >     These were missed in commit eb22aeca65 "docs: document deprecation
> > > >     policy & deprecated features in appendix" (v2.10.0).
> > > > 
> > > >     Commit 3c605f4074 "commit: Add top-node/base-node options" (v3.1.0)
> > > >     deprecated block-commit arguments @base and @top.
> > > 
> > > This command also has two modes:
> > > 
> > > In 'drive' mode we pass in path strings as @base and @ top.
> > > 
> > > In 'blockdev' mode we pass in nodenames as @base-node and @top-node.
> > > 
> > > Starting from qemu-4.2 libvirt uses 'blockdev' mode for VMs unless an SD
> > > card is configured as we didn't convert to the '-device' approach for
> > > those as AFAIK not everything is possible to be converted.
> > 
> > Hm... I guess in 'drive' mode, you stil don't assign node names, so you
> > actually have to rely on paths?
> 
> Well, in some specific cases we could detect the node names
> auto-assigned by qemu and use them instead of paths, but in my opinion
> it's not worth the effort and extra code.

Well, the question is what to do on the QEMU side then. Deprecation
should mean that we have a plan for removing the feature. If we're
planning to keep the feature indefinitely because libvirt needs it, we
might want to consider removing the deprecation notice.

(Though honestly, keeping this one indefinitely feels wrong because
using file names to identify nodes was really one of the worse ideas.)

Kevin

Re: Insufficiently documented deprecated command arguments

[Daniel P. Berrangé]

On Wed, Dec 11, 2019 at 01:24:17PM +0100, Kevin Wolf wrote:
> Am 11.12.2019 um 11:51 hat Peter Krempa geschrieben:
> > On Wed, Dec 11, 2019 at 11:14:39 +0100, Kevin Wolf wrote:
> > > Am 11.12.2019 um 10:33 hat Peter Krempa geschrieben:
> > > > On Wed, Dec 11, 2019 at 09:12:41 +0100, Markus Armbruster wrote:
> > > > >     Commit 7a9877a026 "block: Accept device model name for
> > > > >     block_set_io_throttle" (v2.8.0) deprecated block_set_io_throttle
> > > > >     argument @device.
> > > > 
> > > > This one is more complex. The command is used both in 'blockdev' and in
> > > > 'drive' mode:
> > > > 
> > > > In 'drive' mode we pass the alias of the 'drive' as the @device
> > > > argument.
> > > > 
> > > > In 'blockdev' mode we pass the qom name as @id
> > > 
> > > Any reason you couldn't use the QOM name even in 'drive' mode for any
> > > QEMU version that has the @id option?
> > 
> > Honestly, I didn't want to change the existing implementation at all.
> > 
> > If it will help I can change that as it will be pretty easy to do.
> 
> You can keep it for until after the blockdev dust has settled. It would
> "just" be to address the deprecation since QEMU 2.8, not because of any
> functional change.
> 
> > > 
> > > > >     Commit c42e8742f5 "block: Use JSON null instead of "" to disable
> > > > >     backing file" (v2.10.0) deprecated blockdev-add empty string
> > > > >     argument @backing.
> > > > 
> > > > This is used in 'blockdev' mode only and we always pass the JSON null or
> > > > a node name string.
> > > 
> > > Here the thing to consider might be that JSON null isn't easy to use on
> > > the command line for manual users.
> > > 
> > > > >     These were missed in commit eb22aeca65 "docs: document deprecation
> > > > >     policy & deprecated features in appendix" (v2.10.0).
> > > > > 
> > > > >     Commit 3c605f4074 "commit: Add top-node/base-node options" (v3.1.0)
> > > > >     deprecated block-commit arguments @base and @top.
> > > > 
> > > > This command also has two modes:
> > > > 
> > > > In 'drive' mode we pass in path strings as @base and @ top.
> > > > 
> > > > In 'blockdev' mode we pass in nodenames as @base-node and @top-node.
> > > > 
> > > > Starting from qemu-4.2 libvirt uses 'blockdev' mode for VMs unless an SD
> > > > card is configured as we didn't convert to the '-device' approach for
> > > > those as AFAIK not everything is possible to be converted.
> > > 
> > > Hm... I guess in 'drive' mode, you stil don't assign node names, so you
> > > actually have to rely on paths?
> > 
> > Well, in some specific cases we could detect the node names
> > auto-assigned by qemu and use them instead of paths, but in my opinion
> > it's not worth the effort and extra code.
> 
> Well, the question is what to do on the QEMU side then. Deprecation
> should mean that we have a plan for removing the feature. If we're
> planning to keep the feature indefinitely because libvirt needs it, we
> might want to consider removing the deprecation notice.

Ideally libvirt would stop using -drive entirely, as I hate the idea of
having to keep this around indefinitely in libvirt too. This needs QEMU
to close the last gaps wrt SD cards

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 :|

Re: Insufficiently documented deprecated command arguments

[Peter Krempa]

On Wed, Dec 11, 2019 at 12:32:10 +0000, Daniel Berrange wrote:
> On Wed, Dec 11, 2019 at 01:24:17PM +0100, Kevin Wolf wrote:
> > Am 11.12.2019 um 11:51 hat Peter Krempa geschrieben:
> > > On Wed, Dec 11, 2019 at 11:14:39 +0100, Kevin Wolf wrote:

[...]

> > > Well, in some specific cases we could detect the node names
> > > auto-assigned by qemu and use them instead of paths, but in my opinion
> > > it's not worth the effort and extra code.
> > 
> > Well, the question is what to do on the QEMU side then. Deprecation
> > should mean that we have a plan for removing the feature. If we're
> > planning to keep the feature indefinitely because libvirt needs it, we
> > might want to consider removing the deprecation notice.
> 
> Ideally libvirt would stop using -drive entirely, as I hate the idea of
> having to keep this around indefinitely in libvirt too. This needs QEMU
> to close the last gaps wrt SD cards

Yes and also give us guidance how to convert it. Looking at the code
didn't help. There's a plethora of controllers and options to configure
without clear indication what is default behaviour expected.

Trying to convert it blindly would end up worse than just ditching
support for sdcards altogehter.

Re: Insufficiently documented deprecated command arguments

[Markus Armbruster]

Peter Krempa <pkrempa@redhat.com> writes:

> On Wed, Dec 11, 2019 at 12:32:10 +0000, Daniel Berrange wrote:
>> On Wed, Dec 11, 2019 at 01:24:17PM +0100, Kevin Wolf wrote:
>> > Am 11.12.2019 um 11:51 hat Peter Krempa geschrieben:
>> > > On Wed, Dec 11, 2019 at 11:14:39 +0100, Kevin Wolf wrote:
>
> [...]
>
>> > > Well, in some specific cases we could detect the node names
>> > > auto-assigned by qemu and use them instead of paths, but in my opinion
>> > > it's not worth the effort and extra code.
>> > 
>> > Well, the question is what to do on the QEMU side then. Deprecation
>> > should mean that we have a plan for removing the feature. If we're
>> > planning to keep the feature indefinitely because libvirt needs it, we
>> > might want to consider removing the deprecation notice.
>> 
>> Ideally libvirt would stop using -drive entirely, as I hate the idea of
>> having to keep this around indefinitely in libvirt too. This needs QEMU
>> to close the last gaps wrt SD cards
>
> Yes and also give us guidance how to convert it. Looking at the code
> didn't help. There's a plethora of controllers and options to configure
> without clear indication what is default behaviour expected.

Similar situation as for if=pflash.  That one we addressed just for
"tier 1" machine types: i386 pc-*, arm virt-*.

Would addressing if=sd just for these machines suffice?

Addressing if=pflash and if=sd for all machines looks is beyond my
capacity.

For a more detailed analysis, see my "Review of onboard block device
configuration with -drive", Message-ID:
<87fti1i0oi.fsf@dusky.pond.sub.org>.  Relevant parts:

    The interface types are:
    [...]
    * if=pflash

      Many machines have onboard pflash devices.  They recognize
      bus=0,unit=U where 0 <= U < number of such onboard devices.  ARM
      machine sbsa-ref, virt, i386 machines pc*, isapc, xenfv, Risc-V
      machine virt provide machine properties for connecting backends to
      their onboard pflash devices.  For the other machines, -drive is still
      the only way to connect backends.

      PPC machines pseries* create spapr-nvram devices instead.  It can be
      created with -device instead.

      SPARC machine niagara creates a memory region instead *boggle*.
      -drive is the only way to configure that.

      Other machines create the onboard pflash device only when the
      corresponing drive is present.  Since pflash devices are not
      pluggable, -drive is the only way to create them.
    [...]
    * if=sd

      Many machines have onboard sd-card devices.  They recognize
      bus=0,unit=U where 0 <= U < number of such onboard devices.  -drive is
      the only way to connect backends.
    [...]

    Summary
    [...]
    * if=pflash, if=mtd and if=sd are blocked by the "no way to connect
      backends to onboard devices" issue, and the "no way to create the
      device" issue.  We solved them for if=pflash with some boards.  Many
      more remain.

> Trying to convert it blindly would end up worse than just ditching
> support for sdcards altogehter.

Tempting, isn't it?