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@xxxxxxxxxx>
> > ---
> > 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
Attachment:
signature.asc
Description: PGP signature
