On Fri, Dec 12, 2008 at 07:27:55PM +0100, Guido G?nther wrote:
> The earlier patches didn't change libvirtd behaviour (except for
> kvm/qemu not being teared down on an unexpected daemon crash), all vms
> were still being shutoff at daemon shutdown. This patch now adds the
> code to read back the vm status after on daemon startup and reconnects
> all running vms found in stateDir which are left over from a daemon
> restart or crash and also disables shutting down of VMs when libvirt
> quits.
>
> ---
> src/qemu_driver.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 158 insertions(+), 7 deletions(-)
>
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index d8b87e4..57a396c 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -183,6 +183,46 @@ qemudAutostartConfigs(struct qemud_driver *driver) {
> }
>
>
> +static int
> +qemudGetProcFD(pid_t pid, int fdnum)
> +{
> + int fd;
> +
> +#ifdef __linux__
> + char *path = NULL;
> +
> + if (!asprintf(&path, "/proc/%d/fd/%d", pid, fdnum)) {
> + qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
> + return -1;
> + }
> +
> + if((fd = open(path, O_RDONLY)) < 0) {
> + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> + _("Unable to open %s"), path);
> + return -1;
> + }
> + if (qemudSetCloseExec(fd) < 0) {
> + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> + _("Unable to set close-on-exec flag on %s"), path);
> + goto error;
> + }
> +
> + if (qemudSetNonBlock(fd) < 0) {
> + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> + _("Unable to put %s into non-blocking mode"), path);
> + goto error;
> + }
> +
> + VIR_FREE(path);
> + return fd;
> +error:
> + VIR_FREE(path);
> + close(fd);
> +#endif
> + return -1;
> +}
> +
> +
> /**
> * qemudRemoveDomainStatus
> *
> @@ -220,6 +260,104 @@ cleanup:
> }
>
>
> +static int qemudOpenMonitor(virConnectPtr conn,
> + virDomainObjPtr vm,
> + const char *monitor,
> + int reconnect);
> +
> +/**
> + * qemudReconnectVMs
> + *
> + * Reconnect running vms to the daemon process
> + */
> +static int
> +qemudReconnectVMs(struct qemud_driver *driver)
> +{
> + int i;
> +
> + for (i = 0 ; i < qemu_driver->domains.count ; i++) {
> + virDomainObjPtr vm = qemu_driver->domains.objs[i];
> + qemudDomainStatusPtr status = NULL;
> + char *config = NULL;
> + int rc;
> +
> + virDomainObjLock(vm);
> + /* Read pid */
> + if ((rc = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) == 0)
> + DEBUG("Found pid %d for '%s'", vm->pid, vm->def->name);
> + else
> + goto next;
> +
> + if ((config = virDomainConfigFile(NULL,
> + driver->stateDir,
> + vm->def->name)) == NULL) {
> + qemudLog(QEMUD_ERR, _("Failed to read domain status for %s\n"),
> + vm->def->name);
> + goto next_error;
> + }
> +
> + status = qemudDomainStatusParseFile(NULL, driver->caps, config, 0);
> + if (status) {
> + vm->newDef = vm->def;
> + vm->def = status->def;
> + } else {
> + qemudLog(QEMUD_ERR, _("Failed to parse domain status for %s\n"),
> + vm->def->name);
> + goto next_error;
> + }
> +
> + if ((rc = qemudOpenMonitor(NULL, vm, status->monitorpath, 1)) != 0) {
> + qemudLog(QEMUD_ERR, _("Failed to reconnect monitor for %s: %d\n"),
> + vm->def->name, rc);
> + goto next_error;
> + } else
> + vm->monitorpath = status->monitorpath;
> +
> + vm->stdin_fd = qemudGetProcFD(vm->pid, 0);
> + vm->stdout_fd = qemudGetProcFD(vm->pid, 1);
> + vm->stderr_fd = qemudGetProcFD(vm->pid, 2);
NACK, to these 3 lines - since we go to trouble of creating the statefile,
we should store the FD numbers in the statefile too, instead of relying
in the linux specific /proc code.
> + if (vm->stdin_fd == -1 || vm->stdout_fd == -1 || vm->stderr_fd == -1)
> + goto next_error;
> +
> + if (((vm->stdout_watch = virEventAddHandle(vm->stdout_fd,
> + VIR_EVENT_HANDLE_READABLE |
> + VIR_EVENT_HANDLE_ERROR |
> + VIR_EVENT_HANDLE_HANGUP,
> + qemudDispatchVMEvent,
> + driver, NULL)) < 0) ||
> + ((vm->stderr_watch = virEventAddHandle(vm->stderr_fd,
> + VIR_EVENT_HANDLE_READABLE |
> + VIR_EVENT_HANDLE_ERROR |
> + VIR_EVENT_HANDLE_HANGUP,
> + qemudDispatchVMEvent,
> + driver, NULL)) < 0)) {
It is actually not neccessary to add the _ERROR or _HANGUP flags when
registering a handle. Those two conditions are implict for every read
or write watch. Yes, other code in QEMU driver adding them is bogus too.
> /**
> * qemudStartup:
> *
> @@ -316,6 +454,7 @@ qemudStartup(void) {
> qemu_driver->autostartDir,
> NULL, NULL) < 0)
> goto error;
> + qemudReconnectVMs(qemu_driver);
> qemudAutostartConfigs(qemu_driver);
>
> qemuDriverUnlock(qemu_driver);
> @@ -418,11 +557,14 @@ qemudShutdown(void) {
>
> /* shutdown active VMs */
> for (i = 0 ; i < qemu_driver->domains.count ; i++) {
> + /* FIXME: don't shutdown VMs on daemon shutdown for now */
> +#if 0
> virDomainObjPtr dom = qemu_driver->domains.objs[i];
> virDomainObjLock(dom);
> if (virDomainIsActive(dom))
> qemudShutdownVMDaemon(NULL, qemu_driver, dom);
> virDomainObjUnlock(dom);
> +#endif
Yep, just remove this block of code entirely - we dont want to
shutdown VMs anymore. If someone wants to explicitly save or
shutdown VMs during machine shutdown, they're probably better
off explicitly using virsh to do it from an initscript.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list