* [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API
@ 2017-05-24 2:52 Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 01/16] block: Define BLK_PERM_AIO_CONTEXT_CHANGE Fam Zheng
` (16 more replies)
0 siblings, 17 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
v3: Move blk_set_aio_context to the front of mirror_start_job to avoid
accessing target without acquiring its aio context. [Stefan]
Use error_free_or_abort in test code. [Stefan]
v2: Address Stefan's comments:
- Clean up redundancy in bdrv_format_default_perms change.
- Add a test case to check both success/failure cases.
A failure case is not possible at user interface level because of other
checks we have, so write a unit test in tests/test-blk-perm.c.
Eject / change of scsi-cd on a virtio-scsi dataplane bus causes abort() because
the new BDS doesn't get proper bdrv_set_aio_context().
Store the AioContext in BB and do it in blk_insert_bs. That is done by
Vladimir's patch.
Other patches are to make sure such a bdrv_set_aio_context() doesn't interfere
with other BBs using other nodes from this graph.
Fam
Fam Zheng (15):
block: Define BLK_PERM_AIO_CONTEXT_CHANGE
block-backend: Add blk_request_perm
blockjob: Add BLK_PERM_AIO_CONTEXT_CHANGE shared perm on bs
blockjob: Allow aio context change on intermediate nodes
block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph
backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target
backup: Do initial aio context move of target via BB interface
mirror: Request aio context change permission on target
commit: Allow aio context change on s->base
mirror: Do initial aio context move of target via BB interface
virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane
virtio-blk: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane
nbd: Allow BLK_PERM_AIO_CONTEXT_CHANGE on BB
block: Add perm assertion on blk_set_aio_context
tests: Add test case for BLK_PERM_AIO_CONTEXT_CHANGE
Vladimir Sementsov-Ogievskiy (1):
blk: fix aio context loss on media change
block.c | 12 ++++++---
block/backup.c | 11 +++++++-
block/block-backend.c | 22 +++++++++++++++
block/commit.c | 6 +++--
block/mirror.c | 56 +++++++++++++++++++++++---------------
block/stream.c | 3 ++-
block/vvfat.c | 2 +-
blockdev.c | 18 -------------
blockjob.c | 3 +++
hw/block/dataplane/virtio-blk.c | 15 ++++++++---
hw/scsi/virtio-scsi.c | 4 +++
include/block/block.h | 7 ++++-
include/sysemu/block-backend.h | 1 +
nbd/server.c | 6 +++--
tests/Makefile.include | 2 ++
tests/test-blk-perm.c | 59 +++++++++++++++++++++++++++++++++++++++++
16 files changed, 171 insertions(+), 56 deletions(-)
create mode 100644 tests/test-blk-perm.c
--
2.9.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 01/16] block: Define BLK_PERM_AIO_CONTEXT_CHANGE
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 02/16] block-backend: Add blk_request_perm Fam Zheng
` (15 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 2 ++
include/block/block.h | 7 ++++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 50ba264..d98662f 100644
--- a/block.c
+++ b/block.c
@@ -1649,6 +1649,8 @@ char *bdrv_perm_names(uint64_t perm)
{ BLK_PERM_WRITE_UNCHANGED, "write unchanged" },
{ BLK_PERM_RESIZE, "resize" },
{ BLK_PERM_GRAPH_MOD, "change children" },
+ { BLK_PERM_AIO_CONTEXT_CHANGE,
+ "aio context change" },
{ 0, NULL }
};
diff --git a/include/block/block.h b/include/block/block.h
index 9b355e9..017d6c8 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -225,7 +225,12 @@ enum {
*/
BLK_PERM_GRAPH_MOD = 0x10,
- BLK_PERM_ALL = 0x1f,
+ /**
+ * This permission is required to change the AioContext of this node.
+ */
+ BLK_PERM_AIO_CONTEXT_CHANGE = 0x20,
+
+ BLK_PERM_ALL = 0x3f,
};
char *bdrv_perm_names(uint64_t perm);
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 02/16] block-backend: Add blk_request_perm
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 01/16] block: Define BLK_PERM_AIO_CONTEXT_CHANGE Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-31 9:41 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 03/16] blockjob: Add BLK_PERM_AIO_CONTEXT_CHANGE shared perm on bs Fam Zheng
` (14 subsequent siblings)
16 siblings, 1 reply; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
This function tries to request, if not granted yet, for the given
permissions.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/block-backend.c | 12 ++++++++++++
include/sysemu/block-backend.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index f3a6008..5492f64 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -654,6 +654,18 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm)
*shared_perm = blk->shared_perm;
}
+int blk_request_perm(BlockBackend *blk, uint64_t perm, Error **errp)
+{
+ uint64_t blk_perm, shared_perm;
+
+ blk_get_perm(blk, &blk_perm, &shared_perm);
+ if ((blk_perm & perm) == perm) {
+ return 0;
+ }
+ blk_perm |= perm;
+ return blk_set_perm(blk, blk_perm, shared_perm, errp);
+}
+
static int blk_do_attach_dev(BlockBackend *blk, void *dev)
{
if (blk->dev) {
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 840ad61..fc23a9e 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -116,6 +116,7 @@ bool bdrv_is_root_node(BlockDriverState *bs);
int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm,
Error **errp);
void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm);
+int blk_request_perm(BlockBackend *blk, uint64_t perm, Error **errp);
void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow);
void blk_iostatus_enable(BlockBackend *blk);
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 03/16] blockjob: Add BLK_PERM_AIO_CONTEXT_CHANGE shared perm on bs
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 01/16] block: Define BLK_PERM_AIO_CONTEXT_CHANGE Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 02/16] block-backend: Add blk_request_perm Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 04/16] blockjob: Allow aio context change on intermediate nodes Fam Zheng
` (13 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
blockjob.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/blockjob.c b/blockjob.c
index 6e48932..b20fb86 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -214,6 +214,9 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
}
}
+ /* The notifier we'll register on @blk takes care of following context
+ * change, so permit it. */
+ shared_perm |= BLK_PERM_AIO_CONTEXT_CHANGE;
blk = blk_new(perm, shared_perm);
ret = blk_insert_bs(blk, bs, errp);
if (ret < 0) {
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 04/16] blockjob: Allow aio context change on intermediate nodes
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (2 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 03/16] blockjob: Add BLK_PERM_AIO_CONTEXT_CHANGE shared perm on bs Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 05/16] block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph Fam Zheng
` (12 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
The intermediate nodes do work with aio context change, so allow that
operations.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/commit.c | 3 ++-
block/mirror.c | 3 ++-
block/stream.c | 3 ++-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 76a0d98..e2ee0ff 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -365,7 +365,8 @@ void commit_start(const char *job_id, BlockDriverState *bs,
* for its backing file). The other options would be a second filter
* driver above s->base. */
ret = block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
- BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE,
+ BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE |
+ BLK_PERM_AIO_CONTEXT_CHANGE,
errp);
if (ret < 0) {
goto fail;
diff --git a/block/mirror.c b/block/mirror.c
index e86f8f8..03e82eb 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1232,7 +1232,8 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
* also blocked for its backing file). The other options would be a
* second filter driver above s->base (== target). */
ret = block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
- BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE,
+ BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE |
+ BLK_PERM_AIO_CONTEXT_CHANGE,
errp);
if (ret < 0) {
goto fail;
diff --git a/block/stream.c b/block/stream.c
index 0113710..2fab1f4 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -265,7 +265,8 @@ void stream_start(const char *job_id, BlockDriverState *bs,
* and resizes. */
for (iter = backing_bs(bs); iter && iter != base; iter = backing_bs(iter)) {
block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
- BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED,
+ BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
+ BLK_PERM_AIO_CONTEXT_CHANGE,
&error_abort);
}
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 05/16] block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (3 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 04/16] blockjob: Allow aio context change on intermediate nodes Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 06/16] backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target Fam Zheng
` (11 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
bdrv_set_aio_context can take care of children recursively, so it is
okay to pass down the perm.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 10 ++++++----
block/vvfat.c | 2 +-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index d98662f..1e5eae7 100644
--- a/block.c
+++ b/block.c
@@ -1772,7 +1772,8 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
#define DEFAULT_PERM_PASSTHROUGH (BLK_PERM_CONSISTENT_READ \
| BLK_PERM_WRITE \
| BLK_PERM_WRITE_UNCHANGED \
- | BLK_PERM_RESIZE)
+ | BLK_PERM_RESIZE \
+ | BLK_PERM_AIO_CONTEXT_CHANGE)
#define DEFAULT_PERM_UNCHANGED (BLK_PERM_ALL & ~DEFAULT_PERM_PASSTHROUGH)
void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c,
@@ -1815,9 +1816,10 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
perm |= BLK_PERM_CONSISTENT_READ;
shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
} else {
- /* We want consistent read from backing files if the parent needs it.
+ /* We want consistent read and aio context change from backing files if
+ * the parent needs it.
* No other operations are performed on backing files. */
- perm &= BLK_PERM_CONSISTENT_READ;
+ perm &= BLK_PERM_CONSISTENT_READ | BLK_PERM_AIO_CONTEXT_CHANGE;
/* If the parent can deal with changing data, we're okay with a
* writable and resizable backing file. */
@@ -1829,7 +1831,7 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
}
shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_GRAPH_MOD |
- BLK_PERM_WRITE_UNCHANGED;
+ BLK_PERM_WRITE_UNCHANGED | BLK_PERM_AIO_CONTEXT_CHANGE;
}
if (bs->open_flags & BDRV_O_INACTIVE) {
diff --git a/block/vvfat.c b/block/vvfat.c
index 426ca70..599a370 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3091,7 +3091,7 @@ static void vvfat_child_perm(BlockDriverState *bs, BdrvChild *c,
if (c == s->qcow) {
/* This is a private node, nobody should try to attach to it */
*nperm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE;
- *nshared = BLK_PERM_WRITE_UNCHANGED;
+ *nshared = BLK_PERM_WRITE_UNCHANGED | BLK_PERM_AIO_CONTEXT_CHANGE;
} else {
/* The backing file is there so 'commit' can use it. vvfat doesn't
* access it in any way. */
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 06/16] backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (4 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 05/16] block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-06-07 12:13 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 07/16] backup: Do initial aio context move of target via BB interface Fam Zheng
` (10 subsequent siblings)
16 siblings, 1 reply; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
What's done in the source's context change notifier is moving the
target's context to follow the new one, so we request this permission
here.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/backup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/backup.c b/block/backup.c
index a4fb288..546c5c5 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -636,7 +636,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
}
/* The target must match the source in size, so no resize here either */
- job->target = blk_new(BLK_PERM_WRITE,
+ job->target = blk_new(BLK_PERM_WRITE | BLK_PERM_AIO_CONTEXT_CHANGE,
BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE |
BLK_PERM_WRITE_UNCHANGED | BLK_PERM_GRAPH_MOD);
ret = blk_insert_bs(job->target, target, errp);
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 07/16] backup: Do initial aio context move of target via BB interface
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (5 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 06/16] backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 08/16] mirror: Request aio context change permission on target Fam Zheng
` (9 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
The old aio context check is hacky because when it was added we didn't
have the permission system to enforce a general rule. It only checks if
the target BDS has a BB, which is in fact insufficient because there may
be other BBs in the graph that cannot handle the aio context change.
Do this through blk_set_aio_context interface, in backup_job_create()
which is a central place for both drive-backup and blockdev-backup, to
take care of the compatibility check.
Also the bdrv_set_aio_context in do_drive_backup could have been
conditional, to save a recursion when possible.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/backup.c | 9 +++++++++
blockdev.c | 14 --------------
2 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 546c5c5..9136f91 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -564,6 +564,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BlockDriverInfo bdi;
BackupBlockJob *job = NULL;
int ret;
+ AioContext *aio_context, *target_context;
assert(bs);
assert(target);
@@ -644,6 +645,14 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
goto error;
}
+ aio_context = bdrv_get_aio_context(bs);
+ target_context = bdrv_get_aio_context(target);
+ if (target_context != aio_context) {
+ aio_context_acquire(target_context);
+ blk_set_aio_context(job->target, aio_context);
+ aio_context_release(target_context);
+ }
+
job->on_source_error = on_source_error;
job->on_target_error = on_target_error;
job->sync_mode = sync_mode;
diff --git a/blockdev.c b/blockdev.c
index c63f4e8..e8f72a1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3240,8 +3240,6 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
goto out;
}
- bdrv_set_aio_context(target_bs, aio_context);
-
if (set_backing_hd) {
bdrv_set_backing_hd(target_bs, source, &local_err);
if (local_err) {
@@ -3326,18 +3324,6 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
if (!target_bs) {
goto out;
}
-
- if (bdrv_get_aio_context(target_bs) != aio_context) {
- if (!bdrv_has_blk(target_bs)) {
- /* The target BDS is not attached, we can safely move it to another
- * AioContext. */
- bdrv_set_aio_context(target_bs, aio_context);
- } else {
- error_setg(errp, "Target is attached to a different thread from "
- "source.");
- goto out;
- }
- }
job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
backup->sync, NULL, backup->compress,
backup->on_source_error, backup->on_target_error,
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 08/16] mirror: Request aio context change permission on target
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (6 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 07/16] backup: Do initial aio context move of target via BB interface Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 09/16] commit: Allow aio context change on s->base Fam Zheng
` (8 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
What's done in the source's context change notifier is moving the
target's context to follow the new one, so we request this permission
here.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/mirror.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/mirror.c b/block/mirror.c
index 03e82eb..a3337ee 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1190,6 +1190,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
target_is_backing = bdrv_chain_contains(bs, target);
target_graph_mod = (backing_mode != MIRROR_LEAVE_BACKING_CHAIN);
s->target = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE |
+ BLK_PERM_AIO_CONTEXT_CHANGE |
(target_graph_mod ? BLK_PERM_GRAPH_MOD : 0),
BLK_PERM_WRITE_UNCHANGED |
(target_is_backing ? BLK_PERM_CONSISTENT_READ |
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 09/16] commit: Allow aio context change on s->base
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (7 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 08/16] mirror: Request aio context change permission on target Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 10/16] mirror: Do initial aio context move of target via BB interface Fam Zheng
` (7 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
The block job has the aio context change notifier, we should allow it
here as well.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/commit.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/commit.c b/block/commit.c
index e2ee0ff..bbf7768 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -391,7 +391,8 @@ void commit_start(const char *job_id, BlockDriverState *bs,
| BLK_PERM_RESIZE,
BLK_PERM_CONSISTENT_READ
| BLK_PERM_GRAPH_MOD
- | BLK_PERM_WRITE_UNCHANGED);
+ | BLK_PERM_WRITE_UNCHANGED
+ | BLK_PERM_AIO_CONTEXT_CHANGE);
ret = blk_insert_bs(s->base, base, errp);
if (ret < 0) {
goto fail;
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 10/16] mirror: Do initial aio context move of target via BB interface
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (8 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 09/16] commit: Allow aio context change on s->base Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-31 9:43 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 11/16] virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane Fam Zheng
` (6 subsequent siblings)
16 siblings, 1 reply; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
While blockdev-backup tried to verify before moving target's aio context, the
same is missing for blockdev-mirror. Now that we have the right interface, fix
this as well.
As a bonus, the aio context move is now conditional, which avoids some
unnecessary operations in bdrv_set_aio_context.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/mirror.c | 54 ++++++++++++++++++++++++++++++++----------------------
blockdev.c | 4 ----
2 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index a3337ee..4eccb8d 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1118,13 +1118,44 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
bool auto_complete, const char *filter_node_name,
Error **errp)
{
+ AioContext *aio_context, *target_context;
MirrorBlockJob *s;
BlockDriverState *mirror_top_bs;
+ BlockBackend *target_blk;
bool target_graph_mod;
bool target_is_backing;
Error *local_err = NULL;
int ret;
+ /* No resize for the target either; while the mirror is still running, a
+ * consistent read isn't necessarily possible. We could possibly allow
+ * writes and graph modifications, though it would likely defeat the
+ * purpose of a mirror, so leave them blocked for now.
+ *
+ * In the case of active commit, things look a bit different, though,
+ * because the target is an already populated backing file in active use.
+ * We can allow anything except resize there.*/
+ target_is_backing = bdrv_chain_contains(bs, target);
+ target_graph_mod = (backing_mode != MIRROR_LEAVE_BACKING_CHAIN);
+ target_blk = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE |
+ BLK_PERM_AIO_CONTEXT_CHANGE |
+ (target_graph_mod ? BLK_PERM_GRAPH_MOD : 0),
+ BLK_PERM_WRITE_UNCHANGED |
+ (target_is_backing ? BLK_PERM_CONSISTENT_READ |
+ BLK_PERM_WRITE |
+ BLK_PERM_GRAPH_MOD : 0));
+ ret = blk_insert_bs(target_blk, target, errp);
+ if (ret < 0) {
+ blk_unref(target_blk);
+ return;
+ }
+ aio_context = bdrv_get_aio_context(bs);
+ target_context = bdrv_get_aio_context(target);
+ if (target_context != aio_context) {
+ aio_context_acquire(target_context);
+ blk_set_aio_context(target_blk, aio_context);
+ aio_context_release(target_context);
+ }
if (granularity == 0) {
granularity = bdrv_get_default_bitmap_granularity(target);
}
@@ -1179,28 +1210,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
s->source = bs;
s->mirror_top_bs = mirror_top_bs;
- /* No resize for the target either; while the mirror is still running, a
- * consistent read isn't necessarily possible. We could possibly allow
- * writes and graph modifications, though it would likely defeat the
- * purpose of a mirror, so leave them blocked for now.
- *
- * In the case of active commit, things look a bit different, though,
- * because the target is an already populated backing file in active use.
- * We can allow anything except resize there.*/
- target_is_backing = bdrv_chain_contains(bs, target);
- target_graph_mod = (backing_mode != MIRROR_LEAVE_BACKING_CHAIN);
- s->target = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE |
- BLK_PERM_AIO_CONTEXT_CHANGE |
- (target_graph_mod ? BLK_PERM_GRAPH_MOD : 0),
- BLK_PERM_WRITE_UNCHANGED |
- (target_is_backing ? BLK_PERM_CONSISTENT_READ |
- BLK_PERM_WRITE |
- BLK_PERM_GRAPH_MOD : 0));
- ret = blk_insert_bs(s->target, target, errp);
- if (ret < 0) {
- goto fail;
- }
-
+ s->target = target_blk;
s->replaces = g_strdup(replaces);
s->on_source_error = on_source_error;
s->on_target_error = on_target_error;
diff --git a/blockdev.c b/blockdev.c
index e8f72a1..52d81c4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3545,8 +3545,6 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
goto out;
}
- bdrv_set_aio_context(target_bs, aio_context);
-
blockdev_mirror_common(arg->has_job_id ? arg->job_id : NULL, bs, target_bs,
arg->has_replaces, arg->replaces, arg->sync,
backing_mode, arg->has_speed, arg->speed,
@@ -3597,8 +3595,6 @@ void qmp_blockdev_mirror(bool has_job_id, const char *job_id,
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
- bdrv_set_aio_context(target_bs, aio_context);
-
blockdev_mirror_common(has_job_id ? job_id : NULL, bs, target_bs,
has_replaces, replaces, sync, backing_mode,
has_speed, speed,
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 11/16] virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (9 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 10/16] mirror: Do initial aio context move of target via BB interface Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-06-07 8:11 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 12/16] virtio-blk: " Fam Zheng
` (5 subsequent siblings)
16 siblings, 1 reply; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
blk_set_aio_context is audited by perm API, so follow the protocol and
request for permission first.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/scsi/virtio-scsi.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 46a3e3f..074e235 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -794,6 +794,10 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
return;
}
virtio_scsi_acquire(s);
+ if (!blk_request_perm(sd->conf.blk, BLK_PERM_AIO_CONTEXT_CHANGE, errp)) {
+ virtio_scsi_release(s);
+ return;
+ }
blk_set_aio_context(sd->conf.blk, s->ctx);
virtio_scsi_release(s);
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 12/16] virtio-blk: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (10 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 11/16] virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 13/16] blk: fix aio context loss on media change Fam Zheng
` (4 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
blk_set_aio_context is audited by perm API, so follow the protocol and
request for permission first.
Previously the return code in error path was hardcoded as -EPERM, but
returning 'r' is more meaningful here both for the new error and
existing ones.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/block/dataplane/virtio-blk.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 5556f0e..8f2ff2df 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -168,6 +168,7 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
unsigned i;
unsigned nvqs = s->conf->num_queues;
int r;
+ Error *local_err = NULL;
if (vblk->dataplane_started || s->starting) {
return 0;
@@ -175,12 +176,18 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
s->starting = true;
+ r = blk_request_perm(s->conf->conf.blk, BLK_PERM_AIO_CONTEXT_CHANGE,
+ &local_err);
+ if (r) {
+ error_report_err(local_err);
+ goto fail;
+ }
/* Set up guest notifier (irq) */
r = k->set_guest_notifiers(qbus->parent, nvqs, true);
if (r != 0) {
fprintf(stderr, "virtio-blk failed to set guest notifier (%d), "
"ensure -enable-kvm is set\n", r);
- goto fail_guest_notifiers;
+ goto fail;
}
/* Set up virtqueue notify */
@@ -191,7 +198,7 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
while (i--) {
virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
}
- goto fail_guest_notifiers;
+ goto fail;
}
}
@@ -219,11 +226,11 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
aio_context_release(s->ctx);
return 0;
- fail_guest_notifiers:
+fail:
vblk->dataplane_disabled = true;
s->starting = false;
vblk->dataplane_started = true;
- return -ENOSYS;
+ return r;
}
/* Context: QEMU global mutex held */
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 13/16] blk: fix aio context loss on media change
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (11 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 12/16] virtio-blk: " Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 14/16] nbd: Allow BLK_PERM_AIO_CONTEXT_CHANGE on BB Fam Zheng
` (3 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
If we have separate iothread for cdrom, we lose connection to it on
qmp_blockdev_change_medium, as aio_context is on bds which is dropped
and switched with new one.
As an example result, after such media change we have crash on
virtio_scsi_ctx_check: Assertion `blk_get_aio_context(d->conf.blk) == s->ctx' failed.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/block-backend.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index 5492f64..dfe577d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -68,6 +68,7 @@ struct BlockBackend {
NotifierList remove_bs_notifiers, insert_bs_notifiers;
int quiesce_counter;
+ AioContext *aio_context;
};
typedef struct BlockBackendAIOCB {
@@ -618,6 +619,10 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
}
bdrv_ref(bs);
+ if (blk->aio_context != NULL) {
+ bdrv_set_aio_context(bs, blk->aio_context);
+ }
+
notifier_list_notify(&blk->insert_bs_notifiers, blk);
if (blk->public.throttle_state) {
throttle_timers_attach_aio_context(
@@ -1692,6 +1697,7 @@ void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
{
BlockDriverState *bs = blk_bs(blk);
+ blk->aio_context = new_context;
if (bs) {
if (blk->public.throttle_state) {
throttle_timers_detach_aio_context(&blk->public.throttle_timers);
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 14/16] nbd: Allow BLK_PERM_AIO_CONTEXT_CHANGE on BB
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (12 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 13/16] blk: fix aio context loss on media change Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 15/16] block: Add perm assertion on blk_set_aio_context Fam Zheng
` (2 subsequent siblings)
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
This is safe because of the aio context notifier we'll register on this
node. So allow it.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
nbd/server.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 924a1fe..a8f58fb 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -900,8 +900,10 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) {
perm |= BLK_PERM_WRITE;
}
- blk = blk_new(perm, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
- BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD);
+ blk = blk_new(perm,
+ BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
+ BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD |
+ BLK_PERM_AIO_CONTEXT_CHANGE);
ret = blk_insert_bs(blk, bs, errp);
if (ret < 0) {
goto fail;
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 15/16] block: Add perm assertion on blk_set_aio_context
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (13 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 14/16] nbd: Allow BLK_PERM_AIO_CONTEXT_CHANGE on BB Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 16/16] tests: Add test case for BLK_PERM_AIO_CONTEXT_CHANGE Fam Zheng
2017-05-31 9:45 ` [Qemu-devel] [Qemu-block] [PATCH v3 00/16] block: Protect AIO context change with perm API Stefan Hajnoczi
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
Now that all BB users comply with the BLK_PERM_AIO_CONTEXT_CHANGE
rule, we can assert it.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/block-backend.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index dfe577d..51c62ea 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1695,8 +1695,12 @@ static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb)
void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
{
+ uint64_t perm, shared_perm;
BlockDriverState *bs = blk_bs(blk);
+ blk_get_perm(blk, &perm, &shared_perm);
+ assert(perm & BLK_PERM_AIO_CONTEXT_CHANGE);
+
blk->aio_context = new_context;
if (bs) {
if (blk->public.throttle_state) {
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 16/16] tests: Add test case for BLK_PERM_AIO_CONTEXT_CHANGE
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (14 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 15/16] block: Add perm assertion on blk_set_aio_context Fam Zheng
@ 2017-05-24 2:52 ` Fam Zheng
2017-05-31 9:45 ` [Qemu-devel] [Qemu-block] [PATCH v3 00/16] block: Protect AIO context change with perm API Stefan Hajnoczi
16 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-24 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/Makefile.include | 2 ++
tests/test-blk-perm.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
create mode 100644 tests/test-blk-perm.c
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7589383..5811296 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -56,6 +56,7 @@ gcov-files-test-thread-pool-y = thread-pool.c
gcov-files-test-hbitmap-y = util/hbitmap.c
check-unit-y += tests/test-hbitmap$(EXESUF)
gcov-files-test-hbitmap-y = blockjob.c
+check-unit-y += tests/test-blk-perm$(EXESUF)
check-unit-y += tests/test-blockjob$(EXESUF)
check-unit-y += tests/test-blockjob-txn$(EXESUF)
check-unit-y += tests/test-x86-cpuid$(EXESUF)
@@ -548,6 +549,7 @@ tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y)
tests/test-aio$(EXESUF): tests/test-aio.o $(test-block-obj-y)
tests/test-aio-multithread$(EXESUF): tests/test-aio-multithread.o $(test-block-obj-y)
tests/test-throttle$(EXESUF): tests/test-throttle.o $(test-block-obj-y)
+tests/test-blk-perm$(EXESUF): tests/test-blk-perm.o $(test-block-obj-y)
tests/test-blockjob$(EXESUF): tests/test-blockjob.o $(test-block-obj-y) $(test-util-obj-y)
tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o $(test-block-obj-y) $(test-util-obj-y)
tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y)
diff --git a/tests/test-blk-perm.c b/tests/test-blk-perm.c
new file mode 100644
index 0000000..1c7b5d2
--- /dev/null
+++ b/tests/test-blk-perm.c
@@ -0,0 +1,59 @@
+/*
+ * Block permission tests
+ *
+ * Copyright Red Hat, Inc. 2017
+ *
+ * Authors:
+ * Fam Zheng <famz@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "sysemu/block-backend.h"
+
+static void test_aio_context_success(void)
+{
+ BlockBackend *blk1 = blk_new(BLK_PERM_AIO_CONTEXT_CHANGE, BLK_PERM_ALL);
+ BlockBackend *blk2 = blk_new(BLK_PERM_AIO_CONTEXT_CHANGE, BLK_PERM_ALL);
+ BlockDriverState *bs = bdrv_open("null-co://", NULL, NULL, 0, &error_abort);
+
+ blk_insert_bs(blk1, bs, &error_abort);
+ blk_insert_bs(blk2, bs, &error_abort);
+
+ blk_unref(blk1);
+ blk_unref(blk2);
+ bdrv_unref(bs);
+}
+
+static void test_aio_context_failure(void)
+{
+ Error *local_err = NULL;
+ BlockBackend *blk1 = blk_new(BLK_PERM_AIO_CONTEXT_CHANGE,
+ BLK_PERM_ALL & ~BLK_PERM_AIO_CONTEXT_CHANGE);
+ BlockBackend *blk2 = blk_new(BLK_PERM_AIO_CONTEXT_CHANGE, BLK_PERM_ALL);
+ BlockDriverState *bs = bdrv_open("null-co://", NULL, NULL, 0, &error_abort);
+
+ blk_insert_bs(blk1, bs, &error_abort);
+ blk_insert_bs(blk2, bs, &local_err);
+
+ error_free_or_abort(&local_err);
+
+ blk_unref(blk1);
+ blk_unref(blk2);
+ bdrv_unref(bs);
+}
+
+int main(int argc, char **argv)
+{
+ bdrv_init();
+ qemu_init_main_loop(&error_abort);
+ g_test_init(&argc, &argv, NULL);
+ g_test_add_func("/block/perm/aio-context/success",
+ test_aio_context_success);
+ g_test_add_func("/block/perm/aio-context/failure",
+ test_aio_context_failure);
+ return g_test_run();
+}
--
2.9.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 02/16] block-backend: Add blk_request_perm
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 02/16] block-backend: Add blk_request_perm Fam Zheng
@ 2017-05-31 9:41 ` Stefan Hajnoczi
0 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2017-05-31 9:41 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, Kevin Wolf, qemu-block, Stefan Hajnoczi, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 389 bytes --]
On Wed, May 24, 2017 at 10:52:21AM +0800, Fam Zheng wrote:
> This function tries to request, if not granted yet, for the given
> permissions.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block/block-backend.c | 12 ++++++++++++
> include/sysemu/block-backend.h | 1 +
> 2 files changed, 13 insertions(+)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 10/16] mirror: Do initial aio context move of target via BB interface
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 10/16] mirror: Do initial aio context move of target via BB interface Fam Zheng
@ 2017-05-31 9:43 ` Stefan Hajnoczi
0 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2017-05-31 9:43 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, Kevin Wolf, qemu-block, Stefan Hajnoczi, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 643 bytes --]
On Wed, May 24, 2017 at 10:52:29AM +0800, Fam Zheng wrote:
> While blockdev-backup tried to verify before moving target's aio context, the
> same is missing for blockdev-mirror. Now that we have the right interface, fix
> this as well.
>
> As a bonus, the aio context move is now conditional, which avoids some
> unnecessary operations in bdrv_set_aio_context.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block/mirror.c | 54 ++++++++++++++++++++++++++++++++----------------------
> blockdev.c | 4 ----
> 2 files changed, 32 insertions(+), 26 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 00/16] block: Protect AIO context change with perm API
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
` (15 preceding siblings ...)
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 16/16] tests: Add test case for BLK_PERM_AIO_CONTEXT_CHANGE Fam Zheng
@ 2017-05-31 9:45 ` Stefan Hajnoczi
2017-05-31 10:46 ` Fam Zheng
16 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2017-05-31 9:45 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, Kevin Wolf, qemu-block, Stefan Hajnoczi, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 627 bytes --]
On Wed, May 24, 2017 at 10:52:19AM +0800, Fam Zheng wrote:
> v3: Move blk_set_aio_context to the front of mirror_start_job to avoid
> accessing target without acquiring its aio context. [Stefan]
> Use error_free_or_abort in test code. [Stefan]
Please include my Reviewed-bys from the previous revision. That way
they will be included in the commit log and I don't need to look back at
previous revisions when you send the next revision.
I've also left a comment on one of your replies to the previous
revision because I think blockdev-backup's 'target' parameter isn't
protected against AioContext changes.
Thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 00/16] block: Protect AIO context change with perm API
2017-05-31 9:45 ` [Qemu-devel] [Qemu-block] [PATCH v3 00/16] block: Protect AIO context change with perm API Stefan Hajnoczi
@ 2017-05-31 10:46 ` Fam Zheng
0 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-05-31 10:46 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Kevin Wolf, qemu-block, Stefan Hajnoczi, Max Reitz
On Wed, 05/31 10:45, Stefan Hajnoczi wrote:
> On Wed, May 24, 2017 at 10:52:19AM +0800, Fam Zheng wrote:
> > v3: Move blk_set_aio_context to the front of mirror_start_job to avoid
> > accessing target without acquiring its aio context. [Stefan]
> > Use error_free_or_abort in test code. [Stefan]
>
> Please include my Reviewed-bys from the previous revision. That way
> they will be included in the commit log and I don't need to look back at
> previous revisions when you send the next revision.
Yes, sorry for forgetting that.
Fam
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 11/16] virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 11/16] virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane Fam Zheng
@ 2017-06-07 8:11 ` Fam Zheng
0 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-06-07 8:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, qemu-block, Stefan Hajnoczi, Max Reitz
On Wed, 05/24 10:52, Fam Zheng wrote:
> blk_set_aio_context is audited by perm API, so follow the protocol and
> request for permission first.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> hw/scsi/virtio-scsi.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 46a3e3f..074e235 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -794,6 +794,10 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
> return;
> }
> virtio_scsi_acquire(s);
> + if (!blk_request_perm(sd->conf.blk, BLK_PERM_AIO_CONTEXT_CHANGE, errp)) {
Inversed condition, should be s/!//.
Fam
> + virtio_scsi_release(s);
> + return;
> + }
> blk_set_aio_context(sd->conf.blk, s->ctx);
> virtio_scsi_release(s);
>
> --
> 2.9.4
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/16] backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 06/16] backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target Fam Zheng
@ 2017-06-07 12:13 ` Fam Zheng
0 siblings, 0 replies; 23+ messages in thread
From: Fam Zheng @ 2017-06-07 12:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, qemu-block, Stefan Hajnoczi, Max Reitz
On Wed, 05/24 10:52, Fam Zheng wrote:
> What's done in the source's context change notifier is moving the
> target's context to follow the new one, so we request this permission
> here.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block/backup.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index a4fb288..546c5c5 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -636,7 +636,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
> }
>
> /* The target must match the source in size, so no resize here either */
> - job->target = blk_new(BLK_PERM_WRITE,
> + job->target = blk_new(BLK_PERM_WRITE | BLK_PERM_AIO_CONTEXT_CHANGE,
> BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE |
> BLK_PERM_WRITE_UNCHANGED | BLK_PERM_GRAPH_MOD);
> ret = blk_insert_bs(job->target, target, errp);
Since fc0932fdc we reuse @bs as the backing of @target if sync=none, and this
hunk causes problem: the device's BB probably doesn't permit
BLK_PERM_AIO_CONTEXT_CHANGE.
Still scratching my head on this.
Fam
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-06-07 12:13 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-24 2:52 [Qemu-devel] [PATCH v3 00/16] block: Protect AIO context change with perm API Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 01/16] block: Define BLK_PERM_AIO_CONTEXT_CHANGE Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 02/16] block-backend: Add blk_request_perm Fam Zheng
2017-05-31 9:41 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 03/16] blockjob: Add BLK_PERM_AIO_CONTEXT_CHANGE shared perm on bs Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 04/16] blockjob: Allow aio context change on intermediate nodes Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 05/16] block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 06/16] backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target Fam Zheng
2017-06-07 12:13 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 07/16] backup: Do initial aio context move of target via BB interface Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 08/16] mirror: Request aio context change permission on target Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 09/16] commit: Allow aio context change on s->base Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 10/16] mirror: Do initial aio context move of target via BB interface Fam Zheng
2017-05-31 9:43 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 11/16] virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane Fam Zheng
2017-06-07 8:11 ` Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 12/16] virtio-blk: " Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 13/16] blk: fix aio context loss on media change Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 14/16] nbd: Allow BLK_PERM_AIO_CONTEXT_CHANGE on BB Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 15/16] block: Add perm assertion on blk_set_aio_context Fam Zheng
2017-05-24 2:52 ` [Qemu-devel] [PATCH v3 16/16] tests: Add test case for BLK_PERM_AIO_CONTEXT_CHANGE Fam Zheng
2017-05-31 9:45 ` [Qemu-devel] [Qemu-block] [PATCH v3 00/16] block: Protect AIO context change with perm API Stefan Hajnoczi
2017-05-31 10:46 ` Fam Zheng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.