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 :|