Re: [Qemu-devel] [PATCH 3/3] block: allow BLOCK_IMAGE_CORRUPTED to have a node name

[Eric Blake]

On 03/19/2015 03:42 PM, Alberto Garcia wrote:
> (I forgot to Cc Eric in this series, doing it now)
> 
> On Thu, Mar 19, 2015 at 03:42:35PM -0400, Max Reitz wrote:
>>>  # Emitted when a corruption has been detected in a disk image
>>>  #
>>> -# @device: device name
>>> +# @device: device name, or node name if not present
>>
>> Normally, if a field in QMP is designed @device, it contains a
>> device name. We do have combined device/node name fields, though (as
>> of John's incremental backup series, at least), but those are named
>> @node (which I proposed for patch 2, too).
>>
>> But renaming the field here will lead to breaking backwards
>> compatibility. I think just adding a @node-name field and keeping
>> @device as it is should be good enough here.
> 
> I was doing the same that we discussed for BlockJobInfo here, where
> option b) seemed to have a bit more support:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg03651.html
> 
> But yeah I personally don't mind extending the event with a new field.
> Would we make 'device' optional in this case?

How hard is it to output both 'device' and 'node' in the same event, if
both are available?  And does it add anything?  I could live with the
simplicity of just returning a device name where we have one
(back-compat for older clients that only ever start jobs on a device)
and a node name where we don't (such an event can only be triggered by a
job started by a client new enough to know how to start a job by node
name), and just always use the 'device' field while documenting that it
is not the best field name for what its contents represent.

On the other hand, if libvirt starts using node names everywhere, then
returning a device name instead of a node name makes libvirt have to do
a bit more work to map a device name down to a node name (not the end of
the world, because the device name is still unambiguous); whereas
returning both device AND node name at once may make libvirt's life easier.

And for this particular event, which is not tied to block jobs but to
generic block operation, isn't it possible that we could be reporting a
corrupted backing chain where we have neither a device name (it is not
the active layer) nor a node name (if we don't add Jeff's patch to
auto-name all nodes)?  In such a case, I don't know that we can do much
better anyways.

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

signature.asc
Description: OpenPGP digital signature