Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Tomáš Golembiovský
On Fri, 21 Jul 2017 13:26:14 +0200
Michal Privoznik  wrote:

> On 07/21/2017 01:17 PM, Tomáš Golembiovský wrote:
> > On Fri, 21 Jul 2017 12:58:55 +0200
> > Michal Privoznik  wrote:
> >   
> >> On 07/21/2017 12:25 PM, Tomáš Golembiovský wrote:  
> >>> On Fri, 21 Jul 2017 10:12:38 +0200
> >>> Michal Privoznik  wrote:
> >>> 
>  On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
> > The documentation string has to follow the definition of a constant in
> > the enum. Otherwise, the HTML documentation will be generated
> > incorrectly.
> >
> > Signed-off-by: Tomáš Golembiovský 
> > ---
> >  include/libvirt/libvirt-domain.h | 62 
> > 
> >  1 file changed, 31 insertions(+), 31 deletions(-)
> >
> > diff --git a/include/libvirt/libvirt-domain.h 
> > b/include/libvirt/libvirt-domain.h
> > index 45f939a8c..2f3162d0f 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> > *virDomainInterfaceStatsPtr;
> >   * Memory Statistics Tags:
> >   */
> >  typedef enum {
> > -/* The total amount of data read from swap space (in kB). */
> >  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> > -/* The total amount of memory written out to swap space (in kB). */
> > +/* The total amount of data read from swap space (in kB). */
> >  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> > +/* The total amount of memory written out to swap space (in kB). 
> > */  
> 
>  While this fixes generated HTML, it messes up the header file. So if
>  somebody is looking directly into header file they might get confused.
>  The problem is in our doc generator.
> >>>
> >>> I agree that it's not ideal solution. But since it's already used in the
> >>> header, e.g. in virDomainBlockJobType and
> >>> virDomainEventShutdownDetailType, 
> >>
> >> This one actually looks okay. Did you perhaps mean
> >> virConnectDomainEventDiskChangeReason?  
> > 
> > No, I really meant virDomainEventShutdownDetailType.  
> 
> Are we looking at the same code? Here's what mine looks like:

Damn! We're not. I've been looking at my patched version and
virDomainEventShutdownDetailType was changed by me. Sorry for confusion.

Tomas

> 
> typedef enum {
> /* Guest finished shutdown sequence */
> VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED = 0,
> 
> /* Domain finished shutting down after request from the guest itself
>  * (e.g. hardware-specific action) */
> VIR_DOMAIN_EVENT_SHUTDOWN_GUEST = 1,
> 
> /* Domain finished shutting down after request from the host (e.g. killed 
> by
>  * a signal) */
> VIR_DOMAIN_EVENT_SHUTDOWN_HOST = 2,
> 
> # ifdef VIR_ENUM_SENTINELS
> VIR_DOMAIN_EVENT_SHUTDOWN_LAST
> # endif
> } virDomainEventShutdownDetailType;
> 
> 
> I see nothing wrong about it. Do you?
> 
> Michal


-- 
Tomáš Golembiovský 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Michal Privoznik
On 07/21/2017 01:17 PM, Tomáš Golembiovský wrote:
> On Fri, 21 Jul 2017 12:58:55 +0200
> Michal Privoznik  wrote:
> 
>> On 07/21/2017 12:25 PM, Tomáš Golembiovský wrote:
>>> On Fri, 21 Jul 2017 10:12:38 +0200
>>> Michal Privoznik  wrote:
>>>   
 On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:  
> The documentation string has to follow the definition of a constant in
> the enum. Otherwise, the HTML documentation will be generated
> incorrectly.
>
> Signed-off-by: Tomáš Golembiovský 
> ---
>  include/libvirt/libvirt-domain.h | 62 
> 
>  1 file changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index 45f939a8c..2f3162d0f 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> *virDomainInterfaceStatsPtr;
>   * Memory Statistics Tags:
>   */
>  typedef enum {
> -/* The total amount of data read from swap space (in kB). */
>  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> -/* The total amount of memory written out to swap space (in kB). */
> +/* The total amount of data read from swap space (in kB). */
>  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> +/* The total amount of memory written out to swap space (in kB). */  
>   

 While this fixes generated HTML, it messes up the header file. So if
 somebody is looking directly into header file they might get confused.
 The problem is in our doc generator.  
>>>
>>> I agree that it's not ideal solution. But since it's already used in the
>>> header, e.g. in virDomainBlockJobType and
>>> virDomainEventShutdownDetailType,   
>>
>> This one actually looks okay. Did you perhaps mean
>> virConnectDomainEventDiskChangeReason?
> 
> No, I really meant virDomainEventShutdownDetailType.

Are we looking at the same code? Here's what mine looks like:

typedef enum {
/* Guest finished shutdown sequence */
VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED = 0,

/* Domain finished shutting down after request from the guest itself
 * (e.g. hardware-specific action) */
VIR_DOMAIN_EVENT_SHUTDOWN_GUEST = 1,

/* Domain finished shutting down after request from the host (e.g. killed by
 * a signal) */
VIR_DOMAIN_EVENT_SHUTDOWN_HOST = 2,

# ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_EVENT_SHUTDOWN_LAST
# endif
} virDomainEventShutdownDetailType;


I see nothing wrong about it. Do you?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Tomáš Golembiovský
On Fri, 21 Jul 2017 12:58:55 +0200
Michal Privoznik  wrote:

> On 07/21/2017 12:25 PM, Tomáš Golembiovský wrote:
> > On Fri, 21 Jul 2017 10:12:38 +0200
> > Michal Privoznik  wrote:
> >   
> >> On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:  
> >>> The documentation string has to follow the definition of a constant in
> >>> the enum. Otherwise, the HTML documentation will be generated
> >>> incorrectly.
> >>>
> >>> Signed-off-by: Tomáš Golembiovský 
> >>> ---
> >>>  include/libvirt/libvirt-domain.h | 62 
> >>> 
> >>>  1 file changed, 31 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/include/libvirt/libvirt-domain.h 
> >>> b/include/libvirt/libvirt-domain.h
> >>> index 45f939a8c..2f3162d0f 100644
> >>> --- a/include/libvirt/libvirt-domain.h
> >>> +++ b/include/libvirt/libvirt-domain.h
> >>> @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> >>> *virDomainInterfaceStatsPtr;
> >>>   * Memory Statistics Tags:
> >>>   */
> >>>  typedef enum {
> >>> -/* The total amount of data read from swap space (in kB). */
> >>>  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> >>> -/* The total amount of memory written out to swap space (in kB). */
> >>> +/* The total amount of data read from swap space (in kB). */
> >>>  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> >>> +/* The total amount of memory written out to swap space (in kB). */  
> >>>   
> >>
> >> While this fixes generated HTML, it messes up the header file. So if
> >> somebody is looking directly into header file they might get confused.
> >> The problem is in our doc generator.  
> > 
> > I agree that it's not ideal solution. But since it's already used in the
> > header, e.g. in virDomainBlockJobType and
> > virDomainEventShutdownDetailType,   
> 
> This one actually looks okay. Did you perhaps mean
> virConnectDomainEventDiskChangeReason?

No, I really meant virDomainEventShutdownDetailType. But you're right
about virConnectDomainEventDiskChangeReason too.

I didn't look at other headers, but as far as libvirt-domain.h is
concerned those three seem to be all. There are also several (5?)
misplaced comments for the sentinels if you care about those too.


> > I assumed it's acceptable. And the
> > overall readability is not that bad because the constant+doc pairs are
> > separated with newline from one another.  
> 
> Nope. It's not. I've sent a patch that fixes those two places (I've went
> through all of our public headers and found just those two though):
> 
> https://www.redhat.com/archives/libvir-list/2017-July/msg00871.html
> 
> > 
> > 
> >   
> >> I recall this being discussed
> >> somewhere recently (probably on the list?). The proper fix IMO is to fix
> >> the generator so that it accepts both:  
> > 
> > That would be the best solution, but I'm too scared to look at the
> > generator. That would be a job for somebody else.  
> 
> Yeah. Our generator is not that great. I wish that we'd switch to
> something already out there so that we don't have to maintain our own
> generator.
> 
> Michal


-- 
Tomáš Golembiovský 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Daniel P. Berrange
On Fri, Jul 21, 2017 at 01:03:13PM +0200, Michal Privoznik wrote:
> On 07/21/2017 01:01 PM, Daniel P. Berrange wrote:
> > On Fri, Jul 21, 2017 at 12:58:55PM +0200, Michal Privoznik wrote:
> >> 
> >> Yeah. Our generator is not that great. I wish that we'd switch to
> >> something already out there so that we don't have to maintain our own
> >> generator.
> > 
> > There's a good few options for docs generators. Unfortunately our code
> > is also used for generating the XML API description, that's in turn
> > used to generate some language bindings, and I don't know of any tools
> > that could replace that part. So it looks like we're stuck as long as
> > we want to support auto-generated bindings from the XML description
> 
> Well we can use proper doc generator for generating doc and then our own
> generator for generating the XML API description. BTW: what good options
> do you have in mind?

Doxygen, Sphinx, GTK-DOC, though I would not be in favour of Doxygen as
IMHO the docs it generates have awful navigation. QEMU is switching to
use Sphinx IIRC.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Michal Privoznik
On 07/21/2017 01:01 PM, Daniel P. Berrange wrote:
> On Fri, Jul 21, 2017 at 12:58:55PM +0200, Michal Privoznik wrote:
>> 
>> Yeah. Our generator is not that great. I wish that we'd switch to
>> something already out there so that we don't have to maintain our own
>> generator.
> 
> There's a good few options for docs generators. Unfortunately our code
> is also used for generating the XML API description, that's in turn
> used to generate some language bindings, and I don't know of any tools
> that could replace that part. So it looks like we're stuck as long as
> we want to support auto-generated bindings from the XML description

Well we can use proper doc generator for generating doc and then our own
generator for generating the XML API description. BTW: what good options
do you have in mind?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Daniel P. Berrange
On Fri, Jul 21, 2017 at 12:58:55PM +0200, Michal Privoznik wrote:
> On 07/21/2017 12:25 PM, Tomáš Golembiovský wrote:
> > On Fri, 21 Jul 2017 10:12:38 +0200
> > Michal Privoznik  wrote:
> > 
> >> On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
> >>> The documentation string has to follow the definition of a constant in
> >>> the enum. Otherwise, the HTML documentation will be generated
> >>> incorrectly.
> >>>
> >>> Signed-off-by: Tomáš Golembiovský 
> >>> ---
> >>>  include/libvirt/libvirt-domain.h | 62 
> >>> 
> >>>  1 file changed, 31 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/include/libvirt/libvirt-domain.h 
> >>> b/include/libvirt/libvirt-domain.h
> >>> index 45f939a8c..2f3162d0f 100644
> >>> --- a/include/libvirt/libvirt-domain.h
> >>> +++ b/include/libvirt/libvirt-domain.h
> >>> @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> >>> *virDomainInterfaceStatsPtr;
> >>>   * Memory Statistics Tags:
> >>>   */
> >>>  typedef enum {
> >>> -/* The total amount of data read from swap space (in kB). */
> >>>  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> >>> -/* The total amount of memory written out to swap space (in kB). */
> >>> +/* The total amount of data read from swap space (in kB). */
> >>>  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> >>> +/* The total amount of memory written out to swap space (in kB). */  
> >>
> >> While this fixes generated HTML, it messes up the header file. So if
> >> somebody is looking directly into header file they might get confused.
> >> The problem is in our doc generator.
> > 
> > I agree that it's not ideal solution. But since it's already used in the
> > header, e.g. in virDomainBlockJobType and
> > virDomainEventShutdownDetailType, 
> 
> This one actually looks okay. Did you perhaps mean
> virConnectDomainEventDiskChangeReason?
> 
> > I assumed it's acceptable. And the
> > overall readability is not that bad because the constant+doc pairs are
> > separated with newline from one another.
> 
> Nope. It's not. I've sent a patch that fixes those two places (I've went
> through all of our public headers and found just those two though):
> 
> https://www.redhat.com/archives/libvir-list/2017-July/msg00871.html
> 
> > 
> > 
> > 
> >> I recall this being discussed
> >> somewhere recently (probably on the list?). The proper fix IMO is to fix
> >> the generator so that it accepts both:
> > 
> > That would be the best solution, but I'm too scared to look at the
> > generator. That would be a job for somebody else.
> 
> Yeah. Our generator is not that great. I wish that we'd switch to
> something already out there so that we don't have to maintain our own
> generator.

There's a good few options for docs generators. Unfortunately our code
is also used for generating the XML API description, that's in turn
used to generate some language bindings, and I don't know of any tools
that could replace that part. So it looks like we're stuck as long as
we want to support auto-generated bindings from the XML description


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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Michal Privoznik
On 07/21/2017 12:25 PM, Tomáš Golembiovský wrote:
> On Fri, 21 Jul 2017 10:12:38 +0200
> Michal Privoznik  wrote:
> 
>> On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
>>> The documentation string has to follow the definition of a constant in
>>> the enum. Otherwise, the HTML documentation will be generated
>>> incorrectly.
>>>
>>> Signed-off-by: Tomáš Golembiovský 
>>> ---
>>>  include/libvirt/libvirt-domain.h | 62 
>>> 
>>>  1 file changed, 31 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/include/libvirt/libvirt-domain.h 
>>> b/include/libvirt/libvirt-domain.h
>>> index 45f939a8c..2f3162d0f 100644
>>> --- a/include/libvirt/libvirt-domain.h
>>> +++ b/include/libvirt/libvirt-domain.h
>>> @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
>>> *virDomainInterfaceStatsPtr;
>>>   * Memory Statistics Tags:
>>>   */
>>>  typedef enum {
>>> -/* The total amount of data read from swap space (in kB). */
>>>  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
>>> -/* The total amount of memory written out to swap space (in kB). */
>>> +/* The total amount of data read from swap space (in kB). */
>>>  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
>>> +/* The total amount of memory written out to swap space (in kB). */  
>>
>> While this fixes generated HTML, it messes up the header file. So if
>> somebody is looking directly into header file they might get confused.
>> The problem is in our doc generator.
> 
> I agree that it's not ideal solution. But since it's already used in the
> header, e.g. in virDomainBlockJobType and
> virDomainEventShutdownDetailType, 

This one actually looks okay. Did you perhaps mean
virConnectDomainEventDiskChangeReason?

> I assumed it's acceptable. And the
> overall readability is not that bad because the constant+doc pairs are
> separated with newline from one another.

Nope. It's not. I've sent a patch that fixes those two places (I've went
through all of our public headers and found just those two though):

https://www.redhat.com/archives/libvir-list/2017-July/msg00871.html

> 
> 
> 
>> I recall this being discussed
>> somewhere recently (probably on the list?). The proper fix IMO is to fix
>> the generator so that it accepts both:
> 
> That would be the best solution, but I'm too scared to look at the
> generator. That would be a job for somebody else.

Yeah. Our generator is not that great. I wish that we'd switch to
something already out there so that we don't have to maintain our own
generator.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Daniel P. Berrange
On Fri, Jul 21, 2017 at 12:25:02PM +0200, Tomáš Golembiovský wrote:
> On Fri, 21 Jul 2017 10:12:38 +0200
> Michal Privoznik  wrote:
> 
> > On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
> > > The documentation string has to follow the definition of a constant in
> > > the enum. Otherwise, the HTML documentation will be generated
> > > incorrectly.
> > > 
> > > Signed-off-by: Tomáš Golembiovský 
> > > ---
> > >  include/libvirt/libvirt-domain.h | 62 
> > > 
> > >  1 file changed, 31 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/include/libvirt/libvirt-domain.h 
> > > b/include/libvirt/libvirt-domain.h
> > > index 45f939a8c..2f3162d0f 100644
> > > --- a/include/libvirt/libvirt-domain.h
> > > +++ b/include/libvirt/libvirt-domain.h
> > > @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> > > *virDomainInterfaceStatsPtr;
> > >   * Memory Statistics Tags:
> > >   */
> > >  typedef enum {
> > > -/* The total amount of data read from swap space (in kB). */
> > >  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> > > -/* The total amount of memory written out to swap space (in kB). */
> > > +/* The total amount of data read from swap space (in kB). */
> > >  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> > > +/* The total amount of memory written out to swap space (in kB). */  
> > 
> > While this fixes generated HTML, it messes up the header file. So if
> > somebody is looking directly into header file they might get confused.
> > The problem is in our doc generator.
> 
> I agree that it's not ideal solution. But since it's already used in the
> header, e.g. in virDomainBlockJobType and
> virDomainEventShutdownDetailType, I assumed it's acceptable. And the
> overall readability is not that bad because the constant+doc pairs are
> separated with newline from one another.

I'm afraid, those examples are bad too and should never have been committed
as they are, so not a good guide to follow.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Daniel P. Berrange
On Fri, Jul 21, 2017 at 10:12:38AM +0200, Michal Privoznik wrote:
> On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
> > The documentation string has to follow the definition of a constant in
> > the enum. Otherwise, the HTML documentation will be generated
> > incorrectly.
> > 
> > Signed-off-by: Tomáš Golembiovský 
> > ---
> >  include/libvirt/libvirt-domain.h | 62 
> > 
> >  1 file changed, 31 insertions(+), 31 deletions(-)
> > 
> > diff --git a/include/libvirt/libvirt-domain.h 
> > b/include/libvirt/libvirt-domain.h
> > index 45f939a8c..2f3162d0f 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> > *virDomainInterfaceStatsPtr;
> >   * Memory Statistics Tags:
> >   */
> >  typedef enum {
> > -/* The total amount of data read from swap space (in kB). */
> >  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> > -/* The total amount of memory written out to swap space (in kB). */
> > +/* The total amount of data read from swap space (in kB). */
> >  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> > +/* The total amount of memory written out to swap space (in kB). */
> 
> While this fixes generated HTML, it messes up the header file. So if
> somebody is looking directly into header file they might get confused.

Yeah, absolutely NACK to this approach. Putting comments after the
constant name is awful for people reading the code.

> The problem is in our doc generator. I recall this being discussed
> somewhere recently (probably on the list?). The proper fix IMO is to fix
> the generator so that it accepts both:
> 
> enum {
>   /* Some very long description - therefore it's before the value. */
>   VIR_ENUM_A_VAL = 0,
> } virEnumA;
> 
> enum {
>   VIR_ENUM_B_VAL = 0, /* Short description */
> } virEnumB;
> 

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Tomáš Golembiovský
On Fri, 21 Jul 2017 10:12:38 +0200
Michal Privoznik  wrote:

> On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
> > The documentation string has to follow the definition of a constant in
> > the enum. Otherwise, the HTML documentation will be generated
> > incorrectly.
> > 
> > Signed-off-by: Tomáš Golembiovský 
> > ---
> >  include/libvirt/libvirt-domain.h | 62 
> > 
> >  1 file changed, 31 insertions(+), 31 deletions(-)
> > 
> > diff --git a/include/libvirt/libvirt-domain.h 
> > b/include/libvirt/libvirt-domain.h
> > index 45f939a8c..2f3162d0f 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> > *virDomainInterfaceStatsPtr;
> >   * Memory Statistics Tags:
> >   */
> >  typedef enum {
> > -/* The total amount of data read from swap space (in kB). */
> >  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> > -/* The total amount of memory written out to swap space (in kB). */
> > +/* The total amount of data read from swap space (in kB). */
> >  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> > +/* The total amount of memory written out to swap space (in kB). */  
> 
> While this fixes generated HTML, it messes up the header file. So if
> somebody is looking directly into header file they might get confused.
> The problem is in our doc generator.

I agree that it's not ideal solution. But since it's already used in the
header, e.g. in virDomainBlockJobType and
virDomainEventShutdownDetailType, I assumed it's acceptable. And the
overall readability is not that bad because the constant+doc pairs are
separated with newline from one another.



> I recall this being discussed
> somewhere recently (probably on the list?). The proper fix IMO is to fix
> the generator so that it accepts both:

That would be the best solution, but I'm too scared to look at the
generator. That would be a job for somebody else.

Tomas

> 
> enum {
>   /* Some very long description - therefore it's before the value. */
>   VIR_ENUM_A_VAL = 0,
> } virEnumA;
> 
> enum {
>   VIR_ENUM_B_VAL = 0, /* Short description */
> } virEnumB;
> 
> 
> Michal


-- 
Tomáš Golembiovský 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-21 Thread Michal Privoznik
On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
> The documentation string has to follow the definition of a constant in
> the enum. Otherwise, the HTML documentation will be generated
> incorrectly.
> 
> Signed-off-by: Tomáš Golembiovský 
> ---
>  include/libvirt/libvirt-domain.h | 62 
> 
>  1 file changed, 31 insertions(+), 31 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index 45f939a8c..2f3162d0f 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
> *virDomainInterfaceStatsPtr;
>   * Memory Statistics Tags:
>   */
>  typedef enum {
> -/* The total amount of data read from swap space (in kB). */
>  VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
> -/* The total amount of memory written out to swap space (in kB). */
> +/* The total amount of data read from swap space (in kB). */
>  VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
> +/* The total amount of memory written out to swap space (in kB). */

While this fixes generated HTML, it messes up the header file. So if
somebody is looking directly into header file they might get confused.
The problem is in our doc generator. I recall this being discussed
somewhere recently (probably on the list?). The proper fix IMO is to fix
the generator so that it accepts both:

enum {
  /* Some very long description - therefore it's before the value. */
  VIR_ENUM_A_VAL = 0,
} virEnumA;

enum {
  VIR_ENUM_B_VAL = 0, /* Short description */
} virEnumB;


Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] docs: fix documentation of enum constants

2017-07-20 Thread Tomáš Golembiovský
The documentation string has to follow the definition of a constant in
the enum. Otherwise, the HTML documentation will be generated
incorrectly.

Signed-off-by: Tomáš Golembiovský 
---
 include/libvirt/libvirt-domain.h | 62 
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 45f939a8c..2f3162d0f 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct 
*virDomainInterfaceStatsPtr;
  * Memory Statistics Tags:
  */
 typedef enum {
-/* The total amount of data read from swap space (in kB). */
 VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0,
-/* The total amount of memory written out to swap space (in kB). */
+/* The total amount of data read from swap space (in kB). */
 VIR_DOMAIN_MEMORY_STAT_SWAP_OUT= 1,
+/* The total amount of memory written out to swap space (in kB). */
 
+VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT = 2,
+VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT = 3,
 /*
  * Page faults occur when a process makes a valid access to virtual memory
  * that is not available.  When servicing the page fault, if disk IO is
  * required, it is considered a major fault.  If not, it is a minor fault.
  * These are expressed as the number of faults that have occurred.
  */
-VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT = 2,
-VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT = 3,
 
+VIR_DOMAIN_MEMORY_STAT_UNUSED  = 4,
 /*
  * The amount of memory left completely unused by the system.  Memory that
  * is available but used for reclaimable caches should NOT be reported as
  * free.  This value is expressed in kB.
  */
-VIR_DOMAIN_MEMORY_STAT_UNUSED  = 4,
 
+VIR_DOMAIN_MEMORY_STAT_AVAILABLE   = 5,
 /*
  * The total amount of usable memory as seen by the domain.  This value
  * may be less than the amount of memory assigned to the domain if a
  * balloon driver is in use or if the guest OS does not initialize all
  * assigned pages.  This value is expressed in kB.
  */
-VIR_DOMAIN_MEMORY_STAT_AVAILABLE   = 5,
 
-/* Current balloon value (in KB). */
 VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON  = 6,
+/* Current balloon value (in KB). */
 
+VIR_DOMAIN_MEMORY_STAT_RSS = 7,
 /* Resident Set Size of the process running the domain. This value
  * is in kB */
-VIR_DOMAIN_MEMORY_STAT_RSS = 7,
 
+VIR_DOMAIN_MEMORY_STAT_USABLE  = 8,
 /*
  * How much the balloon can be inflated without pushing the guest system
  * to swap, corresponds to 'Available' in /proc/meminfo
  */
-VIR_DOMAIN_MEMORY_STAT_USABLE  = 8,
 
-/* Timestamp of the last update of statistics, in seconds. */
 VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE = 9,
+/* Timestamp of the last update of statistics, in seconds. */
 
+VIR_DOMAIN_MEMORY_STAT_NR  = 10,
 /*
  * The number of statistics supported by this version of the interface.
  * To add new statistics, add them to the enum and increase this value.
  */
-VIR_DOMAIN_MEMORY_STAT_NR  = 10,
 
 # ifdef VIR_ENUM_SENTINELS
 VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR
@@ -683,22 +683,23 @@ typedef enum {
 
 /* Domain migration flags. */
 typedef enum {
+VIR_MIGRATE_LIVE  = (1 << 0),
 /* Do not pause the domain during migration. The domain's memory will
  * be transferred to the destination host while the domain is running.
  * The migration may never converge if the domain is changing its memory
  * faster then it can be transferred. The domain can be manually paused
  * anytime during migration using virDomainSuspend.
  */
-VIR_MIGRATE_LIVE  = (1 << 0),
 
+VIR_MIGRATE_PEER2PEER = (1 << 1),
 /* Tell the source libvirtd to connect directly to the destination host.
  * Without this flag the client (e.g., virsh) connects to both hosts and
  * controls the migration process. In peer-to-peer mode, the source
  * libvirtd controls the migration by calling the destination daemon
  * directly.
  */
-VIR_MIGRATE_PEER2PEER = (1 << 1),
 
+VIR_MIGRATE_TUNNELLED = (1 << 2),
 /* Tunnel migration data over libvirtd connection. Without this flag the
  * source hypervisor sends migration data directly to the destination
  * hypervisor. This flag can only be used when VIR_MIGRATE_PEER2PEER is
@@ -707,26 +708,26 @@ typedef enum {
  * Note the less-common spelling that we're stuck with:
  * VIR_MIGRATE_TUNNELLED should be VIR_MIGRATE_TUNNELED.
  */
-VIR_MIGRATE_TUNNELLED = (1 << 2),
 
+VIR_MIGRATE_PERSIST_DEST  = (1 << 3),
 /* Define the domain as persistent on the destination host after suc