From patchwork Fri Oct 22 02:55:53 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ryan Harper X-Patchwork-Id: 68794 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 08036B6EF1 for ; Fri, 22 Oct 2010 13:59:24 +1100 (EST) Received: from localhost ([127.0.0.1]:53259 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P97qj-0002Jv-5T for incoming@patchwork.ozlabs.org; Thu, 21 Oct 2010 22:59:21 -0400 Received: from [140.186.70.92] (port=35014 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P97no-0000le-5P for qemu-devel@nongnu.org; Thu, 21 Oct 2010 22:56:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P97nl-0000Bf-V0 for qemu-devel@nongnu.org; Thu, 21 Oct 2010 22:56:20 -0400 Received: from e8.ny.us.ibm.com ([32.97.182.138]:55971) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P97nl-0000BG-Ns for qemu-devel@nongnu.org; Thu, 21 Oct 2010 22:56:17 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by e8.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id o9M2ekDE002737 for ; Thu, 21 Oct 2010 22:40:46 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o9M2u0lr2142400 for ; Thu, 21 Oct 2010 22:56:02 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id o9M2txxq007345 for ; Fri, 22 Oct 2010 00:55:59 -0200 Received: from localhost.localdomain (sig-9-65-186-9.mts.ibm.com [9.65.186.9]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id o9M2ttvF006766; Fri, 22 Oct 2010 00:55:59 -0200 From: Ryan Harper To: Date: Thu, 21 Oct 2010 21:55:53 -0500 Message-Id: <1287716153-25305-3-git-send-email-ryanh@us.ibm.com> X-Mailer: git-send-email 1.6.3.3 In-Reply-To: <1287716153-25305-1-git-send-email-ryanh@us.ibm.com> References: <1287716153-25305-1-git-send-email-ryanh@us.ibm.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) Cc: Stefan Hajnoczi , Anthony Liguori , Ryan Harper , Kevin Wolf Subject: [Qemu-devel] [PATCH 2/2] v3 Fix Block Hotplug race with drive_unplug() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Block hot unplug is racy since the guest is required to acknowlege the ACPI unplug event; this may not happen synchronously with the device removal command This series aims to close a gap where by mgmt applications that assume the block resource has been removed without confirming that the guest has acknowledged the removal may re-assign the underlying device to a second guest leading to data leakage. This series introduces a new montor command to decouple asynchornous device removal from restricting guest access to a block device. We do this by creating a new monitor command drive_unplug which maps to a bdrv_unplug() command which does a qemu_aio_flush; bdrv_flush() and bdrv_close(). Once complete, subsequent IO is rejected from the device and the guest will get IO errors but continue to function. A subsequent device removal command can be issued to remove the device, to which the guest may or maynot respond, but as long as the unplugged bit is set, no IO will be sumbitted. Changes since v2: - Added qmp command for drive_unplug Changes since v1: - Added qemu_aio_flush() before bdrv_flush() to wait on pending io Signed-off-by: Ryan Harper --- block.c | 7 +++++++ block.h | 1 + blockdev.c | 26 ++++++++++++++++++++++++++ blockdev.h | 1 + hmp-commands.hx | 15 +++++++++++++++ qmp-commands.hx | 26 ++++++++++++++++++++++++++ 6 files changed, 76 insertions(+), 0 deletions(-) diff --git a/block.c b/block.c index a19374d..be47655 100644 --- a/block.c +++ b/block.c @@ -1328,6 +1328,13 @@ void bdrv_set_removable(BlockDriverState *bs, int removable) } } +void bdrv_unplug(BlockDriverState *bs) +{ + qemu_aio_flush(); + bdrv_flush(bs); + bdrv_close(bs); +} + int bdrv_is_removable(BlockDriverState *bs) { return bs->removable; diff --git a/block.h b/block.h index 5f64380..732f63e 100644 --- a/block.h +++ b/block.h @@ -171,6 +171,7 @@ void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error, BlockErrorAction on_write_error); BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read); void bdrv_set_removable(BlockDriverState *bs, int removable); +void bdrv_unplug(BlockDriverState *bs); int bdrv_is_removable(BlockDriverState *bs); int bdrv_is_read_only(BlockDriverState *bs); int bdrv_is_sg(BlockDriverState *bs); diff --git a/blockdev.c b/blockdev.c index 5fc3b9b..68eb329 100644 --- a/blockdev.c +++ b/blockdev.c @@ -610,3 +610,29 @@ int do_change_block(Monitor *mon, const char *device, } return monitor_read_bdrv_key_start(mon, bs, NULL, NULL); } + +int do_drive_unplug(Monitor *mon, const QDict *qdict, QObject **ret_data) +{ + DriveInfo *dinfo; + BlockDriverState *bs; + const char *id; + + if (!qdict_haskey(qdict, "id")) { + qerror_report(QERR_MISSING_PARAMETER, "id"); + return -1; + } + + id = qdict_get_str(qdict, "id"); + dinfo = drive_get_by_id(id); + if (!dinfo) { + qerror_report(QERR_DEVICE_NOT_FOUND, id); + return -1; + } + + /* mark block device unplugged */ + bs = dinfo->bdrv; + bdrv_unplug(bs); + + return 0; +} + diff --git a/blockdev.h b/blockdev.h index 19c6915..ecb9ac8 100644 --- a/blockdev.h +++ b/blockdev.h @@ -52,5 +52,6 @@ int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data); int do_block_set_passwd(Monitor *mon, const QDict *qdict, QObject **ret_data); int do_change_block(Monitor *mon, const char *device, const char *filename, const char *fmt); +int do_drive_unplug(Monitor *mon, const QDict *qdict, QObject **ret_data); #endif diff --git a/hmp-commands.hx b/hmp-commands.hx index 81999aa..7a32a2e 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -68,6 +68,21 @@ Eject a removable medium (use -f to force it). ETEXI { + .name = "drive_unplug", + .args_type = "id:s", + .params = "device", + .help = "unplug block device", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_drive_unplug, + }, + +STEXI +@item unplug @var{device} +@findex unplug +Unplug block device. +ETEXI + + { .name = "change", .args_type = "device:B,target:F,arg:s?", .params = "device filename [format]", diff --git a/qmp-commands.hx b/qmp-commands.hx index 793cf1c..e8f3d4a 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -338,6 +338,32 @@ Example: EQMP { + .name = "drive_unplug", + .args_type = "id:s", + .params = "device", + .help = "unplug block device", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_drive_unplug, + }, + +SQMP +drive unplug +---------- + +Unplug a block device. + +Arguments: + +- "id": the device's ID (json-string) + +Example: + +-> { "execute": "drive_unplug", "arguments": { "id": "drive-virtio-blk1" } } +<- { "return": {} } + +EQMP + + { .name = "cpu", .args_type = "index:i", .params = "index",