Acked-by: Fam Zheng <famz@redhat.com>

Acked-by: Max Reitz <mreitz@redhat.com>

- BlockDriverState *src, *target;

+ BlockDriverState *target;

- src = blk_bs(job->blk);

target = blk_bs(s->target);

if (!s->synced) {

@@ -XXX,XX +XXX,XX @@ static void mirror_complete(BlockJob *job, Error **errp)

replace_aio_context = bdrv_get_aio_context(s->to_replace);

aio_context_acquire(replace_aio_context);

+ /* TODO Translate this into permission system. Current definition of

+ * GRAPH_MOD would require to request it for the parents; they might

+ * not even be BlockDriverStates, however, so a BdrvChild can't address

+ * them. May need redefinition of GRAPH_MOD. */

error_setg(&s->replace_blocker,

"block device is in use by block-job-complete");

bdrv_op_block_all(s->to_replace, s->replace_blocker);

@@ -XXX,XX +XXX,XX @@ static void mirror_complete(BlockJob *job, Error **errp)

aio_context_release(replace_aio_context);

}

- if (s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {

- BlockDriverState *backing = s->is_none_mode ? src : s->base;

- if (backing_bs(target) != backing) {

- bdrv_set_backing_hd(target, backing);

- }

- }

-

s->should_complete = true;

block_job_enter(&s->common);

}

@@ -XXX,XX +XXX,XX @@ static const BlockJobDriver commit_active_job_driver = {

.drain = mirror_drain,

};

+static int coroutine_fn bdrv_mirror_top_preadv(BlockDriverState *bs,

+ uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)

+{

+ return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);

+}

void mirror_start(const char *job_id, BlockDriverState *bs,

diff --git a/qemu-img.c b/qemu-img.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ static void run_block_job(BlockJob *job, Error **errp)

{

AioContext *aio_context = blk_get_aio_context(job->blk);

+ /* FIXME In error cases, the job simply goes away and we access a dangling

+ * pointer below. */

aio_context_acquire(aio_context);

do {

aio_poll(aio_context, true);

@@ -XXX,XX +XXX,XX @@ static int img_commit(int argc, char **argv)

const char *filename, *fmt, *cache, *base;

BlockBackend *blk;

BlockDriverState *bs, *base_bs;

+ BlockJob *job;

bool progress = false, quiet = false, drop = false;

bool writethrough;

Error *local_err = NULL;

@@ -XXX,XX +XXX,XX @@ static int img_commit(int argc, char **argv)

bdrv_ref(bs);

}

- run_block_job(bs->job, &local_err);

+ job = block_job_get("commit");

+ run_block_job(job, &local_err);

if (local_err) {

goto unref_backing;

}

diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/141

+++ b/tests/qemu-iotests/141

@@ -XXX,XX +XXX,XX @@ test_blockjob()

_send_qemu_cmd $QEMU_HANDLE \

"{'execute': 'x-blockdev-del',

'arguments': {'node-name': 'drv0'}}" \

- 'error'

+ 'error' | _filter_generated_node_ids

_send_qemu_cmd $QEMU_HANDLE \

"{'execute': 'block-job-cancel',

diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/141.out

+++ b/tests/qemu-iotests/141.out

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.

Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}

{"return": {}}

-{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}}

+{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is used as backing hd of 'NODE_NAME'"}}

{"return": {}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}

{"return": {}}

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.

{"return": {}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}

{"return": {}}

-{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}}

+{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is used as backing hd of 'NODE_NAME'"}}

{"return": {}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}

{"return": {}}

diff --git a/block.c b/block.c

index XXXXXXX..XXXXXXX 100644

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ static BlockDriverState *bdrv_open_inherit(const char *filename,

goto fail;

}

if (file_bs != NULL) {

- file = blk_new();

+ file = blk_new(BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL);

blk_insert_bs(file, file_bs);

bdrv_unref(file_bs);

diff --git a/block/backup.c b/block/backup.c

index XXXXXXX..XXXXXXX 100644

--- a/block/backup.c

+++ b/block/backup.c

@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,

goto error;

}

- job->target = blk_new();

+ /* FIXME Use real permissions */

+ job->target = blk_new(0, BLK_PERM_ALL);

blk_insert_bs(job->target, target);

job->on_source_error = on_source_error;

diff --git a/block/block-backend.c b/block/block-backend.c

index XXXXXXX..XXXXXXX 100644

--- a/block/block-backend.c

+++ b/block/block-backend.c

@@ -XXX,XX +XXX,XX @@ static const BdrvChildRole child_root = {

/*

* Create a new BlockBackend with a reference count of one.

- * Store an error through @errp on failure, unless it's null.

+ *

+ * @perm is a bitmasks of BLK_PERM_* constants which describes the permissions

+ * to request for a block driver node that is attached to this BlockBackend.

+ * @shared_perm is a bitmask which describes which permissions may be granted

+ * to other users of the attached node.

+ * Both sets of permissions can be changed later using blk_set_perm().

+ *

* Return the new BlockBackend on success, null on failure.

*/

-BlockBackend *blk_new(void)

+BlockBackend *blk_new(uint64_t perm, uint64_t shared_perm)

{

BlockBackend *blk;

blk = g_new0(BlockBackend, 1);

blk->refcnt = 1;

- blk->perm = 0;

- blk->shared_perm = BLK_PERM_ALL;

+ blk->perm = perm;

+ blk->shared_perm = shared_perm;

blk_set_enable_write_cache(blk, true);

qemu_co_queue_init(&blk->public.throttled_reqs[0]);

@@ -XXX,XX +XXX,XX @@ BlockBackend *blk_new_open(const char *filename, const char *reference,

BlockBackend *blk;

BlockDriverState *bs;

- blk = blk_new();

+ blk = blk_new(0, BLK_PERM_ALL);

bs = bdrv_open(filename, reference, options, flags, errp);

if (!bs) {

blk_unref(blk);

@@ -XXX,XX +XXX,XX @@ void blk_remove_bs(BlockBackend *blk)

void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)

{

bdrv_ref(bs);

- /* FIXME Use real permissions */

+ /* FIXME Error handling */

blk->root = bdrv_root_attach_child(bs, "root", &child_root,

- 0, BLK_PERM_ALL, blk, &error_abort);

+ blk->perm, blk->shared_perm, blk,

+ &error_abort);

notifier_list_notify(&blk->insert_bs_notifiers, blk);

if (blk->public.throttle_state) {

diff --git a/block/commit.c b/block/commit.c

index XXXXXXX..XXXXXXX 100644

--- a/block/commit.c

+++ b/block/commit.c

@@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs,

block_job_add_bdrv(&s->common, overlay_bs);

}

- s->base = blk_new();

+ /* FIXME Use real permissions */

+ s->base = blk_new(0, BLK_PERM_ALL);

blk_insert_bs(s->base, base);

- s->top = blk_new();

+ /* FIXME Use real permissions */

+ s->top = blk_new(0, BLK_PERM_ALL);

blk_insert_bs(s->top, top);

s->active = bs;

@@ -XXX,XX +XXX,XX @@ int bdrv_commit(BlockDriverState *bs)

}

}

- src = blk_new();

+ /* FIXME Use real permissions */

+ src = blk_new(0, BLK_PERM_ALL);

blk_insert_bs(src, bs);

- backing = blk_new();

+ /* FIXME Use real permissions */

+ backing = blk_new(0, BLK_PERM_ALL);

blk_insert_bs(backing, bs->backing->bs);

length = blk_getlength(src);

diff --git a/block/mirror.c b/block/mirror.c

index XXXXXXX..XXXXXXX 100644

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,

return;

}

- s->target = blk_new();

+ /* FIXME Use real permissions */

+ s->target = blk_new(0, BLK_PERM_ALL);

blk_insert_bs(s->target, target);

s->replaces = g_strdup(replaces);

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/ide/qdev.c

+++ b/hw/ide/qdev.c

@@ -XXX,XX +XXX,XX @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)

return -1;

} else {

/* Anonymous BlockBackend for an empty drive */

- dev->conf.blk = blk_new();

+ /* FIXME Use real permissions */

+ dev->conf.blk = blk_new(0, BLK_PERM_ALL);

}

}

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/scsi/scsi-disk.c

+++ b/hw/scsi/scsi-disk.c

@@ -XXX,XX +XXX,XX @@ static void scsi_cd_realize(SCSIDevice *dev, Error **errp)

SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);

if (!dev->conf.blk) {

- dev->conf.blk = blk_new();

+ /* FIXME Use real permissions */

+ dev->conf.blk = blk_new(0, BLK_PERM_ALL);

}

s->qdev.blocksize = 2048;

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h

index XXXXXXX..XXXXXXX 100644

--- a/include/sysemu/block-backend.h

+++ b/include/sysemu/block-backend.h

@@ -XXX,XX +XXX,XX @@ typedef struct BlockBackendPublic {

QLIST_ENTRY(BlockBackendPublic) round_robin;

} BlockBackendPublic;

-BlockBackend *blk_new(void);

+BlockBackend *blk_new(uint64_t perm, uint64_t shared_perm);

BlockBackend *blk_new_open(const char *filename, const char *reference,

QDict *options, int flags, Error **errp);

int blk_get_refcnt(BlockBackend *blk);

diff --git a/migration/block.c b/migration/block.c

index XXXXXXX..XXXXXXX 100644

--- a/migration/block.c

+++ b/migration/block.c

@@ -XXX,XX +XXX,XX @@ static void init_blk_migration(QEMUFile *f)

}

bmds = g_new0(BlkMigDevState, 1);

- bmds->blk = blk_new();

+ /* FIXME Use real permissions */

+ bmds->blk = blk_new(0, BLK_PERM_ALL);

bmds->blk_name = g_strdup(bdrv_get_device_name(bs));

bmds->bulk_completed = 0;

bmds->total_sectors = sectors;

diff --git a/nbd/server.c b/nbd/server.c

index XXXXXXX..XXXXXXX 100644

--- a/nbd/server.c

+++ b/nbd/server.c

@@ -XXX,XX +XXX,XX @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,

BlockBackend *blk;

NBDExport *exp = g_malloc0(sizeof(NBDExport));

- blk = blk_new();

+ /* FIXME Use real permissions */

+ blk = blk_new(0, BLK_PERM_ALL);

blk_insert_bs(blk, bs);

blk_set_enable_write_cache(blk, !writethrough);

diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c

index XXXXXXX..XXXXXXX 100644

--- a/tests/test-blockjob.c

+++ b/tests/test-blockjob.c

@@ -XXX,XX +XXX,XX @@ static BlockJob *do_test_id(BlockBackend *blk, const char *id,

* BlockDriverState inserted. */

static BlockBackend *create_blk(const char *name)

{

- BlockBackend *blk = blk_new();

+ /* FIXME Use real permissions */

+ BlockBackend *blk = blk_new(0, BLK_PERM_ALL);

BlockDriverState *bs;

bs = bdrv_open("null-co://", NULL, NULL, 0, &error_abort);

diff --git a/tests/test-throttle.c b/tests/test-throttle.c

index XXXXXXX..XXXXXXX 100644

--- a/tests/test-throttle.c

+++ b/tests/test-throttle.c

@@ -XXX,XX +XXX,XX @@ static void test_groups(void)

BlockBackend *blk1, *blk2, *blk3;

BlockBackendPublic *blkp1, *blkp2, *blkp3;

- blk1 = blk_new();

- blk2 = blk_new();

- blk3 = blk_new();

+ /* FIXME Use real permissions */

+ blk1 = blk_new(0, BLK_PERM_ALL);

+ blk2 = blk_new(0, BLK_PERM_ALL);

+ blk3 = blk_new(0, BLK_PERM_ALL);

blkp1 = blk_get_public(blk1);

blkp2 = blk_get_public(blk2);

Acked-by: Fam Zheng <famz@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

diff --git a/block/commit.c b/block/commit.c

index XXXXXXX..XXXXXXX 100644

--- a/block/commit.c

+++ b/block/commit.c

@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_commit_top = {

void commit_start(const char *job_id, BlockDriverState *bs,

BlockDriverState *base, BlockDriverState *top, int64_t speed,

BlockdevOnError on_error, const char *backing_file_str,

- Error **errp)

+ const char *filter_node_name, Error **errp)

{

CommitBlockJob *s;

BlockReopenQueue *reopen_queue = NULL;

@@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs,

/* Insert commit_top block node above top, so we can block consistent read

* on the backing chain below it */

- commit_top_bs = bdrv_new_open_driver(&bdrv_commit_top, NULL, 0, errp);

+ commit_top_bs = bdrv_new_open_driver(&bdrv_commit_top, filter_node_name, 0,

+ errp);

if (commit_top_bs == NULL) {

goto fail;

}

diff --git a/block/mirror.c b/block/mirror.c

index XXXXXXX..XXXXXXX 100644

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ void mirror_start(const char *job_id, BlockDriverState *bs,

void commit_active_start(const char *job_id, BlockDriverState *bs,

BlockDriverState *base, int creation_flags,

int64_t speed, BlockdevOnError on_error,

+ const char *filter_node_name,

BlockCompletionFunc *cb, void *opaque, Error **errp,

bool auto_complete)

{

@@ -XXX,XX +XXX,XX @@ void commit_active_start(const char *job_id, BlockDriverState *bs,

MIRROR_LEAVE_BACKING_CHAIN,

on_error, on_error, true, cb, opaque, &local_err,

&commit_active_job_driver, false, base, auto_complete,

- NULL);

+ filter_node_name);

if (local_err) {

error_propagate(errp, local_err);

goto error_restore_flags;

diff --git a/block/replication.c b/block/replication.c

index XXXXXXX..XXXXXXX 100644

--- a/block/replication.c

+++ b/block/replication.c

@@ -XXX,XX +XXX,XX @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp)

s->replication_state = BLOCK_REPLICATION_FAILOVER;

commit_active_start(NULL, s->active_disk->bs, s->secondary_disk->bs,

BLOCK_JOB_INTERNAL, 0, BLOCKDEV_ON_ERROR_REPORT,

- replication_done, bs, errp, true);

+ NULL, replication_done, bs, errp, true);

break;

default:

aio_context_release(aio_context);

diff --git a/blockdev.c b/blockdev.c

index XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,

bool has_top, const char *top,

bool has_backing_file, const char *backing_file,

bool has_speed, int64_t speed,

+ bool has_filter_node_name, const char *filter_node_name,

Error **errp)

{

BlockDriverState *bs;

@@ -XXX,XX +XXX,XX @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,

if (!has_speed) {

speed = 0;

}

+ if (!has_filter_node_name) {

+ filter_node_name = NULL;

+ }

/* Important Note:

* libvirt relies on the DeviceNotFound error class in order to probe for

@@ -XXX,XX +XXX,XX @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,

goto out;

}

commit_active_start(has_job_id ? job_id : NULL, bs, base_bs,

- BLOCK_JOB_DEFAULT, speed, on_error, NULL, NULL,

- &local_err, false);

+ BLOCK_JOB_DEFAULT, speed, on_error,

+ filter_node_name, NULL, NULL, &local_err, false);

} else {

BlockDriverState *overlay_bs = bdrv_find_overlay(bs, top_bs);

if (bdrv_op_is_blocked(overlay_bs, BLOCK_OP_TYPE_COMMIT_TARGET, errp)) {

@@ -XXX,XX +XXX,XX @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,

}

commit_start(has_job_id ? job_id : NULL, bs, base_bs, top_bs, speed,

on_error, has_backing_file ? backing_file : NULL,

- &local_err);

+ filter_node_name, &local_err);

}

if (local_err != NULL) {

error_propagate(errp, local_err);

diff --git a/include/block/block_int.h b/include/block/block_int.h

index XXXXXXX..XXXXXXX 100644

--- a/include/block/block_int.h

+++ b/include/block/block_int.h

@@ -XXX,XX +XXX,XX @@ void stream_start(const char *job_id, BlockDriverState *bs,

* @speed: The maximum speed, in bytes per second, or 0 for unlimited.

* @on_error: The action to take upon error.

* @backing_file_str: String to use as the backing file in @top's overlay

+ * @filter_node_name: The node name that should be assigned to the filter

+ * driver that the commit job inserts into the graph above @top. NULL means

+ * that a node name should be autogenerated.

* @errp: Error object.

*

*/

void commit_start(const char *job_id, BlockDriverState *bs,

BlockDriverState *base, BlockDriverState *top, int64_t speed,

BlockdevOnError on_error, const char *backing_file_str,

- Error **errp);

+ const char *filter_node_name, Error **errp);

/**

* commit_active_start:

* @job_id: The id of the newly-created job, or %NULL to use the

@@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs,

* See @BlockJobCreateFlags

* @speed: The maximum speed, in bytes per second, or 0 for unlimited.

* @on_error: The action to take upon error.

+ * @filter_node_name: The node name that should be assigned to the filter

+ * driver that the commit job inserts into the graph above @bs. NULL means that

+ * a node name should be autogenerated.

* @cb: Completion function for the job.

* @opaque: Opaque pointer value passed to @cb.

* @errp: Error object.

@@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs,

void commit_active_start(const char *job_id, BlockDriverState *bs,

BlockDriverState *base, int creation_flags,

int64_t speed, BlockdevOnError on_error,

- BlockCompletionFunc *cb,

- void *opaque, Error **errp, bool auto_complete);

+ const char *filter_node_name,

+ BlockCompletionFunc *cb, void *opaque, Error **errp,

+ bool auto_complete);

/*

* mirror_start:

* @job_id: The id of the newly-created job, or %NULL to use the

Reviewed-by: Max Reitz <mreitz@redhat.com>

Acked-by: Fam Zheng <famz@redhat.com>