On Tue, Nov 25, 2008 at 10:16:35PM +0100, Guido G?nther wrote: > 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. e should avoid /proc because this is Linux specific, and QEMU is perfectly capable of being used on Solaris / BSD too. I'm fine with storing it explicitly in /var/run/libvirt/qemu/$DOMAN.monitor > > > > 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. Hmm, interesting idea - we already have a variant for parsing based off an arbitrary XML node, rather than assuming the root, virDomainDefPtr virDomainDefParseNode(virConnectPtr conn, virCapsPtr caps, xmlDocPtr doc, xmlNodePtr root); So we could add a wrapper function which writes out a XML doc <domstate state="running" pid="1234"> <monitor path="/dev/pts/4"> <domain id='32'> ... normal domain XML config ... </domain> </domstate> And so have a function virDomainObjPtr virDomainObjParse(const char *filename) which'd parse the custom QEMU state, and then just invoke the virDomainDefParseNode for the nested <domain> tag. Now, some things like VNC port, domain ID really are easily tracked in the regular domain XML - rather than adding VIR_DOMAIN_XML_STATE I've a slight alternative idea. The formatXML methods already accept a flag VIR_DOMAIN_XML_INACTIVE which when passed causes it to skip some attributes which only make sense for running VMs - VNC port, ID, VIF name The parseXML methods, just hardcode this and always skip these attributes. We should fix this so the parsing only skips these attrs if this same VIR_DOMAIN_XML_INACTIVE flag is passed. Then just make sure all existing code is changed to set this flag when parsing XML. The code for loading VM from disk can simply not set this flag. Technically the LXC driver shold be doing this already, precisely for the VIF name issue. 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