All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.