On Thu 14 Jul 2016 03:28:05 PM CEST, Kevin Wolf wrote:
> - blk = blk_by_name(device);
> - if (!blk) {
> - error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> - "Device '%s' not found", device);
> + bs = qmp_get_root_bs(device, &local_err);
> + if (!bs) {
> + bs = bdrv_lookup_bs(device, device, NULL);
> + if (!bs) {
> + error_free(local_err);
> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
> + "Device '%s' not found", device);
> + } else {
> + error_propagate(errp, local_err);
> + }
> return;
It seems that you're calling bdrv_lookup_bs() here twice, once in
qmp_get_root_bs() and then again directly. If the purpose is to see
whether the error is "device not found" or "device is not a root node" I
think the code would be clearer if you do everything here.
On a related note, you're keeping the DeviceNotFound error here, but not
in block-stream. Wouldn't it be better to keep both APIs consistent?
Berto
