* [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
@ 2017-01-23 15:40 Pradeep Jagadeesh
0 siblings, 0 replies; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 15:40 UTC (permalink / raw)
To: Aneesh Kumar K.V, Greg Kurz; +Cc: Pradeep Jagadeesh, Alberto Garcia, qemu-devel
This patch set adds the IO throttling functionality to fsdev/9p devices.
So far cgroups were used for throttling IO opertions on the fsdev/9p devices.
It is difficult to use cgroups for throttling because we have to set up
cgroups externally before we start the qemu process.
Qemu provides the throttling apis for implementing the throttling.
Block devices already make use of these APIs for throtting the IO operations.
So, we use the same APIs to enable the throttling functionality for
fsdevices.As of now the feature is enabled only on 9p-local driver.
This feature can be used as shown in the below example:
-fsdev local,id=sdb1,path=PATH_TO_DEVICE,security_model=none,writeout=immediate,
throttling.bps-read=4194304,throttling.bps-write=4194304 -device
virtio-9p-pci,fsdev=sdb1,mount_tag=sdb1
The main advantages are:
- Easy to use because the throttling options are part of qemu cli options
- Provides a uniform way of using throttling options across block and
fsdev/9p devices
- No need to setup cgroup to provide throttling functionality for the fsdev
devices.
- Removes the redundant throttling code that was present in block and fsdev
files
Missing features:
-QMP support
-Throttling support for other fsdev/9p drivers.
Thanks,
Pradeep
Pradeep Jagadeesh (2):
fsdev: add IO throttle support to fsdev devices
throttle: removed duplicate throtlle code from block and fsdev files
blockdev.c | 81 ++-------------------------
fsdev/Makefile.objs | 2 +-
fsdev/file-op-9p.h | 3 +
fsdev/qemu-fsdev-opts.c | 3 +
fsdev/qemu-fsdev-throttle.c | 118 ++++++++++++++++++++++++++++++++++++++++
fsdev/qemu-fsdev-throttle.h | 39 +++++++++++++
hw/9pfs/9p-local.c | 8 +++
hw/9pfs/9p.c | 5 ++
hw/9pfs/cofile.c | 2 +
include/qemu/throttle-options.h | 92 +++++++++++++++++++++++++++++++
10 files changed, 275 insertions(+), 78 deletions(-)
create mode 100644 fsdev/qemu-fsdev-throttle.c
create mode 100644 fsdev/qemu-fsdev-throttle.h
create mode 100644 include/qemu/throttle-options.h
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
@ 2017-01-23 15:50 Pradeep Jagadeesh
2017-01-23 16:03 ` no-reply
0 siblings, 1 reply; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 15:50 UTC (permalink / raw)
To: Aneesh Kumar K.V, Greg Kurz; +Cc: Pradeep Jagadeesh, Alberto Garcia, qemu-devel
This patch set adds the IO throttling functionality to fsdev/9p devices.
So far cgroups were used for throttling IO opertions on the fsdev/9p devices.
It is difficult to use cgroups for throttling because we have to set up
cgroups externally before we start the qemu process.
Qemu provides the throttling apis for implementing the throttling.
Block devices already make use of these APIs for throtting the IO operations.
So, we use the same APIs to enable the throttling functionality for
fsdevices.As of now the feature is enabled only on 9p-local driver.
This feature can be used as shown in the below example:
-fsdev local,id=sdb1,path=PATH_TO_DEVICE,security_model=none,writeout=immediate,
throttling.bps-read=4194304,throttling.bps-write=4194304 -device
virtio-9p-pci,fsdev=sdb1,mount_tag=sdb1
The main advantages are:
- Easy to use because the throttling options are part of qemu cli options
- Provides a uniform way of using throttling options across block and
fsdev/9p devices
- No need to setup cgroup to provide throttling functionality for the fsdev
devices.
- Removes the redundant throttling code that was present in block and fsdev
files
Missing features:
-QMP support
-Throttling support for other fsdev/9p drivers.
Thanks,
Pradeep
Pradeep Jagadeesh (2):
fsdev: add IO throttle support to fsdev devices
throttle: removed duplicate throtlle code from block and fsdev files
blockdev.c | 81 ++-------------------------
fsdev/Makefile.objs | 2 +-
fsdev/file-op-9p.h | 3 +
fsdev/qemu-fsdev-opts.c | 3 +
fsdev/qemu-fsdev-throttle.c | 118 ++++++++++++++++++++++++++++++++++++++++
fsdev/qemu-fsdev-throttle.h | 39 +++++++++++++
hw/9pfs/9p-local.c | 8 +++
hw/9pfs/9p.c | 5 ++
hw/9pfs/cofile.c | 2 +
include/qemu/throttle-options.h | 92 +++++++++++++++++++++++++++++++
10 files changed, 275 insertions(+), 78 deletions(-)
create mode 100644 fsdev/qemu-fsdev-throttle.c
create mode 100644 fsdev/qemu-fsdev-throttle.h
create mode 100644 include/qemu/throttle-options.h
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 15:50 Pradeep Jagadeesh
@ 2017-01-23 16:03 ` no-reply
2017-01-23 16:22 ` Greg Kurz
0 siblings, 1 reply; 8+ messages in thread
From: no-reply @ 2017-01-23 16:03 UTC (permalink / raw)
To: pradeepkiruvale
Cc: famz, aneesh.kumar, groug, berto, pradeep.jagadeesh, qemu-devel
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
Message-id: 1485186641-12220-1-git-send-email-pradeep.jagadeesh@huawei.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
7686dc8 throttle: factor out duplicate code
7568614 fsdev: add IO throttle support to fsdev devices
=== OUTPUT BEGIN ===
Checking PATCH 1/2: fsdev: add IO throttle support to fsdev devices...
Checking PATCH 2/2: throttle: factor out duplicate code...
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#228: FILE: include/qemu/throttle-options.h:13:
+#define THROTTLE_OPTS \
+ { \
+ .name = "throttling.iops-total",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit total I/O operations per second",\
+ },{ \
+ .name = "throttling.iops-read",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit read operations per second",\
+ },{ \
+ .name = "throttling.iops-write",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit write operations per second",\
+ },{ \
+ .name = "throttling.bps-total",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit total bytes per second",\
+ },{ \
+ .name = "throttling.bps-read",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit read bytes per second",\
+ },{ \
+ .name = "throttling.bps-write",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit write bytes per second",\
+ },{ \
+ .name = "throttling.iops-total-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "I/O operations burst",\
+ },{ \
+ .name = "throttling.iops-read-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "I/O operations read burst",\
+ },{ \
+ .name = "throttling.iops-write-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "I/O operations write burst",\
+ },{ \
+ .name = "throttling.bps-total-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "total bytes burst",\
+ },{ \
+ .name = "throttling.bps-read-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "total bytes read burst",\
+ },{ \
+ .name = "throttling.bps-write-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "total bytes write burst",\
+ },{ \
+ .name = "throttling.iops-total-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the iops-total-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.iops-read-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the iops-read-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.iops-write-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the iops-write-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.bps-total-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the bps-total-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.bps-read-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the bps-read-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.bps-write-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the bps-write-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.iops-size",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "when limiting by iops max size of an I/O in bytes",\
+ }
ERROR: Missing Signed-off-by: line(s)
total: 2 errors, 0 warnings, 278 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:03 ` no-reply
@ 2017-01-23 16:22 ` Greg Kurz
2017-01-23 16:30 ` Pradeep Jagadeesh
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kurz @ 2017-01-23 16:22 UTC (permalink / raw)
To: pradeepkiruvale; +Cc: qemu-devel, famz, aneesh.kumar, berto, pradeep.jagadeesh
On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
no-reply@patchew.org wrote:
> Hi,
>
> Your series seems to have some coding style problems. See output below for
> more information:
>
Pradeep,
One should usually take patchew's findings into account. See below.
> Type: series
> Subject: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
> Message-id: 1485186641-12220-1-git-send-email-pradeep.jagadeesh@huawei.com
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
>
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
>
> # Useful git options
> git config --local diff.renamelimit 0
> git config --local diff.renames True
>
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
> failed=1
> echo
> fi
> n=$((n+1))
> done
>
> exit $failed
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> Switched to a new branch 'test'
> 7686dc8 throttle: factor out duplicate code
> 7568614 fsdev: add IO throttle support to fsdev devices
>
> === OUTPUT BEGIN ===
> Checking PATCH 1/2: fsdev: add IO throttle support to fsdev devices...
> Checking PATCH 2/2: throttle: factor out duplicate code...
> ERROR: Macros with multiple statements should be enclosed in a do - while loop
I guess your patch is ok here: checkpatch.pl is simply not smart enough to
parse this.
> #228: FILE: include/qemu/throttle-options.h:13:
> +#define THROTTLE_OPTS \
> + { \
> + .name = "throttling.iops-total",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "limit total I/O operations per second",\
> + },{ \
> + .name = "throttling.iops-read",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "limit read operations per second",\
> + },{ \
> + .name = "throttling.iops-write",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "limit write operations per second",\
> + },{ \
> + .name = "throttling.bps-total",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "limit total bytes per second",\
> + },{ \
> + .name = "throttling.bps-read",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "limit read bytes per second",\
> + },{ \
> + .name = "throttling.bps-write",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "limit write bytes per second",\
> + },{ \
> + .name = "throttling.iops-total-max",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "I/O operations burst",\
> + },{ \
> + .name = "throttling.iops-read-max",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "I/O operations read burst",\
> + },{ \
> + .name = "throttling.iops-write-max",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "I/O operations write burst",\
> + },{ \
> + .name = "throttling.bps-total-max",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "total bytes burst",\
> + },{ \
> + .name = "throttling.bps-read-max",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "total bytes read burst",\
> + },{ \
> + .name = "throttling.bps-write-max",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "total bytes write burst",\
> + },{ \
> + .name = "throttling.iops-total-max-length",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "length of the iops-total-max burst period, in seconds",\
> + },{ \
> + .name = "throttling.iops-read-max-length",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "length of the iops-read-max burst period, in seconds",\
> + },{ \
> + .name = "throttling.iops-write-max-length",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "length of the iops-write-max burst period, in seconds",\
> + },{ \
> + .name = "throttling.bps-total-max-length",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "length of the bps-total-max burst period, in seconds",\
> + },{ \
> + .name = "throttling.bps-read-max-length",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "length of the bps-read-max burst period, in seconds",\
> + },{ \
> + .name = "throttling.bps-write-max-length",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "length of the bps-write-max burst period, in seconds",\
> + },{ \
> + .name = "throttling.iops-size",\
> + .type = QEMU_OPT_NUMBER,\
> + .help = "when limiting by iops max size of an I/O in bytes",\
> + }
>
> ERROR: Missing Signed-off-by: line(s)
>
As Eric pointed out in another mail, all patches you send must have your
Signed-off-by line.
> total: 2 errors, 0 warnings, 278 lines checked
>
> Your patch has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> === OUTPUT END ===
>
> Test command exited with code: 1
>
>
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:22 ` Greg Kurz
@ 2017-01-23 16:30 ` Pradeep Jagadeesh
2017-01-23 16:34 ` Eric Blake
2017-01-23 16:42 ` Greg Kurz
0 siblings, 2 replies; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 16:30 UTC (permalink / raw)
To: Greg Kurz, pradeepkiruvale; +Cc: qemu-devel, famz, aneesh.kumar, berto
On 1/23/2017 5:22 PM, Greg Kurz wrote:
> On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
> no-reply@patchew.org wrote:
>> Hi,
>>
>> Your series seems to have some coding style problems. See output below for
>> more information:
>>
>
Ya, I observed signoff issue. I have created next version files.
I will send them tomorrow.
But, I do not know how to solve the second warning.
i.e multi line macro issue.
-Pradeep
> Pradeep,
>
> One should usually take patchew's findings into account. See below.
>
>> Type: series
>> Subject: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
>> Message-id: 1485186641-12220-1-git-send-email-pradeep.jagadeesh@huawei.com
>>
>> === TEST SCRIPT BEGIN ===
>> #!/bin/bash
>>
>> BASE=base
>> n=1
>> total=$(git log --oneline $BASE.. | wc -l)
>> failed=0
>>
>> # Useful git options
>> git config --local diff.renamelimit 0
>> git config --local diff.renames True
>>
>> commits="$(git log --format=%H --reverse $BASE..)"
>> for c in $commits; do
>> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
>> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
>> failed=1
>> echo
>> fi
>> n=$((n+1))
>> done
>>
>> exit $failed
>> === TEST SCRIPT END ===
>>
>> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
>> Switched to a new branch 'test'
>> 7686dc8 throttle: factor out duplicate code
>> 7568614 fsdev: add IO throttle support to fsdev devices
>>
>> === OUTPUT BEGIN ===
>> Checking PATCH 1/2: fsdev: add IO throttle support to fsdev devices...
>> Checking PATCH 2/2: throttle: factor out duplicate code...
>> ERROR: Macros with multiple statements should be enclosed in a do - while loop
>
> I guess your patch is ok here: checkpatch.pl is simply not smart enough to
> parse this.
>
>> #228: FILE: include/qemu/throttle-options.h:13:
>> +#define THROTTLE_OPTS \
>> + { \
>> + .name = "throttling.iops-total",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "limit total I/O operations per second",\
>> + },{ \
>> + .name = "throttling.iops-read",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "limit read operations per second",\
>> + },{ \
>> + .name = "throttling.iops-write",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "limit write operations per second",\
>> + },{ \
>> + .name = "throttling.bps-total",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "limit total bytes per second",\
>> + },{ \
>> + .name = "throttling.bps-read",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "limit read bytes per second",\
>> + },{ \
>> + .name = "throttling.bps-write",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "limit write bytes per second",\
>> + },{ \
>> + .name = "throttling.iops-total-max",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "I/O operations burst",\
>> + },{ \
>> + .name = "throttling.iops-read-max",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "I/O operations read burst",\
>> + },{ \
>> + .name = "throttling.iops-write-max",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "I/O operations write burst",\
>> + },{ \
>> + .name = "throttling.bps-total-max",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "total bytes burst",\
>> + },{ \
>> + .name = "throttling.bps-read-max",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "total bytes read burst",\
>> + },{ \
>> + .name = "throttling.bps-write-max",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "total bytes write burst",\
>> + },{ \
>> + .name = "throttling.iops-total-max-length",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "length of the iops-total-max burst period, in seconds",\
>> + },{ \
>> + .name = "throttling.iops-read-max-length",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "length of the iops-read-max burst period, in seconds",\
>> + },{ \
>> + .name = "throttling.iops-write-max-length",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "length of the iops-write-max burst period, in seconds",\
>> + },{ \
>> + .name = "throttling.bps-total-max-length",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "length of the bps-total-max burst period, in seconds",\
>> + },{ \
>> + .name = "throttling.bps-read-max-length",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "length of the bps-read-max burst period, in seconds",\
>> + },{ \
>> + .name = "throttling.bps-write-max-length",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "length of the bps-write-max burst period, in seconds",\
>> + },{ \
>> + .name = "throttling.iops-size",\
>> + .type = QEMU_OPT_NUMBER,\
>> + .help = "when limiting by iops max size of an I/O in bytes",\
>> + }
>>
>> ERROR: Missing Signed-off-by: line(s)
>>
>
> As Eric pointed out in another mail, all patches you send must have your
> Signed-off-by line.
>
>> total: 2 errors, 0 warnings, 278 lines checked
>>
>> Your patch has style problems, please review. If any of these errors
>> are false positives report them to the maintainer, see
>> CHECKPATCH in MAINTAINERS.
>>
>> === OUTPUT END ===
>>
>> Test command exited with code: 1
>>
>>
>> ---
>> Email generated automatically by Patchew [http://patchew.org/].
>> Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:30 ` Pradeep Jagadeesh
@ 2017-01-23 16:34 ` Eric Blake
2017-01-23 16:41 ` Pradeep Jagadeesh
2017-01-23 16:42 ` Greg Kurz
1 sibling, 1 reply; 8+ messages in thread
From: Eric Blake @ 2017-01-23 16:34 UTC (permalink / raw)
To: Pradeep Jagadeesh, Greg Kurz, pradeepkiruvale
Cc: berto, famz, qemu-devel, aneesh.kumar
[-- Attachment #1: Type: text/plain, Size: 926 bytes --]
On 01/23/2017 10:30 AM, Pradeep Jagadeesh wrote:
> On 1/23/2017 5:22 PM, Greg Kurz wrote:
>> On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
>> no-reply@patchew.org wrote:
>>> Hi,
>>>
>>> Your series seems to have some coding style problems. See output
>>> below for
>>> more information:
>>>
>>
> Ya, I observed signoff issue. I have created next version files.
> I will send them tomorrow.
>
> But, I do not know how to solve the second warning.
> i.e multi line macro issue.
I don't think there is anything to fix there; it is a false positive
from checkpatch.pl (unless you want to dive in to the perl script and
figure out how to relax it to allow that usage, as you are not the first
to have it). Documenting in the commit message that checkpatch has a
false-positive warning is acceptable.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:34 ` Eric Blake
@ 2017-01-23 16:41 ` Pradeep Jagadeesh
0 siblings, 0 replies; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 16:41 UTC (permalink / raw)
To: Eric Blake, Greg Kurz, pradeepkiruvale
Cc: berto, famz, qemu-devel, aneesh.kumar
On 1/23/2017 5:34 PM, Eric Blake wrote:
> On 01/23/2017 10:30 AM, Pradeep Jagadeesh wrote:
>> On 1/23/2017 5:22 PM, Greg Kurz wrote:
>>> On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
>>> no-reply@patchew.org wrote:
>>>> Hi,
>>>>
>>>> Your series seems to have some coding style problems. See output
>>>> below for
>>>> more information:
>>>>
>>>
>> Ya, I observed signoff issue. I have created next version files.
>> I will send them tomorrow.
>>
>> But, I do not know how to solve the second warning.
>> i.e multi line macro issue.
>
> I don't think there is anything to fix there; it is a false positive
> from checkpatch.pl (unless you want to dive in to the perl script and
> figure out how to relax it to allow that usage, as you are not the first
> to have it). Documenting in the commit message that checkpatch has a
> false-positive warning is acceptable.
>
OK
Thanks,
Pradeep
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:30 ` Pradeep Jagadeesh
2017-01-23 16:34 ` Eric Blake
@ 2017-01-23 16:42 ` Greg Kurz
1 sibling, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2017-01-23 16:42 UTC (permalink / raw)
To: Pradeep Jagadeesh; +Cc: pradeepkiruvale, qemu-devel, famz, aneesh.kumar, berto
On Mon, 23 Jan 2017 17:30:13 +0100
Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote:
> On 1/23/2017 5:22 PM, Greg Kurz wrote:
> > On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
> > no-reply@patchew.org wrote:
> >> Hi,
> >>
> >> Your series seems to have some coding style problems. See output below for
> >> more information:
> >>
> >
> Ya, I observed signoff issue. I have created next version files.
> I will send them tomorrow.
>
> But, I do not know how to solve the second warning.
> i.e multi line macro issue.
>
As I had written in my mail:
> > I guess your patch is ok here: checkpatch.pl is simply not smart enough to
> > parse this.
You can ignore this warning.
Also, you had a positive review for patch 2/2:
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04637.html
You can add this line below your S-o-b.
> -Pradeep
>
> > Pradeep,
> >
> > One should usually take patchew's findings into account. See below.
> >
> >> Type: series
> >> Subject: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
> >> Message-id: 1485186641-12220-1-git-send-email-pradeep.jagadeesh@huawei.com
> >>
> >> === TEST SCRIPT BEGIN ===
> >> #!/bin/bash
> >>
> >> BASE=base
> >> n=1
> >> total=$(git log --oneline $BASE.. | wc -l)
> >> failed=0
> >>
> >> # Useful git options
> >> git config --local diff.renamelimit 0
> >> git config --local diff.renames True
> >>
> >> commits="$(git log --format=%H --reverse $BASE..)"
> >> for c in $commits; do
> >> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> >> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
> >> failed=1
> >> echo
> >> fi
> >> n=$((n+1))
> >> done
> >>
> >> exit $failed
> >> === TEST SCRIPT END ===
> >>
> >> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> >> Switched to a new branch 'test'
> >> 7686dc8 throttle: factor out duplicate code
> >> 7568614 fsdev: add IO throttle support to fsdev devices
> >>
> >> === OUTPUT BEGIN ===
> >> Checking PATCH 1/2: fsdev: add IO throttle support to fsdev devices...
> >> Checking PATCH 2/2: throttle: factor out duplicate code...
> >> ERROR: Macros with multiple statements should be enclosed in a do - while loop
> >
> > I guess your patch is ok here: checkpatch.pl is simply not smart enough to
> > parse this.
> >
> >> #228: FILE: include/qemu/throttle-options.h:13:
> >> +#define THROTTLE_OPTS \
> >> + { \
> >> + .name = "throttling.iops-total",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "limit total I/O operations per second",\
> >> + },{ \
> >> + .name = "throttling.iops-read",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "limit read operations per second",\
> >> + },{ \
> >> + .name = "throttling.iops-write",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "limit write operations per second",\
> >> + },{ \
> >> + .name = "throttling.bps-total",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "limit total bytes per second",\
> >> + },{ \
> >> + .name = "throttling.bps-read",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "limit read bytes per second",\
> >> + },{ \
> >> + .name = "throttling.bps-write",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "limit write bytes per second",\
> >> + },{ \
> >> + .name = "throttling.iops-total-max",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "I/O operations burst",\
> >> + },{ \
> >> + .name = "throttling.iops-read-max",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "I/O operations read burst",\
> >> + },{ \
> >> + .name = "throttling.iops-write-max",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "I/O operations write burst",\
> >> + },{ \
> >> + .name = "throttling.bps-total-max",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "total bytes burst",\
> >> + },{ \
> >> + .name = "throttling.bps-read-max",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "total bytes read burst",\
> >> + },{ \
> >> + .name = "throttling.bps-write-max",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "total bytes write burst",\
> >> + },{ \
> >> + .name = "throttling.iops-total-max-length",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "length of the iops-total-max burst period, in seconds",\
> >> + },{ \
> >> + .name = "throttling.iops-read-max-length",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "length of the iops-read-max burst period, in seconds",\
> >> + },{ \
> >> + .name = "throttling.iops-write-max-length",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "length of the iops-write-max burst period, in seconds",\
> >> + },{ \
> >> + .name = "throttling.bps-total-max-length",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "length of the bps-total-max burst period, in seconds",\
> >> + },{ \
> >> + .name = "throttling.bps-read-max-length",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "length of the bps-read-max burst period, in seconds",\
> >> + },{ \
> >> + .name = "throttling.bps-write-max-length",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "length of the bps-write-max burst period, in seconds",\
> >> + },{ \
> >> + .name = "throttling.iops-size",\
> >> + .type = QEMU_OPT_NUMBER,\
> >> + .help = "when limiting by iops max size of an I/O in bytes",\
> >> + }
> >>
> >> ERROR: Missing Signed-off-by: line(s)
> >>
> >
> > As Eric pointed out in another mail, all patches you send must have your
> > Signed-off-by line.
> >
> >> total: 2 errors, 0 warnings, 278 lines checked
> >>
> >> Your patch has style problems, please review. If any of these errors
> >> are false positives report them to the maintainer, see
> >> CHECKPATCH in MAINTAINERS.
> >>
> >> === OUTPUT END ===
> >>
> >> Test command exited with code: 1
> >>
> >>
> >> ---
> >> Email generated automatically by Patchew [http://patchew.org/].
> >> Please send your feedback to patchew-devel@freelists.org
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-01-23 16:42 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-23 15:40 [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
-- strict thread matches above, loose matches on Subject: below --
2017-01-23 15:50 Pradeep Jagadeesh
2017-01-23 16:03 ` no-reply
2017-01-23 16:22 ` Greg Kurz
2017-01-23 16:30 ` Pradeep Jagadeesh
2017-01-23 16:34 ` Eric Blake
2017-01-23 16:41 ` Pradeep Jagadeesh
2017-01-23 16:42 ` Greg Kurz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).