3:16 p.m.
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.
[..snip..]
> +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,
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