Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
---
tests/qemu-iotests/184 | 205 +++++++++++++++++++++++++++++++
tests/qemu-iotests/184.out | 300 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 506 insertions(+)
create mode 100755 tests/qemu-iotests/184
create mode 100644 tests/qemu-iotests/184.out
diff --git a/tests/qemu-iotests/184 b/tests/qemu-iotests/184
new file mode 100755
index 0000000000..704f38f936
--- /dev/null
+++ b/tests/qemu-iotests/184
@@ -0,0 +1,205 @@
+#!/bin/bash
+#
+# Test I/O throttle block filter driver interface
+#
+# Copyright (C) 2017 Manos Pitsidianakis
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner="Manos Pitsidianakis"
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+function do_run_qemu()
+{
+ echo Testing: "$@" | _filter_imgfmt
+ $QEMU -nographic -qmp-pretty stdio -serial none "$@"
+ echo
+}
+
+function run_qemu()
+{
+ do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_qmp\
+ | _filter_qemu_io | _filter_generated_node_ids
+}
+
+_make_test_img 64M
+test_throttle=$($QEMU_IMG --help|grep throttle)
+[ "$test_throttle" = "" ] && _supported_fmt throttle
+
+echo
+echo "== checking interface =="
+
+run_qemu <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "blockdev-add",
+ "arguments": {
+ "driver": "$IMGFMT",
+ "node-name": "disk0",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
+ }
+ }
+}
+{ "execute": "object-add",
+ "arguments": {
+ "qom-type": "throttle-group",
+ "id": "group0",
+ "props": {
+ "limits" : {
+ "iops-total": 1000
+ }
+ }
+ }
+}
+{ "execute": "blockdev-add",
+ "arguments": {
+ "driver": "throttle",
+ "node-name": "throttle0",
+ "throttle-group": "group0",
+ "file": "disk0"
+ }
+}
+{ "execute": "query-named-block-nodes" }
+{ "execute": "query-block" }
+{ "execute": "quit" }
+EOF
+
+echo
+echo "== property changes in ThrottleGroup =="
+
+run_qemu <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "object-add",
+ "arguments": {
+ "qom-type": "throttle-group",
+ "id": "group0",
+ "props" : {
+ "limits": {
+ "iops-total": 1000
+ }
+ }
+ }
+}
+{ "execute" : "qom-get",
+ "arguments" : {
+ "path" : "group0",
+ "property" : "limits"
+ }
+}
+{ "execute" : "qom-set",
+ "arguments" : {
+ "path" : "group0",
+ "property" : "limits",
+ "value" : {
+ "iops-total" : 0
+ }
+ }
+}
+{ "execute" : "qom-get",
+ "arguments" : {
+ "path" : "group0",
+ "property" : "limits"
+ }
+}
+{ "execute": "quit" }
+EOF
+
+echo
+echo "== object creation/set errors =="
+
+run_qemu <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "object-add",
+ "arguments": {
+ "qom-type": "throttle-group",
+ "id": "group0",
+ "props" : {
+ "limits": {
+ "iops-total": 1000
+ }
+ }
+ }
+}
+{ "execute" : "qom-set",
+ "arguments" : {
+ "path" : "group0",
+ "property" : "x-iops-total",
+ "value" : 0
+ }
+}
+{ "execute" : "qom-set",
+ "arguments" : {
+ "path" : "group0",
+ "property" : "limits",
+ "value" : {
+ "iops-total" : 10,
+ "iops-read" : 10
+ }
+ }
+}
+{ "execute": "quit" }
+EOF
+
+echo
+echo "== don't specify group =="
+
+run_qemu <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "blockdev-add",
+ "arguments": {
+ "driver": "$IMGFMT",
+ "node-name": "disk0",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
+ }
+ }
+}
+{ "execute": "blockdev-add",
+ "arguments": {
+ "driver": "throttle",
+ "node-name": "throttle0",
+ "file": "disk0"
+ }
+}
+{ "execute": "quit" }
+EOF
+
+echo
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/184.out b/tests/qemu-iotests/184.out
new file mode 100644
index 0000000000..0e2889b3b6
--- /dev/null
+++ b/tests/qemu-iotests/184.out
@@ -0,0 +1,300 @@
+QA output created by 184
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+
+== checking interface ==
+Testing:
+{
+ QMP_VERSION
+}
+{
+ "return": {
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "return": [
+ {
+ "iops_rd": 0,
+ "detect_zeroes": "off",
+ "image": {
+ "virtual-size": 67108864,
+ "filename": "json:{\"throttle-group\": \"group0\", \"driver\": \"throttle\", \"file\": {\"driver\": \"qcow2\", \"file\": {\"driver\": \"file\", \"filename\": \"TEST_DIR/t.qcow2\"}}}",
+ "format": "throttle",
+ "actual-size": 200704
+ },
+ "iops_wr": 0,
+ "ro": false,
+ "node-name": "throttle0",
+ "backing_file_depth": 0,
+ "drv": "throttle",
+ "iops": 0,
+ "bps_wr": 0,
+ "write_threshold": 0,
+ "encrypted": false,
+ "bps": 0,
+ "bps_rd": 0,
+ "cache": {
+ "no-flush": false,
+ "direct": false,
+ "writeback": true
+ },
+ "file": "json:{\"throttle-group\": \"group0\", \"driver\": \"throttle\", \"file\": {\"driver\": \"qcow2\", \"file\": {\"driver\": \"file\", \"filename\": \"TEST_DIR/t.qcow2\"}}}",
+ "encryption_key_missing": false
+ },
+ {
+ "iops_rd": 0,
+ "detect_zeroes": "off",
+ "image": {
+ "virtual-size": 67108864,
+ "filename": "TEST_DIR/t.qcow2",
+ "cluster-size": 65536,
+ "format": "qcow2",
+ "actual-size": 200704,
+ "format-specific": {
+ "type": "qcow2",
+ "data": {
+ "compat": "1.1",
+ "lazy-refcounts": false,
+ "refcount-bits": 16,
+ "corrupt": false
+ }
+ },
+ "dirty-flag": false
+ },
+ "iops_wr": 0,
+ "ro": false,
+ "node-name": "disk0",
+ "backing_file_depth": 0,
+ "drv": "qcow2",
+ "iops": 0,
+ "bps_wr": 0,
+ "write_threshold": 0,
+ "encrypted": false,
+ "bps": 0,
+ "bps_rd": 0,
+ "cache": {
+ "no-flush": false,
+ "direct": false,
+ "writeback": true
+ },
+ "file": "TEST_DIR/t.qcow2",
+ "encryption_key_missing": false
+ },
+ {
+ "iops_rd": 0,
+ "detect_zeroes": "off",
+ "image": {
+ "virtual-size": 197120,
+ "filename": "TEST_DIR/t.qcow2",
+ "format": "file",
+ "actual-size": 200704,
+ "dirty-flag": false
+ },
+ "iops_wr": 0,
+ "ro": false,
+ "node-name": "NODE_NAME",
+ "backing_file_depth": 0,
+ "drv": "file",
+ "iops": 0,
+ "bps_wr": 0,
+ "write_threshold": 0,
+ "encrypted": false,
+ "bps": 0,
+ "bps_rd": 0,
+ "cache": {
+ "no-flush": false,
+ "direct": false,
+ "writeback": true
+ },
+ "file": "TEST_DIR/t.qcow2",
+ "encryption_key_missing": false
+ }
+ ]
+}
+{
+ "return": [
+ ]
+}
+{
+ "return": {
+ }
+}
+{
+ "timestamp": {
+ "seconds": TIMESTAMP,
+ "microseconds": TIMESTAMP
+ },
+ "event": "SHUTDOWN",
+ "data": {
+ "guest": false
+ }
+}
+
+
+== property changes in ThrottleGroup ==
+Testing:
+{
+ QMP_VERSION
+}
+{
+ "return": {
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "return": {
+ "bps-read-max-length": 1,
+ "iops-read-max-length": 1,
+ "bps-read-max": 0,
+ "bps-total": 0,
+ "iops-total-max-length": 1,
+ "iops-total": 1000,
+ "iops-write-max": 0,
+ "bps-write": 0,
+ "bps-total-max": 0,
+ "bps-write-max": 0,
+ "iops-size": 0,
+ "iops-read": 0,
+ "iops-write-max-length": 1,
+ "iops-write": 0,
+ "bps-total-max-length": 1,
+ "iops-read-max": 0,
+ "bps-read": 0,
+ "bps-write-max-length": 1,
+ "iops-total-max": 0
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "return": {
+ "bps-read-max-length": 1,
+ "iops-read-max-length": 1,
+ "bps-read-max": 0,
+ "bps-total": 0,
+ "iops-total-max-length": 1,
+ "iops-total": 0,
+ "iops-write-max": 0,
+ "bps-write": 0,
+ "bps-total-max": 0,
+ "bps-write-max": 0,
+ "iops-size": 0,
+ "iops-read": 0,
+ "iops-write-max-length": 1,
+ "iops-write": 0,
+ "bps-total-max-length": 1,
+ "iops-read-max": 0,
+ "bps-read": 0,
+ "bps-write-max-length": 1,
+ "iops-total-max": 0
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "timestamp": {
+ "seconds": TIMESTAMP,
+ "microseconds": TIMESTAMP
+ },
+ "event": "SHUTDOWN",
+ "data": {
+ "guest": false
+ }
+}
+
+
+== object creation/set errors ==
+Testing:
+{
+ QMP_VERSION
+}
+{
+ "return": {
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "error": {
+ "class": "GenericError",
+ "desc": "Property cannot be set after initialization"
+ }
+}
+{
+ "error": {
+ "class": "GenericError",
+ "desc": "bps/iops/max total values and read/write values cannot be used at the same time"
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "timestamp": {
+ "seconds": TIMESTAMP,
+ "microseconds": TIMESTAMP
+ },
+ "event": "SHUTDOWN",
+ "data": {
+ "guest": false
+ }
+}
+
+
+== don't specify group ==
+Testing:
+{
+ QMP_VERSION
+}
+{
+ "return": {
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "error": {
+ "class": "GenericError",
+ "desc": "Parameter 'throttle-group' is missing"
+ }
+}
+{
+ "return": {
+ }
+}
+{
+ "timestamp": {
+ "seconds": TIMESTAMP,
+ "microseconds": TIMESTAMP
+ },
+ "event": "SHUTDOWN",
+ "data": {
+ "guest": false
+ }
+}
+
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index afbdc427ea..fee98440a5 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -180,6 +180,7 @@
181 rw auto migration
182 rw auto quick
183 rw auto migration
+184 rw auto quick
185 rw auto
186 rw auto
187 rw auto
--
2.11.0
On Fri, Aug 25, 2017 at 04:20:28PM +0300, Manos Pitsidianakis wrote: > Reviewed-by: Alberto Garcia <berto@igalia.com> > Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> > --- > tests/qemu-iotests/184 | 205 +++++++++++++++++++++++++++++++ > tests/qemu-iotests/184.out | 300 +++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/group | 1 + > 3 files changed, 506 insertions(+) > create mode 100755 tests/qemu-iotests/184 > create mode 100644 tests/qemu-iotests/184.out Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Am 25.08.2017 um 15:20 hat Manos Pitsidianakis geschrieben:
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
Does this test actually (still) pass for you? I can't see that it's
related to any recent change in master, but this is the diff that I get.
I can update the reference output while applying, but obviously if it's
currently passing for you, it will fail after I "fix" it.
Kevin
184 [18:07:39] [18:07:39] - output mismatch (see 184.out.bad)
--- /home/kwolf/source/qemu/tests/qemu-iotests/184.out 2017-09-05 18:07:30.751340633 +0200
+++ 184.out.bad 2017-09-05 18:07:39.707341854 +0200
@@ -30,8 +30,10 @@
"image": {
"virtual-size": 67108864,
"filename": "json:{\"throttle-group\": \"group0\", \"driver\": \"throttle\", \"file\": {\"driver\": \"qcow2\", \"file\": {\"driver\": \"file\", \"filename\": \"TEST_DIR/t.qcow2\"}}}",
+ "cluster-size": 65536,
"format": "throttle",
- "actual-size": 200704
+ "actual-size": 200704,
+ "dirty-flag": false
},
"iops_wr": 0,
"ro": false,
Am 05.09.2017 um 18:16 hat Kevin Wolf geschrieben: > Am 25.08.2017 um 15:20 hat Manos Pitsidianakis geschrieben: > > Reviewed-by: Alberto Garcia <berto@igalia.com> > > Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> > > Does this test actually (still) pass for you? I can't see that it's > related to any recent change in master, but this is the diff that I get. > > I can update the reference output while applying, but obviously if it's > currently passing for you, it will fail after I "fix" it. For the record, we discussed this on IRC. The test works correctly on master, but on my block branch there is a conflict with "block: pass bdrv_* methods to bs->file by default in block filters". The correct action is to merge this throttle driver series after the conflicting patch because throttle doesn't implement .bdrv_get_info and needs the forwarding that the other patch implements. I updated the test output accordingly and applied the series to my block branch. Kevin
On 09/05/2017 02:06 PM, Kevin Wolf wrote:
> Am 05.09.2017 um 18:16 hat Kevin Wolf geschrieben:
>> Am 25.08.2017 um 15:20 hat Manos Pitsidianakis geschrieben:
>>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>>> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
>>
>> Does this test actually (still) pass for you? I can't see that it's
>> related to any recent change in master, but this is the diff that I get.
>>
>> I can update the reference output while applying, but obviously if it's
>> currently passing for you, it will fail after I "fix" it.
>
> For the record, we discussed this on IRC. The test works correctly on
> master, but on my block branch there is a conflict with "block: pass
> bdrv_* methods to bs->file by default in block filters".
>
> The correct action is to merge this throttle driver series after the
> conflicting patch because throttle doesn't implement .bdrv_get_info and
> needs the forwarding that the other patch implements.
>
> I updated the test output accordingly and applied the series to my block
> branch.
Could you also squash this in to 5/6? (as long as we're intentionally
basing throttle on top of defaults, then we should use the right default
instead of duplicating things)
diff --git i/block/throttle.c w/block/throttle.c
index 7b33613372..5bca76300f 100644
--- i/block/throttle.c
+++ w/block/throttle.c
@@ -197,19 +197,6 @@ static bool
throttle_recurse_is_first_non_filter(BlockDriverState *bs,
return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate);
}
-static int64_t coroutine_fn
throttle_co_get_block_status(BlockDriverState *bs,
- int64_t
sector_num,
- int nb_sectors,
- int *pnum,
-
BlockDriverState **file)
-{
- assert(bs->file && bs->file->bs);
- *pnum = nb_sectors;
- *file = bs->file->bs;
- return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
- (sector_num << BDRV_SECTOR_BITS);
-}
-
static BlockDriver bdrv_throttle = {
.format_name = "throttle",
.protocol_name = "throttle",
@@ -237,7 +224,7 @@ static BlockDriver bdrv_throttle = {
.bdrv_reopen_prepare = throttle_reopen_prepare,
.bdrv_reopen_commit = throttle_reopen_commit,
.bdrv_reopen_abort = throttle_reopen_abort,
- .bdrv_co_get_block_status = throttle_co_get_block_status,
+ .bdrv_co_get_block_status =
bdrv_co_get_block_status_from_file,
.is_filter = true,
};
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On Tue, Sep 05, 2017 at 04:13:39PM -0500, Eric Blake wrote:
>On 09/05/2017 02:06 PM, Kevin Wolf wrote:
>> Am 05.09.2017 um 18:16 hat Kevin Wolf geschrieben:
>>> Am 25.08.2017 um 15:20 hat Manos Pitsidianakis geschrieben:
>>>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>>>> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
>>>
>>> Does this test actually (still) pass for you? I can't see that it's
>>> related to any recent change in master, but this is the diff that I get.
>>>
>>> I can update the reference output while applying, but obviously if it's
>>> currently passing for you, it will fail after I "fix" it.
>>
>> For the record, we discussed this on IRC. The test works correctly on
>> master, but on my block branch there is a conflict with "block: pass
>> bdrv_* methods to bs->file by default in block filters".
>>
>> The correct action is to merge this throttle driver series after the
>> conflicting patch because throttle doesn't implement .bdrv_get_info and
>> needs the forwarding that the other patch implements.
>>
>> I updated the test output accordingly and applied the series to my block
>> branch.
>
>Could you also squash this in to 5/6? (as long as we're intentionally
>basing throttle on top of defaults, then we should use the right default
>instead of duplicating things)
Yes, this change makes sense, if it's no trouble with Kevin.
>diff --git i/block/throttle.c w/block/throttle.c
>index 7b33613372..5bca76300f 100644
>--- i/block/throttle.c
>+++ w/block/throttle.c
>@@ -197,19 +197,6 @@ static bool
>throttle_recurse_is_first_non_filter(BlockDriverState *bs,
> return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate);
> }
>
>-static int64_t coroutine_fn
>throttle_co_get_block_status(BlockDriverState *bs,
>- int64_t
>sector_num,
>- int nb_sectors,
>- int *pnum,
>-
>BlockDriverState **file)
>-{
>- assert(bs->file && bs->file->bs);
>- *pnum = nb_sectors;
>- *file = bs->file->bs;
>- return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
>- (sector_num << BDRV_SECTOR_BITS);
>-}
>-
> static BlockDriver bdrv_throttle = {
> .format_name = "throttle",
> .protocol_name = "throttle",
>@@ -237,7 +224,7 @@ static BlockDriver bdrv_throttle = {
> .bdrv_reopen_prepare = throttle_reopen_prepare,
> .bdrv_reopen_commit = throttle_reopen_commit,
> .bdrv_reopen_abort = throttle_reopen_abort,
>- .bdrv_co_get_block_status = throttle_co_get_block_status,
>+ .bdrv_co_get_block_status =
>bdrv_co_get_block_status_from_file,
>
> .is_filter = true,
> };
>
>
>--
>Eric Blake, Principal Software Engineer
>Red Hat, Inc. +1-919-301-3266
>Virtualization: qemu.org | libvirt.org
>
Am 06.09.2017 um 10:26 hat Manos Pitsidianakis geschrieben: > On Tue, Sep 05, 2017 at 04:13:39PM -0500, Eric Blake wrote: > > On 09/05/2017 02:06 PM, Kevin Wolf wrote: > > > Am 05.09.2017 um 18:16 hat Kevin Wolf geschrieben: > > > > Am 25.08.2017 um 15:20 hat Manos Pitsidianakis geschrieben: > > > > > Reviewed-by: Alberto Garcia <berto@igalia.com> > > > > > Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> > > > > > > > > Does this test actually (still) pass for you? I can't see that it's > > > > related to any recent change in master, but this is the diff that I get. > > > > > > > > I can update the reference output while applying, but obviously if it's > > > > currently passing for you, it will fail after I "fix" it. > > > > > > For the record, we discussed this on IRC. The test works correctly on > > > master, but on my block branch there is a conflict with "block: pass > > > bdrv_* methods to bs->file by default in block filters". > > > > > > The correct action is to merge this throttle driver series after the > > > conflicting patch because throttle doesn't implement .bdrv_get_info and > > > needs the forwarding that the other patch implements. > > > > > > I updated the test output accordingly and applied the series to my block > > > branch. > > > > Could you also squash this in to 5/6? (as long as we're intentionally > > basing throttle on top of defaults, then we should use the right default > > instead of duplicating things) > > Yes, this change makes sense, if it's no trouble with Kevin. Already done, I was just waiting for your opinion. :-) Kevin
© 2016 - 2026 Red Hat, Inc.