qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>, Kevin Wolf <kwolf@redhat.com>
Cc: Fam Zheng <famz@redhat.com>,
	jcody@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com,
	Stefan Hajnoczi <stefanha@redhat.com>,
	benoit.canet@nodalink.com
Subject: Re: [Qemu-devel] Review of monitor commands identifying BDS / BB by name
Date: Thu, 04 Dec 2014 12:44:05 -0700	[thread overview]
Message-ID: <5480B985.4050708@redhat.com> (raw)
In-Reply-To: <87k327e7dp.fsf@blackfin.pond.sub.org>

[-- Attachment #1: Type: text/plain, Size: 3855 bytes --]

On 12/04/2014 08:56 AM, Markus Armbruster wrote:

> 
> @device is a sub-optimal name for this single parameter.  Either we
> accept that and move on, or we deprecate it in favor of a new parameter
> with a better name.  I guess the better name isn't worth that much
> trouble, in particular since the command name is already ugly.
> 
> When @node-name is given, @device must not be given.  So @device is
> mandatory *except* in the @node-name usage we retain for compatibility.
> Deprecate the usage.
> 
> Command documentation could then look like this:
> 
> ##
> # @block-resize
> #
> # Resize a block image while a guest is running.
> #
> # @device: the name of the block backend or node to resize (node names
> # supported since 2.3)
> #
> # @size: new image size in bytes
> #
> # Deprecated usage (since 2.3):
> #
> # @device: #optional the name of the block backend to resize
> #
> # @node-name: #optional name of the node to resize (since 2.0)
> #
> # Either @device or @node-name must be set but not both.

But this isn't discoverable.  You aren't changing whether any parameters
are optional, only enhancing the semantics of existing parameters.  How
is libvirt supposed to know if qemu is old (node names have to go
through node-name) or new (node names are preferred through device)?
Just blindly try the 'device' argument, and if it fails, fall back to an
attempt with node-name?

On the other hand, if 'node-name' becomes the preferred interface, then
libvirt has a solution: if node-name is present (it is easy during
up-front QMP probing to determine whether 'node-name' is a recognized
optional argument or an unknown argument), then always use node-name.
As long as libvirt always names the nodes of each device (which it
should be doing, but that's a patch series for another day and another
list), then a device lookup is never needed.  If 'node-name' is not
present, then only the 'device' form is supported, and libvirt can only
manage the top image of a backend (but can make that point obvious to
the end user that they should upgrade qemu for more functionality).


> Let's get the easy case out of the way first: a query that reports only
> backend properties and not node properties can return an array where
> each element carries a backend name, like query-block does now.
> 
> For queries that report node properties, we have a couple of options:
> 
> * Flat array with node names
> 
>   Like current query-named-block-nodes.
> 
>   Can't report on nodes without names.  Jeff's project to give all nodes
>   names would moot this issue.

I could live with this.

> 
> * Array of trees mirroring the actual node forest,
> 
>   Similar to current query-blockstats.
> 
>   Tree roots correspond to backends, and backends have names.
> 
>   Unfortunately, the nodes don't actually form a forest: node trees may
>   be shared.  To turn it into make a forest, we'd have to duplicate the
>   shared trees.
> 
> * Hybrid: array of trees, but named sub-trees are represented by name
> 
>   Like the previous one, except the recursion stops at named nodes.
>   Instead of including such a sub-tree, we reference it by name, then
>   add it to the array if it's not already there.

This one seems like it might be easier for avoiding the reconstruction
of relationships; but if management doesn't already know relationships,
I'm not sure it is worth the complexity.

> 
> "Flat array" seems simplest.

Simplest to implement.  Management can't easily reconstruct the tree,
but for looking up information about a known node, iterating over the
simpler structure will be faster than trying to find a known node in a
complex structure.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

  reply	other threads:[~2014-12-04 19:44 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-02 19:06 [Qemu-devel] Review of monitor commands identifying BDS / BB by name Markus Armbruster
2014-12-03  5:52 ` Fam Zheng
2014-12-03 11:13   ` Paolo Bonzini
2014-12-04 15:59   ` Markus Armbruster
2014-12-03 10:30 ` Kevin Wolf
2014-12-03 13:59   ` Eric Blake
2014-12-03 14:51     ` Markus Armbruster
2014-12-04 16:56       ` Markus Armbruster
2014-12-04 15:56   ` Markus Armbruster
2014-12-04 19:44     ` Eric Blake [this message]
2014-12-05  9:19       ` Kevin Wolf
2014-12-05 12:19         ` Markus Armbruster
2014-12-05  9:34       ` Markus Armbruster
2014-12-05  9:46         ` Kevin Wolf
2014-12-05 12:08           ` Markus Armbruster
2014-12-16 18:12 ` [Qemu-devel] Can we make monitor commands identify BDS / BB by name consistently? (was: Review of monitor commands identifying BDS / BB by name) Markus Armbruster
2014-12-17 14:12   ` Kevin Wolf
2014-12-17 16:17     ` [Qemu-devel] Can we make monitor commands identify BDS / BB by name consistently? Markus Armbruster
2014-12-19 18:27   ` Markus Armbruster
2014-12-18 15:25 ` [Qemu-devel] Review of ways to create BDSes (was: Review of monitor commands identifying BDS / BB by name) Markus Armbruster
2014-12-19 12:18   ` Kevin Wolf
2014-12-19 14:02     ` [Qemu-devel] Review of ways to create BDSes Markus Armbruster
2014-12-19 14:24   ` Markus Armbruster
2014-12-19 15:52 ` [Qemu-devel] Review of ways to reopen BDSes (was: Review of monitor commands identifying BDS / BB by name) Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5480B985.4050708@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=benoit.canet@nodalink.com \
    --cc=famz@redhat.com \
    --cc=jcody@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).