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