[libvirt] [PATCH 5/5] qemu: implement the 'libvirt_qemu' shim for launching guests externally

Tue Jan 9 14:13:41 UTC 2018

On Wed, Dec 20, 2017 at 04:47:50PM +0000, Daniel P. Berrange wrote:
>This introduces a new binary 'libvirt_qemu' which can be used to launch
>guests externally from libvirtd. eg
>
>  libvirt_qemu -c qemu:///system /path/to/xml/file
>
>This will launch a guest from the requested XML file and then connect to
>qemu:///system to register it with libvirtd. At this point all normal
>libvirtd operations should generally work.
>
>NB, this impl has many flaws:
>
> - We don't generate a unique 'id', so all guests will get id==1.
>   Surprisingly this doesn't break the world.
>
> - Tracking of unique name + UUIDs is *not* race free.
>
> - No tracking/allocation of shared resource state (PCI devices,
>   etc)
>

You will also not get the same events as you would when starting the domain
as usual.

>Most other functionality works, but might have expected results.
>

If I start a domain with a network, it segfaults.  I'll see later if that's my
fault or not.  It probably (and hopefully) is.

>In terms of namespace support there's some restrictions
>
> - The mount namespace must share certain dirs
>
>     /etc/libvirt
>     /var/run/libvirt
>     /var/lib/libvirt
>     /var/cache/libvirt
>
>   between the libvirtd and libvirt_qemu processes
>
> - The PID namespace must match libvirtd & libvirt_qemu
>   though this will be addressed in another patch to
>   come
>
> - The network namespace can be different
>
> - Hotplug/unplug will likely break if separate
>   namespaces are used
>
> - The libvirt_qemu will exit if startup fails, however, it
>   won't exit when QEMU later shuts down - you must listen
>   for libvirtd events to detect that right now. We'll
>   fix that
>
> - Killing the libvirt_qemu shim will not kill the QEMU
>   process. You must kill via the libvirt API as normal.
>   This won't change, because we need to be able to
>   ultimately restart the libvirt_qemu shim in order to
>   apply software updates to running instance.
>   We might wire up a special signal though to let
>   you kill libvirt_qemu & take out QEMU at same time
>   eg SIGQUIT or something like that perhaps.
>

IMHO we should use standard signals to do standard things.  TERM should
gracefully shutdown the domain.  Not in PoC, but later.  That way users
can treat the shim process as other processes.

>Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
>---
> po/POTFILES.in             |   1 +
> src/Makefile.am            |  49 ++++
> src/qemu/qemu_conf.h       |   2 +-
> src/qemu/qemu_controller.c | 551 +++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_domain.c     |   2 +-
> src/qemu/qemu_driver.c     |   2 +-
> 6 files changed, 604 insertions(+), 3 deletions(-)
> create mode 100644 src/qemu/qemu_controller.c
>
>diff --git a/po/POTFILES.in b/po/POTFILES.in
>index c1fa23427e..d97175b6d5 100644
>--- a/po/POTFILES.in
>+++ b/po/POTFILES.in
>@@ -133,6 +133,7 @@ src/qemu/qemu_block.c
> src/qemu/qemu_capabilities.c
> src/qemu/qemu_cgroup.c
> src/qemu/qemu_command.c
>+src/qemu/qemu_controller.c
> src/qemu/qemu_conf.c
> src/qemu/qemu_domain.c
> src/qemu/qemu_domain_address.c
>diff --git a/src/Makefile.am b/src/Makefile.am
>index 4c022d1e44..7ca5ad4d3c 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -922,6 +922,9 @@ QEMU_DRIVER_SOURCES = \
> 		qemu/qemu_capspriv.h \
> 		qemu/qemu_security.c qemu/qemu_security.h
>
>+QEMU_CONTROLLER_SOURCES = \
>+		qemu/qemu_controller.c
>+
> XENAPI_DRIVER_SOURCES = \
> 		xenapi/xenapi_driver.c xenapi/xenapi_driver.h \
> 		xenapi/xenapi_driver_private.h \
>@@ -3115,6 +3118,52 @@ endif WITH_LIBVIRTD
> endif WITH_LXC
> EXTRA_DIST += $(LXC_CONTROLLER_SOURCES)
>
>+if WITH_QEMU
>+if WITH_LIBVIRTD
>+libexec_PROGRAMS += libvirt_qemu
>+

Shouldn't I be able to launch this as a user without mgmt app?  It
should go to bin_PROGRAMS, I presume.

>+libvirt_qemu_SOURCES = \
>+		$(QEMU_CONTROLLER_SOURCES) \
>+		$(DATATYPES_SOURCES)
>+libvirt_qemu_LDFLAGS = \
>+		$(AM_LDFLAGS) \
>+		$(PIE_LDFLAGS) \
>+		$(NULL)
>+libvirt_qemu_LDADD = \
>+		libvirt.la \
>+		libvirt-qemu.la \
>+		libvirt_driver_qemu_impl.la
>+if WITH_NETWORK
>+libvirt_qemu_LDADD += libvirt_driver_network_impl.la
>+endif WITH_NETWORK
>+if WITH_STORAGE
>+libvirt_qemu_LDADD += libvirt_driver_storage_impl.la
>+endif WITH_STORAGE

>+libvirt_qemu_LDADD += ../gnulib/lib/libgnu.la $(LIBSOCKET)

This is not part of any condition, should be up there with libvirt.la
just for clarity.

Those are just nits, of course.

[...]

>+static void
>+virQEMUControllerDriverFree(virQEMUDriverPtr driver)
>+{
>+    if (!driver)
>+        return;
>+
>+    virObjectUnref(driver->config);
>+    virObjectUnref(driver->hostdevMgr);
>+    virHashFree(driver->sharedDevices);
>+    virObjectUnref(driver->caps);
>+    virObjectUnref(driver->qemuCapsCache);
>+
>+    virObjectUnref(driver->domains);
>+    virObjectUnref(driver->remotePorts);
>+    virObjectUnref(driver->webSocketPorts);
>+    virObjectUnref(driver->migrationPorts);
>+    virObjectUnref(driver->migrationErrors);
>+
>+    virObjectUnref(driver->xmlopt);
>+
>+    virSysinfoDefFree(driver->hostsysinfo);
>+
>+    virObjectUnref(driver->closeCallbacks);
>+
>+    VIR_FREE(driver->qemuImgBinary);
>+
>+    virObjectUnref(driver->securityManager);
>+
>+    ebtablesContextFree(driver->ebtables);
>+
>+    virLockManagerPluginUnref(driver->lockManager);
>+
>+    virMutexDestroy(&driver->lock);
>+    VIR_FREE(driver);
>+}
>+
>+static virQEMUDriverPtr virQEMUControllerNewDriver(bool privileged)

This is very similar to what I did, but I just exported this function
privately si that there is less duplication of code and I added an enum
that is used as a parameter instead of the bool @privileged.  It is a
tristate (user, system, shim) that special-cases the shim slightly.

The reason behind that is that I kind of presumed we need to be able to
launch system QEMUs with non-root user.  Thinking about it I'm not sure
that's the case for kubevirt, but it definitely is for others.  The way
it is done now is that you initialize the driver based on geteuid(), but
you connect to any uri specified by the user.  What I had in mind was
that with this shim PoC users would be able to start their own VMs (if
libvirtd permissions are set up the right way, even with some directory
access changes done in the daemon) as system VMs as if they had seclabel
set up for their user.  I think it's feasible, but I maybe overestimated
what's possible for the PoC.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180109/0d8ad839/attachment-0001.sig>