* [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation
@ 2017-03-01 11:50 Stefan Hajnoczi
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 1/3] qemu-options: explain disk I/O throttling options Stefan Hajnoczi
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-03-01 11:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Nini Gu, qemu-block, Alberto Garcia, Stefan Hajnoczi
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 1/3] qemu-options: explain disk I/O throttling options
2017-03-01 11:50 [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Stefan Hajnoczi
@ 2017-03-01 11:50 ` Stefan Hajnoczi
2017-03-01 13:07 ` Alberto Garcia
2017-03-01 21:29 ` Greg Kurz
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 2/3] throttle: do not use invalid config in test Stefan Hajnoczi
` (3 subsequent siblings)
4 siblings, 2 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-03-01 11:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Nini Gu, qemu-block, Alberto Garcia, Stefan Hajnoczi
The disk I/O throttling options have been listed for a long time but
never explained on the QEMU man page.
Suggested-by: Nini Gu <ngu@redhat.com>
Cc: Alberto Garcia <berto@igalia.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
qemu-options.hx | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/qemu-options.hx b/qemu-options.hx
index c85f77d..bbd1bd9 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -607,6 +607,30 @@ file sectors into the image file.
conversion of plain zero writes by the OS to driver specific optimized
zero write commands. You may even choose "unmap" if @var{discard} is set
to "unmap" to allow a zero write to be converted to an UNMAP operation.
+@item bps=@var{b},bps_rd=@var{r},bps_wr=@var{w}
+Specify bandwidth throttling limits in bytes per second, either for all request
+types or for reads or writes only. Small values can lead to timeouts or hangs
+inside the guest. A safe minimum for disks is 2 MB/s.
+@item bps_max=@var{bm},bps_rd_max=@var{rm},bps_wr_max=@var{wm}
+Specify bursts in bytes per second, either for all request types or for reads
+or writes only. Bursts allow the guest I/O to spike above the limit
+temporarily.
+@item iops=@var{i},iops_rd=@var{r},iops_wr=@var{w}
+Specify request rate limits in requests per second, either for all request
+types or for reads or writes only.
+@item iops_max=@var{bm},iops_rd_max=@var{rm},iops_wr_max=@var{wm}
+Specify bursts in requests per second, either for all request types or for reads
+or writes only. Bursts allow the guest I/O to spike above the limit
+temporarily.
+@item iops_size=@var{is}
+Let every @var{is} bytes of a request count as a new request for iops
+throttling purposes. Use this option to prevent guests from circumventing iops
+limits by sending fewer but larger requests.
+@item group=@var{g}
+Join a throttling quota group with given name @var{g}. All drives that are
+members of the same group are accounted for together. Use this option to
+prevent guests from circumventing throttling limits by using many small disks
+instead of a single larger disk.
@end table
By default, the @option{cache=writeback} mode is used. It will report data
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 2/3] throttle: do not use invalid config in test
2017-03-01 11:50 [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Stefan Hajnoczi
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 1/3] qemu-options: explain disk I/O throttling options Stefan Hajnoczi
@ 2017-03-01 11:50 ` Stefan Hajnoczi
2017-03-01 12:29 ` Alberto Garcia
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 3/3] throttle: make throttle_config(throttle_get_config()) symmetric Stefan Hajnoczi
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-03-01 11:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Nini Gu, qemu-block, Alberto Garcia, Stefan Hajnoczi
The (burst) max parameter cannot be smaller than the avg parameter.
There is a test case that uses avg = 56, max = 1 and gets away with it
because no input validation is performed by the test case.
This patch switches to valid test input parameters.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/test-throttle.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 363b59a..8531809 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -205,8 +205,8 @@ static void test_config_functions(void)
orig_cfg.buckets[THROTTLE_OPS_READ].avg = 69;
orig_cfg.buckets[THROTTLE_OPS_WRITE].avg = 23;
- orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0; /* should be corrected */
- orig_cfg.buckets[THROTTLE_BPS_READ].max = 1; /* should not be corrected */
+ orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0; /* should be corrected */
+ orig_cfg.buckets[THROTTLE_BPS_READ].max = 56; /* should not be corrected */
orig_cfg.buckets[THROTTLE_BPS_WRITE].max = 120;
orig_cfg.buckets[THROTTLE_OPS_TOTAL].max = 150;
@@ -246,8 +246,8 @@ static void test_config_functions(void)
g_assert(final_cfg.buckets[THROTTLE_OPS_READ].avg == 69);
g_assert(final_cfg.buckets[THROTTLE_OPS_WRITE].avg == 23);
- g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 15.3);/* fixed */
- g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max == 1); /* not fixed */
+ g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 15.3); /* fixed */
+ g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max == 56); /* not fixed */
g_assert(final_cfg.buckets[THROTTLE_BPS_WRITE].max == 120);
g_assert(final_cfg.buckets[THROTTLE_OPS_TOTAL].max == 150);
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 3/3] throttle: make throttle_config(throttle_get_config()) symmetric
2017-03-01 11:50 [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Stefan Hajnoczi
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 1/3] qemu-options: explain disk I/O throttling options Stefan Hajnoczi
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 2/3] throttle: do not use invalid config in test Stefan Hajnoczi
@ 2017-03-01 11:50 ` Stefan Hajnoczi
2017-03-01 12:30 ` Alberto Garcia
2017-03-01 21:27 ` [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Greg Kurz
2017-03-03 2:21 ` Stefan Hajnoczi
4 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-03-01 11:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Nini Gu, qemu-block, Alberto Garcia, Stefan Hajnoczi
Throttling has a weird property that throttle_get_config() does not
always return the same throttling settings that were given with
throttle_config(). In other words, the set and get functions aren't
symmetric.
If .max is 0 then the throttling code assigns a default value of .avg /
10 in throttle_config(). This is an implementation detail of the
throttling algorithm. When throttle_get_config() is called the .max
value returned should still be 0.
Users are exposed to this quirk via "info block" or "query-block"
monitor commands. This has caused confusion because it looks like a bug
when an unexpected value is reported.
This patch hides the .max value adjustment in throttle_get_config() and
updates test-throttle.c appropriately.
Reported-by: Nini Gu <ngu@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/test-throttle.c | 8 ++++----
util/throttle.c | 14 ++++++++++++++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 8531809..2a8bf64 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -205,8 +205,8 @@ static void test_config_functions(void)
orig_cfg.buckets[THROTTLE_OPS_READ].avg = 69;
orig_cfg.buckets[THROTTLE_OPS_WRITE].avg = 23;
- orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0; /* should be corrected */
- orig_cfg.buckets[THROTTLE_BPS_READ].max = 56; /* should not be corrected */
+ orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0;
+ orig_cfg.buckets[THROTTLE_BPS_READ].max = 56;
orig_cfg.buckets[THROTTLE_BPS_WRITE].max = 120;
orig_cfg.buckets[THROTTLE_OPS_TOTAL].max = 150;
@@ -246,8 +246,8 @@ static void test_config_functions(void)
g_assert(final_cfg.buckets[THROTTLE_OPS_READ].avg == 69);
g_assert(final_cfg.buckets[THROTTLE_OPS_WRITE].avg == 23);
- g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 15.3); /* fixed */
- g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max == 56); /* not fixed */
+ g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 0);
+ g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max == 56);
g_assert(final_cfg.buckets[THROTTLE_BPS_WRITE].max == 120);
g_assert(final_cfg.buckets[THROTTLE_OPS_TOTAL].max == 150);
diff --git a/util/throttle.c b/util/throttle.c
index 3817d9b..3570ed2 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -380,6 +380,14 @@ static void throttle_fix_bucket(LeakyBucket *bkt)
}
}
+/* undo internal bucket parameter changes (see throttle_fix_bucket()) */
+static void throttle_unfix_bucket(LeakyBucket *bkt)
+{
+ if (bkt->max < bkt->avg) {
+ bkt->max = 0;
+ }
+}
+
/* take care of canceling a timer */
static void throttle_cancel_timer(QEMUTimer *timer)
{
@@ -420,7 +428,13 @@ void throttle_config(ThrottleState *ts,
*/
void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg)
{
+ int i;
+
*cfg = ts->cfg;
+
+ for (i = 0; i < BUCKETS_COUNT; i++) {
+ throttle_unfix_bucket(&cfg->buckets[i]);
+ }
}
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/3] throttle: do not use invalid config in test
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 2/3] throttle: do not use invalid config in test Stefan Hajnoczi
@ 2017-03-01 12:29 ` Alberto Garcia
0 siblings, 0 replies; 12+ messages in thread
From: Alberto Garcia @ 2017-03-01 12:29 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel; +Cc: Nini Gu, qemu-block
On Wed 01 Mar 2017 12:50:25 PM CET, Stefan Hajnoczi wrote:
> The (burst) max parameter cannot be smaller than the avg parameter.
> There is a test case that uses avg = 56, max = 1 and gets away with it
> because no input validation is performed by the test case.
>
> This patch switches to valid test input parameters.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] throttle: make throttle_config(throttle_get_config()) symmetric
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 3/3] throttle: make throttle_config(throttle_get_config()) symmetric Stefan Hajnoczi
@ 2017-03-01 12:30 ` Alberto Garcia
0 siblings, 0 replies; 12+ messages in thread
From: Alberto Garcia @ 2017-03-01 12:30 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel; +Cc: Nini Gu, qemu-block
On Wed 01 Mar 2017 12:50:26 PM CET, Stefan Hajnoczi wrote:
> Throttling has a weird property that throttle_get_config() does not
> always return the same throttling settings that were given with
> throttle_config(). In other words, the set and get functions aren't
> symmetric.
>
> If .max is 0 then the throttling code assigns a default value of .avg /
> 10 in throttle_config(). This is an implementation detail of the
> throttling algorithm. When throttle_get_config() is called the .max
> value returned should still be 0.
>
> Users are exposed to this quirk via "info block" or "query-block"
> monitor commands. This has caused confusion because it looks like a bug
> when an unexpected value is reported.
>
> This patch hides the .max value adjustment in throttle_get_config() and
> updates test-throttle.c appropriately.
>
> Reported-by: Nini Gu <ngu@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/3] qemu-options: explain disk I/O throttling options
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 1/3] qemu-options: explain disk I/O throttling options Stefan Hajnoczi
@ 2017-03-01 13:07 ` Alberto Garcia
2017-03-01 21:29 ` Greg Kurz
1 sibling, 0 replies; 12+ messages in thread
From: Alberto Garcia @ 2017-03-01 13:07 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel; +Cc: Nini Gu, qemu-block
On Wed 01 Mar 2017 12:50:24 PM CET, Stefan Hajnoczi wrote:
> The disk I/O throttling options have been listed for a long time but
> never explained on the QEMU man page.
>
> Suggested-by: Nini Gu <ngu@redhat.com>
> Cc: Alberto Garcia <berto@igalia.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation
2017-03-01 11:50 [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Stefan Hajnoczi
` (2 preceding siblings ...)
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 3/3] throttle: make throttle_config(throttle_get_config()) symmetric Stefan Hajnoczi
@ 2017-03-01 21:27 ` Greg Kurz
2017-03-03 2:11 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-03 2:21 ` Stefan Hajnoczi
4 siblings, 1 reply; 12+ messages in thread
From: Greg Kurz @ 2017-03-01 21:27 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Nini Gu, Alberto Garcia, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]
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(-)
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/3] qemu-options: explain disk I/O throttling options
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 1/3] qemu-options: explain disk I/O throttling options Stefan Hajnoczi
2017-03-01 13:07 ` Alberto Garcia
@ 2017-03-01 21:29 ` Greg Kurz
1 sibling, 0 replies; 12+ messages in thread
From: Greg Kurz @ 2017-03-01 21:29 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Nini Gu, Alberto Garcia, qemu-block
[-- Attachment #1: Type: text/plain, Size: 2475 bytes --]
On Wed, 1 Mar 2017 11:50:24 +0000
Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The disk I/O throttling options have been listed for a long time but
> never explained on the QEMU man page.
>
> Suggested-by: Nini Gu <ngu@redhat.com>
> Cc: Alberto Garcia <berto@igalia.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> qemu-options.hx | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index c85f77d..bbd1bd9 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -607,6 +607,30 @@ file sectors into the image file.
> conversion of plain zero writes by the OS to driver specific optimized
> zero write commands. You may even choose "unmap" if @var{discard} is set
> to "unmap" to allow a zero write to be converted to an UNMAP operation.
> +@item bps=@var{b},bps_rd=@var{r},bps_wr=@var{w}
> +Specify bandwidth throttling limits in bytes per second, either for all request
> +types or for reads or writes only. Small values can lead to timeouts or hangs
> +inside the guest. A safe minimum for disks is 2 MB/s.
> +@item bps_max=@var{bm},bps_rd_max=@var{rm},bps_wr_max=@var{wm}
> +Specify bursts in bytes per second, either for all request types or for reads
> +or writes only. Bursts allow the guest I/O to spike above the limit
> +temporarily.
> +@item iops=@var{i},iops_rd=@var{r},iops_wr=@var{w}
> +Specify request rate limits in requests per second, either for all request
> +types or for reads or writes only.
> +@item iops_max=@var{bm},iops_rd_max=@var{rm},iops_wr_max=@var{wm}
> +Specify bursts in requests per second, either for all request types or for reads
> +or writes only. Bursts allow the guest I/O to spike above the limit
> +temporarily.
> +@item iops_size=@var{is}
> +Let every @var{is} bytes of a request count as a new request for iops
> +throttling purposes. Use this option to prevent guests from circumventing iops
> +limits by sending fewer but larger requests.
> +@item group=@var{g}
> +Join a throttling quota group with given name @var{g}. All drives that are
> +members of the same group are accounted for together. Use this option to
> +prevent guests from circumventing throttling limits by using many small disks
> +instead of a single larger disk.
> @end table
>
> By default, the @option{cache=writeback} mode is used. It will report data
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] throttle: improve command-line parameter documentation
2017-03-01 21:27 ` [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Greg Kurz
@ 2017-03-03 2:11 ` Stefan Hajnoczi
2017-03-03 7:01 ` Greg Kurz
0 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-03-03 2:11 UTC (permalink / raw)
To: Greg Kurz; +Cc: Stefan Hajnoczi, Nini Gu, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1773 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] throttle: improve command-line parameter documentation
2017-03-01 11:50 [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Stefan Hajnoczi
` (3 preceding siblings ...)
2017-03-01 21:27 ` [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Greg Kurz
@ 2017-03-03 2:21 ` Stefan Hajnoczi
4 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-03-03 2:21 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Nini Gu, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] throttle: improve command-line parameter documentation
2017-03-03 2:11 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2017-03-03 7:01 ` Greg Kurz
0 siblings, 0 replies; 12+ messages in thread
From: Greg Kurz @ 2017-03-03 7:01 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Stefan Hajnoczi, Nini Gu, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-03-03 7:01 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-01 11:50 [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Stefan Hajnoczi
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 1/3] qemu-options: explain disk I/O throttling options Stefan Hajnoczi
2017-03-01 13:07 ` Alberto Garcia
2017-03-01 21:29 ` Greg Kurz
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 2/3] throttle: do not use invalid config in test Stefan Hajnoczi
2017-03-01 12:29 ` Alberto Garcia
2017-03-01 11:50 ` [Qemu-devel] [PATCH v3 3/3] throttle: make throttle_config(throttle_get_config()) symmetric Stefan Hajnoczi
2017-03-01 12:30 ` Alberto Garcia
2017-03-01 21:27 ` [Qemu-devel] [PATCH v3 0/3] throttle: improve command-line parameter documentation Greg Kurz
2017-03-03 2:11 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-03 7:01 ` Greg Kurz
2017-03-03 2:21 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.