On Tue, Jan 09, 2018 at 03:13:41PM +0100, Martin Kletzander wrote:
> 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.
Hmm, yes, interesting. If someone is connected to libvirtd watching events
they'll not get any startup events. That's something we'd need to fix.
> > 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.
Not intentional if that's happening !
> > - 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.
I guess we can use SIGUSR1 to trigger restart of the shim while
leaving QEMU running.
> > +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.
Yes
> > +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.
IIRC, libgnu.la needs to be the last library listed
> > +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.
Yeah, there's definitely much more duplication here that I would like.
I would expect to refactor this better.
> 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.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
