All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alberto Garcia <berto@igalia.com>
To: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Jeff Cody <jcody@redhat.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/3] block: Add QMP support for streaming to an intermediate layer
Date: Tue, 17 Mar 2015 16:40:51 +0100	[thread overview]
Message-ID: <20150317154051.GA18262@igalia.com> (raw)
In-Reply-To: <550846CF.10706@redhat.com>

On Tue, Mar 17, 2015 at 09:22:55AM -0600, Eric Blake wrote:

> > The BlockJobInfo object returned by query-block-jobs identifies
> > the owner of the job using the 'device' field. If jobs can be in
> > any intermediate node then we cannot simply rely on the device
> > name. We also cannot simply replace it with a node name because
> > 1) it might not exist and 2) existing libvirt versions expect a
> > device name.
> > 
> > So I see several alternatives:
> > 
> >    a) Add a new 'node-name' field to BlockJobInfo. It's simple,
> >       'device' keeps the current semantics so we don't break
> >       compatibility.
> > 
> >    b) Make 'device' return the device name as it currently does,
> >       or the node name if it's not present. The main problem is
> >       that libvirt cannot easily know what to expect. On the
> >       other hand since both device and node-name share the same
> >       namespace the returned value is not ambiguous.
> 
> If libvirt is new enough to create the block job via node name
> instead of device name, then it is also new enough to expect a node
> name instead of device name in the returned job information.

That is clear.

> >    c) Make 'device' return the same name that was used when
> >       the job was created. It's maybe simpler for libvirt than
> >       option b), but it would require us to remember how the job
> >       was created, possibly in the BlockJob structure. This is
> >       personally my least favorite option.
> 
> If you're going to reuse 'device' on the creation, then reuse it on
> the reporting.

The problem with c) is that the name is only needed early in the
operation to get a BlockDriverState, we don't use it afterwards.

So returning the same name that was used to request the operation
would force us to keep that information internally, because in the
case of a job owned by a BlockDriverState with both device name
'virtio0' and node name 'node0' it's otherwise impossible to know if
the job was requested using 'virtio0' or 'node0'.

> >    d) Create a new query command that returns a different data
> >       structure.
> > 
> > I would opt for a) or b), but I'd like to hear if you have a
> > different opinion.
> 
> I'm kind of leaning towards b).

But note that in the example I just mentioned, if you create a job
using 'node0' to refer to the node, you would still get 'virtio0' in
return, and not 'node0'.

With b) you only get 'node0' if the node does not have a device name.

Berto

  reply	other threads:[~2015-03-17 15:41 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-20 13:53 [Qemu-devel] [PATCH 0/3] Support streaming to an intermediate layer Alberto Garcia
2015-02-20 13:53 ` [Qemu-devel] [PATCH 1/3] block: " Alberto Garcia
2015-03-05 14:04   ` Kevin Wolf
2015-03-05 14:58     ` Alberto Garcia
2015-03-05 15:15       ` Kevin Wolf
2015-03-05 15:47         ` Alberto Garcia
2015-03-12 13:18     ` Alberto Garcia
2015-02-20 13:53 ` [Qemu-devel] [PATCH 2/3] block: Add QMP support for " Alberto Garcia
2015-02-20 22:38   ` Eric Blake
2015-02-23 12:23     ` Alberto Garcia
2015-02-23 13:04       ` Kevin Wolf
2015-02-24 14:08         ` Markus Armbruster
2015-03-05 14:09   ` Kevin Wolf
2015-03-05 15:12     ` Alberto Garcia
2015-03-11 16:38     ` Alberto Garcia
2015-03-12 15:45       ` Kevin Wolf
2015-03-17 15:00         ` Alberto Garcia
2015-03-17 15:22           ` Eric Blake
2015-03-17 15:40             ` Alberto Garcia [this message]
2015-03-17 15:28           ` Kevin Wolf
2015-03-18 12:29         ` Alberto Garcia
2015-02-20 13:53 ` [Qemu-devel] [PATCH 3/3] docs: Document how to stream " Alberto Garcia
2015-02-20 17:34 ` [Qemu-devel] [PATCH 0/3] Support streaming " Eric Blake
2015-02-20 19:05   ` Alberto Garcia
2015-02-20 22:49     ` Eric Blake
2015-02-22 15:08       ` Alberto Garcia

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=20150317154051.GA18262@igalia.com \
    --to=berto@igalia.com \
    --cc=eblake@redhat.com \
    --cc=jcody@redhat.com \
    --cc=kwolf@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.