| Message ID | 1289576289-27888-2-git-send-email-ryanh@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
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]",
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
* 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
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]",
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(-)