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

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.

> - 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.

> +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.

> +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.