On Thu, 2009-09-24 at 16:00 +0100, Daniel P. Berrange wrote:
* src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h: Add new APis qemuMonitorChangeMedia and qemuMonitorEjectMedia. Pull in code for qemudEscape * src/qemu/qemu_driver.c: Remove code that directly issues 'eject' and 'change' commands in favour of API calls. --- src/qemu/qemu_driver.c | 52 +++----------- src/qemu/qemu_monitor_text.c | 159 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 10 +++ 3 files changed, 178 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0b5e49..b15dc03 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4573,9 +4573,9 @@ static int qemudDomainChangeEjectableMedia(virConnectPtr conn, unsigned int qemuCmdFlags) { virDomainDiskDefPtr origdisk = NULL, newdisk; - char *cmd, *reply, *safe_path; char *devname = NULL; int i; + int ret;
origdisk = NULL; newdisk = dev->data.disk; @@ -4621,52 +4621,18 @@ static int qemudDomainChangeEjectableMedia(virConnectPtr conn, }
if (newdisk->src) { - safe_path = qemudEscapeMonitorArg(newdisk->src); - if (!safe_path) { - virReportOOMError(conn); - VIR_FREE(devname); - return -1; - } - if (virAsprintf(&cmd, "change %s \"%s\"", devname, safe_path) == -1) { - virReportOOMError(conn); - VIR_FREE(safe_path); - VIR_FREE(devname); - return -1; - } - VIR_FREE(safe_path); - - } else if (virAsprintf(&cmd, "eject %s", devname) == -1) { - virReportOOMError(conn); - VIR_FREE(devname); - return -1; - } - VIR_FREE(devname); - - if (qemudMonitorCommand(vm, cmd, &reply) < 0) { - qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("could not change cdrom media")); - VIR_FREE(cmd); - return -1; + ret = qemuMonitorChangeMedia(vm, devname, newdisk->src); + } else { + ret = qemuMonitorEjectMedia(vm, devname); }
- /* If the command failed qemu prints: - * device not found, device is locked ... - * No message is printed on success it seems */ - DEBUG ("%s: ejectable media change reply: %s", vm->def->name, reply); - if (strstr(reply, "\ndevice ")) { - qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - _("changing cdrom media failed: %s"), reply); - VIR_FREE(reply); - VIR_FREE(cmd); - return -1; + if (ret == 0) { + VIR_FREE(origdisk->src); + origdisk->src = newdisk->src; + newdisk->src = NULL; + origdisk->type = newdisk->type; } - VIR_FREE(reply); - VIR_FREE(cmd);
- VIR_FREE(origdisk->src); - origdisk->src = newdisk->src; - newdisk->src = NULL; - origdisk->type = newdisk->type; return 0;
Should return ret here
}
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index be13dce..8be8047 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -40,6 +40,84 @@
#define VIR_FROM_THIS VIR_FROM_QEMU
+static char *qemudEscape(const char *in, int shell) +{
Personally, rather than creating a copy of the code, I'd have moved it and exported it back to qemu_driver.c - end result is the same, but the way you've done it, I couldn't just look at the patch to confirm that the copy of the code was the same as the original ...
@@ -651,3 +729,84 @@ int qemuMonitorSetBalloon(const virDomainObjPtr vm, return ret; }
+int qemuMonitorEjectMedia(const virDomainObjPtr vm, + const char *devname) +{ + char *cmd = NULL; + char *reply = NULL; + int ret = -1; + + if (virAsprintf(&cmd, "eject %s", devname) < 0) { + virReportOOMError(NULL); + goto cleanup; + } + + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("could not eject media on %s"), devname); + goto cleanup; + } + + /* If the command failed qemu prints: + * device not found, device is locked ... + * No message is printed on success it seems */ + DEBUG ("%s: ejectable media change reply: %s", vm->def->name, reply); + if (strstr(reply, "\ndevice ")) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("could not eject media on %s: %s"), devname, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(reply); + VIR_FREE(cmd); + return ret; +} + + +int qemuMonitorChangeMedia(const virDomainObjPtr vm, + const char *devname, + const char *newmedia) +{ + char *cmd = NULL; + char *reply = NULL; + char *safepath = NULL; + int ret = -1; + + if (!(safepath = qemudEscapeMonitorArg(newmedia))) { + virReportOOMError(NULL); + goto cleanup; + } + + if (virAsprintf(&cmd, "change %s \"%s\"", devname, safepath) < 0) { + virReportOOMError(NULL); + goto cleanup; + } + + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("could not eject media on %s"), devname); + goto cleanup; + } + + /* If the command failed qemu prints: + * device not found, device is locked ... + * No message is printed on success it seems */ + DEBUG ("%s: ejectable media change reply: %s", vm->def->name, reply); + if (strstr(reply, "\ndevice ")) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("could not eject media on %s: %s"), devname, reply); + goto cleanup; + }
Pity this code is duplicated Should be 'could not eject' here Otherwise, ACK Cheers, Mark.