Finally got around to have another look at qemu/kvm surviving libvirt restarts: On Tue, Oct 07, 2008 at 05:37:52PM +0100, Daniel P. Berrange wrote:
diff --git a/src/domain_conf.h b/src/domain_conf.h index 632e5ad..1ac1561 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -448,6 +448,7 @@ struct _virDomainObj { int stdout_fd; int stderr_fd; int monitor; + char *monitorpath;
I think we can avoid needing this. If we write out the staste the moment the VM is started, we don't need to carry this around in memory. Alternatively, since we're writing all stdout/err data to the logfile, we could simply re-parse the log to extract the monitor path upon restart. I'm not sure reparsing the log is that good. Maybe the log got rotated in the meantime or the admin moved it away. So I'd rather keep this in /var/run/ too. We don't need that extra monitorpath variable we though, we can simply pick it from /proc/<libvirtpid>/fd/<monitorfd> when writing out the state information.
+static int +qemudFileReadMonitor(const char *dir, + const char *name, + char **path) +{ + int rc; + FILE *file; + char *monitorfile = NULL; + + if (asprintf(&monitorfile, "%s/%s.monitor", dir, name) < 0) { + rc = ENOMEM; + goto cleanup; + } + + if (!(file = fopen(monitorfile, "r"))) { + rc = errno; + goto cleanup; + } + + if (fscanf(file, "%as", path) != 1) { + rc = EINVAL; + goto cleanup; + } + + if (fclose(file) < 0) { + rc = errno; + goto cleanup; + } + + rc = 0; + + cleanup: + VIR_FREE(monitorfile); + return rc; +}
If we re-parse the logfile to extract the monitiro PTY path, this is unneccessary. If not, this can be simplied by just calling the convenient virFileReadAll() method. We have several things that need to be (re)stored. The id (if we don't switch over to using PIDs), the monitor path, the domain state (running,
[..snip..] paused, ...), and the acutally used vncport are things that come to mind. Some of the information is already in the domain.xml but not getting reparsed properly (e.g. def->id is always set to -1 and the vncport isn't reread if "autport=yes"). Therefore I introduced a flag VIR_DOMAIN_XML_STATE that tells the virDomain*DefParse functions to set those values when reading back "state" not "config". This somewhat mixes config with state which I don't like too much. It would probably be better to add an extra set of functions that takes the virDomainObjPtr instead of the virDomainDefPtr? This way we could also fold in the monitor path and the domain state easily like: <domstate state="running"> <monitor path="/dev/pts/4"/> <domstate/> Does this make sense? For now I use an extra file for the monitor path. [..snip..]
+static int +qemudSaveDomainState(struct qemud_driver * driver, virDomainObjPtr vm) { + int ret; + + if ((ret = virFileWritePid(driver->stateDir, vm->def->name, vm->pid)) != 0) { + qemudLog(QEMUD_ERR, _("Unable to safe pidfile for %s: %s\n"), + vm->def->name, strerror(ret)); + return ret; + } + if ((ret = virDomainSaveConfig(NULL, driver->stateDir, vm->def))) { + qemudLog(QEMUD_ERR, _("Unable to save domain %s\n"), vm->def->name); + return ret; + } + if ((ret = qemudFileWriteMonitor(driver->stateDir, vm->def->name, vm->monitorpath)) != 0) { + qemudLog(QEMUD_ERR, _("Unable to monitor file for %s: %s\n"), + vm->def->name, strerror(ret)); + return ret; + } + return 0; +} +
This will need to be called at time of VM creation, and the XSL config will need updating whether live config changes. O.k. I'm currently only calling this when forking qemu, I'll add the calls to the places where the config changes soonish.
[..snip..]
diff --git a/src/util.h b/src/util.h index 093ef46..8fbe2cd 100644 --- a/src/util.h +++ b/src/util.h @@ -32,6 +32,7 @@ enum { VIR_EXEC_NONE = 0, VIR_EXEC_NONBLOCK = (1 << 0), VIR_EXEC_DAEMON = (1 << 1), + VIR_EXEC_SETSID = (1 << 2), };
Shouldn't we simply be starting all the QEMU VMs with VIR_EXEC_DAEMON so they totally disassociate themselves from libvirtd right away. They will be disassociated anyway if the libvirtd is ever restarted, so best to daemonize from time they are started, to avoid any surprising changes in behaviour Done. -- Guido