* [Qemu-devel] [PULL 01/31] block: Make sure throttled BDSes always have a BB
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 02/31] block: Introduce BlockBackendPublic Kevin Wolf
` (30 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
It was already true in principle that a throttled BDS always has a BB
attached, except that the order of operations while attaching or
detaching a BDS to/from a BB wasn't careful enough.
This commit breaks graph manipulations while I/O throttling is enabled.
It would have been possible to keep things working with some temporary
hacks, but quite cumbersome, so it's not worth the hassle. We'll fix
things again in a minute.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 14 ++++++++++++++
block/block-backend.c | 3 +++
blockdev.c | 4 ++--
tests/test-throttle.c | 11 ++++++++---
4 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index 18a497f..a789844 100644
--- a/block.c
+++ b/block.c
@@ -2259,8 +2259,22 @@ static void swap_feature_fields(BlockDriverState *bs_top,
assert(!bs_new->throttle_state);
if (bs_top->throttle_state) {
+ /*
+ * FIXME Need to break I/O throttling with graph manipulations
+ * temporarily because of conflicting invariants (3. will go away when
+ * throttling is fully converted to work on BlockBackends):
+ *
+ * 1. Every BlockBackend has a single root BDS
+ * 2. I/O throttling functions require an attached BlockBackend
+ * 3. We need to first enable throttling on the new BDS and then
+ * disable it on the old one (because of throttle group refcounts)
+ */
+#if 0
bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top));
bdrv_io_limits_disable(bs_top);
+#else
+ abort();
+#endif
}
}
diff --git a/block/block-backend.c b/block/block-backend.c
index a1e2c7f..53fd9d2 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -420,6 +420,9 @@ void blk_remove_bs(BlockBackend *blk)
notifier_list_notify(&blk->remove_bs_notifiers, blk);
blk_update_root_state(blk);
+ if (blk->root->bs->throttle_state) {
+ bdrv_io_limits_disable(blk->root->bs);
+ }
blk->root->bs->blk = NULL;
bdrv_root_unref_child(blk->root);
diff --git a/blockdev.c b/blockdev.c
index 1892b8e..8106ca7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2570,8 +2570,6 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
goto fail;
}
- blk_apply_root_state(blk, medium_bs);
-
bdrv_add_key(medium_bs, NULL, &err);
if (err) {
error_propagate(errp, err);
@@ -2596,6 +2594,8 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
goto fail;
}
+ blk_apply_root_state(blk, medium_bs);
+
qmp_blockdev_close_tray(device, errp);
fail:
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 744a524..77b95d6 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -574,11 +574,16 @@ static void test_accounting(void)
static void test_groups(void)
{
ThrottleConfig cfg1, cfg2;
+ BlockBackend *blk1, *blk2, *blk3;
BlockDriverState *bdrv1, *bdrv2, *bdrv3;
- bdrv1 = bdrv_new();
- bdrv2 = bdrv_new();
- bdrv3 = bdrv_new();
+ blk1 = blk_new_with_bs(&error_abort);
+ blk2 = blk_new_with_bs(&error_abort);
+ blk3 = blk_new_with_bs(&error_abort);
+
+ bdrv1 = blk_bs(blk1);
+ bdrv2 = blk_bs(blk2);
+ bdrv3 = blk_bs(blk3);
g_assert(bdrv1->throttle_state == NULL);
g_assert(bdrv2->throttle_state == NULL);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 02/31] block: Introduce BlockBackendPublic
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 01/31] block: Make sure throttled BDSes always have a BB Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 03/31] block: throttle-groups: Use BlockBackend pointers internally Kevin Wolf
` (29 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
Some features, like I/O throttling, are implemented outside
block-backend.c, but still want to keep information in BlockBackend,
e.g. list entries that allow keeping a list of BlockBackends.
In order to avoid exposing the whole struct layout in the public header
file, this patch introduces an embedded public struct where such
information can be added and a pair of functions to convert between
BlockBackend and BlockBackendPublic.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/block-backend.c | 17 +++++++++++++++++
include/sysemu/block-backend.h | 10 ++++++++++
2 files changed, 27 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index 53fd9d2..2f8acbd 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -34,6 +34,7 @@ struct BlockBackend {
DriveInfo *legacy_dinfo; /* null unless created by drive_new() */
QTAILQ_ENTRY(BlockBackend) link; /* for block_backends */
QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
+ BlockBackendPublic public;
void *dev; /* attached device model, if any */
/* TODO change to DeviceState when all users are qdevified */
@@ -411,6 +412,22 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo)
}
/*
+ * Returns a pointer to the publicly accessible fields of @blk.
+ */
+BlockBackendPublic *blk_get_public(BlockBackend *blk)
+{
+ return &blk->public;
+}
+
+/*
+ * Returns a BlockBackend given the associated @public fields.
+ */
+BlockBackend *blk_by_public(BlockBackendPublic *public)
+{
+ return container_of(public, BlockBackend, public);
+}
+
+/*
* Disassociates the currently associated BlockDriverState from @blk.
*/
void blk_remove_bs(BlockBackend *blk)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 26736ed..a771603 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -59,6 +59,13 @@ typedef struct BlockDevOps {
void (*resize_cb)(void *opaque);
} BlockDevOps;
+/* This struct is embedded in (the private) BlockBackend struct and contains
+ * fields that must be public. This is in particular for QLIST_ENTRY() and
+ * friends so that BlockBackends can be kept in lists outside block-backend.c */
+typedef struct BlockBackendPublic {
+ int dummy; /* empty structs are illegal */
+} BlockBackendPublic;
+
BlockBackend *blk_new(Error **errp);
BlockBackend *blk_new_with_bs(Error **errp);
BlockBackend *blk_new_open(const char *filename, const char *reference,
@@ -74,6 +81,9 @@ BlockDriverState *blk_next_root_bs(BlockDriverState *bs);
bool monitor_add_blk(BlockBackend *blk, const char *name, Error **errp);
void monitor_remove_blk(BlockBackend *blk);
+BlockBackendPublic *blk_get_public(BlockBackend *blk);
+BlockBackend *blk_by_public(BlockBackendPublic *public);
+
BlockDriverState *blk_bs(BlockBackend *blk);
void blk_remove_bs(BlockBackend *blk);
void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 03/31] block: throttle-groups: Use BlockBackend pointers internally
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 01/31] block: Make sure throttled BDSes always have a BB Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 02/31] block: Introduce BlockBackendPublic Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 04/31] block: Convert throttle_group_get_name() to BlockBackend Kevin Wolf
` (28 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
As a first step towards moving I/O throttling to the BlockBackend level,
this patch changes all pointers in struct ThrottleGroup from referencing
a BlockDriverState to referencing a BlockBackend.
This change is valid because we made sure that throttling can only be
enabled on BDSes which have a BB attached.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/io.c | 4 +-
block/throttle-groups.c | 134 ++++++++++++++++++++--------------------
include/block/block_int.h | 1 -
include/block/throttle-groups.h | 4 +-
include/sysemu/block-backend.h | 5 +-
tests/test-throttle.c | 13 ++--
6 files changed, 83 insertions(+), 78 deletions(-)
diff --git a/block/io.c b/block/io.c
index cd6d71a..ede74c5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -70,7 +70,7 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
{
assert(bs->throttle_state);
bdrv_no_throttling_begin(bs);
- throttle_group_unregister_bs(bs);
+ throttle_group_unregister_blk(bs->blk);
bdrv_no_throttling_end(bs);
}
@@ -78,7 +78,7 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
void bdrv_io_limits_enable(BlockDriverState *bs, const char *group)
{
assert(!bs->throttle_state);
- throttle_group_register_bs(bs, group);
+ throttle_group_register_blk(bs->blk, group);
}
void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 9ac063a..1e6e2e5 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -23,6 +23,7 @@
*/
#include "qemu/osdep.h"
+#include "sysemu/block-backend.h"
#include "block/throttle-groups.h"
#include "qemu/queue.h"
#include "qemu/thread.h"
@@ -57,8 +58,8 @@ typedef struct ThrottleGroup {
QemuMutex lock; /* This lock protects the following four fields */
ThrottleState ts;
- QLIST_HEAD(, BlockDriverState) head;
- BlockDriverState *tokens[2];
+ QLIST_HEAD(, BlockBackendPublic) head;
+ BlockBackend *tokens[2];
bool any_timer_armed[2];
/* These two are protected by the global throttle_groups_lock */
@@ -145,81 +146,81 @@ const char *throttle_group_get_name(BlockDriverState *bs)
return tg->name;
}
-/* Return the next BlockDriverState in the round-robin sequence,
- * simulating a circular list.
+/* Return the next BlockBackend in the round-robin sequence, simulating a
+ * circular list.
*
* This assumes that tg->lock is held.
*
- * @bs: the current BlockDriverState
- * @ret: the next BlockDriverState in the sequence
+ * @blk: the current BlockBackend
+ * @ret: the next BlockBackend in the sequence
*/
-static BlockDriverState *throttle_group_next_bs(BlockDriverState *bs)
+static BlockBackend *throttle_group_next_blk(BlockBackend *blk)
{
+ BlockDriverState *bs = blk_bs(blk);
ThrottleState *ts = bs->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
- BlockDriverState *next = QLIST_NEXT(bs, round_robin);
+ BlockBackendPublic *next = QLIST_NEXT(blk_get_public(blk), round_robin);
if (!next) {
- return QLIST_FIRST(&tg->head);
+ next = QLIST_FIRST(&tg->head);
}
- return next;
+ return blk_by_public(next);
}
-/* Return the next BlockDriverState in the round-robin sequence with
- * pending I/O requests.
+/* Return the next BlockBackend in the round-robin sequence with pending I/O
+ * requests.
*
* This assumes that tg->lock is held.
*
- * @bs: the current BlockDriverState
+ * @blk: the current BlockBackend
* @is_write: the type of operation (read/write)
- * @ret: the next BlockDriverState with pending requests, or bs
- * if there is none.
+ * @ret: the next BlockBackend with pending requests, or blk if there is
+ * none.
*/
-static BlockDriverState *next_throttle_token(BlockDriverState *bs,
- bool is_write)
+static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write)
{
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
- BlockDriverState *token, *start;
+ ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
+ ThrottleGroup, ts);
+ BlockBackend *token, *start;
start = token = tg->tokens[is_write];
/* get next bs round in round robin style */
- token = throttle_group_next_bs(token);
- while (token != start && !token->pending_reqs[is_write]) {
- token = throttle_group_next_bs(token);
+ token = throttle_group_next_blk(token);
+ while (token != start && !blk_bs(token)->pending_reqs[is_write]) {
+ token = throttle_group_next_blk(token);
}
/* If no IO are queued for scheduling on the next round robin token
* then decide the token is the current bs because chances are
* the current bs get the current request queued.
*/
- if (token == start && !token->pending_reqs[is_write]) {
- token = bs;
+ if (token == start && !blk_bs(token)->pending_reqs[is_write]) {
+ token = blk;
}
return token;
}
-/* Check if the next I/O request for a BlockDriverState needs to be
- * throttled or not. If there's no timer set in this group, set one
- * and update the token accordingly.
+/* Check if the next I/O request for a BlockBackend needs to be throttled or
+ * not. If there's no timer set in this group, set one and update the token
+ * accordingly.
*
* This assumes that tg->lock is held.
*
- * @bs: the current BlockDriverState
+ * @blk: the current BlockBackend
* @is_write: the type of operation (read/write)
* @ret: whether the I/O request needs to be throttled or not
*/
-static bool throttle_group_schedule_timer(BlockDriverState *bs,
- bool is_write)
+static bool throttle_group_schedule_timer(BlockBackend *blk, bool is_write)
{
- ThrottleState *ts = bs->throttle_state;
- ThrottleTimers *tt = &bs->throttle_timers;
+ ThrottleState *ts = blk_bs(blk)->throttle_state;
+ ThrottleTimers *tt = &blk_bs(blk)->throttle_timers;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
bool must_wait;
- if (bs->io_limits_disabled) {
+ if (blk_bs(blk)->io_limits_disabled) {
return false;
}
@@ -230,9 +231,9 @@ static bool throttle_group_schedule_timer(BlockDriverState *bs,
must_wait = throttle_schedule_timer(ts, tt, is_write);
- /* If a timer just got armed, set bs as the current token */
+ /* If a timer just got armed, set blk as the current token */
if (must_wait) {
- tg->tokens[is_write] = bs;
+ tg->tokens[is_write] = blk;
tg->any_timer_armed[is_write] = true;
}
@@ -243,18 +244,19 @@ static bool throttle_group_schedule_timer(BlockDriverState *bs,
*
* This assumes that tg->lock is held.
*
- * @bs: the current BlockDriverState
+ * @blk: the current BlockBackend
* @is_write: the type of operation (read/write)
*/
-static void schedule_next_request(BlockDriverState *bs, bool is_write)
+static void schedule_next_request(BlockBackend *blk, bool is_write)
{
+ BlockDriverState *bs = blk_bs(blk);
ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
bool must_wait;
- BlockDriverState *token;
+ BlockBackend *token;
/* Check if there's any pending request to schedule next */
- token = next_throttle_token(bs, is_write);
- if (!token->pending_reqs[is_write]) {
+ token = next_throttle_token(blk, is_write);
+ if (!blk_bs(token)->pending_reqs[is_write]) {
return;
}
@@ -266,9 +268,9 @@ static void schedule_next_request(BlockDriverState *bs, bool is_write)
/* Give preference to requests from the current bs */
if (qemu_in_coroutine() &&
qemu_co_queue_next(&bs->throttled_reqs[is_write])) {
- token = bs;
+ token = blk;
} else {
- ThrottleTimers *tt = &token->throttle_timers;
+ ThrottleTimers *tt = &blk_bs(token)->throttle_timers;
int64_t now = qemu_clock_get_ns(tt->clock_type);
timer_mod(tt->timers[is_write], now + 1);
tg->any_timer_armed[is_write] = true;
@@ -290,13 +292,13 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
bool is_write)
{
bool must_wait;
- BlockDriverState *token;
+ BlockBackend *token;
ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
/* First we check if this I/O has to be throttled. */
- token = next_throttle_token(bs, is_write);
+ token = next_throttle_token(bs->blk, is_write);
must_wait = throttle_group_schedule_timer(token, is_write);
/* Wait if there's a timer set or queued requests of this type */
@@ -312,7 +314,7 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
throttle_account(bs->throttle_state, is_write, bytes);
/* Schedule the next request */
- schedule_next_request(bs, is_write);
+ schedule_next_request(bs->blk, is_write);
qemu_mutex_unlock(&tg->lock);
}
@@ -395,7 +397,7 @@ static void timer_cb(BlockDriverState *bs, bool is_write)
* scheduling the next one */
if (empty_queue) {
qemu_mutex_lock(&tg->lock);
- schedule_next_request(bs, is_write);
+ schedule_next_request(bs->blk, is_write);
qemu_mutex_unlock(&tg->lock);
}
}
@@ -410,17 +412,17 @@ static void write_timer_cb(void *opaque)
timer_cb(opaque, true);
}
-/* Register a BlockDriverState in the throttling group, also
- * initializing its timers and updating its throttle_state pointer to
- * point to it. If a throttling group with that name does not exist
- * yet, it will be created.
+/* Register a BlockBackend in the throttling group, also initializing its
+ * timers and updating its throttle_state pointer to point to it. If a
+ * throttling group with that name does not exist yet, it will be created.
*
- * @bs: the BlockDriverState to insert
+ * @blk: the BlockBackend to insert
* @groupname: the name of the group
*/
-void throttle_group_register_bs(BlockDriverState *bs, const char *groupname)
+void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
{
int i;
+ BlockDriverState *bs = blk_bs(blk);
ThrottleState *ts = throttle_group_incref(groupname);
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
int clock_type = QEMU_CLOCK_REALTIME;
@@ -433,14 +435,14 @@ void throttle_group_register_bs(BlockDriverState *bs, const char *groupname)
bs->throttle_state = ts;
qemu_mutex_lock(&tg->lock);
- /* If the ThrottleGroup is new set this BlockDriverState as the token */
+ /* If the ThrottleGroup is new set this BlockBackend as the token */
for (i = 0; i < 2; i++) {
if (!tg->tokens[i]) {
- tg->tokens[i] = bs;
+ tg->tokens[i] = blk;
}
}
- QLIST_INSERT_HEAD(&tg->head, bs, round_robin);
+ QLIST_INSERT_HEAD(&tg->head, blk_get_public(blk), round_robin);
throttle_timers_init(&bs->throttle_timers,
bdrv_get_aio_context(bs),
@@ -452,19 +454,19 @@ void throttle_group_register_bs(BlockDriverState *bs, const char *groupname)
qemu_mutex_unlock(&tg->lock);
}
-/* Unregister a BlockDriverState from its group, removing it from the
- * list, destroying the timers and setting the throttle_state pointer
- * to NULL.
+/* Unregister a BlockBackend from its group, removing it from the list,
+ * destroying the timers and setting the throttle_state pointer to NULL.
*
- * The BlockDriverState must not have pending throttled requests, so
- * the caller has to drain them first.
+ * The BlockBackend must not have pending throttled requests, so the caller has
+ * to drain them first.
*
* The group will be destroyed if it's empty after this operation.
*
- * @bs: the BlockDriverState to remove
+ * @blk: the BlockBackend to remove
*/
-void throttle_group_unregister_bs(BlockDriverState *bs)
+void throttle_group_unregister_blk(BlockBackend *blk)
{
+ BlockDriverState *bs = blk_bs(blk);
ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
int i;
@@ -474,10 +476,10 @@ void throttle_group_unregister_bs(BlockDriverState *bs)
qemu_mutex_lock(&tg->lock);
for (i = 0; i < 2; i++) {
- if (tg->tokens[i] == bs) {
- BlockDriverState *token = throttle_group_next_bs(bs);
+ if (tg->tokens[i] == blk) {
+ BlockBackend *token = throttle_group_next_blk(blk);
/* Take care of the case where this is the last bs in the group */
- if (token == bs) {
+ if (token == blk) {
token = NULL;
}
tg->tokens[i] = token;
@@ -485,7 +487,7 @@ void throttle_group_unregister_bs(BlockDriverState *bs)
}
/* remove the current bs from the list */
- QLIST_REMOVE(bs, round_robin);
+ QLIST_REMOVE(blk_get_public(blk), round_robin);
throttle_timers_destroy(&bs->throttle_timers);
qemu_mutex_unlock(&tg->lock);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a029c20..3f5d2b1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -435,7 +435,6 @@ struct BlockDriverState {
ThrottleState *throttle_state;
ThrottleTimers throttle_timers;
unsigned pending_reqs[2];
- QLIST_ENTRY(BlockDriverState) round_robin;
/* Offset after the highest byte written to */
uint64_t wr_highest_offset;
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index 395f72d..b9114ee 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -36,8 +36,8 @@ void throttle_group_unref(ThrottleState *ts);
void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg);
void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg);
-void throttle_group_register_bs(BlockDriverState *bs, const char *groupname);
-void throttle_group_unregister_bs(BlockDriverState *bs);
+void throttle_group_register_blk(BlockBackend *blk, const char *groupname);
+void throttle_group_unregister_blk(BlockBackend *blk);
void throttle_group_restart_bs(BlockDriverState *bs);
void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index a771603..1dcd70e 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -63,7 +63,10 @@ typedef struct BlockDevOps {
* fields that must be public. This is in particular for QLIST_ENTRY() and
* friends so that BlockBackends can be kept in lists outside block-backend.c */
typedef struct BlockBackendPublic {
- int dummy; /* empty structs are illegal */
+ /* I/O throttling */
+ /* The following fields are protected by the ThrottleGroup lock.
+ * See the ThrottleGroup documentation for details. */
+ QLIST_ENTRY(BlockBackendPublic) round_robin;
} BlockBackendPublic;
BlockBackend *blk_new(Error **errp);
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 77b95d6..beaa2a3 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -20,6 +20,7 @@
#include "qemu/throttle.h"
#include "qemu/error-report.h"
#include "block/throttle-groups.h"
+#include "sysemu/block-backend.h"
static AioContext *ctx;
static LeakyBucket bkt;
@@ -589,9 +590,9 @@ static void test_groups(void)
g_assert(bdrv2->throttle_state == NULL);
g_assert(bdrv3->throttle_state == NULL);
- throttle_group_register_bs(bdrv1, "bar");
- throttle_group_register_bs(bdrv2, "foo");
- throttle_group_register_bs(bdrv3, "bar");
+ throttle_group_register_blk(blk1, "bar");
+ throttle_group_register_blk(blk2, "foo");
+ throttle_group_register_blk(blk3, "bar");
g_assert(bdrv1->throttle_state != NULL);
g_assert(bdrv2->throttle_state != NULL);
@@ -623,9 +624,9 @@ static void test_groups(void)
throttle_group_get_config(bdrv3, &cfg2);
g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1)));
- throttle_group_unregister_bs(bdrv1);
- throttle_group_unregister_bs(bdrv2);
- throttle_group_unregister_bs(bdrv3);
+ throttle_group_unregister_blk(blk1);
+ throttle_group_unregister_blk(blk2);
+ throttle_group_unregister_blk(blk3);
g_assert(bdrv1->throttle_state == NULL);
g_assert(bdrv2->throttle_state == NULL);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 04/31] block: Convert throttle_group_get_name() to BlockBackend
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (2 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 03/31] block: throttle-groups: Use BlockBackend pointers internally Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 05/31] block: Move throttling fields from BDS to BB Kevin Wolf
` (27 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/block-backend.c | 2 +-
block/io.c | 2 +-
block/qapi.c | 2 +-
block/throttle-groups.c | 12 ++++++------
include/block/throttle-groups.h | 2 +-
tests/test-throttle.c | 4 ++--
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 2f8acbd..964a205 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1525,7 +1525,7 @@ void blk_update_root_state(BlockBackend *blk)
throttle_group_unref(blk->root_state.throttle_state);
}
if (blk->root->bs->throttle_state) {
- const char *name = throttle_group_get_name(blk->root->bs);
+ const char *name = throttle_group_get_name(blk);
blk->root_state.throttle_group = g_strdup(name);
blk->root_state.throttle_state = throttle_group_incref(name);
} else {
diff --git a/block/io.c b/block/io.c
index ede74c5..f6fb868 100644
--- a/block/io.c
+++ b/block/io.c
@@ -89,7 +89,7 @@ void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
}
/* this bs is a part of the same group than the one we want */
- if (!g_strcmp0(throttle_group_get_name(bs), group)) {
+ if (!g_strcmp0(throttle_group_get_name(bs->blk), group)) {
return;
}
diff --git a/block/qapi.c b/block/qapi.c
index c5f6ba6..a3e514d 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -118,7 +118,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
info->iops_size = cfg.op_size;
info->has_group = true;
- info->group = g_strdup(throttle_group_get_name(bs));
+ info->group = g_strdup(throttle_group_get_name(bs->blk));
}
info->write_threshold = bdrv_write_threshold_get(bs);
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 1e6e2e5..e50ccaa 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -133,16 +133,16 @@ void throttle_group_unref(ThrottleState *ts)
qemu_mutex_unlock(&throttle_groups_lock);
}
-/* Get the name from a BlockDriverState's ThrottleGroup. The name (and
- * the pointer) is guaranteed to remain constant during the lifetime
- * of the group.
+/* Get the name from a BlockBackend's ThrottleGroup. The name (and the pointer)
+ * is guaranteed to remain constant during the lifetime of the group.
*
- * @bs: a BlockDriverState that is member of a throttling group
+ * @blk: a BlockBackend that is member of a throttling group
* @ret: the name of the group.
*/
-const char *throttle_group_get_name(BlockDriverState *bs)
+const char *throttle_group_get_name(BlockBackend *blk)
{
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
+ ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
+ ThrottleGroup, ts);
return tg->name;
}
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index b9114ee..bd55a34 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -28,7 +28,7 @@
#include "qemu/throttle.h"
#include "block/block_int.h"
-const char *throttle_group_get_name(BlockDriverState *bs);
+const char *throttle_group_get_name(BlockBackend *blk);
ThrottleState *throttle_group_incref(const char *name);
void throttle_group_unref(ThrottleState *ts);
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index beaa2a3..1a322f1 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -598,8 +598,8 @@ static void test_groups(void)
g_assert(bdrv2->throttle_state != NULL);
g_assert(bdrv3->throttle_state != NULL);
- g_assert(!strcmp(throttle_group_get_name(bdrv1), "bar"));
- g_assert(!strcmp(throttle_group_get_name(bdrv2), "foo"));
+ g_assert(!strcmp(throttle_group_get_name(blk1), "bar"));
+ g_assert(!strcmp(throttle_group_get_name(blk2), "foo"));
g_assert(bdrv1->throttle_state == bdrv3->throttle_state);
/* Setting the config of a group member affects the whole group */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 05/31] block: Move throttling fields from BDS to BB
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (3 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 04/31] block: Convert throttle_group_get_name() to BlockBackend Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 06/31] block: Move actual I/O throttling to BlockBackend Kevin Wolf
` (26 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
This patch changes where the throttling state is stored (used to be the
BlockDriverState, now it is the BlockBackend), but it doesn't actually
make it a BB level feature yet. For example, throttling is still
disabled when the BDS is detached from the BB.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 22 +++----
block/block-backend.c | 13 ++--
block/io.c | 36 +++++++----
block/qapi.c | 2 +-
block/throttle-groups.c | 129 +++++++++++++++++++++-------------------
blockdev.c | 4 +-
include/block/block_int.h | 13 ----
include/block/throttle-groups.h | 2 +-
include/sysemu/block-backend.h | 11 +++-
tests/test-throttle.c | 28 +++++----
10 files changed, 142 insertions(+), 118 deletions(-)
diff --git a/block.c b/block.c
index a789844..f723060 100644
--- a/block.c
+++ b/block.c
@@ -237,8 +237,6 @@ BlockDriverState *bdrv_new(void)
QLIST_INIT(&bs->op_blockers[i]);
}
notifier_with_return_list_init(&bs->before_write_notifiers);
- qemu_co_queue_init(&bs->throttled_reqs[0]);
- qemu_co_queue_init(&bs->throttled_reqs[1]);
bs->refcnt = 1;
bs->aio_context = qemu_get_aio_context();
@@ -1525,7 +1523,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
return -ENODEV;
}
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
error_setg(errp, "Cannot reference an existing block device for "
"which I/O throttling is enabled");
return -EINVAL;
@@ -2124,7 +2122,7 @@ static void bdrv_close(BlockDriverState *bs)
assert(!bs->job);
/* Disable I/O limits and drain all pending throttled requests */
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
bdrv_io_limits_disable(bs);
}
@@ -2257,8 +2255,8 @@ static void swap_feature_fields(BlockDriverState *bs_top,
bdrv_move_feature_fields(bs_top, bs_new);
bdrv_move_feature_fields(bs_new, &tmp);
- assert(!bs_new->throttle_state);
- if (bs_top->throttle_state) {
+ assert(!bs_new->blk);
+ if (bs_top->blk && blk_get_public(bs_top->blk)->throttle_state) {
/*
* FIXME Need to break I/O throttling with graph manipulations
* temporarily because of conflicting invariants (3. will go away when
@@ -2300,11 +2298,11 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
assert(!bdrv_requests_pending(bs_new));
bdrv_ref(bs_top);
- change_parent_backing_link(bs_top, bs_new);
/* Some fields always stay on top of the backing file chain */
swap_feature_fields(bs_top, bs_new);
+ change_parent_backing_link(bs_top, bs_new);
bdrv_set_backing_hd(bs_new, bs_top);
bdrv_unref(bs_top);
@@ -3676,8 +3674,9 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
baf->detach_aio_context(baf->opaque);
}
- if (bs->throttle_state) {
- throttle_timers_detach_aio_context(&bs->throttle_timers);
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
+ throttle_timers_detach_aio_context(
+ &blk_get_public(bs->blk)->throttle_timers);
}
if (bs->drv->bdrv_detach_aio_context) {
bs->drv->bdrv_detach_aio_context(bs);
@@ -3712,8 +3711,9 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
if (bs->drv->bdrv_attach_aio_context) {
bs->drv->bdrv_attach_aio_context(bs, new_context);
}
- if (bs->throttle_state) {
- throttle_timers_attach_aio_context(&bs->throttle_timers, new_context);
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
+ throttle_timers_attach_aio_context(
+ &blk_get_public(bs->blk)->throttle_timers, new_context);
}
QLIST_FOREACH(ban, &bs->aio_notifiers, list) {
diff --git a/block/block-backend.c b/block/block-backend.c
index 964a205..6880659 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -107,8 +107,12 @@ BlockBackend *blk_new(Error **errp)
blk = g_new0(BlockBackend, 1);
blk->refcnt = 1;
+ qemu_co_queue_init(&blk->public.throttled_reqs[0]);
+ qemu_co_queue_init(&blk->public.throttled_reqs[1]);
+
notifier_list_init(&blk->remove_bs_notifiers);
notifier_list_init(&blk->insert_bs_notifiers);
+
QTAILQ_INSERT_TAIL(&block_backends, blk, link);
return blk;
}
@@ -437,7 +441,7 @@ void blk_remove_bs(BlockBackend *blk)
notifier_list_notify(&blk->remove_bs_notifiers, blk);
blk_update_root_state(blk);
- if (blk->root->bs->throttle_state) {
+ if (blk->public.throttle_state) {
bdrv_io_limits_disable(blk->root->bs);
}
@@ -795,7 +799,6 @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
int count)
{
- BlockDriverState *bs = blk_bs(blk);
int ret;
ret = blk_check_byte_request(blk, offset, count);
@@ -803,9 +806,9 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
return ret;
}
- bdrv_no_throttling_begin(bs);
+ bdrv_no_throttling_begin(blk_bs(blk));
ret = blk_pread(blk, offset, buf, count);
- bdrv_no_throttling_end(bs);
+ bdrv_no_throttling_end(blk_bs(blk));
return ret;
}
@@ -1524,7 +1527,7 @@ void blk_update_root_state(BlockBackend *blk)
g_free(blk->root_state.throttle_group);
throttle_group_unref(blk->root_state.throttle_state);
}
- if (blk->root->bs->throttle_state) {
+ if (blk->public.throttle_state) {
const char *name = throttle_group_get_name(blk);
blk->root_state.throttle_group = g_strdup(name);
blk->root_state.throttle_state = throttle_group_incref(name);
diff --git a/block/io.c b/block/io.c
index f6fb868..bdbaa1c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -55,20 +55,31 @@ void bdrv_set_io_limits(BlockDriverState *bs,
void bdrv_no_throttling_begin(BlockDriverState *bs)
{
- if (bs->io_limits_disabled++ == 0) {
- throttle_group_restart_bs(bs);
+ if (!bs->blk) {
+ return;
+ }
+
+ if (blk_get_public(bs->blk)->io_limits_disabled++ == 0) {
+ throttle_group_restart_blk(bs->blk);
}
}
void bdrv_no_throttling_end(BlockDriverState *bs)
{
- assert(bs->io_limits_disabled);
- --bs->io_limits_disabled;
+ BlockBackendPublic *blkp;
+
+ if (!bs->blk) {
+ return;
+ }
+
+ blkp = blk_get_public(bs->blk);
+ assert(blkp->io_limits_disabled);
+ --blkp->io_limits_disabled;
}
void bdrv_io_limits_disable(BlockDriverState *bs)
{
- assert(bs->throttle_state);
+ assert(blk_get_public(bs->blk)->throttle_state);
bdrv_no_throttling_begin(bs);
throttle_group_unregister_blk(bs->blk);
bdrv_no_throttling_end(bs);
@@ -77,14 +88,16 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
/* should be called before bdrv_set_io_limits if a limit is set */
void bdrv_io_limits_enable(BlockDriverState *bs, const char *group)
{
- assert(!bs->throttle_state);
+ BlockBackendPublic *blkp = blk_get_public(bs->blk);
+
+ assert(!blkp->throttle_state);
throttle_group_register_blk(bs->blk, group);
}
void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
{
/* this bs is not part of any group */
- if (!bs->throttle_state) {
+ if (!blk_get_public(bs->blk)->throttle_state) {
return;
}
@@ -178,14 +191,15 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
bool bdrv_requests_pending(BlockDriverState *bs)
{
BdrvChild *child;
+ BlockBackendPublic *blkp = bs->blk ? blk_get_public(bs->blk) : NULL;
if (!QLIST_EMPTY(&bs->tracked_requests)) {
return true;
}
- if (!qemu_co_queue_empty(&bs->throttled_reqs[0])) {
+ if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[0])) {
return true;
}
- if (!qemu_co_queue_empty(&bs->throttled_reqs[1])) {
+ if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[1])) {
return true;
}
@@ -1070,7 +1084,7 @@ int coroutine_fn bdrv_co_preadv(BlockDriverState *bs,
}
/* throttling disk I/O */
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
throttle_group_co_io_limits_intercept(bs, bytes, false);
}
@@ -1431,7 +1445,7 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs,
}
/* throttling disk I/O */
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
throttle_group_co_io_limits_intercept(bs, bytes, true);
}
diff --git a/block/qapi.c b/block/qapi.c
index a3e514d..1e4bb8a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -67,7 +67,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
info->backing_file_depth = bdrv_get_backing_file_depth(bs);
info->detect_zeroes = bs->detect_zeroes;
- if (bs->throttle_state) {
+ if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
ThrottleConfig cfg;
throttle_group_get_config(bs, &cfg);
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index e50ccaa..56dc311 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -30,7 +30,7 @@
#include "sysemu/qtest.h"
/* The ThrottleGroup structure (with its ThrottleState) is shared
- * among different BlockDriverState and it's independent from
+ * among different BlockBackends and it's independent from
* AioContext, so in order to use it from different threads it needs
* its own locking.
*
@@ -40,18 +40,18 @@
* The whole ThrottleGroup structure is private and invisible to
* outside users, that only use it through its ThrottleState.
*
- * In addition to the ThrottleGroup structure, BlockDriverState has
+ * In addition to the ThrottleGroup structure, BlockBackendPublic has
* fields that need to be accessed by other members of the group and
- * therefore also need to be protected by this lock. Once a BDS is
- * registered in a group those fields can be accessed by other threads
- * any time.
+ * therefore also need to be protected by this lock. Once a
+ * BlockBackend is registered in a group those fields can be accessed
+ * by other threads any time.
*
* Again, all this is handled internally and is mostly transparent to
* the outside. The 'throttle_timers' field however has an additional
* constraint because it may be temporarily invalid (see for example
* bdrv_set_aio_context()). Therefore in this file a thread will
- * access some other BDS's timers only after verifying that that BDS
- * has throttled requests in the queue.
+ * access some other BlockBackend's timers only after verifying that
+ * that BlockBackend has throttled requests in the queue.
*/
typedef struct ThrottleGroup {
char *name; /* This is constant during the lifetime of the group */
@@ -141,8 +141,8 @@ void throttle_group_unref(ThrottleState *ts)
*/
const char *throttle_group_get_name(BlockBackend *blk)
{
- ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
- ThrottleGroup, ts);
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
return tg->name;
}
@@ -156,10 +156,10 @@ const char *throttle_group_get_name(BlockBackend *blk)
*/
static BlockBackend *throttle_group_next_blk(BlockBackend *blk)
{
- BlockDriverState *bs = blk_bs(blk);
- ThrottleState *ts = bs->throttle_state;
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
- BlockBackendPublic *next = QLIST_NEXT(blk_get_public(blk), round_robin);
+ BlockBackendPublic *next = QLIST_NEXT(blkp, round_robin);
if (!next) {
next = QLIST_FIRST(&tg->head);
@@ -180,15 +180,15 @@ static BlockBackend *throttle_group_next_blk(BlockBackend *blk)
*/
static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write)
{
- ThrottleGroup *tg = container_of(blk_bs(blk)->throttle_state,
- ThrottleGroup, ts);
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
BlockBackend *token, *start;
start = token = tg->tokens[is_write];
/* get next bs round in round robin style */
token = throttle_group_next_blk(token);
- while (token != start && !blk_bs(token)->pending_reqs[is_write]) {
+ while (token != start && !blkp->pending_reqs[is_write]) {
token = throttle_group_next_blk(token);
}
@@ -196,7 +196,7 @@ static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write)
* then decide the token is the current bs because chances are
* the current bs get the current request queued.
*/
- if (token == start && !blk_bs(token)->pending_reqs[is_write]) {
+ if (token == start && !blkp->pending_reqs[is_write]) {
token = blk;
}
@@ -215,12 +215,13 @@ static BlockBackend *next_throttle_token(BlockBackend *blk, bool is_write)
*/
static bool throttle_group_schedule_timer(BlockBackend *blk, bool is_write)
{
- ThrottleState *ts = blk_bs(blk)->throttle_state;
- ThrottleTimers *tt = &blk_bs(blk)->throttle_timers;
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleState *ts = blkp->throttle_state;
+ ThrottleTimers *tt = &blkp->throttle_timers;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
bool must_wait;
- if (blk_bs(blk)->io_limits_disabled) {
+ if (blkp->io_limits_disabled) {
return false;
}
@@ -249,14 +250,14 @@ static bool throttle_group_schedule_timer(BlockBackend *blk, bool is_write)
*/
static void schedule_next_request(BlockBackend *blk, bool is_write)
{
- BlockDriverState *bs = blk_bs(blk);
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
bool must_wait;
BlockBackend *token;
/* Check if there's any pending request to schedule next */
token = next_throttle_token(blk, is_write);
- if (!blk_bs(token)->pending_reqs[is_write]) {
+ if (!blkp->pending_reqs[is_write]) {
return;
}
@@ -265,12 +266,12 @@ static void schedule_next_request(BlockBackend *blk, bool is_write)
/* If it doesn't have to wait, queue it for immediate execution */
if (!must_wait) {
- /* Give preference to requests from the current bs */
+ /* Give preference to requests from the current blk */
if (qemu_in_coroutine() &&
- qemu_co_queue_next(&bs->throttled_reqs[is_write])) {
+ qemu_co_queue_next(&blkp->throttled_reqs[is_write])) {
token = blk;
} else {
- ThrottleTimers *tt = &blk_bs(token)->throttle_timers;
+ ThrottleTimers *tt = &blkp->throttle_timers;
int64_t now = qemu_clock_get_ns(tt->clock_type);
timer_mod(tt->timers[is_write], now + 1);
tg->any_timer_armed[is_write] = true;
@@ -294,37 +295,40 @@ void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
bool must_wait;
BlockBackend *token;
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
+ BlockBackend *blk = bs->blk;
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
/* First we check if this I/O has to be throttled. */
- token = next_throttle_token(bs->blk, is_write);
+ token = next_throttle_token(blk, is_write);
must_wait = throttle_group_schedule_timer(token, is_write);
/* Wait if there's a timer set or queued requests of this type */
- if (must_wait || bs->pending_reqs[is_write]) {
- bs->pending_reqs[is_write]++;
+ if (must_wait || blkp->pending_reqs[is_write]) {
+ blkp->pending_reqs[is_write]++;
qemu_mutex_unlock(&tg->lock);
- qemu_co_queue_wait(&bs->throttled_reqs[is_write]);
+ qemu_co_queue_wait(&blkp->throttled_reqs[is_write]);
qemu_mutex_lock(&tg->lock);
- bs->pending_reqs[is_write]--;
+ blkp->pending_reqs[is_write]--;
}
/* The I/O will be executed, so do the accounting */
- throttle_account(bs->throttle_state, is_write, bytes);
+ throttle_account(blkp->throttle_state, is_write, bytes);
/* Schedule the next request */
- schedule_next_request(bs->blk, is_write);
+ schedule_next_request(blk, is_write);
qemu_mutex_unlock(&tg->lock);
}
-void throttle_group_restart_bs(BlockDriverState *bs)
+void throttle_group_restart_blk(BlockBackend *blk)
{
+ BlockBackendPublic *blkp = blk_get_public(blk);
int i;
for (i = 0; i < 2; i++) {
- while (qemu_co_enter_next(&bs->throttled_reqs[i])) {
+ while (qemu_co_enter_next(&blkp->throttled_reqs[i])) {
;
}
}
@@ -339,8 +343,9 @@ void throttle_group_restart_bs(BlockDriverState *bs)
*/
void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
{
- ThrottleTimers *tt = &bs->throttle_timers;
- ThrottleState *ts = bs->throttle_state;
+ BlockBackendPublic *blkp = blk_get_public(bs->blk);
+ ThrottleTimers *tt = &blkp->throttle_timers;
+ ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
/* throttle_config() cancels the timers */
@@ -353,8 +358,8 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
throttle_config(ts, tt, cfg);
qemu_mutex_unlock(&tg->lock);
- qemu_co_enter_next(&bs->throttled_reqs[0]);
- qemu_co_enter_next(&bs->throttled_reqs[1]);
+ qemu_co_enter_next(&blkp->throttled_reqs[0]);
+ qemu_co_enter_next(&blkp->throttled_reqs[1]);
}
/* Get the throttle configuration from a particular group. Similar to
@@ -366,7 +371,8 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
*/
void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg)
{
- ThrottleState *ts = bs->throttle_state;
+ BlockBackendPublic *blkp = blk_get_public(bs->blk);
+ ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
throttle_get_config(ts, cfg);
@@ -376,12 +382,13 @@ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg)
/* ThrottleTimers callback. This wakes up a request that was waiting
* because it had been throttled.
*
- * @bs: the BlockDriverState whose request had been throttled
+ * @blk: the BlockBackend whose request had been throttled
* @is_write: the type of operation (read/write)
*/
-static void timer_cb(BlockDriverState *bs, bool is_write)
+static void timer_cb(BlockBackend *blk, bool is_write)
{
- ThrottleState *ts = bs->throttle_state;
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
bool empty_queue;
@@ -391,13 +398,13 @@ static void timer_cb(BlockDriverState *bs, bool is_write)
qemu_mutex_unlock(&tg->lock);
/* Run the request that was waiting for this timer */
- empty_queue = !qemu_co_enter_next(&bs->throttled_reqs[is_write]);
+ empty_queue = !qemu_co_enter_next(&blkp->throttled_reqs[is_write]);
/* If the request queue was empty then we have to take care of
* scheduling the next one */
if (empty_queue) {
qemu_mutex_lock(&tg->lock);
- schedule_next_request(bs->blk, is_write);
+ schedule_next_request(blk, is_write);
qemu_mutex_unlock(&tg->lock);
}
}
@@ -422,7 +429,7 @@ static void write_timer_cb(void *opaque)
void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
{
int i;
- BlockDriverState *bs = blk_bs(blk);
+ BlockBackendPublic *blkp = blk_get_public(blk);
ThrottleState *ts = throttle_group_incref(groupname);
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
int clock_type = QEMU_CLOCK_REALTIME;
@@ -432,7 +439,7 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
clock_type = QEMU_CLOCK_VIRTUAL;
}
- bs->throttle_state = ts;
+ blkp->throttle_state = ts;
qemu_mutex_lock(&tg->lock);
/* If the ThrottleGroup is new set this BlockBackend as the token */
@@ -442,14 +449,14 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
}
}
- QLIST_INSERT_HEAD(&tg->head, blk_get_public(blk), round_robin);
+ QLIST_INSERT_HEAD(&tg->head, blkp, round_robin);
- throttle_timers_init(&bs->throttle_timers,
- bdrv_get_aio_context(bs),
+ throttle_timers_init(&blkp->throttle_timers,
+ blk_get_aio_context(blk),
clock_type,
read_timer_cb,
write_timer_cb,
- bs);
+ blk);
qemu_mutex_unlock(&tg->lock);
}
@@ -466,19 +473,19 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname)
*/
void throttle_group_unregister_blk(BlockBackend *blk)
{
- BlockDriverState *bs = blk_bs(blk);
- ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
+ BlockBackendPublic *blkp = blk_get_public(blk);
+ ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
int i;
- assert(bs->pending_reqs[0] == 0 && bs->pending_reqs[1] == 0);
- assert(qemu_co_queue_empty(&bs->throttled_reqs[0]));
- assert(qemu_co_queue_empty(&bs->throttled_reqs[1]));
+ assert(blkp->pending_reqs[0] == 0 && blkp->pending_reqs[1] == 0);
+ assert(qemu_co_queue_empty(&blkp->throttled_reqs[0]));
+ assert(qemu_co_queue_empty(&blkp->throttled_reqs[1]));
qemu_mutex_lock(&tg->lock);
for (i = 0; i < 2; i++) {
if (tg->tokens[i] == blk) {
BlockBackend *token = throttle_group_next_blk(blk);
- /* Take care of the case where this is the last bs in the group */
+ /* Take care of the case where this is the last blk in the group */
if (token == blk) {
token = NULL;
}
@@ -486,13 +493,13 @@ void throttle_group_unregister_blk(BlockBackend *blk)
}
}
- /* remove the current bs from the list */
- QLIST_REMOVE(blk_get_public(blk), round_robin);
- throttle_timers_destroy(&bs->throttle_timers);
+ /* remove the current blk from the list */
+ QLIST_REMOVE(blkp, round_robin);
+ throttle_timers_destroy(&blkp->throttle_timers);
qemu_mutex_unlock(&tg->lock);
throttle_group_unref(&tg->ts);
- bs->throttle_state = NULL;
+ blkp->throttle_state = NULL;
}
static void throttle_groups_init(void)
diff --git a/blockdev.c b/blockdev.c
index 8106ca7..3211a40 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2726,14 +2726,14 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
if (throttle_enabled(&cfg)) {
/* Enable I/O limits if they're not enabled yet, otherwise
* just update the throttling group. */
- if (!bs->throttle_state) {
+ if (!blk_get_public(bs->blk)->throttle_state) {
bdrv_io_limits_enable(bs, has_group ? group : device);
} else if (has_group) {
bdrv_io_limits_update_group(bs, group);
}
/* Set the new throttling configuration */
bdrv_set_io_limits(bs, &cfg);
- } else if (bs->throttle_state) {
+ } else if (blk_get_public(bs->blk)->throttle_state) {
/* If all throttling settings are set to 0, disable I/O limits */
bdrv_io_limits_disable(bs);
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3f5d2b1..2bbc2c0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -26,7 +26,6 @@
#include "block/accounting.h"
#include "block/block.h"
-#include "block/throttle-groups.h"
#include "qemu/option.h"
#include "qemu/queue.h"
#include "qemu/coroutine.h"
@@ -424,18 +423,6 @@ struct BlockDriverState {
/* number of in-flight serialising requests */
unsigned int serialising_in_flight;
- /* I/O throttling.
- * throttle_state tells us if this BDS has I/O limits configured.
- * io_limits_disabled tells us if they are currently being enforced */
- CoQueue throttled_reqs[2];
- unsigned int io_limits_disabled;
-
- /* The following fields are protected by the ThrottleGroup lock.
- * See the ThrottleGroup documentation for details. */
- ThrottleState *throttle_state;
- ThrottleTimers throttle_timers;
- unsigned pending_reqs[2];
-
/* Offset after the highest byte written to */
uint64_t wr_highest_offset;
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index bd55a34..840ba44 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -38,7 +38,7 @@ void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg);
void throttle_group_register_blk(BlockBackend *blk, const char *groupname);
void throttle_group_unregister_blk(BlockBackend *blk);
-void throttle_group_restart_bs(BlockDriverState *bs);
+void throttle_group_restart_blk(BlockBackend *blk);
void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
unsigned int bytes,
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 1dcd70e..08d27a8 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -14,6 +14,7 @@
#define BLOCK_BACKEND_H
#include "qemu/iov.h"
+#include "block/throttle-groups.h"
/*
* TODO Have to include block/block.h for a bunch of block layer
@@ -63,9 +64,17 @@ typedef struct BlockDevOps {
* fields that must be public. This is in particular for QLIST_ENTRY() and
* friends so that BlockBackends can be kept in lists outside block-backend.c */
typedef struct BlockBackendPublic {
- /* I/O throttling */
+ /* I/O throttling.
+ * throttle_state tells us if this BlockBackend has I/O limits configured.
+ * io_limits_disabled tells us if they are currently being enforced */
+ CoQueue throttled_reqs[2];
+ unsigned int io_limits_disabled;
+
/* The following fields are protected by the ThrottleGroup lock.
* See the ThrottleGroup documentation for details. */
+ ThrottleState *throttle_state;
+ ThrottleTimers throttle_timers;
+ unsigned pending_reqs[2];
QLIST_ENTRY(BlockBackendPublic) round_robin;
} BlockBackendPublic;
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 1a322f1..a020068 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -576,31 +576,35 @@ static void test_groups(void)
{
ThrottleConfig cfg1, cfg2;
BlockBackend *blk1, *blk2, *blk3;
- BlockDriverState *bdrv1, *bdrv2, *bdrv3;
+ BlockBackendPublic *blkp1, *blkp2, *blkp3;
+ BlockDriverState *bdrv1, *bdrv3;
blk1 = blk_new_with_bs(&error_abort);
blk2 = blk_new_with_bs(&error_abort);
blk3 = blk_new_with_bs(&error_abort);
bdrv1 = blk_bs(blk1);
- bdrv2 = blk_bs(blk2);
bdrv3 = blk_bs(blk3);
- g_assert(bdrv1->throttle_state == NULL);
- g_assert(bdrv2->throttle_state == NULL);
- g_assert(bdrv3->throttle_state == NULL);
+ blkp1 = blk_get_public(blk1);
+ blkp2 = blk_get_public(blk2);
+ blkp3 = blk_get_public(blk3);
+
+ g_assert(blkp1->throttle_state == NULL);
+ g_assert(blkp2->throttle_state == NULL);
+ g_assert(blkp3->throttle_state == NULL);
throttle_group_register_blk(blk1, "bar");
throttle_group_register_blk(blk2, "foo");
throttle_group_register_blk(blk3, "bar");
- g_assert(bdrv1->throttle_state != NULL);
- g_assert(bdrv2->throttle_state != NULL);
- g_assert(bdrv3->throttle_state != NULL);
+ g_assert(blkp1->throttle_state != NULL);
+ g_assert(blkp2->throttle_state != NULL);
+ g_assert(blkp3->throttle_state != NULL);
g_assert(!strcmp(throttle_group_get_name(blk1), "bar"));
g_assert(!strcmp(throttle_group_get_name(blk2), "foo"));
- g_assert(bdrv1->throttle_state == bdrv3->throttle_state);
+ g_assert(blkp1->throttle_state == blkp3->throttle_state);
/* Setting the config of a group member affects the whole group */
throttle_config_init(&cfg1);
@@ -628,9 +632,9 @@ static void test_groups(void)
throttle_group_unregister_blk(blk2);
throttle_group_unregister_blk(blk3);
- g_assert(bdrv1->throttle_state == NULL);
- g_assert(bdrv2->throttle_state == NULL);
- g_assert(bdrv3->throttle_state == NULL);
+ g_assert(blkp1->throttle_state == NULL);
+ g_assert(blkp2->throttle_state == NULL);
+ g_assert(blkp3->throttle_state == NULL);
}
int main(int argc, char **argv)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 06/31] block: Move actual I/O throttling to BlockBackend
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (4 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 05/31] block: Move throttling fields from BDS to BB Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 07/31] block: Move I/O throttling configuration functions " Kevin Wolf
` (25 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/block-backend.c | 10 ++++++++++
block/io.c | 10 ----------
block/throttle-groups.c | 5 ++---
include/block/throttle-groups.h | 2 +-
4 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 6880659..730b8a9 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -716,6 +716,11 @@ static int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
return ret;
}
+ /* throttling disk I/O */
+ if (blk->public.throttle_state) {
+ throttle_group_co_io_limits_intercept(blk, bytes, false);
+ }
+
return bdrv_co_preadv(blk_bs(blk), offset, bytes, qiov, flags);
}
@@ -730,6 +735,11 @@ static int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
return ret;
}
+ /* throttling disk I/O */
+ if (blk->public.throttle_state) {
+ throttle_group_co_io_limits_intercept(blk, bytes, true);
+ }
+
if (!blk->enable_write_cache) {
flags |= BDRV_REQ_FUA;
}
diff --git a/block/io.c b/block/io.c
index bdbaa1c..cf2ac4c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1083,11 +1083,6 @@ int coroutine_fn bdrv_co_preadv(BlockDriverState *bs,
flags |= BDRV_REQ_COPY_ON_READ;
}
- /* throttling disk I/O */
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- throttle_group_co_io_limits_intercept(bs, bytes, false);
- }
-
/* Align read if necessary by padding qiov */
if (offset & (align - 1)) {
head_buf = qemu_blockalign(bs, align);
@@ -1444,11 +1439,6 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs,
return ret;
}
- /* throttling disk I/O */
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- throttle_group_co_io_limits_intercept(bs, bytes, true);
- }
-
/*
* Align write if necessary by performing a read-modify-write cycle.
* Pad qiov with the read parts and be sure to have a tracked request not
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 56dc311..3db8cf7 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -284,18 +284,17 @@ static void schedule_next_request(BlockBackend *blk, bool is_write)
* if necessary, and schedule the next request using a round robin
* algorithm.
*
- * @bs: the current BlockDriverState
+ * @blk: the current BlockBackend
* @bytes: the number of bytes for this I/O
* @is_write: the type of operation (read/write)
*/
-void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
+void coroutine_fn throttle_group_co_io_limits_intercept(BlockBackend *blk,
unsigned int bytes,
bool is_write)
{
bool must_wait;
BlockBackend *token;
- BlockBackend *blk = bs->blk;
BlockBackendPublic *blkp = blk_get_public(blk);
ThrottleGroup *tg = container_of(blkp->throttle_state, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index 840ba44..ac42248 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -40,7 +40,7 @@ void throttle_group_register_blk(BlockBackend *blk, const char *groupname);
void throttle_group_unregister_blk(BlockBackend *blk);
void throttle_group_restart_blk(BlockBackend *blk);
-void coroutine_fn throttle_group_co_io_limits_intercept(BlockDriverState *bs,
+void coroutine_fn throttle_group_co_io_limits_intercept(BlockBackend *blk,
unsigned int bytes,
bool is_write);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 07/31] block: Move I/O throttling configuration functions to BlockBackend
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (5 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 06/31] block: Move actual I/O throttling to BlockBackend Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 08/31] block: Introduce BdrvChild.opaque Kevin Wolf
` (24 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 2 +-
block/block-backend.c | 43 +++++++++++++++++++++++++++++++++++++++--
block/io.c | 41 ---------------------------------------
block/qapi.c | 2 +-
block/throttle-groups.c | 12 ++++++------
blockdev.c | 16 +++++++--------
include/block/block.h | 4 ----
include/block/block_int.h | 3 +--
include/block/throttle-groups.h | 4 ++--
include/sysemu/block-backend.h | 5 +++++
tests/test-throttle.c | 16 ++++++---------
11 files changed, 71 insertions(+), 77 deletions(-)
diff --git a/block.c b/block.c
index f723060..b5c02c4 100644
--- a/block.c
+++ b/block.c
@@ -2123,7 +2123,7 @@ static void bdrv_close(BlockDriverState *bs)
/* Disable I/O limits and drain all pending throttled requests */
if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- bdrv_io_limits_disable(bs);
+ blk_io_limits_disable(bs->blk);
}
bdrv_drained_begin(bs); /* complete I/O */
diff --git a/block/block-backend.c b/block/block-backend.c
index 730b8a9..b9aaa60 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -442,7 +442,7 @@ void blk_remove_bs(BlockBackend *blk)
blk_update_root_state(blk);
if (blk->public.throttle_state) {
- bdrv_io_limits_disable(blk->root->bs);
+ blk_io_limits_disable(blk);
}
blk->root->bs->blk = NULL;
@@ -1556,7 +1556,7 @@ void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs)
{
bs->detect_zeroes = blk->root_state.detect_zeroes;
if (blk->root_state.throttle_group) {
- bdrv_io_limits_enable(bs, blk->root_state.throttle_group);
+ blk_io_limits_enable(blk, blk->root_state.throttle_group);
}
}
@@ -1620,3 +1620,42 @@ int blk_flush_all(void)
return result;
}
+
+
+/* throttling disk I/O limits */
+void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg)
+{
+ throttle_group_config(blk, cfg);
+}
+
+void blk_io_limits_disable(BlockBackend *blk)
+{
+ assert(blk->public.throttle_state);
+ bdrv_no_throttling_begin(blk_bs(blk));
+ throttle_group_unregister_blk(blk);
+ bdrv_no_throttling_end(blk_bs(blk));
+}
+
+/* should be called before blk_set_io_limits if a limit is set */
+void blk_io_limits_enable(BlockBackend *blk, const char *group)
+{
+ assert(!blk->public.throttle_state);
+ throttle_group_register_blk(blk, group);
+}
+
+void blk_io_limits_update_group(BlockBackend *blk, const char *group)
+{
+ /* this BB is not part of any group */
+ if (!blk->public.throttle_state) {
+ return;
+ }
+
+ /* this BB is a part of the same group than the one we want */
+ if (!g_strcmp0(throttle_group_get_name(blk), group)) {
+ return;
+ }
+
+ /* need to change the group this bs belong to */
+ blk_io_limits_disable(blk);
+ blk_io_limits_enable(blk, group);
+}
diff --git a/block/io.c b/block/io.c
index cf2ac4c..1699f1e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -46,13 +46,6 @@ static void coroutine_fn bdrv_co_do_rw(void *opaque);
static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, BdrvRequestFlags flags);
-/* throttling disk I/O limits */
-void bdrv_set_io_limits(BlockDriverState *bs,
- ThrottleConfig *cfg)
-{
- throttle_group_config(bs, cfg);
-}
-
void bdrv_no_throttling_begin(BlockDriverState *bs)
{
if (!bs->blk) {
@@ -77,40 +70,6 @@ void bdrv_no_throttling_end(BlockDriverState *bs)
--blkp->io_limits_disabled;
}
-void bdrv_io_limits_disable(BlockDriverState *bs)
-{
- assert(blk_get_public(bs->blk)->throttle_state);
- bdrv_no_throttling_begin(bs);
- throttle_group_unregister_blk(bs->blk);
- bdrv_no_throttling_end(bs);
-}
-
-/* should be called before bdrv_set_io_limits if a limit is set */
-void bdrv_io_limits_enable(BlockDriverState *bs, const char *group)
-{
- BlockBackendPublic *blkp = blk_get_public(bs->blk);
-
- assert(!blkp->throttle_state);
- throttle_group_register_blk(bs->blk, group);
-}
-
-void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
-{
- /* this bs is not part of any group */
- if (!blk_get_public(bs->blk)->throttle_state) {
- return;
- }
-
- /* this bs is a part of the same group than the one we want */
- if (!g_strcmp0(throttle_group_get_name(bs->blk), group)) {
- return;
- }
-
- /* need to change the group this bs belong to */
- bdrv_io_limits_disable(bs);
- bdrv_io_limits_enable(bs, group);
-}
-
void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
{
BlockDriver *drv = bs->drv;
diff --git a/block/qapi.c b/block/qapi.c
index 1e4bb8a..b0d8d6b 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -70,7 +70,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
ThrottleConfig cfg;
- throttle_group_get_config(bs, &cfg);
+ throttle_group_get_config(bs->blk, &cfg);
info->bps = cfg.buckets[THROTTLE_BPS_TOTAL].avg;
info->bps_rd = cfg.buckets[THROTTLE_BPS_READ].avg;
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 3db8cf7..59545e2 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -337,12 +337,12 @@ void throttle_group_restart_blk(BlockBackend *blk)
* to throttle_config(), but guarantees atomicity within the
* throttling group.
*
- * @bs: a BlockDriverState that is member of the group
+ * @blk: a BlockBackend that is a member of the group
* @cfg: the configuration to set
*/
-void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
+void throttle_group_config(BlockBackend *blk, ThrottleConfig *cfg)
{
- BlockBackendPublic *blkp = blk_get_public(bs->blk);
+ BlockBackendPublic *blkp = blk_get_public(blk);
ThrottleTimers *tt = &blkp->throttle_timers;
ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
@@ -365,12 +365,12 @@ void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg)
* throttle_get_config(), but guarantees atomicity within the
* throttling group.
*
- * @bs: a BlockDriverState that is member of the group
+ * @blk: a BlockBackend that is a member of the group
* @cfg: the configuration will be written here
*/
-void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg)
+void throttle_group_get_config(BlockBackend *blk, ThrottleConfig *cfg)
{
- BlockBackendPublic *blkp = blk_get_public(bs->blk);
+ BlockBackendPublic *blkp = blk_get_public(blk);
ThrottleState *ts = blkp->throttle_state;
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
qemu_mutex_lock(&tg->lock);
diff --git a/blockdev.c b/blockdev.c
index 3211a40..89768ab 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -616,8 +616,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
if (!throttling_group) {
throttling_group = blk_name(blk);
}
- bdrv_io_limits_enable(bs, throttling_group);
- bdrv_set_io_limits(bs, &cfg);
+ blk_io_limits_enable(blk, throttling_group);
+ blk_set_io_limits(blk, &cfg);
}
if (bdrv_key_required(bs)) {
@@ -2726,16 +2726,16 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
if (throttle_enabled(&cfg)) {
/* Enable I/O limits if they're not enabled yet, otherwise
* just update the throttling group. */
- if (!blk_get_public(bs->blk)->throttle_state) {
- bdrv_io_limits_enable(bs, has_group ? group : device);
+ if (!blk_get_public(blk)->throttle_state) {
+ blk_io_limits_enable(blk, has_group ? group : device);
} else if (has_group) {
- bdrv_io_limits_update_group(bs, group);
+ blk_io_limits_update_group(blk, group);
}
/* Set the new throttling configuration */
- bdrv_set_io_limits(bs, &cfg);
- } else if (blk_get_public(bs->blk)->throttle_state) {
+ blk_set_io_limits(blk, &cfg);
+ } else if (blk_get_public(blk)->throttle_state) {
/* If all throttling settings are set to 0, disable I/O limits */
- bdrv_io_limits_disable(bs);
+ blk_io_limits_disable(blk);
}
out:
diff --git a/include/block/block.h b/include/block/block.h
index b210832..2c5c280 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -187,10 +187,6 @@ void bdrv_stats_print(Monitor *mon, const QObject *data);
void bdrv_info_stats(Monitor *mon, QObject **ret_data);
/* disk I/O throttling */
-void bdrv_io_limits_enable(BlockDriverState *bs, const char *group);
-void bdrv_io_limits_disable(BlockDriverState *bs);
-void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group);
-
void bdrv_init(void);
void bdrv_init_with_whitelist(void);
bool bdrv_uses_whitelist(void);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2bbc2c0..1218857 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -525,8 +525,7 @@ int get_tmp_filename(char *filename, int size);
BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
const char *filename);
-void bdrv_set_io_limits(BlockDriverState *bs,
- ThrottleConfig *cfg);
+bool bdrv_start_throttled_reqs(BlockDriverState *bs);
/**
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index ac42248..d983d34 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -33,8 +33,8 @@ const char *throttle_group_get_name(BlockBackend *blk);
ThrottleState *throttle_group_incref(const char *name);
void throttle_group_unref(ThrottleState *ts);
-void throttle_group_config(BlockDriverState *bs, ThrottleConfig *cfg);
-void throttle_group_get_config(BlockDriverState *bs, ThrottleConfig *cfg);
+void throttle_group_config(BlockBackend *blk, ThrottleConfig *cfg);
+void throttle_group_get_config(BlockBackend *blk, ThrottleConfig *cfg);
void throttle_group_register_blk(BlockBackend *blk, const char *groupname);
void throttle_group_unregister_blk(BlockBackend *blk);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 08d27a8..dd9c8ca 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -212,4 +212,9 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
BlockCompletionFunc *cb,
void *opaque, int ret);
+void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg);
+void blk_io_limits_disable(BlockBackend *blk);
+void blk_io_limits_enable(BlockBackend *blk, const char *group);
+void blk_io_limits_update_group(BlockBackend *blk, const char *group);
+
#endif
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index a020068..5ec966c 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -577,15 +577,11 @@ static void test_groups(void)
ThrottleConfig cfg1, cfg2;
BlockBackend *blk1, *blk2, *blk3;
BlockBackendPublic *blkp1, *blkp2, *blkp3;
- BlockDriverState *bdrv1, *bdrv3;
blk1 = blk_new_with_bs(&error_abort);
blk2 = blk_new_with_bs(&error_abort);
blk3 = blk_new_with_bs(&error_abort);
- bdrv1 = blk_bs(blk1);
- bdrv3 = blk_bs(blk3);
-
blkp1 = blk_get_public(blk1);
blkp2 = blk_get_public(blk2);
blkp3 = blk_get_public(blk3);
@@ -612,20 +608,20 @@ static void test_groups(void)
cfg1.buckets[THROTTLE_BPS_WRITE].avg = 285000;
cfg1.buckets[THROTTLE_OPS_READ].avg = 20000;
cfg1.buckets[THROTTLE_OPS_WRITE].avg = 12000;
- throttle_group_config(bdrv1, &cfg1);
+ throttle_group_config(blk1, &cfg1);
- throttle_group_get_config(bdrv1, &cfg1);
- throttle_group_get_config(bdrv3, &cfg2);
+ throttle_group_get_config(blk1, &cfg1);
+ throttle_group_get_config(blk3, &cfg2);
g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1)));
cfg2.buckets[THROTTLE_BPS_READ].avg = 4547;
cfg2.buckets[THROTTLE_BPS_WRITE].avg = 1349;
cfg2.buckets[THROTTLE_OPS_READ].avg = 123;
cfg2.buckets[THROTTLE_OPS_WRITE].avg = 86;
- throttle_group_config(bdrv3, &cfg1);
+ throttle_group_config(blk3, &cfg1);
- throttle_group_get_config(bdrv1, &cfg1);
- throttle_group_get_config(bdrv3, &cfg2);
+ throttle_group_get_config(blk1, &cfg1);
+ throttle_group_get_config(blk3, &cfg2);
g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1)));
throttle_group_unregister_blk(blk1);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 08/31] block: Introduce BdrvChild.opaque
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (6 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 07/31] block: Move I/O throttling configuration functions " Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 09/31] block: Drain throttling queue with BdrvChild callback Kevin Wolf
` (23 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
BlockBackends use it to get a back pointer from BdrvChild to
BlockBackend in any BdrvChildRole callbacks.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/block-backend.c | 2 ++
include/block/block_int.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index b9aaa60..52b7b92 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -133,6 +133,7 @@ BlockBackend *blk_new_with_bs(Error **errp)
bs = bdrv_new_root();
blk->root = bdrv_root_attach_child(bs, "root", &child_root);
+ blk->root->opaque = blk;
bs->blk = blk;
return blk;
}
@@ -458,6 +459,7 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
assert(!blk->root && !bs->blk);
bdrv_ref(bs);
blk->root = bdrv_root_attach_child(bs, "root", &child_root);
+ blk->root->opaque = blk;
bs->blk = blk;
notifier_list_notify(&blk->insert_bs_notifiers, blk);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1218857..47dfafa 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -373,6 +373,7 @@ struct BdrvChild {
BlockDriverState *bs;
char *name;
const BdrvChildRole *role;
+ void *opaque;
QLIST_ENTRY(BdrvChild) next;
QLIST_ENTRY(BdrvChild) next_parent;
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 09/31] block: Drain throttling queue with BdrvChild callback
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (7 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 08/31] block: Introduce BdrvChild.opaque Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 10/31] block/io: Quiesce parents between drained_begin/end Kevin Wolf
` (22 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
This removes the last part of I/O throttling from block/io.c and moves
it to the BlockBackend.
Instead of having knowledge about throttling inside io.c, we can call a
BdrvChild callback .drained_begin/end, which happens to drain the
throttled requests for BlockBackend parents.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/block-backend.c | 32 +++++++++++++++++++++++++++-----
block/io.c | 39 ++++++++++++++++++---------------------
include/block/block_int.h | 16 +++++++++++-----
3 files changed, 56 insertions(+), 31 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 52b7b92..74429ae 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -91,9 +91,14 @@ static void blk_root_inherit_options(int *child_flags, QDict *child_options,
/* We're not supposed to call this function for root nodes */
abort();
}
+static void blk_root_drained_begin(BdrvChild *child);
+static void blk_root_drained_end(BdrvChild *child);
static const BdrvChildRole child_root = {
- .inherit_options = blk_root_inherit_options,
+ .inherit_options = blk_root_inherit_options,
+
+ .drained_begin = blk_root_drained_begin,
+ .drained_end = blk_root_drained_end,
};
/*
@@ -818,9 +823,9 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
return ret;
}
- bdrv_no_throttling_begin(blk_bs(blk));
+ blk_root_drained_begin(blk->root);
ret = blk_pread(blk, offset, buf, count);
- bdrv_no_throttling_end(blk_bs(blk));
+ blk_root_drained_end(blk->root);
return ret;
}
@@ -1633,9 +1638,9 @@ void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg)
void blk_io_limits_disable(BlockBackend *blk)
{
assert(blk->public.throttle_state);
- bdrv_no_throttling_begin(blk_bs(blk));
+ bdrv_drained_begin(blk_bs(blk));
throttle_group_unregister_blk(blk);
- bdrv_no_throttling_end(blk_bs(blk));
+ bdrv_drained_end(blk_bs(blk));
}
/* should be called before blk_set_io_limits if a limit is set */
@@ -1661,3 +1666,20 @@ void blk_io_limits_update_group(BlockBackend *blk, const char *group)
blk_io_limits_disable(blk);
blk_io_limits_enable(blk, group);
}
+
+static void blk_root_drained_begin(BdrvChild *child)
+{
+ BlockBackend *blk = child->opaque;
+
+ if (blk->public.io_limits_disabled++ == 0) {
+ throttle_group_restart_blk(blk);
+ }
+}
+
+static void blk_root_drained_end(BdrvChild *child)
+{
+ BlockBackend *blk = child->opaque;
+
+ assert(blk->public.io_limits_disabled);
+ --blk->public.io_limits_disabled;
+}
diff --git a/block/io.c b/block/io.c
index 1699f1e..7c213ec 100644
--- a/block/io.c
+++ b/block/io.c
@@ -27,7 +27,6 @@
#include "sysemu/block-backend.h"
#include "block/blockjob.h"
#include "block/block_int.h"
-#include "block/throttle-groups.h"
#include "qemu/cutils.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
@@ -46,28 +45,26 @@ static void coroutine_fn bdrv_co_do_rw(void *opaque);
static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, BdrvRequestFlags flags);
-void bdrv_no_throttling_begin(BlockDriverState *bs)
+static void bdrv_parent_drained_begin(BlockDriverState *bs)
{
- if (!bs->blk) {
- return;
- }
+ BdrvChild *c;
- if (blk_get_public(bs->blk)->io_limits_disabled++ == 0) {
- throttle_group_restart_blk(bs->blk);
+ QLIST_FOREACH(c, &bs->parents, next_parent) {
+ if (c->role->drained_begin) {
+ c->role->drained_begin(c);
+ }
}
}
-void bdrv_no_throttling_end(BlockDriverState *bs)
+static void bdrv_parent_drained_end(BlockDriverState *bs)
{
- BlockBackendPublic *blkp;
+ BdrvChild *c;
- if (!bs->blk) {
- return;
+ QLIST_FOREACH(c, &bs->parents, next_parent) {
+ if (c->role->drained_end) {
+ c->role->drained_end(c);
+ }
}
-
- blkp = blk_get_public(bs->blk);
- assert(blkp->io_limits_disabled);
- --blkp->io_limits_disabled;
}
void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
@@ -248,17 +245,17 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
*/
void coroutine_fn bdrv_co_drain(BlockDriverState *bs)
{
- bdrv_no_throttling_begin(bs);
+ bdrv_parent_drained_begin(bs);
bdrv_io_unplugged_begin(bs);
bdrv_drain_recurse(bs);
bdrv_co_yield_to_drain(bs);
bdrv_io_unplugged_end(bs);
- bdrv_no_throttling_end(bs);
+ bdrv_parent_drained_end(bs);
}
void bdrv_drain(BlockDriverState *bs)
{
- bdrv_no_throttling_begin(bs);
+ bdrv_parent_drained_begin(bs);
bdrv_io_unplugged_begin(bs);
bdrv_drain_recurse(bs);
if (qemu_in_coroutine()) {
@@ -267,7 +264,7 @@ void bdrv_drain(BlockDriverState *bs)
bdrv_drain_poll(bs);
}
bdrv_io_unplugged_end(bs);
- bdrv_no_throttling_end(bs);
+ bdrv_parent_drained_end(bs);
}
/*
@@ -290,7 +287,7 @@ void bdrv_drain_all(void)
if (bs->job) {
block_job_pause(bs->job);
}
- bdrv_no_throttling_begin(bs);
+ bdrv_parent_drained_begin(bs);
bdrv_io_unplugged_begin(bs);
bdrv_drain_recurse(bs);
aio_context_release(aio_context);
@@ -333,7 +330,7 @@ void bdrv_drain_all(void)
aio_context_acquire(aio_context);
bdrv_io_unplugged_end(bs);
- bdrv_no_throttling_end(bs);
+ bdrv_parent_drained_end(bs);
if (bs->job) {
block_job_resume(bs->job);
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 47dfafa..a1f8488 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -364,6 +364,17 @@ typedef struct BdrvAioNotifier {
struct BdrvChildRole {
void (*inherit_options)(int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options);
+
+ /*
+ * If this pair of functions is implemented, the parent doesn't issue new
+ * requests after returning from .drained_begin() until .drained_end() is
+ * called.
+ *
+ * Note that this can be nested. If drained_begin() was called twice, new
+ * I/O is allowed only after drained_end() was called twice, too.
+ */
+ void (*drained_begin)(BdrvChild *child);
+ void (*drained_end)(BdrvChild *child);
};
extern const BdrvChildRole child_file;
@@ -526,8 +537,6 @@ int get_tmp_filename(char *filename, int size);
BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
const char *filename);
-bool bdrv_start_throttled_reqs(BlockDriverState *bs);
-
/**
* bdrv_add_before_write_notifier:
@@ -710,9 +719,6 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
const BdrvChildRole *child_role);
void bdrv_root_unref_child(BdrvChild *child);
-void bdrv_no_throttling_begin(BlockDriverState *bs);
-void bdrv_no_throttling_end(BlockDriverState *bs);
-
void blk_dev_change_media_cb(BlockBackend *blk, bool load);
bool blk_dev_has_removable_media(BlockBackend *blk);
bool blk_dev_has_tray(BlockBackend *blk);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 10/31] block/io: Quiesce parents between drained_begin/end
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (8 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 09/31] block: Drain throttling queue with BdrvChild callback Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 11/31] block: Decouple throttling from BlockDriverState Kevin Wolf
` (21 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
So far, bdrv_parent_drained_begin/end() was called for the duration of
the actual bdrv_drain() at the beginning of a drained section, but we
really should keep parents quiesced until the end of the drained
section.
This does not actually change behaviour at this point because the only
user of the .drained_begin/end BdrvChildRole callback is I/O throttling,
which already doesn't send any new requests after flushing its queue in
.drained_begin. The patch merely removes a trap for future users.
Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/io.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/block/io.c b/block/io.c
index 7c213ec..23abbc5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2749,11 +2749,14 @@ void bdrv_drained_begin(BlockDriverState *bs)
if (!bs->quiesce_counter++) {
aio_disable_external(bdrv_get_aio_context(bs));
}
+ bdrv_parent_drained_begin(bs);
bdrv_drain(bs);
}
void bdrv_drained_end(BlockDriverState *bs)
{
+ bdrv_parent_drained_end(bs);
+
assert(bs->quiesce_counter > 0);
if (--bs->quiesce_counter > 0) {
return;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 11/31] block: Decouple throttling from BlockDriverState
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (9 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 10/31] block/io: Quiesce parents between drained_begin/end Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 12/31] block: Remove bdrv_move_feature_fields() Kevin Wolf
` (20 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
This moves the throttling related part of the BDS life cycle management
to BlockBackend. The throttling group reference is now kept even when no
medium is inserted.
With this commit, throttling isn't disabled and then re-enabled any more
during graph reconfiguration. This fixes the temporary breakage of I/O
throttling when used with live snapshots or block jobs that manipulate
the graph.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 34 ----------------------------------
block/block-backend.c | 37 ++++++++++++++-----------------------
blockdev.c | 27 +++++++++------------------
include/block/block_int.h | 3 ---
4 files changed, 23 insertions(+), 78 deletions(-)
diff --git a/block.c b/block.c
index b5c02c4..98226fe 100644
--- a/block.c
+++ b/block.c
@@ -38,7 +38,6 @@
#include "qmp-commands.h"
#include "qemu/timer.h"
#include "qapi-event.h"
-#include "block/throttle-groups.h"
#include "qemu/cutils.h"
#include "qemu/id.h"
@@ -2121,11 +2120,6 @@ static void bdrv_close(BlockDriverState *bs)
assert(!bs->job);
- /* Disable I/O limits and drain all pending throttled requests */
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- blk_io_limits_disable(bs->blk);
- }
-
bdrv_drained_begin(bs); /* complete I/O */
bdrv_flush(bs);
bdrv_drain(bs); /* in case flush left pending I/O */
@@ -2254,26 +2248,6 @@ static void swap_feature_fields(BlockDriverState *bs_top,
bdrv_move_feature_fields(&tmp, bs_top);
bdrv_move_feature_fields(bs_top, bs_new);
bdrv_move_feature_fields(bs_new, &tmp);
-
- assert(!bs_new->blk);
- if (bs_top->blk && blk_get_public(bs_top->blk)->throttle_state) {
- /*
- * FIXME Need to break I/O throttling with graph manipulations
- * temporarily because of conflicting invariants (3. will go away when
- * throttling is fully converted to work on BlockBackends):
- *
- * 1. Every BlockBackend has a single root BDS
- * 2. I/O throttling functions require an attached BlockBackend
- * 3. We need to first enable throttling on the new BDS and then
- * disable it on the old one (because of throttle group refcounts)
- */
-#if 0
- bdrv_io_limits_enable(bs_new, throttle_group_get_name(bs_top));
- bdrv_io_limits_disable(bs_top);
-#else
- abort();
-#endif
- }
}
/*
@@ -3674,10 +3648,6 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
baf->detach_aio_context(baf->opaque);
}
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- throttle_timers_detach_aio_context(
- &blk_get_public(bs->blk)->throttle_timers);
- }
if (bs->drv->bdrv_detach_aio_context) {
bs->drv->bdrv_detach_aio_context(bs);
}
@@ -3711,10 +3681,6 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
if (bs->drv->bdrv_attach_aio_context) {
bs->drv->bdrv_attach_aio_context(bs, new_context);
}
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- throttle_timers_attach_aio_context(
- &blk_get_public(bs->blk)->throttle_timers, new_context);
- }
QLIST_FOREACH(ban, &bs->aio_notifiers, list) {
ban->attached_aio_context(new_context, ban->opaque);
diff --git a/block/block-backend.c b/block/block-backend.c
index 74429ae..a71b54d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -188,10 +188,6 @@ static void blk_delete(BlockBackend *blk)
}
assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers));
assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers));
- if (blk->root_state.throttle_state) {
- g_free(blk->root_state.throttle_group);
- throttle_group_unref(blk->root_state.throttle_state);
- }
QTAILQ_REMOVE(&block_backends, blk, link);
drive_info_del(blk->legacy_dinfo);
block_acct_cleanup(&blk->stats);
@@ -445,12 +441,12 @@ void blk_remove_bs(BlockBackend *blk)
assert(blk->root->bs->blk == blk);
notifier_list_notify(&blk->remove_bs_notifiers, blk);
-
- blk_update_root_state(blk);
if (blk->public.throttle_state) {
- blk_io_limits_disable(blk);
+ throttle_timers_detach_aio_context(&blk->public.throttle_timers);
}
+ blk_update_root_state(blk);
+
blk->root->bs->blk = NULL;
bdrv_root_unref_child(blk->root);
blk->root = NULL;
@@ -468,6 +464,10 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
bs->blk = blk;
notifier_list_notify(&blk->insert_bs_notifiers, blk);
+ if (blk->public.throttle_state) {
+ throttle_timers_attach_aio_context(
+ &blk->public.throttle_timers, bdrv_get_aio_context(bs));
+ }
}
/*
@@ -1374,7 +1374,14 @@ void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
BlockDriverState *bs = blk_bs(blk);
if (bs) {
+ if (blk->public.throttle_state) {
+ throttle_timers_detach_aio_context(&blk->public.throttle_timers);
+ }
bdrv_set_aio_context(bs, new_context);
+ if (blk->public.throttle_state) {
+ throttle_timers_attach_aio_context(&blk->public.throttle_timers,
+ new_context);
+ }
}
}
@@ -1539,19 +1546,6 @@ void blk_update_root_state(BlockBackend *blk)
blk->root_state.open_flags = blk->root->bs->open_flags;
blk->root_state.read_only = blk->root->bs->read_only;
blk->root_state.detect_zeroes = blk->root->bs->detect_zeroes;
-
- if (blk->root_state.throttle_group) {
- g_free(blk->root_state.throttle_group);
- throttle_group_unref(blk->root_state.throttle_state);
- }
- if (blk->public.throttle_state) {
- const char *name = throttle_group_get_name(blk);
- blk->root_state.throttle_group = g_strdup(name);
- blk->root_state.throttle_state = throttle_group_incref(name);
- } else {
- blk->root_state.throttle_group = NULL;
- blk->root_state.throttle_state = NULL;
- }
}
/*
@@ -1562,9 +1556,6 @@ void blk_update_root_state(BlockBackend *blk)
void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs)
{
bs->detect_zeroes = blk->root_state.detect_zeroes;
- if (blk->root_state.throttle_group) {
- blk_io_limits_enable(blk, blk->root_state.throttle_group);
- }
}
/*
diff --git a/blockdev.c b/blockdev.c
index 89768ab..93ddc27 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -577,15 +577,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
blk_rs->read_only = !(bdrv_flags & BDRV_O_RDWR);
blk_rs->detect_zeroes = detect_zeroes;
- if (throttle_enabled(&cfg)) {
- if (!throttling_group) {
- throttling_group = blk_name(blk);
- }
- blk_rs->throttle_group = g_strdup(throttling_group);
- blk_rs->throttle_state = throttle_group_incref(throttling_group);
- blk_rs->throttle_state->cfg = cfg;
- }
-
QDECREF(bs_opts);
} else {
if (file && !*file) {
@@ -611,15 +602,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
bs->detect_zeroes = detect_zeroes;
- /* disk I/O throttling */
- if (throttle_enabled(&cfg)) {
- if (!throttling_group) {
- throttling_group = blk_name(blk);
- }
- blk_io_limits_enable(blk, throttling_group);
- blk_set_io_limits(blk, &cfg);
- }
-
if (bdrv_key_required(bs)) {
autostart = 0;
}
@@ -633,6 +615,15 @@ 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);
+ }
+ blk_io_limits_enable(blk, throttling_group);
+ blk_set_io_limits(blk, &cfg);
+ }
+
blk_set_enable_write_cache(blk, !writethrough);
blk_set_on_error(blk, on_read_error, on_write_error);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a1f8488..db2b4bc 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -500,9 +500,6 @@ struct BlockBackendRootState {
int open_flags;
bool read_only;
BlockdevDetectZeroesOptions detect_zeroes;
-
- char *throttle_group;
- ThrottleState *throttle_state;
};
static inline BlockDriverState *backing_bs(BlockDriverState *bs)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 12/31] block: Remove bdrv_move_feature_fields()
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (10 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 11/31] block: Decouple throttling from BlockDriverState Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 13/31] Revert "block: Forbid I/O throttling on nodes with multiple parents for 2.6" Kevin Wolf
` (19 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
bdrv_move_feature_fields() and swap_feature_fields() are empty now, they
can be removed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 30 ------------------------------
1 file changed, 30 deletions(-)
diff --git a/block.c b/block.c
index 98226fe..39f14c7 100644
--- a/block.c
+++ b/block.c
@@ -2210,13 +2210,6 @@ void bdrv_close_all(void)
}
}
-/* Fields that need to stay with the top-level BDS */
-static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
- BlockDriverState *bs_src)
-{
- /* move some fields that need to stay attached to the device */
-}
-
static void change_parent_backing_link(BlockDriverState *from,
BlockDriverState *to)
{
@@ -2240,16 +2233,6 @@ static void change_parent_backing_link(BlockDriverState *from,
}
}
-static void swap_feature_fields(BlockDriverState *bs_top,
- BlockDriverState *bs_new)
-{
- BlockDriverState tmp;
-
- bdrv_move_feature_fields(&tmp, bs_top);
- bdrv_move_feature_fields(bs_top, bs_new);
- bdrv_move_feature_fields(bs_new, &tmp);
-}
-
/*
* Add new bs contents at the top of an image chain while the chain is
* live, while keeping required fields on the top layer.
@@ -2273,9 +2256,6 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
bdrv_ref(bs_top);
- /* Some fields always stay on top of the backing file chain */
- swap_feature_fields(bs_top, bs_new);
-
change_parent_backing_link(bs_top, bs_new);
bdrv_set_backing_hd(bs_new, bs_top);
bdrv_unref(bs_top);
@@ -2292,16 +2272,6 @@ void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverState *new)
bdrv_ref(old);
- if (old->blk) {
- /* As long as these fields aren't in BlockBackend, but in the top-level
- * BlockDriverState, it's not possible for a BDS to have two BBs.
- *
- * We really want to copy the fields from old to new, but we go for a
- * swap instead so that pointers aren't duplicated and cause trouble.
- * (Also, bdrv_swap() used to do the same.) */
- assert(!new->blk);
- swap_feature_fields(old, new);
- }
change_parent_backing_link(old, new);
/* Change backing files if a previously independent node is added to the
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 13/31] Revert "block: Forbid I/O throttling on nodes with multiple parents for 2.6"
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (11 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 12/31] block: Remove bdrv_move_feature_fields() Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 14/31] block: Don't check throttled reqs in bdrv_requests_pending() Kevin Wolf
` (18 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
This reverts commit 76b223200ef4fb09dd87f0e213159795eb68e7a5.
Now that I/O throttling is fully done on the BlockBackend level, there
is no reason any more to block I/O throttling for nodes with multiple
parents as the parents don't influence each other any more.
Conflicts:
block.c
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 6 ------
blockdev.c | 7 -------
2 files changed, 13 deletions(-)
diff --git a/block.c b/block.c
index 39f14c7..cedbd70 100644
--- a/block.c
+++ b/block.c
@@ -1522,12 +1522,6 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
return -ENODEV;
}
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
- error_setg(errp, "Cannot reference an existing block device for "
- "which I/O throttling is enabled");
- return -EINVAL;
- }
-
bdrv_ref(bs);
*pbs = bs;
return 0;
diff --git a/blockdev.c b/blockdev.c
index 93ddc27..03ddd3a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2652,13 +2652,6 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
goto out;
}
- /* The BlockBackend must be the only parent */
- assert(QLIST_FIRST(&bs->parents));
- if (QLIST_NEXT(QLIST_FIRST(&bs->parents), next_parent)) {
- error_setg(errp, "Cannot throttle device with multiple parents");
- goto out;
- }
-
throttle_config_init(&cfg);
cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps;
cfg.buckets[THROTTLE_BPS_READ].avg = bps_rd;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 14/31] block: Don't check throttled reqs in bdrv_requests_pending()
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (12 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 13/31] Revert "block: Forbid I/O throttling on nodes with multiple parents for 2.6" Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 15/31] block: Use BdrvChild callbacks for change_media/resize Kevin Wolf
` (17 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
Checking whether there are throttled requests requires going to the
associated BlockBackend, which we want to avoid.
All users of bdrv_requests_pending() in block/io.c already call
bdrv_parent_drained_begin() first, which restarts all throttled
requests, so no throttled requests can be left here and this is removal
of dead code.
The remaining users (assertions during graph manipulation in block.c)
don't care about requests that are still queued in the BlockBackend and
haven't been issued for a BlockDriverState yet.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/io.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/block/io.c b/block/io.c
index 23abbc5..6e90805 100644
--- a/block/io.c
+++ b/block/io.c
@@ -147,17 +147,10 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
bool bdrv_requests_pending(BlockDriverState *bs)
{
BdrvChild *child;
- BlockBackendPublic *blkp = bs->blk ? blk_get_public(bs->blk) : NULL;
if (!QLIST_EMPTY(&bs->tracked_requests)) {
return true;
}
- if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[0])) {
- return true;
- }
- if (blkp && !qemu_co_queue_empty(&blkp->throttled_reqs[1])) {
- return true;
- }
QLIST_FOREACH(child, &bs->children, next) {
if (bdrv_requests_pending(child->bs)) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 15/31] block: Use BdrvChild callbacks for change_media/resize
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (13 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 14/31] block: Don't check throttled reqs in bdrv_requests_pending() Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 16/31] block: User BdrvChild callback for device name Kevin Wolf
` (16 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
We want to get rid of BlockDriverState.blk in order to allow multiple
BlockBackends per BDS. Converting the device callbacks in block.c (which
assume a single BlockBackend) to per-child callbacks gets us rid of the
first few instances.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 39 ++++++++++++++++++++++++++-------------
block/block-backend.c | 15 ++++++++++++++-
include/block/block_int.h | 4 +++-
3 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/block.c b/block.c
index cedbd70..ab592ac 100644
--- a/block.c
+++ b/block.c
@@ -1214,6 +1214,27 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child)
bdrv_root_unref_child(child);
}
+
+static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load)
+{
+ BdrvChild *c;
+ QLIST_FOREACH(c, &bs->parents, next_parent) {
+ if (c->role->change_media) {
+ c->role->change_media(c, load);
+ }
+ }
+}
+
+static void bdrv_parent_cb_resize(BlockDriverState *bs)
+{
+ BdrvChild *c;
+ QLIST_FOREACH(c, &bs->parents, next_parent) {
+ if (c->role->resize) {
+ c->role->resize(c);
+ }
+ }
+}
+
/*
* Sets the backing file link of a BDS. A new reference is created; callers
* which don't need their own reference any more must call bdrv_unref().
@@ -1673,9 +1694,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
}
if (!bdrv_key_required(bs)) {
- if (bs->blk) {
- blk_dev_change_media_cb(bs->blk, true);
- }
+ bdrv_parent_cb_change_media(bs, true);
} else if (!runstate_check(RUN_STATE_PRELAUNCH)
&& !runstate_check(RUN_STATE_INMIGRATE)
&& !runstate_check(RUN_STATE_PAUSED)) { /* HACK */
@@ -2121,9 +2140,7 @@ static void bdrv_close(BlockDriverState *bs)
bdrv_release_named_dirty_bitmaps(bs);
assert(QLIST_EMPTY(&bs->dirty_bitmaps));
- if (bs->blk) {
- blk_dev_change_media_cb(bs->blk, false);
- }
+ bdrv_parent_cb_change_media(bs, false);
if (bs->drv) {
BdrvChild *child, *next;
@@ -2574,9 +2591,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
if (ret == 0) {
ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
bdrv_dirty_bitmap_truncate(bs);
- if (bs->blk) {
- blk_dev_resize_cb(bs->blk);
- }
+ bdrv_parent_cb_resize(bs);
}
return ret;
}
@@ -2686,11 +2701,9 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
if (ret < 0) {
bs->valid_key = 0;
} else if (!bs->valid_key) {
+ /* call the change callback now, we skipped it on open */
bs->valid_key = 1;
- if (bs->blk) {
- /* call the change callback now, we skipped it on open */
- blk_dev_change_media_cb(bs->blk, true);
- }
+ bdrv_parent_cb_change_media(bs, true);
}
return ret;
}
diff --git a/block/block-backend.c b/block/block-backend.c
index a71b54d..a085455 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -94,9 +94,15 @@ static void blk_root_inherit_options(int *child_flags, QDict *child_options,
static void blk_root_drained_begin(BdrvChild *child);
static void blk_root_drained_end(BdrvChild *child);
+static void blk_root_change_media(BdrvChild *child, bool load);
+static void blk_root_resize(BdrvChild *child);
+
static const BdrvChildRole child_root = {
.inherit_options = blk_root_inherit_options,
+ .change_media = blk_root_change_media,
+ .resize = blk_root_resize,
+
.drained_begin = blk_root_drained_begin,
.drained_end = blk_root_drained_end,
};
@@ -556,6 +562,11 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load)
}
}
+static void blk_root_change_media(BdrvChild *child, bool load)
+{
+ blk_dev_change_media_cb(child->opaque, load);
+}
+
/*
* Does @blk's attached device model have removable media?
* %true if no device model is attached.
@@ -610,8 +621,10 @@ bool blk_dev_is_medium_locked(BlockBackend *blk)
/*
* Notify @blk's attached device model of a backend size change.
*/
-void blk_dev_resize_cb(BlockBackend *blk)
+static void blk_root_resize(BdrvChild *child)
{
+ BlockBackend *blk = child->opaque;
+
if (blk->dev_ops && blk->dev_ops->resize_cb) {
blk->dev_ops->resize_cb(blk->dev_opaque);
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index db2b4bc..9555429 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -365,6 +365,9 @@ struct BdrvChildRole {
void (*inherit_options)(int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options);
+ void (*change_media)(BdrvChild *child, bool load);
+ void (*resize)(BdrvChild *child);
+
/*
* If this pair of functions is implemented, the parent doesn't issue new
* requests after returning from .drained_begin() until .drained_end() is
@@ -722,7 +725,6 @@ bool blk_dev_has_tray(BlockBackend *blk);
void blk_dev_eject_request(BlockBackend *blk, bool force);
bool blk_dev_is_tray_open(BlockBackend *blk);
bool blk_dev_is_medium_locked(BlockBackend *blk);
-void blk_dev_resize_cb(BlockBackend *blk);
void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
bool bdrv_requests_pending(BlockDriverState *bs);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 16/31] block: User BdrvChild callback for device name
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (14 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 15/31] block: Use BdrvChild callbacks for change_media/resize Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 17/31] blockjob: Don't set iostatus of target Kevin Wolf
` (15 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
In order to get rid of bs->blk for bdrv_get_device_name() and
bdrv_get_device_or_node_name(), ask all parents for their name and
simply pick the first one.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 22 ++++++++++++++++++++--
block/block-backend.c | 6 ++++++
include/block/block_int.h | 5 +++++
3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index ab592ac..fd4cf81 100644
--- a/block.c
+++ b/block.c
@@ -2894,10 +2894,28 @@ const char *bdrv_get_node_name(const BlockDriverState *bs)
return bs->node_name;
}
+static const char *bdrv_get_parent_name(const BlockDriverState *bs)
+{
+ BdrvChild *c;
+ const char *name;
+
+ /* If multiple parents have a name, just pick the first one. */
+ QLIST_FOREACH(c, &bs->parents, next_parent) {
+ if (c->role->get_name) {
+ name = c->role->get_name(c);
+ if (name && *name) {
+ return name;
+ }
+ }
+ }
+
+ return NULL;
+}
+
/* TODO check what callers really want: bs->node_name or blk_name() */
const char *bdrv_get_device_name(const BlockDriverState *bs)
{
- return bs->blk ? blk_name(bs->blk) : "";
+ return bdrv_get_parent_name(bs) ?: "";
}
/* This can be used to identify nodes that might not have a device
@@ -2906,7 +2924,7 @@ const char *bdrv_get_device_name(const BlockDriverState *bs)
* absent, then this returns an empty (non-null) string. */
const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
{
- return bs->blk ? blk_name(bs->blk) : bs->node_name;
+ return bdrv_get_parent_name(bs) ?: bs->node_name;
}
int bdrv_get_flags(BlockDriverState *bs)
diff --git a/block/block-backend.c b/block/block-backend.c
index a085455..a31fc20 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -97,11 +97,17 @@ static void blk_root_drained_end(BdrvChild *child);
static void blk_root_change_media(BdrvChild *child, bool load);
static void blk_root_resize(BdrvChild *child);
+static const char *blk_root_get_name(BdrvChild *child)
+{
+ return blk_name(child->opaque);
+}
+
static const BdrvChildRole child_root = {
.inherit_options = blk_root_inherit_options,
.change_media = blk_root_change_media,
.resize = blk_root_resize,
+ .get_name = blk_root_get_name,
.drained_begin = blk_root_drained_begin,
.drained_end = blk_root_drained_end,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 9555429..80e2da5 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -368,6 +368,11 @@ struct BdrvChildRole {
void (*change_media)(BdrvChild *child, bool load);
void (*resize)(BdrvChild *child);
+ /* Returns a name that is supposedly more useful for human users than the
+ * node name for identifying the node in question (in particular, a BB
+ * name), or NULL if the parent can't provide a better name. */
+ const char* (*get_name)(BdrvChild *child);
+
/*
* If this pair of functions is implemented, the parent doesn't issue new
* requests after returning from .drained_begin() until .drained_end() is
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 17/31] blockjob: Don't set iostatus of target
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (15 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 16/31] block: User BdrvChild callback for device name Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 18/31] blockjob: Don't touch BDS iostatus Kevin Wolf
` (14 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
When block job errors were introduced, we assigned the iostatus of the
target BDS "just in case". The field has never been accessible for the
user because the target isn't listed in query-block.
Before we can allow the user to have a second BlockBackend on the
target, we need to clean this up. If anything, we would want to set the
iostatus for the internal BB of the job (which we can always do later),
but certainly not for a separate BB which the job doesn't even use.
As a nice side effect, this gets us rid of another bs->blk use.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/backup.c | 8 ++++----
block/mirror.c | 8 ++++----
block/stream.c | 3 +--
blockjob.c | 6 +-----
include/block/blockjob.h | 4 +---
5 files changed, 11 insertions(+), 18 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 491fd14..ef7609d 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -272,11 +272,11 @@ static BlockErrorAction backup_error_action(BackupBlockJob *job,
bool read, int error)
{
if (read) {
- return block_job_error_action(&job->common, job->common.bs,
- job->on_source_error, true, error);
+ return block_job_error_action(&job->common, job->on_source_error,
+ true, error);
} else {
- return block_job_error_action(&job->common, job->target,
- job->on_target_error, false, error);
+ return block_job_error_action(&job->common, job->on_target_error,
+ false, error);
}
}
diff --git a/block/mirror.c b/block/mirror.c
index 039f481..c7f51ad 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -80,11 +80,11 @@ static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read,
{
s->synced = false;
if (read) {
- return block_job_error_action(&s->common, s->common.bs,
- s->on_source_error, true, error);
+ return block_job_error_action(&s->common, s->on_source_error,
+ true, error);
} else {
- return block_job_error_action(&s->common, s->target,
- s->on_target_error, false, error);
+ return block_job_error_action(&s->common, s->on_target_error,
+ false, error);
}
}
diff --git a/block/stream.c b/block/stream.c
index 332b9a1..16c3e90 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -163,8 +163,7 @@ wait:
}
if (ret < 0) {
BlockErrorAction action =
- block_job_error_action(&s->common, s->common.bs, s->on_error,
- true, -ret);
+ block_job_error_action(&s->common, s->on_error, true, -ret);
if (action == BLOCK_ERROR_ACTION_STOP) {
n = 0;
continue;
diff --git a/blockjob.c b/blockjob.c
index 9fc37ca..5b840a7 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -411,8 +411,7 @@ void block_job_event_ready(BlockJob *job)
job->speed, &error_abort);
}
-BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
- BlockdevOnError on_err,
+BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,
int is_read, int error)
{
BlockErrorAction action;
@@ -443,9 +442,6 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
job->user_paused = true;
block_job_pause(job);
block_job_iostatus_set_err(job, error);
- if (bs->blk && bs != job->bs) {
- blk_iostatus_set_err(bs->blk, error);
- }
}
return action;
}
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 8bedc49..073a433 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -383,7 +383,6 @@ void block_job_iostatus_reset(BlockJob *job);
/**
* block_job_error_action:
* @job: The job to signal an error for.
- * @bs: The block device on which to set an I/O error.
* @on_err: The error action setting.
* @is_read: Whether the operation was a read.
* @error: The error that was reported.
@@ -391,8 +390,7 @@ void block_job_iostatus_reset(BlockJob *job);
* Report an I/O error for a block job and possibly stop the VM. Return the
* action that was selected based on @on_err and @error.
*/
-BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
- BlockdevOnError on_err,
+BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,
int is_read, int error);
typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 18/31] blockjob: Don't touch BDS iostatus
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (16 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 17/31] blockjob: Don't set iostatus of target Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 19/31] block: Remove bdrv_aio_multiwrite() Kevin Wolf
` (13 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
Block jobs don't actually make use of the iostatus for their BDSes, but
they manage a separate block job iostatus. Still, they require that it
is enabled for the source BDS and they enable it automatically for the
target and set the error handling mode - which ends up never being used
by the job.
This patch removes all of the BDS iostatus handling from the block job,
which removes another few bs->blk accesses.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/backup.c | 26 --------------------------
block/commit.c | 7 -------
block/mirror.c | 26 --------------------------
block/stream.c | 7 -------
4 files changed, 66 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index ef7609d..fec45e8 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -218,15 +218,6 @@ static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp)
ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME);
}
-static void backup_iostatus_reset(BlockJob *job)
-{
- BackupBlockJob *s = container_of(job, BackupBlockJob, common);
-
- if (s->target->blk) {
- blk_iostatus_reset(s->target->blk);
- }
-}
-
static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
{
BdrvDirtyBitmap *bm;
@@ -263,7 +254,6 @@ static const BlockJobDriver backup_job_driver = {
.instance_size = sizeof(BackupBlockJob),
.job_type = BLOCK_JOB_TYPE_BACKUP,
.set_speed = backup_set_speed,
- .iostatus_reset = backup_iostatus_reset,
.commit = backup_commit,
.abort = backup_abort,
};
@@ -388,7 +378,6 @@ static void coroutine_fn backup_run(void *opaque)
BackupCompleteData *data;
BlockDriverState *bs = job->common.bs;
BlockDriverState *target = job->target;
- BlockdevOnError on_target_error = job->on_target_error;
NotifierWithReturn before_write = {
.notify = backup_before_write_notify,
};
@@ -404,11 +393,6 @@ static void coroutine_fn backup_run(void *opaque)
job->done_bitmap = bitmap_new(end);
- if (target->blk) {
- blk_set_on_error(target->blk, on_target_error, on_target_error);
- blk_iostatus_enable(target->blk);
- }
-
bdrv_add_before_write_notifier(bs, &before_write);
if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
@@ -484,9 +468,6 @@ static void coroutine_fn backup_run(void *opaque)
qemu_co_rwlock_unlock(&job->flush_rwlock);
g_free(job->done_bitmap);
- if (target->blk) {
- blk_iostatus_disable(target->blk);
- }
bdrv_op_unblock_all(target, job->common.blocker);
data = g_malloc(sizeof(*data));
@@ -515,13 +496,6 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
return;
}
- if ((on_source_error == BLOCKDEV_ON_ERROR_STOP ||
- on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
- (!bs->blk || !blk_iostatus_is_enabled(bs->blk))) {
- error_setg(errp, QERR_INVALID_PARAMETER, "on-source-error");
- return;
- }
-
if (!bdrv_is_inserted(bs)) {
error_setg(errp, "Device is not inserted: %s",
bdrv_get_device_name(bs));
diff --git a/block/commit.c b/block/commit.c
index cba0e8c..f308c8c 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -214,13 +214,6 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
BlockDriverState *overlay_bs;
Error *local_err = NULL;
- if ((on_error == BLOCKDEV_ON_ERROR_STOP ||
- on_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
- (!bs->blk || !blk_iostatus_is_enabled(bs->blk))) {
- error_setg(errp, "Invalid parameter combination");
- return;
- }
-
assert(top != bs);
if (top == base) {
error_setg(errp, "Invalid files for merge: top and base are the same");
diff --git a/block/mirror.c b/block/mirror.c
index c7f51ad..71d7a4e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -710,9 +710,6 @@ immediate_exit:
g_free(s->cow_bitmap);
g_free(s->in_flight_bitmap);
bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
- if (s->target->blk) {
- blk_iostatus_disable(s->target->blk);
- }
data = g_malloc(sizeof(*data));
data->ret = ret;
@@ -739,15 +736,6 @@ static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp)
ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME);
}
-static void mirror_iostatus_reset(BlockJob *job)
-{
- MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
-
- if (s->target->blk) {
- blk_iostatus_reset(s->target->blk);
- }
-}
-
static void mirror_complete(BlockJob *job, Error **errp)
{
MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
@@ -793,7 +781,6 @@ static const BlockJobDriver mirror_job_driver = {
.instance_size = sizeof(MirrorBlockJob),
.job_type = BLOCK_JOB_TYPE_MIRROR,
.set_speed = mirror_set_speed,
- .iostatus_reset= mirror_iostatus_reset,
.complete = mirror_complete,
};
@@ -801,8 +788,6 @@ static const BlockJobDriver commit_active_job_driver = {
.instance_size = sizeof(MirrorBlockJob),
.job_type = BLOCK_JOB_TYPE_COMMIT,
.set_speed = mirror_set_speed,
- .iostatus_reset
- = mirror_iostatus_reset,
.complete = mirror_complete,
};
@@ -827,13 +812,6 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
assert ((granularity & (granularity - 1)) == 0);
- if ((on_source_error == BLOCKDEV_ON_ERROR_STOP ||
- on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
- (!bs->blk || !blk_iostatus_is_enabled(bs->blk))) {
- error_setg(errp, QERR_INVALID_PARAMETER, "on-source-error");
- return;
- }
-
if (buf_size < 0) {
error_setg(errp, "Invalid parameter 'buf-size'");
return;
@@ -882,10 +860,6 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
bdrv_op_block_all(s->target, s->common.blocker);
- if (s->target->blk) {
- blk_set_on_error(s->target->blk, on_target_error, on_target_error);
- blk_iostatus_enable(s->target->blk);
- }
s->common.co = qemu_coroutine_create(mirror_run);
trace_mirror_start(bs, s, s->common.co, opaque);
qemu_coroutine_enter(s->common.co, s);
diff --git a/block/stream.c b/block/stream.c
index 16c3e90..40aa322 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -223,13 +223,6 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base,
{
StreamBlockJob *s;
- if ((on_error == BLOCKDEV_ON_ERROR_STOP ||
- on_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
- (!bs->blk || !blk_iostatus_is_enabled(bs->blk))) {
- error_setg(errp, QERR_INVALID_PARAMETER, "on-error");
- return;
- }
-
s = block_job_create(&stream_job_driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 19/31] block: Remove bdrv_aio_multiwrite()
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (17 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 18/31] blockjob: Don't touch BDS iostatus Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 20/31] block: Add bdrv_has_blk() Kevin Wolf
` (12 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
Since virtio-blk implements request merging itself these days, the only
remaining users are test cases for the function. That doesn't make the
function exactly useful any more.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/block-backend.c | 14 ---
block/io.c | 194 ---------------------------------------
include/block/block.h | 7 +-
include/sysemu/block-backend.h | 1 -
qemu-io-cmds.c | 203 -----------------------------------------
tests/qemu-iotests/100 | 152 ------------------------------
tests/qemu-iotests/100.out | 103 ---------------------
tests/qemu-iotests/136 | 20 +---
tests/qemu-iotests/136.out | 4 +-
tests/qemu-iotests/group | 2 +-
trace-events | 2 -
11 files changed, 9 insertions(+), 693 deletions(-)
delete mode 100755 tests/qemu-iotests/100
delete mode 100644 tests/qemu-iotests/100.out
diff --git a/block/block-backend.c b/block/block-backend.c
index a31fc20..8d6fc77 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1067,20 +1067,6 @@ void blk_aio_cancel_async(BlockAIOCB *acb)
bdrv_aio_cancel_async(acb);
}
-int blk_aio_multiwrite(BlockBackend *blk, BlockRequest *reqs, int num_reqs)
-{
- int i, ret;
-
- for (i = 0; i < num_reqs; i++) {
- ret = blk_check_request(blk, reqs[i].sector, reqs[i].nb_sectors);
- if (ret < 0) {
- return ret;
- }
- }
-
- return bdrv_aio_multiwrite(blk_bs(blk), reqs, num_reqs);
-}
-
int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
{
if (!blk_is_available(blk)) {
diff --git a/block/io.c b/block/io.c
index 6e90805..8a6f470 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1878,200 +1878,6 @@ BlockAIOCB *bdrv_aio_write_zeroes(BlockDriverState *bs,
cb, opaque, true);
}
-
-typedef struct MultiwriteCB {
- int error;
- int num_requests;
- int num_callbacks;
- struct {
- BlockCompletionFunc *cb;
- void *opaque;
- QEMUIOVector *free_qiov;
- } callbacks[];
-} MultiwriteCB;
-
-static void multiwrite_user_cb(MultiwriteCB *mcb)
-{
- int i;
-
- for (i = 0; i < mcb->num_callbacks; i++) {
- mcb->callbacks[i].cb(mcb->callbacks[i].opaque, mcb->error);
- if (mcb->callbacks[i].free_qiov) {
- qemu_iovec_destroy(mcb->callbacks[i].free_qiov);
- }
- g_free(mcb->callbacks[i].free_qiov);
- }
-}
-
-static void multiwrite_cb(void *opaque, int ret)
-{
- MultiwriteCB *mcb = opaque;
-
- trace_multiwrite_cb(mcb, ret);
-
- if (ret < 0 && !mcb->error) {
- mcb->error = ret;
- }
-
- mcb->num_requests--;
- if (mcb->num_requests == 0) {
- multiwrite_user_cb(mcb);
- g_free(mcb);
- }
-}
-
-static int multiwrite_req_compare(const void *a, const void *b)
-{
- const BlockRequest *req1 = a, *req2 = b;
-
- /*
- * Note that we can't simply subtract req2->sector from req1->sector
- * here as that could overflow the return value.
- */
- if (req1->sector > req2->sector) {
- return 1;
- } else if (req1->sector < req2->sector) {
- return -1;
- } else {
- return 0;
- }
-}
-
-/*
- * Takes a bunch of requests and tries to merge them. Returns the number of
- * requests that remain after merging.
- */
-static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs,
- int num_reqs, MultiwriteCB *mcb)
-{
- int i, outidx;
-
- // Sort requests by start sector
- qsort(reqs, num_reqs, sizeof(*reqs), &multiwrite_req_compare);
-
- // Check if adjacent requests touch the same clusters. If so, combine them,
- // filling up gaps with zero sectors.
- outidx = 0;
- for (i = 1; i < num_reqs; i++) {
- int merge = 0;
- int64_t oldreq_last = reqs[outidx].sector + reqs[outidx].nb_sectors;
-
- // Handle exactly sequential writes and overlapping writes.
- if (reqs[i].sector <= oldreq_last) {
- merge = 1;
- }
-
- if (reqs[outidx].qiov->niov + reqs[i].qiov->niov + 1 >
- bs->bl.max_iov) {
- merge = 0;
- }
-
- if (bs->bl.max_transfer_length && reqs[outidx].nb_sectors +
- reqs[i].nb_sectors > bs->bl.max_transfer_length) {
- merge = 0;
- }
-
- if (merge) {
- size_t size;
- QEMUIOVector *qiov = g_malloc0(sizeof(*qiov));
- qemu_iovec_init(qiov,
- reqs[outidx].qiov->niov + reqs[i].qiov->niov + 1);
-
- // Add the first request to the merged one. If the requests are
- // overlapping, drop the last sectors of the first request.
- size = (reqs[i].sector - reqs[outidx].sector) << 9;
- qemu_iovec_concat(qiov, reqs[outidx].qiov, 0, size);
-
- // We should need to add any zeros between the two requests
- assert (reqs[i].sector <= oldreq_last);
-
- // Add the second request
- qemu_iovec_concat(qiov, reqs[i].qiov, 0, reqs[i].qiov->size);
-
- // Add tail of first request, if necessary
- if (qiov->size < reqs[outidx].qiov->size) {
- qemu_iovec_concat(qiov, reqs[outidx].qiov, qiov->size,
- reqs[outidx].qiov->size - qiov->size);
- }
-
- reqs[outidx].nb_sectors = qiov->size >> 9;
- reqs[outidx].qiov = qiov;
-
- mcb->callbacks[i].free_qiov = reqs[outidx].qiov;
- } else {
- outidx++;
- reqs[outidx].sector = reqs[i].sector;
- reqs[outidx].nb_sectors = reqs[i].nb_sectors;
- reqs[outidx].qiov = reqs[i].qiov;
- }
- }
-
- if (bs->blk) {
- block_acct_merge_done(blk_get_stats(bs->blk), BLOCK_ACCT_WRITE,
- num_reqs - outidx - 1);
- }
-
- return outidx + 1;
-}
-
-/*
- * Submit multiple AIO write requests at once.
- *
- * On success, the function returns 0 and all requests in the reqs array have
- * been submitted. In error case this function returns -1, and any of the
- * requests may or may not be submitted yet. In particular, this means that the
- * callback will be called for some of the requests, for others it won't. The
- * caller must check the error field of the BlockRequest to wait for the right
- * callbacks (if error != 0, no callback will be called).
- *
- * The implementation may modify the contents of the reqs array, e.g. to merge
- * requests. However, the fields opaque and error are left unmodified as they
- * are used to signal failure for a single request to the caller.
- */
-int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs)
-{
- MultiwriteCB *mcb;
- int i;
-
- /* don't submit writes if we don't have a medium */
- if (bs->drv == NULL) {
- for (i = 0; i < num_reqs; i++) {
- reqs[i].error = -ENOMEDIUM;
- }
- return -1;
- }
-
- if (num_reqs == 0) {
- return 0;
- }
-
- // Create MultiwriteCB structure
- mcb = g_malloc0(sizeof(*mcb) + num_reqs * sizeof(*mcb->callbacks));
- mcb->num_requests = 0;
- mcb->num_callbacks = num_reqs;
-
- for (i = 0; i < num_reqs; i++) {
- mcb->callbacks[i].cb = reqs[i].cb;
- mcb->callbacks[i].opaque = reqs[i].opaque;
- }
-
- // Check for mergable requests
- num_reqs = multiwrite_merge(bs, reqs, num_reqs, mcb);
-
- trace_bdrv_aio_multiwrite(mcb, mcb->num_callbacks, num_reqs);
-
- /* Run the aio requests. */
- mcb->num_requests = num_reqs;
- for (i = 0; i < num_reqs; i++) {
- bdrv_co_aio_rw_vector(bs, reqs[i].sector, reqs[i].qiov,
- reqs[i].nb_sectors, reqs[i].flags,
- multiwrite_cb, mcb,
- true);
- }
-
- return 0;
-}
-
void bdrv_aio_cancel(BlockAIOCB *acb)
{
qemu_aio_ref(acb);
diff --git a/include/block/block.h b/include/block/block.h
index 2c5c280..d1f9380 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -329,7 +329,7 @@ void bdrv_aio_cancel(BlockAIOCB *acb);
void bdrv_aio_cancel_async(BlockAIOCB *acb);
typedef struct BlockRequest {
- /* Fields to be filled by multiwrite caller */
+ /* Fields to be filled by caller */
union {
struct {
int64_t sector;
@@ -345,13 +345,10 @@ typedef struct BlockRequest {
BlockCompletionFunc *cb;
void *opaque;
- /* Filled by multiwrite implementation */
+ /* Filled by block layer */
int error;
} BlockRequest;
-int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs,
- int num_reqs);
-
/* sg packet commands */
int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf);
BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index dd9c8ca..79f39b8 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -138,7 +138,6 @@ BlockAIOCB *blk_aio_discard(BlockBackend *blk,
BlockCompletionFunc *cb, void *opaque);
void blk_aio_cancel(BlockAIOCB *acb);
void blk_aio_cancel_async(BlockAIOCB *acb);
-int blk_aio_multiwrite(BlockBackend *blk, BlockRequest *reqs, int num_reqs);
int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
BlockCompletionFunc *cb, void *opaque);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 4a00bc6..22f2ecf 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -574,49 +574,6 @@ static int do_aio_writev(BlockBackend *blk, QEMUIOVector *qiov,
return async_ret < 0 ? async_ret : 1;
}
-struct multiwrite_async_ret {
- int num_done;
- int error;
-};
-
-static void multiwrite_cb(void *opaque, int ret)
-{
- struct multiwrite_async_ret *async_ret = opaque;
-
- async_ret->num_done++;
- if (ret < 0) {
- async_ret->error = ret;
- }
-}
-
-static int do_aio_multiwrite(BlockBackend *blk, BlockRequest* reqs,
- int num_reqs, int *total)
-{
- int i, ret;
- struct multiwrite_async_ret async_ret = {
- .num_done = 0,
- .error = 0,
- };
-
- *total = 0;
- for (i = 0; i < num_reqs; i++) {
- reqs[i].cb = multiwrite_cb;
- reqs[i].opaque = &async_ret;
- *total += reqs[i].qiov->size;
- }
-
- ret = blk_aio_multiwrite(blk, reqs, num_reqs);
- if (ret < 0) {
- return ret;
- }
-
- while (async_ret.num_done < num_reqs) {
- main_loop_wait(false);
- }
-
- return async_ret.error < 0 ? async_ret.error : 1;
-}
-
static void read_help(void)
{
printf(
@@ -1211,165 +1168,6 @@ out:
return 0;
}
-static void multiwrite_help(void)
-{
- printf(
-"\n"
-" writes a range of bytes from the given offset source from multiple buffers,\n"
-" in a batch of requests that may be merged by qemu\n"
-"\n"
-" Example:\n"
-" 'multiwrite 512 1k 1k ; 4k 1k'\n"
-" writes 2 kB at 512 bytes and 1 kB at 4 kB into the open file\n"
-"\n"
-" Writes into a segment of the currently open file, using a buffer\n"
-" filled with a set pattern (0xcdcdcdcd). The pattern byte is increased\n"
-" by one for each request contained in the multiwrite command.\n"
-" -P, -- use different pattern to fill file\n"
-" -C, -- report statistics in a machine parsable format\n"
-" -q, -- quiet mode, do not show I/O statistics\n"
-"\n");
-}
-
-static int multiwrite_f(BlockBackend *blk, int argc, char **argv);
-
-static const cmdinfo_t multiwrite_cmd = {
- .name = "multiwrite",
- .cfunc = multiwrite_f,
- .argmin = 2,
- .argmax = -1,
- .args = "[-Cq] [-P pattern ] off len [len..] [; off len [len..]..]",
- .oneline = "issues multiple write requests at once",
- .help = multiwrite_help,
-};
-
-static int multiwrite_f(BlockBackend *blk, int argc, char **argv)
-{
- struct timeval t1, t2;
- bool Cflag = false, qflag = false;
- int c, cnt;
- char **buf;
- int64_t offset, first_offset = 0;
- /* Some compilers get confused and warn if this is not initialized. */
- int total = 0;
- int nr_iov;
- int nr_reqs;
- int pattern = 0xcd;
- QEMUIOVector *qiovs;
- int i;
- BlockRequest *reqs;
-
- while ((c = getopt(argc, argv, "CqP:")) != -1) {
- switch (c) {
- case 'C':
- Cflag = true;
- break;
- case 'q':
- qflag = true;
- break;
- case 'P':
- pattern = parse_pattern(optarg);
- if (pattern < 0) {
- return 0;
- }
- break;
- default:
- return qemuio_command_usage(&writev_cmd);
- }
- }
-
- if (optind > argc - 2) {
- return qemuio_command_usage(&writev_cmd);
- }
-
- nr_reqs = 1;
- for (i = optind; i < argc; i++) {
- if (!strcmp(argv[i], ";")) {
- nr_reqs++;
- }
- }
-
- reqs = g_new0(BlockRequest, nr_reqs);
- buf = g_new0(char *, nr_reqs);
- qiovs = g_new(QEMUIOVector, nr_reqs);
-
- for (i = 0; i < nr_reqs && optind < argc; i++) {
- int j;
-
- /* Read the offset of the request */
- offset = cvtnum(argv[optind]);
- if (offset < 0) {
- print_cvtnum_err(offset, argv[optind]);
- goto out;
- }
- optind++;
-
- if (offset & 0x1ff) {
- printf("offset %lld is not sector aligned\n",
- (long long)offset);
- goto out;
- }
-
- if (i == 0) {
- first_offset = offset;
- }
-
- /* Read lengths for qiov entries */
- for (j = optind; j < argc; j++) {
- if (!strcmp(argv[j], ";")) {
- break;
- }
- }
-
- nr_iov = j - optind;
-
- /* Build request */
- buf[i] = create_iovec(blk, &qiovs[i], &argv[optind], nr_iov, pattern);
- if (buf[i] == NULL) {
- goto out;
- }
-
- reqs[i].qiov = &qiovs[i];
- reqs[i].sector = offset >> 9;
- reqs[i].nb_sectors = reqs[i].qiov->size >> 9;
-
- optind = j + 1;
-
- pattern++;
- }
-
- /* If there were empty requests at the end, ignore them */
- nr_reqs = i;
-
- gettimeofday(&t1, NULL);
- cnt = do_aio_multiwrite(blk, reqs, nr_reqs, &total);
- gettimeofday(&t2, NULL);
-
- if (cnt < 0) {
- printf("aio_multiwrite failed: %s\n", strerror(-cnt));
- goto out;
- }
-
- if (qflag) {
- goto out;
- }
-
- /* Finally, report back -- -C gives a parsable format */
- t2 = tsub(t2, t1);
- print_report("wrote", &t2, first_offset, total, total, cnt, Cflag);
-out:
- for (i = 0; i < nr_reqs; i++) {
- qemu_io_free(buf[i]);
- if (reqs[i].qiov != NULL) {
- qemu_iovec_destroy(&qiovs[i]);
- }
- }
- g_free(buf);
- g_free(reqs);
- g_free(qiovs);
- return 0;
-}
-
struct aio_ctx {
BlockBackend *blk;
QEMUIOVector qiov;
@@ -2436,7 +2234,6 @@ static void __attribute((constructor)) init_qemuio_commands(void)
qemuio_add_command(&readv_cmd);
qemuio_add_command(&write_cmd);
qemuio_add_command(&writev_cmd);
- qemuio_add_command(&multiwrite_cmd);
qemuio_add_command(&aio_read_cmd);
qemuio_add_command(&aio_write_cmd);
qemuio_add_command(&aio_flush_cmd);
diff --git a/tests/qemu-iotests/100 b/tests/qemu-iotests/100
deleted file mode 100755
index e66db07..0000000
--- a/tests/qemu-iotests/100
+++ /dev/null
@@ -1,152 +0,0 @@
-#!/bin/bash
-#
-# Test simple read/write using plain bdrv_read/bdrv_write
-#
-# Copyright (C) 2014 Red Hat, Inc.
-#
-# 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=stefanha@redhat.com
-
-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 generic
-_supported_proto generic
-_supported_os Linux
-
-
-size=128M
-
-echo
-echo "== Single request =="
-_make_test_img $size
-$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "multiwrite 0 4k" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== verify pattern =="
-$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
-
-_cleanup_test_img
-
-echo
-echo "== Sequential requests =="
-_make_test_img $size
-$QEMU_IO -c "write -z 0 12k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "multiwrite 0 4k ; 4k 4k" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== verify pattern =="
-$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0xce 4k 4k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0 8k 4k" "$TEST_IMG" | _filter_qemu_io
-
-_cleanup_test_img
-
-echo
-echo "== Superset overlapping requests =="
-_make_test_img $size
-$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "multiwrite 0 4k ; 1k 2k" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== verify pattern =="
-# Order of overlapping in-flight requests is not guaranteed so we cannot verify
-# [1k, 3k) since it could have either pattern 0xcd or 0xce.
-$QEMU_IO -c "read -P 0xcd 0 1k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0xcd 3k 1k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
-
-_cleanup_test_img
-
-echo
-echo "== Subset overlapping requests =="
-_make_test_img $size
-$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "multiwrite 1k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== verify pattern =="
-# Order of overlapping in-flight requests is not guaranteed so we cannot verify
-# [1k, 3k) since it could have either pattern 0xcd or 0xce.
-$QEMU_IO -c "read -P 0xce 0 1k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0xce 3k 1k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
-
-_cleanup_test_img
-
-echo
-echo "== Head overlapping requests =="
-_make_test_img $size
-$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "multiwrite 0k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== verify pattern =="
-# Order of overlapping in-flight requests is not guaranteed so we cannot verify
-# [0k, 2k) since it could have either pattern 0xcd or 0xce.
-$QEMU_IO -c "read -P 0xce 2k 2k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
-
-_cleanup_test_img
-
-echo
-echo "== Tail overlapping requests =="
-_make_test_img $size
-$QEMU_IO -c "write -z 0 8k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "multiwrite 2k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== verify pattern =="
-# Order of overlapping in-flight requests is not guaranteed so we cannot verify
-# [2k, 4k) since it could have either pattern 0xcd or 0xce.
-$QEMU_IO -c "read -P 0xce 0k 2k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
-
-_cleanup_test_img
-
-echo
-echo "== Disjoint requests =="
-_make_test_img $size
-$QEMU_IO -c "write -z 0 72k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "multiwrite 0 4k ; 64k 4k" "$TEST_IMG" | _filter_qemu_io
-
-echo
-echo "== verify pattern =="
-$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0 4k 60k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0xce 64k 4k" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "read -P 0 68k 4k" "$TEST_IMG" | _filter_qemu_io
-
-# success, all done
-echo "*** done"
-rm -f $seq.full
-status=0
diff --git a/tests/qemu-iotests/100.out b/tests/qemu-iotests/100.out
deleted file mode 100644
index a44cae4..0000000
--- a/tests/qemu-iotests/100.out
+++ /dev/null
@@ -1,103 +0,0 @@
-QA output created by 100
-
-== Single request ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-wrote 8192/8192 bytes at offset 0
-8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 4096/4096 bytes at offset 0
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== verify pattern ==
-read 4096/4096 bytes at offset 0
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 4096/4096 bytes at offset 4096
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== Sequential requests ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-wrote 12288/12288 bytes at offset 0
-12 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 8192/8192 bytes at offset 0
-8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== verify pattern ==
-read 4096/4096 bytes at offset 0
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 4096/4096 bytes at offset 4096
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 4096/4096 bytes at offset 8192
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== Superset overlapping requests ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-wrote 8192/8192 bytes at offset 0
-8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 6144/6144 bytes at offset 0
-6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== verify pattern ==
-read 1024/1024 bytes at offset 0
-1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 1024/1024 bytes at offset 3072
-1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 4096/4096 bytes at offset 4096
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== Subset overlapping requests ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-wrote 8192/8192 bytes at offset 0
-8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 6144/6144 bytes at offset 1024
-6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== verify pattern ==
-read 1024/1024 bytes at offset 0
-1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 1024/1024 bytes at offset 3072
-1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 4096/4096 bytes at offset 4096
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== Head overlapping requests ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-wrote 8192/8192 bytes at offset 0
-8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 6144/6144 bytes at offset 0
-6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== verify pattern ==
-read 2048/2048 bytes at offset 2048
-2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 4096/4096 bytes at offset 4096
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== Tail overlapping requests ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-wrote 8192/8192 bytes at offset 0
-8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 6144/6144 bytes at offset 2048
-6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== verify pattern ==
-read 2048/2048 bytes at offset 0
-2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 4096/4096 bytes at offset 4096
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== Disjoint requests ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-wrote 73728/73728 bytes at offset 0
-72 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 8192/8192 bytes at offset 0
-8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-== verify pattern ==
-read 4096/4096 bytes at offset 0
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 61440/61440 bytes at offset 4096
-60 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 4096/4096 bytes at offset 65536
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 4096/4096 bytes at offset 69632
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-*** done
diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136
index e8c6937..996265e 100644
--- a/tests/qemu-iotests/136
+++ b/tests/qemu-iotests/136
@@ -248,14 +248,6 @@ sector = "%d"
if failed_wr_ops > 0:
highest_offset = max(highest_offset, bad_offset + 512)
- for i in range(wr_merged):
- first = i * wr_size * 2
- second = first + wr_size
- ops.append("multiwrite %d %d ; %d %d" %
- (first, wr_size, second, wr_size))
-
- highest_offset = max(highest_offset, wr_merged * wr_size * 2)
-
# Now perform all operations
for op in ops:
self.vm.hmp_qemu_io("drive0", op)
@@ -309,19 +301,15 @@ sector = "%d"
def test_flush(self):
self.do_test_stats(flush_ops = 8)
- def test_merged(self):
- for i in range(5):
- self.do_test_stats(wr_merged = i * 3)
-
def test_all(self):
# rd_size, rd_ops, wr_size, wr_ops, flush_ops
# invalid_rd_ops, invalid_wr_ops,
# failed_rd_ops, failed_wr_ops
# wr_merged
- test_values = [[512, 1, 512, 1, 1, 4, 7, 5, 2, 1],
- [65536, 1, 2048, 12, 7, 7, 5, 2, 5, 5],
- [32768, 9, 8192, 1, 4, 3, 2, 4, 6, 4],
- [16384, 11, 3584, 16, 9, 8, 6, 7, 3, 4]]
+ test_values = [[512, 1, 512, 1, 1, 4, 7, 5, 2, 0],
+ [65536, 1, 2048, 12, 7, 7, 5, 2, 5, 0],
+ [32768, 9, 8192, 1, 4, 3, 2, 4, 6, 0],
+ [16384, 11, 3584, 16, 9, 8, 6, 7, 3, 0]]
for i in test_values:
self.do_test_stats(*i)
diff --git a/tests/qemu-iotests/136.out b/tests/qemu-iotests/136.out
index 0a5e958..cfa5c0d 100644
--- a/tests/qemu-iotests/136.out
+++ b/tests/qemu-iotests/136.out
@@ -1,5 +1,5 @@
-........................................
+...................................
----------------------------------------------------------------------
-Ran 40 tests
+Ran 35 tests
OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 822953b..6067673 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -106,7 +106,7 @@
097 rw auto backing
098 rw auto backing quick
099 rw auto quick
-100 rw auto quick
+# 100 was removed, do not reuse
101 rw auto quick
102 rw auto quick
103 rw auto quick
diff --git a/trace-events b/trace-events
index e35b80e..b53c354 100644
--- a/trace-events
+++ b/trace-events
@@ -62,8 +62,6 @@ bdrv_open_common(void *bs, const char *filename, int flags, const char *format_n
bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
# block/io.c
-multiwrite_cb(void *mcb, int ret) "mcb %p ret %d"
-bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) "mcb %p num_callbacks %d num_reqs %d"
bdrv_aio_discard(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p"
bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 20/31] block: Add bdrv_has_blk()
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (18 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 19/31] block: Remove bdrv_aio_multiwrite() Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-19 15:21 ` [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next() Kevin Wolf
` (11 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
In many cases we just want to know whether a BDS has at least one BB
attached, without needing to know the exact BB that is attached. In
contrast to bs->blk, this is still a valid question when more than one
BB can be attached, so just answer it by checking the parents list.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/block-backend.c | 17 +++++++++++++++++
include/sysemu/block-backend.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index 8d6fc77..9dcac97 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -395,6 +395,23 @@ BlockDriverState *blk_bs(BlockBackend *blk)
}
/*
+ * Returns true if @bs has an associated BlockBackend.
+ */
+bool bdrv_has_blk(BlockDriverState *bs)
+{
+ BdrvChild *child;
+ QLIST_FOREACH(child, &bs->parents, next_parent) {
+ if (child->role == &child_root) {
+ assert(bs->blk);
+ return true;
+ }
+ }
+
+ assert(!bs->blk);
+ return false;
+}
+
+/*
* Return @blk's DriveInfo if any, else null.
*/
DriveInfo *blk_legacy_dinfo(BlockBackend *blk)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 79f39b8..44a222d 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -99,6 +99,7 @@ BlockBackend *blk_by_public(BlockBackendPublic *public);
BlockDriverState *blk_bs(BlockBackend *blk);
void blk_remove_bs(BlockBackend *blk);
void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
+bool bdrv_has_blk(BlockDriverState *bs);
void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow);
void blk_iostatus_enable(BlockBackend *blk);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next()
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (19 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 20/31] block: Add bdrv_has_blk() Kevin Wolf
@ 2016-05-19 15:21 ` Kevin Wolf
2016-05-20 7:54 ` Paolo Bonzini
2016-05-19 15:22 ` [Qemu-devel] [PULL 22/31] block: Don't return throttling info in query-named-block-nodes Kevin Wolf
` (10 subsequent siblings)
31 siblings, 1 reply; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
We need to introduce a separate BdrvNextIterator struct that can keep
more state than just the current BDS in order to avoid using the bs->blk
pointer.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 40 +++++++----------------
block/block-backend.c | 72 +++++++++++++++++++++++++++++-------------
block/io.c | 13 ++++----
block/snapshot.c | 30 +++++++++++-------
blockdev.c | 3 +-
include/block/block.h | 3 +-
include/sysemu/block-backend.h | 1 -
migration/block.c | 4 ++-
monitor.c | 6 ++--
qmp.c | 5 ++-
10 files changed, 102 insertions(+), 75 deletions(-)
diff --git a/block.c b/block.c
index fd4cf81..91bf431 100644
--- a/block.c
+++ b/block.c
@@ -2870,25 +2870,6 @@ BlockDriverState *bdrv_next_node(BlockDriverState *bs)
return QTAILQ_NEXT(bs, node_list);
}
-/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by
- * the monitor or attached to a BlockBackend */
-BlockDriverState *bdrv_next(BlockDriverState *bs)
-{
- if (!bs || bs->blk) {
- bs = blk_next_root_bs(bs);
- if (bs) {
- return bs;
- }
- }
-
- /* Ignore all BDSs that are attached to a BlockBackend here; they have been
- * handled by the above block already */
- do {
- bs = bdrv_next_monitor_owned(bs);
- } while (bs && bs->blk);
- return bs;
-}
-
const char *bdrv_get_node_name(const BlockDriverState *bs)
{
return bs->node_name;
@@ -3220,10 +3201,11 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
void bdrv_invalidate_cache_all(Error **errp)
{
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
Error *local_err = NULL;
+ BdrvNextIterator *it = NULL;
- while ((bs = bdrv_next(bs)) != NULL) {
+ while ((it = bdrv_next(it, &bs)) != NULL) {
AioContext *aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@@ -3265,10 +3247,11 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs,
int bdrv_inactivate_all(void)
{
BlockDriverState *bs = NULL;
+ BdrvNextIterator *it = NULL;
int ret = 0;
int pass;
- while ((bs = bdrv_next(bs)) != NULL) {
+ while ((it = bdrv_next(it, &bs)) != NULL) {
aio_context_acquire(bdrv_get_aio_context(bs));
}
@@ -3277,8 +3260,8 @@ int bdrv_inactivate_all(void)
* the second pass sets the BDRV_O_INACTIVE flag so that no further write
* is allowed. */
for (pass = 0; pass < 2; pass++) {
- bs = NULL;
- while ((bs = bdrv_next(bs)) != NULL) {
+ it = NULL;
+ while ((it = bdrv_next(it, &bs)) != NULL) {
ret = bdrv_inactivate_recurse(bs, pass);
if (ret < 0) {
goto out;
@@ -3287,8 +3270,8 @@ int bdrv_inactivate_all(void)
}
out:
- bs = NULL;
- while ((bs = bdrv_next(bs)) != NULL) {
+ it = NULL;
+ while ((it = bdrv_next(it, &bs)) != NULL) {
aio_context_release(bdrv_get_aio_context(bs));
}
@@ -3781,10 +3764,11 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
*/
bool bdrv_is_first_non_filter(BlockDriverState *candidate)
{
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
/* walk down the bs forest recursively */
- while ((bs = bdrv_next(bs)) != NULL) {
+ while ((it = bdrv_next(it, &bs)) != NULL) {
bool perm;
/* try to recurse in this top level bs */
diff --git a/block/block-backend.c b/block/block-backend.c
index 9dcac97..7f2eeb0 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -75,6 +75,7 @@ static const AIOCBInfo block_backend_aiocb_info = {
};
static void drive_info_del(DriveInfo *dinfo);
+static BlockBackend *bdrv_first_blk(BlockDriverState *bs);
/* All BlockBackends */
static QTAILQ_HEAD(, BlockBackend) block_backends =
@@ -286,28 +287,50 @@ BlockBackend *blk_next(BlockBackend *blk)
: QTAILQ_FIRST(&monitor_block_backends);
}
-/*
- * Iterates over all BlockDriverStates which are attached to a BlockBackend.
- * This function is for use by bdrv_next().
- *
- * @bs must be NULL or a BDS that is attached to a BB.
- */
-BlockDriverState *blk_next_root_bs(BlockDriverState *bs)
-{
+struct BdrvNextIterator {
+ enum {
+ BDRV_NEXT_BACKEND_ROOTS,
+ BDRV_NEXT_MONITOR_OWNED,
+ } phase;
BlockBackend *blk;
+ BlockDriverState *bs;
+};
- if (bs) {
- assert(bs->blk);
- blk = bs->blk;
- } else {
- blk = NULL;
+/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by
+ * the monitor or attached to a BlockBackend */
+BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs)
+{
+ if (!it) {
+ it = g_new(BdrvNextIterator, 1);
+ *it = (BdrvNextIterator) {
+ .phase = BDRV_NEXT_BACKEND_ROOTS,
+ };
+ }
+
+ /* First, return all root nodes of BlockBackends. In order to avoid
+ * returning a BDS twice when multiple BBs refer to it, we only return it
+ * if the BB is the first one in the parent list of the BDS. */
+ if (it->phase == BDRV_NEXT_BACKEND_ROOTS) {
+ do {
+ it->blk = blk_all_next(it->blk);
+ *bs = it->blk ? blk_bs(it->blk) : NULL;
+ } while (it->blk && (*bs == NULL || bdrv_first_blk(*bs) != it->blk));
+
+ if (*bs) {
+ return it;
+ }
+ it->phase = BDRV_NEXT_MONITOR_OWNED;
}
+ /* Then return the monitor-owned BDSes without a BB attached. Ignore all
+ * BDSes that are attached to a BlockBackend here; they have been handled
+ * by the above block already */
do {
- blk = blk_all_next(blk);
- } while (blk && !blk->root);
+ it->bs = bdrv_next_monitor_owned(it->bs);
+ *bs = it->bs;
+ } while (*bs && bdrv_has_blk(*bs));
- return blk ? blk->root->bs : NULL;
+ return *bs ? it : NULL;
}
/*
@@ -394,21 +417,26 @@ BlockDriverState *blk_bs(BlockBackend *blk)
return blk->root ? blk->root->bs : NULL;
}
-/*
- * Returns true if @bs has an associated BlockBackend.
- */
-bool bdrv_has_blk(BlockDriverState *bs)
+static BlockBackend *bdrv_first_blk(BlockDriverState *bs)
{
BdrvChild *child;
QLIST_FOREACH(child, &bs->parents, next_parent) {
if (child->role == &child_root) {
assert(bs->blk);
- return true;
+ return child->opaque;
}
}
assert(!bs->blk);
- return false;
+ return NULL;
+}
+
+/*
+ * Returns true if @bs has an associated BlockBackend.
+ */
+bool bdrv_has_blk(BlockDriverState *bs)
+{
+ return bdrv_first_blk(bs) != NULL;
}
/*
diff --git a/block/io.c b/block/io.c
index 8a6f470..60a6bd8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -270,10 +270,11 @@ void bdrv_drain_all(void)
{
/* Always run first iteration so any pending completion BHs run */
bool busy = true;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
GSList *aio_ctxs = NULL, *ctx;
- while ((bs = bdrv_next(bs))) {
+ while ((it = bdrv_next(it, &bs))) {
AioContext *aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@@ -301,10 +302,10 @@ void bdrv_drain_all(void)
for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) {
AioContext *aio_context = ctx->data;
- bs = NULL;
+ it = NULL;
aio_context_acquire(aio_context);
- while ((bs = bdrv_next(bs))) {
+ while ((it = bdrv_next(it, &bs))) {
if (aio_context == bdrv_get_aio_context(bs)) {
if (bdrv_requests_pending(bs)) {
busy = true;
@@ -317,8 +318,8 @@ void bdrv_drain_all(void)
}
}
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
+ it = NULL;
+ while ((it = bdrv_next(it, &bs))) {
AioContext *aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
diff --git a/block/snapshot.c b/block/snapshot.c
index e9d721d..3917ec5 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -373,9 +373,10 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
{
bool ok = true;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
- while (ok && (bs = bdrv_next(bs))) {
+ while (ok && (it = bdrv_next(it, &bs))) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -393,10 +394,11 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
Error **err)
{
int ret = 0;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
QEMUSnapshotInfo sn1, *snapshot = &sn1;
- while (ret == 0 && (bs = bdrv_next(bs))) {
+ while (ret == 0 && (it = bdrv_next(it, &bs))) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -415,9 +417,10 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
{
int err = 0;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
- while (err == 0 && (bs = bdrv_next(bs))) {
+ while (err == 0 && (it = bdrv_next(it, &bs))) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -435,9 +438,10 @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
{
QEMUSnapshotInfo sn;
int err = 0;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
- while (err == 0 && (bs = bdrv_next(bs))) {
+ while (err == 0 && (it = bdrv_next(it, &bs))) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -457,9 +461,10 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
BlockDriverState **first_bad_bs)
{
int err = 0;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
- while (err == 0 && (bs = bdrv_next(bs))) {
+ while (err == 0 && (it = bdrv_next(it, &bs))) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
@@ -480,9 +485,10 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
BlockDriverState *bdrv_all_find_vmstate_bs(void)
{
bool not_found = true;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
- while (not_found && (bs = bdrv_next(bs))) {
+ while (not_found && (it = bdrv_next(it, &bs))) {
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
diff --git a/blockdev.c b/blockdev.c
index 03ddd3a..442ca8d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4135,8 +4135,9 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
{
BlockJobInfoList *head = NULL, **p_next = &head;
BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
- for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
+ while ((it = bdrv_next(it, &bs))) {
AioContext *aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
diff --git a/include/block/block.h b/include/block/block.h
index d1f9380..a8c15e3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -17,6 +17,7 @@ typedef struct BlockJob BlockJob;
typedef struct BdrvChild BdrvChild;
typedef struct BdrvChildRole BdrvChildRole;
typedef struct BlockJobTxn BlockJobTxn;
+typedef struct BdrvNextIterator BdrvNextIterator;
typedef struct BlockDriverInfo {
/* in bytes, 0 if irrelevant */
@@ -401,7 +402,7 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
Error **errp);
bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base);
BlockDriverState *bdrv_next_node(BlockDriverState *bs);
-BlockDriverState *bdrv_next(BlockDriverState *bs);
+BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs);
BlockDriverState *bdrv_next_monitor_owned(BlockDriverState *bs);
int bdrv_is_encrypted(BlockDriverState *bs);
int bdrv_key_required(BlockDriverState *bs);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 44a222d..68d92b5 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -89,7 +89,6 @@ void blk_remove_all_bs(void);
const char *blk_name(BlockBackend *blk);
BlockBackend *blk_by_name(const char *name);
BlockBackend *blk_next(BlockBackend *blk);
-BlockDriverState *blk_next_root_bs(BlockDriverState *bs);
bool monitor_add_blk(BlockBackend *blk, const char *name, Error **errp);
void monitor_remove_blk(BlockBackend *blk);
diff --git a/migration/block.c b/migration/block.c
index 1743317..a7a76a0 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -383,6 +383,7 @@ static void init_blk_migration(QEMUFile *f)
BlockDriverState *bs;
BlkMigDevState *bmds;
int64_t sectors;
+ BdrvNextIterator *it = NULL;
block_mig_state.submitted = 0;
block_mig_state.read_done = 0;
@@ -392,7 +393,8 @@ static void init_blk_migration(QEMUFile *f)
block_mig_state.bulk_completed = 0;
block_mig_state.zero_blocks = migrate_zero_blocks();
- for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
+
+ while ((it = bdrv_next(it, &bs))) {
if (bdrv_is_read_only(bs)) {
continue;
}
diff --git a/monitor.c b/monitor.c
index d1c1930..2c87244 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3427,11 +3427,13 @@ void host_net_remove_completion(ReadLineState *rs, int nb_args, const char *str)
static void vm_completion(ReadLineState *rs, const char *str)
{
size_t len;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
len = strlen(str);
readline_set_completion_index(rs, len);
- while ((bs = bdrv_next(bs))) {
+
+ while ((it = bdrv_next(it, &bs))) {
SnapshotInfoList *snapshots, *snapshot;
AioContext *ctx = bdrv_get_aio_context(bs);
bool ok = false;
diff --git a/qmp.c b/qmp.c
index e784a67..8f8ae3a 100644
--- a/qmp.c
+++ b/qmp.c
@@ -181,6 +181,7 @@ void qmp_cont(Error **errp)
Error *local_err = NULL;
BlockBackend *blk;
BlockDriverState *bs;
+ BdrvNextIterator *it;
/* if there is a dump in background, we should wait until the dump
* finished */
@@ -199,7 +200,9 @@ void qmp_cont(Error **errp)
for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
blk_iostatus_reset(blk);
}
- for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
+
+ it = NULL;
+ while ((it = bdrv_next(it, &bs))) {
bdrv_add_key(bs, NULL, &local_err);
if (local_err) {
error_propagate(errp, local_err);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next()
2016-05-19 15:21 ` [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next() Kevin Wolf
@ 2016-05-20 7:54 ` Paolo Bonzini
2016-05-20 8:05 ` Kevin Wolf
0 siblings, 1 reply; 39+ messages in thread
From: Paolo Bonzini @ 2016-05-20 7:54 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
On 19/05/2016 17:21, Kevin Wolf wrote:
> We need to introduce a separate BdrvNextIterator struct that can keep
> more state than just the current BDS in order to avoid using the bs->blk
> pointer.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 40 +++++++----------------
> block/block-backend.c | 72 +++++++++++++++++++++++++++++-------------
> block/io.c | 13 ++++----
> block/snapshot.c | 30 +++++++++++-------
> blockdev.c | 3 +-
> include/block/block.h | 3 +-
> include/sysemu/block-backend.h | 1 -
> migration/block.c | 4 ++-
> monitor.c | 6 ++--
> qmp.c | 5 ++-
> 10 files changed, 102 insertions(+), 75 deletions(-)
>
> diff --git a/block.c b/block.c
> index fd4cf81..91bf431 100644
> --- a/block.c
> +++ b/block.c
> @@ -2870,25 +2870,6 @@ BlockDriverState *bdrv_next_node(BlockDriverState *bs)
> return QTAILQ_NEXT(bs, node_list);
> }
>
> -/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by
> - * the monitor or attached to a BlockBackend */
> -BlockDriverState *bdrv_next(BlockDriverState *bs)
> -{
> - if (!bs || bs->blk) {
> - bs = blk_next_root_bs(bs);
> - if (bs) {
> - return bs;
> - }
> - }
> -
> - /* Ignore all BDSs that are attached to a BlockBackend here; they have been
> - * handled by the above block already */
> - do {
> - bs = bdrv_next_monitor_owned(bs);
> - } while (bs && bs->blk);
> - return bs;
> -}
> -
> const char *bdrv_get_node_name(const BlockDriverState *bs)
> {
> return bs->node_name;
> @@ -3220,10 +3201,11 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
>
> void bdrv_invalidate_cache_all(Error **errp)
> {
> - BlockDriverState *bs = NULL;
> + BlockDriverState *bs;
> Error *local_err = NULL;
> + BdrvNextIterator *it = NULL;
>
> - while ((bs = bdrv_next(bs)) != NULL) {
> + while ((it = bdrv_next(it, &bs)) != NULL) {
> AioContext *aio_context = bdrv_get_aio_context(bs);
>
> aio_context_acquire(aio_context);
> @@ -3265,10 +3247,11 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs,
> int bdrv_inactivate_all(void)
> {
> BlockDriverState *bs = NULL;
> + BdrvNextIterator *it = NULL;
> int ret = 0;
> int pass;
>
> - while ((bs = bdrv_next(bs)) != NULL) {
> + while ((it = bdrv_next(it, &bs)) != NULL) {
> aio_context_acquire(bdrv_get_aio_context(bs));
> }
>
> @@ -3277,8 +3260,8 @@ int bdrv_inactivate_all(void)
> * the second pass sets the BDRV_O_INACTIVE flag so that no further write
> * is allowed. */
> for (pass = 0; pass < 2; pass++) {
> - bs = NULL;
> - while ((bs = bdrv_next(bs)) != NULL) {
> + it = NULL;
> + while ((it = bdrv_next(it, &bs)) != NULL) {
> ret = bdrv_inactivate_recurse(bs, pass);
> if (ret < 0) {
> goto out;
> @@ -3287,8 +3270,8 @@ int bdrv_inactivate_all(void)
> }
>
> out:
> - bs = NULL;
> - while ((bs = bdrv_next(bs)) != NULL) {
> + it = NULL;
> + while ((it = bdrv_next(it, &bs)) != NULL) {
> aio_context_release(bdrv_get_aio_context(bs));
> }
>
> @@ -3781,10 +3764,11 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
> */
> bool bdrv_is_first_non_filter(BlockDriverState *candidate)
> {
> - BlockDriverState *bs = NULL;
> + BlockDriverState *bs;
> + BdrvNextIterator *it = NULL;
>
> /* walk down the bs forest recursively */
> - while ((bs = bdrv_next(bs)) != NULL) {
> + while ((it = bdrv_next(it, &bs)) != NULL) {
> bool perm;
>
> /* try to recurse in this top level bs */
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 9dcac97..7f2eeb0 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -75,6 +75,7 @@ static const AIOCBInfo block_backend_aiocb_info = {
> };
>
> static void drive_info_del(DriveInfo *dinfo);
> +static BlockBackend *bdrv_first_blk(BlockDriverState *bs);
>
> /* All BlockBackends */
> static QTAILQ_HEAD(, BlockBackend) block_backends =
> @@ -286,28 +287,50 @@ BlockBackend *blk_next(BlockBackend *blk)
> : QTAILQ_FIRST(&monitor_block_backends);
> }
>
> -/*
> - * Iterates over all BlockDriverStates which are attached to a BlockBackend.
> - * This function is for use by bdrv_next().
> - *
> - * @bs must be NULL or a BDS that is attached to a BB.
> - */
> -BlockDriverState *blk_next_root_bs(BlockDriverState *bs)
> -{
> +struct BdrvNextIterator {
> + enum {
> + BDRV_NEXT_BACKEND_ROOTS,
> + BDRV_NEXT_MONITOR_OWNED,
> + } phase;
> BlockBackend *blk;
> + BlockDriverState *bs;
> +};
>
> - if (bs) {
> - assert(bs->blk);
> - blk = bs->blk;
> - } else {
> - blk = NULL;
> +/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by
> + * the monitor or attached to a BlockBackend */
> +BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs)
> +{
> + if (!it) {
> + it = g_new(BdrvNextIterator, 1);
Hmm, who frees it? (Especially if the caller exits the loop
prematurely, which means you cannot just free it before returning NULL).
I think it's better to:
- allocate the iterator on the stack and make bdrv_next return a BDS *
- and add a bdrv_first function that does this:
> + *it = (BdrvNextIterator) {
> + .phase = BDRV_NEXT_BACKEND_ROOTS,
> + };
and then returns bdrv_next(it);
- if desirable add a macro that abstracts the calls to bdrv_first and
bdrv_next.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next()
2016-05-20 7:54 ` Paolo Bonzini
@ 2016-05-20 8:05 ` Kevin Wolf
2016-05-20 8:10 ` Kevin Wolf
0 siblings, 1 reply; 39+ messages in thread
From: Kevin Wolf @ 2016-05-20 8:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-block, qemu-devel
Am 20.05.2016 um 09:54 hat Paolo Bonzini geschrieben:
>
>
> On 19/05/2016 17:21, Kevin Wolf wrote:
> > We need to introduce a separate BdrvNextIterator struct that can keep
> > more state than just the current BDS in order to avoid using the bs->blk
> > pointer.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > ---
> > block.c | 40 +++++++----------------
> > block/block-backend.c | 72 +++++++++++++++++++++++++++++-------------
> > block/io.c | 13 ++++----
> > block/snapshot.c | 30 +++++++++++-------
> > blockdev.c | 3 +-
> > include/block/block.h | 3 +-
> > include/sysemu/block-backend.h | 1 -
> > migration/block.c | 4 ++-
> > monitor.c | 6 ++--
> > qmp.c | 5 ++-
> > 10 files changed, 102 insertions(+), 75 deletions(-)
> >
> > diff --git a/block.c b/block.c
> > index fd4cf81..91bf431 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -2870,25 +2870,6 @@ BlockDriverState *bdrv_next_node(BlockDriverState *bs)
> > return QTAILQ_NEXT(bs, node_list);
> > }
> >
> > -/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by
> > - * the monitor or attached to a BlockBackend */
> > -BlockDriverState *bdrv_next(BlockDriverState *bs)
> > -{
> > - if (!bs || bs->blk) {
> > - bs = blk_next_root_bs(bs);
> > - if (bs) {
> > - return bs;
> > - }
> > - }
> > -
> > - /* Ignore all BDSs that are attached to a BlockBackend here; they have been
> > - * handled by the above block already */
> > - do {
> > - bs = bdrv_next_monitor_owned(bs);
> > - } while (bs && bs->blk);
> > - return bs;
> > -}
> > -
> > const char *bdrv_get_node_name(const BlockDriverState *bs)
> > {
> > return bs->node_name;
> > @@ -3220,10 +3201,11 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
> >
> > void bdrv_invalidate_cache_all(Error **errp)
> > {
> > - BlockDriverState *bs = NULL;
> > + BlockDriverState *bs;
> > Error *local_err = NULL;
> > + BdrvNextIterator *it = NULL;
> >
> > - while ((bs = bdrv_next(bs)) != NULL) {
> > + while ((it = bdrv_next(it, &bs)) != NULL) {
> > AioContext *aio_context = bdrv_get_aio_context(bs);
> >
> > aio_context_acquire(aio_context);
> > @@ -3265,10 +3247,11 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs,
> > int bdrv_inactivate_all(void)
> > {
> > BlockDriverState *bs = NULL;
> > + BdrvNextIterator *it = NULL;
> > int ret = 0;
> > int pass;
> >
> > - while ((bs = bdrv_next(bs)) != NULL) {
> > + while ((it = bdrv_next(it, &bs)) != NULL) {
> > aio_context_acquire(bdrv_get_aio_context(bs));
> > }
> >
> > @@ -3277,8 +3260,8 @@ int bdrv_inactivate_all(void)
> > * the second pass sets the BDRV_O_INACTIVE flag so that no further write
> > * is allowed. */
> > for (pass = 0; pass < 2; pass++) {
> > - bs = NULL;
> > - while ((bs = bdrv_next(bs)) != NULL) {
> > + it = NULL;
> > + while ((it = bdrv_next(it, &bs)) != NULL) {
> > ret = bdrv_inactivate_recurse(bs, pass);
> > if (ret < 0) {
> > goto out;
> > @@ -3287,8 +3270,8 @@ int bdrv_inactivate_all(void)
> > }
> >
> > out:
> > - bs = NULL;
> > - while ((bs = bdrv_next(bs)) != NULL) {
> > + it = NULL;
> > + while ((it = bdrv_next(it, &bs)) != NULL) {
> > aio_context_release(bdrv_get_aio_context(bs));
> > }
> >
> > @@ -3781,10 +3764,11 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
> > */
> > bool bdrv_is_first_non_filter(BlockDriverState *candidate)
> > {
> > - BlockDriverState *bs = NULL;
> > + BlockDriverState *bs;
> > + BdrvNextIterator *it = NULL;
> >
> > /* walk down the bs forest recursively */
> > - while ((bs = bdrv_next(bs)) != NULL) {
> > + while ((it = bdrv_next(it, &bs)) != NULL) {
> > bool perm;
> >
> > /* try to recurse in this top level bs */
> > diff --git a/block/block-backend.c b/block/block-backend.c
> > index 9dcac97..7f2eeb0 100644
> > --- a/block/block-backend.c
> > +++ b/block/block-backend.c
> > @@ -75,6 +75,7 @@ static const AIOCBInfo block_backend_aiocb_info = {
> > };
> >
> > static void drive_info_del(DriveInfo *dinfo);
> > +static BlockBackend *bdrv_first_blk(BlockDriverState *bs);
> >
> > /* All BlockBackends */
> > static QTAILQ_HEAD(, BlockBackend) block_backends =
> > @@ -286,28 +287,50 @@ BlockBackend *blk_next(BlockBackend *blk)
> > : QTAILQ_FIRST(&monitor_block_backends);
> > }
> >
> > -/*
> > - * Iterates over all BlockDriverStates which are attached to a BlockBackend.
> > - * This function is for use by bdrv_next().
> > - *
> > - * @bs must be NULL or a BDS that is attached to a BB.
> > - */
> > -BlockDriverState *blk_next_root_bs(BlockDriverState *bs)
> > -{
> > +struct BdrvNextIterator {
> > + enum {
> > + BDRV_NEXT_BACKEND_ROOTS,
> > + BDRV_NEXT_MONITOR_OWNED,
> > + } phase;
> > BlockBackend *blk;
> > + BlockDriverState *bs;
> > +};
> >
> > - if (bs) {
> > - assert(bs->blk);
> > - blk = bs->blk;
> > - } else {
> > - blk = NULL;
> > +/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by
> > + * the monitor or attached to a BlockBackend */
> > +BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs)
> > +{
> > + if (!it) {
> > + it = g_new(BdrvNextIterator, 1);
>
> Hmm, who frees it? (Especially if the caller exits the loop
> prematurely, which means you cannot just free it before returning NULL).
> I think it's better to:
>
> - allocate the iterator on the stack and make bdrv_next return a BDS *
>
> - and add a bdrv_first function that does this:
>
> > + *it = (BdrvNextIterator) {
> > + .phase = BDRV_NEXT_BACKEND_ROOTS,
> > + };
>
> and then returns bdrv_next(it);
>
> - if desirable add a macro that abstracts the calls to bdrv_first and
> bdrv_next.
Already posted a fix. I chose to keep the interface and free the
BdrvNextIterator inside bdrv_next(), when we return NULL after the last
element.
Kevin
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next()
2016-05-20 8:05 ` Kevin Wolf
@ 2016-05-20 8:10 ` Kevin Wolf
2016-05-20 9:39 ` Paolo Bonzini
0 siblings, 1 reply; 39+ messages in thread
From: Kevin Wolf @ 2016-05-20 8:10 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-block, qemu-devel
Am 20.05.2016 um 10:05 hat Kevin Wolf geschrieben:
> Am 20.05.2016 um 09:54 hat Paolo Bonzini geschrieben:
> > > +/* Iterates over all top-level BlockDriverStates, i.e. BDSs that are owned by
> > > + * the monitor or attached to a BlockBackend */
> > > +BdrvNextIterator *bdrv_next(BdrvNextIterator *it, BlockDriverState **bs)
> > > +{
> > > + if (!it) {
> > > + it = g_new(BdrvNextIterator, 1);
> >
> > Hmm, who frees it? (Especially if the caller exits the loop
> > prematurely, which means you cannot just free it before returning NULL).
> > I think it's better to:
> >
> > - allocate the iterator on the stack and make bdrv_next return a BDS *
> >
> > - and add a bdrv_first function that does this:
> >
> > > + *it = (BdrvNextIterator) {
> > > + .phase = BDRV_NEXT_BACKEND_ROOTS,
> > > + };
> >
> > and then returns bdrv_next(it);
> >
> > - if desirable add a macro that abstracts the calls to bdrv_first and
> > bdrv_next.
>
> Already posted a fix. I chose to keep the interface and free the
> BdrvNextIterator inside bdrv_next(), when we return NULL after the last
> element.
Oops, should have actually read your email... You're right about callers
that prematurely exit the loop, of course.
I still don't really like first/next interfaces, though. Perhaps start
the iteration with bs == NULL instead of it == NULL?
Kevin
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next()
2016-05-20 8:10 ` Kevin Wolf
@ 2016-05-20 9:39 ` Paolo Bonzini
2016-05-20 10:26 ` Kevin Wolf
0 siblings, 1 reply; 39+ messages in thread
From: Paolo Bonzini @ 2016-05-20 9:39 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block
On 20/05/2016 10:10, Kevin Wolf wrote:
>> > Already posted a fix. I chose to keep the interface and free the
>> > BdrvNextIterator inside bdrv_next(), when we return NULL after the last
>> > element.
> Oops, should have actually read your email... You're right about callers
> that prematurely exit the loop, of course.
>
> I still don't really like first/next interfaces, though. Perhaps start
> the iteration with bs == NULL instead of it == NULL?
Yet another alternative is to add a BDRV_NEXT_ITERATOR_INITIALIZER
macro. I like it because it's less magic than "x is NULL" and because I
would prefer an interface with just the BdrvNextIterator* as the
argument to bdrv_next.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next()
2016-05-20 9:39 ` Paolo Bonzini
@ 2016-05-20 10:26 ` Kevin Wolf
2016-05-20 10:46 ` Paolo Bonzini
0 siblings, 1 reply; 39+ messages in thread
From: Kevin Wolf @ 2016-05-20 10:26 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-block
Am 20.05.2016 um 11:39 hat Paolo Bonzini geschrieben:
>
>
> On 20/05/2016 10:10, Kevin Wolf wrote:
> >> > Already posted a fix. I chose to keep the interface and free the
> >> > BdrvNextIterator inside bdrv_next(), when we return NULL after the last
> >> > element.
> > Oops, should have actually read your email... You're right about callers
> > that prematurely exit the loop, of course.
> >
> > I still don't really like first/next interfaces, though. Perhaps start
> > the iteration with bs == NULL instead of it == NULL?
>
> Yet another alternative is to add a BDRV_NEXT_ITERATOR_INITIALIZER
> macro. I like it because it's less magic than "x is NULL" and because I
> would prefer an interface with just the BdrvNextIterator* as the
> argument to bdrv_next.
Hm, we have a few instances where an iterator variable is used for
multiple loops, so we need to be able to use it in an assignment, i.e.
it should be a compound literal. On the other hand, I seem to remember
that compound literals can't be used as initialisers.
Maybe a bdrv_next_iterator_reset() function then? Which would be like
first/next, except that it doesn't return the first value yet.
Kevin
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next()
2016-05-20 10:26 ` Kevin Wolf
@ 2016-05-20 10:46 ` Paolo Bonzini
0 siblings, 0 replies; 39+ messages in thread
From: Paolo Bonzini @ 2016-05-20 10:46 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block
On 20/05/2016 12:26, Kevin Wolf wrote:
> Hm, we have a few instances where an iterator variable is used for
> multiple loops, so we need to be able to use it in an assignment, i.e.
> it should be a compound literal. On the other hand, I seem to remember
> that compound literals can't be used as initialisers.
>
> Maybe a bdrv_next_iterator_reset() function then? Which would be like
> first/next, except that it doesn't return the first value yet.
What qemu/queue.h does is is provide both FOO_INITIALIZER and
FOO_INIT(&some_foo).
Thanks,
Paolo
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PULL 22/31] block: Don't return throttling info in query-named-block-nodes
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (20 preceding siblings ...)
2016-05-19 15:21 ` [Qemu-devel] [PULL 21/31] block: Avoid bs->blk in bdrv_next() Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 15:22 ` [Qemu-devel] [PULL 23/31] block: Remove BlockDriverState.blk Kevin Wolf
` (9 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
query-named-block-nodes should not return information that is related
to the attached BlockBackend rather than the node itself, so throttling
information needs to be removed from it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/qapi.c | 6 +++---
tests/qemu-iotests/096 | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index b0d8d6b..5594f74 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -67,10 +67,10 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
info->backing_file_depth = bdrv_get_backing_file_depth(bs);
info->detect_zeroes = bs->detect_zeroes;
- if (bs->blk && blk_get_public(bs->blk)->throttle_state) {
+ if (blk && blk_get_public(blk)->throttle_state) {
ThrottleConfig cfg;
- throttle_group_get_config(bs->blk, &cfg);
+ throttle_group_get_config(blk, &cfg);
info->bps = cfg.buckets[THROTTLE_BPS_TOTAL].avg;
info->bps_rd = cfg.buckets[THROTTLE_BPS_READ].avg;
@@ -118,7 +118,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
info->iops_size = cfg.op_size;
info->has_group = true;
- info->group = g_strdup(throttle_group_get_name(bs->blk));
+ info->group = g_strdup(throttle_group_get_name(blk));
}
info->write_threshold = bdrv_write_threshold_get(bs);
diff --git a/tests/qemu-iotests/096 b/tests/qemu-iotests/096
index e34204b..aeeb375 100644
--- a/tests/qemu-iotests/096
+++ b/tests/qemu-iotests/096
@@ -45,8 +45,9 @@ class TestLiveSnapshot(iotests.QMPTestCase):
os.remove(self.target_img)
def checkConfig(self, active_layer):
- result = self.vm.qmp('query-named-block-nodes')
+ result = self.vm.qmp('query-block')
for r in result['return']:
+ r = r['inserted']
if r['node-name'] == active_layer:
self.assertEqual(r['group'], self.group)
self.assertEqual(r['iops'], self.iops)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 23/31] block: Remove BlockDriverState.blk
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (21 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 22/31] block: Don't return throttling info in query-named-block-nodes Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 15:22 ` [Qemu-devel] [PULL 24/31] block: Propagate AioContext change to all children Kevin Wolf
` (8 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
This patch removes the remaining users of bs->blk, which will allow us
to have multiple BBs on top of a single BDS. In the meantime, all checks
that are currently in place to prevent the user from creating such
setups can be switched to bdrv_has_blk() instead of accessing BDS.blk.
Future patches can allow them and e.g. enable users to mirror to a block
device that already has a BlockBackend on it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 10 +---------
block/block-backend.c | 8 --------
block/mirror.c | 4 ++--
blockdev.c | 16 ++++++++--------
include/block/block_int.h | 3 +--
5 files changed, 12 insertions(+), 29 deletions(-)
diff --git a/block.c b/block.c
index 91bf431..cbc10f7 100644
--- a/block.c
+++ b/block.c
@@ -2226,14 +2226,6 @@ static void change_parent_backing_link(BlockDriverState *from,
{
BdrvChild *c, *next;
- if (from->blk) {
- /* FIXME We bypass blk_set_bs(), so we need to make these updates
- * manually. The root problem is not in this change function, but the
- * existence of BlockDriverState.blk. */
- to->blk = from->blk;
- from->blk = NULL;
- }
-
QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
assert(c->role != &child_backing);
c->bs = to;
@@ -2875,7 +2867,7 @@ const char *bdrv_get_node_name(const BlockDriverState *bs)
return bs->node_name;
}
-static const char *bdrv_get_parent_name(const BlockDriverState *bs)
+const char *bdrv_get_parent_name(const BlockDriverState *bs)
{
BdrvChild *c;
const char *name;
diff --git a/block/block-backend.c b/block/block-backend.c
index 7f2eeb0..6928d61 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -152,7 +152,6 @@ BlockBackend *blk_new_with_bs(Error **errp)
bs = bdrv_new_root();
blk->root = bdrv_root_attach_child(bs, "root", &child_root);
blk->root->opaque = blk;
- bs->blk = blk;
return blk;
}
@@ -422,12 +421,10 @@ static BlockBackend *bdrv_first_blk(BlockDriverState *bs)
BdrvChild *child;
QLIST_FOREACH(child, &bs->parents, next_parent) {
if (child->role == &child_root) {
- assert(bs->blk);
return child->opaque;
}
}
- assert(!bs->blk);
return NULL;
}
@@ -495,8 +492,6 @@ BlockBackend *blk_by_public(BlockBackendPublic *public)
*/
void blk_remove_bs(BlockBackend *blk)
{
- assert(blk->root->bs->blk == blk);
-
notifier_list_notify(&blk->remove_bs_notifiers, blk);
if (blk->public.throttle_state) {
throttle_timers_detach_aio_context(&blk->public.throttle_timers);
@@ -504,7 +499,6 @@ void blk_remove_bs(BlockBackend *blk)
blk_update_root_state(blk);
- blk->root->bs->blk = NULL;
bdrv_root_unref_child(blk->root);
blk->root = NULL;
}
@@ -514,11 +508,9 @@ void blk_remove_bs(BlockBackend *blk)
*/
void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
{
- assert(!blk->root && !bs->blk);
bdrv_ref(bs);
blk->root = bdrv_root_attach_child(bs, "root", &child_root);
blk->root->opaque = blk;
- bs->blk = blk;
notifier_list_notify(&blk->insert_bs_notifiers, blk);
if (blk->public.throttle_state) {
diff --git a/block/mirror.c b/block/mirror.c
index 71d7a4e..b9986d8 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -468,7 +468,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
/* This was checked in mirror_start_job(), but meanwhile one of the
* nodes could have been newly attached to a BlockBackend. */
- if (to_replace->blk && s->target->blk) {
+ if (bdrv_has_blk(to_replace) && bdrv_has_blk(s->target)) {
error_report("block job: Can't create node with two BlockBackends");
data->ret = -EINVAL;
goto out;
@@ -831,7 +831,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
} else {
replaced_bs = bs;
}
- if (replaced_bs->blk && target->blk) {
+ if (bdrv_has_blk(replaced_bs) && bdrv_has_blk(target)) {
error_setg(errp, "Can't create node with two BlockBackends");
return;
}
diff --git a/blockdev.c b/blockdev.c
index 442ca8d..b6fa210 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1776,9 +1776,9 @@ static void external_snapshot_prepare(BlkActionState *common,
return;
}
- if (state->new_bs->blk != NULL) {
+ if (bdrv_has_blk(state->new_bs)) {
error_setg(errp, "The snapshot is already in use by %s",
- blk_name(state->new_bs->blk));
+ bdrv_get_parent_name(state->new_bs));
return;
}
@@ -2494,9 +2494,9 @@ void qmp_x_blockdev_insert_medium(const char *device, const char *node_name,
return;
}
- if (bs->blk) {
+ if (bdrv_has_blk(bs)) {
error_setg(errp, "Node '%s' is already in use by '%s'", node_name,
- blk_name(bs->blk));
+ bdrv_get_parent_name(bs));
return;
}
@@ -3441,7 +3441,7 @@ static void blockdev_mirror_common(BlockDriverState *bs,
if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_MIRROR_TARGET, errp)) {
return;
}
- if (target->blk) {
+ if (bdrv_has_blk(target)) {
error_setg(errp, "Cannot mirror to an attached block device");
return;
}
@@ -4030,15 +4030,15 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
bs = blk_bs(blk);
aio_context = blk_get_aio_context(blk);
} else {
+ blk = NULL;
bs = bdrv_find_node(node_name);
if (!bs) {
error_setg(errp, "Cannot find node %s", node_name);
return;
}
- blk = bs->blk;
- if (blk) {
+ if (bdrv_has_blk(bs)) {
error_setg(errp, "Node %s is in use by %s",
- node_name, blk_name(blk));
+ node_name, bdrv_get_parent_name(bs));
return;
}
aio_context = bdrv_get_aio_context(bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 80e2da5..b6f4755 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -418,8 +418,6 @@ struct BlockDriverState {
BlockDriver *drv; /* NULL means no media */
void *opaque;
- BlockBackend *blk; /* owning backend, if any */
-
AioContext *aio_context; /* event loop used for fd handlers, timers, etc */
/* long-running tasks intended to always use the same AioContext as this
* BDS may register themselves in this list to be notified of changes
@@ -724,6 +722,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
const BdrvChildRole *child_role);
void bdrv_root_unref_child(BdrvChild *child);
+const char *bdrv_get_parent_name(const BlockDriverState *bs);
void blk_dev_change_media_cb(BlockBackend *blk, bool load);
bool blk_dev_has_removable_media(BlockBackend *blk);
bool blk_dev_has_tray(BlockBackend *blk);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 24/31] block: Propagate AioContext change to all children
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (22 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 23/31] block: Remove BlockDriverState.blk Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 15:22 ` [Qemu-devel] [PULL 25/31] qcow2: fix condition in is_zero_cluster Kevin Wolf
` (7 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Instead of propagating any change of a BDS's AioContext only to its file
and backing children and letting driver-specific code do the rest, just
propagate it to all and drop the thus superfluous implementations of
bdrv_{at,de}tach_aio_context() in Quorum, blkverify and VMDK.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 16 ++++++----------
block/blkverify.c | 19 -------------------
block/quorum.c | 24 ------------------------
block/vmdk.c | 23 -----------------------
4 files changed, 6 insertions(+), 76 deletions(-)
diff --git a/block.c b/block.c
index cbc10f7..1205ef8 100644
--- a/block.c
+++ b/block.c
@@ -3609,6 +3609,7 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs)
void bdrv_detach_aio_context(BlockDriverState *bs)
{
BdrvAioNotifier *baf;
+ BdrvChild *child;
if (!bs->drv) {
return;
@@ -3621,11 +3622,8 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
if (bs->drv->bdrv_detach_aio_context) {
bs->drv->bdrv_detach_aio_context(bs);
}
- if (bs->file) {
- bdrv_detach_aio_context(bs->file->bs);
- }
- if (bs->backing) {
- bdrv_detach_aio_context(bs->backing->bs);
+ QLIST_FOREACH(child, &bs->children, next) {
+ bdrv_detach_aio_context(child->bs);
}
bs->aio_context = NULL;
@@ -3635,6 +3633,7 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
AioContext *new_context)
{
BdrvAioNotifier *ban;
+ BdrvChild *child;
if (!bs->drv) {
return;
@@ -3642,11 +3641,8 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
bs->aio_context = new_context;
- if (bs->backing) {
- bdrv_attach_aio_context(bs->backing->bs, new_context);
- }
- if (bs->file) {
- bdrv_attach_aio_context(bs->file->bs, new_context);
+ QLIST_FOREACH(child, &bs->children, next) {
+ bdrv_attach_aio_context(child->bs, new_context);
}
if (bs->drv->bdrv_attach_aio_context) {
bs->drv->bdrv_attach_aio_context(bs, new_context);
diff --git a/block/blkverify.c b/block/blkverify.c
index 9414b7a..4045396 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -293,22 +293,6 @@ static bool blkverify_recurse_is_first_non_filter(BlockDriverState *bs,
return bdrv_recurse_is_first_non_filter(s->test_file->bs, candidate);
}
-/* Propagate AioContext changes to ->test_file */
-static void blkverify_detach_aio_context(BlockDriverState *bs)
-{
- BDRVBlkverifyState *s = bs->opaque;
-
- bdrv_detach_aio_context(s->test_file->bs);
-}
-
-static void blkverify_attach_aio_context(BlockDriverState *bs,
- AioContext *new_context)
-{
- BDRVBlkverifyState *s = bs->opaque;
-
- bdrv_attach_aio_context(s->test_file->bs, new_context);
-}
-
static void blkverify_refresh_filename(BlockDriverState *bs, QDict *options)
{
BDRVBlkverifyState *s = bs->opaque;
@@ -356,9 +340,6 @@ static BlockDriver bdrv_blkverify = {
.bdrv_aio_writev = blkverify_aio_writev,
.bdrv_aio_flush = blkverify_aio_flush,
- .bdrv_attach_aio_context = blkverify_attach_aio_context,
- .bdrv_detach_aio_context = blkverify_detach_aio_context,
-
.is_filter = true,
.bdrv_recurse_is_first_non_filter = blkverify_recurse_is_first_non_filter,
};
diff --git a/block/quorum.c b/block/quorum.c
index 1ec3511..ec6f3b9 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -989,27 +989,6 @@ static void quorum_close(BlockDriverState *bs)
g_free(s->children);
}
-static void quorum_detach_aio_context(BlockDriverState *bs)
-{
- BDRVQuorumState *s = bs->opaque;
- int i;
-
- for (i = 0; i < s->num_children; i++) {
- bdrv_detach_aio_context(s->children[i]->bs);
- }
-}
-
-static void quorum_attach_aio_context(BlockDriverState *bs,
- AioContext *new_context)
-{
- BDRVQuorumState *s = bs->opaque;
- int i;
-
- for (i = 0; i < s->num_children; i++) {
- bdrv_attach_aio_context(s->children[i]->bs, new_context);
- }
-}
-
static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs,
Error **errp)
{
@@ -1127,9 +1106,6 @@ static BlockDriver bdrv_quorum = {
.bdrv_aio_readv = quorum_aio_readv,
.bdrv_aio_writev = quorum_aio_writev,
- .bdrv_detach_aio_context = quorum_detach_aio_context,
- .bdrv_attach_aio_context = quorum_attach_aio_context,
-
.bdrv_add_child = quorum_add_child,
.bdrv_del_child = quorum_del_child,
diff --git a/block/vmdk.c b/block/vmdk.c
index e6c97c2..5b3b067 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2343,27 +2343,6 @@ static int vmdk_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
return 0;
}
-static void vmdk_detach_aio_context(BlockDriverState *bs)
-{
- BDRVVmdkState *s = bs->opaque;
- int i;
-
- for (i = 0; i < s->num_extents; i++) {
- bdrv_detach_aio_context(s->extents[i].file->bs);
- }
-}
-
-static void vmdk_attach_aio_context(BlockDriverState *bs,
- AioContext *new_context)
-{
- BDRVVmdkState *s = bs->opaque;
- int i;
-
- for (i = 0; i < s->num_extents; i++) {
- bdrv_attach_aio_context(s->extents[i].file->bs, new_context);
- }
-}
-
static QemuOptsList vmdk_create_opts = {
.name = "vmdk-create-opts",
.head = QTAILQ_HEAD_INITIALIZER(vmdk_create_opts.head),
@@ -2433,8 +2412,6 @@ static BlockDriver bdrv_vmdk = {
.bdrv_get_specific_info = vmdk_get_specific_info,
.bdrv_refresh_limits = vmdk_refresh_limits,
.bdrv_get_info = vmdk_get_info,
- .bdrv_detach_aio_context = vmdk_detach_aio_context,
- .bdrv_attach_aio_context = vmdk_attach_aio_context,
.supports_backing = true,
.create_opts = &vmdk_create_opts,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 25/31] qcow2: fix condition in is_zero_cluster
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (23 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 24/31] block: Propagate AioContext change to all children Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 15:22 ` [Qemu-devel] [PULL 26/31] qcow2: Fix write_zeroes with partially allocated backing file cluster Kevin Wolf
` (6 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: "Denis V. Lunev" <den@openvz.org>
We should check for (res & BDRV_BLOCK_ZERO) only. The situation when we
will have !(res & BDRV_BLOCK_DATA) and will not have BDRV_BLOCK_ZERO is
not possible for images with bdi.unallocated_blocks_are_zero == true.
For those images where it's false, however, it can happen and we must
not consider the data zeroed then or we would corrupt the image.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 62febfc..a6012dc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2412,7 +2412,7 @@ static bool is_zero_cluster(BlockDriverState *bs, int64_t start)
BlockDriverState *file;
int64_t res = bdrv_get_block_status_above(bs, NULL, start,
s->cluster_sectors, &nr, &file);
- return res >= 0 && ((res & BDRV_BLOCK_ZERO) || !(res & BDRV_BLOCK_DATA));
+ return res >= 0 && (res & BDRV_BLOCK_ZERO);
}
static bool is_zero_cluster_top_locked(BlockDriverState *bs, int64_t start)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 26/31] qcow2: Fix write_zeroes with partially allocated backing file cluster
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (24 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 25/31] qcow2: fix condition in is_zero_cluster Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 15:22 ` [Qemu-devel] [PULL 27/31] qemu-iotests: Some more write_zeroes tests Kevin Wolf
` (5 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
In order to correctly check whether a given cluster is read as zero, we
don't only need to check whether bdrv_get_block_status_above() sets
BDRV_BLOCK_ZERO, but also if all sectors for the whole cluster have the
same status.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
block/qcow2.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index a6012dc..c197ff3 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2412,7 +2412,7 @@ static bool is_zero_cluster(BlockDriverState *bs, int64_t start)
BlockDriverState *file;
int64_t res = bdrv_get_block_status_above(bs, NULL, start,
s->cluster_sectors, &nr, &file);
- return res >= 0 && (res & BDRV_BLOCK_ZERO);
+ return res >= 0 && (res & BDRV_BLOCK_ZERO) && nr == s->cluster_sectors;
}
static bool is_zero_cluster_top_locked(BlockDriverState *bs, int64_t start)
@@ -2423,6 +2423,7 @@ static bool is_zero_cluster_top_locked(BlockDriverState *bs, int64_t start)
int ret;
ret = qcow2_get_cluster_offset(bs, start << BDRV_SECTOR_BITS, &nr, &off);
+ assert(nr == s->cluster_sectors);
return ret == QCOW2_CLUSTER_UNALLOCATED || ret == QCOW2_CLUSTER_ZERO;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 27/31] qemu-iotests: Some more write_zeroes tests
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (25 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 26/31] qcow2: Fix write_zeroes with partially allocated backing file cluster Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 15:22 ` [Qemu-devel] [PULL 28/31] block: clarify error message for qmp-eject Kevin Wolf
` (4 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
This covers some more write_zeroes cases which are relevant for the
recent qcow2 optimisations that check the allocation status of the
backing file for partial cluster write_zeroes requests.
This needs to be separate from 034 because we can only support qcow2 in
this test case for multiple reasons: We check the allocation status
after write_zeroes with 'qemu-img map' and the optimised behaviour that
produces zero clusters is only implemented in qcow2; second, the map
command returns offsets that are qcow2 specific; and finally, we also
use 512 byte clusters which aren't supported for formats like qed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/qemu-iotests/154 | 265 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/154.out | 242 +++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 508 insertions(+)
create mode 100755 tests/qemu-iotests/154
create mode 100644 tests/qemu-iotests/154.out
diff --git a/tests/qemu-iotests/154 b/tests/qemu-iotests/154
new file mode 100755
index 0000000..23f1b3a
--- /dev/null
+++ b/tests/qemu-iotests/154
@@ -0,0 +1,265 @@
+#!/bin/bash
+#
+# qcow2 specific bdrv_write_zeroes tests with backing files (complements 034)
+#
+# Copyright (C) 2016 Red Hat, Inc.
+#
+# 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=kwolf@redhat.com
+
+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
+
+CLUSTER_SIZE=4k
+size=128M
+
+echo
+echo == backing file contains zeros ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Make sure that the whole cluster is allocated even for partial write_zeroes
+# when the backing file contains zeros
+
+# X = non-zero data sector in backing file
+# - = sector unallocated in whole backing chain
+# 0 = sector touched by write_zeroes request
+
+# 1. Tail unaligned: 00 00 -- --
+# 2. Head unaligned: -- -- 00 00
+# 3. Both unaligned: -- 00 00 --
+# 4. Both, 2 clusters: -- -- -- 00 | 00 -- -- --
+
+$QEMU_IO -c "write -z 0 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "write -z 10k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "write -z 17k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "write -z 27k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+echo
+echo == backing file contains non-zero data before write_zeroes ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Single cluster; non-zero data at the cluster start
+# ... | XX -- 00 -- | ...
+$QEMU_IO -c "write -P 0x11 32k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 34k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 32k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 33k 3k" "$TEST_IMG" | _filter_qemu_io
+
+# Single cluster; non-zero data exists, but not at the cluster start
+# ... | -- XX 00 -- | ...
+$QEMU_IO -c "write -P 0x11 65k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 66k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 65k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 64k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 66k 2k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+echo
+echo == backing file contains non-zero data after write_zeroes ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Single cluster; non-zero data directly after request
+# ... | -- 00 XX -- | ...
+$QEMU_IO -c "write -P 0x11 34k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 33k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 32k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 34k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 35k 1k" "$TEST_IMG" | _filter_qemu_io
+
+# Single cluster; non-zero data exists, but not directly after request
+# ... | -- 00 -- XX | ...
+$QEMU_IO -c "write -P 0x11 43k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 41k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 43k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 40k 3k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+echo
+echo == spanning two clusters, non-zero before request ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Two clusters; non-zero data before request:
+# 1. At cluster start: 32k: XX -- -- 00 | 00 -- -- --
+# 2. Between unallocated space: 48k: -- XX -- 00 | 00 -- -- --
+# 3. Directly before request: 64k: -- -- XX 00 | 00 -- -- --
+
+$QEMU_IO -c "write -P 0x11 32k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 35k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 32k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 33k 7k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IO -c "write -P 0x11 49k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 51k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 48k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 49k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 50k 6k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IO -c "write -P 0x11 66k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 67k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 64k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 66k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 67k 5k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+echo
+echo == spanning two clusters, non-zero after request ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Two clusters; non-zero data after request:
+# 1. Directly after request: 32k: -- -- -- 00 | 00 XX -- --
+# 2. Between unallocated space: 48k: -- -- -- 00 | 00 -- XX --
+# 3. At cluster end: 64k: -- -- -- 00 | 00 -- -- XX
+
+$QEMU_IO -c "write -P 0x11 37k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 35k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 32k 5k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 37k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 38k 2k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IO -c "write -P 0x11 54k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 51k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 48k 6k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 54k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 55k 1k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IO -c "write -P 0x11 71k 1k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 67k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 64k 7k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 71k 1k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+echo
+echo == spanning two clusters, partially overwriting backing file ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Backing file: -- -- XX XX | XX XX -- --
+# Active layer: -- -- XX 00 | 00 XX -- --
+
+$QEMU_IO -c "write -P 0x11 2k 4k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 3k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 0k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 2k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 3k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 5k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 6k 2k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+echo
+echo == spanning multiple clusters, non-zero in first cluster ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Backing file: 64k: XX XX -- -- | -- -- -- -- | -- -- -- --
+# Active layer: 64k: XX XX 00 00 | 00 00 00 00 | 00 -- -- --
+
+$QEMU_IO -c "write -P 0x11 64k 2k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 66k 7k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 64k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 66k 10k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+echo
+echo == spanning multiple clusters, non-zero in intermediate cluster ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Backing file: 64k: -- -- -- -- | -- XX XX -- | -- -- -- --
+# Active layer: 64k: -- -- 00 00 | 00 00 00 00 | 00 -- -- --
+
+$QEMU_IO -c "write -P 0x11 69k 2k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 66k 7k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 64k 12k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+echo
+echo == spanning multiple clusters, non-zero in final cluster ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Backing file: 64k: -- -- -- -- | -- -- -- -- | -- -- XX XX
+# Active layer: 64k: -- -- 00 00 | 00 00 00 00 | 00 -- XX XX
+
+$QEMU_IO -c "write -P 0x11 74k 2k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 66k 7k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 64k 10k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 74k 2k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+echo
+echo == spanning multiple clusters, partially overwriting backing file ==
+
+CLUSTER_SIZE=512 TEST_IMG="$TEST_IMG.base" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base"
+
+# Backing file: 64k: -- XX XX XX | XX XX XX XX | XX XX XX --
+# Active layer: 64k: -- XX 00 00 | 00 00 00 00 | 00 XX XX --
+
+$QEMU_IO -c "write -P 0x11 65k 10k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "write -z 66k 7k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 64k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 65k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 66k 7k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0x11 73k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 75k 1k" "$TEST_IMG" | _filter_qemu_io
+
+$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/154.out b/tests/qemu-iotests/154.out
new file mode 100644
index 0000000..8946b73
--- /dev/null
+++ b/tests/qemu-iotests/154.out
@@ -0,0 +1,242 @@
+QA output created by 154
+
+== backing file contains zeros ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 2048/2048 bytes at offset 0
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 10240
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 17408
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 27648
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 4096, "depth": 0, "zero": true, "data": false},
+{ "start": 4096, "length": 4096, "depth": 1, "zero": true, "data": false},
+{ "start": 8192, "length": 4096, "depth": 0, "zero": true, "data": false},
+{ "start": 12288, "length": 4096, "depth": 1, "zero": true, "data": false},
+{ "start": 16384, "length": 4096, "depth": 0, "zero": true, "data": false},
+{ "start": 20480, "length": 4096, "depth": 1, "zero": true, "data": false},
+{ "start": 24576, "length": 8192, "depth": 0, "zero": true, "data": false},
+{ "start": 32768, "length": 134184960, "depth": 1, "zero": true, "data": false}]
+
+== backing file contains non-zero data before write_zeroes ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 1024/1024 bytes at offset 32768
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 34816
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 32768
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 3072/3072 bytes at offset 33792
+3 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 66560
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 67584
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 66560
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 65536
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 67584
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 32768, "depth": 1, "zero": true, "data": false},
+{ "start": 32768, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": 20480},
+{ "start": 36864, "length": 28672, "depth": 1, "zero": true, "data": false},
+{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": 24576},
+{ "start": 69632, "length": 134148096, "depth": 1, "zero": true, "data": false}]
+
+== backing file contains non-zero data after write_zeroes ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 1024/1024 bytes at offset 34816
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 33792
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 32768
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 34816
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 35840
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 44032
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 41984
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 44032
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 3072/3072 bytes at offset 40960
+3 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 32768, "depth": 1, "zero": true, "data": false},
+{ "start": 32768, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": 20480},
+{ "start": 36864, "length": 4096, "depth": 1, "zero": true, "data": false},
+{ "start": 40960, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": 24576},
+{ "start": 45056, "length": 134172672, "depth": 1, "zero": true, "data": false}]
+
+== spanning two clusters, non-zero before request ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 1024/1024 bytes at offset 32768
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 35840
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 32768
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 7168/7168 bytes at offset 33792
+7 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 50176
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 52224
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 49152
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 50176
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 6144/6144 bytes at offset 51200
+6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 67584
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 68608
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 65536
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 67584
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 5120/5120 bytes at offset 68608
+5 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 32768, "depth": 1, "zero": true, "data": false},
+{ "start": 32768, "length": 8192, "depth": 0, "zero": false, "data": true, "offset": 20480},
+{ "start": 40960, "length": 8192, "depth": 1, "zero": true, "data": false},
+{ "start": 49152, "length": 8192, "depth": 0, "zero": false, "data": true, "offset": 28672},
+{ "start": 57344, "length": 8192, "depth": 1, "zero": true, "data": false},
+{ "start": 65536, "length": 8192, "depth": 0, "zero": false, "data": true, "offset": 36864},
+{ "start": 73728, "length": 134144000, "depth": 1, "zero": true, "data": false}]
+
+== spanning two clusters, non-zero after request ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 1024/1024 bytes at offset 37888
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 35840
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 5120/5120 bytes at offset 32768
+5 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 37888
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 38912
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 55296
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 52224
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 6144/6144 bytes at offset 49152
+6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 55296
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 56320
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 72704
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 68608
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 7168/7168 bytes at offset 65536
+7 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 72704
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 32768, "depth": 1, "zero": true, "data": false},
+{ "start": 32768, "length": 8192, "depth": 0, "zero": false, "data": true, "offset": 20480},
+{ "start": 40960, "length": 8192, "depth": 1, "zero": true, "data": false},
+{ "start": 49152, "length": 8192, "depth": 0, "zero": false, "data": true, "offset": 28672},
+{ "start": 57344, "length": 8192, "depth": 1, "zero": true, "data": false},
+{ "start": 65536, "length": 8192, "depth": 0, "zero": false, "data": true, "offset": 36864},
+{ "start": 73728, "length": 134144000, "depth": 1, "zero": true, "data": false}]
+
+== spanning two clusters, partially overwriting backing file ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 4096/4096 bytes at offset 2048
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 3072
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 0
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 2048
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 3072
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 5120
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 6144
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 8192, "depth": 0, "zero": false, "data": true, "offset": 20480},
+{ "start": 8192, "length": 134209536, "depth": 1, "zero": true, "data": false}]
+
+== spanning multiple clusters, non-zero in first cluster ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 2048/2048 bytes at offset 65536
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 7168/7168 bytes at offset 67584
+7 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 65536
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 10240/10240 bytes at offset 67584
+10 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 65536, "depth": 1, "zero": true, "data": false},
+{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": 20480},
+{ "start": 69632, "length": 8192, "depth": 0, "zero": true, "data": false},
+{ "start": 77824, "length": 134139904, "depth": 1, "zero": true, "data": false}]
+
+== spanning multiple clusters, non-zero in intermediate cluster ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 2048/2048 bytes at offset 70656
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 7168/7168 bytes at offset 67584
+7 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 12288/12288 bytes at offset 65536
+12 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 65536, "depth": 1, "zero": true, "data": false},
+{ "start": 65536, "length": 12288, "depth": 0, "zero": true, "data": false},
+{ "start": 77824, "length": 134139904, "depth": 1, "zero": true, "data": false}]
+
+== spanning multiple clusters, non-zero in final cluster ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 2048/2048 bytes at offset 75776
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 7168/7168 bytes at offset 67584
+7 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 10240/10240 bytes at offset 65536
+10 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 75776
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 65536, "depth": 1, "zero": true, "data": false},
+{ "start": 65536, "length": 8192, "depth": 0, "zero": true, "data": false},
+{ "start": 73728, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": 20480},
+{ "start": 77824, "length": 134139904, "depth": 1, "zero": true, "data": false}]
+
+== spanning multiple clusters, partially overwriting backing file ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+wrote 10240/10240 bytes at offset 66560
+10 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 7168/7168 bytes at offset 67584
+7 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 65536
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 66560
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 7168/7168 bytes at offset 67584
+7 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2048/2048 bytes at offset 74752
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 76800
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+[{ "start": 0, "length": 65536, "depth": 1, "zero": true, "data": false},
+{ "start": 65536, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": 20480},
+{ "start": 69632, "length": 4096, "depth": 0, "zero": true, "data": false},
+{ "start": 73728, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": 24576},
+{ "start": 77824, "length": 134139904, "depth": 1, "zero": true, "data": false}]
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 6067673..ab1d76e 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -153,3 +153,4 @@
149 rw auto sudo
150 rw auto quick
152 rw auto quick
+154 rw auto backing quick
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 28/31] block: clarify error message for qmp-eject
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (26 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 27/31] qemu-iotests: Some more write_zeroes tests Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 15:22 ` [Qemu-devel] [PULL 29/31] qemu-io: Fix recent UI updates Kevin Wolf
` (3 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: John Snow <jsnow@redhat.com>
If you use HMP's eject but the CDROM tray is locked, you may get a
confusing error message informing you that the "tray isn't open."
As this is the point of eject, we can do a little better and help
clarify that the tray was locked and that it (might) open up later,
so try again.
It's not ideal, but it makes the semantics of the (legacy) eject
command more understandable to end users when they try to use it.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 51 ++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 11 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index b6fa210..40e4e6f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2281,16 +2281,29 @@ exit:
block_job_txn_unref(block_job_txn);
}
+static int do_open_tray(const char *device, bool force, Error **errp);
+
void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
{
Error *local_err = NULL;
+ int rc;
+
+ if (!has_force) {
+ force = false;
+ }
- qmp_blockdev_open_tray(device, has_force, force, &local_err);
+ rc = do_open_tray(device, force, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
+ if (rc == EINPROGRESS) {
+ error_setg(errp, "Device '%s' is locked and force was not specified, "
+ "wait for tray to open and try again", device);
+ return;
+ }
+
qmp_x_blockdev_remove_medium(device, errp);
}
@@ -2318,35 +2331,36 @@ void qmp_block_passwd(bool has_device, const char *device,
aio_context_release(aio_context);
}
-void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
- Error **errp)
+/**
+ * returns -errno on fatal error, +errno for non-fatal situations.
+ * errp will always be set when the return code is negative.
+ * May return +ENOSYS if the device has no tray,
+ * or +EINPROGRESS if the tray is locked and the guest has been notified.
+ */
+static int do_open_tray(const char *device, bool force, Error **errp)
{
BlockBackend *blk;
bool locked;
- if (!has_force) {
- force = false;
- }
-
blk = blk_by_name(device);
if (!blk) {
error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
"Device '%s' not found", device);
- return;
+ return -ENODEV;
}
if (!blk_dev_has_removable_media(blk)) {
error_setg(errp, "Device '%s' is not removable", device);
- return;
+ return -ENOTSUP;
}
if (!blk_dev_has_tray(blk)) {
/* Ignore this command on tray-less devices */
- return;
+ return ENOSYS;
}
if (blk_dev_is_tray_open(blk)) {
- return;
+ return 0;
}
locked = blk_dev_is_medium_locked(blk);
@@ -2357,6 +2371,21 @@ void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
if (!locked || force) {
blk_dev_change_media_cb(blk, false);
}
+
+ if (locked && !force) {
+ return EINPROGRESS;
+ }
+
+ return 0;
+}
+
+void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
+ Error **errp)
+{
+ if (!has_force) {
+ force = false;
+ }
+ do_open_tray(device, force, errp);
}
void qmp_blockdev_close_tray(const char *device, Error **errp)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 29/31] qemu-io: Fix recent UI updates
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (27 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 28/31] block: clarify error message for qmp-eject Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 15:22 ` [Qemu-devel] [PULL 30/31] qemu-iotests: Simplify 109 with unaligned qemu-img compare Kevin Wolf
` (2 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Eric Blake <eblake@redhat.com>
Commit 770e0e0e [*] tried to add 'writev -f', but didn't tweak
the getopt() call to actually let it work. Likewise, commit
c2e001c missed implementing 'aio_write -u -z'. The latter commit
also introduced a leak of ctx.
[*] does it sound "ech0e" in here? :)
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 1463416983-28318-2-git-send-email-eblake@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-io-cmds.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 22f2ecf..b5dbc67 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1107,7 +1107,7 @@ static int writev_f(BlockBackend *blk, int argc, char **argv)
int pattern = 0xcd;
QEMUIOVector qiov;
- while ((c = getopt(argc, argv, "CqP:")) != -1) {
+ while ((c = getopt(argc, argv, "CfqP:")) != -1) {
switch (c) {
case 'C':
Cflag = true;
@@ -1393,7 +1393,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
int flags = 0;
ctx->blk = blk;
- while ((c = getopt(argc, argv, "CfqP:z")) != -1) {
+ while ((c = getopt(argc, argv, "CfqP:uz")) != -1) {
switch (c) {
case 'C':
ctx->Cflag = true;
@@ -1436,6 +1436,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
if ((flags & BDRV_REQ_MAY_UNMAP) && !ctx->zflag) {
printf("-u requires -z to be specified\n");
+ g_free(ctx);
return 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 30/31] qemu-iotests: Simplify 109 with unaligned qemu-img compare
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (28 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 29/31] qemu-io: Fix recent UI updates Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 15:22 ` [Qemu-devel] [PULL 31/31] qemu-iotests: Fix regression in 136 on aio_read invalid Kevin Wolf
2016-05-19 16:40 ` [Qemu-devel] [PULL 00/31] Block layer patches Peter Maydell
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Eric Blake <eblake@redhat.com>
For some time now, qemu-img compare has been able to compare
unaligned images. So we no longer need test 109's hack of
resizing to sector boundaries before invoking compare.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 1463416983-28318-3-git-send-email-eblake@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/109 | 2 --
tests/qemu-iotests/109.out | 4 ----
2 files changed, 6 deletions(-)
diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
index f980b0c..adf9889 100755
--- a/tests/qemu-iotests/109
+++ b/tests/qemu-iotests/109
@@ -104,8 +104,6 @@ for sample_img in empty.bochs iotest-dirtylog-10G-4M.vhdx parallels-v1 \
$QEMU_IO -c 'read -P 0 0 64k' "$TEST_IMG" | _filter_qemu_io
run_qemu "$TEST_IMG" "$TEST_IMG.src" "'format': 'raw'," "BLOCK_JOB_READY"
- # qemu-img compare can't handle unaligned file sizes
- $QEMU_IMG resize -f raw "$TEST_IMG.src" +0
$QEMU_IMG compare -f raw -F raw "$TEST_IMG" "$TEST_IMG.src"
done
diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
index 38bc073..7c797ed 100644
--- a/tests/qemu-iotests/109.out
+++ b/tests/qemu-iotests/109.out
@@ -143,7 +143,6 @@ read 65536/65536 bytes at offset 0
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2560, "offset": 2560, "speed": 0, "type": "mirror"}}
{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2560, "offset": 2560, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
-Image resized.
Warning: Image size mismatch!
Images are identical.
@@ -164,7 +163,6 @@ read 65536/65536 bytes at offset 0
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 31457280, "offset": 31457280, "speed": 0, "type": "mirror"}}
{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 31457280, "offset": 31457280, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
-Image resized.
Warning: Image size mismatch!
Images are identical.
@@ -185,7 +183,6 @@ read 65536/65536 bytes at offset 0
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 327680, "offset": 327680, "speed": 0, "type": "mirror"}}
{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 327680, "offset": 327680, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
-Image resized.
Warning: Image size mismatch!
Images are identical.
@@ -206,7 +203,6 @@ read 65536/65536 bytes at offset 0
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2048, "offset": 2048, "speed": 0, "type": "mirror"}}
{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2048, "offset": 2048, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
-Image resized.
Warning: Image size mismatch!
Images are identical.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PULL 31/31] qemu-iotests: Fix regression in 136 on aio_read invalid
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (29 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 30/31] qemu-iotests: Simplify 109 with unaligned qemu-img compare Kevin Wolf
@ 2016-05-19 15:22 ` Kevin Wolf
2016-05-19 16:40 ` [Qemu-devel] [PULL 00/31] Block layer patches Peter Maydell
31 siblings, 0 replies; 39+ messages in thread
From: Kevin Wolf @ 2016-05-19 15:22 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Eric Blake <eblake@redhat.com>
Commit 093ea232 removed the ability for aio_read and aio_write
to artificially inflate the invalid statistics counters for
block devices, since it no longer flags unaligned offset or
length. Add 'aio_read -i' and 'aio_write -i' to restore
the ability, and update test 136 to use it.
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 1463416983-28318-4-git-send-email-eblake@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-io-cmds.c | 20 ++++++++++++++++----
tests/qemu-iotests/136 | 15 ++++-----------
2 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index b5dbc67..e766791 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1274,6 +1274,7 @@ static void aio_read_help(void)
" used to ensure all outstanding aio requests have been completed.\n"
" -C, -- report statistics in a machine parsable format\n"
" -P, -- use a pattern to verify read data\n"
+" -i, -- treat request as invalid, for exercising stats\n"
" -v, -- dump buffer to standard output\n"
" -q, -- quiet mode, do not show I/O statistics\n"
"\n");
@@ -1286,7 +1287,7 @@ static const cmdinfo_t aio_read_cmd = {
.cfunc = aio_read_f,
.argmin = 2,
.argmax = -1,
- .args = "[-Cqv] [-P pattern] off len [len..]",
+ .args = "[-Ciqv] [-P pattern] off len [len..]",
.oneline = "asynchronously reads a number of bytes",
.help = aio_read_help,
};
@@ -1297,7 +1298,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
ctx->blk = blk;
- while ((c = getopt(argc, argv, "CP:qv")) != -1) {
+ while ((c = getopt(argc, argv, "CP:iqv")) != -1) {
switch (c) {
case 'C':
ctx->Cflag = true;
@@ -1310,6 +1311,11 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
return 0;
}
break;
+ case 'i':
+ printf("injecting invalid read request\n");
+ block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_READ);
+ g_free(ctx);
+ return 0;
case 'q':
ctx->qflag = true;
break;
@@ -1367,6 +1373,7 @@ static void aio_write_help(void)
" -P, -- use different pattern to fill file\n"
" -C, -- report statistics in a machine parsable format\n"
" -f, -- use Force Unit Access semantics\n"
+" -i, -- treat request as invalid, for exercising stats\n"
" -q, -- quiet mode, do not show I/O statistics\n"
" -u, -- with -z, allow unmapping\n"
" -z, -- write zeroes using blk_aio_write_zeroes\n"
@@ -1380,7 +1387,7 @@ static const cmdinfo_t aio_write_cmd = {
.cfunc = aio_write_f,
.argmin = 2,
.argmax = -1,
- .args = "[-Cfquz] [-P pattern] off len [len..]",
+ .args = "[-Cfiquz] [-P pattern] off len [len..]",
.oneline = "asynchronously writes a number of bytes",
.help = aio_write_help,
};
@@ -1393,7 +1400,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
int flags = 0;
ctx->blk = blk;
- while ((c = getopt(argc, argv, "CfqP:uz")) != -1) {
+ while ((c = getopt(argc, argv, "CfiqP:uz")) != -1) {
switch (c) {
case 'C':
ctx->Cflag = true;
@@ -1414,6 +1421,11 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
return 0;
}
break;
+ case 'i':
+ printf("injecting invalid write request\n");
+ block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_WRITE);
+ g_free(ctx);
+ return 0;
case 'z':
ctx->zflag = true;
break;
diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136
index 996265e..635b977 100644
--- a/tests/qemu-iotests/136
+++ b/tests/qemu-iotests/136
@@ -226,18 +226,11 @@ sector = "%d"
highest_offset = wr_ops * wr_size
- # Two types of invalid operations: unaligned length and unaligned offset
- for i in range(invalid_rd_ops / 2):
- ops.append("aio_read 0 511")
+ for i in range(invalid_rd_ops):
+ ops.append("aio_read -i 0 512")
- for i in range(invalid_rd_ops / 2, invalid_rd_ops):
- ops.append("aio_read 13 512")
-
- for i in range(invalid_wr_ops / 2):
- ops.append("aio_write 0 511")
-
- for i in range(invalid_wr_ops / 2, invalid_wr_ops):
- ops.append("aio_write 13 512")
+ for i in range(invalid_wr_ops):
+ ops.append("aio_write -i 0 512")
for i in range(failed_rd_ops):
ops.append("aio_read %d 512" % bad_offset)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PULL 00/31] Block layer patches
2016-05-19 15:21 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
` (30 preceding siblings ...)
2016-05-19 15:22 ` [Qemu-devel] [PULL 31/31] qemu-iotests: Fix regression in 136 on aio_read invalid Kevin Wolf
@ 2016-05-19 16:40 ` Peter Maydell
31 siblings, 0 replies; 39+ messages in thread
From: Peter Maydell @ 2016-05-19 16:40 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Qemu-block, QEMU Developers
On 19 May 2016 at 16:21, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit 8ec4fe0a4bed4fa27e6f28a746bcf77b27cd05a3:
>
> Merge remote-tracking branch 'remotes/mjt/tags/pull-trivial-patches-2016-05-18' into staging (2016-05-19 09:27:28 +0100)
>
> are available in the git repository at:
>
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 7753da2351e0b0ff6825d080aff58d73c994ff47:
>
> Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2016-05-19' into queue-block (2016-05-19 16:59:46 +0200)
>
> ----------------------------------------------------------------
>
> Block layer patches
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 39+ messages in thread