diff mbox

[2/3] xen: hypercall docs annotations for xen_sysctl_cpupool_op

Message ID 1460653660-6654-3-git-send-email-ian.jackson@eu.citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ian Jackson April 14, 2016, 5:07 p.m. UTC
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
---
 xen/include/public/sysctl.h |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Dario Faggioli April 14, 2016, 8:37 p.m. UTC | #1
On Thu, 2016-04-14 at 18:07 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Tim Deegan <tim@xen.org>
>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

One thing, out of curiosity. This syntax, here:

> -/* XEN_SYSCTL_cpupool_op */
> +/* ` enum XEN_SYSCTL_cpupool_op { */
>  #define XEN_SYSCTL_CPUPOOL_OP_CREATE                1  /* C */
>  #define XEN_SYSCTL_CPUPOOL_OP_DESTROY               2  /* D */
>  #define XEN_SYSCTL_CPUPOOL_OP_INFO                  3  /* I */
> @@ -546,9 +546,10 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_numainfo_t);
>  #define XEN_SYSCTL_CPUPOOL_OP_RMCPU                 5  /* R */
>  #define XEN_SYSCTL_CPUPOOL_OP_MOVEDOMAIN            6  /* M */
>  #define XEN_SYSCTL_CPUPOOL_OP_FREEINFO              7  /* F */
> +/* ` } */
>  #define XEN_SYSCTL_CPUPOOL_PAR_ANY     0xFFFFFFFF
>  struct xen_sysctl_cpupool_op {
> -    uint32_t op;          /* IN */
> +    uint32_t op;          /* IN ` enum XEN_SYSCTL_cpupool_op ` */
>
and here, is I guess useful to the hypercall HTML docs generator, as
mentioned in the cover letter?

It is not something we do in many other places (if at all, at least in
this file)... If it is, I'll happily add to my TODO list to convert
more entries to it.

Regards,
Dario
Ian Jackson April 15, 2016, 10:27 a.m. UTC | #2
Dario Faggioli writes ("Re: [PATCH 2/3] xen: hypercall docs annotations for xen_sysctl_cpupool_op"):
> On Thu, 2016-04-14 at 18:07 +0100, Ian Jackson wrote:
> > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > CC: Jan Beulich <jbeulich@suse.com>
> > CC: Tim Deegan <tim@xen.org>
> >
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> 
> One thing, out of curiosity. This syntax, here:
> 
> > -/* XEN_SYSCTL_cpupool_op */
> > +/* ` enum XEN_SYSCTL_cpupool_op { */
...
> > +    uint32_t op;          /* IN ` enum XEN_SYSCTL_cpupool_op ` */
> >
> and here, is I guess useful to the hypercall HTML docs generator, as
> mentioned in the cover letter?

Yes.  Observe that here
  http://xenbits.xen.org/docs/unstable/hypercall/x86_64/index.html
does not contain an entry for the cpupool ops in the section
"Enums and sets of #defines".  And it doesn't even formally state that
the `op' in that struct is one of the XEN_SYSCTL_cpupool_op values -
although the reader will probably guess that that's the case.

The extra markup syntax is pretty minimal and is documented at the top
of the script
  docs/xen-headers

> It is not something we do in many other places (if at all, at least in
> this file)... If it is, I'll happily add to my TODO list to convert
> more entries to it.

Ideally I would like to see all hypercalls and all their arguments
documented properly.  That includes annotating them for the html
cross-reference generator.  But it also includes documenting their
semantics and their error conditions.

Unfortunately, we are starting from a rather sketchy baseline.

Thanks for the offer to help!

Ian.
diff mbox

Patch

diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 4596d20..0849908 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -538,7 +538,7 @@  struct xen_sysctl_numainfo {
 typedef struct xen_sysctl_numainfo xen_sysctl_numainfo_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_numainfo_t);
 
-/* XEN_SYSCTL_cpupool_op */
+/* ` enum XEN_SYSCTL_cpupool_op { */
 #define XEN_SYSCTL_CPUPOOL_OP_CREATE                1  /* C */
 #define XEN_SYSCTL_CPUPOOL_OP_DESTROY               2  /* D */
 #define XEN_SYSCTL_CPUPOOL_OP_INFO                  3  /* I */
@@ -546,9 +546,10 @@  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_numainfo_t);
 #define XEN_SYSCTL_CPUPOOL_OP_RMCPU                 5  /* R */
 #define XEN_SYSCTL_CPUPOOL_OP_MOVEDOMAIN            6  /* M */
 #define XEN_SYSCTL_CPUPOOL_OP_FREEINFO              7  /* F */
+/* ` } */
 #define XEN_SYSCTL_CPUPOOL_PAR_ANY     0xFFFFFFFF
 struct xen_sysctl_cpupool_op {
-    uint32_t op;          /* IN */
+    uint32_t op;          /* IN ` enum XEN_SYSCTL_cpupool_op ` */
     uint32_t cpupool_id;  /* IN: CDIARM OUT: CI */
     uint32_t sched_id;    /* IN: C      OUT: I  */
     uint32_t domid;       /* IN: M              */