[libvirt] [PATCH 0/4] Misc fixes + enhancements to the QEMU JSON mode monitor
This series is the latest update to the JSON mode monitor code, based on current QEMU state of development. This work is still ongoing, at the very least QEMU is still lacking support for device_del, drive_add netdev_add and delvm/loadvm/savedvm commands that libvirt needs.
The QEMU developers have stated that they will not be porting the commands 'pci_add', 'pci_del', 'usb_add', 'usb_del' to the JSON mode monitor, since they're obsoleted by 'device_add' and 'device_del'. libvirt has (untested) code that would have supported those commands in theory, but since we already use device_add/del where available, there's no need to keep the legacy stuff anymore. The text mode monitor keeps support for all commands for sake of historical compatability. * src/qemu/qemu_monitor_json.c: Remove 'pci_add', 'pci_del', 'usb_add', 'usb_del' commands --- src/qemu/qemu_monitor_json.c | 324 ++++++------------------------------------ 1 files changed, 47 insertions(+), 277 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2904201..ec04d79 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1476,272 +1476,72 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) } -static int qemuMonitorJSONAddUSB(qemuMonitorPtr mon, - const char *dev) +int qemuMonitorJSONAddUSBDisk(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED) { - int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("usb_add", - "s:devname", dev, - NULL); - virJSONValuePtr reply = NULL; - - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; -} - - -int qemuMonitorJSONAddUSBDisk(qemuMonitorPtr mon, - const char *path) -{ - int ret; - char *disk; - - if (virAsprintf(&disk, "disk:%s", path) < 0) { - virReportOOMError(); - return -1; - } - - ret = qemuMonitorJSONAddUSB(mon, disk); - - VIR_FREE(disk); - - return ret; -} - - -int qemuMonitorJSONAddUSBDeviceExact(qemuMonitorPtr mon, - int bus, - int dev) -{ - int ret; - char *addr; - - if (virAsprintf(&addr, "host:%.3d.%.3d", bus, dev) < 0) { - virReportOOMError(); - return -1; - } - - ret = qemuMonitorJSONAddUSB(mon, addr); - - VIR_FREE(addr); - return ret; + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("usb_add not suppported in JSON mode")); + return -1; } -int qemuMonitorJSONAddUSBDeviceMatch(qemuMonitorPtr mon, - int vendor, - int product) +int qemuMonitorJSONAddUSBDeviceExact(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + int bus ATTRIBUTE_UNUSED, + int dev ATTRIBUTE_UNUSED) { - int ret; - char *addr; - - if (virAsprintf(&addr, "host:%.4x:%.4x", vendor, product) < 0) { - virReportOOMError(); - return -1; - } - - ret = qemuMonitorJSONAddUSB(mon, addr); - - VIR_FREE(addr); - return ret; + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("usb_add not suppported in JSON mode")); + return -1; } -static int -qemuMonitorJSONGetGuestPCIAddress(virJSONValuePtr reply, - virDomainDevicePCIAddress *guestAddr) +int qemuMonitorJSONAddUSBDeviceMatch(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + int vendor ATTRIBUTE_UNUSED, + int product ATTRIBUTE_UNUSED) { - virJSONValuePtr addr; - - addr = virJSONValueObjectGet(reply, "return"); - if (!addr || addr->type != VIR_JSON_TYPE_OBJECT) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("pci_add reply was missing device address")); - return -1; - } - - if (virJSONValueObjectGetNumberUint(addr, "domain", &guestAddr->domain) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("pci_add reply was missing device domain number")); - return -1; - } - - if (virJSONValueObjectGetNumberUint(addr, "bus", &guestAddr->bus) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("pci_add reply was missing device bus number")); - return -1; - } - - if (virJSONValueObjectGetNumberUint(addr, "slot", &guestAddr->slot) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("pci_add reply was missing device slot number")); - return -1; - } - - if (virJSONValueObjectGetNumberUint(addr, "function", &guestAddr->function) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("pci_add reply was missing device function number")); - return -1; - } - - return 0; + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("usb_add not suppported in JSON mode")); + return -1; } -int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *hostAddr, - virDomainDevicePCIAddress *guestAddr) +int qemuMonitorJSONAddPCIHostDevice(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainDevicePCIAddress *hostAddr ATTRIBUTE_UNUSED, + virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - char *dev; - - memset(guestAddr, 0, sizeof(*guestAddr)); - - /* XXX hostDomain */ - if (virAsprintf(&dev, "host=%.2x:%.2x.%.1x", - hostAddr->bus, hostAddr->slot, hostAddr->function) < 0) { - virReportOOMError(); - return -1; - } - - cmd = qemuMonitorJSONMakeCommand("pci_add", - "s:pci_addr", "auto" - "s:type", "host", - "s:opts", dev, - NULL); - VIR_FREE(dev); - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret == 0 && - qemuMonitorJSONGetGuestPCIAddress(reply, guestAddr) < 0) - ret = -1; - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("pci_add not suppported in JSON mode")); + return -1; } -int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon, - const char *path, - const char *bus, - virDomainDevicePCIAddress *guestAddr) +int qemuMonitorJSONAddPCIDisk(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + const char *bus ATTRIBUTE_UNUSED, + virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - char *dev; - - memset(guestAddr, 0, sizeof(*guestAddr)); - - if (virAsprintf(&dev, "file=%s,if=%s", path, bus) < 0) { - virReportOOMError(); - return -1; - } - - cmd = qemuMonitorJSONMakeCommand("pci_add", - "s:pci_addr", "auto", - "s:type", "storage", - "s:opts", dev, - NULL); - VIR_FREE(dev); - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret == 0 && - qemuMonitorJSONGetGuestPCIAddress(reply, guestAddr) < 0) - ret = -1; - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("pci_add not suppported in JSON mode")); + return -1; } -int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon, - const char *nicstr, - virDomainDevicePCIAddress *guestAddr) +int qemuMonitorJSONAddPCINetwork(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + const char *nicstr ATTRIBUTE_UNUSED, + virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { - int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("pci_add", - "s:pci_addr", "auto", - "s:type", "nic", - "s:opts", nicstr, - NULL); - virJSONValuePtr reply = NULL; - - memset(guestAddr, 0, sizeof(*guestAddr)); - - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret == 0 && - qemuMonitorJSONGetGuestPCIAddress(reply, guestAddr) < 0) - ret = -1; - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("pci_add not suppported in JSON mode")); + return -1; } -int qemuMonitorJSONRemovePCIDevice(qemuMonitorPtr mon, - virDomainDevicePCIAddress *guestAddr) +int qemuMonitorJSONRemovePCIDevice(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - char *addr; - - /* XXX what about function ? */ - if (virAsprintf(&addr, "%.4x:%.2x:%.2x", - guestAddr->domain, guestAddr->bus, guestAddr->slot) < 0) { - virReportOOMError(); - return -1; - } - - cmd = qemuMonitorJSONMakeCommand("pci_del", - "s:pci_addr", addr, - NULL); - VIR_FREE(addr); - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("pci_del not suppported in JSON mode")); + return -1; } @@ -1935,43 +1735,13 @@ int qemuMonitorJSONGetPtyPaths(qemuMonitorPtr mon, } -int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon, - const char *bus, - virDomainDevicePCIAddress *guestAddr) +int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + const char *bus ATTRIBUTE_UNUSED, + virDomainDevicePCIAddress *guestAddr ATTRIBUTE_UNUSED) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - char *dev; - - memset(guestAddr, 0, sizeof(*guestAddr)); - - if (virAsprintf(&dev, "if=%s", bus) < 0) { - virReportOOMError(); - return -1; - } - - cmd = qemuMonitorJSONMakeCommand("pci_add", - "s:pci_addr", "auto", - "s:type", "storage", - "s:opts", dev, - NULL); - VIR_FREE(dev); - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret == 0 && - qemuMonitorJSONGetGuestPCIAddress(reply, guestAddr) < 0) - ret = -1; - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("pci_add not suppported in JSON mode")); + return -1; } -- 1.6.6.1
On Thu, Apr 15, 2010 at 11:43:08AM +0100, Daniel P. Berrange wrote:
The QEMU developers have stated that they will not be porting the commands 'pci_add', 'pci_del', 'usb_add', 'usb_del' to the JSON mode monitor, since they're obsoleted by 'device_add' and 'device_del'. libvirt has (untested) code that would have supported those commands in theory, but since we already use device_add/del where available, there's no need to keep the legacy stuff anymore.
The text mode monitor keeps support for all commands for sake of historical compatability.
* src/qemu/qemu_monitor_json.c: Remove 'pci_add', 'pci_del', 'usb_add', 'usb_del' commands --- src/qemu/qemu_monitor_json.c | 324 ++++++------------------------------------ 1 files changed, 47 insertions(+), 277 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2904201..ec04d79 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1476,272 +1476,72 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) }
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
The parameter for the qemuMonitorDeviceDel() is a device alias, not a device config string. Rename the parameter reflect this and avoid confusion to readers. * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Rename devicestr to devalias in qemuMonitorDeviceDel() --- src/qemu/qemu_monitor.c | 8 ++++---- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 4 ++-- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_monitor_text.c | 8 ++++---- src/qemu/qemu_monitor_text.h | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 01e3a46..c3cb3f8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1438,15 +1438,15 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, } int qemuMonitorDelDevice(qemuMonitorPtr mon, - const char *devicestr) + const char *devalias) { - DEBUG("mon=%p, fd=%d device(del)=%s", mon, mon->fd, devicestr); + DEBUG("mon=%p, fd=%d devalias=%s", mon, mon->fd, devalias); int ret; if (mon->json) - ret = qemuMonitorJSONDelDevice(mon, devicestr); + ret = qemuMonitorJSONDelDevice(mon, devalias); else - ret = qemuMonitorTextDelDevice(mon, devicestr); + ret = qemuMonitorTextDelDevice(mon, devalias); return ret; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 21b8989..1d77f99 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -335,7 +335,7 @@ int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr); int qemuMonitorDelDevice(qemuMonitorPtr mon, - const char *devicestr); + const char *devalias); int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ec04d79..6c73685 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1823,14 +1823,14 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED, int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, - const char *devicestr) + const char *devalias) { int ret; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; cmd = qemuMonitorJSONMakeCommand("device_del", - "s:config", devicestr, + "s:config", devalias, NULL); if (!cmd) return -1; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index e7baf84..c6ab1e8 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -166,7 +166,7 @@ int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr); int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, - const char *devicestr); + const char *devalias); int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 9942768..48c9a54 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2133,14 +2133,14 @@ error: int qemuMonitorTextDelDevice(qemuMonitorPtr mon, - const char *devicestr) + const char *devalias) { char *cmd = NULL; char *reply = NULL; char *safedev; int ret = -1; - if (!(safedev = qemuMonitorEscapeArg(devicestr))) { + if (!(safedev = qemuMonitorEscapeArg(devalias))) { virReportOOMError(); goto cleanup; } @@ -2152,13 +2152,13 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon, if (qemuMonitorCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("cannot detach %s device"), devicestr); + _("cannot detach %s device"), devalias); goto cleanup; } if (STRNEQ(reply, "")) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("detaching %s device failed: %s"), devicestr, reply); + _("detaching %s device failed: %s"), devalias, reply); goto cleanup; } diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index fb7d08b..3200660 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -168,7 +168,7 @@ int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr); int qemuMonitorTextDelDevice(qemuMonitorPtr mon, - const char *devicestr); + const char *devalias); int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr); -- 1.6.6.1
On Thu, Apr 15, 2010 at 11:43:09AM +0100, Daniel P. Berrange wrote:
The parameter for the qemuMonitorDeviceDel() is a device alias, not a device config string. Rename the parameter reflect this and avoid confusion to readers.
* src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Rename devicestr to devalias in qemuMonitorDeviceDel() --- src/qemu/qemu_monitor.c | 8 ++++---- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 4 ++-- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_monitor_text.c | 8 ++++---- src/qemu/qemu_monitor_text.h | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 01e3a46..c3cb3f8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1438,15 +1438,15 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, }
int qemuMonitorDelDevice(qemuMonitorPtr mon, - const char *devicestr) + const char *devalias) { - DEBUG("mon=%p, fd=%d device(del)=%s", mon, mon->fd, devicestr); + DEBUG("mon=%p, fd=%d devalias=%s", mon, mon->fd, devalias); int ret;
if (mon->json) - ret = qemuMonitorJSONDelDevice(mon, devicestr); + ret = qemuMonitorJSONDelDevice(mon, devalias); else - ret = qemuMonitorTextDelDevice(mon, devicestr); + ret = qemuMonitorTextDelDevice(mon, devalias); return ret; }
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 21b8989..1d77f99 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -335,7 +335,7 @@ int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr);
int qemuMonitorDelDevice(qemuMonitorPtr mon, - const char *devicestr); + const char *devalias);
int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ec04d79..6c73685 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1823,14 +1823,14 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, - const char *devicestr) + const char *devalias) { int ret; virJSONValuePtr cmd; virJSONValuePtr reply = NULL;
cmd = qemuMonitorJSONMakeCommand("device_del", - "s:config", devicestr, + "s:config", devalias, NULL); if (!cmd) return -1; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index e7baf84..c6ab1e8 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -166,7 +166,7 @@ int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr);
int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, - const char *devicestr); + const char *devalias);
int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 9942768..48c9a54 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2133,14 +2133,14 @@ error:
int qemuMonitorTextDelDevice(qemuMonitorPtr mon, - const char *devicestr) + const char *devalias) { char *cmd = NULL; char *reply = NULL; char *safedev; int ret = -1;
- if (!(safedev = qemuMonitorEscapeArg(devicestr))) { + if (!(safedev = qemuMonitorEscapeArg(devalias))) { virReportOOMError(); goto cleanup; } @@ -2152,13 +2152,13 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon,
if (qemuMonitorCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("cannot detach %s device"), devicestr); + _("cannot detach %s device"), devalias); goto cleanup; }
if (STRNEQ(reply, "")) { qemuReportError(VIR_ERR_OPERATION_FAILED, - _("detaching %s device failed: %s"), devicestr, reply); + _("detaching %s device failed: %s"), devalias, reply); goto cleanup; }
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index fb7d08b..3200660 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -168,7 +168,7 @@ int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr);
int qemuMonitorTextDelDevice(qemuMonitorPtr mon, - const char *devicestr); + const char *devalias);
int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr);
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
The device_add command was added in JSON mode in a way I didn't expect. Instead of passing the normal device string to the JSON command: { "execute": "device_add", "arguments": { "device": "ne2k_pci,id=nic.1,netdev=net.1" } } We need to split up the device string into a full JSON object { "execute": "device_add", "arguments": { "driver": "ne2k_pci", "id": "nic.1", "netdev": "net.1" } } * src/qemu/qemu_conf.h, src/qemu/qemu_conf.c: Rename the qemuCommandLineParseKeywords method to qemuParseKeywords and export it to monitor * src/qemu/qemu_monitor_json.c: Split up device string into a JSON object for device_add command --- src/qemu/qemu_conf.c | 30 ++++++++-------- src/qemu/qemu_conf.h | 6 +++ src/qemu/qemu_monitor_json.c | 78 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 95 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 48252a5..5b5cdb0 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -4769,11 +4769,11 @@ static const char *qemuFindEnv(const char **progenv, * the "=value" part is optional and if a key with no value is found, * NULL is be placed into corresponding place in retvalues. */ -static int -qemuParseCommandLineKeywords(const char *str, - char ***retkeywords, - char ***retvalues, - int allowEmptyValue) +int +qemuParseKeywords(const char *str, + char ***retkeywords, + char ***retvalues, + int allowEmptyValue) { int keywordCount = 0; int keywordAlloc = 0; @@ -4872,9 +4872,9 @@ qemuParseCommandLineDisk(const char *val, int busid = -1; int unitid = -1; - if ((nkeywords = qemuParseCommandLineKeywords(val, - &keywords, - &values, 0)) < 0) + if ((nkeywords = qemuParseKeywords(val, + &keywords, + &values, 0)) < 0) return NULL; if (VIR_ALLOC(def) < 0) { @@ -5112,9 +5112,9 @@ qemuParseCommandLineNet(virCapsPtr caps, tmp = strchr(val, ','); if (tmp) { - if ((nkeywords = qemuParseCommandLineKeywords(tmp+1, - &keywords, - &values, 0)) < 0) + if ((nkeywords = qemuParseKeywords(tmp+1, + &keywords, + &values, 0)) < 0) return NULL; } else { nkeywords = 0; @@ -5184,9 +5184,9 @@ qemuParseCommandLineNet(virCapsPtr caps, VIR_FREE(values); if (STRPREFIX(nic, "nic,")) { - if ((nkeywords = qemuParseCommandLineKeywords(nic + strlen("nic,"), - &keywords, - &values, 0)) < 0) { + if ((nkeywords = qemuParseKeywords(nic + strlen("nic,"), + &keywords, + &values, 0)) < 0) { virDomainNetDefFree(def); def = NULL; goto cleanup; @@ -5594,7 +5594,7 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, char *end; int ret; - nkws = qemuParseCommandLineKeywords(val, &kws, &vals, 1); + nkws = qemuParseKeywords(val, &kws, &vals, 1); if (nkws < 0) return -1; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index e0666cb..574709e 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -311,5 +311,11 @@ int qemuAssignDeviceDiskAlias(virDomainDiskDefPtr def, unsigned long long qemuCm int qemuAssignDeviceHostdevAlias(virDomainDefPtr def, virDomainHostdevDefPtr net, int idx); int qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller); +int +qemuParseKeywords(const char *str, + char ***retkeywords, + char ***retvalues, + int allowEmptyValue); + #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 6c73685..3b97d05 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1846,24 +1846,94 @@ int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, } +static void +qemuFreeKeywords(int nkeywords, char **keywords, char **values) +{ + int i; + for (i = 0 ; i < nkeywords ; i++) { + VIR_FREE(keywords[i]); + VIR_FREE(values[i]); + } + VIR_FREE(keywords); + VIR_FREE(values); +} + +static virJSONValuePtr +qemuMonitorJSONKeywordStringToJSON(const char *str, const char *firstkeyword) +{ + virJSONValuePtr ret = NULL; + char **keywords = NULL; + char **values = NULL; + int nkeywords = 0; + int i; + + if (!(ret = virJSONValueNewObject())) + goto no_memory; + + nkeywords = qemuParseKeywords(str, &keywords, &values, 1); + + if (nkeywords < 0) + goto error; + + for (i = 0 ; i < nkeywords ; i++) { + if (values[i] == NULL) { + if (i != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected empty keyword in %s"), str); + goto error; + } else { + /* This 3rd arg isn't a typo - the way the parser works is + * that the value ended up in the keyword field */ + if (virJSONValueObjectAppendString(ret, firstkeyword, keywords[i]) < 0) + goto no_memory; + } + } else { + if (virJSONValueObjectAppendString(ret, keywords[i], values[i]) < 0) + goto no_memory; + } + } + + qemuFreeKeywords(nkeywords, keywords, values); + return ret; + +no_memory: + virReportOOMError(); +error: + qemuFreeKeywords(nkeywords, keywords, values); + virJSONValueFree(ret); + return NULL; +} + + int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; + virJSONValuePtr args; - cmd = qemuMonitorJSONMakeCommand("device_add", - "s:config", devicestr, - NULL); + cmd = qemuMonitorJSONMakeCommand("device_add", NULL); if (!cmd) return -1; + args = qemuMonitorJSONKeywordStringToJSON(devicestr, "driver"); + if (!args) + goto cleanup; + + if (virJSONValueObjectAppend(cmd, "arguments", args) < 0) { + virReportOOMError(); + goto cleanup; + } + args = NULL; /* obj owns reference to args now */ + ret = qemuMonitorJSONCommand(mon, cmd, &reply); if (ret == 0) ret = qemuMonitorJSONCheckError(cmd, reply); +cleanup: + virJSONValueFree(args); virJSONValueFree(cmd); virJSONValueFree(reply); return ret; -- 1.6.6.1
On Thu, Apr 15, 2010 at 11:43:10AM +0100, Daniel P. Berrange wrote:
The device_add command was added in JSON mode in a way I didn't expect. Instead of passing the normal device string to the JSON command:
{ "execute": "device_add", "arguments": { "device": "ne2k_pci,id=nic.1,netdev=net.1" } }
We need to split up the device string into a full JSON object
{ "execute": "device_add", "arguments": { "driver": "ne2k_pci", "id": "nic.1", "netdev": "net.1" } }
haha, that explains some of the hotplug device bugs we got,
* src/qemu/qemu_conf.h, src/qemu/qemu_conf.c: Rename the qemuCommandLineParseKeywords method to qemuParseKeywords and export it to monitor * src/qemu/qemu_monitor_json.c: Split up device string into a JSON object for device_add command --- src/qemu/qemu_conf.c | 30 ++++++++-------- src/qemu/qemu_conf.h | 6 +++ src/qemu/qemu_monitor_json.c | 78 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 95 insertions(+), 19 deletions(-)
ACK, the parsing code is not trivial though, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Disk devices in QEMU have two parts, the guest device and the host backend driver. Historically these two parts have had the same "unique" name. With the switch to using -device though, they now have separate names. Thus when changing CDROM media, for guests using -device syntax, we need to prepend the QEMU_DRIVE_HOST_PREFIX constant * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Add helper function qemuDeviceDriveHostAlias() for building a host backend alias * src/qemu/qemu_driver.c: Use qemuDeviceDriveHostAlias() to determine the host backend alias for performing eject/change commands in the monitor --- src/qemu/qemu_conf.c | 20 ++++++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 18 +++++++++++++----- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 5b5cdb0..e1cfbbb 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1699,6 +1699,26 @@ static int qemuAssignDeviceDiskAliasLegacy(virDomainDiskDefPtr disk) } +char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, + unsigned long long qemudCmdFlags) +{ + char *ret; + + if (qemudCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (virAsprintf(&ret, "%s%s", QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) { + virReportOOMError(); + return NULL; + } + } else { + if (!(ret = strdup(disk->info.alias))) { + virReportOOMError(); + return NULL; + } + } + return ret; +} + + /* Names used before -drive supported the id= option */ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 574709e..b2820f0 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -220,6 +220,9 @@ char * qemuBuildNicStr(virDomainNetDefPtr net, char * qemuBuildNicDevStr(virDomainNetDefPtr net, int vlan); +char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, + unsigned long long qemudCmdFlags); + /* Both legacy & current support */ char *qemuBuildDriveStr(virDomainDiskDefPtr disk, int bootable, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df1d435..25257df 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6567,11 +6567,13 @@ cleanup: static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, virDomainObjPtr vm, - virDomainDiskDefPtr disk) + virDomainDiskDefPtr disk, + unsigned long long qemuCmdFlags) { virDomainDiskDefPtr origdisk = NULL; int i; int ret; + char *driveAlias = NULL; origdisk = NULL; for (i = 0 ; i < vm->def->ndisks ; i++) { @@ -6609,6 +6611,9 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, driver->securityDriver->domainSetSecurityImageLabel(vm, disk) < 0) return -1; + if (!(driveAlias = qemuDeviceDriveHostAlias(origdisk, qemuCmdFlags))) + goto error; + qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); if (disk->src) { @@ -6620,10 +6625,10 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, format = origdisk->driverType; } ret = qemuMonitorChangeMedia(priv->mon, - origdisk->info.alias, + driveAlias, disk->src, format); } else { - ret = qemuMonitorEjectMedia(priv->mon, origdisk->info.alias); + ret = qemuMonitorEjectMedia(priv->mon, driveAlias); } qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -6640,11 +6645,14 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, disk->src = NULL; origdisk->type = disk->type; + VIR_FREE(driveAlias); + virDomainDiskDefFree(disk); return ret; error: + VIR_FREE(driveAlias); if (driver->securityDriver && driver->securityDriver->domainRestoreSecurityImageLabel && driver->securityDriver->domainRestoreSecurityImageLabel(vm, disk) < 0) @@ -7443,7 +7451,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom, switch (dev->data.disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk); + ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk, qemuCmdFlags); if (ret == 0) dev->data.disk = NULL; break; @@ -7688,7 +7696,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, switch (dev->data.disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk); + ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk, qemuCmdFlags); if (ret == 0) dev->data.disk = NULL; break; -- 1.6.6.1
On Thu, Apr 15, 2010 at 11:43:11AM +0100, Daniel P. Berrange wrote:
Disk devices in QEMU have two parts, the guest device and the host backend driver. Historically these two parts have had the same "unique" name. With the switch to using -device though, they now have separate names. Thus when changing CDROM media, for guests using -device syntax, we need to prepend the QEMU_DRIVE_HOST_PREFIX constant
* src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Add helper function qemuDeviceDriveHostAlias() for building a host backend alias * src/qemu/qemu_driver.c: Use qemuDeviceDriveHostAlias() to determine the host backend alias for performing eject/change commands in the monitor --- src/qemu/qemu_conf.c | 20 ++++++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 18 +++++++++++++----- 3 files changed, 36 insertions(+), 5 deletions(-) [...] @@ -7443,7 +7451,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom, switch (dev->data.disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk); + ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk, qemuCmdFlags); if (ret == 0) dev->data.disk = NULL; break; @@ -7688,7 +7696,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, switch (dev->data.disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk); + ret = qemudDomainChangeEjectableMedia(driver, vm, dev->data.disk, qemuCmdFlags); if (ret == 0) dev->data.disk = NULL; break;
small nit, can we keep those two lines on 80 chars ? ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel P. Berrange -
Daniel Veillard