From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55881) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYgFo-0003z3-7q for qemu-devel@nongnu.org; Thu, 19 Mar 2015 15:37:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YYgFn-0000TR-Av for qemu-devel@nongnu.org; Thu, 19 Mar 2015 15:37:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47544) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYgFn-0000TN-5r for qemu-devel@nongnu.org; Thu, 19 Mar 2015 15:37:15 -0400 Message-ID: <550B2569.40908@redhat.com> Date: Thu, 19 Mar 2015 15:37:13 -0400 From: Max Reitz MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/3] Add bdrv_get_device_or_node_name() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi On 2015-03-19 at 11:43, Alberto Garcia wrote: > We have several cases of error messages related to BlockDriverState that expect it to have a device name. > > However many of those can happen in nodes that don't have a device name associated so the generated error messages are not very meaningful. > > This series adds a new function bdrv_get_device_or_node_name(), that returns the node name if there is no device name for that node. Since both the device and the node name live in the same namespace, there's no ambiguity. And since it gives preference to the device name, it maintains backward compatibility. Not necessarily. Imagine a program interpreting the BLOCK_IMAGE_CORRUPTED event, for example (after this series): It sees the @device value and expects a device name there; then it tries to do something with that, assuming it is a device, but that may fail (like looking it up in its own list of devices, or launching a block job, or whatever; I can very well imagine someone wanting to automatically start a blockdev_backup() on that event, for instance). Of course, this is a very weak argument. Before this series, in these cases the caller only received an empty string, which didn't help it very much either. So I just want to point out that it does not fully maintain compatibility, but I don't think there will be any problems whatsoever either. Max > It also updates all calls to bdrv_get_device_name() where it makes sense: several error messages and the BLOCK_IMAGE_CORRUPTED event. > > I wrote this thinking on the case where any arbitrary node will be able to own a block job, but I think it's useful already for the reasons explained above. > > Berto > > Alberto Garcia (3): > block: add bdrv_get_device_or_node_name() > block: use bdrv_get_device_or_node_name() in error messages > block: allow BLOCK_IMAGE_CORRUPTED to have a node name > > block.c | 18 ++++++++++++------ > block/qcow.c | 4 ++-- > block/qcow2.c | 7 ++++--- > block/qed.c | 2 +- > block/quorum.c | 5 +---- > block/vdi.c | 2 +- > block/vhdx.c | 2 +- > block/vmdk.c | 4 ++-- > block/vpc.c | 2 +- > block/vvfat.c | 3 ++- > docs/qmp/qmp-events.txt | 2 +- > include/block/block.h | 1 + > qapi/block-core.json | 2 +- > 13 files changed, 30 insertions(+), 24 deletions(-) >