From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50925) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YW5wp-0004en-It for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:27:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YW5wk-0001l4-Vj for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:26:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48838) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YW5wk-0001jq-PG for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:26:54 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2CGQrK9021283 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 12 Mar 2015 12:26:54 -0400 From: Markus Armbruster Date: Thu, 12 Mar 2015 17:26:49 +0100 Message-Id: <1426177610-4848-5-git-send-email-armbru@redhat.com> In-Reply-To: <1426177610-4848-1-git-send-email-armbru@redhat.com> References: <1426177610-4848-1-git-send-email-armbru@redhat.com> Subject: [Qemu-devel] [PATCH 4/5] usb/dev-storage: Fix QMP device_add missing encryption key failure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kraxel@redhat.com, lcapitulino@redhat.com 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 --- hw/usb/dev-storage.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index f47c856..f50bcb8 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -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) -- 1.9.3