12:30 p.m.
Andrea Bolognani (2):
lib: Drop "type hint" comments
include: Explicitly reserve values for overlapping flag types
include/libvirt/libvirt-common.h.in | 19 +++++++++++++++++--
include/libvirt/libvirt-domain.h | 8 ++++----
src/libvirt-network.c | 4 ++--
3 files changed, 23 insertions(+), 8 deletions(-)
--
2.35.1
12:30 p.m.
New subject: [libvirt PATCH 1/2] lib: Drop "type hint" comments
Their presence confuses apibuild.py, which considers them to be
part of the previous comment block and so appends them verbatim
to the documentation for virNetworkUpdate().
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/libvirt-network.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index 50c4108641..dc50f33eba 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -637,8 +637,8 @@ virNetworkUndefine(virNetworkPtr network)
*/
int
virNetworkUpdate(virNetworkPtr network,
- unsigned int command, /* virNetworkUpdateCommand */
- unsigned int section, /* virNetworkUpdateSection */
+ unsigned int command,
+ unsigned int section,
int parentIndex,
const char *xml,
unsigned int flags)
--
2.35.1
12:30 p.m.
New subject: [libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types
Due to hystorical reasons, it needs to be possible to pass values
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.
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
1:02 p.m.
New subject: [libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types
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
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.
It seems like we only need apibuild.py to not merge together
distinct comment blocks.
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
--
|: 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 :|
2:48 a.m.
New subject: [libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types
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
2:57 a.m.
New subject: [libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types
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(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
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 :|
4:04 a.m.
New subject: [libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types
Hi,
On Thu, May 05, 2022 at 08:57:04AM +0100, Daniel P. Berrangé wrote:
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.
While working on the Since metadata, I recall finding instances
of:
/* (C1) Comment before enum value */
ENUM_VALUE, /* (C2) Comment same line
that might span to multiple lines */
/* (C3) Comment after enum value */
Not necessary at the same time, but the point is that they were
meant to document ENUM_VALUE. IIRC, (C3) was only for the enum
sentinel value (_ENUM_TYPE_LAST).
Sure, we can pick one and follow it but there is some fixing to
do in the existing documentation to keep it consistent.
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.
I think we should not merge them, but apibuild should warn/error
instead as it seems unlikely that two block of comments refer to
a single type.
> > > 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
With regards,
Daniel
Cheers,
Victor
4:13 a.m.
New subject: [libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types
On Thu, May 05, 2022 at 08:57:04AM +0100, Daniel P. Berrangé wrote:
On Thu, May 05, 2022 at 09:48:54AM +0200, Victor Toso wrote:
> On Wed, May 04, 2022 at 07:02:48PM +0100, Daniel P. Berrangé wrote:
> > 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.
This makes sense, at least in theory. I have no idea how difficult it
would be to actually convince apibuild.py to behave this way though.
I can give it a shot, but I'm concerned about falling into a real
rabbit hole with this one. If I don't manage to bend the script to my
will quickly enough, I'll just give up on the idea and leave things
as they are.
--
Andrea Bolognani / Red Hat / Virtualization
5:40 a.m.
New subject: [libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types
On Thu, May 05, 2022 at 02:13:15AM -0700, Andrea Bolognani wrote:
On Thu, May 05, 2022 at 08:57:04AM +0100, Daniel P. Berrangé wrote:
> On Thu, May 05, 2022 at 09:48:54AM +0200, Victor Toso wrote:
> > On Wed, May 04, 2022 at 07:02:48PM +0100, Daniel P. Berrangé wrote:
> > > 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.
This makes sense, at least in theory. I have no idea how difficult it
would be to actually convince apibuild.py to behave this way though.
I can give it a shot, but I'm concerned about falling into a real
rabbit hole with this one. If I don't manage to bend the script to my
will quickly enough, I'll just give up on the idea and leave things
as they are.
Alternatively the classic approach to this problem taken by javadoc
and gtk-doc, is to require API comments to use '/**' and leave '/*'
for non-API comments. We've got a reasonably large amount of usage
of '/**' but I'm guessing it isn't complete.
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 :|
6:58 a.m.
New subject: [libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types
On Thu, May 05, 2022 at 11:40:55AM +0100, Daniel P. Berrangé wrote:
On Thu, May 05, 2022 at 02:13:15AM -0700, Andrea Bolognani wrote:
> On Thu, May 05, 2022 at 08:57:04AM +0100, Daniel P. Berrangé wrote:
> > 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.
>
> This makes sense, at least in theory. I have no idea how difficult it
> would be to actually convince apibuild.py to behave this way though.
>
> I can give it a shot, but I'm concerned about falling into a real
> rabbit hole with this one. If I don't manage to bend the script to my
> will quickly enough, I'll just give up on the idea and leave things
> as they are.
Alternatively the classic approach to this problem taken by javadoc
and gtk-doc, is to require API comments to use '/**' and leave '/*'
for non-API comments. We've got a reasonably large amount of usage
of '/**' but I'm guessing it isn't complete.
Yeah I wouldn't mind if we got rid of same-line comments altogether
and used block comments everywhere. We wouldn't necessarily even have
to change apibuild.py, just update all existing uses. That'd result
in a lot of churn, of course.
Do you have an opinion on the possibility of switching to an
off-the-shelf solution for documenting our API? I name-dropped
Doxygen in the past, but I'm not actually familiar with it. Not sure
how gtk-doc would work for a library that doesn't expose a GLib-based
API.
--
Andrea Bolognani / Red Hat / Virtualization
7:07 a.m.
New subject: [libvirt PATCH 2/2] include: Explicitly reserve values for overlapping flag types
On Thu, May 05, 2022 at 04:58:10AM -0700, Andrea Bolognani wrote:
On Thu, May 05, 2022 at 11:40:55AM +0100, Daniel P. Berrangé wrote:
> On Thu, May 05, 2022 at 02:13:15AM -0700, Andrea Bolognani wrote:
> > On Thu, May 05, 2022 at 08:57:04AM +0100, Daniel P. Berrangé wrote:
> > > 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.
> >
> > This makes sense, at least in theory. I have no idea how difficult it
> > would be to actually convince apibuild.py to behave this way though.
> >
> > I can give it a shot, but I'm concerned about falling into a real
> > rabbit hole with this one. If I don't manage to bend the script to my
> > will quickly enough, I'll just give up on the idea and leave things
> > as they are.
>
> Alternatively the classic approach to this problem taken by javadoc
> and gtk-doc, is to require API comments to use '/**' and leave '/*'
> for non-API comments. We've got a reasonably large amount of usage
> of '/**' but I'm guessing it isn't complete.
Yeah I wouldn't mind if we got rid of same-line comments altogether
and used block comments everywhere. We wouldn't necessarily even have
to change apibuild.py, just update all existing uses. That'd result
in a lot of churn, of course.
Do you have an opinion on the possibility of switching to an
off-the-shelf solution for documenting our API? I name-dropped
Doxygen in the past, but I'm not actually familiar with it. Not sure
how gtk-doc would work for a library that doesn't expose a GLib-based
API.
I'm filled with despair any time I look at a project whose API docs
are generated by Doxygen. All the information is there, but somehow
Doxygen manages to make it awful to find the things you want. To be
fair I think is largely because C is unimaginably flexible.
gtk-doc produces much nicer docs, but it is slightly opinionated
about the way your API is structured and naming conventions in use,
so won't work for all C libraries. It has support for GObject specific
features but you can ignore that. It'd probably be able to make it
work for libvirt with some hacking.
Unfortunately gtk-doc is being pushed towards retirement:
https://blog.gtk.org/2021/08/19/the-gtk-documentation/
while the code isn't going anywhere, it wont likely get much love
going fowards.
The replacement based on gobject introspection is likely to be quite
challenging to use for libvirt, as it is a bit more opinionated about
API design than even gtk-doc. On the flip side if it were possible to
get gobject introspection working with libvirt, it'd open up ability
to use libvirt from many languages without manually writing bindings.
Finally, even if we did use something else for API docs, we still
actually need apibuild.py to work correctly, because we use it to
expose an XML representation of our API that several language bindings
rely on.
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 :|
933
days inactive
934
days old
10 comments
3 participants
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Victor Toso