Bug 872292

Summary: libvirt should not attempt QMP commands that have not been documented in qemu.git
Product: Red Hat Enterprise Linux 7 Reporter: Eric Blake <eblake>
Component: libvirtAssignee: Eric Blake <eblake>
Status: CLOSED CURRENTRELEASE QA Contact: Virtualization Bugs <virt-bugs>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 7.0CC: acathrow, cwei, dallan, dyuan, eblake, lcapitulino, mzhan
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: libvirt-1.0.1-1.el7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-06-13 09:42:59 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

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):
v0.10.2-6.el6

How reproducible:
100%

Steps to Reproduce:
1. Trace the qemu monitor commands issued by 'virsh snapshot-create'
2.
3.
  
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.
https://www.redhat.com/archives/libvir-list/2012-November/msg01474.html
Comment 5 Eric Blake 2012-11-30 16:58:41 UTC 
commit 3d7f6649e87945a39808bf3ba4e7cec5df8fc13c
Author: Eric Blake <eblake>
Date:   Thu Nov 29 17:35:23 2012 -0700

    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.
    
    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:
    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 (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):
    Likewise.
Comment 6 Huang Wenlong 2012-12-20 06:15:04 UTC 
Verify this bug :
libvirt-1.0.1-1.el7.x86_64

1)

edit /etc/libvirt/libvirtd.cfg

log_level = 1

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

log_outputs="1:file:/tmp/libvirtd.log"


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
 fd=-1
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.