diff --git a/block.c b/block.c index c139540f2bac..29e931e217fe 100644 --- a/block.c +++ b/block.c @@ -2472,18 +2472,20 @@ void bdrv_root_unref_child(BdrvChild *child) bdrv_unref(child_bs); } -void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child) +/** + * Clear all inherits_from pointers from children and grandchildren of + * @root that point to @root, where necessary. + */ +static void bdrv_unset_inherits_from(BlockDriverState *root, BdrvChild *child) { - if (child == NULL) { - return; - } - - if (child->bs->inherits_from == parent) { - BdrvChild *c; + BdrvChild *c; - /* Remove inherits_from only when the last reference between parent and - * child->bs goes away. */ - QLIST_FOREACH(c, &parent->children, next) { + if (child->bs->inherits_from == root) { + /* + * Remove inherits_from only when the last reference between root and + * child->bs goes away. + */ + QLIST_FOREACH(c, &root->children, next) { if (c != child && c->bs == child->bs) { break; } @@ -2493,6 +2495,18 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child) } } + QLIST_FOREACH(c, &child->bs->children, next) { + bdrv_unset_inherits_from(root, c); + } +} + +void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child) +{ + if (child == NULL) { + return; + } + + bdrv_unset_inherits_from(parent, child); bdrv_root_unref_child(child); } @@ -4416,6 +4430,14 @@ int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, return -EPERM; } + for (i = bs; i != base; i = backing_bs(i)) { + if (i->backing && backing_bs(i)->never_freeze) { + error_setg(errp, "Cannot freeze '%s' link to '%s'", + i->backing->name, backing_bs(i)->node_name); + return -EPERM; + } + } + for (i = bs; i != base; i = backing_bs(i)) { if (i->backing) { i->backing->frozen = true; diff --git a/block/commit.c b/block/commit.c index ca7e408b26f4..2c5a6d4ebca3 100644 --- a/block/commit.c +++ b/block/commit.c @@ -298,6 +298,10 @@ void commit_start(const char *job_id, BlockDriverState *bs, if (!filter_node_name) { commit_top_bs->implicit = true; } + + /* So that we can always drop this node */ + commit_top_bs->never_freeze = true; + commit_top_bs->total_sectors = top->total_sectors; bdrv_append(commit_top_bs, top, &local_err); diff --git a/block/gluster.c b/block/gluster.c index 62f8ff2147e3..f64dc5b01e42 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -931,7 +931,17 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState *state, gconf->has_debug = true; gconf->logfile = g_strdup(s->logfile); gconf->has_logfile = true; - reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp); + + /* + * If 'state->bs->exact_filename' is empty, 'state->options' should contain + * the JSON parameters already parsed. + */ + if (state->bs->exact_filename[0] != '\0') { + reop_s->glfs = qemu_gluster_init(gconf, state->bs->exact_filename, NULL, + errp); + } else { + reop_s->glfs = qemu_gluster_init(gconf, NULL, state->options, errp); + } if (reop_s->glfs == NULL) { ret = -errno; goto exit; diff --git a/block/mirror.c b/block/mirror.c index 2fcec70e3583..8cb75fb409b6 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1551,6 +1551,10 @@ static BlockJob *mirror_start_job( if (!filter_node_name) { mirror_top_bs->implicit = true; } + + /* So that we can always drop this node */ + mirror_top_bs->never_freeze = true; + mirror_top_bs->total_sectors = bs->total_sectors; mirror_top_bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED; mirror_top_bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED | diff --git a/block/nvme.c b/block/nvme.c index 73ed5fa75f2e..9896b7f7c60d 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -613,12 +613,12 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, /* Set up admin queue. */ s->queues = g_new(NVMeQueuePair *, 1); - s->nr_queues = 1; s->queues[0] = nvme_create_queue_pair(bs, 0, NVME_QUEUE_SIZE, errp); if (!s->queues[0]) { ret = -EINVAL; goto out; } + s->nr_queues = 1; QEMU_BUILD_BUG_ON(NVME_QUEUE_SIZE & 0xF000); s->regs->aqa = cpu_to_le32((NVME_QUEUE_SIZE << 16) | NVME_QUEUE_SIZE); s->regs->asq = cpu_to_le64(s->queues[0]->sq.iova); diff --git a/block/stream.c b/block/stream.c index cd5e2ba9b071..6ac1e7bec42c 100644 --- a/block/stream.c +++ b/block/stream.c @@ -78,8 +78,8 @@ static int stream_prepare(Job *job) base_fmt = base->drv->format_name; } } - ret = bdrv_change_backing_file(bs, base_id, base_fmt); bdrv_set_backing_hd(bs, base, &local_err); + ret = bdrv_change_backing_file(bs, base_id, base_fmt); if (local_err) { error_report_err(local_err); return -EPERM; @@ -284,5 +284,5 @@ void stream_start(const char *job_id, BlockDriverState *bs, if (bs_read_only) { bdrv_reopen_set_read_only(bs, true, NULL); } - bdrv_unfreeze_backing_chain(bs, base); + bdrv_unfreeze_backing_chain(bs, bottom); } diff --git a/include/block/block_int.h b/include/block/block_int.h index d6415b53c1c3..50902531b730 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -885,6 +885,9 @@ struct BlockDriverState { /* Only read/written by whoever has set active_flush_req to true. */ unsigned int flushed_gen; /* Flushed write generation */ + + /* BdrvChild links to this node may never be frozen */ + bool never_freeze; }; struct BlockBackendRootState { diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index c6311d182561..1b69f318c640 100755 --- a/tests/qemu-iotests/030 +++ b/tests/qemu-iotests/030 @@ -36,7 +36,9 @@ class TestSingleDrive(iotests.QMPTestCase): qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img) qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 512', backing_img) qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 524288 512', mid_img) - self.vm = iotests.VM().add_drive("blkdebug::" + test_img, "backing.node-name=mid") + self.vm = iotests.VM().add_drive("blkdebug::" + test_img, + "backing.node-name=mid," + + "backing.backing.node-name=base") self.vm.launch() def tearDown(self): @@ -144,17 +146,43 @@ class TestSingleDrive(iotests.QMPTestCase): def test_device_not_found(self): result = self.vm.qmp('block-stream', device='nonexistent') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + 'Cannot find device=nonexistent nor node_name=nonexistent') def test_job_id_missing(self): result = self.vm.qmp('block-stream', device='mid') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', "Invalid job ID ''") + + def test_read_only(self): + # Create a new file that we can attach (we need a read-only top) + with iotests.FilePath('ro-top.img') as ro_top_path: + qemu_img('create', '-f', iotests.imgfmt, ro_top_path, + str(self.image_len)) + + result = self.vm.qmp('blockdev-add', + node_name='ro-top', + driver=iotests.imgfmt, + read_only=True, + file={ + 'driver': 'file', + 'filename': ro_top_path, + 'read-only': True + }, + backing='mid') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('block-stream', job_id='stream', + device='ro-top', base_node='base') + self.assert_qmp(result, 'error/desc', 'Block node is read-only') + + result = self.vm.qmp('blockdev-del', node_name='ro-top') + self.assert_qmp(result, 'return', {}) class TestParallelOps(iotests.QMPTestCase): num_ops = 4 # Number of parallel block-stream operations num_imgs = num_ops * 2 + 1 - image_len = num_ops * 512 * 1024 + image_len = num_ops * 4 * 1024 * 1024 imgs = [] def setUp(self): @@ -176,11 +204,11 @@ class TestParallelOps(iotests.QMPTestCase): # Put data into the images we are copying data from odd_img_indexes = [x for x in reversed(range(self.num_imgs)) if x % 2 == 1] for i in range(len(odd_img_indexes)): - # Alternate between 256KB and 512KB. + # Alternate between 2MB and 4MB. # This way jobs will not finish in the same order they were created - num_kb = 256 + 256 * (i % 2) + num_mb = 2 + 2 * (i % 2) qemu_io('-f', iotests.imgfmt, - '-c', 'write -P 0xFF %dk %dk' % (i * 512, num_kb), + '-c', 'write -P 0xFF %dM %dM' % (i * 4, num_mb), self.imgs[odd_img_indexes[i]]) # Attach the drive to the VM @@ -213,6 +241,10 @@ class TestParallelOps(iotests.QMPTestCase): result = self.vm.qmp('block-stream', device=node_name, job_id=job_id, base=self.imgs[i-2], speed=512*1024) self.assert_qmp(result, 'return', {}) + for job in pending_jobs: + result = self.vm.qmp('block-job-set-speed', device=job, speed=0) + self.assert_qmp(result, 'return', {}) + # Wait for all jobs to be finished. while len(pending_jobs) > 0: for event in self.vm.get_qmp_events(wait=True): @@ -241,24 +273,33 @@ class TestParallelOps(iotests.QMPTestCase): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-stream', device='node5', job_id='stream-node5', base=self.imgs[2]) - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node4' is busy: block device is in use by block job: stream") result = self.vm.qmp('block-stream', device='node3', job_id='stream-node3', base=self.imgs[2]) - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node3' is busy: block device is in use by block job: stream") result = self.vm.qmp('block-stream', device='node4', job_id='stream-node4-v2') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node4' is busy: block device is in use by block job: stream") # block-commit should also fail if it touches nodes used by the stream job result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[4], job_id='commit-node4') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node4' is busy: block device is in use by block job: stream") result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[1], top=self.imgs[3], job_id='commit-node1') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node3' is busy: block device is in use by block job: stream") # This fails because it needs to modify the backing string in node2, which is blocked result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[0], top=self.imgs[1], job_id='commit-node0') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node2' is busy: block device is in use by block job: stream") + + result = self.vm.qmp('block-job-set-speed', device='stream-node4', speed=0) + self.assert_qmp(result, 'return', {}) self.wait_until_completed(drive='stream-node4') self.assert_no_active_block_jobs() @@ -274,20 +315,28 @@ class TestParallelOps(iotests.QMPTestCase): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-stream', device='node3', job_id='stream-node3') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node3' is busy: block device is in use by block job: commit") result = self.vm.qmp('block-stream', device='node6', base=self.imgs[2], job_id='stream-node6') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node5' is busy: block device is in use by block job: commit") result = self.vm.qmp('block-stream', device='node4', base=self.imgs[2], job_id='stream-node4') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node4' is busy: block device is in use by block job: commit") result = self.vm.qmp('block-stream', device='node6', base=self.imgs[4], job_id='stream-node6-v2') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node5' is busy: block device is in use by block job: commit") # This fails because block-commit currently blocks the active layer even if it's not used result = self.vm.qmp('block-stream', device='drive0', base=self.imgs[5], job_id='stream-drive0') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'drive0' is busy: block device is in use by block job: commit") + + result = self.vm.qmp('block-job-set-speed', device='commit-node3', speed=0) + self.assert_qmp(result, 'return', {}) self.wait_until_completed(drive='commit-node3') @@ -302,35 +351,70 @@ class TestParallelOps(iotests.QMPTestCase): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-stream', device='node5', base=self.imgs[3], job_id='stream-node6') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node5' is busy: block device is in use by block job: commit") event = self.vm.event_wait(name='BLOCK_JOB_READY') self.assert_qmp(event, 'data/device', 'commit-drive0') self.assert_qmp(event, 'data/type', 'commit') self.assert_qmp_absent(event, 'data/error') + result = self.vm.qmp('block-job-set-speed', device='commit-drive0', speed=0) + self.assert_qmp(result, 'return', {}) + result = self.vm.qmp('block-job-complete', device='commit-drive0') self.assert_qmp(result, 'return', {}) self.wait_until_completed(drive='commit-drive0') # In this case the base node of the stream job is the same as the - # top node of commit job. Since block-commit removes the top node - # when it finishes, this is not allowed. + # top node of commit job. Since this results in the commit filter + # node being part of the stream chain, this is not allowed. def test_overlapping_4(self): self.assert_no_active_block_jobs() # Commit from node2 into node0 - result = self.vm.qmp('block-commit', device='drive0', top=self.imgs[2], base=self.imgs[0]) + result = self.vm.qmp('block-commit', device='drive0', + top=self.imgs[2], base=self.imgs[0], + filter_node_name='commit-filter', speed=1024*1024) self.assert_qmp(result, 'return', {}) # Stream from node2 into node4 result = self.vm.qmp('block-stream', device='node4', base_node='node2', job_id='node4') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Cannot freeze 'backing' link to 'commit-filter'") + + result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0) + self.assert_qmp(result, 'return', {}) self.wait_until_completed() self.assert_no_active_block_jobs() + # In this case the base node of the stream job is the commit job's + # filter node. stream does not have a real dependency on its base + # node, so even though commit removes it when it is done, there is + # no conflict. + def test_overlapping_5(self): + self.assert_no_active_block_jobs() + + # Commit from node2 into node0 + result = self.vm.qmp('block-commit', device='drive0', + top_node='node2', base_node='node0', + filter_node_name='commit-filter', speed=1024*1024) + self.assert_qmp(result, 'return', {}) + + # Stream from node2 into node4 + result = self.vm.qmp('block-stream', device='node4', + base_node='commit-filter', job_id='node4') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0) + self.assert_qmp(result, 'return', {}) + + self.vm.run_job(job='drive0', auto_dismiss=True, use_log=False) + self.vm.run_job(job='node4', auto_dismiss=True, use_log=False) + self.assert_no_active_block_jobs() + # Test a block-stream and a block-commit job in parallel # Here the stream job is supposed to finish quickly in order to reproduce # the scenario that triggers the bug fixed in 3d5d319e1221 and 1a63a907507 @@ -378,6 +462,10 @@ class TestParallelOps(iotests.QMPTestCase): result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[5], speed=1024*1024) self.assert_qmp(result, 'return', {}) + for job in ['drive0', 'node4']: + result = self.vm.qmp('block-job-set-speed', device=job, speed=0) + self.assert_qmp(result, 'return', {}) + # Wait for all jobs to be finished. pending_jobs = ['node4', 'drive0'] while len(pending_jobs) > 0: @@ -406,19 +494,23 @@ class TestParallelOps(iotests.QMPTestCase): # Error: the base node does not exist result = self.vm.qmp('block-stream', device='node4', base_node='none', job_id='stream') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + 'Cannot find device= nor node_name=none') # Error: the base node is not a backing file of the top node result = self.vm.qmp('block-stream', device='node4', base_node='node6', job_id='stream') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node6' is not a backing image of 'node4'") # Error: the base node is the same as the top node result = self.vm.qmp('block-stream', device='node4', base_node='node4', job_id='stream') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "Node 'node4' is not a backing image of 'node4'") # Error: cannot specify 'base' and 'base-node' at the same time result = self.vm.qmp('block-stream', device='node4', base=self.imgs[2], base_node='node2', job_id='stream') - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + "'base' and 'base-node' cannot be specified at the same time") # Success: the base node is a backing file of the top node result = self.vm.qmp('block-stream', device='node4', base_node='node2', job_id='stream') @@ -851,7 +943,7 @@ class TestSetSpeed(iotests.QMPTestCase): self.assert_no_active_block_jobs() result = self.vm.qmp('block-stream', device='drive0', speed=-1) - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'") self.assert_no_active_block_jobs() @@ -860,7 +952,7 @@ class TestSetSpeed(iotests.QMPTestCase): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1) - self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'") self.cancel_and_wait(resume=True) diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out index 4fd1c2dcd218..6d9bee1a4b3c 100644 --- a/tests/qemu-iotests/030.out +++ b/tests/qemu-iotests/030.out @@ -1,5 +1,5 @@ -......................... +........................... ---------------------------------------------------------------------- -Ran 25 tests +Ran 27 tests OK diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 3ecef5bc9091..ce74177ab164 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -542,7 +542,7 @@ def qmp_log(self, cmd, filters=[], indent=None, **kwargs): # Returns None on success, and an error string on failure def run_job(self, job, auto_finalize=True, auto_dismiss=False, - pre_finalize=None, wait=60.0): + pre_finalize=None, use_log=True, wait=60.0): match_device = {'data': {'device': job}} match_id = {'data': {'id': job}} events = [ @@ -557,7 +557,8 @@ def run_job(self, job, auto_finalize=True, auto_dismiss=False, while True: ev = filter_qmp_event(self.events_wait(events)) if ev['event'] != 'JOB_STATUS_CHANGE': - log(ev) + if use_log: + log(ev) continue status = ev['data']['status'] if status == 'aborting': @@ -565,13 +566,20 @@ def run_job(self, job, auto_finalize=True, auto_dismiss=False, for j in result['return']: if j['id'] == job: error = j['error'] - log('Job failed: %s' % (j['error'])) + if use_log: + log('Job failed: %s' % (j['error'])) elif status == 'pending' and not auto_finalize: if pre_finalize: pre_finalize() - self.qmp_log('job-finalize', id=job) + if use_log: + self.qmp_log('job-finalize', id=job) + else: + self.qmp('job-finalize', id=job) elif status == 'concluded' and not auto_dismiss: - self.qmp_log('job-dismiss', id=job) + if use_log: + self.qmp_log('job-dismiss', id=job) + else: + self.qmp('job-dismiss', id=job) elif status == 'null': return error