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(a)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