Re: [RFC] Add basic driver for the Cloud-Hypervisor

On Tue, Sep 15, 2020 at 5:53 AM Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: > > On Thu, Aug 27, 2020 at 11:24:32AM -0700, William Douglas wrote: <snip> > > +virCHMonitorPtr > > +virCHMonitorNew(virDomainObjPtr vm, const char *socketdir) > > +{ > > + virCHMonitorPtr ret = NULL; > > + virCHMonitorPtr mon = NULL; > > + virCommandPtr cmd = NULL; > > + int pings = 0; > > + > > + if (virCHMonitorInitialize() < 0) > > + return NULL; > > + > > + if (!(mon = virObjectLockableNew(virCHMonitorClass))) > > + return NULL; > > + > > + mon->socketpath = g_strdup_printf("%s/%s-socket", socketdir, vm->def->name); > > + > > + /* prepare to launch Cloud-Hypervisor socket */ > > + if (!(cmd = chMonitorBuildSocketCmd(vm, mon->socketpath))) > > + goto cleanup; > > + > > + if (virFileMakePath(socketdir) < 0) { > > + virReportSystemError(errno, > > + _("Cannot create socket directory '%s'"), > > + socketdir); > > + goto cleanup; > > + } > > + > > + /* launch Cloud-Hypervisor socket */ > > + if (virCommandRunAsync(cmd, &mon->pid) < 0) > > + goto cleanup; > > + > > + /* get a curl handle */ > > + mon->handle = curl_easy_init(); > > + > > + /* try to ping VMM socket 5 times to make sure it is ready */ > > + while (pings < 5) { > > + if (virCHMonitorPingVMM(mon) == 0) > > + break; > > + if (pings == 5) > > + goto cleanup; > > + > > + g_usleep(100 * 1000); > > + } > > This is highly undesirable. Is there no way to launch the CH process > such that the socket is guaranteed to be accepting requests by the > time it has forked into the background ? Or is there a way to pass > in a pre-opened FD for the monitor socket ? > > This kind of wait with timeout will cause startup failures due to > timeout under load. Currently the cloud-hypervisor process doesn't fork into the background so that was initially what I was thinking to add to cloud-hypervsior. Would tracking the pid of the cloud-hypervsior running in the background be required at that point (assuming the socket path to control the daemon would be known and working given virCommandRun returns successfully)?

> What is the security model for the processes ? The libvirt driver > here is running as root and spawning CH as root. We don't really > want the VM process running as root though. It really needs to > be unprivileged from a DAC POV. Obviously that's quite a bit more > work for you todo, but it should be opssible to share alot of the > security mgmt infrastructure that we already have for QEMU and > LXC drivers. It can manage DAC permissions and apply SELinux or > AppArmor MAC labelling/profiles. Currently cloud-hypervisor runs with cap_net_admin permitted and effective on it. The implementation will be updated to run under the user's session using a non-root driver (with its own daemon as you had mentioned earlier). > > The other big question is around device addressing. If using PCI, > then PCI address assignment logic is critical to ensure consistent > guest ABI. > I'll be adding the device passthrough for the actual driver submission but if you would like to see an updated RFC with that added I could do so.