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

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.