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
