[libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types

[Victor Toso]

Hi,

On Wed, May 04, 2022 at 07:02:48PM +0100, Daniel P. Berrangé wrote:
> On Wed, May 04, 2022 at 07:30:08PM +0200, Andrea Bolognani wrote:
> > Due to hystorical reasons, it needs to be possible to pass values

hystorical -> historical ?

> > from the virTypedParameterFlags and virDomainModificationImpact
> > enumerations to a function at the same time, so it is very
> > important that the two never overlap.
> > 
> > Right now this is "enforced" by the presence of special comments;
> > unfortunately, said comments are not handled correctly by
> > apibuild.py and end up, quite confusingly, showing up as part of
> > the documentation for symbols preceding or following them.
> > 
> > Introduce actual entires in each enumeration for each of the
> > overlapping values, which is more explicit and results in
> > comments being parsed correctly.
> 
> I don't really like the idea of adding stuff to the public API
> to workaround brokenness in apibuild.py.

While apibuild.py needs to be fixed to error/warn in this
scenarios, I'd argue that the patch moves towards consistency
with comments blocks and improves the documentation of already
exposed API.

> It seems like we only need apibuild.py to not merge together
> distinct comment blocks.

What is not trivial is to (1) define which comment block belongs
to which element/type. We need to define what is acceptable and
what is not and (2) enforce that to stay consistent.

> > Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> > ---
> >  include/libvirt/libvirt-common.h.in | 19 +++++++++++++++++--
> >  include/libvirt/libvirt-domain.h    |  8 ++++----
> >  2 files changed, 21 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/libvirt/libvirt-common.h.in b/include/libvirt/libvirt-common.h.in
> > index ccdbb2a100..2f20456dfd 100644
> > --- a/include/libvirt/libvirt-common.h.in
> > +++ b/include/libvirt/libvirt-common.h.in
> > @@ -159,8 +159,23 @@ typedef enum {
> >   * Since: 0.9.8
> >   */
> >  typedef enum {
> > -    /* 1 << 0 is reserved for virDomainModificationImpact */
> > -    /* 1 << 1 is reserved for virDomainModificationImpact */
> > +    /* Reserved for virDomainModificationImpact. Do not use.
> > +     *
> > +     * Since: 8.4.0
> > +     */
> > +    VIR_TYPED_PARAM_RESERVED1 = 0,
> > +
> > +    /* Reserved for virDomainModificationImpact. Do not use.
> > +     *
> > +     * Since: 8.4.0
> > +     */
> > +    VIR_TYPED_PARAM_RESERVED2 = 1 << 0,
> > +
> > +    /* Reserved for virDomainModificationImpact. Do not use.
> > +     *
> > +     * Since: 8.4.0
> > +     */
> > +    VIR_TYPED_PARAM_RESERVED3 = 1 << 1,
> >  
> >      /* Older servers lacked the ability to handle string typed
> >       * parameters.  Attempts to set a string parameter with an older
> > diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> > index 2edef9c4e1..94cb4a6615 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -321,10 +321,10 @@ typedef virDomainControlInfo *virDomainControlInfoPtr;
> >   * Since: 0.9.2
> >   */
> >  typedef enum {
> > -    VIR_DOMAIN_AFFECT_CURRENT = 0,      /* Affect current domain state. (Since: 0.9.2)  */
> > -    VIR_DOMAIN_AFFECT_LIVE    = 1 << 0, /* Affect running domain state. (Since: 0.9.2)  */
> > -    VIR_DOMAIN_AFFECT_CONFIG  = 1 << 1, /* Affect persistent domain state. (Since: 0.9.2) */
> > -    /* 1 << 2 is reserved for virTypedParameterFlags */
> > +    VIR_DOMAIN_AFFECT_CURRENT   = 0,      /* Affect current domain state. (Since: 0.9.2)  */
> > +    VIR_DOMAIN_AFFECT_LIVE      = 1 << 0, /* Affect running domain state. (Since: 0.9.2)  */
> > +    VIR_DOMAIN_AFFECT_CONFIG    = 1 << 1, /* Affect persistent domain state. (Since: 0.9.2) */
> > +    VIR_DOMAIN_AFFECT_RESERVED1 = 1 << 2, /* Reserved for virTypedParameterFlags. Do not use. (Since: 8.4.0) */
> >  } virDomainModificationImpact;
> >  
> >  /**
> > -- 
> > 2.35.1
> > 
> 
> With regards,
> Daniel

Cheers,
Victor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220505/11c5ad3c/attachment.sig>