[libvirt] [PATCH v3] qemu: call drive_unplug in DetachPciDiskDevice
Currently libvirt doesn't confirm whether the guest has responded to the disk removal request. In some cases this can leave the guest with continued access to the device while the mgmt layer believes that it has been removed. With a recent qemu monitor command[1] we can deterministically revoke a guests access to the disk (on the QEMU side) to ensure no futher access is permitted. This patch adds support for the drive_unplug() command and introduces it in the disk removal paths. There is some discussion to be had about how to handle the case where the guest is running in a QEMU without this command (and the fact that we currently don't have a way of detecting what monitor commands are available). Changes since v2: - use VIR_ERROR to report when unplug command not found Changes since v1: - return > 0 when command isn't present, < 0 on command failure - detect when drive_unplug command isn't present and log error instead of failing entire command Signed-off-by: Ryan Harper <ryanh@us.ibm.com> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index abd8e9d..615427a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8646,6 +8646,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char drivestr[PATH_MAX]; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -8673,13 +8674,36 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, goto cleanup; } + /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", + QEMU_DRIVE_HOST_PREFIX, + detach->info.alias)) + < 0 || ret >= sizeof(drivestr)) { + virReportOOMError(); + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ret = qemuMonitorDriveUnplug(priv->mon, drivestr); + DEBUG("DriveUnplug ret=%d", ret); + /* ret > 0 indicates unplug isn't supported, issue will be logged */ + if (ret < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); goto cleanup; } } else { + ret = qemuMonitorDriveUnplug(priv->mon, drivestr); + /* ret > 0 indicates unplug isn't supported, issue will be logged */ + if (ret < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { qemuDomainObjExitMonitor(vm); @@ -8723,6 +8747,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char drivestr[PATH_MAX]; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -8749,7 +8774,22 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, } } + /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", + QEMU_DRIVE_HOST_PREFIX, + detach->info.alias)) + < 0 || ret >= sizeof(drivestr)) { + virReportOOMError(); + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); + /* ret > 0 indicates unplug isn't supported, issue will be logged */ + if (qemuMonitorDriveUnplug(priv->mon, drivestr) < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2366fdb..285381d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1781,6 +1781,25 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, return ret; } +int qemuMonitorDriveUnplug(qemuMonitorPtr mon, + const char *drivestr) +{ + DEBUG("mon=%p drivestr=%s", mon, drivestr); + int ret; + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONDriveUnplug(mon, drivestr); + else + ret = qemuMonitorTextDriveUnplug(mon, drivestr); + return ret; +} + int qemuMonitorDelDevice(qemuMonitorPtr mon, const char *devalias) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 48f4c20..bfe3641 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -381,6 +381,9 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon, int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorDriveUnplug(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d3ab25f..b11fd4f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2243,6 +2243,39 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, } +int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon, + const char *drivestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + DEBUG("JSONDriveUnplug drivestr=%s", drivestr); + cmd = qemuMonitorJSONMakeCommand("drive_unplug", + "s:id", drivestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) { + /* See if drive_unplug isn't supported */ + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_ERROR0(_("unplugging disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + } + ret = qemuMonitorJSONCheckError(cmd, reply); + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 94806c1..6a8692e 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -188,6 +188,9 @@ int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 69971a6..6e5bc95 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2380,6 +2380,52 @@ cleanup: return ret; } +/* Attempts to unplug a drive. Returns 1 if unsupported, 0 if ok, and -1 on + * other failure */ +int qemuMonitorTextDriveUnplug(qemuMonitorPtr mon, + const char *drivestr) +{ + char *cmd = NULL; + char *reply = NULL; + char *safedev; + int ret = -1; + DEBUG("TextDriveUnplug drivestr=%s", drivestr); + + if (!(safedev = qemuMonitorEscapeArg(drivestr))) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&cmd, "drive_unplug %s", safedev) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot unplug %s drive"), drivestr); + goto cleanup; + } + + if (strstr(reply, "unknown command:")) { + VIR_ERROR0(_("unplugging disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + } else if (STRNEQ(reply, "")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unplugging %s drive failed: %s"), drivestr, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safedev); + return ret; +} int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index c017509..8355ce8 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -186,6 +186,9 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon, int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorTextDriveUnplug(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh@us.ibm.com
On Fri, Oct 22, 2010 at 09:14:22AM -0500, Ryan Harper wrote:
Currently libvirt doesn't confirm whether the guest has responded to the disk removal request. In some cases this can leave the guest with continued access to the device while the mgmt layer believes that it has been removed. With a recent qemu monitor command[1] we can deterministically revoke a guests access to the disk (on the QEMU side) to ensure no futher access is permitted.
This patch adds support for the drive_unplug() command and introduces it in the disk removal paths. There is some discussion to be had about how to handle the case where the guest is running in a QEMU without this command (and the fact that we currently don't have a way of detecting what monitor commands are available).
Changes since v2: - use VIR_ERROR to report when unplug command not found Changes since v1: - return > 0 when command isn't present, < 0 on command failure - detect when drive_unplug command isn't present and log error instead of failing entire command
Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index abd8e9d..615427a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8646,6 +8646,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char drivestr[PATH_MAX];
i = qemudFindDisk(vm->def, dev->data.disk->dst);
@@ -8673,13 +8674,36 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, goto cleanup; }
+ /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", + QEMU_DRIVE_HOST_PREFIX, + detach->info.alias)) + < 0 || ret >= sizeof(drivestr)) { + virReportOOMError(); + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ret = qemuMonitorDriveUnplug(priv->mon, drivestr); + DEBUG("DriveUnplug ret=%d", ret); + /* ret > 0 indicates unplug isn't supported, issue will be logged */ + if (ret < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); goto cleanup; } } else { + ret = qemuMonitorDriveUnplug(priv->mon, drivestr); + /* ret > 0 indicates unplug isn't supported, issue will be logged */ + if (ret < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { qemuDomainObjExitMonitor(vm); @@ -8723,6 +8747,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char drivestr[PATH_MAX];
i = qemudFindDisk(vm->def, dev->data.disk->dst);
@@ -8749,7 +8774,22 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, } }
+ /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", + QEMU_DRIVE_HOST_PREFIX, + detach->info.alias)) + < 0 || ret >= sizeof(drivestr)) { + virReportOOMError(); + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); + /* ret > 0 indicates unplug isn't supported, issue will be logged */ + if (qemuMonitorDriveUnplug(priv->mon, drivestr) < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2366fdb..285381d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1781,6 +1781,25 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, return ret; }
+int qemuMonitorDriveUnplug(qemuMonitorPtr mon, + const char *drivestr) +{ + DEBUG("mon=%p drivestr=%s", mon, drivestr); + int ret; + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONDriveUnplug(mon, drivestr); + else + ret = qemuMonitorTextDriveUnplug(mon, drivestr); + return ret; +} + int qemuMonitorDelDevice(qemuMonitorPtr mon, const char *devalias) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 48f4c20..bfe3641 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -381,6 +381,9 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon, int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr);
+int qemuMonitorDriveUnplug(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d3ab25f..b11fd4f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2243,6 +2243,39 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, }
+int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon, + const char *drivestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + DEBUG("JSONDriveUnplug drivestr=%s", drivestr); + cmd = qemuMonitorJSONMakeCommand("drive_unplug", + "s:id", drivestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) { + /* See if drive_unplug isn't supported */ + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_ERROR0(_("unplugging disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + } + ret = qemuMonitorJSONCheckError(cmd, reply); + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 94806c1..6a8692e 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -188,6 +188,9 @@ int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr);
+int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 69971a6..6e5bc95 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2380,6 +2380,52 @@ cleanup: return ret; }
+/* Attempts to unplug a drive. Returns 1 if unsupported, 0 if ok, and -1 on + * other failure */ +int qemuMonitorTextDriveUnplug(qemuMonitorPtr mon, + const char *drivestr) +{ + char *cmd = NULL; + char *reply = NULL; + char *safedev; + int ret = -1; + DEBUG("TextDriveUnplug drivestr=%s", drivestr); + + if (!(safedev = qemuMonitorEscapeArg(drivestr))) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&cmd, "drive_unplug %s", safedev) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot unplug %s drive"), drivestr); + goto cleanup; + } + + if (strstr(reply, "unknown command:")) { + VIR_ERROR0(_("unplugging disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + } else if (STRNEQ(reply, "")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unplugging %s drive failed: %s"), drivestr, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safedev); + return ret; +}
int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index c017509..8355ce8 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -186,6 +186,9 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon, int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr);
+int qemuMonitorTextDriveUnplug(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase);
This looks good to me now, ACK, pending my testing with old QEMU versions under the TCK Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 10/25/2010 09:28 AM, Daniel P. Berrange wrote:
On Fri, Oct 22, 2010 at 09:14:22AM -0500, Ryan Harper wrote:
Currently libvirt doesn't confirm whether the guest has responded to the disk removal request. In some cases this can leave the guest with continued access to the device while the mgmt layer believes that it has been removed. With a recent qemu monitor command[1] we can deterministically revoke a guests access to the disk (on the QEMU side) to ensure no futher access is permitted.
This patch adds support for the drive_unplug() command and introduces it in the disk removal paths. There is some discussion to be had about how to handle the case where the guest is running in a QEMU without this command (and the fact that we currently don't have a way of detecting what monitor commands are available).
Changes since v2: - use VIR_ERROR to report when unplug command not found Changes since v1: - return > 0 when command isn't present, < 0 on command failure - detect when drive_unplug command isn't present and log error instead of failing entire command
This looks good to me now, ACK, pending my testing with old QEMU versions under the TCK
Is there any further TCK testing needed for this before we push, or should I go ahead and push it now? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Nov 02, 2010 at 04:43:06PM -0600, Eric Blake wrote:
On 10/25/2010 09:28 AM, Daniel P. Berrange wrote:
On Fri, Oct 22, 2010 at 09:14:22AM -0500, Ryan Harper wrote:
Currently libvirt doesn't confirm whether the guest has responded to the disk removal request. In some cases this can leave the guest with continued access to the device while the mgmt layer believes that it has been removed. With a recent qemu monitor command[1] we can deterministically revoke a guests access to the disk (on the QEMU side) to ensure no futher access is permitted.
This patch adds support for the drive_unplug() command and introduces it in the disk removal paths. There is some discussion to be had about how to handle the case where the guest is running in a QEMU without this command (and the fact that we currently don't have a way of detecting what monitor commands are available).
Changes since v2: - use VIR_ERROR to report when unplug command not found Changes since v1: - return > 0 when command isn't present, < 0 on command failure - detect when drive_unplug command isn't present and log error instead of failing entire command
This looks good to me now, ACK, pending my testing with old QEMU versions under the TCK
Is there any further TCK testing needed for this before we push, or should I go ahead and push it now?
I'm not clear that they have accepted the drive_unplug command into QEMU GIT yet, since there is still a huge ongoing discussion about it on the qemu mailing list, and the QEMU git server has been unavailable for days now :-( Until I can confirm and test this, we'll have to wait. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 11/03/2010 03:57 AM, Daniel P. Berrange wrote:
On Tue, Nov 02, 2010 at 04:43:06PM -0600, Eric Blake wrote:
On 10/25/2010 09:28 AM, Daniel P. Berrange wrote:
On Fri, Oct 22, 2010 at 09:14:22AM -0500, Ryan Harper wrote:
Currently libvirt doesn't confirm whether the guest has responded to the disk removal request. In some cases this can leave the guest with continued access to the device while the mgmt layer believes that it has been removed. With a recent qemu monitor command[1] we can deterministically revoke a guests access to the disk (on the QEMU side) to ensure no futher access is permitted.
This looks good to me now, ACK, pending my testing with old QEMU versions under the TCK
Is there any further TCK testing needed for this before we push, or should I go ahead and push it now?
I'm not clear that they have accepted the drive_unplug command into QEMU GIT yet, since there is still a huge ongoing discussion about it on the qemu mailing list, and the QEMU git server has been unavailable for days now :-( Until I can confirm and test this, we'll have to wait.
Confirmed that this is in upstream qemu (commit 9063f81415f35), so I'm going ahead and applying this. I've got some followups in mind (using virAsprintf instead of snprintf and a stack-allocation of PATH_MAX bytes), but they can be a separate patch. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Ryan Harper