Re: [Qemu-devel] [PATCH 2/3] block: Add QMP support for streaming to an intermediate layer

[Eric Blake <eblake@redhat.com>]

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

On 03/17/2015 09:00 AM, Alberto Garcia 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, I'm okay with
either:

old libvirt: creates job using 'device' as a device name, status about
the job is reported with 'device' as the device name

new libvirt: creates job using 'device' as a node name, status about the
job is reported with 'device' as the node name

(no new parameter, 'device' is used on both creation and query as a
[poorly-named] device-or-node, back-compat is obvious)


or with:

old libvirt: creates job using 'device' as a device name, status about
the job is reported with 'device' as the device name

new libvirt: creates job using 'node' as a node name, status about the
job is reported with 'node' as the node name

(new parameter; old usage remains the same, and new usage has proper
naming, but now we have to track which name is in use)


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

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

> 
> Regarding the 'block-stream' command, I think the current option to
> reuse the 'device' parameter to refer to either a device or a node
> name is ok, so I'll go forward with that one.

Particularly if we don't have two parameters for starting the job, then
we don't need two parameters for reporting it.

> 
>> On the other hand, having an owner BDS for a block job is considered
>> a mistake meanwhile because there is no clear rule which BDS to pick
>> when the job involves more than one.
> 
> Does it really matter as long as all the operations blockers are
> correctly set?
> 
>> In fact, without tying a job to a BDS, it could be just a background
>> job instead of specifically a block job.
> 
> I don't understand what you mean by this.
> 
> Berto
> 
> 

-- 
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: 604 bytes --]