[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.


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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]