From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44608) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYD6Y-00070v-Dz for qemu-devel@nongnu.org; Wed, 18 Mar 2015 08:29:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YYD6U-00007w-AV for qemu-devel@nongnu.org; Wed, 18 Mar 2015 08:29:46 -0400 Received: from smtp3.mundo-r.com ([212.51.32.191]:17890 helo=smtp4.mundo-r.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYD6U-00007H-4T for qemu-devel@nongnu.org; Wed, 18 Mar 2015 08:29:42 -0400 Date: Wed, 18 Mar 2015 13:29:21 +0100 From: Alberto Garcia Message-ID: <20150318122921.GA12024@igalia.com> References: <49b845c4a362ffd64ec78bbe0b165cd7addd2a4b.1424439295.git.berto@igalia.com> <20150305140958.GE5427@noname.redhat.com> <20150311163806.GA5883@igalia.com> <20150312154517.GB7029@noname.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150312154517.GB7029@noname.redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/3] block: Add QMP support for streaming to an intermediate layer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Jeff Cody , qemu-devel@nongnu.org, Stefan Hajnoczi On Thu, Mar 12, 2015 at 04:45:17PM +0100, Kevin Wolf wrote: > > One issue that I'm finding is that when we move the block-stream > > job to an intermediate node, where the device name is empty, we > > get messages like "Device '' is busy". > My first thought was "then make it 'Source/Target device is busy' > without mentioning any name". In the context of any given command, > it would still be clear which BDS is meant. In fact, I have argued > before that mentioning the device name in an error to a command that > refers to a specific device is redundant and should be avoided. > > The problem here is that it's not stream_start() that generates the > error, but block_job_create(), which doesn't know which role it's bs > argument has for the block job. So it can't decide whether to say > "source device", "target device" or something completely different. The problem is actually not there. The error message generated by block_job_create() is "block device is in use by block job: stream". It's bdrv_op_is_blocked() that adds the extra "Device '' is busy". error_setg(errp, "Device '%s' is busy: %s", bdrv_get_device_name(bs), error_get_pretty(blocker->reason)); I can use the same approach as in the BlockJobInfo case and fall back to the node name if the device name is empty, but the problem is that bdrv_get_device_name() is used all over the place, so this probably needs a more general solution. Even at the moment the backing blocker set by bdrv_set_backing_hd() has problems: error_setg(&bs->backing_blocker, "device is used as backing hd of '%s'", bdrv_get_device_name(bs)); This only works if 'bs' is a root node, but if you try to perform an operation on the backing image of another backing image, you get a "device is used as backing hd of ''". Error messages aside, I would probably need to check all uses of bdrv_get_device_name() because there could be more surprises if the node is not at the root. Berto