[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: don't attempt undefined QMP commands



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=5dfa0523915e1b3ea88d674492c3eb3ffb81a24f;hb=HEAD#l2297

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]