On Mon, 2009-07-20 at 14:06 +0100, Daniel P. Berrange wrote:
> On Mon, Jul 20, 2009 at 02:18:38PM +0200, Daniel Veillard wrote:
>
> > 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;
How's this?
Cheers,
Mark.
From: Mark McLoughlin <markmc@xxxxxxxxxx>
Subject: [PATCH 01/14] Add internal XML parsing/formatting flag
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.
Make bit 16 and above of the flags bitmask for internal use only and
consume the first bit for this new status flag.
* include/libvirt/libvirt.h: add VIR_DOMAIN_XML_FLAGS_MASK
* src/libvirt.c: reject private flags in virDomainGetXMLDesc()
* src/domain_conf.h: add VIR_DOMAIN_XML_INTERNAL_STATUS
* src/domain_conf.c: pass the flag from virDomainObjParseXML() and
virDomainSaveStatus
---
include/libvirt/libvirt.h | 5 +++--
include/libvirt/libvirt.h.in | 5 +++--
src/domain_conf.c | 8 ++++----
src/domain_conf.h | 5 +++++
src/libvirt.c | 6 ++++++
5 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index 90007a1..d1cf5fb 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -584,8 +584,9 @@ 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_SECURE = (1<<0), /* dump security sensitive information too */
+ VIR_DOMAIN_XML_INACTIVE = (1<<1), /* dump inactive domain information */
+ VIR_DOMAIN_XML_FLAGS_MASK = 0xffff, /* bits 16 and above are for internal use */
} virDomainXMLFlags;
char * virDomainGetXMLDesc (virDomainPtr domain,
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index ba2b6f0..5445cba 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -584,8 +584,9 @@ 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_SECURE = (1<<0), /* dump security sensitive information too */
+ VIR_DOMAIN_XML_INACTIVE = (1<<1), /* dump inactive domain information */
+ VIR_DOMAIN_XML_FLAGS_MASK = 0xffff, /* bits 16 and above are for internal use */
} 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/domain_conf.h b/src/domain_conf.h
index 6e111fa..69b665f 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -33,6 +33,11 @@
#include "util.h"
#include "threads.h"
+/* Private component of virDomainXMLFlags */
+typedef enum {
+ VIR_DOMAIN_XML_INTERNAL_STATUS = (1<<16), /* dump internal domain status information */
+} virDomainXMLInternalFlags;
+
/* Different types of hypervisor */
/* NB: Keep in sync with virDomainVirtTypeToString impl */
enum virDomainVirtType {
diff --git a/src/libvirt.c b/src/libvirt.c
index f4a7fa7..5507750 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2730,6 +2730,12 @@ virDomainGetXMLDesc(virDomainPtr domain, int flags)
goto error;
}
+ if (flags != (flags & VIR_DOMAIN_XML_FLAGS_MASK)) {
+ virLibConnError(conn, VIR_ERR_OPERATION_DENIED,
+ _("virDomainGetXMLDesc with internal flags"));
+ goto error;
+ }
+
if (conn->driver->domainDumpXML) {
char *ret;
ret = conn->driver->domainDumpXML (domain, flags);
--
1.6.2.5
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list