[Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation

Stefan Hajnoczi posted 3 patches 7 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170301115026.22621-1-stefanha@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
tests/test-throttle.c |  8 ++++----
util/throttle.c       | 14 ++++++++++++++
qemu-options.hx       | 24 ++++++++++++++++++++++++
3 files changed, 42 insertions(+), 4 deletions(-)
[Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation
Posted by Stefan Hajnoczi 7 years, 2 months ago
v3:
 * Added Patch 2 to fix invalid test parameters
 * Switched to nicer max < avg check [Berto]
v2:
 * Fixed s/bps/iops/ copy-paste error in Patch 1 [Berto]
 * Rephrased warning about guest hangs and errors [Berto]
 * Added Patch 2 to hide the internal .max value from the monitor

Patch 1 fleshes out the documentation for I/O throttling command-line
parameters.

Patch 2 hides an internal value that was being exposed to users via the
monitor and caused confusion.

I ended up not adding QMP-style throttling.* names to the command-line
documentation because the names are very long and unlikely to be used.  I
couldn't see a nice way of adding them while still keeping the documentation
readable.

Stefan Hajnoczi (3):
  qemu-options: explain disk I/O throttling options
  throttle: do not use invalid config in test
  throttle: make throttle_config(throttle_get_config()) symmetric

 tests/test-throttle.c |  8 ++++----
 util/throttle.c       | 14 ++++++++++++++
 qemu-options.hx       | 24 ++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 4 deletions(-)

-- 
2.9.3


Re: [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation
Posted by Greg Kurz 7 years, 2 months ago
On Wed,  1 Mar 2017 11:50:23 +0000
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> v3:
>  * Added Patch 2 to fix invalid test parameters
>  * Switched to nicer max < avg check [Berto]
> v2:
>  * Fixed s/bps/iops/ copy-paste error in Patch 1 [Berto]
>  * Rephrased warning about guest hangs and errors [Berto]
>  * Added Patch 2 to hide the internal .max value from the monitor
> 
> Patch 1 fleshes out the documentation for I/O throttling command-line
> parameters.
> 
> Patch 2 hides an internal value that was being exposed to users via the
> monitor and caused confusion.
> 
> I ended up not adding QMP-style throttling.* names to the command-line
> documentation because the names are very long and unlikely to be used.  I
> couldn't see a nice way of adding them while still keeping the documentation
> readable.
> 

I only see this series now and it's a bit unfortunate... throttling options
are now also available for fsdev. Only QMP-style names were added though,
and they appear in the documentation. I agree the names are long, and the
result isn't pretty on 80 columns, but it is readable still.

I don't really want to add code, just to have shorter names and a prettier
output. But it would be a good thing for fsdev to benefit from this new
documentation... any suggestion how to do that ?

> Stefan Hajnoczi (3):
>   qemu-options: explain disk I/O throttling options
>   throttle: do not use invalid config in test
>   throttle: make throttle_config(throttle_get_config()) symmetric
> 
>  tests/test-throttle.c |  8 ++++----
>  util/throttle.c       | 14 ++++++++++++++
>  qemu-options.hx       | 24 ++++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 4 deletions(-)
> 

Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] throttle: improve command-line parameter documentation
Posted by Stefan Hajnoczi 7 years, 2 months ago
On Wed, Mar 01, 2017 at 10:27:48PM +0100, Greg Kurz wrote:
> On Wed,  1 Mar 2017 11:50:23 +0000
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> > v3:
> >  * Added Patch 2 to fix invalid test parameters
> >  * Switched to nicer max < avg check [Berto]
> > v2:
> >  * Fixed s/bps/iops/ copy-paste error in Patch 1 [Berto]
> >  * Rephrased warning about guest hangs and errors [Berto]
> >  * Added Patch 2 to hide the internal .max value from the monitor
> > 
> > Patch 1 fleshes out the documentation for I/O throttling command-line
> > parameters.
> > 
> > Patch 2 hides an internal value that was being exposed to users via the
> > monitor and caused confusion.
> > 
> > I ended up not adding QMP-style throttling.* names to the command-line
> > documentation because the names are very long and unlikely to be used.  I
> > couldn't see a nice way of adding them while still keeping the documentation
> > readable.
> > 
> 
> I only see this series now and it's a bit unfortunate... throttling options
> are now also available for fsdev. Only QMP-style names were added though,
> and they appear in the documentation. I agree the names are long, and the
> result isn't pretty on 80 columns, but it is readable still.
> 
> I don't really want to add code, just to have shorter names and a prettier
> output. But it would be a good thing for fsdev to benefit from this new
> documentation... any suggestion how to do that ?

I think another patch series would be good to:
1. Document QMP-style names for -drive
2. Reference throttling parameter documention in a common place for both
   -fsdev and -drive.

I'll do that.  This is QEMU 2.10 material anyway (it has missed the
freeze deadline), so there's no time pressure.

Stefan
Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] throttle: improve command-line parameter documentation
Posted by Greg Kurz 7 years, 2 months ago
On Fri, 3 Mar 2017 10:11:09 +0800
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Wed, Mar 01, 2017 at 10:27:48PM +0100, Greg Kurz wrote:
> > On Wed,  1 Mar 2017 11:50:23 +0000
> > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >   
> > > v3:
> > >  * Added Patch 2 to fix invalid test parameters
> > >  * Switched to nicer max < avg check [Berto]
> > > v2:
> > >  * Fixed s/bps/iops/ copy-paste error in Patch 1 [Berto]
> > >  * Rephrased warning about guest hangs and errors [Berto]
> > >  * Added Patch 2 to hide the internal .max value from the monitor
> > > 
> > > Patch 1 fleshes out the documentation for I/O throttling command-line
> > > parameters.
> > > 
> > > Patch 2 hides an internal value that was being exposed to users via the
> > > monitor and caused confusion.
> > > 
> > > I ended up not adding QMP-style throttling.* names to the command-line
> > > documentation because the names are very long and unlikely to be used.  I
> > > couldn't see a nice way of adding them while still keeping the documentation
> > > readable.
> > >   
> > 
> > I only see this series now and it's a bit unfortunate... throttling options
> > are now also available for fsdev. Only QMP-style names were added though,
> > and they appear in the documentation. I agree the names are long, and the
> > result isn't pretty on 80 columns, but it is readable still.
> > 
> > I don't really want to add code, just to have shorter names and a prettier
> > output. But it would be a good thing for fsdev to benefit from this new
> > documentation... any suggestion how to do that ?  
> 
> I think another patch series would be good to:
> 1. Document QMP-style names for -drive
> 2. Reference throttling parameter documention in a common place for both
>    -fsdev and -drive.
> 
> I'll do that.  This is QEMU 2.10 material anyway (it has missed the
> freeze deadline), so there's no time pressure.
> 
> Stefan

Sure. Please Cc me when you do that.

Thanks.

--
Greg
Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] throttle: improve command-line parameter documentation
Posted by Stefan Hajnoczi 7 years, 2 months ago
On Wed, Mar 01, 2017 at 11:50:23AM +0000, Stefan Hajnoczi wrote:
> v3:
>  * Added Patch 2 to fix invalid test parameters
>  * Switched to nicer max < avg check [Berto]
> v2:
>  * Fixed s/bps/iops/ copy-paste error in Patch 1 [Berto]
>  * Rephrased warning about guest hangs and errors [Berto]
>  * Added Patch 2 to hide the internal .max value from the monitor
> 
> Patch 1 fleshes out the documentation for I/O throttling command-line
> parameters.
> 
> Patch 2 hides an internal value that was being exposed to users via the
> monitor and caused confusion.
> 
> I ended up not adding QMP-style throttling.* names to the command-line
> documentation because the names are very long and unlikely to be used.  I
> couldn't see a nice way of adding them while still keeping the documentation
> readable.
> 
> Stefan Hajnoczi (3):
>   qemu-options: explain disk I/O throttling options
>   throttle: do not use invalid config in test
>   throttle: make throttle_config(throttle_get_config()) symmetric
> 
>  tests/test-throttle.c |  8 ++++----
>  util/throttle.c       | 14 ++++++++++++++
>  qemu-options.hx       | 24 ++++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 4 deletions(-)
> 
> -- 
> 2.9.3
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan