From dc71ce45ded4e872e25c2de32d5e7a71842b0985 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 24 Jun 2014 20:26:35 +0800 Subject: [PATCH 01/47] blockjob: Add block_job_yield() This will unset busy flag and put coroutine to sleep, can be used to wait for QMP complete/cancel. Signed-off-by: Fam Zheng Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- blockjob.c | 14 ++++++++++++++ include/block/blockjob.h | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/blockjob.c b/blockjob.c index 4da86cdfcd..a6db01e953 100644 --- a/blockjob.c +++ b/blockjob.c @@ -210,6 +210,20 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns) job->busy = true; } +void block_job_yield(BlockJob *job) +{ + assert(job->busy); + + /* Check cancellation *before* setting busy = false, too! */ + if (block_job_is_cancelled(job)) { + return; + } + + job->busy = false; + qemu_coroutine_yield(); + job->busy = true; +} + BlockJobInfo *block_job_query(BlockJob *job) { BlockJobInfo *info = g_new0(BlockJobInfo, 1); diff --git a/include/block/blockjob.h b/include/block/blockjob.h index e443987ea8..67ca076380 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -146,6 +146,14 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, */ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns); +/** + * block_job_yield: + * @job: The job that calls the function. + * + * Yield the block job coroutine. + */ +void block_job_yield(BlockJob *job); + /** * block_job_completed: * @job: The job being completed. From 9e48b025400b2d284e17860862b0a4aa02c6032d Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 24 Jun 2014 20:26:36 +0800 Subject: [PATCH 02/47] mirror: Go through ready -> complete process for 0 len image When mirroring or active committing a zero length image, BLOCK_JOB_READY is not reported now, instead the job completes because we short circuit the mirror job loop. This is inconsistent with non-zero length images, and only confuses management software. Let's do the same thing when seeing a 0-length image: report ready immediately; wait for block-job-cancel or block-job-complete; clear the cancel flag as existing non-zero image synced case (cancelled after ready); then jump to the exit. Reported-by: Eric Blake Signed-off-by: Fam Zheng Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/mirror.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/block/mirror.c b/block/mirror.c index 301a04de8e..7c9f898089 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -324,9 +324,18 @@ static void coroutine_fn mirror_run(void *opaque) } s->common.len = bdrv_getlength(bs); - if (s->common.len <= 0) { + if (s->common.len < 0) { ret = s->common.len; goto immediate_exit; + } else if (s->common.len == 0) { + /* Report BLOCK_JOB_READY and wait for complete. */ + block_job_event_ready(&s->common); + s->synced = true; + while (!block_job_is_cancelled(&s->common) && !s->should_complete) { + block_job_yield(&s->common); + } + s->common.cancelled = false; + goto immediate_exit; } length = DIV_ROUND_UP(s->common.len, s->granularity); From 8b9a30ca5bc10545637429486836f3c206c39fab Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 24 Jun 2014 20:26:37 +0800 Subject: [PATCH 03/47] qemu-iotests: Test BLOCK_JOB_READY event for 0Kb image active commit There should be a BLOCK_JOB_READY event with active commit, regardless of image length. Let's test the 0 length image case, and make sure it goes through the ready->complete process. Signed-off-by: Fam Zheng Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/040 | 12 +++++++++--- tests/qemu-iotests/040.out | 4 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index 734b6a6bb4..d1668109dd 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -35,12 +35,13 @@ test_img = os.path.join(iotests.test_dir, 'test.img') class ImageCommitTestCase(iotests.QMPTestCase): '''Abstract base class for image commit test cases''' - def run_commit_test(self, top, base): + def run_commit_test(self, top, base, need_ready=False): self.assert_no_active_block_jobs() result = self.vm.qmp('block-commit', device='drive0', top=top, base=base) self.assert_qmp(result, 'return', {}) completed = False + ready = False while not completed: for event in self.vm.get_qmp_events(wait=True): if event['event'] == 'BLOCK_JOB_COMPLETED': @@ -48,8 +49,11 @@ class ImageCommitTestCase(iotests.QMPTestCase): self.assert_qmp(event, 'data/device', 'drive0') self.assert_qmp(event, 'data/offset', self.image_len) self.assert_qmp(event, 'data/len', self.image_len) + if need_ready: + self.assertTrue(ready, "Expecting BLOCK_JOB_COMPLETED event") completed = True elif event['event'] == 'BLOCK_JOB_READY': + ready = True self.assert_qmp(event, 'data/type', 'commit') self.assert_qmp(event, 'data/device', 'drive0') self.assert_qmp(event, 'data/len', self.image_len) @@ -63,7 +67,7 @@ class TestSingleDrive(ImageCommitTestCase): test_len = 1 * 1024 * 256 def setUp(self): - iotests.create_image(backing_img, TestSingleDrive.image_len) + iotests.create_image(backing_img, self.image_len) qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img) qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img) qemu_io('-c', 'write -P 0xab 0 524288', backing_img) @@ -105,7 +109,7 @@ class TestSingleDrive(ImageCommitTestCase): self.assert_qmp(result, 'error/desc', 'Base \'badfile\' not found') def test_top_is_active(self): - self.run_commit_test(test_img, backing_img) + self.run_commit_test(test_img, backing_img, need_ready=True) self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed")) self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed")) @@ -238,6 +242,8 @@ class TestSetSpeed(ImageCommitTestCase): self.cancel_and_wait(resume=True) +class TestActiveZeroLengthImage(TestSingleDrive): + image_len = 0 if __name__ == '__main__': iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out index b6f257674e..42314e9c00 100644 --- a/tests/qemu-iotests/040.out +++ b/tests/qemu-iotests/040.out @@ -1,5 +1,5 @@ -................ +........................ ---------------------------------------------------------------------- -Ran 16 tests +Ran 24 tests OK From 3b9f27d2b34cb8c5cc6cec993712c7e1943e9de9 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 24 Jun 2014 20:26:38 +0800 Subject: [PATCH 04/47] qemu-iotests: Test 0-length image for mirror All behavior and invariant should hold for images with 0 length, so add a class to repeat all the tests in TestSingleDrive. Hide two unapplicable test methods that would fail with 0 image length because it's also used as cluster size. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- tests/qemu-iotests/041 | 11 ++++++++--- tests/qemu-iotests/041.out | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index ec470b2007..ef4f4657cc 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -64,7 +64,7 @@ class TestSingleDrive(ImageMirroringTestCase): image_len = 1 * 1024 * 1024 # MB def setUp(self): - iotests.create_image(backing_img, TestSingleDrive.image_len) + iotests.create_image(backing_img, self.image_len) qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img) self.vm = iotests.VM().add_drive(test_img) self.vm.launch() @@ -163,7 +163,7 @@ class TestSingleDrive(ImageMirroringTestCase): self.assert_no_active_block_jobs() qemu_img('create', '-f', iotests.imgfmt, '-o', 'cluster_size=%d,size=%d' - % (TestSingleDrive.image_len, TestSingleDrive.image_len), target_img) + % (self.image_len, self.image_len), target_img) result = self.vm.qmp('drive-mirror', device='drive0', sync='full', buf_size=65536, mode='existing', target=target_img) self.assert_qmp(result, 'return', {}) @@ -179,7 +179,7 @@ class TestSingleDrive(ImageMirroringTestCase): self.assert_no_active_block_jobs() qemu_img('create', '-f', iotests.imgfmt, '-o', 'cluster_size=%d,backing_file=%s' - % (TestSingleDrive.image_len, backing_img), target_img) + % (self.image_len, backing_img), target_img) result = self.vm.qmp('drive-mirror', device='drive0', sync='full', mode='existing', target=target_img) self.assert_qmp(result, 'return', {}) @@ -206,6 +206,11 @@ class TestSingleDrive(ImageMirroringTestCase): target=target_img) self.assert_qmp(result, 'error/class', 'DeviceNotFound') +class TestSingleDriveZeroLength(TestSingleDrive): + image_len = 0 + test_small_buffer2 = None + test_large_cluster = None + class TestMirrorNoBacking(ImageMirroringTestCase): image_len = 2 * 1024 * 1024 # MB diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out index 6d9bee1a4b..cfa5c0d0e6 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ -........................... +................................... ---------------------------------------------------------------------- -Ran 27 tests +Ran 35 tests OK From 7c24384b3b984f0256ba10eb26d877ec28985019 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 25 Jun 2014 00:06:00 +0200 Subject: [PATCH 05/47] block/nfs: fix url parameter checking this patch fixes the incorrect usage of strncmp and adds simple error checking by means of parse_uint_full instead of atoi for the supplied URL parameters. Signed-off-by: Peter Lieven Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/nfs.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/block/nfs.c b/block/nfs.c index ec43201817..0b44483871 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -304,17 +304,23 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename, qp = query_params_parse(uri->query); for (i = 0; i < qp->n; i++) { + unsigned long long val; if (!qp->p[i].value) { error_setg(errp, "Value for NFS parameter expected: %s", qp->p[i].name); goto fail; } - if (!strncmp(qp->p[i].name, "uid", 3)) { - nfs_set_uid(client->context, atoi(qp->p[i].value)); - } else if (!strncmp(qp->p[i].name, "gid", 3)) { - nfs_set_gid(client->context, atoi(qp->p[i].value)); - } else if (!strncmp(qp->p[i].name, "tcp-syncnt", 10)) { - nfs_set_tcp_syncnt(client->context, atoi(qp->p[i].value)); + if (parse_uint_full(qp->p[i].value, &val, 0)) { + error_setg(errp, "Illegal value for NFS parameter: %s", + qp->p[i].name); + goto fail; + } + if (!strcmp(qp->p[i].name, "uid")) { + nfs_set_uid(client->context, val); + } else if (!strcmp(qp->p[i].name, "gid")) { + nfs_set_gid(client->context, val); + } else if (!strcmp(qp->p[i].name, "tcp-syncnt")) { + nfs_set_tcp_syncnt(client->context, val); } else { error_setg(errp, "Unknown NFS parameter name: %s", qp->p[i].name); From f42ca3cad10f74777481c201722b598763e1771c Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 25 Jun 2014 00:06:01 +0200 Subject: [PATCH 06/47] block/nfs: add knob to set readahead upcoming libnfs will feature internal readahead support. Add a knob to pass the optional readahead value as a URL parameter. Signed-off-by: Peter Lieven Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/nfs.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/block/nfs.c b/block/nfs.c index 0b44483871..8439e0d389 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -321,6 +321,10 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename, nfs_set_gid(client->context, val); } else if (!strcmp(qp->p[i].name, "tcp-syncnt")) { nfs_set_tcp_syncnt(client->context, val); +#ifdef LIBNFS_FEATURE_READAHEAD + } else if (!strcmp(qp->p[i].name, "readahead")) { + nfs_set_readahead(client->context, val); +#endif } else { error_setg(errp, "Unknown NFS parameter name: %s", qp->p[i].name); From f54120ff1aa030d96ee64aefef9898e9e80b3a71 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 26 May 2014 11:09:59 +0200 Subject: [PATCH 07/47] block: Create bdrv_fill_options() The idea of bdrv_fill_options() is to convert every parameter for opening images, in particular the filename and flags, to entries in the options QDict. This patch starts with moving the filename parsing and driver probing part from bdrv_file_open() to the new function. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 124 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 75 insertions(+), 49 deletions(-) diff --git a/block.c b/block.c index 217f523dd0..757d139a1f 100644 --- a/block.c +++ b/block.c @@ -1005,6 +1005,72 @@ free_and_fail: return ret; } +/* + * Fills in default options for opening images and converts the legacy + * filename/flags pair to option QDict entries. + */ +static int bdrv_fill_options(QDict **options, const char *filename, + Error **errp) +{ + const char *drvname; + bool parse_filename = false; + Error *local_err = NULL; + BlockDriver *drv; + + /* Fetch the file name from the options QDict if necessary */ + if (filename) { + if (!qdict_haskey(*options, "filename")) { + qdict_put(*options, "filename", qstring_from_str(filename)); + parse_filename = true; + } else { + error_setg(errp, "Can't specify 'file' and 'filename' options at " + "the same time"); + return -EINVAL; + } + } + + /* Find the right block driver */ + filename = qdict_get_try_str(*options, "filename"); + drvname = qdict_get_try_str(*options, "driver"); + + if (!drvname) { + if (filename) { + drv = bdrv_find_protocol(filename, parse_filename); + if (!drv) { + error_setg(errp, "Unknown protocol"); + return -EINVAL; + } + + drvname = drv->format_name; + qdict_put(*options, "driver", qstring_from_str(drvname)); + } else { + error_setg(errp, "Must specify either driver or file"); + return -EINVAL; + } + } + + drv = bdrv_find_format(drvname); + if (!drv) { + error_setg(errp, "Unknown driver '%s'", drvname); + return -ENOENT; + } + + /* Driver-specific filename parsing */ + if (drv->bdrv_parse_filename && parse_filename) { + drv->bdrv_parse_filename(filename, *options, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return -EINVAL; + } + + if (!drv->bdrv_needs_filename) { + qdict_del(*options, "filename"); + } + } + + return 0; +} + /* * Opens a file using a protocol (file, host_device, nbd, ...) * @@ -1020,63 +1086,23 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename, { BlockDriver *drv; const char *drvname; - bool parse_filename = false; Error *local_err = NULL; int ret; - /* Fetch the file name from the options QDict if necessary */ - if (!filename) { - filename = qdict_get_try_str(*options, "filename"); - } else if (filename && !qdict_haskey(*options, "filename")) { - qdict_put(*options, "filename", qstring_from_str(filename)); - parse_filename = true; - } else { - error_setg(errp, "Can't specify 'file' and 'filename' options at the " - "same time"); - ret = -EINVAL; + ret = bdrv_fill_options(options, filename, &local_err); + if (local_err) { + error_propagate(errp, local_err); goto fail; } - /* Find the right block driver */ - drvname = qdict_get_try_str(*options, "driver"); - if (drvname) { - drv = bdrv_find_format(drvname); - if (!drv) { - error_setg(errp, "Unknown driver '%s'", drvname); - } - qdict_del(*options, "driver"); - } else if (filename) { - drv = bdrv_find_protocol(filename, parse_filename); - if (!drv) { - error_setg(errp, "Unknown protocol"); - } - } else { - error_setg(errp, "Must specify either driver or file"); - drv = NULL; - } + filename = qdict_get_try_str(*options, "filename"); + drvname = qdict_get_str(*options, "driver"); - if (!drv) { - /* errp has been set already */ - ret = -ENOENT; - goto fail; - } - - /* Parse the filename and open it */ - if (drv->bdrv_parse_filename && parse_filename) { - drv->bdrv_parse_filename(filename, *options, &local_err); - if (local_err) { - error_propagate(errp, local_err); - ret = -EINVAL; - goto fail; - } - - if (!drv->bdrv_needs_filename) { - qdict_del(*options, "filename"); - } else { - filename = qdict_get_str(*options, "filename"); - } - } + drv = bdrv_find_format(drvname); + assert(drv); + qdict_del(*options, "driver"); + /* Open the file */ if (!drv->bdrv_file_open) { ret = bdrv_open(&bs, filename, NULL, *options, flags, drv, &local_err); *options = NULL; From 462f5bcf69d3cae4a93ad55bee59516c15236a55 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 26 May 2014 11:39:55 +0200 Subject: [PATCH 08/47] block: Move bdrv_fill_options() call to bdrv_open() bs->options now contains the modified version of the options. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/block.c b/block.c index 757d139a1f..0da39e6811 100644 --- a/block.c +++ b/block.c @@ -1009,14 +1009,19 @@ free_and_fail: * Fills in default options for opening images and converts the legacy * filename/flags pair to option QDict entries. */ -static int bdrv_fill_options(QDict **options, const char *filename, +static int bdrv_fill_options(QDict **options, const char *filename, int flags, Error **errp) { const char *drvname; + bool protocol = flags & BDRV_O_PROTOCOL; bool parse_filename = false; Error *local_err = NULL; BlockDriver *drv; + if (!protocol) { + return 0; + } + /* Fetch the file name from the options QDict if necessary */ if (filename) { if (!qdict_haskey(*options, "filename")) { @@ -1081,20 +1086,15 @@ static int bdrv_fill_options(QDict **options, const char *filename, * returns. Then, the caller is responsible for freeing it. If it intends to * reuse the QDict, QINCREF() should be called beforehand. */ -static int bdrv_file_open(BlockDriverState *bs, const char *filename, - QDict **options, int flags, Error **errp) +static int bdrv_file_open(BlockDriverState *bs, QDict **options, int flags, + Error **errp) { BlockDriver *drv; + const char *filename; const char *drvname; Error *local_err = NULL; int ret; - ret = bdrv_fill_options(options, filename, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto fail; - } - filename = qdict_get_try_str(*options, "filename"); drvname = qdict_get_str(*options, "driver"); @@ -1436,12 +1436,17 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, filename = NULL; } + ret = bdrv_fill_options(&options, filename, flags, &local_err); + if (local_err) { + goto fail; + } + bs->options = options; options = qdict_clone_shallow(options); if (flags & BDRV_O_PROTOCOL) { assert(!drv); - ret = bdrv_file_open(bs, filename, &options, flags & ~BDRV_O_PROTOCOL, + ret = bdrv_file_open(bs, &options, flags & ~BDRV_O_PROTOCOL, &local_err); if (!ret) { drv = bs->drv; From 5e5c4f63f4a7f48f571ea5671bf8452fe9655cdd Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 26 May 2014 11:45:08 +0200 Subject: [PATCH 09/47] block: Move json: parsing to bdrv_fill_options() Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 88 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/block.c b/block.c index 0da39e6811..0d97fde366 100644 --- a/block.c +++ b/block.c @@ -1005,19 +1005,62 @@ free_and_fail: return ret; } +static QDict *parse_json_filename(const char *filename, Error **errp) +{ + QObject *options_obj; + QDict *options; + int ret; + + ret = strstart(filename, "json:", &filename); + assert(ret); + + options_obj = qobject_from_json(filename); + if (!options_obj) { + error_setg(errp, "Could not parse the JSON options"); + return NULL; + } + + if (qobject_type(options_obj) != QTYPE_QDICT) { + qobject_decref(options_obj); + error_setg(errp, "Invalid JSON object given"); + return NULL; + } + + options = qobject_to_qdict(options_obj); + qdict_flatten(options); + + return options; +} + /* * Fills in default options for opening images and converts the legacy * filename/flags pair to option QDict entries. */ -static int bdrv_fill_options(QDict **options, const char *filename, int flags, +static int bdrv_fill_options(QDict **options, const char **pfilename, int flags, Error **errp) { + const char *filename = *pfilename; const char *drvname; bool protocol = flags & BDRV_O_PROTOCOL; bool parse_filename = false; Error *local_err = NULL; BlockDriver *drv; + /* Parse json: pseudo-protocol */ + if (filename && g_str_has_prefix(filename, "json:")) { + QDict *json_options = parse_json_filename(filename, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return -EINVAL; + } + + /* Options given in the filename have lower priority than options + * specified directly */ + qdict_join(*options, json_options, false); + QDECREF(json_options); + *pfilename = filename = NULL; + } + if (!protocol) { return 0; } @@ -1332,33 +1375,6 @@ out: g_free(tmp_filename); } -static QDict *parse_json_filename(const char *filename, Error **errp) -{ - QObject *options_obj; - QDict *options; - int ret; - - ret = strstart(filename, "json:", &filename); - assert(ret); - - options_obj = qobject_from_json(filename); - if (!options_obj) { - error_setg(errp, "Could not parse the JSON options"); - return NULL; - } - - if (qobject_type(options_obj) != QTYPE_QDICT) { - qobject_decref(options_obj); - error_setg(errp, "Invalid JSON object given"); - return NULL; - } - - options = qobject_to_qdict(options_obj); - qdict_flatten(options); - - return options; -} - /* * Opens a disk image (raw, qcow2, vmdk, ...) * @@ -1422,21 +1438,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, options = qdict_new(); } - if (filename && g_str_has_prefix(filename, "json:")) { - QDict *json_options = parse_json_filename(filename, &local_err); - if (local_err) { - ret = -EINVAL; - goto fail; - } - - /* Options given in the filename have lower priority than options - * specified directly */ - qdict_join(options, json_options, false); - QDECREF(json_options); - filename = NULL; - } - - ret = bdrv_fill_options(&options, filename, flags, &local_err); + ret = bdrv_fill_options(&options, &filename, flags, &local_err); if (local_err) { goto fail; } From 17b005f1d422d4581f8ce95b75d603deb081f4f3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 27 May 2014 10:50:29 +0200 Subject: [PATCH 10/47] block: Always pass driver name through options QDict The "driver" entry in the options QDict is now only missing if we're opening an image with format probing. We also catch cases now where both the drv argument and a "driver" option is specified, e.g. by specifying -drive format=qcow2,driver=raw Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 76 ++++++++++++++++++++------------------ tests/qemu-iotests/051 | 1 + tests/qemu-iotests/051.out | 5 ++- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/block.c b/block.c index 0d97fde366..0aa444e6ba 100644 --- a/block.c +++ b/block.c @@ -1037,14 +1037,13 @@ static QDict *parse_json_filename(const char *filename, Error **errp) * filename/flags pair to option QDict entries. */ static int bdrv_fill_options(QDict **options, const char **pfilename, int flags, - Error **errp) + BlockDriver *drv, Error **errp) { const char *filename = *pfilename; const char *drvname; bool protocol = flags & BDRV_O_PROTOCOL; bool parse_filename = false; Error *local_err = NULL; - BlockDriver *drv; /* Parse json: pseudo-protocol */ if (filename && g_str_has_prefix(filename, "json:")) { @@ -1061,12 +1060,8 @@ static int bdrv_fill_options(QDict **options, const char **pfilename, int flags, *pfilename = filename = NULL; } - if (!protocol) { - return 0; - } - /* Fetch the file name from the options QDict if necessary */ - if (filename) { + if (protocol && filename) { if (!qdict_haskey(*options, "filename")) { qdict_put(*options, "filename", qstring_from_str(filename)); parse_filename = true; @@ -1081,30 +1076,41 @@ static int bdrv_fill_options(QDict **options, const char **pfilename, int flags, filename = qdict_get_try_str(*options, "filename"); drvname = qdict_get_try_str(*options, "driver"); - if (!drvname) { - if (filename) { - drv = bdrv_find_protocol(filename, parse_filename); - if (!drv) { - error_setg(errp, "Unknown protocol"); + if (drv) { + if (drvname) { + error_setg(errp, "Driver specified twice"); + return -EINVAL; + } + drvname = drv->format_name; + qdict_put(*options, "driver", qstring_from_str(drvname)); + } else { + if (!drvname && protocol) { + if (filename) { + drv = bdrv_find_protocol(filename, parse_filename); + if (!drv) { + error_setg(errp, "Unknown protocol"); + return -EINVAL; + } + + drvname = drv->format_name; + qdict_put(*options, "driver", qstring_from_str(drvname)); + } else { + error_setg(errp, "Must specify either driver or file"); return -EINVAL; } - - drvname = drv->format_name; - qdict_put(*options, "driver", qstring_from_str(drvname)); - } else { - error_setg(errp, "Must specify either driver or file"); - return -EINVAL; + } else if (drvname) { + drv = bdrv_find_format(drvname); + if (!drv) { + error_setg(errp, "Unknown driver '%s'", drvname); + return -ENOENT; + } } } - drv = bdrv_find_format(drvname); - if (!drv) { - error_setg(errp, "Unknown driver '%s'", drvname); - return -ENOENT; - } + assert(drv || !protocol); /* Driver-specific filename parsing */ - if (drv->bdrv_parse_filename && parse_filename) { + if (drv && drv->bdrv_parse_filename && parse_filename) { drv->bdrv_parse_filename(filename, *options, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -1438,7 +1444,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, options = qdict_new(); } - ret = bdrv_fill_options(&options, &filename, flags, &local_err); + ret = bdrv_fill_options(&options, &filename, flags, drv, &local_err); if (local_err) { goto fail; } @@ -1478,7 +1484,10 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } /* Find the right image format driver */ + drv = NULL; drvname = qdict_get_try_str(options, "driver"); + assert(drvname || !(flags & BDRV_O_PROTOCOL)); + if (drvname) { drv = bdrv_find_format(drvname); qdict_del(options, "driver"); @@ -1487,19 +1496,14 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, ret = -EINVAL; goto fail; } - } - - if (!drv) { - if (file) { - ret = find_image_format(file, filename, &drv, &local_err); - } else { - error_setg(errp, "Must specify either driver or file"); - ret = -EINVAL; + } else if (file) { + ret = find_image_format(file, filename, &drv, &local_err); + if (ret < 0) { goto fail; } - } - - if (!drv) { + } else { + error_setg(errp, "Must specify either driver or file"); + ret = -EINVAL; goto fail; } diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index c4af131a66..30a712f847 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -92,6 +92,7 @@ echo run_qemu -drive file="$TEST_IMG",format=foo run_qemu -drive file="$TEST_IMG",driver=foo +run_qemu -drive file="$TEST_IMG",driver=raw,format=qcow2 echo echo === Overriding backing file === diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 31e329e893..94c7107250 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -38,7 +38,10 @@ Testing: -drive file=TEST_DIR/t.qcow2,format=foo QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=foo: 'foo' invalid format Testing: -drive file=TEST_DIR/t.qcow2,driver=foo -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=foo: could not open disk image TEST_DIR/t.qcow2: Invalid driver: 'foo' +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=foo: could not open disk image TEST_DIR/t.qcow2: Unknown driver 'foo' + +Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2 +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: could not open disk image TEST_DIR/t.qcow2: Driver specified twice === Overriding backing file === From f4788adcb4903e20dadc5455d3d69d0a481f35e4 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 3 Jun 2014 16:44:19 +0200 Subject: [PATCH 11/47] block: Use common driver selection code for bdrv_open_file() This moves the bdrv_open_file() call a bit down so that it can use the bdrv_open() code that selects the right block driver. The code between the old and the new call site is either common code (the error message for an unknown driver has been unified now) or doesn't run with cleared BDRV_O_PROTOCOL (added an if block in one place, whereas the right path was already asserted in another place) Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Benoit Canet --- block.c | 67 ++++++++++++++++++++++++++------------------------------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/block.c b/block.c index 0aa444e6ba..97b4ddc776 100644 --- a/block.c +++ b/block.c @@ -1135,21 +1135,14 @@ static int bdrv_fill_options(QDict **options, const char **pfilename, int flags, * returns. Then, the caller is responsible for freeing it. If it intends to * reuse the QDict, QINCREF() should be called beforehand. */ -static int bdrv_file_open(BlockDriverState *bs, QDict **options, int flags, - Error **errp) +static int bdrv_file_open(BlockDriverState *bs, BlockDriver *drv, + QDict **options, int flags, Error **errp) { - BlockDriver *drv; const char *filename; - const char *drvname; Error *local_err = NULL; int ret; filename = qdict_get_try_str(*options, "filename"); - drvname = qdict_get_str(*options, "driver"); - - drv = bdrv_find_format(drvname); - assert(drv); - qdict_del(*options, "driver"); /* Open the file */ if (!drv->bdrv_file_open) { @@ -1452,37 +1445,25 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, bs->options = options; options = qdict_clone_shallow(options); - if (flags & BDRV_O_PROTOCOL) { - assert(!drv); - ret = bdrv_file_open(bs, &options, flags & ~BDRV_O_PROTOCOL, - &local_err); - if (!ret) { - drv = bs->drv; - goto done; - } else if (bs->drv) { - goto close_and_fail; - } else { + /* Open image file without format layer */ + if ((flags & BDRV_O_PROTOCOL) == 0) { + if (flags & BDRV_O_RDWR) { + flags |= BDRV_O_ALLOW_RDWR; + } + if (flags & BDRV_O_SNAPSHOT) { + snapshot_flags = bdrv_temp_snapshot_flags(flags); + flags = bdrv_backing_flags(flags); + } + + assert(file == NULL); + ret = bdrv_open_image(&file, filename, options, "file", + bdrv_inherited_flags(flags), + true, &local_err); + if (ret < 0) { goto fail; } } - /* Open image file without format layer */ - if (flags & BDRV_O_RDWR) { - flags |= BDRV_O_ALLOW_RDWR; - } - if (flags & BDRV_O_SNAPSHOT) { - snapshot_flags = bdrv_temp_snapshot_flags(flags); - flags = bdrv_backing_flags(flags); - } - - assert(file == NULL); - ret = bdrv_open_image(&file, filename, options, "file", - bdrv_inherited_flags(flags), - true, &local_err); - if (ret < 0) { - goto fail; - } - /* Find the right image format driver */ drv = NULL; drvname = qdict_get_try_str(options, "driver"); @@ -1492,7 +1473,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, drv = bdrv_find_format(drvname); qdict_del(options, "driver"); if (!drv) { - error_setg(errp, "Invalid driver: '%s'", drvname); + error_setg(errp, "Unknown driver: '%s'", drvname); ret = -EINVAL; goto fail; } @@ -1508,6 +1489,18 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } /* Open the image */ + if (flags & BDRV_O_PROTOCOL) { + ret = bdrv_file_open(bs, drv, &options, flags & ~BDRV_O_PROTOCOL, + &local_err); + if (!ret) { + goto done; + } else if (bs->drv) { + goto close_and_fail; + } else { + goto fail; + } + } + ret = bdrv_open_common(bs, file, options, flags, drv, &local_err); if (ret < 0) { goto fail; From b348f3311c1c54ab4abfd7958176ce5ec6407543 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 4 Jun 2014 12:03:59 +0200 Subject: [PATCH 12/47] block: Inline bdrv_file_open() It doesn't do much any more, we can move the code to bdrv_open() now. Signed-off-by: Kevin Wolf Reviewed-by: Benoit Canet Reviewed-by: Eric Blake --- block.c | 51 +++++++++++---------------------------------------- 1 file changed, 11 insertions(+), 40 deletions(-) diff --git a/block.c b/block.c index 97b4ddc776..dfe2c7598a 100644 --- a/block.c +++ b/block.c @@ -1125,44 +1125,6 @@ static int bdrv_fill_options(QDict **options, const char **pfilename, int flags, return 0; } -/* - * Opens a file using a protocol (file, host_device, nbd, ...) - * - * options is an indirect pointer to a QDict of options to pass to the block - * drivers, or pointer to NULL for an empty set of options. If this function - * takes ownership of the QDict reference, it will set *options to NULL; - * otherwise, it will contain unused/unrecognized options after this function - * returns. Then, the caller is responsible for freeing it. If it intends to - * reuse the QDict, QINCREF() should be called beforehand. - */ -static int bdrv_file_open(BlockDriverState *bs, BlockDriver *drv, - QDict **options, int flags, Error **errp) -{ - const char *filename; - Error *local_err = NULL; - int ret; - - filename = qdict_get_try_str(*options, "filename"); - - /* Open the file */ - if (!drv->bdrv_file_open) { - ret = bdrv_open(&bs, filename, NULL, *options, flags, drv, &local_err); - *options = NULL; - } else { - ret = bdrv_open_common(bs, NULL, *options, flags, drv, &local_err); - } - if (ret < 0) { - error_propagate(errp, local_err); - goto fail; - } - - bs->growable = 1; - return 0; - -fail: - return ret; -} - void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) { @@ -1490,9 +1452,18 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, /* Open the image */ if (flags & BDRV_O_PROTOCOL) { - ret = bdrv_file_open(bs, drv, &options, flags & ~BDRV_O_PROTOCOL, - &local_err); + if (!drv->bdrv_file_open) { + const char *filename; + filename = qdict_get_try_str(options, "filename"); + ret = bdrv_open(&bs, filename, NULL, options, + flags & ~BDRV_O_PROTOCOL, drv, &local_err); + options = NULL; + } else { + ret = bdrv_open_common(bs, NULL, options, + flags & ~BDRV_O_PROTOCOL, drv, &local_err); + } if (!ret) { + bs->growable = 1; goto done; } else if (bs->drv) { goto close_and_fail; From 76c591b013782217cad67b35c74cd249e0413439 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 4 Jun 2014 14:19:44 +0200 Subject: [PATCH 13/47] block: Remove second bdrv_open() recursion This recursion was introduced in commit 505d7583 in order to allow nesting image formats. It only ever takes effect when the user explicitly specifies a driver name and that driver isn't suitable for the protocol level. We can check this earlier in bdrv_open() and if the explicitly requested driver is a format driver, clear BDRV_O_PROTOCOL so that another bs->file layer is opened. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/block.c b/block.c index dfe2c7598a..0978e8cd59 100644 --- a/block.c +++ b/block.c @@ -1404,6 +1404,26 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, goto fail; } + /* Find the right image format driver */ + drv = NULL; + drvname = qdict_get_try_str(options, "driver"); + if (drvname) { + drv = bdrv_find_format(drvname); + qdict_del(options, "driver"); + if (!drv) { + error_setg(errp, "Unknown driver: '%s'", drvname); + ret = -EINVAL; + goto fail; + } + } + + assert(drvname || !(flags & BDRV_O_PROTOCOL)); + if (drv && !drv->bdrv_file_open) { + /* If the user explicitly wants a format driver here, we'll need to add + * another layer for the protocol in bs->file */ + flags &= ~BDRV_O_PROTOCOL; + } + bs->options = options; options = qdict_clone_shallow(options); @@ -1426,25 +1446,13 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } } - /* Find the right image format driver */ - drv = NULL; - drvname = qdict_get_try_str(options, "driver"); - assert(drvname || !(flags & BDRV_O_PROTOCOL)); - - if (drvname) { - drv = bdrv_find_format(drvname); - qdict_del(options, "driver"); - if (!drv) { - error_setg(errp, "Unknown driver: '%s'", drvname); - ret = -EINVAL; - goto fail; - } - } else if (file) { + /* Image format probing */ + if (!drv && file) { ret = find_image_format(file, filename, &drv, &local_err); if (ret < 0) { goto fail; } - } else { + } else if (!drv) { error_setg(errp, "Must specify either driver or file"); ret = -EINVAL; goto fail; @@ -1452,16 +1460,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, /* Open the image */ if (flags & BDRV_O_PROTOCOL) { - if (!drv->bdrv_file_open) { - const char *filename; - filename = qdict_get_try_str(options, "filename"); - ret = bdrv_open(&bs, filename, NULL, options, - flags & ~BDRV_O_PROTOCOL, drv, &local_err); - options = NULL; - } else { - ret = bdrv_open_common(bs, NULL, options, - flags & ~BDRV_O_PROTOCOL, drv, &local_err); - } + ret = bdrv_open_common(bs, NULL, options, + flags & ~BDRV_O_PROTOCOL, drv, &local_err); if (!ret) { bs->growable = 1; goto done; From 8ee79e707a005c9274df7ce34265bb7d008b8cef Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 4 Jun 2014 15:09:35 +0200 Subject: [PATCH 14/47] block: Catch backing files assigned to non-COW drivers Since we parse backing.* options to add a backing file from the command line when the driver didn't assign one, it has been possible to have a backing file for e.g. raw images (it just was never accessed). This is obvious nonsense and should be rejected. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 7 +++++++ block/cow.c | 1 + block/qcow.c | 1 + block/qcow2.c | 1 + block/qed.c | 1 + block/vmdk.c | 1 + include/block/block_int.h | 3 +++ tests/qemu-iotests/051 | 5 +++++ tests/qemu-iotests/051.out | 9 +++++++++ 9 files changed, 29 insertions(+) diff --git a/block.c b/block.c index 0978e8cd59..2c3c7608ee 100644 --- a/block.c +++ b/block.c @@ -1192,6 +1192,13 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) bdrv_get_full_backing_filename(bs, backing_filename, PATH_MAX); } + if (!bs->drv || !bs->drv->supports_backing) { + ret = -EINVAL; + error_setg(errp, "Driver doesn't support backing files"); + QDECREF(options); + goto free_exit; + } + backing_hd = bdrv_new("", errp); if (bs->backing_format[0] != '\0') { diff --git a/block/cow.c b/block/cow.c index a05a92cada..8f81ee6d56 100644 --- a/block/cow.c +++ b/block/cow.c @@ -414,6 +414,7 @@ static BlockDriver bdrv_cow = { .bdrv_close = cow_close, .bdrv_create = cow_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, + .supports_backing = true, .bdrv_read = cow_co_read, .bdrv_write = cow_co_write, diff --git a/block/qcow.c b/block/qcow.c index 1f2bac8a5f..a874056cf3 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -941,6 +941,7 @@ static BlockDriver bdrv_qcow = { .bdrv_reopen_prepare = qcow_reopen_prepare, .bdrv_create = qcow_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, + .supports_backing = true, .bdrv_co_readv = qcow_co_readv, .bdrv_co_writev = qcow_co_writev, diff --git a/block/qcow2.c b/block/qcow2.c index b9d2fa6632..67e55c9667 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2418,6 +2418,7 @@ static BlockDriver bdrv_qcow2 = { .bdrv_save_vmstate = qcow2_save_vmstate, .bdrv_load_vmstate = qcow2_load_vmstate, + .supports_backing = true, .bdrv_change_backing_file = qcow2_change_backing_file, .bdrv_refresh_limits = qcow2_refresh_limits, diff --git a/block/qed.c b/block/qed.c index 092e6fb1d2..eddae929eb 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1652,6 +1652,7 @@ static BlockDriver bdrv_qed = { .format_name = "qed", .instance_size = sizeof(BDRVQEDState), .create_opts = &qed_create_opts, + .supports_backing = true, .bdrv_probe = bdrv_qed_probe, .bdrv_rebind = bdrv_qed_rebind, diff --git a/block/vmdk.c b/block/vmdk.c index 83dd6fe4fb..d0de0193fc 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -2180,6 +2180,7 @@ static BlockDriver bdrv_vmdk = { .bdrv_detach_aio_context = vmdk_detach_aio_context, .bdrv_attach_aio_context = vmdk_attach_aio_context, + .supports_backing = true, .create_opts = &vmdk_create_opts, }; diff --git a/include/block/block_int.h b/include/block/block_int.h index 715c761fad..135c5dc0e9 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -100,6 +100,9 @@ struct BlockDriver { */ bool bdrv_needs_filename; + /* Set if a driver can support backing files */ + bool supports_backing; + /* For handling image reopen for split or non-split files */ int (*bdrv_reopen_prepare)(BDRVReopenState *reopen_state, BlockReopenQueue *queue, Error **errp); diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index 30a712f847..a41334e022 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -100,6 +100,11 @@ echo echo "info block" | run_qemu -drive file="$TEST_IMG",driver=qcow2,backing.file.filename="$TEST_IMG.orig" -nodefaults +# Drivers that don't support backing files +run_qemu -drive file="$TEST_IMG",driver=raw,backing.file.filename="$TEST_IMG.orig" +run_qemu -drive file="$TEST_IMG",file.backing.driver=file,file.backing.filename="$TEST_IMG.orig" +run_qemu -drive file="$TEST_IMG",file.backing.driver=qcow2,file.backing.file.filename="$TEST_IMG.orig" + echo echo === Enable and disable lazy refcounting on the command line, plus some invalid values === echo diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 94c7107250..d7b0f503af 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -53,6 +53,15 @@ ide0-hd0: TEST_DIR/t.qcow2 (qcow2) Backing file: TEST_DIR/t.qcow2.orig (chain depth: 1) (qemu) qququiquit +Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,backing.file.filename=TEST_DIR/t.qcow2.orig +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,backing.file.filename=TEST_DIR/t.qcow2.orig: could not open disk image TEST_DIR/t.qcow2: Driver doesn't support backing files + +Testing: -drive file=TEST_DIR/t.qcow2,file.backing.driver=file,file.backing.filename=TEST_DIR/t.qcow2.orig +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.backing.driver=file,file.backing.filename=TEST_DIR/t.qcow2.orig: could not open disk image TEST_DIR/t.qcow2: Driver doesn't support backing files + +Testing: -drive file=TEST_DIR/t.qcow2,file.backing.driver=qcow2,file.backing.file.filename=TEST_DIR/t.qcow2.orig +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.backing.driver=qcow2,file.backing.file.filename=TEST_DIR/t.qcow2.orig: could not open disk image TEST_DIR/t.qcow2: Driver doesn't support backing files + === Enable and disable lazy refcounting on the command line, plus some invalid values === From 20cca275c6190ca0027cf7dd369ba985e44a6537 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 4 Jun 2014 14:33:27 +0200 Subject: [PATCH 15/47] block: Remove a special case for protocols The only semantic change is that bs->open_flags gets BDRV_O_PROTOCOL set now. This isn't useful, but it doesn't hurt either. The code that was previously skipped by 'goto done' is automatically disabled because protocol drivers don't support backing files (and if they did, this would probably be a fix) and can't have snapshot_flags set. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/block.c b/block.c index 2c3c7608ee..7d69f312b6 100644 --- a/block.c +++ b/block.c @@ -831,7 +831,7 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags) * Clear flags that are internal to the block layer before opening the * image. */ - open_flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING); + open_flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_PROTOCOL); /* * Snapshots should be writable. @@ -928,6 +928,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, bs->zero_beyond_eof = true; open_flags = bdrv_open_flags(bs, flags); bs->read_only = !(open_flags & BDRV_O_RDWR); + bs->growable = !!(flags & BDRV_O_PROTOCOL); if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) { error_setg(errp, @@ -1466,19 +1467,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } /* Open the image */ - if (flags & BDRV_O_PROTOCOL) { - ret = bdrv_open_common(bs, NULL, options, - flags & ~BDRV_O_PROTOCOL, drv, &local_err); - if (!ret) { - bs->growable = 1; - goto done; - } else if (bs->drv) { - goto close_and_fail; - } else { - goto fail; - } - } - ret = bdrv_open_common(bs, file, options, flags, drv, &local_err); if (ret < 0) { goto fail; @@ -1510,8 +1498,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } } - -done: /* Check if any unknown options were used */ if (options && (qdict_size(options) != 0)) { const QDictEntry *entry = qdict_first(options); From a760715095e9cda6eb97486c040aa35f82297945 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 25 Jun 2014 10:38:41 +0200 Subject: [PATCH 16/47] qemu_opts_append: Play nicely with QemuOptsList's head When running a libvirt test suite I've noticed the qemu-img is crashing occasionally. Tracing the problem down led me to the following valgrind output: qemu.git $ valgrind -q ./qemu-img create -f qed -obacking_file=/dev/null,backing_fmt=raw qed ==14881== Invalid write of size 8 ==14881== at 0x1D263F: qemu_opts_create (qemu-option.c:692) ==14881== by 0x130782: bdrv_img_create (block.c:5531) ==14881== by 0x118DE0: img_create (qemu-img.c:462) ==14881== by 0x11E7E4: main (qemu-img.c:2830) ==14881== Address 0x11fedd38 is 24 bytes inside a block of size 232 free'd ==14881== at 0x4C2CA5E: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==14881== by 0x592D35E: g_realloc (in /usr/lib64/libglib-2.0.so.0.3800.2) ==14881== by 0x1D38D8: qemu_opts_append (qemu-option.c:1129) ==14881== by 0x13075E: bdrv_img_create (block.c:5528) ==14881== by 0x118DE0: img_create (qemu-img.c:462) ==14881== by 0x11E7E4: main (qemu-img.c:2830) ==14881== Formatting 'qed', fmt=qed size=0 backing_file='/dev/null' backing_fmt='raw' cluster_size=65536 ==14881== Invalid write of size 8 ==14881== at 0x1D28BE: qemu_opts_del (qemu-option.c:750) ==14881== by 0x130BF3: bdrv_img_create (block.c:5638) ==14881== by 0x118DE0: img_create (qemu-img.c:462) ==14881== by 0x11E7E4: main (qemu-img.c:2830) ==14881== Address 0x11fedd38 is 24 bytes inside a block of size 232 free'd ==14881== at 0x4C2CA5E: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==14881== by 0x592D35E: g_realloc (in /usr/lib64/libglib-2.0.so.0.3800.2) ==14881== by 0x1D38D8: qemu_opts_append (qemu-option.c:1129) ==14881== by 0x13075E: bdrv_img_create (block.c:5528) ==14881== by 0x118DE0: img_create (qemu-img.c:462) ==14881== by 0x11E7E4: main (qemu-img.c:2830) ==14881== The problem is apparently in the qemu_opts_append(). Well, if it gets called twice or more. On the first call, when @dst is NULL some initialization is done during which @dst->head list gets initialized. The list is initialized in a way, so that the list tail points at the list head. However, the next time qemu_opts_append() is called for new options to be added, g_realloc() may move @dst to a new address making the old list tail point at an invalid address. If that's the case, we must update the list pointers. Signed-off-by: Michal Privoznik Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- util/qemu-option.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index 43de3add29..6dc27ce04f 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -1111,6 +1111,7 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst, size_t num_opts, num_dst_opts; QemuOptDesc *desc; bool need_init = false; + bool need_head_update; if (!list) { return dst; @@ -1121,6 +1122,12 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst, */ if (!dst) { need_init = true; + need_head_update = true; + } else { + /* Moreover, even if dst is not NULL, the realloc may move it to a + * different address in which case we may get a stale tail pointer + * in dst->head. */ + need_head_update = QTAILQ_EMPTY(&dst->head); } num_opts = count_opts_list(dst); @@ -1131,9 +1138,11 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst, if (need_init) { dst->name = NULL; dst->implied_opt_name = NULL; - QTAILQ_INIT(&dst->head); dst->merge_lists = false; } + if (need_head_update) { + QTAILQ_INIT(&dst->head); + } dst->desc[num_dst_opts].name = NULL; /* append list->desc to dst->desc */ From 9c75e168bc388094c04aabb6fc59c91abe06e81c Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Wed, 25 Jun 2014 16:55:30 -0400 Subject: [PATCH 17/47] block: check for RESIZE blocker in the QMP command, not bdrv_truncate() If we check for the RESIZE blocker in bdrv_truncate(), that means a commit will fail if the overlay layer is larger than the base, due to the backing blocker. This is a regression in behavior from 2.0; currently, commit will try to grow the size of the base image to match the overlay size, if the overlay size is larger. By moving this into the QMP command qmp_block_resize(), it allows usage of bdrv_truncate() within block jobs. Signed-off-by: Jeff Cody Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block.c | 4 +--- blockdev.c | 5 +++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/block.c b/block.c index 7d69f312b6..106238d0b7 100644 --- a/block.c +++ b/block.c @@ -3483,9 +3483,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset) return -ENOTSUP; if (bs->read_only) return -EACCES; - if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) { - return -EBUSY; - } + ret = drv->bdrv_truncate(bs, offset); if (ret == 0) { ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); diff --git a/blockdev.c b/blockdev.c index 03ab153d01..e8bfa3c660 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1819,6 +1819,11 @@ void qmp_block_resize(bool has_device, const char *device, return; } + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) { + error_set(errp, QERR_DEVICE_IN_USE, device); + return; + } + /* complete all in-flight operations before resizing the device */ bdrv_drain_all(); From d1fde4ad3c22137f8e589e625c21bf2ea7f6ba62 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Wed, 25 Jun 2014 16:55:31 -0400 Subject: [PATCH 18/47] block: add qemu-iotest for resize base during live commit If 'base' is smaller than the overlay image being committed into it, then the base image will be grown in commit_run via bdrv_truncate(). This tests to make sure that this works, and the bdrv_truncate() is not blocked when it shouldn't be. Signed-off-by: Jeff Cody Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/095 | 86 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/095.out | 31 ++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 118 insertions(+) create mode 100755 tests/qemu-iotests/095 create mode 100644 tests/qemu-iotests/095.out diff --git a/tests/qemu-iotests/095 b/tests/qemu-iotests/095 new file mode 100755 index 0000000000..acc7dbf182 --- /dev/null +++ b/tests/qemu-iotests/095 @@ -0,0 +1,86 @@ +#!/bin/bash +# +# Test for commit of larger active layer +# +# This tests live snapshots of images on a running QEMU instance, using +# QMP commands. Both single disk snapshots, and transactional group +# snapshots are performed. +# +# 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 . +# +# creator +owner=jcody@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_qemu + rm -f "${TEST_IMG}.base" "${TEST_IMG}.snp1" + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter +. ./common.qemu + +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +size_smaller=5M +size_larger=100M + +_make_test_img $size_smaller +mv "${TEST_IMG}" "${TEST_IMG}.base" + +_make_test_img -b "${TEST_IMG}.base" $size_larger +mv "${TEST_IMG}" "${TEST_IMG}.snp1" + +_make_test_img -b "${TEST_IMG}.snp1" $size_larger + +echo +echo "=== Base image info before commit and resize ===" +$QEMU_IMG info "${TEST_IMG}.base" | _filter_testdir + +echo +echo === Running QEMU Live Commit Test === +echo + +qemu_comm_method="qmp" +_launch_qemu -drive file="${TEST_IMG}",if=virtio,id=test +h=$QEMU_HANDLE + +_send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return" + +_send_qemu_cmd $h "{ 'execute': 'block-commit', + 'arguments': { 'device': 'test', + 'top': '"${TEST_IMG}.snp1"' } }" "BLOCK_JOB_COMPLETED" + +echo +echo "=== Base image info after commit and resize ===" +$QEMU_IMG info "${TEST_IMG}.base" | _filter_testdir + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/095.out b/tests/qemu-iotests/095.out new file mode 100644 index 0000000000..5864ddac2b --- /dev/null +++ b/tests/qemu-iotests/095.out @@ -0,0 +1,31 @@ +QA output created by 095 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=5242880 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=104857600 backing_file='TEST_DIR/t.IMGFMT.base' +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=104857600 backing_file='TEST_DIR/t.IMGFMT.snp1' + +=== Base image info before commit and resize === +image: TEST_DIR/t.qcow2.base +file format: qcow2 +virtual size: 5.0M (5242880 bytes) +disk size: 196K +cluster_size: 65536 +Format specific information: + compat: 1.1 + lazy refcounts: false + +=== Running QEMU Live Commit Test === + +{"return": {}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "test", "len": 104857600, "offset": 104857600, "speed": 0, "type": "commit"}} + +=== Base image info after commit and resize === +image: TEST_DIR/t.qcow2.base +file format: qcow2 +virtual size: 100M (104857600 bytes) +disk size: 196K +cluster_size: 65536 +Format specific information: + compat: 1.1 + lazy refcounts: false +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 0f074403ae..e3dc4e8754 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -99,3 +99,4 @@ 090 rw auto quick 091 rw auto 092 rw auto quick +095 rw auto From cf29a570a7aa7abab66bf256fdf9540873590811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Wed, 11 Jun 2014 15:24:10 +0200 Subject: [PATCH 19/47] quorum: Add the rewrite-corrupted parameter to quorum On read operations when this parameter is set and some replicas are corrupted while quorum can be reached quorum will proceed to rewrite the correct version of the data to fix the corrupted replicas. This will shine with SSD where the FTL will remap the same block at another place on rewrite. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 97 +++++++++++++++++++++++++++++++++++--- qapi/block-core.json | 5 +- tests/qemu-iotests/081 | 15 +++++- tests/qemu-iotests/081.out | 10 ++++ 4 files changed, 119 insertions(+), 8 deletions(-) diff --git a/block/quorum.c b/block/quorum.c index 86802d306b..d5ee9c0059 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -23,6 +23,7 @@ #define QUORUM_OPT_VOTE_THRESHOLD "vote-threshold" #define QUORUM_OPT_BLKVERIFY "blkverify" +#define QUORUM_OPT_REWRITE "rewrite-corrupted" /* This union holds a vote hash value */ typedef union QuorumVoteValue { @@ -70,6 +71,9 @@ typedef struct BDRVQuorumState { * It is useful to debug other block drivers by * comparing them with a reference one. */ + bool rewrite_corrupted;/* true if the driver must rewrite-on-read corrupted + * block if Quorum is reached. + */ } BDRVQuorumState; typedef struct QuorumAIOCB QuorumAIOCB; @@ -105,13 +109,17 @@ struct QuorumAIOCB { int count; /* number of completed AIOCB */ int success_count; /* number of successfully completed AIOCB */ + int rewrite_count; /* number of replica to rewrite: count down to + * zero once writes are fired + */ + QuorumVotes votes; bool is_read; int vote_ret; }; -static void quorum_vote(QuorumAIOCB *acb); +static bool quorum_vote(QuorumAIOCB *acb); static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) { @@ -183,6 +191,7 @@ static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, acb->qcrs = g_new0(QuorumChildRequest, s->num_children); acb->count = 0; acb->success_count = 0; + acb->rewrite_count = 0; acb->votes.compare = quorum_sha256_compare; QLIST_INIT(&acb->votes.vote_list); acb->is_read = false; @@ -232,11 +241,27 @@ static bool quorum_has_too_much_io_failed(QuorumAIOCB *acb) return false; } +static void quorum_rewrite_aio_cb(void *opaque, int ret) +{ + QuorumAIOCB *acb = opaque; + + /* one less rewrite to do */ + acb->rewrite_count--; + + /* wait until all rewrite callbacks have completed */ + if (acb->rewrite_count) { + return; + } + + quorum_aio_finalize(acb); +} + static void quorum_aio_cb(void *opaque, int ret) { QuorumChildRequest *sacb = opaque; QuorumAIOCB *acb = sacb->parent; BDRVQuorumState *s = acb->common.bs->opaque; + bool rewrite = false; sacb->ret = ret; acb->count++; @@ -253,12 +278,15 @@ static void quorum_aio_cb(void *opaque, int ret) /* Do the vote on read */ if (acb->is_read) { - quorum_vote(acb); + rewrite = quorum_vote(acb); } else { quorum_has_too_much_io_failed(acb); } - quorum_aio_finalize(acb); + /* if no rewrite is done the code will finish right away */ + if (!rewrite) { + quorum_aio_finalize(acb); + } } static void quorum_report_bad_versions(BDRVQuorumState *s, @@ -278,6 +306,43 @@ static void quorum_report_bad_versions(BDRVQuorumState *s, } } +static bool quorum_rewrite_bad_versions(BDRVQuorumState *s, QuorumAIOCB *acb, + QuorumVoteValue *value) +{ + QuorumVoteVersion *version; + QuorumVoteItem *item; + int count = 0; + + /* first count the number of bad versions: done first to avoid concurrency + * issues. + */ + QLIST_FOREACH(version, &acb->votes.vote_list, next) { + if (acb->votes.compare(&version->value, value)) { + continue; + } + QLIST_FOREACH(item, &version->items, next) { + count++; + } + } + + /* quorum_rewrite_aio_cb will count down this to zero */ + acb->rewrite_count = count; + + /* now fire the correcting rewrites */ + QLIST_FOREACH(version, &acb->votes.vote_list, next) { + if (acb->votes.compare(&version->value, value)) { + continue; + } + QLIST_FOREACH(item, &version->items, next) { + bdrv_aio_writev(s->bs[item->index], acb->sector_num, acb->qiov, + acb->nb_sectors, quorum_rewrite_aio_cb, acb); + } + } + + /* return true if any rewrite is done else false */ + return count; +} + static void quorum_copy_qiov(QEMUIOVector *dest, QEMUIOVector *source) { int i; @@ -468,16 +533,17 @@ static int quorum_vote_error(QuorumAIOCB *acb) return ret; } -static void quorum_vote(QuorumAIOCB *acb) +static bool quorum_vote(QuorumAIOCB *acb) { bool quorum = true; + bool rewrite = false; int i, j, ret; QuorumVoteValue hash; BDRVQuorumState *s = acb->common.bs->opaque; QuorumVoteVersion *winner; if (quorum_has_too_much_io_failed(acb)) { - return; + return false; } /* get the index of the first successful read */ @@ -505,7 +571,7 @@ static void quorum_vote(QuorumAIOCB *acb) /* Every successful read agrees */ if (quorum) { quorum_copy_qiov(acb->qiov, &acb->qcrs[i].qiov); - return; + return false; } /* compute hashes for each successful read, also store indexes */ @@ -538,9 +604,15 @@ static void quorum_vote(QuorumAIOCB *acb) /* some versions are bad print them */ quorum_report_bad_versions(s, acb, &winner->value); + /* corruption correction is enabled */ + if (s->rewrite_corrupted) { + rewrite = quorum_rewrite_bad_versions(s, acb, &winner->value); + } + free_exit: /* free lists */ quorum_free_vote_list(&acb->votes); + return rewrite; } static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs, @@ -705,6 +777,11 @@ static QemuOptsList quorum_runtime_opts = { .type = QEMU_OPT_BOOL, .help = "Trigger block verify mode if set", }, + { + .name = QUORUM_OPT_REWRITE, + .type = QEMU_OPT_BOOL, + .help = "Rewrite corrupted block on read quorum", + }, { /* end of list */ } }, }; @@ -766,6 +843,14 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, "and using two files with vote_threshold=2\n"); } + s->rewrite_corrupted = qemu_opt_get_bool(opts, QUORUM_OPT_REWRITE, false); + if (s->rewrite_corrupted && s->is_blkverify) { + error_setg(&local_err, + "rewrite-corrupted=on cannot be used with blkverify=on"); + ret = -EINVAL; + goto exit; + } + /* allocate the children BlockDriverState array */ s->bs = g_new0(BlockDriverState *, s->num_children); opened = g_new0(bool, s->num_children); diff --git a/qapi/block-core.json b/qapi/block-core.json index af6b436540..de31f9fd54 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1329,12 +1329,15 @@ # # @vote-threshold: the vote limit under which a read will fail # +# @rewrite-corrupted: #optional rewrite corrupted data when quorum is reached +# (Since 2.1) +# # Since: 2.0 ## { 'type': 'BlockdevOptionsQuorum', 'data': { '*blkverify': 'bool', 'children': [ 'BlockdevRef' ], - 'vote-threshold': 'int' } } + 'vote-threshold': 'int', '*rewrite-corrupted': 'bool' } } ## # @BlockdevOptions diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081 index b512d00cc8..7ae4be2053 100755 --- a/tests/qemu-iotests/081 +++ b/tests/qemu-iotests/081 @@ -133,16 +133,29 @@ run_qemu -drive "file=$TEST_DIR/2.raw,format=$IMGFMT,if=none,id=drive2" < Date: Mon, 16 Jun 2014 12:00:55 +0200 Subject: [PATCH 20/47] block: Add node-name argument to drive-mirror This new argument can be used to specify the node-name of the new mirrored BDS. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 11 +++++++++-- hmp.c | 1 + qapi/block-core.json | 4 ++++ qmp-commands.hx | 3 +++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index e8bfa3c660..943301226d 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2099,6 +2099,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp) void qmp_drive_mirror(const char *device, const char *target, bool has_format, const char *format, + bool has_node_name, const char *node_name, enum MirrorSyncMode sync, bool has_mode, enum NewImageMode mode, bool has_speed, int64_t speed, @@ -2112,6 +2113,7 @@ void qmp_drive_mirror(const char *device, const char *target, BlockDriverState *source, *target_bs; BlockDriver *drv = NULL; Error *local_err = NULL; + QDict *options = NULL; int flags; int64_t size; int ret; @@ -2213,12 +2215,17 @@ void qmp_drive_mirror(const char *device, const char *target, return; } + if (has_node_name) { + options = qdict_new(); + qdict_put(options, "node-name", qstring_from_str(node_name)); + } + /* Mirroring takes care of copy-on-write using the source's backing * file. */ target_bs = NULL; - ret = bdrv_open(&target_bs, target, NULL, NULL, flags | BDRV_O_NO_BACKING, - drv, &local_err); + ret = bdrv_open(&target_bs, target, NULL, options, + flags | BDRV_O_NO_BACKING, drv, &local_err); if (ret < 0) { error_propagate(errp, local_err); return; diff --git a/hmp.c b/hmp.c index dc3d2795d2..73acc3283c 100644 --- a/hmp.c +++ b/hmp.c @@ -933,6 +933,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict) } qmp_drive_mirror(device, filename, !!format, format, + false, NULL, full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP, true, mode, false, 0, false, 0, false, 0, false, 0, false, 0, &err); diff --git a/qapi/block-core.json b/qapi/block-core.json index de31f9fd54..a46cdbe6aa 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -765,6 +765,9 @@ # @format: #optional the format of the new destination, default is to # probe if @mode is 'existing', else the format of the source # +# @node-name: #optional the new block driver state node name in the graph +# (Since 2.1) +# # @mode: #optional whether and how QEMU should create a new image, default is # 'absolute-paths'. # @@ -797,6 +800,7 @@ ## { 'command': 'drive-mirror', 'data': { 'device': 'str', 'target': 'str', '*format': 'str', + '*node-name': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', '*speed': 'int', '*granularity': 'uint32', '*buf-size': 'int', '*on-source-error': 'BlockdevOnError', diff --git a/qmp-commands.hx b/qmp-commands.hx index e4a1c80434..5254938878 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1293,6 +1293,7 @@ EQMP { .name = "drive-mirror", .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," + "node-name:s?," "on-source-error:s?,on-target-error:s?," "granularity:i?,buf-size:i?", .mhandler.cmd_new = qmp_marshal_input_drive_mirror, @@ -1314,6 +1315,8 @@ Arguments: - "device": device name to operate on (json-string) - "target": name of new image file (json-string) - "format": format of new image (json-string, optional) +- "node-name": the name of the new block driver state in the node graph + (json-string, optional) - "mode": how an image file should be created into the target file/device (NewImageMode, optional, default 'absolute-paths') - "speed": maximum speed of the streaming job, in bytes per second From 09f645877037590415016b59f6d32be1a27229c6 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:42 +0800 Subject: [PATCH 21/47] virtio-blk: Move VirtIOBlockReq to header For later reusing by dataplane code. Signed-off-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 11 ----------- include/hw/virtio/virtio-blk.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 08562ea390..4ff64870bc 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -27,17 +27,6 @@ #endif #include "hw/virtio/virtio-bus.h" -typedef struct VirtIOBlockReq -{ - VirtIOBlock *dev; - VirtQueueElement elem; - struct virtio_blk_inhdr *in; - struct virtio_blk_outhdr *out; - QEMUIOVector qiov; - struct VirtIOBlockReq *next; - BlockAcctCookie acct; -} VirtIOBlockReq; - static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) { VirtIOBlock *s = req->dev; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 4bc9b549ad..d05d177ec5 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -17,6 +17,7 @@ #include "hw/virtio/virtio.h" #include "hw/block/block.h" #include "sysemu/iothread.h" +#include "block/block.h" #define TYPE_VIRTIO_BLK "virtio-blk-device" #define VIRTIO_BLK(obj) \ @@ -133,6 +134,16 @@ typedef struct VirtIOBlock { #endif } VirtIOBlock; +typedef struct VirtIOBlockReq { + VirtIOBlock *dev; + VirtQueueElement elem; + struct virtio_blk_inhdr *in; + struct virtio_blk_outhdr *out; + QEMUIOVector qiov; + struct VirtIOBlockReq *next; + BlockAcctCookie acct; +} VirtIOBlockReq; + #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \ DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) From 671ec3f056559f22a2531a91dce3a258b9b5eb8a Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:43 +0800 Subject: [PATCH 22/47] virtio-blk: Convert VirtIOBlockReq.elem to pointer This will make converging with dataplane code easier. Add virtio_blk_free_request to handle the freeing of request internal fields. Signed-off-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 85 ++++++++++++++++++---------------- include/hw/virtio/virtio-blk.h | 2 +- 2 files changed, 47 insertions(+), 40 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 4ff64870bc..b5cc3855cf 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -27,6 +27,22 @@ #endif #include "hw/virtio/virtio-bus.h" +static VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s) +{ + VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); + req->dev = s; + req->elem = g_slice_new0(VirtQueueElement); + return req; +} + +static void virtio_blk_free_request(VirtIOBlockReq *req) +{ + if (req) { + g_slice_free(VirtQueueElement, req->elem); + g_slice_free(VirtIOBlockReq, req); + } +} + static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) { VirtIOBlock *s = req->dev; @@ -35,7 +51,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) trace_virtio_blk_req_complete(req, status); stb_p(&req->in->status, status); - virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in)); + virtqueue_push(s->vq, req->elem, req->qiov.size + sizeof(*req->in)); virtio_notify(vdev, s->vq); } @@ -51,7 +67,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, } else if (action == BLOCK_ERROR_ACTION_REPORT) { virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); bdrv_acct_done(s->bs, &req->acct); - g_free(req); + virtio_blk_free_request(req); } bdrv_error_action(s->bs, action, is_read, error); @@ -72,7 +88,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret) virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); bdrv_acct_done(req->dev->bs, &req->acct); - g_free(req); + virtio_blk_free_request(req); } static void virtio_blk_flush_complete(void *opaque, int ret) @@ -87,27 +103,16 @@ static void virtio_blk_flush_complete(void *opaque, int ret) virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); bdrv_acct_done(req->dev->bs, &req->acct); - g_free(req); -} - -static VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s) -{ - VirtIOBlockReq *req = g_malloc(sizeof(*req)); - req->dev = s; - req->qiov.size = 0; - req->next = NULL; - return req; + virtio_blk_free_request(req); } static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s) { VirtIOBlockReq *req = virtio_blk_alloc_request(s); - if (req != NULL) { - if (!virtqueue_pop(s->vq, &req->elem)) { - g_free(req); - return NULL; - } + if (!virtqueue_pop(s->vq, req->elem)) { + virtio_blk_free_request(req); + return NULL; } return req; @@ -236,9 +241,9 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) { int status; - status = virtio_blk_handle_scsi_req(req->dev, &req->elem); + status = virtio_blk_handle_scsi_req(req->dev, req->elem); virtio_blk_req_complete(req, status); - g_free(req); + virtio_blk_free_request(req); } typedef struct MultiReqBuffer { @@ -340,19 +345,19 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, { uint32_t type; - if (req->elem.out_num < 1 || req->elem.in_num < 1) { + if (req->elem->out_num < 1 || req->elem->in_num < 1) { error_report("virtio-blk missing headers"); exit(1); } - if (req->elem.out_sg[0].iov_len < sizeof(*req->out) || - req->elem.in_sg[req->elem.in_num - 1].iov_len < sizeof(*req->in)) { + if (req->elem->out_sg[0].iov_len < sizeof(*req->out) || + req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) { error_report("virtio-blk header not in correct element"); exit(1); } - req->out = (void *)req->elem.out_sg[0].iov_base; - req->in = (void *)req->elem.in_sg[req->elem.in_num - 1].iov_base; + req->out = (void *)req->elem->out_sg[0].iov_base; + req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base; type = ldl_p(&req->out->type); @@ -367,23 +372,23 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, * NB: per existing s/n string convention the string is * terminated by '\0' only when shorter than buffer. */ - strncpy(req->elem.in_sg[0].iov_base, + strncpy(req->elem->in_sg[0].iov_base, s->blk.serial ? s->blk.serial : "", - MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES)); + MIN(req->elem->in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES)); virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); - g_free(req); + virtio_blk_free_request(req); } else if (type & VIRTIO_BLK_T_OUT) { - qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1], - req->elem.out_num - 1); + qemu_iovec_init_external(&req->qiov, &req->elem->out_sg[1], + req->elem->out_num - 1); virtio_blk_handle_write(req, mrb); } else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) { /* VIRTIO_BLK_T_IN is 0, so we can't just & it. */ - qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0], - req->elem.in_num - 1); + qemu_iovec_init_external(&req->qiov, &req->elem->in_sg[0], + req->elem->in_num - 1); virtio_blk_handle_read(req); } else { virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); - g_free(req); + virtio_blk_free_request(req); } } @@ -598,7 +603,8 @@ static void virtio_blk_save(QEMUFile *f, void *opaque) while (req) { qemu_put_sbyte(f, 1); - qemu_put_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem)); + qemu_put_buffer(f, (unsigned char *)req->elem, + sizeof(VirtQueueElement)); req = req->next; } qemu_put_sbyte(f, 0); @@ -620,14 +626,15 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id) while (qemu_get_sbyte(f)) { VirtIOBlockReq *req = virtio_blk_alloc_request(s); - qemu_get_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem)); + qemu_get_buffer(f, (unsigned char *)req->elem, + sizeof(VirtQueueElement)); req->next = s->rq; s->rq = req; - virtqueue_map_sg(req->elem.in_sg, req->elem.in_addr, - req->elem.in_num, 1); - virtqueue_map_sg(req->elem.out_sg, req->elem.out_addr, - req->elem.out_num, 0); + virtqueue_map_sg(req->elem->in_sg, req->elem->in_addr, + req->elem->in_num, 1); + virtqueue_map_sg(req->elem->out_sg, req->elem->out_addr, + req->elem->out_num, 0); } return 0; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index d05d177ec5..b495e42d6d 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -136,7 +136,7 @@ typedef struct VirtIOBlock { typedef struct VirtIOBlockReq { VirtIOBlock *dev; - VirtQueueElement elem; + VirtQueueElement *elem; struct virtio_blk_inhdr *in; struct virtio_blk_outhdr *out; QEMUIOVector qiov; From 0bcb34472db7b9b007415151a1f7a3d495b31f28 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:44 +0800 Subject: [PATCH 23/47] virtio-blk: Drop bounce buffer from dataplane code The block layer will handle the unaligned request. Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index c10b7b70fb..3d1e9e1c43 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -29,8 +29,6 @@ typedef struct { QEMUIOVector *inhdr; /* iovecs for virtio_blk_inhdr */ VirtQueueElement *elem; /* saved data from the virtqueue */ QEMUIOVector qiov; /* original request iovecs */ - struct iovec bounce_iov; /* used if guest buffers are unaligned */ - QEMUIOVector bounce_qiov; /* bounce buffer iovecs */ bool read; /* read or write? */ } VirtIOBlockRequest; @@ -85,14 +83,6 @@ static void complete_rdwr(void *opaque, int ret) trace_virtio_blk_data_plane_complete_request(req->s, req->elem->index, ret); - if (req->read && req->bounce_iov.iov_base) { - qemu_iovec_from_buf(&req->qiov, 0, req->bounce_iov.iov_base, len); - } - - if (req->bounce_iov.iov_base) { - qemu_vfree(req->bounce_iov.iov_base); - } - qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr)); qemu_iovec_destroy(req->inhdr); g_slice_free(QEMUIOVector, req->inhdr); @@ -152,21 +142,6 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, qiov = &req->qiov; - if (!bdrv_qiov_is_aligned(s->blk->conf.bs, qiov)) { - void *bounce_buffer = qemu_blockalign(s->blk->conf.bs, qiov->size); - - /* Populate bounce buffer with data for writes */ - if (!read) { - qemu_iovec_to_buf(qiov, 0, bounce_buffer, qiov->size); - } - - /* Redirect I/O to aligned bounce buffer */ - req->bounce_iov.iov_base = bounce_buffer; - req->bounce_iov.iov_len = qiov->size; - qemu_iovec_init_external(&req->bounce_qiov, &req->bounce_iov, 1); - qiov = &req->bounce_qiov; - } - nb_sectors = qiov->size / BDRV_SECTOR_SIZE; if (read) { From 98e2d49241cb474a9fdd26852cbdbbcf67f18d54 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:45 +0800 Subject: [PATCH 24/47] virtio-blk: Drop VirtIOBlockRequest.read Since it's set but not used. Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 3d1e9e1c43..4e5e4589bd 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -29,7 +29,6 @@ typedef struct { QEMUIOVector *inhdr; /* iovecs for virtio_blk_inhdr */ VirtQueueElement *elem; /* saved data from the virtqueue */ QEMUIOVector qiov; /* original request iovecs */ - bool read; /* read or write? */ } VirtIOBlockRequest; struct VirtIOBlockDataPlane { @@ -137,7 +136,6 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, req->s = s; req->elem = elem; req->inhdr = inhdr; - req->read = read; qemu_iovec_init_external(&req->qiov, iov, iov_cnt); qiov = &req->qiov; From 04af2d70c58136be57d798ad684e1924dcde93f5 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:46 +0800 Subject: [PATCH 25/47] virtio-blk: Replace VirtIOBlockRequest with VirtIOBlockReq Field "inhdr" is added temporarily for a more mechanical change, and will be dropped in the next commit. Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 34 +++++++++++++++------------------ include/hw/virtio/virtio-blk.h | 4 ++++ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 4e5e4589bd..70e8c1452b 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -24,13 +24,6 @@ #include "hw/virtio/virtio-bus.h" #include "qom/object_interfaces.h" -typedef struct { - VirtIOBlockDataPlane *s; - QEMUIOVector *inhdr; /* iovecs for virtio_blk_inhdr */ - VirtQueueElement *elem; /* saved data from the virtqueue */ - QEMUIOVector qiov; /* original request iovecs */ -} VirtIOBlockRequest; - struct VirtIOBlockDataPlane { bool started; bool starting; @@ -68,7 +61,7 @@ static void notify_guest(VirtIOBlockDataPlane *s) static void complete_rdwr(void *opaque, int ret) { - VirtIOBlockRequest *req = opaque; + VirtIOBlockReq *req = opaque; struct virtio_blk_inhdr hdr; int len; @@ -80,7 +73,8 @@ static void complete_rdwr(void *opaque, int ret) len = 0; } - trace_virtio_blk_data_plane_complete_request(req->s, req->elem->index, ret); + trace_virtio_blk_data_plane_complete_request(req->dev->dataplane, + req->elem->index, ret); qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr)); qemu_iovec_destroy(req->inhdr); @@ -90,9 +84,9 @@ static void complete_rdwr(void *opaque, int ret) * written to, but for virtio-blk it seems to be the number of bytes * transferred plus the status bytes. */ - vring_push(&req->s->vring, req->elem, len + sizeof(hdr)); - notify_guest(req->s); - g_slice_free(VirtIOBlockRequest, req); + vring_push(&req->dev->dataplane->vring, req->elem, len + sizeof(hdr)); + notify_guest(req->dev->dataplane); + g_slice_free(VirtIOBlockReq, req); } static void complete_request_early(VirtIOBlockDataPlane *s, VirtQueueElement *elem, @@ -128,14 +122,15 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, int64_t sector_num, VirtQueueElement *elem, QEMUIOVector *inhdr) { - VirtIOBlockRequest *req = g_slice_new0(VirtIOBlockRequest); + VirtIOBlock *dev = VIRTIO_BLK(s->vdev); + VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); QEMUIOVector *qiov; int nb_sectors; /* Fill in virtio block metadata needed for completion */ - req->s = s; req->elem = elem; req->inhdr = inhdr; + req->dev = dev; qemu_iovec_init_external(&req->qiov, iov, iov_cnt); qiov = &req->qiov; @@ -153,7 +148,7 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, static void complete_flush(void *opaque, int ret) { - VirtIOBlockRequest *req = opaque; + VirtIOBlockReq *req = opaque; unsigned char status; if (ret == 0) { @@ -162,15 +157,16 @@ static void complete_flush(void *opaque, int ret) status = VIRTIO_BLK_S_IOERR; } - complete_request_early(req->s, req->elem, req->inhdr, status); - g_slice_free(VirtIOBlockRequest, req); + complete_request_early(req->dev->dataplane, req->elem, req->inhdr, status); + g_slice_free(VirtIOBlockReq, req); } static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem, QEMUIOVector *inhdr) { - VirtIOBlockRequest *req = g_slice_new(VirtIOBlockRequest); - req->s = s; + VirtIOBlock *dev = VIRTIO_BLK(s->vdev); + VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); + req->dev = dev; req->elem = elem; req->inhdr = inhdr; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index b495e42d6d..4211cd645f 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -142,6 +142,10 @@ typedef struct VirtIOBlockReq { QEMUIOVector qiov; struct VirtIOBlockReq *next; BlockAcctCookie acct; + +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE + QEMUIOVector *inhdr; /* iovecs for virtio_blk_inhdr */ +#endif } VirtIOBlockReq; #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \ From eddb102e86f61d4b71877f8ac268ebc4bf7265bf Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:47 +0800 Subject: [PATCH 26/47] virtio-blk: Use VirtIOBlockReq.in to drop VirtIOBlockReq.inhdr In current virtio spec, inhdr is a single byte, and is unlikely to change for both functionality and compatibility considerations. Non-dataplane uses .in, and we are on the way to converge them. So let's unify it to get cleaner code. Remove .inhdr and use .in. Signed-off-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 54 ++++++++++++++------------------- include/hw/virtio/virtio-blk.h | 4 --- 2 files changed, 22 insertions(+), 36 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 70e8c1452b..cef707fac2 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -76,9 +76,7 @@ static void complete_rdwr(void *opaque, int ret) trace_virtio_blk_data_plane_complete_request(req->dev->dataplane, req->elem->index, ret); - qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr)); - qemu_iovec_destroy(req->inhdr); - g_slice_free(QEMUIOVector, req->inhdr); + stb_p(&req->in->status, hdr.status); /* According to the virtio specification len should be the number of bytes * written to, but for virtio-blk it seems to be the number of bytes @@ -90,24 +88,20 @@ static void complete_rdwr(void *opaque, int ret) } static void complete_request_early(VirtIOBlockDataPlane *s, VirtQueueElement *elem, - QEMUIOVector *inhdr, unsigned char status) + struct virtio_blk_inhdr *inhdr, + unsigned char status) { - struct virtio_blk_inhdr hdr = { - .status = status, - }; + stb_p(&inhdr->status, status); - qemu_iovec_from_buf(inhdr, 0, &hdr, sizeof(hdr)); - qemu_iovec_destroy(inhdr); - g_slice_free(QEMUIOVector, inhdr); - - vring_push(&s->vring, elem, sizeof(hdr)); + vring_push(&s->vring, elem, sizeof(*inhdr)); notify_guest(s); } /* Get disk serial number */ static void do_get_id_cmd(VirtIOBlockDataPlane *s, struct iovec *iov, unsigned int iov_cnt, - VirtQueueElement *elem, QEMUIOVector *inhdr) + VirtQueueElement *elem, + struct virtio_blk_inhdr *inhdr) { char id[VIRTIO_BLK_ID_BYTES]; @@ -120,7 +114,7 @@ static void do_get_id_cmd(VirtIOBlockDataPlane *s, static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, struct iovec *iov, unsigned iov_cnt, int64_t sector_num, VirtQueueElement *elem, - QEMUIOVector *inhdr) + struct virtio_blk_inhdr *inhdr) { VirtIOBlock *dev = VIRTIO_BLK(s->vdev); VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); @@ -129,8 +123,8 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, /* Fill in virtio block metadata needed for completion */ req->elem = elem; - req->inhdr = inhdr; req->dev = dev; + req->in = inhdr; qemu_iovec_init_external(&req->qiov, iov, iov_cnt); qiov = &req->qiov; @@ -157,24 +151,24 @@ static void complete_flush(void *opaque, int ret) status = VIRTIO_BLK_S_IOERR; } - complete_request_early(req->dev->dataplane, req->elem, req->inhdr, status); + complete_request_early(req->dev->dataplane, req->elem, req->in, status); g_slice_free(VirtIOBlockReq, req); } static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem, - QEMUIOVector *inhdr) + struct virtio_blk_inhdr *inhdr) { VirtIOBlock *dev = VIRTIO_BLK(s->vdev); VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); req->dev = dev; req->elem = elem; - req->inhdr = inhdr; + req->in = inhdr; bdrv_aio_flush(s->blk->conf.bs, complete_flush, req); } static void do_scsi_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem, - QEMUIOVector *inhdr) + struct virtio_blk_inhdr *inhdr) { int status; @@ -189,8 +183,7 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) unsigned out_num = elem->out_num; unsigned in_num = elem->in_num; struct virtio_blk_outhdr outhdr; - QEMUIOVector *inhdr; - size_t in_size; + struct virtio_blk_inhdr *inhdr; /* Copy in outhdr */ if (unlikely(iov_to_buf(iov, out_num, 0, &outhdr, @@ -200,17 +193,16 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) } iov_discard_front(&iov, &out_num, sizeof(outhdr)); - /* Grab inhdr for later */ - in_size = iov_size(in_iov, in_num); - if (in_size < sizeof(struct virtio_blk_inhdr)) { - error_report("virtio_blk request inhdr too short"); + /* We are likely safe with the iov_len check, because inhdr is only 1 byte, + * but checking here in case the header gets bigger in the future. */ + if (in_num < 1 || in_iov[in_num - 1].iov_len < sizeof(*inhdr)) { + error_report("virtio-blk request inhdr too short"); return -EFAULT; } - inhdr = g_slice_new(QEMUIOVector); - qemu_iovec_init(inhdr, 1); - qemu_iovec_concat_iov(inhdr, in_iov, in_num, - in_size - sizeof(struct virtio_blk_inhdr), - sizeof(struct virtio_blk_inhdr)); + + /* Grab inhdr for later */ + inhdr = (void *)in_iov[in_num - 1].iov_base + + in_iov[in_num - 1].iov_len - sizeof(*inhdr); iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr)); /* TODO Linux sets the barrier bit even when not advertised! */ @@ -243,8 +235,6 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) default: error_report("virtio-blk unsupported request type %#x", outhdr.type); - qemu_iovec_destroy(inhdr); - g_slice_free(QEMUIOVector, inhdr); return -EFAULT; } } diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 4211cd645f..b495e42d6d 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -142,10 +142,6 @@ typedef struct VirtIOBlockReq { QEMUIOVector qiov; struct VirtIOBlockReq *next; BlockAcctCookie acct; - -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE - QEMUIOVector *inhdr; /* iovecs for virtio_blk_inhdr */ -#endif } VirtIOBlockReq; #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \ From 827805a2492c1bbf1c0712ed18ee069b4ebf3dd6 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:48 +0800 Subject: [PATCH 27/47] virtio-blk: Convert VirtIOBlockReq.out to structrue The virtio code currently assumes that the outhdr is in its own iovec. This is not guaranteed by the spec, so we should relax this assumption. Convert the VirtIOBlockReq.out field to structrue so that we can use iov_to_buf and then discard the header from the beginning of iovec. Suggested-by: Paolo Bonzini Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 20 ++++++++++++++------ include/hw/virtio/virtio-blk.h | 2 +- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index b5cc3855cf..05610959a6 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -12,6 +12,7 @@ */ #include "qemu-common.h" +#include "qemu/iov.h" #include "qemu/error-report.h" #include "trace.h" #include "hw/block/block.h" @@ -81,7 +82,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret) trace_virtio_blk_rw_complete(req, ret); if (ret) { - bool is_read = !(ldl_p(&req->out->type) & VIRTIO_BLK_T_OUT); + bool is_read = !(ldl_p(&req->out.type) & VIRTIO_BLK_T_OUT); if (virtio_blk_handle_rw_error(req, -ret, is_read)) return; } @@ -287,7 +288,7 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, MultiReqBuffer *mrb) BlockRequest *blkreq; uint64_t sector; - sector = ldq_p(&req->out->sector); + sector = ldq_p(&req->out.sector); bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_WRITE); @@ -321,7 +322,7 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req) { uint64_t sector; - sector = ldq_p(&req->out->sector); + sector = ldq_p(&req->out.sector); bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_READ); @@ -344,22 +345,29 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) { uint32_t type; + struct iovec *iov = req->elem->out_sg; + unsigned out_num = req->elem->out_num; if (req->elem->out_num < 1 || req->elem->in_num < 1) { error_report("virtio-blk missing headers"); exit(1); } - if (req->elem->out_sg[0].iov_len < sizeof(*req->out) || + if (req->elem->out_sg[0].iov_len < sizeof(req->out) || req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) { error_report("virtio-blk header not in correct element"); exit(1); } - req->out = (void *)req->elem->out_sg[0].iov_base; + if (unlikely(iov_to_buf(iov, out_num, 0, &req->out, + sizeof(req->out)) != sizeof(req->out))) { + error_report("virtio-blk request outhdr too short"); + exit(1); + } + iov_discard_front(&iov, &out_num, sizeof(req->out)); req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base; - type = ldl_p(&req->out->type); + type = ldl_p(&req->out.type); if (type & VIRTIO_BLK_T_FLUSH) { virtio_blk_handle_flush(req, mrb); diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index b495e42d6d..2571e961ec 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -138,7 +138,7 @@ typedef struct VirtIOBlockReq { VirtIOBlock *dev; VirtQueueElement *elem; struct virtio_blk_inhdr *in; - struct virtio_blk_outhdr *out; + struct virtio_blk_outhdr out; QEMUIOVector qiov; struct VirtIOBlockReq *next; BlockAcctCookie acct; From ab2e3cd2dc97ad87fcdfb814a13b2ef386a5f13a Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:49 +0800 Subject: [PATCH 28/47] virtio-blk: Fill in VirtIOBlockReq.out in dataplane code VirtIOBlockReq is allocated in process_request, and freed in command functions. Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 102 ++++++++++++++------------------ 1 file changed, 44 insertions(+), 58 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index cef707fac2..29e7ec3071 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -87,55 +87,49 @@ static void complete_rdwr(void *opaque, int ret) g_slice_free(VirtIOBlockReq, req); } -static void complete_request_early(VirtIOBlockDataPlane *s, VirtQueueElement *elem, - struct virtio_blk_inhdr *inhdr, - unsigned char status) +static void complete_request_early(VirtIOBlockReq *req, unsigned char status) { - stb_p(&inhdr->status, status); + stb_p(&req->in->status, status); - vring_push(&s->vring, elem, sizeof(*inhdr)); - notify_guest(s); + vring_push(&req->dev->dataplane->vring, req->elem, sizeof(*req->in)); + notify_guest(req->dev->dataplane); + g_slice_free(VirtIOBlockReq, req); } /* Get disk serial number */ -static void do_get_id_cmd(VirtIOBlockDataPlane *s, - struct iovec *iov, unsigned int iov_cnt, - VirtQueueElement *elem, - struct virtio_blk_inhdr *inhdr) +static void do_get_id_cmd(VirtIOBlockReq *req, + struct iovec *iov, unsigned int iov_cnt) { char id[VIRTIO_BLK_ID_BYTES]; /* Serial number not NUL-terminated when longer than buffer */ - strncpy(id, s->blk->serial ? s->blk->serial : "", sizeof(id)); + strncpy(id, req->dev->blk.serial ? req->dev->blk.serial : "", sizeof(id)); iov_from_buf(iov, iov_cnt, 0, id, sizeof(id)); - complete_request_early(s, elem, inhdr, VIRTIO_BLK_S_OK); + complete_request_early(req, VIRTIO_BLK_S_OK); } -static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, - struct iovec *iov, unsigned iov_cnt, - int64_t sector_num, VirtQueueElement *elem, - struct virtio_blk_inhdr *inhdr) + +static void do_rdwr_cmd(bool read, VirtIOBlockReq *req, + struct iovec *iov, unsigned iov_cnt) { - VirtIOBlock *dev = VIRTIO_BLK(s->vdev); - VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); QEMUIOVector *qiov; int nb_sectors; + int64_t sector_num; - /* Fill in virtio block metadata needed for completion */ - req->elem = elem; - req->dev = dev; - req->in = inhdr; qemu_iovec_init_external(&req->qiov, iov, iov_cnt); qiov = &req->qiov; + sector_num = req->out.sector * 512 / BDRV_SECTOR_SIZE; nb_sectors = qiov->size / BDRV_SECTOR_SIZE; if (read) { - bdrv_aio_readv(s->blk->conf.bs, sector_num, qiov, nb_sectors, + bdrv_aio_readv(req->dev->blk.conf.bs, + sector_num, qiov, nb_sectors, complete_rdwr, req); } else { - bdrv_aio_writev(s->blk->conf.bs, sector_num, qiov, nb_sectors, + bdrv_aio_writev(req->dev->blk.conf.bs, + sector_num, qiov, nb_sectors, complete_rdwr, req); } } @@ -151,29 +145,21 @@ static void complete_flush(void *opaque, int ret) status = VIRTIO_BLK_S_IOERR; } - complete_request_early(req->dev->dataplane, req->elem, req->in, status); - g_slice_free(VirtIOBlockReq, req); + complete_request_early(req, status); } -static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem, - struct virtio_blk_inhdr *inhdr) +static void do_flush_cmd(VirtIOBlockReq *req) { - VirtIOBlock *dev = VIRTIO_BLK(s->vdev); - VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); - req->dev = dev; - req->elem = elem; - req->in = inhdr; - bdrv_aio_flush(s->blk->conf.bs, complete_flush, req); + bdrv_aio_flush(req->dev->blk.conf.bs, complete_flush, req); } -static void do_scsi_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem, - struct virtio_blk_inhdr *inhdr) +static void do_scsi_cmd(VirtIOBlockReq *req) { int status; - status = virtio_blk_handle_scsi_req(VIRTIO_BLK(s->vdev), elem); - complete_request_early(s, elem, inhdr, status); + status = virtio_blk_handle_scsi_req(req->dev, req->elem); + complete_request_early(req, status); } static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) @@ -182,59 +168,59 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) struct iovec *in_iov = elem->in_sg; unsigned out_num = elem->out_num; unsigned in_num = elem->in_num; - struct virtio_blk_outhdr outhdr; - struct virtio_blk_inhdr *inhdr; + VirtIOBlockReq *req; + req = g_slice_new(VirtIOBlockReq); + req->dev = VIRTIO_BLK(s->vdev); + req->elem = elem; /* Copy in outhdr */ - if (unlikely(iov_to_buf(iov, out_num, 0, &outhdr, - sizeof(outhdr)) != sizeof(outhdr))) { + if (unlikely(iov_to_buf(iov, out_num, 0, &req->out, + sizeof(req->out)) != sizeof(req->out))) { error_report("virtio-blk request outhdr too short"); + g_slice_free(VirtIOBlockReq, req); return -EFAULT; } - iov_discard_front(&iov, &out_num, sizeof(outhdr)); + iov_discard_front(&iov, &out_num, sizeof(req->out)); /* We are likely safe with the iov_len check, because inhdr is only 1 byte, * but checking here in case the header gets bigger in the future. */ - if (in_num < 1 || in_iov[in_num - 1].iov_len < sizeof(*inhdr)) { + if (in_num < 1 || in_iov[in_num - 1].iov_len < sizeof(*req->in)) { error_report("virtio-blk request inhdr too short"); return -EFAULT; } /* Grab inhdr for later */ - inhdr = (void *)in_iov[in_num - 1].iov_base - + in_iov[in_num - 1].iov_len - sizeof(*inhdr); + req->in = (void *)in_iov[in_num - 1].iov_base + + in_iov[in_num - 1].iov_len - sizeof(*req->in); iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr)); /* TODO Linux sets the barrier bit even when not advertised! */ - outhdr.type &= ~VIRTIO_BLK_T_BARRIER; + req->out.type &= ~VIRTIO_BLK_T_BARRIER; - switch (outhdr.type) { + switch (req->out.type) { case VIRTIO_BLK_T_IN: - do_rdwr_cmd(s, true, in_iov, in_num, - outhdr.sector * 512 / BDRV_SECTOR_SIZE, - elem, inhdr); + do_rdwr_cmd(true, req, in_iov, in_num); return 0; case VIRTIO_BLK_T_OUT: - do_rdwr_cmd(s, false, iov, out_num, - outhdr.sector * 512 / BDRV_SECTOR_SIZE, - elem, inhdr); + do_rdwr_cmd(false, req, iov, out_num); return 0; case VIRTIO_BLK_T_SCSI_CMD: - do_scsi_cmd(s, elem, inhdr); + do_scsi_cmd(req); return 0; case VIRTIO_BLK_T_FLUSH: - do_flush_cmd(s, elem, inhdr); + do_flush_cmd(req); return 0; case VIRTIO_BLK_T_GET_ID: - do_get_id_cmd(s, in_iov, in_num, elem, inhdr); + do_get_id_cmd(req, in_iov, in_num); return 0; default: - error_report("virtio-blk unsupported request type %#x", outhdr.type); + error_report("virtio-blk unsupported request type %#x", req->out.type); + g_slice_free(VirtIOBlockReq, req); return -EFAULT; } } From ee17e84830e2e7030d57db5b415719e9022573cd Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:50 +0800 Subject: [PATCH 29/47] virtio-blk: Fix and clean up the in_sg and out_sg check out_sg is checked by iov_to_buf below, so it can be dropped. Add assert and iov_discard_back around in_sg, as the in_sg is handled in dataplane code. Signed-off-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 05610959a6..4b1aeab525 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -345,7 +345,9 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) { uint32_t type; + struct iovec *in_iov = req->elem->in_sg; struct iovec *iov = req->elem->out_sg; + unsigned in_num = req->elem->in_num; unsigned out_num = req->elem->out_num; if (req->elem->out_num < 1 || req->elem->in_num < 1) { @@ -353,19 +355,24 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, exit(1); } - if (req->elem->out_sg[0].iov_len < sizeof(req->out) || - req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) { - error_report("virtio-blk header not in correct element"); - exit(1); - } - if (unlikely(iov_to_buf(iov, out_num, 0, &req->out, sizeof(req->out)) != sizeof(req->out))) { error_report("virtio-blk request outhdr too short"); exit(1); } + iov_discard_front(&iov, &out_num, sizeof(req->out)); - req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base; + + if (in_num < 1 || + in_iov[in_num - 1].iov_len < sizeof(struct virtio_blk_inhdr)) { + error_report("virtio-blk request inhdr too short"); + exit(1); + } + + req->in = (void *)in_iov[in_num - 1].iov_base + + in_iov[in_num - 1].iov_len + - sizeof(struct virtio_blk_inhdr); + iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr)); type = ldl_p(&req->out.type); From ac46821f2c6eb0617ac911daff111cbc30a4c40c Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 17 Jun 2014 14:32:04 +0800 Subject: [PATCH 30/47] block: make bdrv_query_stats() static This function is only called from block/qapi.c. There is no need to keep it public. Signed-off-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/qapi.c | 2 +- include/block/qapi.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index 97e16418ef..aeabaaf85c 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -293,7 +293,7 @@ void bdrv_query_info(BlockDriverState *bs, qapi_free_BlockInfo(info); } -BlockStats *bdrv_query_stats(const BlockDriverState *bs) +static BlockStats *bdrv_query_stats(const BlockDriverState *bs) { BlockStats *s; diff --git a/include/block/qapi.h b/include/block/qapi.h index e92c00daf6..03745464d6 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -39,7 +39,6 @@ void bdrv_query_image_info(BlockDriverState *bs, void bdrv_query_info(BlockDriverState *bs, BlockInfo **p_info, Error **errp); -BlockStats *bdrv_query_stats(const BlockDriverState *bs); void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f, QEMUSnapshotInfo *sn); From 13344f3a17e0a785c0eb8e36f69518f21aa8a91a Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 17 Jun 2014 14:32:05 +0800 Subject: [PATCH 31/47] block: acquire AioContext in qmp_query_blockstats() Make query-blockstats safe for dataplane by acquiring the BlockDriverState's AioContext. This ensures that the dataplane IOThread and the main loop's monitor code do not race. Note the assumption that acquiring the drive's BDS AioContext also protects ->file and ->backing_hd. This assumption is made by other aio_context_acquire() callers too. Signed-off-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/qapi.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/block/qapi.c b/block/qapi.c index aeabaaf85c..f44f6b4012 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -360,7 +360,11 @@ BlockStatsList *qmp_query_blockstats(Error **errp) while ((bs = bdrv_next(bs))) { BlockStatsList *info = g_malloc0(sizeof(*info)); + AioContext *ctx = bdrv_get_aio_context(bs); + + aio_context_acquire(ctx); info->value = bdrv_query_stats(bs); + aio_context_release(ctx); *p_next = info; p_next = &info->next; From bf4bd461b43d90c5af30f61f740c1bb675849ab9 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 17 Jun 2014 14:32:06 +0800 Subject: [PATCH 32/47] virtio-blk: Make request completion function virtual virtio_blk_req_complete will call VirtIOBlock.complete_request() to push data and notify guest. No functional change. Later, this will allow dataplane to provide it's own (vring_) version. Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 9 ++++++++- include/hw/virtio/virtio-blk.h | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 4b1aeab525..9975ad0324 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -44,7 +44,8 @@ static void virtio_blk_free_request(VirtIOBlockReq *req) } } -static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) +static void virtio_blk_complete_request(VirtIOBlockReq *req, + unsigned char status) { VirtIOBlock *s = req->dev; VirtIODevice *vdev = VIRTIO_DEVICE(s); @@ -56,6 +57,11 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) virtio_notify(vdev, s->vq); } +static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) +{ + req->dev->complete_request(req, status); +} + static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, bool is_read) { @@ -740,6 +746,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1; s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output); + s->complete_request = virtio_blk_complete_request; #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE virtio_blk_data_plane_create(vdev, blk, &s->dataplane, &err); if (err != NULL) { diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 2571e961ec..0398f4c46d 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -117,6 +117,7 @@ struct VirtIOBlkConf struct VirtIOBlockDataPlane; +struct VirtIOBlockReq; typedef struct VirtIOBlock { VirtIODevice parent_obj; BlockDriverState *bs; @@ -128,6 +129,8 @@ typedef struct VirtIOBlock { unsigned short sector_mask; bool original_wce; VMChangeStateEntry *change; + /* Function to push to vq and notify guest */ + void (*complete_request)(struct VirtIOBlockReq *req, unsigned char status); #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE Notifier migration_state_notifier; struct VirtIOBlockDataPlane *dataplane; From fee65db77181e6697745b313906bc4fdb30d2ff9 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 17 Jun 2014 14:32:07 +0800 Subject: [PATCH 33/47] virtio-blk: Export request handling functions to dataplane So that dataplane can use virtio_blk_handle_request and virtio_submit_multiwrite. Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 10 ++-------- include/hw/virtio/virtio-blk.h | 9 +++++++++ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 9975ad0324..77fb4477c6 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -253,12 +253,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) virtio_blk_free_request(req); } -typedef struct MultiReqBuffer { - BlockRequest blkreq[32]; - unsigned int num_writes; -} MultiReqBuffer; - -static void virtio_submit_multiwrite(BlockDriverState *bs, MultiReqBuffer *mrb) +void virtio_submit_multiwrite(BlockDriverState *bs, MultiReqBuffer *mrb) { int i, ret; @@ -347,8 +342,7 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req) virtio_blk_rw_complete, req); } -static void virtio_blk_handle_request(VirtIOBlockReq *req, - MultiReqBuffer *mrb) +void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) { uint32_t type; struct iovec *in_iov = req->elem->in_sg; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 0398f4c46d..d0fb26f963 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -137,6 +137,11 @@ typedef struct VirtIOBlock { #endif } VirtIOBlock; +typedef struct MultiReqBuffer { + BlockRequest blkreq[32]; + unsigned int num_writes; +} MultiReqBuffer; + typedef struct VirtIOBlockReq { VirtIOBlock *dev; VirtQueueElement *elem; @@ -172,4 +177,8 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk); int virtio_blk_handle_scsi_req(VirtIOBlock *blk, VirtQueueElement *elem); +void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb); + +void virtio_submit_multiwrite(BlockDriverState *bs, MultiReqBuffer *mrb); + #endif From 4407c1c56adb0d3ef2bcbf577592d72278d6e11f Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 17 Jun 2014 14:32:08 +0800 Subject: [PATCH 34/47] virtio-blk: Schedule BH in the right context The BH must be called in the AioContext of bs. Currently it is only the main loop, but with coming changes, it could also be a dataplane IOThread. Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 77fb4477c6..a222e3f9a4 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -469,7 +469,8 @@ static void virtio_blk_dma_restart_cb(void *opaque, int running, } if (!s->bh) { - s->bh = qemu_bh_new(virtio_blk_dma_restart_bh, s); + s->bh = aio_bh_new(bdrv_get_aio_context(s->blk.conf.bs), + virtio_blk_dma_restart_bh, s); qemu_bh_schedule(s->bh); } } From b002254dbd4c19a01f29790f840f983803e26893 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 17 Jun 2014 14:32:09 +0800 Subject: [PATCH 35/47] virtio-blk: Unify {non-,}dataplane's request handlings This drops request handling code from dataplane, and uses code from hw/block/virtio-blk.c. It starts to use multiwrite as non-dataplane does. Dataplane sets VirtIOBlock.complete_request to vring version, and calls into non-dataplane's process handling. In complete_request_early, qiov.size is added to vring push length, because it's also called in rw completion now. Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 183 ++++---------------------------- 1 file changed, 19 insertions(+), 164 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 29e7ec3071..b6afa55b96 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -47,6 +47,8 @@ struct VirtIOBlockDataPlane { /* Operation blocker on BDS */ Error *blocker; + void (*saved_complete_request)(struct VirtIOBlockReq *req, + unsigned char status); }; /* Raise an interrupt to signal guest, if necessary */ @@ -59,179 +61,27 @@ static void notify_guest(VirtIOBlockDataPlane *s) event_notifier_set(s->guest_notifier); } -static void complete_rdwr(void *opaque, int ret) -{ - VirtIOBlockReq *req = opaque; - struct virtio_blk_inhdr hdr; - int len; - - if (likely(ret == 0)) { - hdr.status = VIRTIO_BLK_S_OK; - len = req->qiov.size; - } else { - hdr.status = VIRTIO_BLK_S_IOERR; - len = 0; - } - - trace_virtio_blk_data_plane_complete_request(req->dev->dataplane, - req->elem->index, ret); - - stb_p(&req->in->status, hdr.status); - - /* According to the virtio specification len should be the number of bytes - * written to, but for virtio-blk it seems to be the number of bytes - * transferred plus the status bytes. - */ - vring_push(&req->dev->dataplane->vring, req->elem, len + sizeof(hdr)); - notify_guest(req->dev->dataplane); - g_slice_free(VirtIOBlockReq, req); -} - static void complete_request_early(VirtIOBlockReq *req, unsigned char status) { stb_p(&req->in->status, status); - vring_push(&req->dev->dataplane->vring, req->elem, sizeof(*req->in)); + vring_push(&req->dev->dataplane->vring, req->elem, + req->qiov.size + sizeof(*req->in)); notify_guest(req->dev->dataplane); g_slice_free(VirtIOBlockReq, req); } -/* Get disk serial number */ -static void do_get_id_cmd(VirtIOBlockReq *req, - struct iovec *iov, unsigned int iov_cnt) -{ - char id[VIRTIO_BLK_ID_BYTES]; - - /* Serial number not NUL-terminated when longer than buffer */ - strncpy(id, req->dev->blk.serial ? req->dev->blk.serial : "", sizeof(id)); - iov_from_buf(iov, iov_cnt, 0, id, sizeof(id)); - complete_request_early(req, VIRTIO_BLK_S_OK); -} - - -static void do_rdwr_cmd(bool read, VirtIOBlockReq *req, - struct iovec *iov, unsigned iov_cnt) -{ - QEMUIOVector *qiov; - int nb_sectors; - int64_t sector_num; - - qemu_iovec_init_external(&req->qiov, iov, iov_cnt); - - qiov = &req->qiov; - - sector_num = req->out.sector * 512 / BDRV_SECTOR_SIZE; - nb_sectors = qiov->size / BDRV_SECTOR_SIZE; - - if (read) { - bdrv_aio_readv(req->dev->blk.conf.bs, - sector_num, qiov, nb_sectors, - complete_rdwr, req); - } else { - bdrv_aio_writev(req->dev->blk.conf.bs, - sector_num, qiov, nb_sectors, - complete_rdwr, req); - } -} - -static void complete_flush(void *opaque, int ret) -{ - VirtIOBlockReq *req = opaque; - unsigned char status; - - if (ret == 0) { - status = VIRTIO_BLK_S_OK; - } else { - status = VIRTIO_BLK_S_IOERR; - } - - complete_request_early(req, status); -} - -static void do_flush_cmd(VirtIOBlockReq *req) -{ - - bdrv_aio_flush(req->dev->blk.conf.bs, complete_flush, req); -} - -static void do_scsi_cmd(VirtIOBlockReq *req) -{ - int status; - - status = virtio_blk_handle_scsi_req(req->dev, req->elem); - complete_request_early(req, status); -} - -static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) -{ - struct iovec *iov = elem->out_sg; - struct iovec *in_iov = elem->in_sg; - unsigned out_num = elem->out_num; - unsigned in_num = elem->in_num; - VirtIOBlockReq *req; - - req = g_slice_new(VirtIOBlockReq); - req->dev = VIRTIO_BLK(s->vdev); - req->elem = elem; - /* Copy in outhdr */ - if (unlikely(iov_to_buf(iov, out_num, 0, &req->out, - sizeof(req->out)) != sizeof(req->out))) { - error_report("virtio-blk request outhdr too short"); - g_slice_free(VirtIOBlockReq, req); - return -EFAULT; - } - iov_discard_front(&iov, &out_num, sizeof(req->out)); - - /* We are likely safe with the iov_len check, because inhdr is only 1 byte, - * but checking here in case the header gets bigger in the future. */ - if (in_num < 1 || in_iov[in_num - 1].iov_len < sizeof(*req->in)) { - error_report("virtio-blk request inhdr too short"); - return -EFAULT; - } - - /* Grab inhdr for later */ - req->in = (void *)in_iov[in_num - 1].iov_base - + in_iov[in_num - 1].iov_len - sizeof(*req->in); - iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr)); - - /* TODO Linux sets the barrier bit even when not advertised! */ - req->out.type &= ~VIRTIO_BLK_T_BARRIER; - - switch (req->out.type) { - case VIRTIO_BLK_T_IN: - do_rdwr_cmd(true, req, in_iov, in_num); - return 0; - - case VIRTIO_BLK_T_OUT: - do_rdwr_cmd(false, req, iov, out_num); - return 0; - - case VIRTIO_BLK_T_SCSI_CMD: - do_scsi_cmd(req); - return 0; - - case VIRTIO_BLK_T_FLUSH: - do_flush_cmd(req); - return 0; - - case VIRTIO_BLK_T_GET_ID: - do_get_id_cmd(req, in_iov, in_num); - return 0; - - default: - error_report("virtio-blk unsupported request type %#x", req->out.type); - g_slice_free(VirtIOBlockReq, req); - return -EFAULT; - } -} - static void handle_notify(EventNotifier *e) { VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane, host_notifier); VirtQueueElement *elem; + VirtIOBlockReq *req; int ret; + MultiReqBuffer mrb = { + .num_writes = 0, + }; event_notifier_test_and_clear(&s->host_notifier); for (;;) { @@ -248,14 +98,14 @@ static void handle_notify(EventNotifier *e) trace_virtio_blk_data_plane_process_request(s, elem->out_num, elem->in_num, elem->index); - if (process_request(s, elem) < 0) { - vring_set_broken(&s->vring); - vring_free_element(elem); - ret = -EFAULT; - break; - } + req = g_slice_new(VirtIOBlockReq); + req->dev = VIRTIO_BLK(s->vdev); + req->elem = elem; + virtio_blk_handle_request(req, &mrb); } + virtio_submit_multiwrite(s->blk->conf.bs, &mrb); + if (likely(ret == -EAGAIN)) { /* vring emptied */ /* Re-enable guest->host notifies and stop processing the vring. * But if the guest has snuck in more descriptors, keep processing. @@ -275,6 +125,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, Error **errp) { VirtIOBlockDataPlane *s; + VirtIOBlock *vblk = VIRTIO_BLK(vdev); Error *local_err = NULL; *dataplane = NULL; @@ -317,6 +168,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, bdrv_op_block_all(blk->conf.bs, s->blocker); *dataplane = s; + s->saved_complete_request = vblk->complete_request; + vblk->complete_request = complete_request_early; } /* Context: QEMU global mutex held */ @@ -391,10 +244,12 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s) { BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev))); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + VirtIOBlock *vblk = VIRTIO_BLK(s->vdev); if (!s->started || s->stopping) { return; } s->stopping = true; + vblk->complete_request = s->saved_complete_request; trace_virtio_blk_data_plane_stop(s); aio_context_acquire(s->ctx); From d64c60a75fde0fc40f7c97acbc972dea59db0162 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 17 Jun 2014 14:32:10 +0800 Subject: [PATCH 36/47] virtio-blk: Rename complete_request_early to complete_request_vring The old name is misleading in its new usage, so rename it. Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index b6afa55b96..09bd2c70ab 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -61,7 +61,7 @@ static void notify_guest(VirtIOBlockDataPlane *s) event_notifier_set(s->guest_notifier); } -static void complete_request_early(VirtIOBlockReq *req, unsigned char status) +static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) { stb_p(&req->in->status, status); @@ -169,7 +169,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, *dataplane = s; s->saved_complete_request = vblk->complete_request; - vblk->complete_request = complete_request_early; + vblk->complete_request = complete_request_vring; } /* Context: QEMU global mutex held */ From 518848a214ff67bbaea087552a709afde4f88fa5 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 27 Jun 2014 19:24:13 +0200 Subject: [PATCH 37/47] blockjob: Fix recent BLOCK_JOB_READY regression Commit bcada37 dropped the (up to now undocumented) members type, len, offset, speed, breaking tests/qemu-iotests/040 and 041. Restore and document them. This fixes 040, and partially fixes 041. Signed-off-by: Markus Armbruster Tested-By: Benoit Canet Signed-off-by: Kevin Wolf --- blockjob.c | 6 +++++- qapi/block-core.json | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/blockjob.c b/blockjob.c index a6db01e953..a32c1c8dd7 100644 --- a/blockjob.c +++ b/blockjob.c @@ -270,7 +270,11 @@ void block_job_event_completed(BlockJob *job, const char *msg) void block_job_event_ready(BlockJob *job) { - qapi_event_send_block_job_ready(bdrv_get_device_name(job->bs), &error_abort); + qapi_event_send_block_job_ready(job->driver->job_type, + bdrv_get_device_name(job->bs), + job->len, + job->offset, + job->speed, &error_abort); } BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs, diff --git a/qapi/block-core.json b/qapi/block-core.json index a46cdbe6aa..6f41f84c44 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1558,12 +1558,25 @@ # # Emitted when a block job is ready to complete # +# @type: job type +# # @device: device name # +# @len: maximum progress value +# +# @offset: current progress value. On success this is equal to len. +# On failure this is less than len +# +# @speed: rate limit, bytes per second +# # Note: The "ready to complete" status is always reset by a @BLOCK_JOB_ERROR # event # # Since: 1.3 ## { 'event': 'BLOCK_JOB_READY', - 'data': { 'device': 'str' } } + 'data': { 'type' : 'BlockJobType', + 'device': 'str', + 'len' : 'int', + 'offset': 'int', + 'speed' : 'int' } } From 823c686356e6758bacb46d3a316b841536d6d707 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 27 Jun 2014 19:24:14 +0200 Subject: [PATCH 38/47] blockjob: Fix recent BLOCK_JOB_ERROR regression Commit 5a2d2cb screwed up the the value of members device and action, breaking tests/qemu-iotests/041. Signed-off-by: Markus Armbruster Tested-By: Benoit Canet Reviewed-by: Kevin Wolf Reviewed-by: Luiz Capitulino Signed-off-by: Kevin Wolf --- blockjob.c | 2 +- qapi/block-core.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/blockjob.c b/blockjob.c index a32c1c8dd7..67a64ea318 100644 --- a/blockjob.c +++ b/blockjob.c @@ -300,7 +300,7 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs, default: abort(); } - qapi_event_send_block_job_error(bdrv_get_device_name(bs), + qapi_event_send_block_job_error(bdrv_get_device_name(job->bs), is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE, action, &error_abort); diff --git a/qapi/block-core.json b/qapi/block-core.json index 6f41f84c44..ff7224f647 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1551,7 +1551,7 @@ { 'event': 'BLOCK_JOB_ERROR', 'data': { 'device' : 'str', 'operation': 'IoOperationType', - 'action' : 'BlockdevOnError' } } + 'action' : 'BlockErrorAction' } } ## # @BLOCK_JOB_READY From 09158f00e0fdb506dcbf36f67c615b7f6c604c5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 27 Jun 2014 18:25:25 +0200 Subject: [PATCH 39/47] block: Add replaces argument to drive-mirror drive-mirror will bdrv_swap the new BDS named node-name with the one pointed by replaces when the mirroring is finished. Signed-off-by: Benoit Canet Signed-off-by: Kevin Wolf --- block.c | 25 ++++++++++++++++ block/mirror.c | 60 ++++++++++++++++++++++++++++++--------- blockdev.c | 31 +++++++++++++++++++- hmp.c | 2 +- include/block/block.h | 4 +++ include/block/block_int.h | 3 ++ qapi/block-core.json | 6 +++- qmp-commands.hx | 4 ++- 8 files changed, 118 insertions(+), 17 deletions(-) diff --git a/block.c b/block.c index 106238d0b7..9ecadf002f 100644 --- a/block.c +++ b/block.c @@ -5766,3 +5766,28 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate) return false; } + +BlockDriverState *check_to_replace_node(const char *node_name, Error **errp) +{ + BlockDriverState *to_replace_bs = bdrv_find_node(node_name); + if (!to_replace_bs) { + error_setg(errp, "Node name '%s' not found", node_name); + return NULL; + } + + if (bdrv_op_is_blocked(to_replace_bs, BLOCK_OP_TYPE_REPLACE, errp)) { + return NULL; + } + + /* We don't want arbitrary node of the BDS chain to be replaced only the top + * most non filter in order to prevent data corruption. + * Another benefit is that this tests exclude backing files which are + * blocked by the backing blockers. + */ + if (!bdrv_is_first_non_filter(to_replace_bs)) { + error_setg(errp, "Only top most non filter can be replaced"); + return NULL; + } + + return to_replace_bs; +} diff --git a/block/mirror.c b/block/mirror.c index 7c9f898089..6c3ee7041c 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -32,6 +32,12 @@ typedef struct MirrorBlockJob { RateLimit limit; BlockDriverState *target; BlockDriverState *base; + /* The name of the graph node to replace */ + char *replaces; + /* The BDS to replace */ + BlockDriverState *to_replace; + /* Used to block operations on the drive-mirror-replace target */ + Error *replace_blocker; bool is_none_mode; BlockdevOnError on_source_error, on_target_error; bool synced; @@ -500,10 +506,14 @@ immediate_exit: bdrv_release_dirty_bitmap(bs, s->dirty_bitmap); bdrv_iostatus_disable(s->target); if (s->should_complete && ret == 0) { - if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) { - bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL); + BlockDriverState *to_replace = s->common.bs; + if (s->to_replace) { + to_replace = s->to_replace; } - bdrv_swap(s->target, s->common.bs); + if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) { + bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL); + } + bdrv_swap(s->target, to_replace); if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) { /* drop the bs loop chain formed by the swap: break the loop then * trigger the unref from the top one */ @@ -512,6 +522,12 @@ immediate_exit: bdrv_unref(p); } } + if (s->to_replace) { + bdrv_op_unblock_all(s->to_replace, s->replace_blocker); + error_free(s->replace_blocker); + bdrv_unref(s->to_replace); + } + g_free(s->replaces); bdrv_unref(s->target); block_job_completed(&s->common, ret); } @@ -550,6 +566,20 @@ static void mirror_complete(BlockJob *job, Error **errp) return; } + /* check the target bs is not blocked and block all operations on it */ + if (s->replaces) { + s->to_replace = check_to_replace_node(s->replaces, &local_err); + if (!s->to_replace) { + error_propagate(errp, local_err); + return; + } + + error_setg(&s->replace_blocker, + "block device is in use by block-job-complete"); + bdrv_op_block_all(s->to_replace, s->replace_blocker); + bdrv_ref(s->to_replace); + } + s->should_complete = true; block_job_resume(job); } @@ -572,14 +602,15 @@ static const BlockJobDriver commit_active_job_driver = { }; static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, - int64_t speed, int64_t granularity, - int64_t buf_size, - BlockdevOnError on_source_error, - BlockdevOnError on_target_error, - BlockDriverCompletionFunc *cb, - void *opaque, Error **errp, - const BlockJobDriver *driver, - bool is_none_mode, BlockDriverState *base) + const char *replaces, + int64_t speed, int64_t granularity, + int64_t buf_size, + BlockdevOnError on_source_error, + BlockdevOnError on_target_error, + BlockDriverCompletionFunc *cb, + void *opaque, Error **errp, + const BlockJobDriver *driver, + bool is_none_mode, BlockDriverState *base) { MirrorBlockJob *s; @@ -610,6 +641,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, return; } + s->replaces = g_strdup(replaces); s->on_source_error = on_source_error; s->on_target_error = on_target_error; s->target = target; @@ -631,6 +663,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, } void mirror_start(BlockDriverState *bs, BlockDriverState *target, + const char *replaces, int64_t speed, int64_t granularity, int64_t buf_size, MirrorSyncMode mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, @@ -642,7 +675,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, is_none_mode = mode == MIRROR_SYNC_MODE_NONE; base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL; - mirror_start_job(bs, target, speed, granularity, buf_size, + mirror_start_job(bs, target, replaces, + speed, granularity, buf_size, on_source_error, on_target_error, cb, opaque, errp, &mirror_job_driver, is_none_mode, base); } @@ -690,7 +724,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, } bdrv_ref(base); - mirror_start_job(bs, base, speed, 0, 0, + mirror_start_job(bs, base, NULL, speed, 0, 0, on_error, on_error, cb, opaque, &local_err, &commit_active_job_driver, false, base); if (local_err) { diff --git a/blockdev.c b/blockdev.c index 943301226d..69b7c2a8c5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2100,6 +2100,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp) void qmp_drive_mirror(const char *device, const char *target, bool has_format, const char *format, bool has_node_name, const char *node_name, + bool has_replaces, const char *replaces, enum MirrorSyncMode sync, bool has_mode, enum NewImageMode mode, bool has_speed, int64_t speed, @@ -2187,6 +2188,29 @@ void qmp_drive_mirror(const char *device, const char *target, return; } + if (has_replaces) { + BlockDriverState *to_replace_bs; + + if (!has_node_name) { + error_setg(errp, "a node-name must be provided when replacing a" + " named node of the graph"); + return; + } + + to_replace_bs = check_to_replace_node(replaces, &local_err); + + if (!to_replace_bs) { + error_propagate(errp, local_err); + return; + } + + if (size != bdrv_getlength(to_replace_bs)) { + error_setg(errp, "cannot replace image with a mirror image of " + "different size"); + return; + } + } + if ((sync == MIRROR_SYNC_MODE_FULL || !source) && mode != NEW_IMAGE_MODE_EXISTING) { @@ -2231,7 +2255,12 @@ void qmp_drive_mirror(const char *device, const char *target, return; } - mirror_start(bs, target_bs, speed, granularity, buf_size, sync, + /* pass the node name to replace to mirror start since it's loose coupling + * and will allow to check whether the node still exist at mirror completion + */ + mirror_start(bs, target_bs, + has_replaces ? replaces : NULL, + speed, granularity, buf_size, sync, on_source_error, on_target_error, block_job_cb, bs, &local_err); if (local_err != NULL) { diff --git a/hmp.c b/hmp.c index 73acc3283c..6429e6b447 100644 --- a/hmp.c +++ b/hmp.c @@ -933,7 +933,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict) } qmp_drive_mirror(device, filename, !!format, format, - false, NULL, + false, NULL, false, NULL, full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP, true, mode, false, 0, false, 0, false, 0, false, 0, false, 0, &err); diff --git a/include/block/block.h b/include/block/block.h index d0baf4fb83..b53833d1d6 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -175,6 +175,7 @@ typedef enum BlockOpType { BLOCK_OP_TYPE_MIRROR, BLOCK_OP_TYPE_RESIZE, BLOCK_OP_TYPE_STREAM, + BLOCK_OP_TYPE_REPLACE, BLOCK_OP_TYPE_MAX, } BlockOpType; @@ -314,6 +315,9 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs, BlockDriverState *candidate); bool bdrv_is_first_non_filter(BlockDriverState *candidate); +/* check if a named node can be replaced when doing drive-mirror */ +BlockDriverState *check_to_replace_node(const char *node_name, Error **errp); + /* async block I/O */ typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector, int sector_num); diff --git a/include/block/block_int.h b/include/block/block_int.h index 135c5dc0e9..53e77cf11e 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -489,6 +489,8 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, * mirror_start: * @bs: Block device to operate on. * @target: Block device to write to. + * @replaces: Block graph node name to replace once the mirror is done. Can + * only be used when full mirroring is selected. * @speed: The maximum speed, in bytes per second, or 0 for unlimited. * @granularity: The chosen granularity for the dirty bitmap. * @buf_size: The amount of data that can be in flight at one time. @@ -505,6 +507,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, * @bs will be switched to read from @target. */ void mirror_start(BlockDriverState *bs, BlockDriverState *target, + const char *replaces, int64_t speed, int64_t granularity, int64_t buf_size, MirrorSyncMode mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, diff --git a/qapi/block-core.json b/qapi/block-core.json index ff7224f647..406ce03951 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -768,6 +768,10 @@ # @node-name: #optional the new block driver state node name in the graph # (Since 2.1) # +# @replaces: #optional with sync=full graph node name to be replaced by the new +# image when a whole image copy is done. This can be used to repair +# broken Quorum files. (Since 2.1) +# # @mode: #optional whether and how QEMU should create a new image, default is # 'absolute-paths'. # @@ -800,7 +804,7 @@ ## { 'command': 'drive-mirror', 'data': { 'device': 'str', 'target': 'str', '*format': 'str', - '*node-name': 'str', + '*node-name': 'str', '*replaces': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', '*speed': 'int', '*granularity': 'uint32', '*buf-size': 'int', '*on-source-error': 'BlockdevOnError', diff --git a/qmp-commands.hx b/qmp-commands.hx index 5254938878..d342b8a067 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1293,7 +1293,7 @@ EQMP { .name = "drive-mirror", .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," - "node-name:s?," + "node-name:s?,replaces:s?," "on-source-error:s?,on-target-error:s?," "granularity:i?,buf-size:i?", .mhandler.cmd_new = qmp_marshal_input_drive_mirror, @@ -1317,6 +1317,8 @@ Arguments: - "format": format of new image (json-string, optional) - "node-name": the name of the new block driver state in the node graph (json-string, optional) +- "replaces": the block driver node name to replace when finished + (json-string, optional) - "mode": how an image file should be created into the target file/device (NewImageMode, optional, default 'absolute-paths') - "speed": maximum speed of the streaming job, in bytes per second From d88964aeda6cfc44585fe35c97c37537235c0a96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 27 Jun 2014 18:25:26 +0200 Subject: [PATCH 40/47] qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror node-name mode. The to-replace-node-name is designed to allow repairing a broken Quorum file. This patch introduces a new class TestRepairQuorum testing that the feature works. Some further work will be done on QEMU to improve the robustness of the tests. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/041 | 196 ++++++++++++++++++++++++++++++++++++- tests/qemu-iotests/041.out | 4 +- 2 files changed, 194 insertions(+), 6 deletions(-) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index ef4f4657cc..0815e19274 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -28,6 +28,12 @@ target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img') test_img = os.path.join(iotests.test_dir, 'test.img') target_img = os.path.join(iotests.test_dir, 'target.img') +quorum_img1 = os.path.join(iotests.test_dir, 'quorum1.img') +quorum_img2 = os.path.join(iotests.test_dir, 'quorum2.img') +quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img') +quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img') +quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img') + class ImageMirroringTestCase(iotests.QMPTestCase): '''Abstract base class for image mirroring test cases''' @@ -42,8 +48,8 @@ class ImageMirroringTestCase(iotests.QMPTestCase): ready = True def wait_ready_and_cancel(self, drive='drive0'): - self.wait_ready(drive) - event = self.cancel_and_wait() + self.wait_ready(drive=drive) + event = self.cancel_and_wait(drive=drive) self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED') self.assert_qmp(event, 'data/type', 'mirror') self.assert_qmp(event, 'data/offset', self.image_len) @@ -52,12 +58,12 @@ class ImageMirroringTestCase(iotests.QMPTestCase): def complete_and_wait(self, drive='drive0', wait_ready=True): '''Complete a block job and wait for it to finish''' if wait_ready: - self.wait_ready() + self.wait_ready(drive=drive) result = self.vm.qmp('block-job-complete', device=drive) self.assert_qmp(result, 'return', {}) - event = self.wait_until_completed() + event = self.wait_until_completed(drive=drive) self.assert_qmp(event, 'data/type', 'mirror') class TestSingleDrive(ImageMirroringTestCase): @@ -723,5 +729,187 @@ class TestUnbackedSource(ImageMirroringTestCase): self.complete_and_wait() self.assert_no_active_block_jobs() +class TestRepairQuorum(ImageMirroringTestCase): + """ This class test quorum file repair using drive-mirror. + It's mostly a fork of TestSingleDrive """ + image_len = 1 * 1024 * 1024 # MB + IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ] + + def setUp(self): + self.vm = iotests.VM() + + # Add each individual quorum images + for i in self.IMAGES: + qemu_img('create', '-f', iotests.imgfmt, i, + str(TestSingleDrive.image_len)) + # Assign a node name to each quorum image in order to manipulate + # them + opts = "node-name=img%i" % self.IMAGES.index(i) + self.vm = self.vm.add_drive(i, opts) + + self.vm.launch() + + #assemble the quorum block device from the individual files + args = { "options" : { "driver": "quorum", "id": "quorum0", + "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] } } + result = self.vm.qmp("blockdev-add", **args) + self.assert_qmp(result, 'return', {}) + + + def tearDown(self): + self.vm.shutdown() + for i in self.IMAGES + [ quorum_repair_img ]: + # Do a try/except because the test may have deleted some images + try: + os.remove(i) + except OSError: + pass + + def test_complete(self): + self.assert_no_active_block_jobs() + + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full', + node_name="repair0", + replaces="img1", + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + self.complete_and_wait(drive="quorum0") + result = self.vm.qmp('query-named-block-nodes') + self.assert_qmp(result, 'return[0]/file', quorum_repair_img) + # TODO: a better test requiring some QEMU infrastructure will be added + # to check that this file is really driven by quorum + self.vm.shutdown() + self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img), + 'target image does not match source after mirroring') + + def test_cancel(self): + self.assert_no_active_block_jobs() + + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full', + node_name="repair0", + replaces="img1", + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + self.cancel_and_wait(drive="quorum0", force=True) + # here we check that the last registered quorum file has not been + # swapped out and unref + result = self.vm.qmp('query-named-block-nodes') + self.assert_qmp(result, 'return[0]/file', quorum_img3) + self.vm.shutdown() + + def test_cancel_after_ready(self): + self.assert_no_active_block_jobs() + + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full', + node_name="repair0", + replaces="img1", + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + self.wait_ready_and_cancel(drive="quorum0") + result = self.vm.qmp('query-named-block-nodes') + # here we check that the last registered quorum file has not been + # swapped out and unref + self.assert_qmp(result, 'return[0]/file', quorum_img3) + self.vm.shutdown() + self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img), + 'target image does not match source after mirroring') + + def test_pause(self): + self.assert_no_active_block_jobs() + + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full', + node_name="repair0", + replaces="img1", + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('block-job-pause', device='quorum0') + self.assert_qmp(result, 'return', {}) + + time.sleep(1) + result = self.vm.qmp('query-block-jobs') + offset = self.dictpath(result, 'return[0]/offset') + + time.sleep(1) + result = self.vm.qmp('query-block-jobs') + self.assert_qmp(result, 'return[0]/offset', offset) + + result = self.vm.qmp('block-job-resume', device='quorum0') + self.assert_qmp(result, 'return', {}) + + self.complete_and_wait(drive="quorum0") + self.vm.shutdown() + self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img), + 'target image does not match source after mirroring') + + def test_medium_not_found(self): + result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full', + node_name='repair0', + replaces='img1', + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'error/class', 'GenericError') + + def test_image_not_found(self): + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full', + node_name='repair0', + replaces='img1', + mode='existing', + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'error/class', 'GenericError') + + def test_device_not_found(self): + result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full', + node_name='repair0', + replaces='img1', + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'error/class', 'DeviceNotFound') + + def test_wrong_sync_mode(self): + result = self.vm.qmp('drive-mirror', device='quorum0', + node_name='repair0', + replaces='img1', + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'error/class', 'GenericError') + + def test_no_node_name(self): + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full', + replaces='img1', + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'error/class', 'GenericError') + + def test_unexistant_replaces(self): + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full', + node_name='repair0', + replaces='img77', + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'error/class', 'GenericError') + + def test_after_a_quorum_snapshot(self): + result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1', + snapshot_file=quorum_snapshot_file, + snapshot_node_name="snap1"); + + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full', + node_name='repair0', + replaces="img1", + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'error/class', 'GenericError') + + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full', + node_name='repair0', + replaces="snap1", + target=quorum_repair_img, format=iotests.imgfmt) + self.assert_qmp(result, 'return', {}) + + self.complete_and_wait(drive="quorum0") + result = self.vm.qmp('query-named-block-nodes') + self.assert_qmp(result, 'return[0]/file', quorum_repair_img) + # TODO: a better test requiring some QEMU infrastructure will be added + # to check that this file is really driven by quorum + self.vm.shutdown() + if __name__ == '__main__': iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out index cfa5c0d0e6..42147c0b58 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ -................................... +.............................................. ---------------------------------------------------------------------- -Ran 35 tests +Ran 46 tests OK From 6b8aeca574a15668c47296d8e0c4f96c72e63c36 Mon Sep 17 00:00:00 2001 From: Chen Gang Date: Mon, 23 Jun 2014 23:28:23 +0800 Subject: [PATCH 41/47] block.c: Don't return success for bdrv_append_temp_snapshot() failure When failure occurs, 'ret' need be set, or may return 0 to indicate success. Previously, an error was set in errp, but 0 was returned anyway. So let bdrv_append_temp_snapshot() return an error code and use that for the bdrv_open() return value. Also, error_propagate() need be called only one time within a function. Signed-off-by: Chen Gang Signed-off-by: Kevin Wolf --- block.c | 7 ++++--- include/block/block.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index 9ecadf002f..6856c18aa8 100644 --- a/block.c +++ b/block.c @@ -1278,7 +1278,7 @@ done: return ret; } -void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) +int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) { /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */ char *tmp_filename = g_malloc0(PATH_MAX + 1); @@ -1296,6 +1296,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) /* Get the required size from the image */ total_size = bdrv_getlength(bs); if (total_size < 0) { + ret = total_size; error_setg_errno(errp, -total_size, "Could not get image size"); goto out; } @@ -1342,6 +1343,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) out: g_free(tmp_filename); + return ret; } /* @@ -1491,9 +1493,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, /* For snapshot=on, create a temporary qcow2 overlay. bs points to the * temporary snapshot afterwards. */ if (snapshot_flags) { - bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err); + ret = bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err); if (local_err) { - error_propagate(errp, local_err); goto close_and_fail; } } diff --git a/include/block/block.h b/include/block/block.h index b53833d1d6..7e92f549fb 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -214,7 +214,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, bool allow_none, Error **errp); void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd); int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp); -void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp); +int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp); int bdrv_open(BlockDriverState **pbs, const char *filename, const char *reference, QDict *options, int flags, BlockDriver *drv, Error **errp); From e8f8624d3b920de968c56e76691f419b16d1ee4a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Sat, 24 May 2014 23:24:55 +0200 Subject: [PATCH 42/47] iotests: Allow out-of-tree run As out-of-tree builds are preferred for qemu, running the qemu-iotests in that out-of-tree build should be supported as well. To do so, a symbolic link has to be created pointing to the check script in the source directory. That script will check whether it has been run through a symlink, and if so, will assume it is run in the build tree. All output and temporary operations performed by iotests are then redirected here and, unless specified otherwise by the user, QEMU_PROG etc. will be set to paths appropriate for the build tree. Also, drop making every test case executable if it is not yet, as this would modify the source tree which is not desired for out-of-tree runs and should be fixed in the repository anyway. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/check | 100 ++++++++++++++++++++++++++----- tests/qemu-iotests/common | 11 ++-- tests/qemu-iotests/common.config | 2 +- tests/qemu-iotests/common.rc | 8 +-- tests/qemu-iotests/iotests.py | 3 +- 5 files changed, 96 insertions(+), 28 deletions(-) diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index e2ed5a95f8..69f328b57c 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -34,22 +34,89 @@ timestamp=${TIMESTAMP:=false} # generic initialization iam=check -# we need common.config -if ! . ./common.config -then - echo "$iam: failed to source common.config" +_init_error() +{ + echo "$iam: $1" >&2 exit 1 +} + +if [ -L "$0" ] +then + # called from the build tree + source_iotests=$(dirname "$(readlink "$0")") + if [ -z "$source_iotests" ] + then + _init_error "failed to obtain source tree name from check symlink" + fi + source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to enter source tree" + build_iotests=$PWD +else + # called from the source tree + source_iotests=$PWD + # this may be an in-tree build (note that in the following code we may not + # assume that it truly is and have to test whether the build results + # actually exist) + build_iotests=$PWD +fi + +build_root="$build_iotests/../.." + +if [ -x "$build_iotests/socket_scm_helper" ] +then + export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper" +fi + +# if ./qemu exists, it should be prioritized and will be chosen by common.config +if [[ -z "$QEMU_PROG" && ! -x './qemu' ]] +then + arch=$(uname -m 2> /dev/null) + + if [[ -n $arch && -x "$build_root/$arch-softmmu/qemu-system-$arch" ]] + then + export QEMU_PROG="$build_root/$arch-softmmu/qemu-system-$arch" + else + pushd "$build_root" > /dev/null + for binary in *-softmmu/qemu-system-* + do + if [ -x "$binary" ] + then + export QEMU_PROG="$build_root/$binary" + break + fi + done + popd > /dev/null + fi +fi + +if [[ -z $QEMU_IMG_PROG && -x "$build_root/qemu-img" && ! -x './qemu-img' ]] +then + export QEMU_IMG_PROG="$build_root/qemu-img" +fi + +if [[ -z $QEMU_IO_PROG && -x "$build_root/qemu-io" && ! -x './qemu-io' ]] +then + export QEMU_IO_PROG="$build_root/qemu-io" +fi + +if [[ -z $QEMU_NBD_PROG && -x "$build_root/qemu-nbd" && ! -x './qemu-nbd' ]] +then + export QEMU_NBD_PROG="$build_root/qemu-nbd" +fi + +# we need common.config +if ! . "$source_iotests/common.config" +then + _init_error "failed to source common.config" fi # we need common.rc -if ! . ./common.rc +if ! . "$source_iotests/common.rc" then - echo "check: failed to source common.rc" - exit 1 + _init_error "failed to source common.rc" fi # we need common -. ./common +. "$source_iotests/common" #if [ `id -u` -ne 0 ] #then @@ -194,7 +261,7 @@ do echo " - expunged" rm -f $seq.out.bad echo "/^$seq\$/d" >>$tmp.expunged - elif [ ! -f $seq ] + elif [ ! -f "$source_iotests/$seq" ] then echo " - no such test?" echo "/^$seq\$/d" >>$tmp.expunged @@ -215,9 +282,10 @@ do start=`_wallclock` $timestamp && echo -n " ["`date "+%T"`"]" - [ ! -x $seq ] && chmod u+x $seq # ensure we can run it + export OUTPUT_DIR=$PWD + (cd "$source_iotests"; MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \ - ./$seq >$tmp.out 2>&1 + ./$seq >$tmp.out 2>&1) sts=$? $timestamp && _timestamp stop=`_wallclock` @@ -242,17 +310,17 @@ do err=true fi - reference=$seq.out + reference="$source_iotests/$seq.out" if [ "$CACHEMODE" = "none" ]; then - [ -f $seq.out.nocache ] && reference=$seq.out.nocache + [ -f "$source_iotests/$seq.out.nocache" ] && reference="$source_iotests/$seq.out.nocache" fi - if [ ! -f $reference ] + if [ ! -f "$reference" ] then echo " - no qualified output" err=true else - if diff -w $reference $tmp.out >/dev/null 2>&1 + if diff -w "$reference" $tmp.out >/dev/null 2>&1 then echo "" if $err @@ -264,7 +332,7 @@ do else echo " - output mismatch (see $seq.out.bad)" mv $tmp.out $seq.out.bad - $diff -w $reference $seq.out.bad + $diff -w "$reference" $seq.out.bad err=true fi fi diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common index 0aaf84d015..e4083f4212 100644 --- a/tests/qemu-iotests/common +++ b/tests/qemu-iotests/common @@ -25,8 +25,7 @@ _setenvironment() export MSGVERB } -here=`pwd` -rm -f $here/$iam.out +rm -f "$OUTPUT_DIR/$iam.out" _setenvironment check=${check-true} @@ -59,7 +58,7 @@ do if $group then # arg after -g - group_list=`sed -n $tmp.list 2>/dev/null - group_list=`sed -n /dev/null + if grep -s "^$id " "$source_iotests/group" >/dev/null then # in group file ... OK echo $id >>$tmp.list @@ -402,7 +401,7 @@ else touch $tmp.list else # no test numbers, do everything from group file - sed -n -e '/^[0-9][0-9][0-9]*/s/[ ].*//p' $tmp.list + sed -n -e '/^[0-9][0-9][0-9]*/s/[ ].*//p' <"$source_iotests/group" >$tmp.list fi fi diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index d90a8bca8b..bd6790be63 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -126,7 +126,7 @@ fi export TEST_DIR if [ -z "$SAMPLE_IMG_DIR" ]; then - SAMPLE_IMG_DIR=`pwd`/sample_images + SAMPLE_IMG_DIR="$source_iotests/sample_images" fi if [ ! -d "$SAMPLE_IMG_DIR" ]; then diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 195c5646aa..e0ea7e3a7d 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -318,9 +318,9 @@ _do() status=1; exit fi - (eval "echo '---' \"$_cmd\"") >>$here/$seq.full + (eval "echo '---' \"$_cmd\"") >>"$OUTPUT_DIR/$seq.full" (eval "$_cmd") >$tmp._out 2>&1; ret=$? - cat $tmp._out >>$here/$seq.full + cat $tmp._out >>"$OUTPUT_DIR/$seq.full" if [ $# -eq 2 ]; then if [ $ret -eq 0 ]; then echo "done" @@ -344,7 +344,7 @@ _do() # _notrun() { - echo "$*" >$seq.notrun + echo "$*" >"$OUTPUT_DIR/$seq.notrun" echo "$seq not run: $*" status=0 exit @@ -354,7 +354,7 @@ _notrun() # _fail() { - echo "$*" | tee -a $here/$seq.full + echo "$*" | tee -a "$OUTPUT_DIR/$seq.full" echo "(see $seq.full for details)" status=1 exit 1 diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index f6c437c0c3..39a4cfcf4d 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -37,6 +37,7 @@ qemu_args = os.environ.get('QEMU', 'qemu').strip().split(' ') imgfmt = os.environ.get('IMGFMT', 'raw') imgproto = os.environ.get('IMGPROTO', 'file') test_dir = os.environ.get('TEST_DIR', '/var/tmp') +output_dir = os.environ.get('OUTPUT_DIR', '.') cachemode = os.environ.get('CACHEMODE') socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper') @@ -278,7 +279,7 @@ def notrun(reason): # Each test in qemu-iotests has a number ("seq") seq = os.path.basename(sys.argv[0]) - open('%s.notrun' % seq, 'wb').write(reason + '\n') + open('%s/%s.notrun' % (output_dir, seq), 'wb').write(reason + '\n') print '%s not run: %s' % (seq, reason) sys.exit(0) From 76c7560ae7ad088d106ef05439ca6520b096d43e Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Sat, 24 May 2014 23:24:56 +0200 Subject: [PATCH 43/47] configure: Enable out-of-tree iotests In order to allow out-of-tree iotests, create a symlink for the check script in the build tree. While doing so, also write configured options relevant to the iotests to common.env in the build tree; currently, this is the command to invoke Python 2. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- configure | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/configure b/configure index 71029643cc..23ecb37c43 100755 --- a/configure +++ b/configure @@ -5273,6 +5273,18 @@ if test "$docs" = "yes" ; then mkdir -p QMP fi +# set up qemu-iotests in this build directory +iotests_common_env="tests/qemu-iotests/common.env" +iotests_check="tests/qemu-iotests/check" + +echo "# Automatically generated by configure - do not modify" > "$iotests_common_env" +echo >> "$iotests_common_env" +echo "export PYTHON='$python'" >> "$iotests_common_env" + +if [ ! -e "$iotests_check" ]; then + symlink "$source_path/$iotests_check" "$iotests_check" +fi + # Save the configure command line for later reuse. cat <config.status #!/bin/sh From 7fed1a49ff72a5b794e3723612fa1731844b38f7 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Sat, 24 May 2014 23:24:57 +0200 Subject: [PATCH 44/47] iotests: Source common.env Source common.env in the iotests' check script. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/check | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 69f328b57c..992b91e14a 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -103,6 +103,12 @@ then export QEMU_NBD_PROG="$build_root/qemu-nbd" fi +# we need common.env +if ! . "$build_iotests/common.env" +then + _init_error "failed to source common.env (make sure the qemu-iotests are run from tests/qemu-iotests in the build tree)" +fi + # we need common.config if ! . "$source_iotests/common.config" then From ea81ca9de1354e9f42c4e09ead2af1b4d010bdde Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Sat, 24 May 2014 23:24:58 +0200 Subject: [PATCH 45/47] iotests: Use $PYTHON for Python scripts Instead of invoking Python scripts directly via ./, use $PYTHON to obtain the correct Python interpreter command. Signed-off-by: Max Reitz Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- tests/qemu-iotests/031 | 8 ++++---- tests/qemu-iotests/036 | 6 +++--- tests/qemu-iotests/039 | 18 +++++++++--------- tests/qemu-iotests/054 | 2 +- tests/qemu-iotests/060 | 20 ++++++++++---------- tests/qemu-iotests/061 | 24 ++++++++++++------------ tests/qemu-iotests/083 | 2 +- tests/qemu-iotests/check | 8 +++++++- 8 files changed, 47 insertions(+), 41 deletions(-) diff --git a/tests/qemu-iotests/031 b/tests/qemu-iotests/031 index 1d920ea87a..2a77ba8cbb 100755 --- a/tests/qemu-iotests/031 +++ b/tests/qemu-iotests/031 @@ -56,22 +56,22 @@ for IMGOPTS in "compat=0.10" "compat=1.1"; do echo === Create image with unknown header extension === echo _make_test_img 64M - ./qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension" - ./qcow2.py "$TEST_IMG" dump-header + $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension" + $PYTHON qcow2.py "$TEST_IMG" dump-header _check_test_img echo echo === Rewrite header with no backing file === echo $QEMU_IMG rebase -u -b "" "$TEST_IMG" - ./qcow2.py "$TEST_IMG" dump-header + $PYTHON qcow2.py "$TEST_IMG" dump-header _check_test_img echo echo === Add a backing file and format === echo $QEMU_IMG rebase -u -b "/some/backing/file/path" -F host_device "$TEST_IMG" - ./qcow2.py "$TEST_IMG" dump-header + $PYTHON qcow2.py "$TEST_IMG" dump-header done # success, all done diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036 index 03b6aa9de7..a77365347d 100755 --- a/tests/qemu-iotests/036 +++ b/tests/qemu-iotests/036 @@ -53,15 +53,15 @@ IMGOPTS="compat=1.1" echo === Create image with unknown autoclear feature bit === echo _make_test_img 64M -./qcow2.py "$TEST_IMG" set-feature-bit autoclear 63 -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" set-feature-bit autoclear 63 +$PYTHON qcow2.py "$TEST_IMG" dump-header echo echo === Repair image === echo _check_test_img -r all -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header # success, all done echo "*** done" diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039 index 27fe4bdacc..84c9167660 100755 --- a/tests/qemu-iotests/039 +++ b/tests/qemu-iotests/039 @@ -63,7 +63,7 @@ _make_test_img $size $QEMU_IO -c "write -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io # The dirty bit must not be set -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features _check_test_img echo @@ -75,7 +75,7 @@ _make_test_img $size _no_dump_exec $QEMU_IO -c "write -P 0x5a 0 512" -c "abort" "$TEST_IMG" 2>&1 | _filter_qemu_io # The dirty bit must be set -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features _check_test_img echo @@ -84,7 +84,7 @@ echo "== Read-only access must still work ==" $QEMU_IO -r -c "read -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io # The dirty bit must be set -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features echo echo "== Repairing the image file must succeed ==" @@ -92,7 +92,7 @@ echo "== Repairing the image file must succeed ==" _check_test_img -r all # The dirty bit must not be set -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features echo echo "== Data should still be accessible after repair ==" @@ -108,12 +108,12 @@ _make_test_img $size _no_dump_exec $QEMU_IO -c "write -P 0x5a 0 512" -c "abort" "$TEST_IMG" 2>&1 | _filter_qemu_io # The dirty bit must be set -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features $QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io # The dirty bit must not be set -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features echo echo "== Creating an image file with lazy_refcounts=off ==" @@ -124,7 +124,7 @@ _make_test_img $size _no_dump_exec $QEMU_IO -c "write -P 0x5a 0 512" -c "abort" "$TEST_IMG" 2>&1 | _filter_qemu_io # The dirty bit must not be set since lazy_refcounts=off -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features _check_test_img echo @@ -140,8 +140,8 @@ $QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io $QEMU_IMG commit "$TEST_IMG" # The dirty bit must not be set -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features -./qcow2.py "$TEST_IMG".base dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG".base dump-header | grep incompatible_features _check_test_img TEST_IMG="$TEST_IMG".base _check_test_img diff --git a/tests/qemu-iotests/054 b/tests/qemu-iotests/054 index c8b7082b4e..bd94153d66 100755 --- a/tests/qemu-iotests/054 +++ b/tests/qemu-iotests/054 @@ -49,7 +49,7 @@ _make_test_img $((1024*1024))T echo echo "creating too large image (1 EB) using qcow2.py" _make_test_img 4G -./qcow2.py "$TEST_IMG" set-header size $((1024 ** 6)) +$PYTHON qcow2.py "$TEST_IMG" set-header size $((1024 ** 6)) _check_test_img # success, all done diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 index f0116aab1d..3cffc12fec 100755 --- a/tests/qemu-iotests/060 +++ b/tests/qemu-iotests/060 @@ -68,13 +68,13 @@ poke_file "$TEST_IMG" "$l1_offset" "\x80\x00\x00\x00\x00\x03\x00\x00" _check_test_img # The corrupt bit should not be set anyway -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features # Try to write something, thereby forcing the corrupt bit to be set $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io # The corrupt bit must now be set -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features # Try to open the image R/W (which should fail) $QEMU_IO -c "$OPEN_RW" -c "read 0 512" 2>&1 | _filter_qemu_io \ @@ -99,19 +99,19 @@ poke_file "$TEST_IMG" "$(($rb_offset+8))" "\x00\x01" # Redirect new data cluster onto refcount block poke_file "$TEST_IMG" "$l2_offset" "\x80\x00\x00\x00\x00\x02\x00\x00" _check_test_img -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features # Try to fix it _check_test_img -r all # The corrupt bit should be cleared -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features # Look if it's really really fixed $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features echo echo "=== Testing cluster data reference into inactive L2 table ===" @@ -124,13 +124,13 @@ $QEMU_IO -c "$OPEN_RW" -c "write -P 2 0 512" | _filter_qemu_io poke_file "$TEST_IMG" "$l2_offset_after_snapshot" \ "\x80\x00\x00\x00\x00\x04\x00\x00" _check_test_img -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features $QEMU_IO -c "$OPEN_RW" -c "write -P 3 0 512" | _filter_qemu_io -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features _check_test_img -r all -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features $QEMU_IO -c "$OPEN_RW" -c "write -P 4 0 512" | _filter_qemu_io -./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features # Check data $QEMU_IO -c "$OPEN_RO" -c "read -P 4 0 512" | _filter_qemu_io diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061 index d3a6b388b5..ab98def6d4 100755 --- a/tests/qemu-iotests/061 +++ b/tests/qemu-iotests/061 @@ -48,9 +48,9 @@ echo "=== Testing version downgrade with zero expansion ===" echo IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M $QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG" -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header $QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io _check_test_img @@ -59,9 +59,9 @@ echo "=== Testing dirty version downgrade ===" echo IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M $QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _filter_qemu_io -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG" -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header $QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io _check_test_img @@ -69,11 +69,11 @@ echo echo "=== Testing version downgrade with unknown compat/autoclear flags ===" echo IMGOPTS="compat=1.1" _make_test_img 64M -./qcow2.py "$TEST_IMG" set-feature-bit compatible 42 -./qcow2.py "$TEST_IMG" set-feature-bit autoclear 42 -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" set-feature-bit compatible 42 +$PYTHON qcow2.py "$TEST_IMG" set-feature-bit autoclear 42 +$PYTHON qcow2.py "$TEST_IMG" dump-header $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG" -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header _check_test_img echo @@ -81,9 +81,9 @@ echo "=== Testing version upgrade and resize ===" echo IMGOPTS="compat=0.10" _make_test_img 64M $QEMU_IO -c "write -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header $QEMU_IMG amend -o "compat=1.1,lazy_refcounts=on,size=128M" "$TEST_IMG" -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header $QEMU_IO -c "read -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io _check_test_img @@ -92,9 +92,9 @@ echo "=== Testing dirty lazy_refcounts=off ===" echo IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M $QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _filter_qemu_io -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header $QEMU_IMG amend -o "lazy_refcounts=off" "$TEST_IMG" -./qcow2.py "$TEST_IMG" dump-header +$PYTHON qcow2.py "$TEST_IMG" dump-header $QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io _check_test_img diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083 index f764534782..b7ba8608e4 100755 --- a/tests/qemu-iotests/083 +++ b/tests/qemu-iotests/083 @@ -81,7 +81,7 @@ EOF nbd_url="nbd:127.0.0.1:$port:exportname=foo" fi - ./nbd-fault-injector.py $extra_args "127.0.0.1:$port" "$TEST_DIR/nbd-fault-injector.conf" 2>&1 >/dev/null & + $PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" "$TEST_DIR/nbd-fault-injector.conf" 2>&1 >/dev/null & wait_for_tcp_port "127.0.0.1:$port" $QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | filter_nbd diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 992b91e14a..8ca40116d7 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -288,10 +288,16 @@ do start=`_wallclock` $timestamp && echo -n " ["`date "+%T"`"]" + + if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then + run_command="$PYTHON $seq" + else + run_command="./$seq" + fi export OUTPUT_DIR=$PWD (cd "$source_iotests"; MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \ - ./$seq >$tmp.out 2>&1) + $run_command >$tmp.out 2>&1) sts=$? $timestamp && _timestamp stop=`_wallclock` From f99b4b5d7e53e85552892cdf6c7d2c3c86d409f8 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Sat, 24 May 2014 23:24:59 +0200 Subject: [PATCH 46/47] iotests: Drop Python version from 065's Shebang Test 065 specified python2 to be used in its Shebang; this might not work on systems without a python2 symlink and furthermore it is now counter-productive, as the check script compares the Shebang to "#!/usr/bin/env python" and only uses the Python interpreter selected by configure on an exact match. Signed-off-by: Max Reitz Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- tests/qemu-iotests/065 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065 index ab5445f62d..e89b61d70b 100755 --- a/tests/qemu-iotests/065 +++ b/tests/qemu-iotests/065 @@ -1,4 +1,4 @@ -#!/usr/bin/env python2 +#!/usr/bin/env python # # Test for additional information emitted by qemu-img info on qcow2 # images From f5264553c381c5f305d6e11bef18da6a29f3f423 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Sat, 24 May 2014 23:25:00 +0200 Subject: [PATCH 47/47] iotests: Fix 083 for out-of-tree builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit iotest 083 filters out debug messages from nbd, which are prefixed (and recognized) by __FILE__. However, the current filter (/^nbd\.c…/) is valid for in-tree builds only, as out-of-tree builds will have a path before that filename (e.g. "/tmp/qemu/nbd.c"). Fix this by adding .* before "nbd\.c". While working on this, also fix the regexes: '.' should be escaped and a single backslash is not enough for escaping when enclosed by double quotes. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/083 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083 index b7ba8608e4..991a9d91db 100755 --- a/tests/qemu-iotests/083 +++ b/tests/qemu-iotests/083 @@ -44,7 +44,7 @@ choose_tcp_port() { wait_for_tcp_port() { while ! (netstat --tcp --listening --numeric | \ - grep "$1.*0.0.0.0:\*.*LISTEN") 2>&1 >/dev/null; do + grep "$1.*0\\.0\\.0\\.0:\\*.*LISTEN") 2>&1 >/dev/null; do sleep 0.1 done } @@ -55,8 +55,8 @@ filter_nbd() { # callbacks sometimes, making them unreliable. # # Filter out the TCP port number since this changes between runs. - sed -e 's#^nbd.c:.*##g' \ - -e 's#nbd:127.0.0.1:[^:]*:#nbd:127.0.0.1:PORT:#g' + sed -e 's#^.*nbd\.c:.*##g' \ + -e 's#nbd:127\.0\.0\.1:[^:]*:#nbd:127\.0\.0\.1:PORT:#g' } check_disconnect() { @@ -82,7 +82,7 @@ EOF fi $PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" "$TEST_DIR/nbd-fault-injector.conf" 2>&1 >/dev/null & - wait_for_tcp_port "127.0.0.1:$port" + wait_for_tcp_port "127\\.0\\.0\\.1:$port" $QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | filter_nbd echo