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

On Mon, Jul 20, 2009 at 06:20:06PM +0100, Mark McLoughlin wrote: > On Mon, 2009-07-20 at 17:03 +0100, Daniel P. Berrange wrote: > > On Mon, Jul 20, 2009 at 02:54:34PM +0100, Mark McLoughlin wrote: > > > 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; > > > > IMHO, this should be kept in the private header file. The fact that > > we have hack reserving some portion for our internal use doesn't need > > to be exposed to applications. > > Okay, I put it in libvirt_internal.h, assuming you don't want > domain_conf.h included in libvirt.c > > > > 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; > > > + } > > > > Here I just think we should be masking the flags off silently. There's > > no need to explicit tell apps about the existance of internal flags. > > Okay, done ACK to this whole series now. I've done some automated testing of it and it seems to be working reasonably reliably.