[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-block] [RFC PATCH 0/7] BlockBackends, nodes and g

From: John Snow
Subject: Re: [Qemu-devel] [Qemu-block] [RFC PATCH 0/7] BlockBackends, nodes and guest devices
Date: Mon, 11 Jul 2016 20:13:18 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1

No oxford comma in the subject? :)

On 06/23/2016 10:36 AM, Kevin Wolf wrote:
> I am relatively confident to say that everything that should use a
> BlockBackend, does so by now. Almost all users create their own anonymous
> BlockBackend internally and use that. The user configures the BB only
> indirectly using the configuration methods of the user that the BB belongs to.
> There is one exception, which are guest devices. There the user is expected to
> manually set up a BlockBackend and pass it to the device. This requires that
> users understand the difference between node and BlockBackends and manage both
> kinds of objects. This is a rather nasty interface.
> My goal is that we allow a user (management tool) to ignore that BlockBackends
> exist as separate entities in qemu. Ideally we could fully make them an
> implementation detail, but I'm not sure to which degree we can do that for
> compatibility reasons. But what I'm pretty sure we can do is provide 
> interfaces
> that address everything using either node names or (qdev) device names, so 
> that
> you don't have to manage BlockBackends if you don't want to.
> This involves several steps, and for most of them this series contains an
> example patch that shows what this could look like:
> 1. Accept node-name in -device drive=... and create an internal anonymous BB
>    for devices configured this way. This is the way to create devices that
>    completely avoid legacy interfaces using the BB name.
> 2. Update all QMP commands touching block devices. There are two kinds of 
> them,
>    concerning either the guest device (which the BlockBackend is logically 
> part
>    of, even though it's not implemented this way) or the actual backend
>    (BlockDriverState/node level)
>    * Device level commands: Accept a guest device ID instead of BB name to
>      identify the BlockBackend the command works on. As device IDs and BB 
> names
>      don't share a single namespace, we'll need a new QMP argument for this.
>    * Node level commands: We need to complete the conversion that makes
>      commands accept node names instead of BlockBackend names. In some places
>      we intentionally allow only BlockBackends because we don't know if the
>      command works in other places than the root. This is okay, but we can
>      accept node names anyway. We just need to check that the node is a root
>      node as expected.
> 3. Remove all BlockBackend options from blockdev-add. This has already 
> happened
>    partially (e.g. WCE flag), but at least id, rerror, werror are still there.
>    This is a very incompatible change, but we declared blockdev-add
>    experimental, so I think it's acceptable.
> 4. Add BlockBackend options as qdev properties to all block devices.
> 5. Add a way on the command line to create block nodes that have a node-name
>    and don't have a BlockBackend. blockdev-add already supports this (and 
> after
>    implementing 3. it will be the only mode supported by blockdev-add), but we
>    can't do this on the command line yet. You always get a BB with -drive.
>    This might finally become the -blockdev we were talking about at the very
>    beginning of the block layer generalisation work.
> So this is my plan. It's pretty radical, but I think we really must do
> something about our interfaces. Having nodes, BlockBackends and guest devices
> to manage is just too much and doesn't really make sense. Making BlockBackends
> visible in the external API essentially only as aliases for either node names
> or guest devices (and that only for compatibility, not when using 
> blockdev-add/
> -blockdev) feels to me like the right thing to do.
> But of course I'm aware that there probably isn't a clear right or wrong, and
> that I might be missing important details, so this needs to be discussed in
> advance before I go and implement the full thing instead of just small example
> patches.
> So please let me know what you guys think about this plan.
> Kevin Wolf (7):
>   block/qdev: Allow node name for drive properties
>   block: Add blk_by_dev()
>   qdev-monitor: Factor out find_device_state()
>   qdev-monitor: Add blk_by_qdev_id()
>   block: Accept device model name for blockdev-open/close-tray
>   block: Accept node-name for block-stream
>   block/qdev: Allow configuring WCE with qdev properties
>  block/block-backend.c            | 19 +++++++++
>  blockdev.c                       | 92 
> ++++++++++++++++++++++++++++------------
>  hw/block/block.c                 | 16 +++++++
>  hw/block/nvme.c                  |  1 +
>  hw/block/virtio-blk.c            |  1 +
>  hw/core/qdev-properties-system.c | 18 +++++++-
>  hw/ide/qdev.c                    |  1 +
>  hw/scsi/scsi-disk.c              |  1 +
>  hw/usb/dev-storage.c             |  1 +
>  include/hw/block/block.h         |  5 ++-
>  include/sysemu/block-backend.h   |  2 +
>  qapi/block-core.json             | 16 ++++---
>  qdev-monitor.c                   | 34 +++++++++++++--
>  qmp-commands.hx                  | 14 +++---
>  14 files changed, 178 insertions(+), 43 deletions(-)

1-4: Reviewed-by: John Snow <address@hidden>
5: Looks good, pending discussion on the right thing to name "ID", but
the patch itself looks perfectly cromulent.
6: Causes only a minor regression in 030 due to different error class
names, but R-B otherwise.
7: No opinion. Looks sane mechanically but I don't know enough about
core block properties to have a meaningful opinion. "ACK."

For non-RFC, some new iotests would be good.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]