12:23 p.m.
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_del() command and introduces it
in the disk removal paths. If the guest is running in a QEMU without this
command we currently explicitly check for unknown command/CommandNotFound
and log the issue.
If QEMU supports the command we issue the drive_del command after we attempt
to remove the device. The guest may respond and remove the block device
before we get to attempt to call drive_del. In that case, we explicitly check
for 'Device not found' from the monitor indicating that the target drive
was auto-deleted upon guest responds to the device removal notification.
1. http://thread.gmane.org/gmane.comp.emulators.qemu/84745
Signed-off-by: Ryan Harper <ryanh(a)us.ibm.com>
---
Changes since v4:
- removed PATH_MAX, use virAsprintf()
- moved drivestr allocation before call to EnterMonitor
Changes since v3:
- Renamed DriveUnplug -> DriveDel, use drive_del monitor cmd.
- Moved invocation to after DelDevice and guest notification.
- Handle the case where drive is auto-deleted before we call
DriveDel by catching and ignoring 'Device not found' error.
- Simplified DriveDel invocation; no need to check return codes
as the monitor implementations handle all failure case and logs
or ignores as needed.
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
src/qemu/qemu_driver.c | 28 +++++++++++++++++++++
src/qemu/qemu_monitor.c | 19 ++++++++++++++
src/qemu/qemu_monitor.h | 3 ++
src/qemu/qemu_monitor_json.c | 38 +++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 3 ++
src/qemu/qemu_monitor_text.c | 54 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_text.h | 3 ++
7 files changed, 148 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e7b37e1..a6a7b8d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9037,6 +9037,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver
*driver,
virDomainDiskDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
virCgroupPtr cgroup = NULL;
+ char *drivestr = NULL;
i = qemudFindDisk(vm->def, dev->data.disk->dst);
@@ -9064,6 +9065,14 @@ 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 (virAsprintf(&drivestr, "%s%s",
+ QEMU_DRIVE_HOST_PREFIX, detach->info.alias) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
@@ -9077,6 +9086,10 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver
*driver,
goto cleanup;
}
}
+
+ /* disconnect guest from host device */
+ qemuMonitorDriveDel(priv->mon, drivestr);
+
qemuDomainObjExitMonitorWithDriver(driver, vm);
qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0);
@@ -9104,6 +9117,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver
*driver,
ret = 0;
cleanup:
+ VIR_FREE(drivestr);
return ret;
}
@@ -9116,6 +9130,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver
*driver,
virDomainDiskDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
virCgroupPtr cgroup = NULL;
+ char *drivestr = NULL;
i = qemudFindDisk(vm->def, dev->data.disk->dst);
@@ -9142,11 +9157,23 @@ 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 (virAsprintf(&drivestr, "%s%s",
+ QEMU_DRIVE_HOST_PREFIX, detach->info.alias) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
qemuDomainObjExitMonitor(vm);
goto cleanup;
}
+
+ /* disconnect guest from host device */
+ qemuMonitorDriveDel(priv->mon, drivestr);
+
qemuDomainObjExitMonitorWithDriver(driver, vm);
qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0);
@@ -9170,6 +9197,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver
*driver,
ret = 0;
cleanup:
+ VIR_FREE(drivestr);
virCgroupFree(&cgroup);
return ret;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2366fdb..80adba4 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1781,6 +1781,25 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
return ret;
}
+int qemuMonitorDriveDel(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 = qemuMonitorJSONDriveDel(mon, drivestr);
+ else
+ ret = qemuMonitorTextDriveDel(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 7d09145..8cda43b 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 qemuMonitorDriveDel(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 d2c6f0a..a380ab2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2244,6 +2244,44 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon,
}
+int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
+ const char *drivestr)
+{
+ int ret;
+ virJSONValuePtr cmd;
+ virJSONValuePtr reply = NULL;
+
+ DEBUG("JSONDriveDel drivestr=%s", drivestr);
+ cmd = qemuMonitorJSONMakeCommand("drive_del",
+ "s:id", drivestr,
+ NULL);
+ if (!cmd)
+ return -1;
+
+ ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+ if (ret == 0) {
+ /* See if drive_del isn't supported */
+ if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+ VIR_ERROR0(_("deleting disk is not supported. "
+ "This may leak data if disk is reassigned"));
+ ret = 1;
+ goto cleanup;
+ } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
+ /* NB: device not found errors mean the drive was
+ * auto-deleted and we ignore the error */
+ ret = 0;
+ } else {
+ 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..82671c7 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 qemuMonitorJSONDriveDel(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 7f15008..483ceb0 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2285,6 +2285,7 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon,
goto cleanup;
}
+ DEBUG("TextDelDevice devalias=%s", devalias);
if (qemuMonitorCommand(mon, cmd, &reply) < 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
_("cannot detach %s device"), devalias);
@@ -2391,6 +2392,59 @@ cleanup:
return ret;
}
+/* Attempts to remove a host drive.
+ * Returns 1 if unsupported, 0 if ok, and -1 on other failure */
+int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
+ const char *drivestr)
+{
+ char *cmd = NULL;
+ char *reply = NULL;
+ char *safedev;
+ int ret = -1;
+ DEBUG("TextDriveDel drivestr=%s", drivestr);
+
+ if (!(safedev = qemuMonitorEscapeArg(drivestr))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virAsprintf(&cmd, "drive_del %s", safedev) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (qemuMonitorCommand(mon, cmd, &reply) < 0) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _("cannot delete %s drive"), drivestr);
+ goto cleanup;
+ }
+
+ if (strstr(reply, "unknown command:")) {
+ VIR_ERROR0(_("deleting drive is not supported. "
+ "This may leak data if disk is reassigned"));
+ ret = 1;
+ goto cleanup;
+
+ /* (qemu) drive_del wark
+ * Device 'wark' not found */
+ } else if (STRPREFIX(reply, "Device '") && (strstr(reply,
"not found"))) {
+ /* NB: device not found errors mean the drive was auto-deleted and we
+ * ignore the error */
+ ret = 0;
+ } else if (STRNEQ(reply, "")) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _("deleting %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..3a88f87 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 qemuMonitorTextDriveDel(qemuMonitorPtr mon,
+ const char *drivestr);
+
int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon,
const char *alias,
const char *passphrase);
--
1.6.3.3
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh(a)us.ibm.com
1:29 p.m.
On 11/12/2010 11:23 AM, 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.
s/futher/further/
This patch adds support for the drive_del() command and introduces it
in the disk removal paths. If the guest is running in a QEMU without this
command we currently explicitly check for unknown command/CommandNotFound
and log the issue.
If QEMU supports the command we issue the drive_del command after we attempt
to remove the device. The guest may respond and remove the block device
before we get to attempt to call drive_del. In that case, we explicitly check
for 'Device not found' from the monitor indicating that the target drive
was auto-deleted upon guest responds to the device removal notification.
1. http://thread.gmane.org/gmane.comp.emulators.qemu/84745
It looks like this one has gone in upstream to qemu. Is there anything
else that would/should force us to delay putting this into libvirt?
src/qemu/qemu_driver.c | 28 +++++++++++++++++++++
src/qemu/qemu_monitor.c | 19 ++++++++++++++
src/qemu/qemu_monitor.h | 3 ++
src/qemu/qemu_monitor_json.c | 38 +++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 3 ++
src/qemu/qemu_monitor_text.c | 54 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_text.h | 3 ++
7 files changed, 148 insertions(+), 0 deletions(-)
At any rate, the code looks okay to me, so once we decide that it is
safe to apply, you have my:
ACK
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4:12 a.m.
On Fri, Nov 12, 2010 at 12:23:41PM -0600, 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_del() command and introduces it
in the disk removal paths. If the guest is running in a QEMU without this
command we currently explicitly check for unknown command/CommandNotFound
and log the issue.
If QEMU supports the command we issue the drive_del command after we attempt
to remove the device. The guest may respond and remove the block device
before we get to attempt to call drive_del. In that case, we explicitly check
for 'Device not found' from the monitor indicating that the target drive
was auto-deleted upon guest responds to the device removal notification.
1. http://thread.gmane.org/gmane.comp.emulators.qemu/84745
Signed-off-by: Ryan Harper <ryanh(a)us.ibm.com>
---
Changes since v4:
- removed PATH_MAX, use virAsprintf()
- moved drivestr allocation before call to EnterMonitor
Changes since v3:
- Renamed DriveUnplug -> DriveDel, use drive_del monitor cmd.
- Moved invocation to after DelDevice and guest notification.
- Handle the case where drive is auto-deleted before we call
DriveDel by catching and ignoring 'Device not found' error.
- Simplified DriveDel invocation; no need to check return codes
as the monitor implementations handle all failure case and logs
or ignores as needed.
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
src/qemu/qemu_driver.c | 28 +++++++++++++++++++++
src/qemu/qemu_monitor.c | 19 ++++++++++++++
src/qemu/qemu_monitor.h | 3 ++
src/qemu/qemu_monitor_json.c | 38 +++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 3 ++
src/qemu/qemu_monitor_text.c | 54 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_text.h | 3 ++
7 files changed, 148 insertions(+), 0 deletions(-)
ACK, once this drive_del hits the main QEMU git repos
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 :|
1:30 p.m.
On 11/16/2010 03:12 AM, Daniel P. Berrange wrote:
On Fri, Nov 12, 2010 at 12:23:41PM -0600, 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_del() command and introduces it
> in the disk removal paths. If the guest is running in a QEMU without this
> command we currently explicitly check for unknown command/CommandNotFound
> and log the issue.
>
> If QEMU supports the command we issue the drive_del command after we attempt
> to remove the device. The guest may respond and remove the block device
> before we get to attempt to call drive_del. In that case, we explicitly check
> for 'Device not found' from the monitor indicating that the target drive
> was auto-deleted upon guest responds to the device removal notification.
>
> 1. http://thread.gmane.org/gmane.comp.emulators.qemu/84745
>
> Signed-off-by: Ryan Harper <ryanh(a)us.ibm.com>
> ---
> Changes since v4:
> - removed PATH_MAX, use virAsprintf()
> - moved drivestr allocation before call to EnterMonitor
ACK, once this drive_del hits the main QEMU git repos
Shoot. I committed v3 rather than v5 upstream. I'll have to fix that
up; sorry for my mess.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:43 p.m.
New subject: [libvirt] [PATCH] qemu: call drive_del in DetachPciDiskDevice
From: Ryan Harper <ryanh(a)us.ibm.com>
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_del() command and introduces it
in the disk removal paths. If the guest is running in a QEMU without this
command we currently explicitly check for unknown command/CommandNotFound
and log the issue.
If QEMU supports the command we issue the drive_del command after we attempt
to remove the device. The guest may respond and remove the block device
before we get to attempt to call drive_del. In that case, we explicitly check
for 'Device not found' from the monitor indicating that the target drive
was auto-deleted upon guest responds to the device removal notification.
1. http://thread.gmane.org/gmane.comp.emulators.qemu/84745
Signed-off-by: Ryan Harper <ryanh(a)us.ibm.com>
---
Here's the diff between v3 and v5 (including some trailing whitespace
changes I had to make to v5 to get 'make syntax-check' to pass).
Since v5 was already ack'd, but v3 was already pushed, I've gone ahead
and pushed this followup, so that the net result is as if only v5 had
been pushed.
Once again, I'm sorry for botching this up. :(
src/qemu/qemu_driver.c | 44 +++++++++++++++--------------------------
src/qemu/qemu_monitor.c | 8 +++---
src/qemu/qemu_monitor.h | 2 +-
src/qemu/qemu_monitor_json.c | 17 ++++++++++-----
src/qemu/qemu_monitor_json.h | 2 +-
src/qemu/qemu_monitor_text.c | 24 +++++++++++++++-------
src/qemu/qemu_monitor_text.h | 2 +-
7 files changed, 50 insertions(+), 49 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 75f4563..4e65f3b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9033,7 +9033,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver
*driver,
virDomainDiskDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
virCgroupPtr cgroup = NULL;
- char drivestr[PATH_MAX];
+ char *drivestr = NULL;
i = qemudFindDisk(vm->def, dev->data.disk->dst);
@@ -9063,40 +9063,29 @@ static int qemudDomainDetachPciDiskDevice(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)) {
+ if (virAsprintf(&drivestr, "%s%s",
+ QEMU_DRIVE_HOST_PREFIX, detach->info.alias) < 0) {
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);
goto cleanup;
}
}
+
+ /* disconnect guest from host device */
+ qemuMonitorDriveDel(priv->mon, drivestr);
+
qemuDomainObjExitMonitorWithDriver(driver, vm);
qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0);
@@ -9124,6 +9113,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver
*driver,
ret = 0;
cleanup:
+ VIR_FREE(drivestr);
return ret;
}
@@ -9136,7 +9126,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver
*driver,
virDomainDiskDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
virCgroupPtr cgroup = NULL;
- char drivestr[PATH_MAX];
+ char *drivestr = NULL;
i = qemudFindDisk(vm->def, dev->data.disk->dst);
@@ -9165,24 +9155,21 @@ 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)) {
+ if (virAsprintf(&drivestr, "%s%s",
+ QEMU_DRIVE_HOST_PREFIX, detach->info.alias) < 0) {
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;
}
+
+ /* disconnect guest from host device */
+ qemuMonitorDriveDel(priv->mon, drivestr);
+
qemuDomainObjExitMonitorWithDriver(driver, vm);
qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0);
@@ -9206,6 +9193,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver
*driver,
ret = 0;
cleanup:
+ VIR_FREE(drivestr);
virCgroupFree(&cgroup);
return ret;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 8330b9c..6ad894d 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1774,8 +1774,8 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
return ret;
}
-int qemuMonitorDriveUnplug(qemuMonitorPtr mon,
- const char *drivestr)
+int qemuMonitorDriveDel(qemuMonitorPtr mon,
+ const char *drivestr)
{
DEBUG("mon=%p drivestr=%s", mon, drivestr);
int ret;
@@ -1787,9 +1787,9 @@ int qemuMonitorDriveUnplug(qemuMonitorPtr mon,
}
if (mon->json)
- ret = qemuMonitorJSONDriveUnplug(mon, drivestr);
+ ret = qemuMonitorJSONDriveDel(mon, drivestr);
else
- ret = qemuMonitorTextDriveUnplug(mon, drivestr);
+ ret = qemuMonitorTextDriveDel(mon, drivestr);
return ret;
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index cf5b402..4d7aaf7 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -379,7 +379,7 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon,
int qemuMonitorAddDrive(qemuMonitorPtr mon,
const char *drivestr);
-int qemuMonitorDriveUnplug(qemuMonitorPtr mon,
+int qemuMonitorDriveDel(qemuMonitorPtr mon,
const char *drivestr);
int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bcf6377..3bf5421 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2239,15 +2239,15 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon,
}
-int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon,
+int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
const char *drivestr)
{
int ret;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
- DEBUG("JSONDriveUnplug drivestr=%s", drivestr);
- cmd = qemuMonitorJSONMakeCommand("drive_unplug",
+ DEBUG("JSONDriveDel drivestr=%s", drivestr);
+ cmd = qemuMonitorJSONMakeCommand("drive_del",
"s:id", drivestr,
NULL);
if (!cmd)
@@ -2256,14 +2256,19 @@ int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon,
ret = qemuMonitorJSONCommand(mon, cmd, &reply);
if (ret == 0) {
- /* See if drive_unplug isn't supported */
+ /* See if drive_del isn't supported */
if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
- VIR_ERROR0(_("unplugging disk is not supported. "
+ VIR_ERROR0(_("deleting disk is not supported. "
"This may leak data if disk is reassigned"));
ret = 1;
goto cleanup;
+ } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
+ /* NB: device not found errors mean the drive was
+ * auto-deleted and we ignore the error */
+ ret = 0;
+ } else {
+ ret = qemuMonitorJSONCheckError(cmd, reply);
}
- ret = qemuMonitorJSONCheckError(cmd, reply);
}
cleanup:
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 4882d4e..8d96146 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -189,7 +189,7 @@ int qemuMonitorJSONDelDevice(qemuMonitorPtr mon,
int qemuMonitorJSONAddDrive(qemuMonitorPtr mon,
const char *drivestr);
-int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon,
+int qemuMonitorJSONDriveDel(qemuMonitorPtr mon,
const char *drivestr);
int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 1a8d0ec..4914027 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2286,6 +2286,7 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon,
goto cleanup;
}
+ DEBUG("TextDelDevice devalias=%s", devalias);
if (qemuMonitorCommand(mon, cmd, &reply) < 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
_("cannot detach %s device"), devalias);
@@ -2392,41 +2393,48 @@ cleanup:
return ret;
}
-/* Attempts to unplug a drive. Returns 1 if unsupported, 0 if ok, and -1 on
- * other failure */
-int qemuMonitorTextDriveUnplug(qemuMonitorPtr mon,
+/* Attempts to remove a host drive.
+ * Returns 1 if unsupported, 0 if ok, and -1 on other failure */
+int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
const char *drivestr)
{
char *cmd = NULL;
char *reply = NULL;
char *safedev;
int ret = -1;
- DEBUG("TextDriveUnplug drivestr=%s", drivestr);
+ DEBUG("TextDriveDel drivestr=%s", drivestr);
if (!(safedev = qemuMonitorEscapeArg(drivestr))) {
virReportOOMError();
goto cleanup;
}
- if (virAsprintf(&cmd, "drive_unplug %s", safedev) < 0) {
+ if (virAsprintf(&cmd, "drive_del %s", safedev) < 0) {
virReportOOMError();
goto cleanup;
}
if (qemuMonitorCommand(mon, cmd, &reply) < 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
- _("cannot unplug %s drive"), drivestr);
+ _("cannot delete %s drive"), drivestr);
goto cleanup;
}
if (strstr(reply, "unknown command:")) {
- VIR_ERROR0(_("unplugging disk is not supported. "
+ VIR_ERROR0(_("deleting drive is not supported. "
"This may leak data if disk is reassigned"));
ret = 1;
goto cleanup;
+
+ /* (qemu) drive_del wark
+ * Device 'wark' not found */
+ } else if (STRPREFIX(reply, "Device '") && (strstr(reply,
"not found"))) {
+ /* NB: device not found errors mean the drive was auto-deleted and we
+ * ignore the error */
+ ret = 0;
} else if (STRNEQ(reply, "")) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
- _("unplugging %s drive failed: %s"), drivestr, reply);
+ _("deleting %s drive failed: %s"), drivestr, reply);
goto cleanup;
}
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index 6ef858c..57d6e9b 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -187,7 +187,7 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon,
int qemuMonitorTextAddDrive(qemuMonitorPtr mon,
const char *drivestr);
-int qemuMonitorTextDriveUnplug(qemuMonitorPtr mon,
+int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
const char *drivestr);
int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon,
--
1.7.3.2
5097
days inactive
5123
days old
4 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Ryan Harper