qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Fam Zheng <famz@redhat.com>,
	qemu-devel@nongnu.org, jcody@redhat.com,
	Markus Armbruster <armbru@redhat.com>,
	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: Fri, 5 Dec 2014 10:19:50 +0100	[thread overview]
Message-ID: <20141205091950.GA4026@noname.str.redhat.com> (raw)
In-Reply-To: <5480B985.4050708@redhat.com>

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

Am 04.12.2014 um 20:44 hat Eric Blake geschrieben:
> 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).

I thought libvirt didn't use any node names yet? Then it should probably
never try node-name, but only device.

There's probably little reason to resize a non-root node anyway, so if
you can't do that with qemu < 2.3, I don't think it would be a big
problem.

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

I think what we need to add in the flat array is references (by name) to
the child nodes.

The approach is a bit complicated by the fact that we already include
random subtrees that appeared useful in some places.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2014-12-05  9:20 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
2014-12-05  9:19       ` Kevin Wolf [this message]
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=20141205091950.GA4026@noname.str.redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=benoit.canet@nodalink.com \
    --cc=eblake@redhat.com \
    --cc=famz@redhat.com \
    --cc=jcody@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).