All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Alberto Garcia <berto@igalia.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/3] block: allow BLOCK_IMAGE_CORRUPTED to have a node name
Date: Thu, 19 Mar 2015 17:14:24 -0600	[thread overview]
Message-ID: <550B5850.5010303@redhat.com> (raw)
In-Reply-To: <20150319223840.GA13196@igalia.com>

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

On 03/19/2015 04:38 PM, Alberto Garcia wrote:

>> 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.
> 
> Yes, it is perfectly possible. I guess any software that wants to
> handle those scenarios probably wants to give names to all nodes.
> 
> From the QEMU side, apart from giving automatic names to all nodes I
> don't see any other solution.

In the context of block commit, libvirt found that reporting device name
to the end user was nicer than file name (since there are two events
emitted - one when the active layer has become synced with the lower
layer, and therefore where the node associated with device is still
'top'; and the other when the active layer has pivoted to the base layer
and top is no longer valid, therefore where the node associated with
device is now 'base' - since the two events are related but have
different file [node] names, having the device name made life easier).
But note that we still haven't exposed 'node' information in any of the
block job events (BLOCK_JOB_COMPLETED, BLOCK_JOB_CANCELLED,
BLOCK_JOB_ERROR), and it might still make sense to do so, even if
libvirt doesn't need to expose those node names on to the end user.

But this is a counter-example - knowing just the device name does NOT
tell you which node is corrupted, if there is any possibility of a
snapshot or active commit changing the active node associated with the
device.  That's because there might be a race between the event
announcing corruption and some other action modifying the chain, such
that the client's notion of the active element of the chain is different
from the active element at the time the event was reported.

So the more I think about it, the more I'd like for this event to output
both 'device' (mandatory, with an empty string if we can't easily tie
the BDS to a single device) and 'node' (where 'node' can be optional,
and omitted if the BDS does not have a node name [if we ever add
generated node names, then we could make 'node' mandatory at that time]).

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

  reply	other threads:[~2015-03-19 23:15 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
2015-03-19 22:38         ` Alberto Garcia
2015-03-19 23:14           ` Eric Blake [this message]
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=550B5850.5010303@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 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.