[libvirt] [PATCH 01/14] Add XML flag for internal domain status

[Daniel P. Berrange]

On Mon, Jul 20, 2009 at 02:18:38PM +0200, Daniel Veillard wrote:
> On Mon, Jul 20, 2009 at 12:51:11PM +0100, Mark McLoughlin wrote:
> > We need to store things like device names and PCI slot numbers in the
> > qemu domain state file so that we don't lose that information on
> > libvirtd restart. Add a flag to indicate that this information should
> > be parsed or formatted.
> > 
> > * include/libvirt/libvirt.h: add VIR_DOMAIN_XML_INTERNAL_STATUS
> > 
> > * src/domain_conf.c: pass the flag from virDomainObjParseXML() and
> >   virDomainSaveStatus
> > 
> > * src/libvirt.c: reject the flag in virDomainGetXMLDesc()
> > ---
> >  include/libvirt/libvirt.h    |    3 ++-
> >  include/libvirt/libvirt.h.in |    3 ++-
> >  src/domain_conf.c            |    8 ++++----
> >  src/libvirt.c                |    6 ++++++
> >  4 files changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
> > index 90007a1..07495fc 100644
> > --- a/include/libvirt/libvirt.h
> > +++ b/include/libvirt/libvirt.h
> > @@ -585,7 +585,8 @@ int                     virDomainGetSecurityLabel (virDomainPtr domain,
> >  
> >  typedef enum {
> >      VIR_DOMAIN_XML_SECURE = 1, /* dump security sensitive information too */
> > -    VIR_DOMAIN_XML_INACTIVE = 2/* dump inactive domain information */
> > +    VIR_DOMAIN_XML_INACTIVE = 2, /* dump inactive domain information */
> > +    VIR_DOMAIN_XML_INTERNAL_STATUS = 4, /* dump internal domain status information */
> >  } virDomainXMLFlags;
> >  
> >  char *                  virDomainGetXMLDesc     (virDomainPtr domain,
> > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> > index ba2b6f0..6794c61 100644
> > --- a/include/libvirt/libvirt.h.in
> > +++ b/include/libvirt/libvirt.h.in
> > @@ -585,7 +585,8 @@ int                     virDomainGetSecurityLabel (virDomainPtr domain,
> >  
> >  typedef enum {
> >      VIR_DOMAIN_XML_SECURE = 1, /* dump security sensitive information too */
> > -    VIR_DOMAIN_XML_INACTIVE = 2/* dump inactive domain information */
> > +    VIR_DOMAIN_XML_INACTIVE = 2, /* dump inactive domain information */
> > +    VIR_DOMAIN_XML_INTERNAL_STATUS = 4, /* dump internal domain status information */
> >  } virDomainXMLFlags;
> >  
> >  char *                  virDomainGetXMLDesc     (virDomainPtr domain,
> > diff --git a/src/domain_conf.c b/src/domain_conf.c
> > index f3e4c6c..10e6ac6 100644
> > --- a/src/domain_conf.c
> > +++ b/src/domain_conf.c
> > @@ -2896,7 +2896,8 @@ static virDomainObjPtr virDomainObjParseXML(virConnectPtr conn,
> >  
> >      oldnode = ctxt->node;
> >      ctxt->node = config;
> > -    obj->def = virDomainDefParseXML(conn, caps, ctxt, 0);
> > +    obj->def = virDomainDefParseXML(conn, caps, ctxt,
> > +                                    VIR_DOMAIN_XML_INTERNAL_STATUS);
> >      ctxt->node = oldnode;
> >      if (!obj->def)
> >          goto error;
> > @@ -4277,12 +4278,11 @@ int virDomainSaveStatus(virConnectPtr conn,
> >                          const char *statusDir,
> >                          virDomainObjPtr obj)
> >  {
> > +    int flags = VIR_DOMAIN_XML_SECURE|VIR_DOMAIN_XML_INTERNAL_STATUS;
> >      int ret = -1;
> >      char *xml;
> >  
> > -    if (!(xml = virDomainObjFormat(conn,
> > -                                   obj,
> > -                                   VIR_DOMAIN_XML_SECURE)))
> > +    if (!(xml = virDomainObjFormat(conn, obj, flags)))
> >          goto cleanup;
> >  
> >      if (virDomainSaveXML(conn, statusDir, obj->def, xml))
> > diff --git a/src/libvirt.c b/src/libvirt.c
> > index f4a7fa7..c8926b3 100644
> > --- a/src/libvirt.c
> > +++ b/src/libvirt.c
> > @@ -2730,6 +2730,12 @@ virDomainGetXMLDesc(virDomainPtr domain, int flags)
> >          goto error;
> >      }
> >  
> > +    if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) {
> > +        virLibConnError(conn, VIR_ERR_OPERATION_DENIED,
> > +                        _("virDomainGetXMLDesc with internal status flag"));
> > +        goto error;
> > +    }
> > +
> >      if (conn->driver->domainDumpXML) {
> >          char *ret;
> >          ret = conn->driver->domainDumpXML (domain, flags);
> 
>   Hum, that's very confusing. Why expose that flag at the API level
> but forbid it's use from the API ?
>   Seems to me adding an extra parameter to the internal function
> virDomainDefParseXML() is a far cleaner way to do things by looking at
> this patch.

It'd be nice to only have 1 flag parameter for the internal methods.
Having 'flags' and 'privateFlags' to the same method is just going
to lead to code errors, passing the wrong flag in and it silently
failing 

We should not be adding this to the public API header file though.

Since we only have 2 flags in use currently, lets just mask off
the top 16 bits for internal use.

So in domain_conf.h add a enum starting at the 16th bit


   typedef enum {
     VIR_DOMAIN_XML_INTERNAL_STATUS = (1<<16), /* dump internal domain status information */
  } virDomainXMLInternalFlags;


And to be sure no one abusing this from public API, in
virDomainGetXMLDesc() scrub the incoming flags

  flags = flags & 0xffff;

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