usb/dev-storage: Fix QMP device_add missing encryption key failure

When the image is encrypted, QMP device_add creates the device, defers
actually attaching it to when the key becomes available, then returns
an error.  This is wrong.  device_add must either create the device
and succeed, or do nothing and fail.

The bug is in usb_msd_realize_storage().  It posts an error with
qerror_report_err(), and returns success.  Device realization relies
on the return value, and completes.  The QMP monitor, however, relies
on the posted error, and sends it in an error reply.

Reproducer:

    $ qemu-system-x86_64 -nodefaults -display none -usb -qmp stdio -drive if=none,id=foo,file=geheim.qcow2
    {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 2}, "package": ""}, "capabilities": []}}
    { "execute": "qmp_capabilities" }
    {"return": {}}
    { "execute": "device_add", "arguments": { "driver": "usb-storage", "id": "bar", "drive": "foo" } }
    {"error": {"class": "DeviceEncrypted", "desc": "'foo' (geheim.qcow2) is encrypted"}}

Even though we got an error back, the device got created just fine.
To demonstrate, let's unplug it again:

    {"execute":"device_del","arguments": { "id": "bar" } }
    {"timestamp": {"seconds": 1426003440, "microseconds": 237181}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/bar/bar.0/legacy[0]"}}
    {"timestamp": {"seconds": 1426003440, "microseconds": 238231}, "event": "DEVICE_DELETED", "data": {"device": "bar", "path": "/machine/peripheral/bar"}}
    {"return": {}}

Fix by making usb_msd_realize_storage() fail properly.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This commit is contained in:
Markus Armbruster 2015-03-12 17:26:49 +01:00 committed by Gerd Hoffmann
parent 9b14e0efcc
commit 7afcc1f9ba

View file

@ -610,6 +610,23 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
return;
}
bdrv_add_key(blk_bs(blk), NULL, &err);
if (err) {
if (monitor_cur_is_qmp()) {
error_propagate(errp, err);
return;
}
error_free(err);
err = NULL;
if (cur_mon) {
monitor_read_bdrv_key_start(cur_mon, blk_bs(blk),
usb_msd_password_cb, s);
s->dev.auto_attach = 0;
} else {
autostart = 0;
}
}
blkconf_serial(&s->conf, &dev->serial);
blkconf_blocksizes(&s->conf);
@ -638,25 +655,6 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
}
usb_msd_handle_reset(dev);
s->scsi_dev = scsi_dev;
if (bdrv_key_required(blk_bs(blk))) {
if (cur_mon) {
bdrv_add_key(blk_bs(blk), NULL, &err);
if (!err) {
usb_msd_password_cb(s, 0);
} else if (monitor_cur_is_qmp()) {
qerror_report_err(err);
error_free(err);
} else {
error_free(err);
monitor_read_bdrv_key_start(cur_mon, blk_bs(blk),
usb_msd_password_cb, s);
}
s->dev.auto_attach = 0;
} else {
autostart = 0;
}
}
}
static void usb_msd_realize_bot(USBDevice *dev, Error **errp)