[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] save state as string



On Tue, Jan 06, 2009 at 04:38:50PM +0100, Guido G?nther wrote:
> On Tue, Jan 06, 2009 at 03:27:52PM +0100, Daniel Veillard wrote:
> > On Mon, Dec 29, 2008 at 03:44:54PM +0100, Guido Günther wrote:
> > > Hi,
> > > current domstatus code saves the domain as number, attached patch safes
> > > it as string which looks a bit nicer.
> > [...]
> > > @@ -1414,7 +1416,9 @@ qemudDomainStatusFormat(virConnectPtr conn,
> > >      char *config_xml = NULL, *xml = NULL;
> > >      virBuffer buf = VIR_BUFFER_INITIALIZER;
> > >  
> > > -    virBufferVSprintf(&buf, "<domstatus state='%d' pid='%d'>\n", vm->state, vm->pid);
> > > +    virBufferVSprintf(&buf, "<domstatus state='%s' pid='%d'>\n",
> > > +                      virDomainStateTypeToString(vm->state),
> > > +                      vm->pid);
> > >      virBufferEscapeString(&buf, "  <monitor path='%s'/>\n", vm->monitorpath);
> > >  
> > >      if (!(config_xml = virDomainDefFormat(conn,
> > 
> >   Hum, I have 2 objections to this patch:
> >     - seems we keep a string value internally in the structure, I don't
> >       see the benefit, or maybe I misunderstood.
> Currently we have state='1' for running, with this patch this reads
> state='running' which is better readable.
> 
> >     - second more important is that we change the XML interface, that we
> >       can't do, maybe we can extend it to provide a stateinfo="%s"
> >       along the state='%d', but we can't just break code which may rely
> >       on the existing format.
> The file we write is /var/run/libvirt/qemu/*.xml where we keep the
> internal state of a running vm. We can change this at at any time.

Yes & no, but mostly no.

These persistent state files will need to be stable across releases of
libvirt, to allow sensible live upgrade path for hosts. eg, you're
running libvirtd with several VMs, upgrade to latest RPM which does
a 'service libvirtd restart' in the %post install section. The new 
libvirtd needs to cope with the state file created by the old libvirtd.

In this particular case though, its not problem changing state=1 to
state=running, since we've not released this code yet and the named
attribute is nicer to read.

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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]