On Thu, May 05, 2022 at 09:48:54AM +0200, Victor Toso wrote:
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.
If we have multiple opened+clsoed comment blocks immediately after each other such as this scenario: /* 1 << 0 is reserved for virDomainModificationImpact */ /* 1 << 1 is reserved for virDomainModificationImpact */ /* Older servers lacked the ability to handle string typed * parameters. Attempts to set a string parameter with an older * server will fail at the client, but attempts to retrieve * parameters must not return strings from a new server to an * older client, so this flag exists to identify newer clients to * newer servers. This flag is automatically set when needed, so * the user does not have to worry about it; however, manually * setting the flag can be used to reject servers that cannot * return typed strings, even if no strings would be returned. * * Since: v0.9.8 */ VIR_TYPED_PARAM_STRING_OKAY = 1 << 2, IMHO it is pretty straightforward for apibuild.py to have a policy that the comment block closest to the declaration is the API docs and the preceeding ones are irrelevant to hte API docs. I very much doubt we hav a case where we have multiple open+closed comment blocks which all should be part of the API docs for a given declaration, and if we did, then we should merge them into a single open+closed comment block.
Signed-off-by: Andrea Bolognani <abologna@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
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|