Bug 872292 - libvirt should not attempt QMP commands that have not been documented in qemu.git
Summary: libvirt should not attempt QMP commands that have not been documented in qemu...
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: libvirt
Version: 7.0
Hardware: Unspecified
OS: Unspecified
Target Milestone: rc
: ---
Assignee: Eric Blake
QA Contact: Virtualization Bugs
Depends On:
TreeView+ depends on / blocked
Reported: 2012-11-01 17:57 UTC by Eric Blake
Modified: 2014-06-18 00:42 UTC (History)
7 users (show)

Fixed In Version: libvirt-1.0.1-1.el7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2014-06-13 09:42:59 UTC
Target Upstream Version:

Attachments (Terms of Use)

Description Eric Blake 2012-11-01 17:57:13 UTC
Description of problem:
In libvirt's qemu_monitor_json.c, we have several commands which attempt a QMP command, then fall back to an HMP command when talking to an older qemu that has not yet implemented a QMP counterpart.  However, libvirt should never attempt a QMP command where the interface has not yet been agreed upon by the qemu folks.  As an example, at the time of this bug report, there is no QMP counterpart for the HMP 'savevm', so there is no guarantee that qemu 1.3 will add a 'savevm' command, and even if it does, it might have different semantics than what libvirt is attempting to pass.  If qemu 1.3 DOES add a 'savevm' with different semantics, then the command will fail, but not with CommandNotFound, which means libvirt won't attempt the HMP fallback, and the libvirt command that relied on this feature will be broken.

We should scrub the libvirt source code to never attempt a QMP command unless it has been documented in at least one qemu release.

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:
1. Trace the qemu monitor commands issued by 'virsh snapshot-create'
Actual results:
It attempts a QMP command named "savevm", then falls back to HMP when getting a CommandNotFound error.

Expected results:
Until qemu.git adds a QMP command for doing an internal snapshot, the libvirt command should be attempting JUST the HMP command.  If qemu.git does add a QMP command, then libvirt should be using the command correctly before looking for a CommandNotFound error.

Additional info:
This could be mitigated if qemu promises to never add commands that libvirt has speculatively used; but we can't ask qemu to poison namespace due to bad choices on libvirt's part.  I will audit the file and post the full list of affected libvirt commands in a separate message, since more than just 'savevm' is impacted.

Comment 1 Eric Blake 2012-11-01 18:09:13 UTC
Affected are:
cpu_set in qemuMonitorJSONSetCPU()
host_net_add in qemuMonitorJSONAddHostNetwork()
host_net_remove in qemuMonitorJSONRemoveHostNetwork()
drive_add in qemuMonitorJSONAttachDrive() and qemuMonitorJSONAddDrive()
drive_del in qemuMonitorJSONDriveDel()
savevm in qemuMonitorJSONCreateSnapshot()
loadvm in qemuMonitorJSONLoadSnapshot()
delvm in qemuMonitorJSONDeleteSnapshot()

Comment 4 Eric Blake 2012-11-30 00:42:00 UTC
Upstream patch proposed.

Comment 5 Eric Blake 2012-11-30 16:58:41 UTC
commit 3d7f6649e87945a39808bf3ba4e7cec5df8fc13c
Author: Eric Blake <eblake@redhat.com>
Date:   Thu Nov 29 17:35:23 2012 -0700

    qemu: don't attempt undefined QMP commands
    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.
    We also had some code that could never be reached - some of our
    commands have an alternate for new vs. old qemu HMP commands; but
    if we are new enough to support QMP, we only need a fallback to
    the new HMP counterpart, and don't need to try for a QMP counterpart
    for the old HMP version.
    See also this attempt to convert the three snapshot commands to QMP:
    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 (qemu_hotplug.c has all callers).
    * src/qemu/qemu_monitor.c (qemuMonitorAddHostNetwork)
    (qemuMonitorRemoveHostNetwork, qemuMonitorAttachDrive): Reflect
    deleted commands.
    * src/qemu/qemu_monitor_json.h (qemuMonitorJSONAddHostNetwork)
    (qemuMonitorJSONRemoveHostNetwork, qemuMonitorJSONAttachDrive):

Comment 6 Huang Wenlong 2012-12-20 06:15:04 UTC
Verify this bug :


edit /etc/libvirt/libvirtd.cfg

log_level = 1

log_filters="1:json 3:remote 4:event"


2) service libvirtd restart

3) start a guest then create system checkpointing snapshot 

#virsh snapshot-create  domain s1

4) check /tmp/libvirtd.log  about savevm 

2012-12-20 06:00:51.093+0000: 9788: debug : qemuMonitorJSONCreateSnapshot:2940 : savevm command not found, trying HMP
2012-12-20 06:00:51.093+0000: 9788: debug : virJSONValueToString:1068 : object=0x7f0cf40984c0
2012-12-20 06:00:51.093+0000: 9788: debug : virJSONValueToStringOne:999 : object=0x7f0cf40984c0 type=0 gen=0x7f0cf40cdc50
2012-12-20 06:00:51.093+0000: 9788: debug : virJSONValueToStringOne:999 : object=0x7f0cf417edd0 type=2 gen=0x7f0cf40cdc50
2012-12-20 06:00:51.093+0000: 9788: debug : virJSONValueToStringOne:999 : object=0x7f0cf40dedd0 type=0 gen=0x7f0cf40cdc50
2012-12-20 06:00:51.093+0000: 9788: debug : virJSONValueToStringOne:999 : object=0x7f0cf40172a0 type=2 gen=0x7f0cf40cdc50
2012-12-20 06:00:51.093+0000: 9788: debug : virJSONValueToStringOne:999 : object=0x7f0cf400f1b0 type=2 gen=0x7f0cf40cdc50
2012-12-20 06:00:51.093+0000: 9788: debug : virJSONValueToString:1102 : result={"execute":"human-monitor-command","arguments":{"command-line":"savevm \"s3\""},"id":"libvirt-7"}
2012-12-20 06:00:51.093+0000: 9788: debug : qemuMonitorJSONCommandWithFd:261 : Send command '{"execute":"human-monitor-command","arguments":{"command-line":"savevm \"s3\""},"id":"libvirt-7"}' for write with FD -1
2012-12-20 06:00:51.093+0000: 9788: debug : qemuMonitorSend:904 : QEMU_MONITOR_SEND_MSG: mon=0x7f0cec000d00 msg={"execute":"human-monitor-command","arguments":{"command-line":"savevm \"s3\""},"id":"libvirt-7"}^M
2012-12-20 06:00:51.093+0000: 9783: debug : virObjectRef:168 : OBJECT_REF: obj=0x7f0cec000d00
2012-12-20 06:00:51.093+0000: 9783: debug : qemuMonitorIOWrite:462 : QEMU_MONITOR_IO_WRITE: mon=0x7f0cec000d00 buf={"execute":"human-monitor-command","arguments":{"command-line":"savevm \"s3\""},"id":"libvirt-7"}^M

Comment 7 Ludek Smid 2014-06-13 09:42:59 UTC
This request was resolved in Red Hat Enterprise Linux 7.0.

Contact your manager or support representative in case you have further questions about the request.

Note You need to log in before you can comment on or make changes to this bug.