qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/3] v2 Fix Block Hotplug race with drive_unplug


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 2/3] v2 Fix Block Hotplug race with drive_unplug()
Date: Fri, 29 Oct 2010 09:15:16 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.12) Gecko/20100915 Lightning/1.0b1 Thunderbird/3.0.8

On 10/29/2010 09:01 AM, Markus Armbruster wrote:
Ryan Harper<address@hidden>  writes:

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 v1:
- Added qemu_aio_flush() before bdrv_flush() to wait on pending io

Signed-off-by: Ryan Harper<address@hidden>
---
  block.c         |    7 +++++++
  block.h         |    1 +
  blockdev.c      |   26 ++++++++++++++++++++++++++
  blockdev.h      |    1 +
  hmp-commands.hx |   15 +++++++++++++++
  5 files changed, 50 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);
+}
Stupid question: why doesn't bdrv_close() flush automatically?

I don't think it's a bad idea to do that but to the extent that the block API is designed after posix file I/O, close does not usually imply flush.

And why do we have to flush here, but not before other uses of
bdrv_close(), such as eject_device()?

Good question. Kevin should also confirm, but looking at the code, I think flush() is needed before close. If there's a pending I/O event and you close before the I/O event is completed, you'll get a callback for completion against a bogus BlockDriverState.

I can't find anything in either raw-posix or the generic block layer that would mitigate this.

Regards,

Anthony Liguori



reply via email to

[Prev in Thread] Current Thread [Next in Thread]