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
