Re: [Qemu-devel] Review of monitor commands identifying BDS / BB by name

[Fam Zheng <famz@redhat.com>]

On Tue, 12/02 20:06, Markus Armbruster wrote:
> == block-core.json ==
> 
> * block-commit
> 
>   @device names a backend, @top and @base each name one of its nodes via
>   file name matching.
> 
>   TODO: support specifying the two nodes via node name.
> 
>   Why do we need @device when a top node is specified?
> 
>   * We currently need the backend to set op blockers, and finding a
>     node's backend isn't easy.  Implementation detail shows through the
>     interface, blech.
> 
>   * We need to know whether the top node is the active layer.
> 
>     Complication: if it's shared by multiple backends, it may be the
>     active layer in one but not the other.  Specifying the backend
>     resolves the ambiguity.  Whether that makes sense I don't know.
> 
> * block-stream
> 
>   @device names a backend, @base names a node via file name matching.
> 
>   TODO: support specifying the node via node name.
> 
>   I think backend is inappropriate here, we should support streaming up
>   to a node, just like block-commit supports committing down from a
>   node.

Agreed.

> 
> * block_passwd
> * block_resize
> 
>   @node-name names a node.
> 
>   @device names a backend, and references its root node, for
>   compatibility.
> 
>   Either @device or @node-name must be given.
> 
>   TODO: should have single mandatory parameter instead of two optionals.
> 
> * blockdev-snapshot-sync
> 
>   @node-name and @device as for block_passwd, including TODO.
> 
>   @snapshot-node-name defines the new node's node name.
> 
> * block_set_io_throttle
> 
>   @device names a backend.
> 
>   TODO: support nodes.
> 
>   Note: we'd like to redo throttling as a block filter node, so maybe
>   we'll have a completely different command instead.

Whether it's implemented in core block layer or as a block filter node is
implementation detail from user's PoV, so it shouldn't matter unless we use a
"general" block filter configuration command.

> 
> * blockdev-add
> 
>   This command defines a backend and its node tree, where sub-trees may
>   be references to existing nodes.
> 
>   @id defines the backend's name.
> 
>   @node-name defines its root node's node name.
> 
>   Note: blockdev-add always defines a backend.  When you build up a
>   backend bottom-up with several commands, you get a bunch of unwanted
>   backends "in the middle".  I'd like to make @id optional, and omit the
>   backend when it's missing.
> 
> * change-backing-file
> 
>   @device names a backend, @image-node-name names a node.
> 
>   As far as I can tell, we need the backend only to set op blockers.
>   Implementation detail shows through the interface.
> 
> * drive-backup
> 
>   @device names a backend.
> 
>   Do we want to be able to back up any node, or only a backend?
> 
>   Note: documentation of @target sounds like it could somehow name a
>   backend, but as far as I can tell it's always interpreted as file
>   name.
> 
> * drive-mirror
> 
>   @device names a backend, @replaces names a node, and @node-name
>   defines the name of the new node.
> 
>   Do we want to be able to mirror any node, or only a backend?
> 
>   Note: documentation of @target sounds like it could somehow name a
>   backend, but as far as I can tell it's always interpreted as file
>   name.
> 
>   Note: drive-mirror supports @replaces, but drive-backup doesn't.  Odd.

It's only asymmetric, not odd: @target has the same data in drive-mirror, so
"replaces" doesn't surprise @device's user. That's not true for drive-backup.

> * query-blockstats
> 
>   Returns some stats for all backends as array of BlockStats.
>   BlockStats member @device is the backend name.  Member @stats contains
>   the stats for the backend's root node.  Member @parent is BlockStats
>   for the child node that is stored in BDS member file.  Member @backing
>   is BlockStats for the chold node that is stored in BDS member
>   backing_hd.  Stats for other children aren't returned.
> 
>   TODO: generalize this to the full tree, include node names.
> 
> * query-block-jobs
> 
>   Returns information on block jobs as array of BlockJobInfo.  A block
>   job is always tied to a backend, and BlockJobInfo member @device is
>   its name.
> 
>   The question whether we need a node name here is moot; see
>   block-job-cancel above.
> 

We are not internally ready to untie block job from backend yet, once we get
there, we should start giving names to block jobs, add support some kind of
default naming/querying to be backward compatible.

Fam