[libvirt] [PATCH 0/4] Enable libvirt to attach to existing QEMU instances
This is the bare minimum code required to allow libvirt to connect to an externally launched QEMU instance, reverse engineering the XML configuration from the command line args. $ qemu-kvm -cdrom ~/demo.iso \ -monitor unix:/tmp/demo,server,nowait \ -name foo \ -uuid cece4f9f-dff0-575d-0e8e-01fe380f12ea & $ QEMUPID=$$ $ virsh qemu-attach $QEMUPID Domain foo attached to pid 10079 $ ./tools/virsh dumpxml foo <domain type='kvm' id='3'> <name>foo</name> <uuid>cece4f9f-dff0-575d-0e8e-01fe380f12ea</uuid> <memory>65536</memory> <currentMemory>65536</currentMemory> <vcpu>1</vcpu> <os> <type arch='i686' machine='pc-0.14'>hvm</type> </os> <features> <acpi/> <pae/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu-kvm</emulator> <disk type='file' device='cdrom'> <source file='/home/berrange/demo.iso'/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' unit='0'/> </disk> <controller type='ide' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='5901' autoport='no'/> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </memballoon> </devices> </domain> The biggest problem is that our ARGV-> XML convertor does not know about the -device syntax people often now use. There is also some robustness work to be done the QEMU driver in general, to remove some assumptions that no longer hold true. For example in our domain shutdown code, there are bits of cleanup we should skip because we will not have done the corresponding setup in the first place. We also need to verify what happens with all the various APIs particularly device hotplug, to ensure there are no bad results. That said, even as it is, this functionality will be quite useful for QEMU developers
Introduce a new API in libvirt-qemu.so virDomainPtr virDomainQemuAttach(virConnectPtr domain, int pid, unsigned int flags); This allows libvirtd to attach to an existing, externally launched QEMU process. This is useful for QEMU developers who prefer to launch QEMU themselves for debugging/devel reasons, but still want the benefit of libvirt based tools like virt-top, virt-viewer, etc * include/libvirt/libvirt-qemu.h: Define virDomainQemuAttach * src/driver.h, src/libvirt-qemu.c, src/libvirt_qemu.syms: Driver glue for virDomainQemuAttach * src/esx/esx_driver.c, src/libxl/libxl_driver.c, src/lxc/lxc_driver.c, src/openvz/openvz_driver.c, src/phyp/phyp_driver.c, src/qemu/qemu_driver.c, src/qemu/qemu_driver.c, src/test/test_driver.c, src/uml/uml_driver.c, src/vbox/vbox_tmpl.c, src/vmware/vmware_driver.c, src/xen/xen_driver.c, src/xenapi/xenapi_driver.c: Driver entry stub --- include/libvirt/libvirt-qemu.h | 4 ++++ src/driver.h | 5 +++++ src/esx/esx_driver.c | 1 + src/libvirt-qemu.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_qemu.syms | 5 +++++ src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + 17 files changed, 67 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h index f172eff..a182739 100644 --- a/include/libvirt/libvirt-qemu.h +++ b/include/libvirt/libvirt-qemu.h @@ -28,6 +28,10 @@ typedef enum { int virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, char **result, unsigned int flags); +virDomainPtr virDomainQemuAttach(virConnectPtr domain, + int pid, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/driver.h b/src/driver.h index a8b79e6..4cf6cbd 100644 --- a/src/driver.h +++ b/src/driver.h @@ -509,6 +509,10 @@ typedef int (*virDrvQemuDomainMonitorCommand)(virDomainPtr domain, const char *cmd, char **result, unsigned int flags); +typedef virDomainPtr + (*virDrvQemuDomainAttach)(virConnectPtr conn, int pid, + unsigned int flags); + typedef int (*virDrvDomainOpenConsole)(virDomainPtr dom, const char *devname, @@ -638,6 +642,7 @@ struct _virDriver { virDrvDomainRevertToSnapshot domainRevertToSnapshot; virDrvDomainSnapshotDelete domainSnapshotDelete; virDrvQemuDomainMonitorCommand qemuDomainMonitorCommand; + virDrvQemuDomainAttach qemuDomainAttach; virDrvDomainOpenConsole domainOpenConsole; }; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index c958197..061c857 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4698,6 +4698,7 @@ static virDriver esxDriver = { esxDomainRevertToSnapshot, /* domainRevertToSnapshot */ esxDomainSnapshotDelete, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ NULL, /* domainOpenConsole */ }; diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c index 46727c8..4c4d2e7 100644 --- a/src/libvirt-qemu.c +++ b/src/libvirt-qemu.c @@ -79,3 +79,43 @@ error: virDispatchError(conn); return -1; } + + +virDomainPtr +virDomainQemuAttach(virConnectPtr conn, int pid, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, pid=%d, flags=%u", conn, pid, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + + if (pid <= 1) { + virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->driver->qemuDomainAttach) { + virDomainPtr ret; + ret = conn->driver->qemuDomainAttach(conn, pid, flags); + if (!ret) + goto error; + return ret; + } + + virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return NULL; +} diff --git a/src/libvirt_qemu.syms b/src/libvirt_qemu.syms index 5702d36..90e6b9e 100644 --- a/src/libvirt_qemu.syms +++ b/src/libvirt_qemu.syms @@ -14,3 +14,8 @@ LIBVIRT_QEMU_0.8.3 { global: virDomainQemuMonitorCommand; }; + +LIBVIRT_QEMU_0.9.2 { + global: + virDomainQemuAttach; +} LIBVIRT_QEMU_0.8.3; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index dec4f43..31c9881 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2745,6 +2745,7 @@ static virDriver libxlDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ NULL, /* domainOpenConsole */ }; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b94941d..56857ee 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2904,6 +2904,7 @@ static virDriver lxcDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ lxcDomainOpenConsole, /* domainOpenConsole */ }; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 4af28e9..7fd8f15 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1666,6 +1666,7 @@ static virDriver openvzDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ NULL, /* domainOpenConsole */ }; diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index ebd4a8a..3b9c187 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3827,6 +3827,7 @@ static virDriver phypDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuMonitorCommand */ + NULL, /* qemuDomainAttach */ NULL, /* domainOpenConsole */ }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dd89786..24447b9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7193,6 +7193,7 @@ static virDriver qemuDriver = { qemuDomainRevertToSnapshot, /* domainRevertToSnapshot */ qemuDomainSnapshotDelete, /* domainSnapshotDelete */ qemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ qemuDomainOpenConsole, /* domainOpenConsole */ }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e30780c..8137faa 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -11297,6 +11297,7 @@ static virDriver remote_driver = { remoteDomainRevertToSnapshot, /* domainRevertToSnapshot */ remoteDomainSnapshotDelete, /* domainSnapshotDelete */ remoteQemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ remoteDomainOpenConsole, /* domainOpenConsole */ }; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0978214..7181753 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5446,6 +5446,7 @@ static virDriver testDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ NULL, /* domainOpenConsole */ }; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 33849a0..2784542 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2252,6 +2252,7 @@ static virDriver umlDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ umlDomainOpenConsole, /* domainOpenConsole */ }; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 8241d34..d24b883 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -8651,6 +8651,7 @@ virDriver NAME(Driver) = { vboxDomainRevertToSnapshot, /* domainRevertToSnapshot */ vboxDomainSnapshotDelete, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ NULL, /* domainOpenConsole */ }; diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index bbfb1a4..c902fc0 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -1006,6 +1006,7 @@ static virDriver vmwareDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ NULL, /* domainOpenConsole */ }; diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index dd94fbc..2bb345b 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2207,6 +2207,7 @@ static virDriver xenUnifiedDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ xenUnifiedDomainOpenConsole, /* domainOpenConsole */ }; diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 3fbdcc6..1273797 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1888,6 +1888,7 @@ static virDriver xenapiDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ + NULL, /* qemuDomainAttach */ NULL, /* domainOpenConsole */ }; -- 1.7.4.4
On 05/05/2011 11:26 AM, Daniel P. Berrange wrote:
Introduce a new API in libvirt-qemu.so
virDomainPtr virDomainQemuAttach(virConnectPtr domain, int pid, unsigned int flags);
Do we want pid_t instead of pid? Or are we guaranteed that this only works on platforms with /proc/nnn, and that all such platforms already have sizeof(pid_t)<=sizeof(int)? I ask because it would be a shame if there is ever a system which supports more than 2G simultaneous processes [pid_t is signed, but negative pids are groups rather than processes], yet we locked ourselves in to a capped pid size. Or, it is quite conceivable for a platform that represents pid_t as the same size as void* (basically, the pointer to the physical memory address in the kernel where the process information is stored), and if such a platform is 64-bits, then even if you can't have 2G simultaneous processes, you still can have large pids. But my guess is that it is not a problem in practice (64-bit Linux uses 4-byte pid_t, and changing that now would be a major ABI change, and most systems, even if they randomize pids rather than assigning the sequentially, still return pids as an index into a kernel array, rather than as an actual kernel pointer). So I guess I'm okay with: ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Thu, May 05, 2011 at 04:33:06PM -0600, Eric Blake wrote:
On 05/05/2011 11:26 AM, Daniel P. Berrange wrote:
Introduce a new API in libvirt-qemu.so
virDomainPtr virDomainQemuAttach(virConnectPtr domain, int pid, unsigned int flags);
Do we want pid_t instead of pid? Or are we guaranteed that this only works on platforms with /proc/nnn, and that all such platforms already have sizeof(pid_t)<=sizeof(int)?
I didn't want to use pid_t, since that doesn't exist on Windows and we can't assume app developers are using gnulib for portability.
I ask because it would be a shame if there is ever a system which supports more than 2G simultaneous processes [pid_t is signed, but negative pids are groups rather than processes], yet we locked ourselves in to a capped pid size.
Or, it is quite conceivable for a platform that represents pid_t as the same size as void* (basically, the pointer to the physical memory address in the kernel where the process information is stored), and if such a platform is 64-bits, then even if you can't have 2G simultaneous processes, you still can have large pids.
But my guess is that it is not a problem in practice (64-bit Linux uses 4-byte pid_t, and changing that now would be a major ABI change, and most systems, even if they randomize pids rather than assigning the sequentially, still return pids as an index into a kernel array, rather than as an actual kernel pointer).
Does anything think is it remotely likely that any common platform will use a 64-bit PID ? I'm sceptical, so I'd think 'int' is fine for the API. Worst case, we have to add another API later
So I guess I'm okay with:
ACK.
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 :|
On 05/11/2011 07:09 AM, Daniel P. Berrange wrote:
On Thu, May 05, 2011 at 04:33:06PM -0600, Eric Blake wrote:
On 05/05/2011 11:26 AM, Daniel P. Berrange wrote:
Introduce a new API in libvirt-qemu.so
virDomainPtr virDomainQemuAttach(virConnectPtr domain, int pid, unsigned int flags);
Do we want pid_t instead of pid? Or are we guaranteed that this only works on platforms with /proc/nnn, and that all such platforms already have sizeof(pid_t)<=sizeof(int)?
I didn't want to use pid_t, since that doesn't exist on Windows and we can't assume app developers are using gnulib for portability.
Ah, I see - while pid_t is fine internally to libvirt, it is not a good idea to expose it in a public header since we can't expose gnulib's substitute types. We may want to add a verify(sizeof(pid_t) <= sizeof(int)) somewhere in our source code, to ensure we get a compile error if our assumption is ever violated.
Does anything think is it remotely likely that any common platform will use a 64-bit PID ? I'm sceptical,
As am I. Using 'int' should be fine, then. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* daemon/remote.c: Server side dispatcher * src/remote/remote_driver.c: Client side dispatcher * src/remote/qemu_protocol.x: Wire protocol definition --- daemon/qemu_dispatch_args.h | 1 + daemon/qemu_dispatch_prototypes.h | 8 ++++++++ daemon/qemu_dispatch_ret.h | 1 + daemon/qemu_dispatch_table.h | 5 +++++ daemon/remote.c | 35 +++++++++++++++++++++++++++++++++++ src/remote/qemu_protocol.c | 20 ++++++++++++++++++++ src/remote/qemu_protocol.h | 16 ++++++++++++++++ src/remote/qemu_protocol.x | 13 ++++++++++++- src/remote/remote_driver.c | 30 +++++++++++++++++++++++++++++- 9 files changed, 127 insertions(+), 2 deletions(-) diff --git a/daemon/qemu_dispatch_args.h b/daemon/qemu_dispatch_args.h index e278fa4..c85b20e 100644 --- a/daemon/qemu_dispatch_args.h +++ b/daemon/qemu_dispatch_args.h @@ -3,3 +3,4 @@ */ qemu_monitor_command_args val_qemu_monitor_command_args; + qemu_attach_args val_qemu_attach_args; diff --git a/daemon/qemu_dispatch_prototypes.h b/daemon/qemu_dispatch_prototypes.h index 4ec1ab4..d22e1d8 100644 --- a/daemon/qemu_dispatch_prototypes.h +++ b/daemon/qemu_dispatch_prototypes.h @@ -2,6 +2,14 @@ * Do not edit this file. Any changes you make will be lost. */ +static int qemuDispatchAttach( + struct qemud_server *server, + struct qemud_client *client, + virConnectPtr conn, + remote_message_header *hdr, + remote_error *rerr, + qemu_attach_args *args, + qemu_attach_ret *ret); static int qemuDispatchMonitorCommand( struct qemud_server *server, struct qemud_client *client, diff --git a/daemon/qemu_dispatch_ret.h b/daemon/qemu_dispatch_ret.h index 492dcf9..20392c4 100644 --- a/daemon/qemu_dispatch_ret.h +++ b/daemon/qemu_dispatch_ret.h @@ -3,3 +3,4 @@ */ qemu_monitor_command_ret val_qemu_monitor_command_ret; + qemu_attach_ret val_qemu_attach_ret; diff --git a/daemon/qemu_dispatch_table.h b/daemon/qemu_dispatch_table.h index c196a3c..9084a74 100644 --- a/daemon/qemu_dispatch_table.h +++ b/daemon/qemu_dispatch_table.h @@ -12,3 +12,8 @@ .args_filter = (xdrproc_t) xdr_qemu_monitor_command_args, .ret_filter = (xdrproc_t) xdr_qemu_monitor_command_ret, }, +{ /* Attach => 2 */ + .fn = (dispatch_fn) qemuDispatchAttach, + .args_filter = (xdrproc_t) xdr_qemu_attach_args, + .ret_filter = (xdrproc_t) xdr_qemu_attach_ret, +}, diff --git a/daemon/remote.c b/daemon/remote.c index 214f7a4..d3b7401 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -8693,6 +8693,41 @@ cleanup: static int +qemuDispatchAttach(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + qemu_attach_args *args, + qemu_attach_ret *ret) +{ + virDomainPtr dom = NULL; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = virDomainQemuAttach(conn, + args->pid, + args->flags))) + goto cleanup; + + make_nonnull_domain(&ret->dom, dom); + + rv = 0; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + + +static int remoteDispatchDomainOpenConsole(struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client, virConnectPtr conn, diff --git a/src/remote/qemu_protocol.c b/src/remote/qemu_protocol.c index 81916ed..b1a8d9f 100644 --- a/src/remote/qemu_protocol.c +++ b/src/remote/qemu_protocol.c @@ -32,6 +32,26 @@ xdr_qemu_monitor_command_ret (XDR *xdrs, qemu_monitor_command_ret *objp) } bool_t +xdr_qemu_attach_args (XDR *xdrs, qemu_attach_args *objp) +{ + + if (!xdr_int (xdrs, &objp->pid)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->flags)) + return FALSE; + return TRUE; +} + +bool_t +xdr_qemu_attach_ret (XDR *xdrs, qemu_attach_ret *objp) +{ + + if (!xdr_remote_nonnull_domain (xdrs, &objp->dom)) + return FALSE; + return TRUE; +} + +bool_t xdr_qemu_procedure (XDR *xdrs, qemu_procedure *objp) { diff --git a/src/remote/qemu_protocol.h b/src/remote/qemu_protocol.h index b822187..1654450 100644 --- a/src/remote/qemu_protocol.h +++ b/src/remote/qemu_protocol.h @@ -28,11 +28,23 @@ struct qemu_monitor_command_ret { remote_nonnull_string result; }; typedef struct qemu_monitor_command_ret qemu_monitor_command_ret; + +struct qemu_attach_args { + int pid; + u_int flags; +}; +typedef struct qemu_attach_args qemu_attach_args; + +struct qemu_attach_ret { + remote_nonnull_domain dom; +}; +typedef struct qemu_attach_ret qemu_attach_ret; #define QEMU_PROGRAM 0x20008087 #define QEMU_PROTOCOL_VERSION 1 enum qemu_procedure { QEMU_PROC_MONITOR_COMMAND = 1, + QEMU_PROC_ATTACH = 2, }; typedef enum qemu_procedure qemu_procedure; @@ -41,11 +53,15 @@ typedef enum qemu_procedure qemu_procedure; #if defined(__STDC__) || defined(__cplusplus) extern bool_t xdr_qemu_monitor_command_args (XDR *, qemu_monitor_command_args*); extern bool_t xdr_qemu_monitor_command_ret (XDR *, qemu_monitor_command_ret*); +extern bool_t xdr_qemu_attach_args (XDR *, qemu_attach_args*); +extern bool_t xdr_qemu_attach_ret (XDR *, qemu_attach_ret*); extern bool_t xdr_qemu_procedure (XDR *, qemu_procedure*); #else /* K&R C */ extern bool_t xdr_qemu_monitor_command_args (); extern bool_t xdr_qemu_monitor_command_ret (); +extern bool_t xdr_qemu_attach_args (); +extern bool_t xdr_qemu_attach_ret (); extern bool_t xdr_qemu_procedure (); #endif /* K&R C */ diff --git a/src/remote/qemu_protocol.x b/src/remote/qemu_protocol.x index 1d07895..33b3c53 100644 --- a/src/remote/qemu_protocol.x +++ b/src/remote/qemu_protocol.x @@ -37,10 +37,21 @@ struct qemu_monitor_command_ret { remote_nonnull_string result; }; + +struct qemu_attach_args { + int pid; + unsigned int flags; +}; + +struct qemu_attach_ret { + remote_nonnull_domain dom; +}; + /* Define the program number, protocol version and procedure numbers here. */ const QEMU_PROGRAM = 0x20008087; const QEMU_PROTOCOL_VERSION = 1; enum qemu_procedure { - QEMU_PROC_MONITOR_COMMAND = 1 + QEMU_PROC_MONITOR_COMMAND = 1, + QEMU_PROC_ATTACH = 2 }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 8137faa..0f0ad6c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -9770,6 +9770,34 @@ done: return rv; } +static virDomainPtr +remoteQemuDomainAttach(virConnectPtr conn, int pid, + unsigned int flags) +{ + virDomainPtr dom = NULL; + qemu_attach_args args; + qemu_attach_ret ret; + struct private_data *priv = conn->privateData; + + remoteDriverLock(priv); + + args.pid = pid; + args.flags = flags; + + memset (&ret, 0, sizeof ret); + if (call (conn, priv, REMOTE_CALL_QEMU, QEMU_PROC_ATTACH, + (xdrproc_t) xdr_qemu_attach_args, (char *) &args, + (xdrproc_t) xdr_qemu_attach_ret, (char *) &ret) == -1) + goto done; + + dom = get_nonnull_domain(conn, ret.dom); + xdr_free((xdrproc_t) &xdr_qemu_attach_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return dom; +} + /*----------------------------------------------------------------------*/ static struct remote_thread_call * @@ -11297,7 +11325,7 @@ static virDriver remote_driver = { remoteDomainRevertToSnapshot, /* domainRevertToSnapshot */ remoteDomainSnapshotDelete, /* domainSnapshotDelete */ remoteQemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ - NULL, /* qemuDomainAttach */ + remoteQemuDomainAttach, /* qemuDomainAttach */ remoteDomainOpenConsole, /* domainOpenConsole */ }; -- 1.7.4.4
On 05/05/2011 11:26 AM, Daniel P. Berrange wrote:
* daemon/remote.c: Server side dispatcher * src/remote/remote_driver.c: Client side dispatcher * src/remote/qemu_protocol.x: Wire protocol definition ---
+++ b/src/remote/qemu_protocol.x @@ -37,10 +37,21 @@ struct qemu_monitor_command_ret { remote_nonnull_string result; };
+ +struct qemu_attach_args { + int pid;
Same question as in 1/4 - should this be hyper, to accommodate all possible pid_t sizes? Or am I worried about nothing? ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
This command allows libvirt to attach to an existing QEMU instance. $ qemu-kvm -cdrom ~/demo.iso \ -monitor unix:/tmp/demo,server,nowait \ -name foo \ -uuid cece4f9f-dff0-575d-0e8e-01fe380f12ea & $ QEMUPID=$$ $ virsh qemu-attach $QEMUPID --- tools/virsh.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 5d8b025..e7122eb 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10679,6 +10679,52 @@ cleanup: return ret; } +/* + * "qemu-attach" command + */ +static const vshCmdInfo info_qemu_attach[] = { + {"help", N_("Qemu Attach")}, + {"desc", N_("Qemu Attach")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_qemu_attach[] = { + {"pid", VSH_OT_DATA, VSH_OFLAG_REQ, N_("pid")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdQemuAttach(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + unsigned int flags = 0; + int pid; + + if (!vshConnectionUsability(ctl, ctl->conn)) + goto cleanup; + + if (vshCommandOptInt(cmd, "pid", &pid) < 0) { + vshError(ctl, "%s", _("missing pid value")); + goto cleanup; + } + + if (!(dom = virDomainQemuAttach(ctl->conn, pid, flags))) + goto cleanup; + + if (dom != NULL) { + vshPrint(ctl, _("Domain %s attached to pid %d\n"), + virDomainGetName(dom), pid); + virDomainFree(dom); + ret = true; + } else { + vshError(ctl, _("Failed to attach to pid %d"), pid); + } + +cleanup: + return ret; +} + static const vshCmdDef domManagementCmds[] = { {"attach-device", cmdAttachDevice, opts_attach_device, info_attach_device}, {"attach-disk", cmdAttachDisk, opts_attach_disk, info_attach_disk}, @@ -10875,6 +10921,7 @@ static const vshCmdDef hostAndHypervisorCmds[] = { {"hostname", cmdHostname, NULL, info_hostname}, {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo}, {"qemu-monitor-command", cmdQemuMonitorCommand, opts_qemu_monitor_command, info_qemu_monitor_command}, + {"qemu-attach", cmdQemuAttach, opts_qemu_attach, info_qemu_attach}, {"sysinfo", cmdSysinfo, NULL, info_sysinfo}, {"uri", cmdURI, NULL, info_uri}, {NULL, NULL, NULL, NULL} -- 1.7.4.4
On 05/05/2011 11:26 AM, Daniel P. Berrange wrote:
This command allows libvirt to attach to an existing QEMU instance.
$ qemu-kvm -cdrom ~/demo.iso \ -monitor unix:/tmp/demo,server,nowait \ -name foo \ -uuid cece4f9f-dff0-575d-0e8e-01fe380f12ea & $ QEMUPID=$$
$!, not $$ (you want to attach to the background qemu, not your current shell!)
$ virsh qemu-attach $QEMUPID --- tools/virsh.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 47 insertions(+), 0 deletions(-)
+static const vshCmdOptDef opts_qemu_attach[] = { + {"pid", VSH_OT_DATA, VSH_OFLAG_REQ, N_("pid")},
s/VSH_OT_DATA/VSH_OT_INT/
+ {NULL, 0, 0, NULL} +}; + +static bool +cmdQemuAttach(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + unsigned int flags = 0; + int pid; + + if (!vshConnectionUsability(ctl, ctl->conn)) + goto cleanup; + + if (vshCommandOptInt(cmd, "pid", &pid) < 0) {
s/</<=/ - pid is a required argument. Since you didn't initialize it, if vshCommandOptInt returns 0, you would be using whatever garbage was on the stack.
@@ -10875,6 +10921,7 @@ static const vshCmdDef hostAndHypervisorCmds[] = { {"hostname", cmdHostname, NULL, info_hostname}, {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo}, {"qemu-monitor-command", cmdQemuMonitorCommand, opts_qemu_monitor_command, info_qemu_monitor_command}, + {"qemu-attach", cmdQemuAttach, opts_qemu_attach, info_qemu_attach},
Swap the above two lines, to keep alphabetical order. Missing tools/virsh.pod changes. You know I can't ack without documentation... :) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Given a PID, the QEMU driver reads /proc/$PID/cmdline and /proc/$PID/environ to get the configuration. This is fed into the ARGV->XML convertor to build an XML configuration for the process. /proc/$PID/exe is resolved to identify the full command binary path After checking for name/uuid uniqueness, an attempt is made to connect to the monitor socket. If successful then 'info status' and 'info kvm' are issued to determine whether the CPUs are running and if KVM is enabled. * src/qemu/qemu_command.h, src/qemu/qemu_command.c: Add qemuParseCommandLinePid to extract XML from a PID. Also extract monitor configuration from command line * src/qemu/qemu_driver.c: Implement virDomainQemuAttach * src/qemu/qemu_process.h, src/qemu/qemu_process.c: Add qemuProcessAttach to connect to the monitor of an existing QEMU process * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add APIs for invoking 'info status' and 'info kvm' --- src/qemu/qemu_command.c | 252 +++++++++++++++++++++++++++++++----------- src/qemu/qemu_command.h | 12 ++- src/qemu/qemu_driver.c | 98 ++++++++++++++++- src/qemu/qemu_monitor.c | 41 +++++++ src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 92 +++++++++++++++ src/qemu/qemu_monitor_json.h | 4 + src/qemu/qemu_monitor_text.c | 43 +++++++ src/qemu/qemu_monitor_text.h | 4 + src/qemu/qemu_process.c | 192 +++++++++++++++++++++++++++++--- src/qemu/qemu_process.h | 7 + 11 files changed, 666 insertions(+), 83 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2205ed1..6c7c3ac 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5315,84 +5315,80 @@ cleanup: /* * Tries to parse a QEMU serial/parallel device */ -static virDomainChrDefPtr -qemuParseCommandLineChr(const char *val) +static int +qemuParseCommandLineChr(virDomainChrSourceDefPtr source, + const char *val) { - virDomainChrDefPtr def; - - if (!(def = virDomainChrDefNew())) - goto error; - if (STREQ(val, "null")) { - def->source.type = VIR_DOMAIN_CHR_TYPE_NULL; + source->type = VIR_DOMAIN_CHR_TYPE_NULL; } else if (STREQ(val, "vc")) { - def->source.type = VIR_DOMAIN_CHR_TYPE_VC; + source->type = VIR_DOMAIN_CHR_TYPE_VC; } else if (STREQ(val, "pty")) { - def->source.type = VIR_DOMAIN_CHR_TYPE_PTY; + source->type = VIR_DOMAIN_CHR_TYPE_PTY; } else if (STRPREFIX(val, "file:")) { - def->source.type = VIR_DOMAIN_CHR_TYPE_FILE; - def->source.data.file.path = strdup(val+strlen("file:")); - if (!def->source.data.file.path) + source->type = VIR_DOMAIN_CHR_TYPE_FILE; + source->data.file.path = strdup(val+strlen("file:")); + if (!source->data.file.path) goto no_memory; } else if (STRPREFIX(val, "pipe:")) { - def->source.type = VIR_DOMAIN_CHR_TYPE_PIPE; - def->source.data.file.path = strdup(val+strlen("pipe:")); - if (!def->source.data.file.path) + source->type = VIR_DOMAIN_CHR_TYPE_PIPE; + source->data.file.path = strdup(val+strlen("pipe:")); + if (!source->data.file.path) goto no_memory; } else if (STREQ(val, "stdio")) { - def->source.type = VIR_DOMAIN_CHR_TYPE_STDIO; + source->type = VIR_DOMAIN_CHR_TYPE_STDIO; } else if (STRPREFIX(val, "udp:")) { const char *svc1, *host2, *svc2; - def->source.type = VIR_DOMAIN_CHR_TYPE_UDP; + source->type = VIR_DOMAIN_CHR_TYPE_UDP; val += strlen("udp:"); svc1 = strchr(val, ':'); host2 = svc1 ? strchr(svc1, '@') : NULL; svc2 = host2 ? strchr(host2, ':') : NULL; if (svc1) - def->source.data.udp.connectHost = strndup(val, svc1-val); + source->data.udp.connectHost = strndup(val, svc1-val); else - def->source.data.udp.connectHost = strdup(val); + source->data.udp.connectHost = strdup(val); - if (!def->source.data.udp.connectHost) + if (!source->data.udp.connectHost) goto no_memory; if (svc1) { svc1++; if (host2) - def->source.data.udp.connectService = strndup(svc1, host2-svc1); + source->data.udp.connectService = strndup(svc1, host2-svc1); else - def->source.data.udp.connectService = strdup(svc1); + source->data.udp.connectService = strdup(svc1); - if (!def->source.data.udp.connectService) + if (!source->data.udp.connectService) goto no_memory; } if (host2) { host2++; if (svc2) - def->source.data.udp.bindHost = strndup(host2, svc2-host2); + source->data.udp.bindHost = strndup(host2, svc2-host2); else - def->source.data.udp.bindHost = strdup(host2); + source->data.udp.bindHost = strdup(host2); - if (!def->source.data.udp.bindHost) + if (!source->data.udp.bindHost) goto no_memory; } if (svc2) { svc2++; - def->source.data.udp.bindService = strdup(svc2); - if (!def->source.data.udp.bindService) + source->data.udp.bindService = strdup(svc2); + if (!source->data.udp.bindService) goto no_memory; } } else if (STRPREFIX(val, "tcp:") || STRPREFIX(val, "telnet:")) { const char *opt, *svc; - def->source.type = VIR_DOMAIN_CHR_TYPE_TCP; + source->type = VIR_DOMAIN_CHR_TYPE_TCP; if (STRPREFIX(val, "tcp:")) { val += strlen("tcp:"); } else { val += strlen("telnet:"); - def->source.data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET; + source->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET; } svc = strchr(val, ':'); if (!svc) { @@ -5402,38 +5398,38 @@ qemuParseCommandLineChr(const char *val) } opt = strchr(svc, ','); if (opt && strstr(opt, "server")) - def->source.data.tcp.listen = true; + source->data.tcp.listen = true; - def->source.data.tcp.host = strndup(val, svc-val); - if (!def->source.data.tcp.host) + source->data.tcp.host = strndup(val, svc-val); + if (!source->data.tcp.host) goto no_memory; svc++; if (opt) { - def->source.data.tcp.service = strndup(svc, opt-svc); + source->data.tcp.service = strndup(svc, opt-svc); } else { - def->source.data.tcp.service = strdup(svc); + source->data.tcp.service = strdup(svc); } - if (!def->source.data.tcp.service) + if (!source->data.tcp.service) goto no_memory; } else if (STRPREFIX(val, "unix:")) { const char *opt; val += strlen("unix:"); opt = strchr(val, ','); - def->source.type = VIR_DOMAIN_CHR_TYPE_UNIX; + source->type = VIR_DOMAIN_CHR_TYPE_UNIX; if (opt) { if (strstr(opt, "listen")) - def->source.data.nix.listen = true; - def->source.data.nix.path = strndup(val, opt-val); + source->data.nix.listen = true; + source->data.nix.path = strndup(val, opt-val); } else { - def->source.data.nix.path = strdup(val); + source->data.nix.path = strdup(val); } - if (!def->source.data.nix.path) + if (!source->data.nix.path) goto no_memory; } else if (STRPREFIX(val, "/dev")) { - def->source.type = VIR_DOMAIN_CHR_TYPE_DEV; - def->source.data.file.path = strdup(val); - if (!def->source.data.file.path) + source->type = VIR_DOMAIN_CHR_TYPE_DEV; + source->data.file.path = strdup(val); + if (!source->data.file.path) goto no_memory; } else { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -5441,13 +5437,12 @@ qemuParseCommandLineChr(const char *val) goto error; } - return def; + return 0; no_memory: virReportOOMError(); error: - virDomainChrDefFree(def); - return NULL; + return -1; } @@ -5621,7 +5616,9 @@ error: */ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, const char **progenv, - const char **progargv) + const char **progargv, + virDomainChrSourceDefPtr *monConfig, + bool *monJSON) { virDomainDefPtr def; int i; @@ -5634,6 +5631,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, int nvirtiodisk = 0; qemuDomainCmdlineDefPtr cmd; + if (monConfig) + *monConfig = NULL; + if (monJSON) + *monJSON = false; + if (!progargv[0]) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no emulator path found")); @@ -5750,10 +5752,12 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, _("cannot parse VNC port '%s'"), tmp+1); goto error; } - vnc->data.vnc.listenAddr = strndup(val, tmp-val); - if (!vnc->data.vnc.listenAddr) { - VIR_FREE(vnc); - goto no_memory; + if (tmp != val) { + vnc->data.vnc.listenAddr = strndup(val, tmp-val); + if (!vnc->data.vnc.listenAddr) { + VIR_FREE(vnc); + goto no_memory; + } } vnc->data.vnc.port += 5900; vnc->data.vnc.autoport = 0; @@ -5972,7 +5976,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, WANT_VALUE(); if (STRNEQ(val, "none")) { virDomainChrDefPtr chr; - if (!(chr = qemuParseCommandLineChr(val))) + + if (!(chr = virDomainChrDefNew())) + goto error; + + if (qemuParseCommandLineChr(&chr->source, val) < 0) goto error; if (VIR_REALLOC_N(def->serials, def->nserials+1) < 0) { virDomainChrDefFree(chr); @@ -5986,7 +5994,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, WANT_VALUE(); if (STRNEQ(val, "none")) { virDomainChrDefPtr chr; - if (!(chr = qemuParseCommandLineChr(val))) + + if (!(chr = virDomainChrDefNew())) + goto error; + + if (qemuParseCommandLineChr(&chr->source, val) < 0) goto error; if (VIR_REALLOC_N(def->parallels, def->nparallels+1) < 0) { virDomainChrDefFree(chr); @@ -6169,7 +6181,17 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, /* ignore, used via restore/migrate APIs */ } else if (STREQ(arg, "-monitor")) { WANT_VALUE(); - /* ignore, used internally by libvirt */ + if (monConfig) { + virDomainChrSourceDefPtr chr; + + if (VIR_ALLOC(chr) < 0) + goto no_memory; + + if (qemuParseCommandLineChr(chr, val) < 0) + goto error; + + *monConfig = chr; + } } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ } else { @@ -6310,11 +6332,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, VIR_FREE(nics); - if (!def->name) { - if (!(def->name = strdup("unnamed"))) - goto no_memory; - } - if (virDomainDefAddImplicitControllers(def) < 0) goto error; @@ -6333,12 +6350,18 @@ error: VIR_FREE(cmd); virDomainDefFree(def); VIR_FREE(nics); + if (monConfig) { + virDomainChrSourceDefFree(*monConfig); + *monConfig = NULL; + } return NULL; } virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps, - const char *args) + const char *args, + virDomainChrSourceDefPtr *monConfig, + bool *monJSON) { const char **progenv = NULL; const char **progargv = NULL; @@ -6348,7 +6371,7 @@ virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps, if (qemuStringToArgvEnv(args, &progenv, &progargv) < 0) goto cleanup; - def = qemuParseCommandLine(caps, progenv, progargv); + def = qemuParseCommandLine(caps, progenv, progargv, monConfig, monJSON); cleanup: for (i = 0 ; progargv && progargv[i] ; i++) @@ -6361,3 +6384,106 @@ cleanup: return def; } + + +static int qemuParseProcFileStrings(int pid, + const char *name, + const char ***list) +{ + char *path = NULL; + int ret = -1; + char *data = NULL; + ssize_t len; + char *tmp; + size_t nstr = 0; + const char **str = NULL; + int i; + + if (virAsprintf(&path, "/proc/%d/%s", pid, name) < 0) { + virReportOOMError(); + goto cleanup; + } + + if ((len = virFileReadAll(path, 1024*128, &data)) < 0) + goto cleanup; + + tmp = data; + while (tmp < (data + len)) { + if (VIR_EXPAND_N(str, nstr, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!(str[nstr-1] = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + /* Skip arg */ + tmp += strlen(tmp); + /* Skip \0 separator */ + tmp++; + } + + if (VIR_EXPAND_N(str, nstr, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + str[nstr-1] = NULL; + + ret = nstr-1; + *list = str; + +cleanup: + if (ret < 0) { + for (i = 0 ; str && str[i] ; i++) + VIR_FREE(str[i]); + VIR_FREE(str); + } + VIR_FREE(data); + VIR_FREE(path); + return ret; +} + +virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, + int pid, + virDomainChrSourceDefPtr *monConfig, + bool *monJSON) +{ + virDomainDefPtr def = NULL; + const char **progargv = NULL; + const char **progenv = NULL; + char *exepath = NULL; + char *emulator; + int i; + + if (qemuParseProcFileStrings(pid, "cmdline", &progargv) < 0 || + qemuParseProcFileStrings(pid, "environ", &progenv) < 0) + goto cleanup; + + if (!(def = qemuParseCommandLine(caps, progenv, progargv, monConfig, monJSON))) + goto cleanup; + + if (virAsprintf(&exepath, "/proc/%d/exe", pid) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virFileResolveLink(exepath, &emulator) < 0) { + virReportSystemError(errno, + _("Unable to resolve %s"), exepath); + goto cleanup; + } + VIR_FREE(def->emulator); + def->emulator = emulator; + +cleanup: + VIR_FREE(exepath); + for (i = 0 ; progargv && progargv[i] ; i++) + VIR_FREE(progargv[i]); + VIR_FREE(progargv); + for (i = 0 ; progenv && progenv[i] ; i++) + VIR_FREE(progenv[i]); + VIR_FREE(progenv); + return def; +} diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 528031d..e64ff8b 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -140,9 +140,17 @@ int qemudCanonicalizeMachine(struct qemud_driver *driver, virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, const char **progenv, - const char **progargv); + const char **progargv, + virDomainChrSourceDefPtr *monConfig, + bool *monJSON); virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps, - const char *args); + const char *args, + virDomainChrSourceDefPtr *monConfig, + bool *monJSON); +virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, + int pid, + virDomainChrSourceDefPtr *monConfig, + bool *monJSON); int qemuDomainAssignPCIAddresses(virDomainDefPtr def); qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 24447b9..a995d94 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3357,11 +3357,17 @@ static char *qemuDomainXMLFromNative(virConnectPtr conn, } qemuDriverLock(driver); - def = qemuParseCommandLineString(driver->caps, config); + def = qemuParseCommandLineString(driver->caps, config, NULL, NULL); qemuDriverUnlock(driver); if (!def) goto cleanup; + if (!def->name && + !(def->name = strdup("unnamed"))) { + virReportOOMError(); + goto cleanup; + } + xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE); cleanup: @@ -3586,7 +3592,7 @@ qemudCanonicalizeMachineFromInfo(virDomainDefPtr def, if (!machine->canonical) continue; - if (STRNEQ(def->os.machine, machine->name)) + if (def->os.machine && STRNEQ(def->os.machine, machine->name)) continue; if (!(*canonical = strdup(machine->canonical))) { @@ -3613,7 +3619,7 @@ qemudCanonicalizeMachineDirect(virDomainDefPtr def, char **canonical) if (!machines[i]->canonical) continue; - if (STRNEQ(def->os.machine, machines[i]->name)) + if (def->os.machine && STRNEQ(def->os.machine, machines[i]->name)) continue; *canonical = machines[i]->canonical; @@ -7008,6 +7014,90 @@ cleanup: } +static virDomainPtr qemuDomainAttach(virConnectPtr conn, + int pid, + unsigned int flags) +{ + struct qemud_driver *driver = conn->privateData; + virDomainObjPtr vm = NULL; + virDomainDefPtr def = NULL; + virDomainPtr dom = NULL; + virDomainChrSourceDefPtr monConfig = NULL; + bool monJSON = false; + + virCheckFlags(0, NULL); + + qemuDriverLock(driver); + + if (!(def = qemuParseCommandLinePid(driver->caps, pid, &monConfig, &monJSON))) + goto cleanup; + + if (!monConfig) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("No monitor connection for pid %d"), + pid); + goto cleanup; + } + if (monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cannot connect to monitor connection of type '%s' for pid %d"), + virDomainChrTypeToString(monConfig->type), pid); + goto cleanup; + } + + if (!(def->name) && + virAsprintf(&def->name, "attach-pid-%d", pid) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup; + + if (qemudCanonicalizeMachine(driver, def) < 0) + goto cleanup; + + if (qemuDomainAssignPCIAddresses(def) < 0) + goto cleanup; + + if (!(vm = virDomainAssignDef(driver->caps, + &driver->domains, + def, false))) + goto cleanup; + + def = NULL; + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + goto cleanup; + + if (qemuProcessAttach(conn, driver, vm, pid, monConfig, monJSON) < 0) { + monConfig = NULL; + goto endjob; + } + + monConfig = NULL; + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (dom) dom->id = vm->def->id; + +endjob: + if (qemuDomainObjEndJob(vm) == 0) { + vm = NULL; + goto cleanup; + } + +cleanup: + if (def) + virDomainDefFree(def); + if (monConfig) + virDomainChrSourceDefFree(monConfig); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return dom; +} + + static int qemuDomainOpenConsole(virDomainPtr dom, const char *devname, @@ -7193,7 +7283,7 @@ static virDriver qemuDriver = { qemuDomainRevertToSnapshot, /* domainRevertToSnapshot */ qemuDomainSnapshotDelete, /* domainSnapshotDelete */ qemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ - NULL, /* qemuDomainAttach */ + qemuDomainAttach, /* qemuDomainAttach */ qemuDomainOpenConsole, /* domainOpenConsole */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2d28f8d..2aad555 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1019,6 +1019,47 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, return ret; } + +int qemuMonitorGetState(qemuMonitorPtr mon, + int *state) +{ + int ret; + VIR_DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONGetState(mon, state); + else + ret = qemuMonitorTextGetState(mon, state); + return ret; +} + + +int qemuMonitorGetVirtType(qemuMonitorPtr mon, + int *virtType) +{ + int ret; + VIR_DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONGetVirtType(mon, virtType); + else + ret = qemuMonitorTextGetVirtType(mon, virtType); + return ret; +} + + int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, unsigned long *currmem) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c90219b..a69a583 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -183,6 +183,10 @@ int qemuMonitorSystemPowerdown(qemuMonitorPtr mon); int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, int **pids); +int qemuMonitorGetState(qemuMonitorPtr mon, + int *state); +int qemuMonitorGetVirtType(qemuMonitorPtr mon, + int *virtType); int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, unsigned long *currmem); int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 20a78e1..5f39c98 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -978,6 +978,98 @@ int qemuMonitorJSONGetCPUInfo(qemuMonitorPtr mon, } +int qemuMonitorJSONGetState(qemuMonitorPtr mon, + int *state) +{ + int ret; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-status", + NULL); + virJSONValuePtr reply = NULL; + + *state = VIR_DOMAIN_RUNNING; + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret == 0) { + virJSONValuePtr data; + int r; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("info status reply was missing return data")); + ret = -1; + goto cleanup; + } + + if ((r = virJSONValueObjectGetBoolean(data, "running")) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("info status reply missing 'running' field")); + ret = -1; + goto cleanup; + } + if (!r) + *state = VIR_DOMAIN_PAUSED; + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + +int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, + int *virtType) +{ + int ret; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-kvm", + NULL); + virJSONValuePtr reply = NULL; + + *virtType = VIR_DOMAIN_VIRT_QEMU; + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret == 0) { + virJSONValuePtr data; + int r; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("info kvm reply was missing return data")); + ret = -1; + goto cleanup; + } + + if ((r = virJSONValueObjectGetBoolean(data, "enabled")) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("info kvm reply missing 'running' field")); + ret = -1; + goto cleanup; + } + if (r) + *virtType = VIR_DOMAIN_VIRT_KVM; + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + /* * Returns: 0 if balloon not supported, +1 if balloon query worked * or -1 on failure diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 086f0e1..e2b5002 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -51,6 +51,10 @@ int qemuMonitorJSONSystemPowerdown(qemuMonitorPtr mon); int qemuMonitorJSONGetCPUInfo(qemuMonitorPtr mon, int **pids); +int qemuMonitorJSONGetState(qemuMonitorPtr mon, + int *state); +int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, + int *virtType); int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, unsigned long *currmem); int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 53781c8..711b7e2 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -468,6 +468,49 @@ error: return 0; } + +int qemuMonitorTextGetState(qemuMonitorPtr mon, + int *state) +{ + char *reply = NULL; + + *state = VIR_DOMAIN_RUNNING; + + if (qemuMonitorHMPCommand(mon, "info status", &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("could not query CPU state")); + return -1; + } + + if (strstr(reply, "paused")) + *state = VIR_DOMAIN_PAUSED; + + VIR_FREE(reply); + return 0; +} + + +int qemuMonitorTextGetVirtType(qemuMonitorPtr mon, + int *virtType) +{ + char *reply = NULL; + + *virtType = VIR_DOMAIN_VIRT_QEMU; + + if (qemuMonitorHMPCommand(mon, "info kvm", &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("could not query kvm status")); + return -1; + } + + if (strstr(reply, "enabled")) + *virtType = VIR_DOMAIN_VIRT_KVM; + + VIR_FREE(reply); + return 0; +} + + static int parseMemoryStat(char **text, unsigned int tag, const char *search, virDomainMemoryStatPtr stat) { diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 0838a2b..cea609c 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -48,6 +48,10 @@ int qemuMonitorTextSystemPowerdown(qemuMonitorPtr mon); int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon, int **pids); +int qemuMonitorTextGetState(qemuMonitorPtr mon, + int *state); +int qemuMonitorTextGetVirtType(qemuMonitorPtr mon, + int *virtType); int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, unsigned long *currmem); int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3460000..5f6980b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -54,6 +54,7 @@ #define VIR_FROM_THIS VIR_FROM_QEMU #define START_POSTFIX ": starting up\n" +#define ATTACH_POSTFIX ": attaching\n" #define SHUTDOWN_POSTFIX ": shutting down\n" /** @@ -1040,29 +1041,32 @@ qemuProcessReadLogFD(int logfd, char *buf, int maxlen, int off) tmpbuf[ret] = '\0'; } + static int qemuProcessWaitForMonitor(struct qemud_driver* driver, virDomainObjPtr vm, off_t pos) { - char *buf; + char *buf = NULL; size_t buf_size = 4096; /* Plenty of space to get startup greeting */ - int logfd; + int logfd = -1; int ret = -1; virHashTablePtr paths = NULL; qemuDomainObjPrivatePtr priv; - if ((logfd = qemuProcessLogReadFD(driver->logDir, vm->def->name, pos)) < 0) - return -1; + if (pos != -1) { + if ((logfd = qemuProcessLogReadFD(driver->logDir, vm->def->name, pos)) < 0) + return -1; - if (VIR_ALLOC_N(buf, buf_size) < 0) { - virReportOOMError(); - return -1; - } + if (VIR_ALLOC_N(buf, buf_size) < 0) { + virReportOOMError(); + return -1; + } - if (qemuProcessReadLogOutput(vm, logfd, buf, buf_size, - qemuProcessFindCharDevicePTYs, - "console", 30) < 0) - goto closelog; + if (qemuProcessReadLogOutput(vm, logfd, buf, buf_size, + qemuProcessFindCharDevicePTYs, + "console", 30) < 0) + goto closelog; + } VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name); if (qemuConnectMonitor(driver, vm) < 0) { @@ -1099,8 +1103,6 @@ cleanup: ret = -1; } - VIR_FREE(buf); - closelog: if (VIR_CLOSE(logfd) < 0) { char ebuf[1024]; @@ -1108,6 +1110,8 @@ closelog: virStrerror(errno, ebuf, sizeof ebuf)); } + VIR_FREE(buf); + return ret; } @@ -2625,3 +2629,163 @@ retry: virFreeError(orig_err); } } + + +int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, + struct qemud_driver *driver, + virDomainObjPtr vm, + int pid, + virDomainChrSourceDefPtr monConfig, + bool monJSON) +{ + char ebuf[1024]; + int logfile = -1; + char *timestamp; + qemuDomainObjPrivatePtr priv = vm->privateData; + + VIR_DEBUG0("Beginning VM attach process"); + + if (virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("VM is already active")); + return -1; + } + + /* Do this upfront, so any part of the startup process can add + * runtime state to vm->def that won't be persisted. This let's us + * report implicit runtime defaults in the XML, like vnc listen/socket + */ + VIR_DEBUG0("Setting current domain def as transient"); + if (virDomainObjSetDefTransient(driver->caps, vm, true) < 0) + goto cleanup; + + vm->def->id = driver->nextvmid++; + + if (virFileMakePath(driver->logDir) != 0) { + virReportSystemError(errno, + _("cannot create log directory %s"), + driver->logDir); + goto cleanup; + } + + VIR_DEBUG0("Creating domain log file"); + if ((logfile = qemuProcessLogFD(driver, vm->def->name, false)) < 0) + goto cleanup; + + VIR_DEBUG0("Determining emulator version"); + qemuCapsFree(priv->qemuCaps); + priv->qemuCaps = NULL; + if (qemuCapsExtractVersionInfo(vm->def->emulator, + vm->def->os.arch, + NULL, + &priv->qemuCaps) < 0) + goto cleanup; + + if (VIR_ALLOC(priv->monConfig) < 0) { + virReportOOMError(); + goto cleanup; + } + + VIR_DEBUG0("Preparing monitor state"); + priv->monConfig = monConfig; + monConfig = NULL; + priv->monJSON = monJSON; + + priv->gotShutdown = false; + +#if 0 + /* + * Normally PCI addresses are assigned in the virDomainCreate + * or virDomainDefine methods. We might still need to assign + * some here to cope with the question of upgrades. Regardless + * we also need to populate the PCi address set cache for later + * use in hotplug + */ + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { + VIR_DEBUG0("Assigning domain PCI addresses"); + /* Populate cache with current addresses */ + if (priv->pciaddrs) { + qemuDomainPCIAddressSetFree(priv->pciaddrs); + priv->pciaddrs = NULL; + } + if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(vm->def))) + goto cleanup; + + + /* Assign any remaining addresses */ + if (qemuAssignDevicePCISlots(vm->def, priv->pciaddrs) < 0) + goto cleanup; + + priv->persistentAddrs = 1; + } else { + priv->persistentAddrs = 0; + } +#else + priv->persistentAddrs = 0; +#endif + + if ((timestamp = virTimestamp()) == NULL) { + virReportOOMError(); + goto cleanup; + } else { + if (safewrite(logfile, timestamp, strlen(timestamp)) < 0 || + safewrite(logfile, ATTACH_POSTFIX, strlen(ATTACH_POSTFIX)) < 0) { + VIR_WARN("Unable to write timestamp to logfile: %s", + virStrerror(errno, ebuf, sizeof ebuf)); + } + + VIR_FREE(timestamp); + } + + vm->pid = pid; + + VIR_DEBUG0("Waiting for monitor to show up"); + if (qemuProcessWaitForMonitor(driver, vm, -1) < 0) + goto cleanup; + + VIR_DEBUG0("Detecting VCPU PIDs"); + if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) + goto cleanup; + +#if 0 + /* If we have -device, then addresses are assigned explicitly. + * If not, then we have to detect dynamic ones here */ + if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { + VIR_DEBUG0("Determining domain device PCI addresses"); + if (qemuProcessInitPCIAddresses(driver, vm) < 0) + goto cleanup; + } +#endif + + VIR_DEBUG0("Getting initial memory amount"); + qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (qemuMonitorGetBalloonInfo(priv->mon, &vm->def->mem.cur_balloon) < 0) { + qemuDomainObjExitMonitorWithDriver(driver, vm); + goto cleanup; + } + if (qemuMonitorGetState(priv->mon, &vm->state) < 0) { + qemuDomainObjExitMonitorWithDriver(driver, vm); + goto cleanup; + } + if (qemuMonitorGetVirtType(priv->mon, &vm->def->virtType) < 0) { + qemuDomainObjExitMonitorWithDriver(driver, vm); + goto cleanup; + } + qemuDomainObjExitMonitorWithDriver(driver, vm); + + VIR_DEBUG0("Writing domain status to disk"); + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + goto cleanup; + + VIR_FORCE_CLOSE(logfile); + + return 0; + +cleanup: + /* We jump here if we failed to start the VM for any reason, or + * if we failed to initialize the now running VM. kill it off and + * pretend we never started it */ + VIR_FORCE_CLOSE(logfile); + virDomainChrSourceDefFree(monConfig); + return -1; +} diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index f1ab599..eb460f4 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -49,4 +49,11 @@ void qemuProcessStop(struct qemud_driver *driver, virDomainObjPtr vm, int migrated); +int qemuProcessAttach(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + int pid, + virDomainChrSourceDefPtr monConfig, + bool monJSON); + #endif /* __QEMU_PROCESS_H__ */ -- 1.7.4.4
On 05/05/2011 11:26 AM, Daniel P. Berrange wrote:
Given a PID, the QEMU driver reads /proc/$PID/cmdline and /proc/$PID/environ to get the configuration. This is fed into the ARGV->XML convertor to build an XML configuration
s/convertor/converter/
+static int qemuParseProcFileStrings(int pid, + const char *name, + const char ***list) +{ + char *path = NULL; + int ret = -1; + char *data = NULL; + ssize_t len; + char *tmp; + size_t nstr = 0; + const char **str = NULL; + int i; + + if (virAsprintf(&path, "/proc/%d/%s", pid, name) < 0) { + virReportOOMError(); + goto cleanup; + } + + if ((len = virFileReadAll(path, 1024*128, &data)) < 0) + goto cleanup;
Can we ever short-change ourselves by not reading the entire file? Qemu can have some pretty hefty command-line lengths. Then again, typing in 128k manually seems obtuse, especially since the most common user of this feature will be developers.
+ + tmp = data; + while (tmp < (data + len)) { + if (VIR_EXPAND_N(str, nstr, 1) < 0) {
VIR_RESIZE_N might be nicer here, since there could be a lot of arguments to the point of noticing the quadratic nature of VIR_EXPAND_N.
+ virReportOOMError(); + goto cleanup; + } + + if (!(str[nstr-1] = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + /* Skip arg */ + tmp += strlen(tmp); + /* Skip \0 separator */ + tmp++; + } + + if (VIR_EXPAND_N(str, nstr, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + str[nstr-1] = NULL;
This line is redundant - VIR_EXPAND_N guarantees that the element starts life as NULL.
+ if (virAsprintf(&exepath, "/proc/%d/exe", pid) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virFileResolveLink(exepath, &emulator) < 0) { + virReportSystemError(errno, + _("Unable to resolve %s"), exepath); + goto cleanup; + }
What happens if /proc/nnn/exe points to an older qemu, and qemu has been upgraded in the meantime? In that case, the symlink resolution will generally fail (because the kernel changes that symlink to have " (deleted)" appended to it), but you can still exec() /proc/nnn/exe (that is, as long as at least one process still has the old inode open, then you can spawn other processes from that same inode, and thus properly query the capabilities of the old qemu, rather than flat out giving up). But in general, I can agree that it's probably safer to require that we resolve the name than it is to just reuse /proc/%d/exe as the name of our binary, even if that means we are not robust to qemu upgrades. After all, this API will mark the new domain tainted.
+#if 0 + /* + * Normally PCI addresses are assigned in the virDomainCreate + * or virDomainDefine methods. We might still need to assign + * some here to cope with the question of upgrades. Regardless + * we also need to populate the PCi address set cache for later
s/PCi/PCI/
+ +cleanup: + /* We jump here if we failed to start the VM for any reason, or + * if we failed to initialize the now running VM. kill it off and + * pretend we never started it */
Fix the comment - we don't try to kill qemu if attach failed. ACK with nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 05/05/2011 11:26 AM, Daniel P. Berrange wrote:
This is the bare minimum code required to allow libvirt to connect to an externally launched QEMU instance, reverse engineering the XML configuration from the command line args.
The biggest problem is that our ARGV-> XML convertor does not know about the -device syntax people often now use.
Agreed. There's definitely some work to do here. At least the <qemu:commandline> XML conversion is a nice backdoor so that we can at least round trip native -> xml -> native in most simple cases.
There is also some robustness work to be done the QEMU driver in general, to remove some assumptions that no longer hold true. For example in our domain shutdown code, there are bits of cleanup we should skip because we will not have done the corresponding setup in the first place.
And I'd really like to make a couple of improvements at some point - the native-to-xml currently expects a space-separated list of arguments for "qemu-argv" mode, when it would be much easier if we introduced a "qemu-argz" mode which took as the native string the name of a filename containing NUL-terminated arguments (note that this is intentionally like /proc/nnn/cmdline!). Furthermore, I want to make virCommandToString be smarter about producing optional shell quoting, as well as making qemu-argv mode smarter when the input is properly shell-quoted, so that it becomes an unambiguous translation from xml to native back to xml (right now, we can really throw off the conversion if we have shell-quoted spaces in one of the arguments). But those can be separate improvements on top of this base framework.
We also need to verify what happens with all the various APIs particularly device hotplug, to ensure there are no bad results. That said, even as it is, this functionality will be quite useful for QEMU developers
It definitely falls in the category of provided but unsupported, much like qemu raw monitor passthrough commands - we are making it easier for debuggers. So this should belong to the qemu set of RPC calls rather than the generic set; linked in through libvirt-qemu.la rather than the normal libvirt.la (I haven't read the rest of the series to see if you actually did this). But ACK on the idea! -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake