Re: [Qemu-devel] [PATCH 2/3] block: Add QMP support for streaming to an intermediate layer

[Alberto Garcia <berto@igalia.com>]

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