qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Alberto Garcia <berto@igalia.com>, Max Reitz <mreitz@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/3] block: allow BLOCK_IMAGE_CORRUPTED to have a node name
Date: Thu, 19 Mar 2015 16:15:49 -0600	[thread overview]
Message-ID: <550B4A95.9060501@redhat.com> (raw)
In-Reply-To: <20150319214201.GA11212@igalia.com>

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  parent reply	other threads:[~2015-03-19 22:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-19 15:43 [Qemu-devel] [PATCH 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
2015-03-19 15:43 ` [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name() Alberto Garcia
2015-03-19 19:26   ` Max Reitz
2015-03-20  7:40   ` Markus Armbruster
2015-03-20  8:03     ` Alberto Garcia
2015-03-20  8:47       ` Markus Armbruster
2015-03-19 15:43 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
2015-03-19 19:37   ` Max Reitz
2015-03-20  7:52   ` Markus Armbruster
2015-03-19 15:43 ` [Qemu-devel] [PATCH 3/3] block: allow BLOCK_IMAGE_CORRUPTED to have a node name Alberto Garcia
2015-03-19 19:42   ` Max Reitz
2015-03-19 21:42     ` Alberto Garcia
2015-03-19 21:52       ` Max Reitz
2015-03-19 22:04         ` Alberto Garcia
2015-03-19 22:15       ` Eric Blake [this message]
2015-03-19 22:38         ` Alberto Garcia
2015-03-19 23:14           ` Eric Blake
2015-03-20  9:23             ` Alberto Garcia
2015-03-19 19:37 ` [Qemu-devel] [PATCH 0/3] Add bdrv_get_device_or_node_name() Max Reitz
2015-03-19 21:49   ` 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=550B4A95.9060501@redhat.com \
    --to=eblake@redhat.com \
    --cc=berto@igalia.com \
    --cc=kwolf@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).