libvmi / libvmi

The official home of the LibVMI project is at https://github.com/libvmi/libvmi.
GNU Lesser General Public License v3.0
671 stars 247 forks source link

virsh qemu-monitor-command executed in QEMU usermode #393

Open Wenzel opened 8 years ago

Wenzel commented 8 years ago

Hi ! I think I found another issue in the KVM driver.

When the command pmemaccess is issued via QMP in a new process, the command line is the following --qmp: virsh qemu-monitor-command winxp '{"execute": "pmemaccess", "arguments": {"path": "/tmp/vmirazy9h"}}' which relates to this code in kvm.c:85

    int rc = snprintf(cmd, cmd_length, "virsh qemu-monitor-command %s %s", name,
             query);    
    if (rc < 0 || rc >= cmd_length) {
        errprint("Failed to properly format `virsh qemu-monitor-command`\n");
        return NULL;
    }
    dbprint(VMI_DEBUG_KVM, "--qmp: %s\n", cmd);

Without additonal arguments, virsh is trying to connect to QEMU usermode session. However we are only using the qemu:///system in the driver.

That's why i have the following output :

LibVMI Version 0.11.0
--found KVM
LibVMI Mode 4
--completed driver init.
--got id from name (winxp --> 6)
**set image_type = winxp
--libvirt version 1003003
--qmp: virsh qemu-monitor-command winxp '{"execute": "pmemaccess", "arguments": {"path": "/tmp/vmirazy9h"}}'
erreur :Requested operation is not valid: domain is not running
--kvm: using custom patch for fast memory access
--connect() failed to /tmp/vmirazy9h
Failed to init LibVMI library.

We should also hardcode qemu:///system when we create the virsh command line : `virsh -c qemu:///system qemu-monitor-command winxp '{"execute": "pmemaccess", "arguments": {"path": "/tmp/vmirazy9h"}}'``

PS: could it be possible to implement the domain lookup in qemu:///session also ? :)

Thanks !

Wenzel commented 8 years ago

I created PR #394 to fix this.

bdpayne commented 8 years ago

Sorry, I missed this when I commented on the PR. You're right, we have hard-coded qemu:///system elsewhere so we might as well do it again to be consistent. I think that it would be useful to have support for session one of these days too, but it looks like that could take a little more refactoring. For now, I think that your PR is the way to go. Thanks.