* [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk
@ 2016-04-27 13:20 Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 1/9] block: Use BdrvChild callbacks for change_media/resize Kevin Wolf
` (9 more replies)
0 siblings, 10 replies; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, qemu-devel
This is the final patch series that is required before we can start allowing
setups with more than one BlockBackend per BlockDriverState.
Depends on 'block: Move I/O throttling to BlockBackend'.
v2:
- Patch 1 ('block: Use BdrvChild callbacks for change_media/resize')
Bring back an accidentally removed comment [Max]
- Patch 2 ('block: User BdrvChild callback for device name')
Documented BdrvChildRole.get_name() callback [Max]
- Patch 3 ('blockjob: Don't set iostatus of target') and
Patch 4 ('Don't touch BDS iostatus')
Replaces 'Use BdrvChild callbacks for iostatus operations'. The whole
problematic iostatus manipulation of block jobs only affected the target node
where it wasn't visible, and it doesn't make sense, so it can simply go away.
- Patch 6 ('block: Add bdrv_has_blk()') and
Patch 7 ('block: Avoid bs->blk in bdrv_next()')
Split differently to keep the bdrv_next() changes in one patch [Max]
Return BDSes only once in bdrv_next() even with multiple BBs [Max]
Use enum for iteration phases instead of magic int [Max]
- Patch 9 ('block: Remove BlockDriverState.blk')
Use bdrv_get_parent_name() to keep error message unchanged [Max]
Kevin Wolf (9):
block: Use BdrvChild callbacks for change_media/resize
block: User BdrvChild callback for device name
blockjob: Don't set iostatus of target
blockjob: Don't touch BDS iostatus
block: Remove bdrv_aio_multiwrite()
block: Add bdrv_has_blk()
block: Avoid bs->blk in bdrv_next()
block: Don't return throttling info in query-named-block-nodes
block: Remove BlockDriverState.blk
block.c | 101 ++++++++++----------
block/backup.c | 34 +------
block/block-backend.c | 116 +++++++++++++++--------
block/commit.c | 7 --
block/io.c | 207 ++---------------------------------------
block/mirror.c | 38 ++------
block/qapi.c | 6 +-
block/snapshot.c | 30 +++---
block/stream.c | 10 +-
blockdev.c | 19 ++--
blockjob.c | 6 +-
include/block/block.h | 10 +-
include/block/block_int.h | 12 ++-
include/block/blockjob.h | 4 +-
include/sysemu/block-backend.h | 3 +-
migration/block.c | 4 +-
monitor.c | 6 +-
qemu-io-cmds.c | 203 ----------------------------------------
qmp.c | 5 +-
tests/qemu-iotests/096 | 3 +-
tests/qemu-iotests/100 | 145 -----------------------------
tests/qemu-iotests/100.out | 89 ------------------
tests/qemu-iotests/136 | 20 +---
tests/qemu-iotests/136.out | 4 +-
tests/qemu-iotests/group | 2 +-
trace-events | 2 -
26 files changed, 218 insertions(+), 868 deletions(-)
delete mode 100755 tests/qemu-iotests/100
delete mode 100644 tests/qemu-iotests/100.out
--
1.8.3.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2 1/9] block: Use BdrvChild callbacks for change_media/resize
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
@ 2016-04-27 13:20 ` Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 2/9] block: User BdrvChild callback for device name Kevin Wolf
` (8 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, 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 d8c6e98..69ee791 100644
--- a/block.c
+++ b/block.c
@@ -1216,6 +1216,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().
@@ -1675,9 +1696,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 */
@@ -2123,9 +2142,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;
@@ -2576,9 +2593,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;
}
@@ -2688,11 +2703,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 063024d..7f507ba 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 8481e80..9938a3d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -362,6 +362,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);
+
void (*drained_begin)(BdrvChild *child);
void (*drained_end)(BdrvChild *child);
};
@@ -706,7 +709,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] 22+ messages in thread
* [Qemu-devel] [PATCH v2 2/9] block: User BdrvChild callback for device name
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 1/9] block: Use BdrvChild callbacks for change_media/resize Kevin Wolf
@ 2016-04-27 13:20 ` Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target Kevin Wolf
` (7 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, 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 69ee791..a8df72f 100644
--- a/block.c
+++ b/block.c
@@ -2896,10 +2896,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
@@ -2908,7 +2926,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 7f507ba..34f747e 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 9938a3d..f4a0941 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -365,6 +365,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);
+
void (*drained_begin)(BdrvChild *child);
void (*drained_end)(BdrvChild *child);
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 1/9] block: Use BdrvChild callbacks for change_media/resize Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 2/9] block: User BdrvChild callback for device name Kevin Wolf
@ 2016-04-27 13:20 ` Kevin Wolf
2016-05-06 12:01 ` Max Reitz
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 4/9] blockjob: Don't touch BDS iostatus Kevin Wolf
` (6 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, 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>
---
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] 22+ messages in thread
* [Qemu-devel] [PATCH v2 4/9] blockjob: Don't touch BDS iostatus
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
` (2 preceding siblings ...)
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target Kevin Wolf
@ 2016-04-27 13:20 ` Kevin Wolf
2016-05-06 13:37 ` Max Reitz
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 5/9] block: Remove bdrv_aio_multiwrite() Kevin Wolf
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, 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>
---
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] 22+ messages in thread
* [Qemu-devel] [PATCH v2 5/9] block: Remove bdrv_aio_multiwrite()
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
` (3 preceding siblings ...)
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 4/9] blockjob: Don't touch BDS iostatus Kevin Wolf
@ 2016-04-27 13:20 ` Kevin Wolf
2016-05-06 12:29 ` Eric Blake
2016-05-12 20:18 ` Eric Blake
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 6/9] block: Add bdrv_has_blk() Kevin Wolf
` (4 subsequent siblings)
9 siblings, 2 replies; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, 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>
---
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 | 145 -----------------------------
tests/qemu-iotests/100.out | 89 ------------------
tests/qemu-iotests/136 | 20 +---
tests/qemu-iotests/136.out | 4 +-
tests/qemu-iotests/group | 2 +-
trace-events | 2 -
11 files changed, 9 insertions(+), 672 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 34f747e..d1974f5 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1106,20 +1106,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 4224044..5baccda 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1791,200 +1791,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 4b04550..10ce058 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 b79608e..8285c97 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -141,7 +141,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 e34f777..471e4a8 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -614,49 +614,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(
@@ -1248,165 +1205,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;
- int Cflag = 0, qflag = 0;
- 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 = 1;
- break;
- case 'q':
- qflag = 1;
- 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;
@@ -2475,7 +2273,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 5b2fb33..0000000
--- a/tests/qemu-iotests/100
+++ /dev/null
@@ -1,145 +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 "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 "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 "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 "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 "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 "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 "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 0564903..0000000
--- a/tests/qemu-iotests/100.out
+++ /dev/null
@@ -1,89 +0,0 @@
-QA output created by 100
-
-== Single request ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-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 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 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 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 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 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 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 8350743..21026d9 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] 22+ messages in thread
* [Qemu-devel] [PATCH v2 6/9] block: Add bdrv_has_blk()
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
` (4 preceding siblings ...)
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 5/9] block: Remove bdrv_aio_multiwrite() Kevin Wolf
@ 2016-04-27 13:20 ` Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 7/9] block: Avoid bs->blk in bdrv_next() Kevin Wolf
` (3 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, 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 d1974f5..bd1bf14 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 8285c97..60db46c 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] 22+ messages in thread
* [Qemu-devel] [PATCH v2 7/9] block: Avoid bs->blk in bdrv_next()
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
` (5 preceding siblings ...)
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 6/9] block: Add bdrv_has_blk() Kevin Wolf
@ 2016-04-27 13:20 ` Kevin Wolf
2016-05-06 12:54 ` Max Reitz
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 8/9] block: Don't return throttling info in query-named-block-nodes Kevin Wolf
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, 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>
---
block.c | 34 ++++++--------------
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, 99 insertions(+), 72 deletions(-)
diff --git a/block.c b/block.c
index a8df72f..117bef7 100644
--- a/block.c
+++ b/block.c
@@ -2872,25 +2872,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;
@@ -3214,10 +3195,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);
@@ -3247,10 +3229,11 @@ static int bdrv_inactivate(BlockDriverState *bs)
int bdrv_inactivate_all(void)
{
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BdrvNextIterator *it = NULL;
int ret;
- 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);
@@ -3750,10 +3733,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 bd1bf14..a355590 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 5baccda..2019441 100644
--- a/block/io.c
+++ b/block/io.c
@@ -300,10 +300,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);
@@ -331,10 +332,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;
@@ -347,8 +348,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 7441212..445a213 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4080,8 +4080,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 10ce058..3603be9 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 60db46c..12e4585 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 9d0953b..3fe8f87 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] 22+ messages in thread
* [Qemu-devel] [PATCH v2 8/9] block: Don't return throttling info in query-named-block-nodes
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
` (6 preceding siblings ...)
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 7/9] block: Avoid bs->blk in bdrv_next() Kevin Wolf
@ 2016-04-27 13:20 ` Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 9/9] block: Remove BlockDriverState.blk Kevin Wolf
2016-05-17 14:28 ` [Qemu-devel] [PATCH v2 0/9] " Kevin Wolf
9 siblings, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, 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] 22+ messages in thread
* [Qemu-devel] [PATCH v2 9/9] block: Remove BlockDriverState.blk
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
` (7 preceding siblings ...)
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 8/9] block: Don't return throttling info in query-named-block-nodes Kevin Wolf
@ 2016-04-27 13:20 ` Kevin Wolf
2016-05-17 14:28 ` [Qemu-devel] [PATCH v2 0/9] " Kevin Wolf
9 siblings, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2016-04-27 13:20 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, jcody, armbru, eblake, 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 117bef7..cf813ef 100644
--- a/block.c
+++ b/block.c
@@ -2228,14 +2228,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;
@@ -2877,7 +2869,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 a355590..ffcd44a 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 445a213..e667b8c 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 f4a0941..647e9b3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -407,8 +407,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
@@ -708,6 +706,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] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target Kevin Wolf
@ 2016-05-06 12:01 ` Max Reitz
2016-05-06 12:32 ` Max Reitz
0 siblings, 1 reply; 22+ messages in thread
From: Max Reitz @ 2016-05-06 12:01 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: jcody, armbru, eblake, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 970 bytes --]
On 27.04.2016 15:20, Kevin Wolf wrote:
> 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>
> ---
> 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(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/9] block: Remove bdrv_aio_multiwrite()
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 5/9] block: Remove bdrv_aio_multiwrite() Kevin Wolf
@ 2016-05-06 12:29 ` Eric Blake
2016-05-12 20:18 ` Eric Blake
1 sibling, 0 replies; 22+ messages in thread
From: Eric Blake @ 2016-05-06 12:29 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, jcody, armbru, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2198 bytes --]
On 04/27/2016 07:20 AM, Kevin Wolf wrote:
> 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>
> ---
> 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 | 145 -----------------------------
> tests/qemu-iotests/100.out | 89 ------------------
> tests/qemu-iotests/136 | 20 +---
> tests/qemu-iotests/136.out | 4 +-
> tests/qemu-iotests/group | 2 +-
> trace-events | 2 -
Reviewed-by: Eric Blake <eblake@redhat.com>
Now that BlockRequest is no longer used in any .h files, should we move
the struct out of block.h and into io.c as a followup? I already
attempted touching the struct in my series on killing blk_read(), before
realizing you were killing the last public use of the struct, so maybe
I'll go ahead and tackle that simplification.
> +++ 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 */
Technically, the 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 */
...is now always the block layer.
> int error;
> } BlockRequest;
Hence my suggestion to move this to io.c and perhaps inline it into the
other structs already there.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target
2016-05-06 12:01 ` Max Reitz
@ 2016-05-06 12:32 ` Max Reitz
2016-05-06 13:31 ` Kevin Wolf
0 siblings, 1 reply; 22+ messages in thread
From: Max Reitz @ 2016-05-06 12:32 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: jcody, armbru, eblake, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1861 bytes --]
On 06.05.2016 14:01, Max Reitz wrote:
> On 27.04.2016 15:20, Kevin Wolf wrote:
>> 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>
>> ---
>> 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(-)
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
Maybe I need to take that back. Using e.g. blockdev-backup, you can
indeed see the target in query-block.
e.g.:
(echo "{'execute':'qmp_capabilities'}
{'execute':'blockdev-backup',
'arguments':{'device':'src','target':'dst',
'sync':'full','on-target-error':'enospc'}}";
sleep 1;
echo "{'execute':'query-block'}") | \
x86_64-softmmu/qemu-system-x86_64 \
-drive if=none,id=src,file=test.qcow2 \
-drive if=none,id=dst,file=mnt/out.qcow2 \
-qmp-pretty stdio -nodefaults
Where mnt is a file system and test.qcow2 is large enough such that an
ENOSPC will occur.
Before this patch, you can see "io-status": "nospace" in query-block for
dst. After it, it says "io-status": "ok", and after the next it doesn't
say anything about the status at all anymore.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/9] block: Avoid bs->blk in bdrv_next()
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 7/9] block: Avoid bs->blk in bdrv_next() Kevin Wolf
@ 2016-05-06 12:54 ` Max Reitz
0 siblings, 0 replies; 22+ messages in thread
From: Max Reitz @ 2016-05-06 12:54 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: jcody, armbru, eblake, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1796 bytes --]
On 27.04.2016 15:20, 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>
> ---
> block.c | 34 ++++++--------------
> 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, 99 insertions(+), 72 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[...]
> @@ -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;
> }
Not sure whether I'm supposed to, but I find this pretty clever. Nice.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target
2016-05-06 12:32 ` Max Reitz
@ 2016-05-06 13:31 ` Kevin Wolf
2016-05-06 13:40 ` Max Reitz
2016-05-06 14:34 ` Eric Blake
0 siblings, 2 replies; 22+ messages in thread
From: Kevin Wolf @ 2016-05-06 13:31 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, jcody, armbru, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]
Am 06.05.2016 um 14:32 hat Max Reitz geschrieben:
> On 06.05.2016 14:01, Max Reitz wrote:
> > On 27.04.2016 15:20, Kevin Wolf wrote:
> >> 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>
> >> ---
> >> 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(-)
> >
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
>
> Maybe I need to take that back. Using e.g. blockdev-backup, you can
> indeed see the target in query-block.
>
> e.g.:
>
> (echo "{'execute':'qmp_capabilities'}
> {'execute':'blockdev-backup',
> 'arguments':{'device':'src','target':'dst',
> 'sync':'full','on-target-error':'enospc'}}";
> sleep 1;
> echo "{'execute':'query-block'}") | \
> x86_64-softmmu/qemu-system-x86_64 \
> -drive if=none,id=src,file=test.qcow2 \
> -drive if=none,id=dst,file=mnt/out.qcow2 \
> -qmp-pretty stdio -nodefaults
>
> Where mnt is a file system and test.qcow2 is large enough such that an
> ENOSPC will occur.
Hm... Let's pretend we didn't notice. In fact, I don't think this
behaviour is documented and it also doesn't make a lot of sense.
I would be surprised if libvirt made use of it, as there is still the
job iostatus which works in drive-* cases, too.
(Eric?)
> Before this patch, you can see "io-status": "nospace" in query-block for
> dst. After it, it says "io-status": "ok", and after the next it doesn't
> say anything about the status at all anymore.
Simply letting the field disappear sounds like a nice solution.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/9] blockjob: Don't touch BDS iostatus
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 4/9] blockjob: Don't touch BDS iostatus Kevin Wolf
@ 2016-05-06 13:37 ` Max Reitz
0 siblings, 0 replies; 22+ messages in thread
From: Max Reitz @ 2016-05-06 13:37 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: jcody, armbru, eblake, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 789 bytes --]
On 27.04.2016 15:20, Kevin Wolf wrote:
> 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>
> ---
> block/backup.c | 26 --------------------------
> block/commit.c | 7 -------
> block/mirror.c | 26 --------------------------
> block/stream.c | 7 -------
> 4 files changed, 66 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target
2016-05-06 13:31 ` Kevin Wolf
@ 2016-05-06 13:40 ` Max Reitz
2016-05-06 14:12 ` Kevin Wolf
2016-05-06 14:34 ` Eric Blake
1 sibling, 1 reply; 22+ messages in thread
From: Max Reitz @ 2016-05-06 13:40 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, jcody, armbru, eblake, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 2839 bytes --]
On 06.05.2016 15:31, Kevin Wolf wrote:
> Am 06.05.2016 um 14:32 hat Max Reitz geschrieben:
>> On 06.05.2016 14:01, Max Reitz wrote:
>>> On 27.04.2016 15:20, Kevin Wolf wrote:
>>>> 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>
>>>> ---
>>>> 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(-)
>>>
>>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>
>> Maybe I need to take that back. Using e.g. blockdev-backup, you can
>> indeed see the target in query-block.
>>
>> e.g.:
>>
>> (echo "{'execute':'qmp_capabilities'}
>> {'execute':'blockdev-backup',
>> 'arguments':{'device':'src','target':'dst',
>> 'sync':'full','on-target-error':'enospc'}}";
>> sleep 1;
>> echo "{'execute':'query-block'}") | \
>> x86_64-softmmu/qemu-system-x86_64 \
>> -drive if=none,id=src,file=test.qcow2 \
>> -drive if=none,id=dst,file=mnt/out.qcow2 \
>> -qmp-pretty stdio -nodefaults
>>
>> Where mnt is a file system and test.qcow2 is large enough such that an
>> ENOSPC will occur.
>
> Hm... Let's pretend we didn't notice. In fact, I don't think this
> behaviour is documented and it also doesn't make a lot of sense.
>
> I would be surprised if libvirt made use of it, as there is still the
> job iostatus which works in drive-* cases, too.
>
> (Eric?)
>
>> Before this patch, you can see "io-status": "nospace" in query-block for
>> dst. After it, it says "io-status": "ok", and after the next it doesn't
>> say anything about the status at all anymore.
>
> Simply letting the field disappear sounds like a nice solution.
I'm not sure I completely agree with that because the iostatus appears
to be the only way of obtaining the information whether the block job
stopped because of an ENOSPC or because of something different (the
BLOCK_JOB_ERROR doesn't tell you).
I don't know whether anyone actually needs that information, though, and
if you think dropping this is fine, then I guess it's fine with me, too,
and my R-b stands.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target
2016-05-06 13:40 ` Max Reitz
@ 2016-05-06 14:12 ` Kevin Wolf
2016-05-11 15:02 ` Max Reitz
0 siblings, 1 reply; 22+ messages in thread
From: Kevin Wolf @ 2016-05-06 14:12 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, jcody, armbru, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3334 bytes --]
Am 06.05.2016 um 15:40 hat Max Reitz geschrieben:
> On 06.05.2016 15:31, Kevin Wolf wrote:
> > Am 06.05.2016 um 14:32 hat Max Reitz geschrieben:
> >> On 06.05.2016 14:01, Max Reitz wrote:
> >>> On 27.04.2016 15:20, Kevin Wolf wrote:
> >>>> 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>
> >>>> ---
> >>>> 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(-)
> >>>
> >>> Reviewed-by: Max Reitz <mreitz@redhat.com>
> >>
> >> Maybe I need to take that back. Using e.g. blockdev-backup, you can
> >> indeed see the target in query-block.
> >>
> >> e.g.:
> >>
> >> (echo "{'execute':'qmp_capabilities'}
> >> {'execute':'blockdev-backup',
> >> 'arguments':{'device':'src','target':'dst',
> >> 'sync':'full','on-target-error':'enospc'}}";
> >> sleep 1;
> >> echo "{'execute':'query-block'}") | \
> >> x86_64-softmmu/qemu-system-x86_64 \
> >> -drive if=none,id=src,file=test.qcow2 \
> >> -drive if=none,id=dst,file=mnt/out.qcow2 \
> >> -qmp-pretty stdio -nodefaults
> >>
> >> Where mnt is a file system and test.qcow2 is large enough such that an
> >> ENOSPC will occur.
> >
> > Hm... Let's pretend we didn't notice. In fact, I don't think this
> > behaviour is documented and it also doesn't make a lot of sense.
> >
> > I would be surprised if libvirt made use of it, as there is still the
> > job iostatus which works in drive-* cases, too.
> >
> > (Eric?)
> >
> >> Before this patch, you can see "io-status": "nospace" in query-block for
> >> dst. After it, it says "io-status": "ok", and after the next it doesn't
> >> say anything about the status at all anymore.
> >
> > Simply letting the field disappear sounds like a nice solution.
>
> I'm not sure I completely agree with that because the iostatus appears
> to be the only way of obtaining the information whether the block job
> stopped because of an ENOSPC or because of something different (the
> BLOCK_JOB_ERROR doesn't tell you).
>
> I don't know whether anyone actually needs that information, though, and
> if you think dropping this is fine, then I guess it's fine with me, too,
> and my R-b stands.
But you don't have this information with the drive-* commands either.
I would be okay with reintroducing this information, but in the right
place. Modifying the guest device iostatus for block jobs certainly
isn't right.
Anyway, we need Eric's input to confirm that libvirt doesn't depend on
this.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target
2016-05-06 13:31 ` Kevin Wolf
2016-05-06 13:40 ` Max Reitz
@ 2016-05-06 14:34 ` Eric Blake
1 sibling, 0 replies; 22+ messages in thread
From: Eric Blake @ 2016-05-06 14:34 UTC (permalink / raw)
To: Kevin Wolf, Max Reitz; +Cc: qemu-block, jcody, armbru, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]
On 05/06/2016 07:31 AM, Kevin Wolf wrote:
>>
>> Maybe I need to take that back. Using e.g. blockdev-backup, you can
>> indeed see the target in query-block.
>>
>> Where mnt is a file system and test.qcow2 is large enough such that an
>> ENOSPC will occur.
>
> Hm... Let's pretend we didn't notice. In fact, I don't think this
> behaviour is documented and it also doesn't make a lot of sense.
>
> I would be surprised if libvirt made use of it, as there is still the
> job iostatus which works in drive-* cases, too.
>
> (Eric?)
>
>> Before this patch, you can see "io-status": "nospace" in query-block for
>> dst. After it, it says "io-status": "ok", and after the next it doesn't
>> say anything about the status at all anymore.
Here's what libvirt does with "io-status":
qemuMonitorJSONGetBlockInfo():
/* Missing io-status indicates no error */
if ((status = virJSONValueObjectGetString(dev, "io-status"))) {
info->io_status = qemuMonitorBlockIOStatusToError(status);
if (info->io_status < 0)
goto cleanup;
}
Then it feeds info->io_status into qemuDomainGetDiskErrors().
However, I'm not sure I know the full context of your question - is
io-status still available on the primary BDS, and we are only deleting
it from a secondary spot used only during a job? I think libvirt is
currently only checking the status of the BDS (ENOSPC when the primary
runs out of room), not what happens during a block job. In fact,
libvirt does not yet use 'blockdev-backup' at all, let alone
blockdev-backup with 'on-target-error':'enospc' (it should, but that's a
story for another day).
>
> Simply letting the field disappear sounds like a nice solution.
So at first glance, I think we're okay here, dropping io-status from the
block job from this location and instead exposing it from a more natural
spot in QMP. And in the worst case, we can use introspection to learn
which of two spots exposes the information, if libvirt is trying to be
portable to 2.6 and 2.7 at the same time.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target
2016-05-06 14:12 ` Kevin Wolf
@ 2016-05-11 15:02 ` Max Reitz
0 siblings, 0 replies; 22+ messages in thread
From: Max Reitz @ 2016-05-11 15:02 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, jcody, armbru, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3525 bytes --]
On 06.05.2016 16:12, Kevin Wolf wrote:
> Am 06.05.2016 um 15:40 hat Max Reitz geschrieben:
>> On 06.05.2016 15:31, Kevin Wolf wrote:
>>> Am 06.05.2016 um 14:32 hat Max Reitz geschrieben:
>>>> On 06.05.2016 14:01, Max Reitz wrote:
>>>>> On 27.04.2016 15:20, Kevin Wolf wrote:
>>>>>> 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>
>>>>>> ---
>>>>>> 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(-)
>>>>>
>>>>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>>>
>>>> Maybe I need to take that back. Using e.g. blockdev-backup, you can
>>>> indeed see the target in query-block.
>>>>
>>>> e.g.:
>>>>
>>>> (echo "{'execute':'qmp_capabilities'}
>>>> {'execute':'blockdev-backup',
>>>> 'arguments':{'device':'src','target':'dst',
>>>> 'sync':'full','on-target-error':'enospc'}}";
>>>> sleep 1;
>>>> echo "{'execute':'query-block'}") | \
>>>> x86_64-softmmu/qemu-system-x86_64 \
>>>> -drive if=none,id=src,file=test.qcow2 \
>>>> -drive if=none,id=dst,file=mnt/out.qcow2 \
>>>> -qmp-pretty stdio -nodefaults
>>>>
>>>> Where mnt is a file system and test.qcow2 is large enough such that an
>>>> ENOSPC will occur.
>>>
>>> Hm... Let's pretend we didn't notice. In fact, I don't think this
>>> behaviour is documented and it also doesn't make a lot of sense.
>>>
>>> I would be surprised if libvirt made use of it, as there is still the
>>> job iostatus which works in drive-* cases, too.
>>>
>>> (Eric?)
>>>
>>>> Before this patch, you can see "io-status": "nospace" in query-block for
>>>> dst. After it, it says "io-status": "ok", and after the next it doesn't
>>>> say anything about the status at all anymore.
>>>
>>> Simply letting the field disappear sounds like a nice solution.
>>
>> I'm not sure I completely agree with that because the iostatus appears
>> to be the only way of obtaining the information whether the block job
>> stopped because of an ENOSPC or because of something different (the
>> BLOCK_JOB_ERROR doesn't tell you).
>>
>> I don't know whether anyone actually needs that information, though, and
>> if you think dropping this is fine, then I guess it's fine with me, too,
>> and my R-b stands.
>
> But you don't have this information with the drive-* commands either.
>
> I would be okay with reintroducing this information, but in the right
> place. Modifying the guest device iostatus for block jobs certainly
> isn't right.
As discussed on IRC, that sounds right to me. Thus, have an explicit
Reviewed-by: Max Reitz <mreitz@redhat.com>
> Anyway, we need Eric's input to confirm that libvirt doesn't depend on
> this.
>
> Kevin
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/9] block: Remove bdrv_aio_multiwrite()
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 5/9] block: Remove bdrv_aio_multiwrite() Kevin Wolf
2016-05-06 12:29 ` Eric Blake
@ 2016-05-12 20:18 ` Eric Blake
1 sibling, 0 replies; 22+ messages in thread
From: Eric Blake @ 2016-05-12 20:18 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, jcody, armbru, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]
On 04/27/2016 07:20 AM, Kevin Wolf wrote:
> 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>
> ---
> +++ 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)
> -
Why not delete the wr_merged parameter from do_test_stats()...
> # 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],
as well as remove the # wr_merged comment and the now-useless final
member of each test_values[] array entry?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
` (8 preceding siblings ...)
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 9/9] block: Remove BlockDriverState.blk Kevin Wolf
@ 2016-05-17 14:28 ` Kevin Wolf
9 siblings, 0 replies; 22+ messages in thread
From: Kevin Wolf @ 2016-05-17 14:28 UTC (permalink / raw)
To: qemu-block; +Cc: mreitz, jcody, armbru, eblake, qemu-devel
Am 27.04.2016 um 15:20 hat Kevin Wolf geschrieben:
> This is the final patch series that is required before we can start allowing
> setups with more than one BlockBackend per BlockDriverState.
>
> Depends on 'block: Move I/O throttling to BlockBackend'.
Applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-05-17 14:29 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-27 13:20 [Qemu-devel] [PATCH v2 0/9] block: Remove BlockDriverState.blk Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 1/9] block: Use BdrvChild callbacks for change_media/resize Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 2/9] block: User BdrvChild callback for device name Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 3/9] blockjob: Don't set iostatus of target Kevin Wolf
2016-05-06 12:01 ` Max Reitz
2016-05-06 12:32 ` Max Reitz
2016-05-06 13:31 ` Kevin Wolf
2016-05-06 13:40 ` Max Reitz
2016-05-06 14:12 ` Kevin Wolf
2016-05-11 15:02 ` Max Reitz
2016-05-06 14:34 ` Eric Blake
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 4/9] blockjob: Don't touch BDS iostatus Kevin Wolf
2016-05-06 13:37 ` Max Reitz
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 5/9] block: Remove bdrv_aio_multiwrite() Kevin Wolf
2016-05-06 12:29 ` Eric Blake
2016-05-12 20:18 ` Eric Blake
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 6/9] block: Add bdrv_has_blk() Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 7/9] block: Avoid bs->blk in bdrv_next() Kevin Wolf
2016-05-06 12:54 ` Max Reitz
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 8/9] block: Don't return throttling info in query-named-block-nodes Kevin Wolf
2016-04-27 13:20 ` [Qemu-devel] [PATCH v2 9/9] block: Remove BlockDriverState.blk Kevin Wolf
2016-05-17 14:28 ` [Qemu-devel] [PATCH v2 0/9] " Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).