[libvirt] [PATCH] qemu: don't attempt undefined QMP commands
https://bugzilla.redhat.com/show_bug.cgi?id=872292 Libvirt should not attempt to call a QMP command that has not been documented in qemu.git - if future qemu introduces a command by the same name but with subtly different semantics, then libvirt will be broken when trying to use that command. See also this attempt to convert the three snapshot commands to QMP: https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg01597.html although it looks like that will still not happen before qemu 1.3. That thread eventually decided that qemu would use the name 'save-vm' rather than 'savevm', which mitigates the fact that libvirt's attempt to use a QMP 'savevm' would be broken, but we might not be as lucky on the other commands. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONSetCPU) (qemuMonitorJSONAddDrive, qemuMonitorJSONDriveDel) (qemuMonitorJSONCreateSnapshot, qemuMonitorJSONLoadSnapshot) (qemuMonitorJSONDeleteSnapshot): Use only HMP fallback for now. (qemuMonitorJSONAddHostNetwork, qemuMonitorJSONRemoveHostNetwork) (qemuMonitorJSONAttachDrive, qemuMonitorJSONGetGuestDriveAddress): Delete; QMP implies QEMU_CAPS_DEVICE, which prefers AddNetdev, RemoveNetdev, and AddDrive anyways. * src/qemu/qemu_monitor.c (qemuMonitorAddHostNetwork) (qemuMonitorRemoveHostNetwork, qemuMonitorAttachDrive): Reflect deleted commands. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONAddHostNetwork) (qemuMonitorJSONRemoveHostNetwork, qemuMonitorJSONAttachDrive): Likewise. --- src/qemu/qemu_monitor.c | 9 +- src/qemu/qemu_monitor_json.c | 311 ++++-------------------------------------- src/qemu/qemu_monitor_json.h | 12 -- 3 files changed, 31 insertions(+), 301 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index aef5044..43e45ef 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2387,7 +2387,8 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, } if (mon->json) - ret = qemuMonitorJSONAddHostNetwork(mon, netstr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using netdev_add")); else ret = qemuMonitorTextAddHostNetwork(mon, netstr); @@ -2418,7 +2419,8 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, } if (mon->json) - ret = qemuMonitorJSONRemoveHostNetwork(mon, vlan, netname); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using netdev_del")); else ret = qemuMonitorTextRemoveHostNetwork(mon, vlan, netname); return ret; @@ -2548,7 +2550,8 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon, } if (mon->json) - ret = qemuMonitorJSONAttachDrive(mon, drivestr, controllerAddr, driveAddr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using AddDrive")); else ret = qemuMonitorTextAttachDrive(mon, drivestr, controllerAddr, driveAddr); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9fcd7e8..195d89f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2094,44 +2094,9 @@ cleanup: int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, int cpu, int online) { - int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("cpu_set", - "U:cpu", (unsigned long long)cpu, - "s:state", online ? "online" : "offline", - NULL); - virJSONValuePtr reply = NULL; - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("cpu_set command not found, trying HMP"); - ret = qemuMonitorTextSetCPU(mon, cpu, online); - goto cleanup; - } - - if (ret == 0) { - /* XXX See if CPU soft-failed due to lack of ACPI */ -#if 0 - if (qemuMonitorJSONHasError(reply, "DeviceNotActive") || - qemuMonitorJSONHasError(reply, "KVMMissingCap")) - goto cleanup; -#endif - - /* See if any other fatal error occurred */ - ret = qemuMonitorJSONCheckError(cmd, reply); - - /* Real success */ - if (ret == 0) - ret = 1; - } - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for cpu hotplug */ + VIR_DEBUG("no QMP support for cpu_set, trying HMP"); + return qemuMonitorTextSetCPU(mon, cpu, online); } @@ -2674,52 +2639,6 @@ int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon, } -int qemuMonitorJSONAddHostNetwork(qemuMonitorPtr mon, - const char *netstr) -{ - int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("host_net_add", - "s:device", netstr, - 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 qemuMonitorJSONRemoveHostNetwork(qemuMonitorPtr mon, - int vlan, - const char *netname) -{ - int ret; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("host_net_remove", - "i:vlan", vlan, - "s:device", netname, - 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 qemuMonitorJSONAddNetdev(qemuMonitorPtr mon, const char *netdevstr) { @@ -2886,74 +2805,6 @@ int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } -static int -qemuMonitorJSONGetGuestDriveAddress(virJSONValuePtr reply, - virDomainDeviceDriveAddress *driveAddr) -{ - virJSONValuePtr addr; - - addr = virJSONValueObjectGet(reply, "return"); - if (!addr || addr->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("drive_add reply was missing device address")); - return -1; - } - - if (virJSONValueObjectGetNumberUint(addr, "bus", &driveAddr->bus) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("drive_add reply was missing device bus number")); - return -1; - } - - if (virJSONValueObjectGetNumberUint(addr, "unit", &driveAddr->unit) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("drive_add reply was missing device unit number")); - return -1; - } - - return 0; -} - - -int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon, - const char *drivestr, - virDevicePCIAddress* controllerAddr, - virDomainDeviceDriveAddress* driveAddr) -{ - int ret; - virJSONValuePtr cmd = NULL; - virJSONValuePtr reply = NULL; - char *dev; - - if (virAsprintf(&dev, "%.2x:%.2x.%.1x", - controllerAddr->bus, controllerAddr->slot, controllerAddr->function) < 0) { - virReportOOMError(); - return -1; - } - - cmd = qemuMonitorJSONMakeCommand("drive_add", - "s:pci_addr", dev, - "s:opts", drivestr, - NULL); - VIR_FREE(dev); - if (!cmd) - return -1; - - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret == 0 && - qemuMonitorJSONGetGuestDriveAddress(reply, driveAddr) < 0) - ret = -1; - - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; -} - - int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuMonitorPCIAddress **addrs ATTRIBUTE_UNUSED) { @@ -3025,32 +2876,9 @@ cleanup: int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommand("drive_add", - "s:pci_addr", "dummy", - "s:opts", drivestr, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply) < 0)) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("drive_add command not found, trying HMP"); - ret = qemuMonitorTextAddDrive(mon, drivestr); - goto cleanup; - } - - ret = qemuMonitorJSONCheckError(cmd, reply); - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for drive_add */ + VIR_DEBUG("drive_add command not found, trying HMP"); + return qemuMonitorTextAddDrive(mon, drivestr); } @@ -3058,42 +2886,19 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon, const char *drivestr) { int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - VIR_DEBUG("JSONDriveDel drivestr=%s", drivestr); - cmd = qemuMonitorJSONMakeCommand("drive_del", - "s:id", drivestr, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("drive_del command not found, trying HMP"); - if ((ret = qemuMonitorTextDriveDel(mon, drivestr)) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->code == VIR_ERR_OPERATION_UNSUPPORTED) { - VIR_ERROR("%s", - _("deleting disk is not supported. " - "This may leak data if disk is reassigned")); - ret = 1; - virResetLastError(); - } + /* XXX Update to use QMP, if QMP ever adds support for drive_del */ + VIR_DEBUG("drive_del command not found, trying HMP"); + if ((ret = qemuMonitorTextDriveDel(mon, drivestr)) < 0) { + virErrorPtr err = virGetLastError(); + if (err && err->code == VIR_ERR_OPERATION_UNSUPPORTED) { + VIR_ERROR("%s", + _("deleting disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + virResetLastError(); } - } 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; } @@ -3131,89 +2936,23 @@ int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, int qemuMonitorJSONCreateSnapshot(qemuMonitorPtr mon, const char *name) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommand("savevm", - "s:name", name, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("savevm command not found, trying HMP"); - ret = qemuMonitorTextCreateSnapshot(mon, name); - goto cleanup; - } - - ret = qemuMonitorJSONCheckError(cmd, reply); - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for savevm */ + VIR_DEBUG("savevm command not found, trying HMP"); + return qemuMonitorTextCreateSnapshot(mon, name); } int qemuMonitorJSONLoadSnapshot(qemuMonitorPtr mon, const char *name) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommand("loadvm", - "s:name", name, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("loadvm command not found, trying HMP"); - ret = qemuMonitorTextLoadSnapshot(mon, name); - goto cleanup; - } - - ret = qemuMonitorJSONCheckError(cmd, reply); - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for loadvm */ + VIR_DEBUG("loadvm command not found, trying HMP"); + return qemuMonitorTextLoadSnapshot(mon, name); } int qemuMonitorJSONDeleteSnapshot(qemuMonitorPtr mon, const char *name) { - int ret; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; - - cmd = qemuMonitorJSONMakeCommand("delvm", - "s:name", name, - NULL); - if (!cmd) - return -1; - - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - VIR_DEBUG("delvm command not found, trying HMP"); - ret = qemuMonitorTextDeleteSnapshot(mon, name); - goto cleanup; - } - - ret = qemuMonitorJSONCheckError(cmd, reply); - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + /* XXX Update to use QMP, if QMP ever adds support for delvm */ + VIR_DEBUG("delvm command not found, trying HMP"); + return qemuMonitorTextDeleteSnapshot(mon, name); } int diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c62ae24..acca4ec 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -179,13 +179,6 @@ int qemuMonitorJSONSendFileHandle(qemuMonitorPtr mon, int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon, const char *fdname); -int qemuMonitorJSONAddHostNetwork(qemuMonitorPtr mon, - const char *netstr); - -int qemuMonitorJSONRemoveHostNetwork(qemuMonitorPtr mon, - int vlan, - const char *netname); - int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon, const char *netdevstr); @@ -199,11 +192,6 @@ int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, virDevicePCIAddress *guestAddr); -int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon, - const char *drivestr, - virDevicePCIAddress *controllerAddr, - virDomainDeviceDriveAddress *driveAddr); - int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon, qemuMonitorPCIAddress **addrs); -- 1.7.1
On 30.11.2012 01:40, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=872292
Libvirt should not attempt to call a QMP command that has not been documented in qemu.git - if future qemu introduces a command by the same name but with subtly different semantics, then libvirt will be broken when trying to use that command.
See also this attempt to convert the three snapshot commands to QMP: https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg01597.html although it looks like that will still not happen before qemu 1.3. That thread eventually decided that qemu would use the name 'save-vm' rather than 'savevm', which mitigates the fact that libvirt's attempt to use a QMP 'savevm' would be broken, but we might not be as lucky on the other commands.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONSetCPU) (qemuMonitorJSONAddDrive, qemuMonitorJSONDriveDel) (qemuMonitorJSONCreateSnapshot, qemuMonitorJSONLoadSnapshot) (qemuMonitorJSONDeleteSnapshot): Use only HMP fallback for now. (qemuMonitorJSONAddHostNetwork, qemuMonitorJSONRemoveHostNetwork) (qemuMonitorJSONAttachDrive, qemuMonitorJSONGetGuestDriveAddress): Delete; QMP implies QEMU_CAPS_DEVICE, which prefers AddNetdev, RemoveNetdev, and AddDrive anyways. * src/qemu/qemu_monitor.c (qemuMonitorAddHostNetwork) (qemuMonitorRemoveHostNetwork, qemuMonitorAttachDrive): Reflect deleted commands. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONAddHostNetwork) (qemuMonitorJSONRemoveHostNetwork, qemuMonitorJSONAttachDrive): Likewise. --- src/qemu/qemu_monitor.c | 9 +- src/qemu/qemu_monitor_json.c | 311 ++++-------------------------------------- src/qemu/qemu_monitor_json.h | 12 -- 3 files changed, 31 insertions(+), 301 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index aef5044..43e45ef 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2387,7 +2387,8 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, }
if (mon->json) - ret = qemuMonitorJSONAddHostNetwork(mon, netstr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using netdev_add")); else ret = qemuMonitorTextAddHostNetwork(mon, netstr);
I might be not getting something, but netdev_add seems documented for me: http://git.qemu.org/?p=qemu.git;a=blob;f=qapi-schema.json;h=5dfa0523915e1b3e... The command is there since v1.2.0-rc0~314^2~2 (at least that's what 'git describe --tags --contains 928059a3' says). And if qemu developers are changing its semantic then they should not. I haven't checked the other commands you are removing. Michal
On Fri, Nov 30, 2012 at 09:01:47AM +0100, Michal Privoznik wrote:
On 30.11.2012 01:40, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=872292
Libvirt should not attempt to call a QMP command that has not been documented in qemu.git - if future qemu introduces a command by the same name but with subtly different semantics, then libvirt will be broken when trying to use that command.
See also this attempt to convert the three snapshot commands to QMP: https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg01597.html although it looks like that will still not happen before qemu 1.3. That thread eventually decided that qemu would use the name 'save-vm' rather than 'savevm', which mitigates the fact that libvirt's attempt to use a QMP 'savevm' would be broken, but we might not be as lucky on the other commands.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONSetCPU) (qemuMonitorJSONAddDrive, qemuMonitorJSONDriveDel) (qemuMonitorJSONCreateSnapshot, qemuMonitorJSONLoadSnapshot) (qemuMonitorJSONDeleteSnapshot): Use only HMP fallback for now. (qemuMonitorJSONAddHostNetwork, qemuMonitorJSONRemoveHostNetwork) (qemuMonitorJSONAttachDrive, qemuMonitorJSONGetGuestDriveAddress): Delete; QMP implies QEMU_CAPS_DEVICE, which prefers AddNetdev, RemoveNetdev, and AddDrive anyways. * src/qemu/qemu_monitor.c (qemuMonitorAddHostNetwork) (qemuMonitorRemoveHostNetwork, qemuMonitorAttachDrive): Reflect deleted commands. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONAddHostNetwork) (qemuMonitorJSONRemoveHostNetwork, qemuMonitorJSONAttachDrive): Likewise. --- src/qemu/qemu_monitor.c | 9 +- src/qemu/qemu_monitor_json.c | 311 ++++-------------------------------------- src/qemu/qemu_monitor_json.h | 12 -- 3 files changed, 31 insertions(+), 301 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index aef5044..43e45ef 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2387,7 +2387,8 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, }
if (mon->json) - ret = qemuMonitorJSONAddHostNetwork(mon, netstr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using netdev_add")); else ret = qemuMonitorTextAddHostNetwork(mon, netstr);
I might be not getting something, but netdev_add seems documented for me:
You're mis-reading the patch - check the mesage again :-) Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, Nov 29, 2012 at 05:40:20PM -0700, Eric Blake wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=872292
Libvirt should not attempt to call a QMP command that has not been documented in qemu.git - if future qemu introduces a command by the same name but with subtly different semantics, then libvirt will be broken when trying to use that command.
The reason these were added was that back in the first days of QMP the intention was that every HMP command would have an identical QMP command. This plan changed before it was ever completed, hence the situation we're in now.
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index aef5044..43e45ef 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2387,7 +2387,8 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, }
if (mon->json) - ret = qemuMonitorJSONAddHostNetwork(mon, netstr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using netdev_add")); else ret = qemuMonitorTextAddHostNetwork(mon, netstr);
@@ -2418,7 +2419,8 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, }
if (mon->json) - ret = qemuMonitorJSONRemoveHostNetwork(mon, vlan, netname); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using netdev_del"));
In these two you recommend different commands
else ret = qemuMonitorTextRemoveHostNetwork(mon, vlan, netname); return ret; @@ -2548,7 +2550,8 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon, }
if (mon->json) - ret = qemuMonitorJSONAttachDrive(mon, drivestr, controllerAddr, driveAddr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using AddDrive"));
while this one recommends a different method ACK you standardize on one. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
https://bugzilla.redhat.com/show_bug.cgi?id=872292
Libvirt should not attempt to call a QMP command that has not been documented in qemu.git - if future qemu introduces a command by the same name but with subtly different semantics, then libvirt will be broken when trying to use that command.
The reason these were added was that back in the first days of QMP the intention was that every HMP command would have an identical QMP command. This plan changed before it was ever completed, hence the situation we're in now.
Yep :)
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using netdev_del"));
In these two you recommend different commands
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor should be using AddDrive"));
while this one recommends a different method
ACK you standardize on one.
I standardized on the method name, improved the commit message to give more details about the commands being deleted from QMP support (since they were dead code based on qemu_hotplug.c usage patterns), and pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Michal Privoznik