* [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups
@ 2016-07-08 14:05 Alberto Garcia
2016-07-08 14:05 ` [Qemu-devel] [PATCH for-2.6 1/2] blockdev: " Alberto Garcia
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Alberto Garcia @ 2016-07-08 14:05 UTC (permalink / raw
To: qemu-devel
Cc: qemu-block, qemu-stable, Stefan Hajnoczi, Max Reitz,
Alberto Garcia
Hi,
Stefan reported this, this is a regression caused by commit
efaa7c4eeb7490c6f37f3.
I sent a separate series for the git master, this is the backport for
QEMU v2.6.0.
Berto
Alberto Garcia (2):
blockdev: Fix regression with the default naming of throttling groups
qemu-iotests: Test naming of throttling groups
blockdev.c | 9 +++--
tests/qemu-iotests/093 | 98 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/093.out | 4 +-
3 files changed, 106 insertions(+), 5 deletions(-)
--
2.8.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH for-2.6 1/2] blockdev: Fix regression with the default naming of throttling groups
2016-07-08 14:05 [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups Alberto Garcia
@ 2016-07-08 14:05 ` Alberto Garcia
2016-07-08 14:38 ` Max Reitz
2016-07-08 14:05 ` [Qemu-devel] [PATCH for-2.6 2/2] qemu-iotests: Test " Alberto Garcia
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Alberto Garcia @ 2016-07-08 14:05 UTC (permalink / raw
To: qemu-devel
Cc: qemu-block, qemu-stable, Stefan Hajnoczi, Max Reitz,
Alberto Garcia
When I/O limits are set for a block device, the name of the throttling
group is taken from the BlockBackend if the user doesn't specify one.
Commit efaa7c4eeb7490c6f37f3 moved the naming of the BlockBackend in
blockdev_init() to the end of the function, after I/O limits are set.
The consequence is that the throttling group gets an empty name.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-stable@nongnu.org
---
blockdev.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index f1f520a..260a6f5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -481,6 +481,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
const char *id;
BlockdevDetectZeroesOptions detect_zeroes =
BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
+ const char *blk_id;
const char *throttling_group = NULL;
/* Check common options by copying from bs_opts to opts, all other options
@@ -510,6 +511,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
writethrough = !qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true);
+ blk_id = qemu_opts_id(opts);
+
qdict_extract_subqdict(bs_opts, &interval_dict, "stats-intervals.");
qdict_array_split(interval_dict, &interval_list);
@@ -579,7 +582,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
if (throttle_enabled(&cfg)) {
if (!throttling_group) {
- throttling_group = blk_name(blk);
+ throttling_group = blk_id;
}
blk_rs->throttle_group = g_strdup(throttling_group);
blk_rs->throttle_state = throttle_group_incref(throttling_group);
@@ -614,7 +617,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
/* disk I/O throttling */
if (throttle_enabled(&cfg)) {
if (!throttling_group) {
- throttling_group = blk_name(blk);
+ throttling_group = blk_id;
}
bdrv_io_limits_enable(bs, throttling_group);
bdrv_set_io_limits(bs, &cfg);
@@ -636,7 +639,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
blk_set_enable_write_cache(blk, !writethrough);
blk_set_on_error(blk, on_read_error, on_write_error);
- if (!monitor_add_blk(blk, qemu_opts_id(opts), errp)) {
+ if (!monitor_add_blk(blk, blk_id, errp)) {
blk_unref(blk);
blk = NULL;
goto err_no_bs_opts;
--
2.8.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH for-2.6 2/2] qemu-iotests: Test naming of throttling groups
2016-07-08 14:05 [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups Alberto Garcia
2016-07-08 14:05 ` [Qemu-devel] [PATCH for-2.6 1/2] blockdev: " Alberto Garcia
@ 2016-07-08 14:05 ` Alberto Garcia
2016-07-08 14:39 ` Max Reitz
2016-07-08 16:29 ` [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default " Stefan Hajnoczi
2016-07-26 12:52 ` Alberto Garcia
3 siblings, 1 reply; 9+ messages in thread
From: Alberto Garcia @ 2016-07-08 14:05 UTC (permalink / raw
To: qemu-devel
Cc: qemu-block, qemu-stable, Stefan Hajnoczi, Max Reitz,
Alberto Garcia
Throttling groups are named using the 'group' parameter of the
block_set_io_throttle command and the throttling.group command-line
option. If that parameter is unspecified the groups get the name of
the block device.
This patch adds a new test to check the naming of throttling groups.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
tests/qemu-iotests/093 | 98 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/093.out | 4 +-
2 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index ce8e13c..ffcb271 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -184,5 +184,103 @@ class ThrottleTestCase(iotests.QMPTestCase):
class ThrottleTestCoroutine(ThrottleTestCase):
test_img = "null-co://"
+class ThrottleTestGroupNames(iotests.QMPTestCase):
+ test_img = "null-aio://"
+ max_drives = 3
+
+ def setUp(self):
+ self.vm = iotests.VM()
+ for i in range(0, self.max_drives):
+ self.vm.add_drive(self.test_img, "throttling.iops-total=100")
+ self.vm.launch()
+
+ def tearDown(self):
+ self.vm.shutdown()
+
+ def set_io_throttle(self, device, params):
+ params["device"] = device
+ result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params)
+ self.assert_qmp(result, 'return', {})
+
+ def verify_name(self, device, name):
+ result = self.vm.qmp("query-block")
+ for r in result["return"]:
+ if r["device"] == device:
+ info = r["inserted"]
+ if name:
+ self.assertEqual(info["group"], name)
+ else:
+ self.assertFalse(info.has_key('group'))
+ return
+
+ raise Exception("No group information found for '%s'" % device)
+
+ def test_group_naming(self):
+ params = {"bps": 0,
+ "bps_rd": 0,
+ "bps_wr": 0,
+ "iops": 0,
+ "iops_rd": 0,
+ "iops_wr": 0}
+
+ # Check the drives added using the command line.
+ # The default throttling group name is the device name.
+ for i in range(self.max_drives):
+ devname = "drive%d" % i
+ self.verify_name(devname, devname)
+
+ # Clear throttling settings => the group name is gone.
+ for i in range(self.max_drives):
+ devname = "drive%d" % i
+ self.set_io_throttle(devname, params)
+ self.verify_name(devname, None)
+
+ # Set throttling settings using block_set_io_throttle and
+ # check the default group names.
+ params["iops"] = 10
+ for i in range(self.max_drives):
+ devname = "drive%d" % i
+ self.set_io_throttle(devname, params)
+ self.verify_name(devname, devname)
+
+ # Set a custom group name for each device
+ for i in range(3):
+ devname = "drive%d" % i
+ groupname = "group%d" % i
+ params['group'] = groupname
+ self.set_io_throttle(devname, params)
+ self.verify_name(devname, groupname)
+
+ # Put drive0 in group1 and check that all other devices remain
+ # unchanged
+ params['group'] = 'group1'
+ self.set_io_throttle('drive0', params)
+ self.verify_name('drive0', 'group1')
+ for i in range(1, self.max_drives):
+ devname = "drive%d" % i
+ groupname = "group%d" % i
+ self.verify_name(devname, groupname)
+
+ # Put drive0 in group2 and check that all other devices remain
+ # unchanged
+ params['group'] = 'group2'
+ self.set_io_throttle('drive0', params)
+ self.verify_name('drive0', 'group2')
+ for i in range(1, self.max_drives):
+ devname = "drive%d" % i
+ groupname = "group%d" % i
+ self.verify_name(devname, groupname)
+
+ # Clear throttling settings from drive0 check that all other
+ # devices remain unchanged
+ params["iops"] = 0
+ self.set_io_throttle('drive0', params)
+ self.verify_name('drive0', None)
+ for i in range(1, self.max_drives):
+ devname = "drive%d" % i
+ groupname = "group%d" % i
+ self.verify_name(devname, groupname)
+
+
if __name__ == '__main__':
iotests.main(supported_fmts=["raw"])
diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out
index 89968f3..914e373 100644
--- a/tests/qemu-iotests/093.out
+++ b/tests/qemu-iotests/093.out
@@ -1,5 +1,5 @@
-....
+.....
----------------------------------------------------------------------
-Ran 4 tests
+Ran 5 tests
OK
--
2.8.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 1/2] blockdev: Fix regression with the default naming of throttling groups
2016-07-08 14:05 ` [Qemu-devel] [PATCH for-2.6 1/2] blockdev: " Alberto Garcia
@ 2016-07-08 14:38 ` Max Reitz
0 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2016-07-08 14:38 UTC (permalink / raw
To: Alberto Garcia, qemu-devel; +Cc: qemu-block, qemu-stable, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 721 bytes --]
On 08.07.2016 16:05, Alberto Garcia wrote:
> When I/O limits are set for a block device, the name of the throttling
> group is taken from the BlockBackend if the user doesn't specify one.
>
> Commit efaa7c4eeb7490c6f37f3 moved the naming of the BlockBackend in
> blockdev_init() to the end of the function, after I/O limits are set.
> The consequence is that the throttling group gets an empty name.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: qemu-stable@nongnu.org
> ---
> blockdev.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 2/2] qemu-iotests: Test naming of throttling groups
2016-07-08 14:05 ` [Qemu-devel] [PATCH for-2.6 2/2] qemu-iotests: Test " Alberto Garcia
@ 2016-07-08 14:39 ` Max Reitz
0 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2016-07-08 14:39 UTC (permalink / raw
To: Alberto Garcia, qemu-devel; +Cc: qemu-block, qemu-stable, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 637 bytes --]
On 08.07.2016 16:05, Alberto Garcia wrote:
> Throttling groups are named using the 'group' parameter of the
> block_set_io_throttle command and the throttling.group command-line
> option. If that parameter is unspecified the groups get the name of
> the block device.
>
> This patch adds a new test to check the naming of throttling groups.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> tests/qemu-iotests/093 | 98 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/093.out | 4 +-
> 2 files changed, 100 insertions(+), 2 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups
2016-07-08 14:05 [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups Alberto Garcia
2016-07-08 14:05 ` [Qemu-devel] [PATCH for-2.6 1/2] blockdev: " Alberto Garcia
2016-07-08 14:05 ` [Qemu-devel] [PATCH for-2.6 2/2] qemu-iotests: Test " Alberto Garcia
@ 2016-07-08 16:29 ` Stefan Hajnoczi
2016-07-26 12:52 ` Alberto Garcia
3 siblings, 0 replies; 9+ messages in thread
From: Stefan Hajnoczi @ 2016-07-08 16:29 UTC (permalink / raw
To: Alberto Garcia; +Cc: qemu-devel, qemu-block, qemu-stable, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 726 bytes --]
On Fri, Jul 08, 2016 at 05:05:12PM +0300, Alberto Garcia wrote:
> Hi,
>
> Stefan reported this, this is a regression caused by commit
> efaa7c4eeb7490c6f37f3.
>
> I sent a separate series for the git master, this is the backport for
> QEMU v2.6.0.
>
> Berto
>
> Alberto Garcia (2):
> blockdev: Fix regression with the default naming of throttling groups
> qemu-iotests: Test naming of throttling groups
>
> blockdev.c | 9 +++--
> tests/qemu-iotests/093 | 98 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/093.out | 4 +-
> 3 files changed, 106 insertions(+), 5 deletions(-)
>
> --
> 2.8.1
>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups
2016-07-08 14:05 [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups Alberto Garcia
` (2 preceding siblings ...)
2016-07-08 16:29 ` [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default " Stefan Hajnoczi
@ 2016-07-26 12:52 ` Alberto Garcia
2016-07-26 15:34 ` Max Reitz
3 siblings, 1 reply; 9+ messages in thread
From: Alberto Garcia @ 2016-07-26 12:52 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-block, qemu-stable, Stefan Hajnoczi, Max Reitz
On Fri, Jul 08, 2016 at 05:05:12PM +0300, Alberto Garcia wrote:
> Hi,
>
> Stefan reported this, this is a regression caused by commit
> efaa7c4eeb7490c6f37f3.
>
> I sent a separate series for the git master, this is the backport
> for QEMU v2.6.0.
ping
Berto
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups
2016-07-26 12:52 ` Alberto Garcia
@ 2016-07-26 15:34 ` Max Reitz
2016-07-26 18:09 ` [Qemu-devel] [Qemu-block] " John Snow
0 siblings, 1 reply; 9+ messages in thread
From: Max Reitz @ 2016-07-26 15:34 UTC (permalink / raw
To: Alberto Garcia, qemu-devel; +Cc: qemu-block, qemu-stable, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 448 bytes --]
On 26.07.2016 14:52, Alberto Garcia wrote:
> On Fri, Jul 08, 2016 at 05:05:12PM +0300, Alberto Garcia wrote:
>> Hi,
>>
>> Stefan reported this, this is a regression caused by commit
>> efaa7c4eeb7490c6f37f3.
>>
>> I sent a separate series for the git master, this is the backport
>> for QEMU v2.6.0.
>
> ping
We don't have a tree for 2.6.0, do we? I thought it only got pulled in
when the stable release is being prepared.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups
2016-07-26 15:34 ` Max Reitz
@ 2016-07-26 18:09 ` John Snow
0 siblings, 0 replies; 9+ messages in thread
From: John Snow @ 2016-07-26 18:09 UTC (permalink / raw
To: Max Reitz, Alberto Garcia, qemu-devel
Cc: Stefan Hajnoczi, qemu-stable, qemu-block, Michael Roth
On 07/26/2016 11:34 AM, Max Reitz wrote:
> On 26.07.2016 14:52, Alberto Garcia wrote:
>> On Fri, Jul 08, 2016 at 05:05:12PM +0300, Alberto Garcia wrote:
>>> Hi,
>>>
>>> Stefan reported this, this is a regression caused by commit
>>> efaa7c4eeb7490c6f37f3.
>>>
>>> I sent a separate series for the git master, this is the backport
>>> for QEMU v2.6.0.
>>
>> ping
>
> We don't have a tree for 2.6.0, do we? I thought it only got pulled in
> when the stable release is being prepared.
>
> Max
>
Yeah, we don't really have a 2.6-stable tree except for the brief period
of time that Michael Roth is working actively on the release.
Usually CCing qemu-stable is good enough.
--js
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-07-26 18:09 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-08 14:05 [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default naming of throttling groups Alberto Garcia
2016-07-08 14:05 ` [Qemu-devel] [PATCH for-2.6 1/2] blockdev: " Alberto Garcia
2016-07-08 14:38 ` Max Reitz
2016-07-08 14:05 ` [Qemu-devel] [PATCH for-2.6 2/2] qemu-iotests: Test " Alberto Garcia
2016-07-08 14:39 ` Max Reitz
2016-07-08 16:29 ` [Qemu-devel] [PATCH for-2.6 0/2] Fix regression with the default " Stefan Hajnoczi
2016-07-26 12:52 ` Alberto Garcia
2016-07-26 15:34 ` Max Reitz
2016-07-26 18:09 ` [Qemu-devel] [Qemu-block] " John Snow
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.