qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal
@ 2010-11-12 15:38 Ryan Harper
  2010-11-12 15:38 ` [Qemu-devel] [PATCH 1/2] Implement drive_del to decouple block " Ryan Harper
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ryan Harper @ 2010-11-12 15:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Michael S. Tsirkin, Markus Armbruster,
	Anthony Liguori, Ryan Harper, Stefan Hajnoczi

Once more dear friends, v7

This patch series decouples the detachment of a block device from the
removal of the backing pci-device.  Removal of a hotplugged pci device
requires the guest to respond before qemu tears down the block device.
In some cases, the guest may not respond leaving the guest with
continued access to the block device.  Mgmt layer doesn't have a
reliable method to force a disconnect.  

The new monitor command, drive_del, will revoke a guests access to the
block device independently of the removal of the pci device.

The first patch implements drive_del, the second patch implements the
qmp version of the monitor command.

Changes since v6:
- Updated patch description
- Dropped bdrv_unplug and inlined in drive_del
- Explicitly invoke drive_uninit()
Changes since v5:
- Removed dangling pointers in guest and host state.  This ensures things like 
  info block no longer displays the deleted drive; though info pci will
  continue to display the pci device until the guest responds to the removal
  request.
- Renamed drive_unplug -> drive_del
Changes since v4:
- Droppped drive_get_by_id patch and use bdrv_find() instead
- Added additional details about drive_unplug to hmp/qmp interface

Changes since v3:
- Moved QMP command for drive_unplug() to separate patch

Changes since v2:
- Added QMP command for drive_unplug()

Changes since v1:
- CodingStyle fixes
- Added qemu_aio_flush() to bdrv_unplug()

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH 1/2] Implement drive_del to decouple block removal from device removal
  2010-11-12 15:38 [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal Ryan Harper
@ 2010-11-12 15:38 ` Ryan Harper
  2010-11-12 16:27   ` Markus Armbruster
  2010-11-12 16:39   ` [Qemu-devel] " Kevin Wolf
  2010-11-12 15:38 ` [Qemu-devel] [PATCH 2/2] Add qmp version of drive_del Ryan Harper
  2010-11-12 16:28 ` [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal Markus Armbruster
  2 siblings, 2 replies; 9+ messages in thread
From: Ryan Harper @ 2010-11-12 15:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Michael S. Tsirkin, Markus Armbruster,
	Anthony Liguori, Ryan Harper, Stefan Hajnoczi

Currently device hotplug removal code is tied to device removal via
ACPI.  All pci devices that are removable via device_del() require the
guest to respond to the request.  In some cases the guest may not
respond leaving the device still accessible to the guest.  The management
layer doesn't currently have a reliable way to revoke access to host
resource in the presence of an uncooperative guest.

This patch implements a new monitor command, drive_del, which
provides an explicit command to revoke access to a host block device.

drive_del first quiesces the block device (qemu_aio_flush;
bdrv_flush() and bdrv_close()).  This prevents further IO from being
submitted against the host device.  Finally, drive_del cleans up
pointers between the drive object (host resource) and the device
object (guest resource).

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 blockdev.c      |   39 +++++++++++++++++++++++++++++++++++++++
 blockdev.h      |    1 +
 hmp-commands.hx |   18 ++++++++++++++++++
 3 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6cb179a..f6ac439 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -14,6 +14,8 @@
 #include "qemu-option.h"
 #include "qemu-config.h"
 #include "sysemu.h"
+#include "hw/qdev.h"
+#include "block_int.h"
 
 static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
 
@@ -597,3 +599,40 @@ int do_change_block(Monitor *mon, const char *device,
     }
     return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
 }
+
+int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    BlockDriverState *bs;
+    BlockDriverState **ptr;
+    Property *prop;
+
+    bs = bdrv_find(id);
+    if (!bs) {
+        qerror_report(QERR_DEVICE_NOT_FOUND, id);
+        return -1;
+    }
+
+    /* quiesce block driver; prevent further io */
+    qemu_aio_flush();
+    bdrv_flush(bs);
+    bdrv_close(bs);
+
+    /* clean up guest state from pointing to host resource by
+     * finding and removing DeviceState "drive" property */
+    for (prop = bs->peer->info->props; prop && prop->name; prop++) {
+        if (prop->info->type == PROP_TYPE_DRIVE) {
+            ptr = qdev_get_prop_ptr(bs->peer, prop);
+            if ((*ptr) == bs) {
+                bdrv_detach(bs, bs->peer);
+                *ptr = NULL;
+                break;
+            }
+        }
+    }
+
+    /* clean up host side */
+    drive_uninit(drive_get_by_blockdev(bs));
+
+    return 0;
+}
diff --git a/blockdev.h b/blockdev.h
index 653affc..2a0559e 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -51,5 +51,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_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 #endif
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e5585ba..d6dc18c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -68,6 +68,24 @@ Eject a removable medium (use -f to force it).
 ETEXI
 
     {
+        .name       = "drive_del",
+        .args_type  = "id:s",
+        .params     = "device",
+        .help       = "remove host block device",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_drive_del,
+    },
+
+STEXI
+@item delete @var{device}
+@findex delete
+Remove host block device.  The result is that guest generated IO is no longer
+submitted against the host device underlying the disk.  Once a drive has
+been deleted, the QEMU Block layer returns -EIO which results in IO 
+errors in the guest for applications that are reading/writing to the device.
+ETEXI
+
+    {
         .name       = "change",
         .args_type  = "device:B,target:F,arg:s?",
         .params     = "device filename [format]",
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH 2/2] Add qmp version of drive_del
  2010-11-12 15:38 [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal Ryan Harper
  2010-11-12 15:38 ` [Qemu-devel] [PATCH 1/2] Implement drive_del to decouple block " Ryan Harper
@ 2010-11-12 15:38 ` Ryan Harper
  2010-11-12 16:28 ` [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal Markus Armbruster
  2 siblings, 0 replies; 9+ messages in thread
From: Ryan Harper @ 2010-11-12 15:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Michael S. Tsirkin, Markus Armbruster,
	Anthony Liguori, Ryan Harper, Stefan Hajnoczi

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 qmp-commands.hx |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index 793cf1c..1e0d4e9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -338,6 +338,35 @@ Example:
 EQMP
 
     {
+        .name       = "drive_del",
+        .args_type  = "id:s",
+        .params     = "device",
+        .help       = "remove host block device",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_drive_del,
+    },
+
+SQMP
+drive del
+----------
+
+Remove host block device.  The result is that guest generated IO is no longer
+submitted against the host device underlying the disk.  Once a drive has
+been deleted, the QEMU Block layer returns -EIO which results in IO 
+errors in the guest for applications that are reading/writing to the device.
+
+Arguments:
+
+- "id": the device's ID (json-string)
+
+Example:
+
+-> { "execute": "drive_del", "arguments": { "id": "drive-virtio-blk1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "cpu",
         .args_type  = "index:i",
         .params     = "index",
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Implement drive_del to decouple block removal from device removal
  2010-11-12 15:38 ` [Qemu-devel] [PATCH 1/2] Implement drive_del to decouple block " Ryan Harper
@ 2010-11-12 16:27   ` Markus Armbruster
  2010-11-12 16:39   ` [Qemu-devel] " Kevin Wolf
  1 sibling, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2010-11-12 16:27 UTC (permalink / raw)
  To: Ryan Harper
  Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel,
	Michael S. Tsirkin

Ryan Harper <ryanh@us.ibm.com> writes:

> Currently device hotplug removal code is tied to device removal via
> ACPI.  All pci devices that are removable via device_del() require the
> guest to respond to the request.  In some cases the guest may not
> respond leaving the device still accessible to the guest.  The management
> layer doesn't currently have a reliable way to revoke access to host
> resource in the presence of an uncooperative guest.
>
> This patch implements a new monitor command, drive_del, which
> provides an explicit command to revoke access to a host block device.
>
> drive_del first quiesces the block device (qemu_aio_flush;
> bdrv_flush() and bdrv_close()).  This prevents further IO from being
> submitted against the host device.  Finally, drive_del cleans up
> pointers between the drive object (host resource) and the device
> object (guest resource).
>
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> ---
>  blockdev.c      |   39 +++++++++++++++++++++++++++++++++++++++
>  blockdev.h      |    1 +
>  hmp-commands.hx |   18 ++++++++++++++++++
>  3 files changed, 58 insertions(+), 0 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 6cb179a..f6ac439 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -14,6 +14,8 @@
>  #include "qemu-option.h"
>  #include "qemu-config.h"
>  #include "sysemu.h"
> +#include "hw/qdev.h"
> +#include "block_int.h"
>  
>  static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
>  
> @@ -597,3 +599,40 @@ int do_change_block(Monitor *mon, const char *device,
>      }
>      return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
>  }
> +
> +int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> +    const char *id = qdict_get_str(qdict, "id");
> +    BlockDriverState *bs;
> +    BlockDriverState **ptr;
> +    Property *prop;
> +
> +    bs = bdrv_find(id);
> +    if (!bs) {
> +        qerror_report(QERR_DEVICE_NOT_FOUND, id);
> +        return -1;
> +    }
> +
> +    /* quiesce block driver; prevent further io */
> +    qemu_aio_flush();
> +    bdrv_flush(bs);
> +    bdrv_close(bs);
> +
> +    /* clean up guest state from pointing to host resource by
> +     * finding and removing DeviceState "drive" property */
> +    for (prop = bs->peer->info->props; prop && prop->name; prop++) {
> +        if (prop->info->type == PROP_TYPE_DRIVE) {
> +            ptr = qdev_get_prop_ptr(bs->peer, prop);
> +            if ((*ptr) == bs) {

Superfluous parenthesis around *ptr.  Not worth a respin; I've tormented
you enough ;)


> +                bdrv_detach(bs, bs->peer);
> +                *ptr = NULL;
> +                break;
> +            }
> +        }
> +    }
> +
> +    /* clean up host side */
> +    drive_uninit(drive_get_by_blockdev(bs));
> +
> +    return 0;
> +}
> diff --git a/blockdev.h b/blockdev.h
> index 653affc..2a0559e 100644
> --- a/blockdev.h
> +++ b/blockdev.h
> @@ -51,5 +51,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_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  
>  #endif
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e5585ba..d6dc18c 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -68,6 +68,24 @@ Eject a removable medium (use -f to force it).
>  ETEXI
>  
>      {
> +        .name       = "drive_del",
> +        .args_type  = "id:s",
> +        .params     = "device",
> +        .help       = "remove host block device",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_drive_del,
> +    },
> +
> +STEXI
> +@item delete @var{device}
> +@findex delete
> +Remove host block device.  The result is that guest generated IO is no longer
> +submitted against the host device underlying the disk.  Once a drive has
> +been deleted, the QEMU Block layer returns -EIO which results in IO 
> +errors in the guest for applications that are reading/writing to the device.
> +ETEXI
> +
> +    {
>          .name       = "change",
>          .args_type  = "device:B,target:F,arg:s?",
>          .params     = "device filename [format]",

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal
  2010-11-12 15:38 [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal Ryan Harper
  2010-11-12 15:38 ` [Qemu-devel] [PATCH 1/2] Implement drive_del to decouple block " Ryan Harper
  2010-11-12 15:38 ` [Qemu-devel] [PATCH 2/2] Add qmp version of drive_del Ryan Harper
@ 2010-11-12 16:28 ` Markus Armbruster
  2010-11-12 16:46   ` Kevin Wolf
  2 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2010-11-12 16:28 UTC (permalink / raw)
  To: Ryan Harper
  Cc: Kevin Wolf, Anthony Liguori, Stefan Hajnoczi, qemu-devel,
	Michael S. Tsirkin

Ryan Harper <ryanh@us.ibm.com> writes:

> Once more dear friends, v7
>
> This patch series decouples the detachment of a block device from the
> removal of the backing pci-device.  Removal of a hotplugged pci device
> requires the guest to respond before qemu tears down the block device.
> In some cases, the guest may not respond leaving the guest with
> continued access to the block device.  Mgmt layer doesn't have a
> reliable method to force a disconnect.  
>
> The new monitor command, drive_del, will revoke a guests access to the
> block device independently of the removal of the pci device.
>
> The first patch implements drive_del, the second patch implements the
> qmp version of the monitor command.
>
> Changes since v6:
> - Updated patch description
> - Dropped bdrv_unplug and inlined in drive_del
> - Explicitly invoke drive_uninit()
> Changes since v5:
> - Removed dangling pointers in guest and host state.  This ensures things like 
>   info block no longer displays the deleted drive; though info pci will
>   continue to display the pci device until the guest responds to the removal
>   request.
> - Renamed drive_unplug -> drive_del
> Changes since v4:
> - Droppped drive_get_by_id patch and use bdrv_find() instead
> - Added additional details about drive_unplug to hmp/qmp interface
>
> Changes since v3:
> - Moved QMP command for drive_unplug() to separate patch
>
> Changes since v2:
> - Added QMP command for drive_unplug()
>
> Changes since v1:
> - CodingStyle fixes
> - Added qemu_aio_flush() to bdrv_unplug()
>
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

ACK series

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] Re: [PATCH 1/2] Implement drive_del to decouple block removal from device removal
  2010-11-12 15:38 ` [Qemu-devel] [PATCH 1/2] Implement drive_del to decouple block " Ryan Harper
  2010-11-12 16:27   ` Markus Armbruster
@ 2010-11-12 16:39   ` Kevin Wolf
  2010-11-12 16:45     ` Ryan Harper
  1 sibling, 1 reply; 9+ messages in thread
From: Kevin Wolf @ 2010-11-12 16:39 UTC (permalink / raw)
  To: Ryan Harper
  Cc: Stefan Hajnoczi, Anthony Liguori, Michael S. Tsirkin, qemu-devel,
	Markus Armbruster

Am 12.11.2010 16:38, schrieb Ryan Harper:
> Currently device hotplug removal code is tied to device removal via
> ACPI.  All pci devices that are removable via device_del() require the
> guest to respond to the request.  In some cases the guest may not
> respond leaving the device still accessible to the guest.  The management
> layer doesn't currently have a reliable way to revoke access to host
> resource in the presence of an uncooperative guest.
> 
> This patch implements a new monitor command, drive_del, which
> provides an explicit command to revoke access to a host block device.
> 
> drive_del first quiesces the block device (qemu_aio_flush;
> bdrv_flush() and bdrv_close()).  This prevents further IO from being
> submitted against the host device.  Finally, drive_del cleans up
> pointers between the drive object (host resource) and the device
> object (guest resource).
> 
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> ---
>  blockdev.c      |   39 +++++++++++++++++++++++++++++++++++++++
>  blockdev.h      |    1 +
>  hmp-commands.hx |   18 ++++++++++++++++++
>  3 files changed, 58 insertions(+), 0 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 6cb179a..f6ac439 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -14,6 +14,8 @@
>  #include "qemu-option.h"
>  #include "qemu-config.h"
>  #include "sysemu.h"
> +#include "hw/qdev.h"
> +#include "block_int.h"
>  
>  static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
>  
> @@ -597,3 +599,40 @@ int do_change_block(Monitor *mon, const char *device,
>      }
>      return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
>  }
> +
> +int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> +    const char *id = qdict_get_str(qdict, "id");
> +    BlockDriverState *bs;
> +    BlockDriverState **ptr;
> +    Property *prop;
> +
> +    bs = bdrv_find(id);
> +    if (!bs) {
> +        qerror_report(QERR_DEVICE_NOT_FOUND, id);
> +        return -1;
> +    }
> +
> +    /* quiesce block driver; prevent further io */
> +    qemu_aio_flush();
> +    bdrv_flush(bs);
> +    bdrv_close(bs);
> +
> +    /* clean up guest state from pointing to host resource by
> +     * finding and removing DeviceState "drive" property */
> +    for (prop = bs->peer->info->props; prop && prop->name; prop++) {
> +        if (prop->info->type == PROP_TYPE_DRIVE) {
> +            ptr = qdev_get_prop_ptr(bs->peer, prop);
> +            if ((*ptr) == bs) {
> +                bdrv_detach(bs, bs->peer);
> +                *ptr = NULL;
> +                break;
> +            }
> +        }
> +    }
> +
> +    /* clean up host side */
> +    drive_uninit(drive_get_by_blockdev(bs));
> +
> +    return 0;
> +}
> diff --git a/blockdev.h b/blockdev.h
> index 653affc..2a0559e 100644
> --- a/blockdev.h
> +++ b/blockdev.h
> @@ -51,5 +51,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_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  
>  #endif
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e5585ba..d6dc18c 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -68,6 +68,24 @@ Eject a removable medium (use -f to force it).
>  ETEXI
>  
>      {
> +        .name       = "drive_del",
> +        .args_type  = "id:s",
> +        .params     = "device",
> +        .help       = "remove host block device",
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_drive_del,
> +    },
> +
> +STEXI
> +@item delete @var{device}
> +@findex delete

I think this should be @item drive_del and @findex drive_del.

Kevin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] Re: [PATCH 1/2] Implement drive_del to decouple block removal from device removal
  2010-11-12 16:39   ` [Qemu-devel] " Kevin Wolf
@ 2010-11-12 16:45     ` Ryan Harper
  0 siblings, 0 replies; 9+ messages in thread
From: Ryan Harper @ 2010-11-12 16:45 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: Michael S. Tsirkin, qemu-devel, Markus Armbruster,
	Anthony Liguori, Ryan Harper, Stefan Hajnoczi

* Kevin Wolf <kwolf@redhat.com> [2010-11-12 10:43]:
> Am 12.11.2010 16:38, schrieb Ryan Harper:
> > Currently device hotplug removal code is tied to device removal via
> > ACPI.  All pci devices that are removable via device_del() require the
> > guest to respond to the request.  In some cases the guest may not
> > respond leaving the device still accessible to the guest.  The management
> > layer doesn't currently have a reliable way to revoke access to host
> > resource in the presence of an uncooperative guest.
> > 
> > This patch implements a new monitor command, drive_del, which
> > provides an explicit command to revoke access to a host block device.
> > 
> > drive_del first quiesces the block device (qemu_aio_flush;
> > bdrv_flush() and bdrv_close()).  This prevents further IO from being
> > submitted against the host device.  Finally, drive_del cleans up
> > pointers between the drive object (host resource) and the device
> > object (guest resource).
> > 
> > Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> > ---
> >  blockdev.c      |   39 +++++++++++++++++++++++++++++++++++++++
> >  blockdev.h      |    1 +
> >  hmp-commands.hx |   18 ++++++++++++++++++
> >  3 files changed, 58 insertions(+), 0 deletions(-)
> > 
> > diff --git a/blockdev.c b/blockdev.c
> > index 6cb179a..f6ac439 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -14,6 +14,8 @@
> >  #include "qemu-option.h"
> >  #include "qemu-config.h"
> >  #include "sysemu.h"
> > +#include "hw/qdev.h"
> > +#include "block_int.h"
> >  
> >  static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
> >  
> > @@ -597,3 +599,40 @@ int do_change_block(Monitor *mon, const char *device,
> >      }
> >      return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
> >  }
> > +
> > +int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
> > +{
> > +    const char *id = qdict_get_str(qdict, "id");
> > +    BlockDriverState *bs;
> > +    BlockDriverState **ptr;
> > +    Property *prop;
> > +
> > +    bs = bdrv_find(id);
> > +    if (!bs) {
> > +        qerror_report(QERR_DEVICE_NOT_FOUND, id);
> > +        return -1;
> > +    }
> > +
> > +    /* quiesce block driver; prevent further io */
> > +    qemu_aio_flush();
> > +    bdrv_flush(bs);
> > +    bdrv_close(bs);
> > +
> > +    /* clean up guest state from pointing to host resource by
> > +     * finding and removing DeviceState "drive" property */
> > +    for (prop = bs->peer->info->props; prop && prop->name; prop++) {
> > +        if (prop->info->type == PROP_TYPE_DRIVE) {
> > +            ptr = qdev_get_prop_ptr(bs->peer, prop);
> > +            if ((*ptr) == bs) {
> > +                bdrv_detach(bs, bs->peer);
> > +                *ptr = NULL;
> > +                break;
> > +            }
> > +        }
> > +    }
> > +
> > +    /* clean up host side */
> > +    drive_uninit(drive_get_by_blockdev(bs));
> > +
> > +    return 0;
> > +}
> > diff --git a/blockdev.h b/blockdev.h
> > index 653affc..2a0559e 100644
> > --- a/blockdev.h
> > +++ b/blockdev.h
> > @@ -51,5 +51,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_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
> >  
> >  #endif
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index e5585ba..d6dc18c 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -68,6 +68,24 @@ Eject a removable medium (use -f to force it).
> >  ETEXI
> >  
> >      {
> > +        .name       = "drive_del",
> > +        .args_type  = "id:s",
> > +        .params     = "device",
> > +        .help       = "remove host block device",
> > +        .user_print = monitor_user_noop,
> > +        .mhandler.cmd_new = do_drive_del,
> > +    },
> > +
> > +STEXI
> > +@item delete @var{device}
> > +@findex delete
> 
> I think this should be @item drive_del and @findex drive_del.

Yep.

> 
> Kevin

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal
  2010-11-12 16:28 ` [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal Markus Armbruster
@ 2010-11-12 16:46   ` Kevin Wolf
  2010-11-12 17:41     ` Markus Armbruster
  0 siblings, 1 reply; 9+ messages in thread
From: Kevin Wolf @ 2010-11-12 16:46 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Stefan Hajnoczi, Anthony Liguori, Ryan Harper, qemu-devel,
	Michael S. Tsirkin

Am 12.11.2010 17:28, schrieb Markus Armbruster:
> Ryan Harper <ryanh@us.ibm.com> writes:
> 
>> Once more dear friends, v7
>>
>> This patch series decouples the detachment of a block device from the
>> removal of the backing pci-device.  Removal of a hotplugged pci device
>> requires the guest to respond before qemu tears down the block device.
>> In some cases, the guest may not respond leaving the guest with
>> continued access to the block device.  Mgmt layer doesn't have a
>> reliable method to force a disconnect.  
>>
>> The new monitor command, drive_del, will revoke a guests access to the
>> block device independently of the removal of the pci device.
>>
>> The first patch implements drive_del, the second patch implements the
>> qmp version of the monitor command.
>>
>> Changes since v6:
>> - Updated patch description
>> - Dropped bdrv_unplug and inlined in drive_del
>> - Explicitly invoke drive_uninit()
>> Changes since v5:
>> - Removed dangling pointers in guest and host state.  This ensures things like 
>>   info block no longer displays the deleted drive; though info pci will
>>   continue to display the pci device until the guest responds to the removal
>>   request.
>> - Renamed drive_unplug -> drive_del
>> Changes since v4:
>> - Droppped drive_get_by_id patch and use bdrv_find() instead
>> - Added additional details about drive_unplug to hmp/qmp interface
>>
>> Changes since v3:
>> - Moved QMP command for drive_unplug() to separate patch
>>
>> Changes since v2:
>> - Added QMP command for drive_unplug()
>>
>> Changes since v1:
>> - CodingStyle fixes
>> - Added qemu_aio_flush() to bdrv_unplug()
>>
>> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> 
> ACK series

I have to admit that I didn't follow your discussion very closely any
more after a few versions, so just to confirm: You came to the
conclusion that we want to add drive_del to QMP and not only the human
monitor, even though there is no drive_add in QMP?

The HMP help text doesn't seem to be completely right (sent a comment),
but once that's fixed, I'm going to merge the series based on Markus's ACK.

Kevin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal
  2010-11-12 16:46   ` Kevin Wolf
@ 2010-11-12 17:41     ` Markus Armbruster
  0 siblings, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2010-11-12 17:41 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: Stefan Hajnoczi, Anthony Liguori, Ryan Harper, qemu-devel,
	Michael S. Tsirkin

Kevin Wolf <kwolf@redhat.com> writes:

> I have to admit that I didn't follow your discussion very closely any
> more after a few versions, so just to confirm: You came to the
> conclusion that we want to add drive_del to QMP and not only the human
> monitor, even though there is no drive_add in QMP?

Fair question.

-drive and drive_add conflate host and guest part.  They were hacked up
to work with -device, and it shows.  I'd rather not copy the mess to
QMP.

Maybe it's best to leave out the QMP part for now, i.e. drop PATCH 2/2.
If we can't get blockdev_add, blockdev_del done in time, we reconsider.

> The HMP help text doesn't seem to be completely right (sent a comment),
> but once that's fixed, I'm going to merge the series based on Markus's ACK.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2010-11-12 17:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-12 15:38 [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal Ryan Harper
2010-11-12 15:38 ` [Qemu-devel] [PATCH 1/2] Implement drive_del to decouple block " Ryan Harper
2010-11-12 16:27   ` Markus Armbruster
2010-11-12 16:39   ` [Qemu-devel] " Kevin Wolf
2010-11-12 16:45     ` Ryan Harper
2010-11-12 15:38 ` [Qemu-devel] [PATCH 2/2] Add qmp version of drive_del Ryan Harper
2010-11-12 16:28 ` [Qemu-devel] [PATCH 0/2] v7 Decouple block device removal from device removal Markus Armbruster
2010-11-12 16:46   ` Kevin Wolf
2010-11-12 17:41     ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).