From a71835a0ccff168b19ffc9656fe27988821ec59a Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Sat, 8 Feb 2014 14:38:33 +0100 Subject: qcow2: Set zero flag for discarded clusters Instead of making the backing file contents visible again after a discard request, set the zero flag if possible (i.e. on version >= 3). Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/qcow2-cluster.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'block') diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index c57f39dd2b..36c1bed350 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1367,13 +1367,31 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset, uint64_t old_offset; old_offset = be64_to_cpu(l2_table[l2_index + i]); - if ((old_offset & L2E_OFFSET_MASK) == 0) { + + /* + * Make sure that a discarded area reads back as zeroes for v3 images + * (we cannot do it for v2 without actually writing a zero-filled + * buffer). We can skip the operation if the cluster is already marked + * as zero, or if it's unallocated and we don't have a backing file. + * + * TODO We might want to use bdrv_get_block_status(bs) here, but we're + * holding s->lock, so that doesn't work today. + */ + if (old_offset & QCOW_OFLAG_ZERO) { + continue; + } + + if ((old_offset & L2E_OFFSET_MASK) == 0 && !bs->backing_hd) { continue; } /* First remove L2 entries */ qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table); - l2_table[l2_index + i] = cpu_to_be64(0); + if (s->qcow_version >= 3) { + l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO); + } else { + l2_table[l2_index + i] = cpu_to_be64(0); + } /* Then decrease the refcount */ qcow2_free_any_clusters(bs, old_offset, 1, type); -- cgit v1.2.1 From f67503e5bd8997ea7ec3f4bfa0af0e06321771a6 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 18 Feb 2014 18:33:05 +0100 Subject: block: Change BDS parameter of bdrv_open() to ** Make bdrv_open() take a pointer to a BDS pointer, similarly to bdrv_file_open(). If a pointer to a NULL pointer is given, bdrv_open() will create a new BDS with an empty name; if the BDS pointer is not NULL, that existing BDS will be reused (in the same way as bdrv_open() already did). Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/blkdebug.c | 1 + block/blkverify.c | 2 ++ block/qcow2.c | 14 +++++++++----- block/vmdk.c | 5 ++--- block/vvfat.c | 6 ++---- 5 files changed, 16 insertions(+), 12 deletions(-) (limited to 'block') diff --git a/block/blkdebug.c b/block/blkdebug.c index ee10013362..46bd086da9 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -410,6 +410,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, s->state = 1; /* Open the backing file */ + assert(bs->file == NULL); ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-image"), options, "image", flags, true, false, &local_err); if (ret < 0) { diff --git a/block/blkverify.c b/block/blkverify.c index 1563c88324..7d8a32ec79 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -135,6 +135,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, } /* Open the raw file */ + assert(bs->file == NULL); ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-raw"), options, "raw", flags, true, false, &local_err); if (ret < 0) { @@ -143,6 +144,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, } /* Open the test file */ + assert(s->test_file == NULL); ret = bdrv_open_image(&s->test_file, qemu_opt_get(opts, "x-image"), options, "test", flags, false, false, &local_err); if (ret < 0) { diff --git a/block/qcow2.c b/block/qcow2.c index b1dbdb120e..309ea12b7d 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1543,7 +1543,8 @@ static int qcow2_create2(const char *filename, int64_t total_size, goto out; } - bdrv_close(bs); + bdrv_unref(bs); + bs = NULL; /* * And now open the image and make it consistent first (i.e. increase the @@ -1552,7 +1553,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, */ BlockDriver* drv = bdrv_find_format("qcow2"); assert(drv != NULL); - ret = bdrv_open(bs, filename, NULL, + ret = bdrv_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err); if (ret < 0) { error_propagate(errp, local_err); @@ -1599,10 +1600,11 @@ static int qcow2_create2(const char *filename, int64_t total_size, } } - bdrv_close(bs); + bdrv_unref(bs); + bs = NULL; /* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */ - ret = bdrv_open(bs, filename, NULL, + ret = bdrv_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING, drv, &local_err); if (local_err) { @@ -1612,7 +1614,9 @@ static int qcow2_create2(const char *filename, int64_t total_size, ret = 0; out: - bdrv_unref(bs); + if (bs) { + bdrv_unref(bs); + } return ret; } diff --git a/block/vmdk.c b/block/vmdk.c index ff6f5ee911..0622db51c4 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1755,10 +1755,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, goto exit; } if (backing_file) { - BlockDriverState *bs = bdrv_new(""); - ret = bdrv_open(bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp); + BlockDriverState *bs = NULL; + ret = bdrv_open(&bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp); if (ret != 0) { - bdrv_unref(bs); goto exit; } if (strcmp(bs->drv->format_name, "vmdk")) { diff --git a/block/vvfat.c b/block/vvfat.c index a19e4ca227..0a9f886a20 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2936,15 +2936,13 @@ static int enable_write_target(BDRVVVFATState *s) goto err; } - s->qcow = bdrv_new(""); - - ret = bdrv_open(s->qcow, s->qcow_filename, NULL, + s->qcow = NULL; + ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow, &local_err); if (ret < 0) { qerror_report_err(local_err); error_free(local_err); - bdrv_unref(s->qcow); goto err; } -- cgit v1.2.1 From ddf5636dc9e4be894f2ab4a5f803d915478b5099 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 18 Feb 2014 18:33:06 +0100 Subject: block: Add reference parameter to bdrv_open() Allow bdrv_open() to handle references to existing block devices just as bdrv_file_open() is already capable of. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.c | 4 ++-- block/vmdk.c | 3 ++- block/vvfat.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) (limited to 'block') diff --git a/block/qcow2.c b/block/qcow2.c index 309ea12b7d..e2aa5c1b3c 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1553,7 +1553,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, */ BlockDriver* drv = bdrv_find_format("qcow2"); assert(drv != NULL); - ret = bdrv_open(&bs, filename, NULL, + ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err); if (ret < 0) { error_propagate(errp, local_err); @@ -1604,7 +1604,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, bs = NULL; /* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */ - ret = bdrv_open(&bs, filename, NULL, + ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING, drv, &local_err); if (local_err) { diff --git a/block/vmdk.c b/block/vmdk.c index 0622db51c4..5df486fa8e 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1756,7 +1756,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, } if (backing_file) { BlockDriverState *bs = NULL; - ret = bdrv_open(&bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp); + ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_NO_BACKING, NULL, + errp); if (ret != 0) { goto exit; } diff --git a/block/vvfat.c b/block/vvfat.c index 0a9f886a20..4bde3bd220 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2937,7 +2937,7 @@ static int enable_write_target(BDRVVVFATState *s) } s->qcow = NULL; - ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, + ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow, &local_err); if (ret < 0) { -- cgit v1.2.1 From 2e40134bfdbb073512f9f264cb96162787ec62b1 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 18 Feb 2014 18:33:07 +0100 Subject: block: Make bdrv_file_open() static Add the bdrv_open() option BDRV_O_PROTOCOL which results in passing the call to bdrv_file_open(). Additionally, make bdrv_file_open() static and therefore bdrv_open() the only way to call it. Consequently, all existing calls to bdrv_file_open() have to be adjusted to use bdrv_open() with the BDRV_O_PROTOCOL flag instead. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- block/cow.c | 5 +++-- block/qcow.c | 5 +++-- block/qcow2.c | 4 +++- block/qed.c | 8 +++++--- block/sheepdog.c | 7 +++++-- block/vhdx.c | 4 +++- block/vmdk.c | 13 +++++++++---- 7 files changed, 31 insertions(+), 15 deletions(-) (limited to 'block') diff --git a/block/cow.c b/block/cow.c index 7fc0b12163..d0385be1c5 100644 --- a/block/cow.c +++ b/block/cow.c @@ -351,8 +351,9 @@ static int cow_create(const char *filename, QEMUOptionParameter *options, return ret; } - ret = bdrv_file_open(&cow_bs, filename, NULL, NULL, BDRV_O_RDWR, - &local_err); + cow_bs = NULL; + ret = bdrv_open(&cow_bs, filename, NULL, NULL, + BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err); if (ret < 0) { qerror_report_err(local_err); error_free(local_err); diff --git a/block/qcow.c b/block/qcow.c index 948b0c5601..8d00853ab5 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -691,8 +691,9 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options, return ret; } - ret = bdrv_file_open(&qcow_bs, filename, NULL, NULL, BDRV_O_RDWR, - &local_err); + qcow_bs = NULL; + ret = bdrv_open(&qcow_bs, filename, NULL, NULL, + BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err); if (ret < 0) { qerror_report_err(local_err); error_free(local_err); diff --git a/block/qcow2.c b/block/qcow2.c index e2aa5c1b3c..9dfd90896b 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1493,7 +1493,9 @@ static int qcow2_create2(const char *filename, int64_t total_size, return ret; } - ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err); + bs = NULL; + ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, + NULL, &local_err); if (ret < 0) { error_propagate(errp, local_err); return ret; diff --git a/block/qed.c b/block/qed.c index b9ca7ac0da..9bc181f041 100644 --- a/block/qed.c +++ b/block/qed.c @@ -562,7 +562,7 @@ static int qed_create(const char *filename, uint32_t cluster_size, size_t l1_size = header.cluster_size * header.table_size; Error *local_err = NULL; int ret = 0; - BlockDriverState *bs = NULL; + BlockDriverState *bs; ret = bdrv_create_file(filename, NULL, &local_err); if (ret < 0) { @@ -571,8 +571,10 @@ static int qed_create(const char *filename, uint32_t cluster_size, return ret; } - ret = bdrv_file_open(&bs, filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_CACHE_WB, &local_err); + bs = NULL; + ret = bdrv_open(&bs, filename, NULL, NULL, + BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL, NULL, + &local_err); if (ret < 0) { qerror_report_err(local_err); error_free(local_err); diff --git a/block/sheepdog.c b/block/sheepdog.c index e6c0376566..f7bd0242e5 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1534,7 +1534,8 @@ static int sd_prealloc(const char *filename) Error *local_err = NULL; int ret; - ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err); + ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, + NULL, &local_err); if (ret < 0) { qerror_report_err(local_err); error_free(local_err); @@ -1695,7 +1696,9 @@ static int sd_create(const char *filename, QEMUOptionParameter *options, goto out; } - ret = bdrv_file_open(&bs, backing_file, NULL, NULL, 0, &local_err); + bs = NULL; + ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, NULL, + &local_err); if (ret < 0) { qerror_report_err(local_err); error_free(local_err); diff --git a/block/vhdx.c b/block/vhdx.c index 55689cf641..366ff2e627 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1797,7 +1797,9 @@ static int vhdx_create(const char *filename, QEMUOptionParameter *options, goto exit; } - ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err); + bs = NULL; + ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, + NULL, &local_err); if (ret < 0) { error_propagate(errp, local_err); goto exit; diff --git a/block/vmdk.c b/block/vmdk.c index 5df486fa8e..54a1c59427 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -776,8 +776,9 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, path_combine(extent_path, sizeof(extent_path), desc_file_path, fname); - ret = bdrv_file_open(&extent_file, extent_path, NULL, NULL, - bs->open_flags, errp); + extent_file = NULL; + ret = bdrv_open(&extent_file, extent_path, NULL, NULL, + bs->open_flags | BDRV_O_PROTOCOL, NULL, errp); if (ret) { return ret; } @@ -1493,7 +1494,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, goto exit; } - ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err); + assert(bs == NULL); + ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, + NULL, &local_err); if (ret < 0) { error_propagate(errp, local_err); goto exit; @@ -1831,7 +1834,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, goto exit; } } - ret = bdrv_file_open(&new_bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err); + assert(new_bs == NULL); + ret = bdrv_open(&new_bs, filename, NULL, NULL, + BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err); if (ret < 0) { error_setg_errno(errp, -ret, "Could not write description"); goto exit; -- cgit v1.2.1 From f7d9fd8c7270de25b1e0d0a462b6958b53aa31b2 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 18 Feb 2014 18:33:12 +0100 Subject: block: Remove bdrv_open_image()'s force_raw option This option is now unnecessary since specifying BDRV_O_PROTOCOL as flag will do exactly the same. Signed-off-by: Max Reitz Reviewed-by: Benoit Canet Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/blkdebug.c | 2 +- block/blkverify.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'block') diff --git a/block/blkdebug.c b/block/blkdebug.c index 46bd086da9..380c736101 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -412,7 +412,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, /* Open the backing file */ assert(bs->file == NULL); ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-image"), options, "image", - flags, true, false, &local_err); + flags | BDRV_O_PROTOCOL, false, &local_err); if (ret < 0) { error_propagate(errp, local_err); goto out; diff --git a/block/blkverify.c b/block/blkverify.c index 7d8a32ec79..0e28502e8e 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -137,7 +137,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, /* Open the raw file */ assert(bs->file == NULL); ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-raw"), options, - "raw", flags, true, false, &local_err); + "raw", flags | BDRV_O_PROTOCOL, false, &local_err); if (ret < 0) { error_propagate(errp, local_err); goto fail; @@ -146,7 +146,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, /* Open the test file */ assert(s->test_file == NULL); ret = bdrv_open_image(&s->test_file, qemu_opt_get(opts, "x-image"), options, - "test", flags, false, false, &local_err); + "test", flags, false, &local_err); if (ret < 0) { error_propagate(errp, local_err); s->test_file = NULL; -- cgit v1.2.1 From a69d9af449e9de200abc751d8614124c7486426f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:48 +0100 Subject: nbd: produce a better error if neither host nor port is passed Before: $ qemu-io-old qemu-io-old> open -r -o file.driver=nbd qemu-io-old: can't open device (null): Could not open image: Invalid argument $ ./qemu-io-old qemu-io-old> open -r -o file.driver=nbd,file.host=foo,file.path=bar path and host may not be used at the same time. qemu-io-old: can't open device (null): Could not open image: Invalid argument After: $ ./qemu-io qemu-io> open -r -o file.driver=nbd one of path and host must be specified. qemu-io: can't open device (null): Could not open image: Invalid argument $ ./qemu-io qemu-io> open -r -o file.driver=nbd,file.host=foo,file.path=bar path and host may not be used at the same time. qemu-io: can't open device (null): Could not open image: Invalid argument Next patch will fix the error propagation. Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/nbd.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'block') diff --git a/block/nbd.c b/block/nbd.c index abae506f04..a83e8565e5 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -192,19 +192,18 @@ static int nbd_config(BDRVNBDState *s, QDict *options, char **export) { Error *local_err = NULL; - if (qdict_haskey(options, "path")) { - if (qdict_haskey(options, "host")) { + if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) { + if (qdict_haskey(options, "path")) { qerror_report(ERROR_CLASS_GENERIC_ERROR, "path and host may not " "be used at the same time."); - return -EINVAL; + } else { + qerror_report(ERROR_CLASS_GENERIC_ERROR, "one of path and host " + "must be specified."); } - s->client.is_unix = true; - } else if (qdict_haskey(options, "host")) { - s->client.is_unix = false; - } else { return -EINVAL; } + s->client.is_unix = qdict_haskey(options, "path"); s->socket_opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort); -- cgit v1.2.1 From 77e8b9ca64e85d3d309f322410964b7852ec091e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:49 +0100 Subject: nbd: correctly propagate errors Before: $ ./qemu-io-old qemu-io-old> open -r -o file.driver=nbd one of path and host must be specified. qemu-io-old: can't open device (null): Could not open image: Invalid argument $ ./qemu-io-old qemu-io-old> open -r -o file.driver=nbd,file.host=foo,file.path=bar path and host may not be used at the same time. qemu-io-old: can't open device (null): Could not open image: Invalid argument After: $ ./qemu-io qemu-io> open -r -o file.driver=nbd qemu-io: can't open device (null): one of path and host must be specified. $ ./qemu-io qemu-io> open -r -o file.driver=nbd,file.host=foo,file.path=bar qemu-io: can't open device (null): path and host may not be used at the same time. Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/nbd.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) (limited to 'block') diff --git a/block/nbd.c b/block/nbd.c index a83e8565e5..55124239df 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -188,19 +188,18 @@ out: g_free(file); } -static int nbd_config(BDRVNBDState *s, QDict *options, char **export) +static void nbd_config(BDRVNBDState *s, QDict *options, char **export, + Error **errp) { Error *local_err = NULL; if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) { if (qdict_haskey(options, "path")) { - qerror_report(ERROR_CLASS_GENERIC_ERROR, "path and host may not " - "be used at the same time."); + error_setg(errp, "path and host may not be used at the same time."); } else { - qerror_report(ERROR_CLASS_GENERIC_ERROR, "one of path and host " - "must be specified."); + error_setg(errp, "one of path and host must be specified."); } - return -EINVAL; + return; } s->client.is_unix = qdict_haskey(options, "path"); @@ -209,9 +208,8 @@ static int nbd_config(BDRVNBDState *s, QDict *options, char **export) qemu_opts_absorb_qdict(s->socket_opts, options, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); - return -EINVAL; + error_propagate(errp, local_err); + return; } if (!qemu_opt_get(s->socket_opts, "port")) { @@ -222,19 +220,17 @@ static int nbd_config(BDRVNBDState *s, QDict *options, char **export) if (*export) { qdict_del(options, "export"); } - - return 0; } -static int nbd_establish_connection(BlockDriverState *bs) +static int nbd_establish_connection(BlockDriverState *bs, Error **errp) { BDRVNBDState *s = bs->opaque; int sock; if (s->client.is_unix) { - sock = unix_socket_outgoing(qemu_opt_get(s->socket_opts, "path")); + sock = unix_connect_opts(s->socket_opts, errp, NULL, NULL); } else { - sock = tcp_socket_outgoing_opts(s->socket_opts); + sock = inet_connect_opts(s->socket_opts, errp, NULL, NULL); if (sock >= 0) { socket_set_nodelay(sock); } @@ -255,17 +251,19 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags, BDRVNBDState *s = bs->opaque; char *export = NULL; int result, sock; + Error *local_err = NULL; /* Pop the config into our state object. Exit if invalid. */ - result = nbd_config(s, options, &export); - if (result != 0) { - return result; + nbd_config(s, options, &export, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return -EINVAL; } /* establish TCP connection, return error if it fails * TODO: Configurable retry-until-timeout behaviour. */ - sock = nbd_establish_connection(bs); + sock = nbd_establish_connection(bs, errp); if (sock < 0) { return sock; } -- cgit v1.2.1 From 35cb1748d54c8e56881a5e10138b3eb090f3a6bc Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:52 +0100 Subject: iscsi: fix indentation Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/iscsi.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) (limited to 'block') diff --git a/block/iscsi.c b/block/iscsi.c index f8e496f8ef..ac5a5c73e3 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1065,35 +1065,36 @@ static QemuOptsList runtime_opts = { }, }; -static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, - int lun, int evpd, int pc) { - int full_size; - struct scsi_task *task = NULL; - task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, 64); +static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun, + int evpd, int pc) +{ + int full_size; + struct scsi_task *task = NULL; + task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, 64); + if (task == NULL || task->status != SCSI_STATUS_GOOD) { + goto fail; + } + full_size = scsi_datain_getfullsize(task); + if (full_size > task->datain.size) { + scsi_free_scsi_task(task); + + /* we need more data for the full list */ + task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, full_size); if (task == NULL || task->status != SCSI_STATUS_GOOD) { goto fail; } - full_size = scsi_datain_getfullsize(task); - if (full_size > task->datain.size) { - scsi_free_scsi_task(task); - - /* we need more data for the full list */ - task = iscsi_inquiry_sync(iscsi, lun, evpd, pc, full_size); - if (task == NULL || task->status != SCSI_STATUS_GOOD) { - goto fail; - } - } + } - return task; + return task; fail: - error_report("iSCSI: Inquiry command failed : %s", - iscsi_get_error(iscsi)); - if (task) { - scsi_free_scsi_task(task); - return NULL; - } + error_report("iSCSI: Inquiry command failed : %s", + iscsi_get_error(iscsi)); + if (task) { + scsi_free_scsi_task(task); return NULL; + } + return NULL; } /* -- cgit v1.2.1 From f2917853f715b0ef55df29eb2ffea29dc69ce814 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:53 +0100 Subject: iscsi: correctly propagate errors in iscsi_open Before: $ ./qemu-io-old qemu-io-old> open -r -o file.driver=iscsi,file.filename=foo Failed to parse URL : foo qemu-io-old: can't open device (null): Could not open 'foo': Invalid argument After: $ ./qemu-io qemu-io> open -r -o file.driver=iscsi,file.filename=foo qemu-io: can't open device (null): Failed to parse URL : foo Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/iscsi.c | 103 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 53 insertions(+), 50 deletions(-) (limited to 'block') diff --git a/block/iscsi.c b/block/iscsi.c index ac5a5c73e3..41ec09709d 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -856,7 +856,8 @@ retry: #endif /* SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED */ -static int parse_chap(struct iscsi_context *iscsi, const char *target) +static void parse_chap(struct iscsi_context *iscsi, const char *target, + Error **errp) { QemuOptsList *list; QemuOpts *opts; @@ -865,37 +866,35 @@ static int parse_chap(struct iscsi_context *iscsi, const char *target) list = qemu_find_opts("iscsi"); if (!list) { - return 0; + return; } opts = qemu_opts_find(list, target); if (opts == NULL) { opts = QTAILQ_FIRST(&list->head); if (!opts) { - return 0; + return; } } user = qemu_opt_get(opts, "user"); if (!user) { - return 0; + return; } password = qemu_opt_get(opts, "password"); if (!password) { - error_report("CHAP username specified but no password was given"); - return -1; + error_setg(errp, "CHAP username specified but no password was given"); + return; } if (iscsi_set_initiator_username_pwd(iscsi, user, password)) { - error_report("Failed to set initiator username and password"); - return -1; + error_setg(errp, "Failed to set initiator username and password"); } - - return 0; } -static void parse_header_digest(struct iscsi_context *iscsi, const char *target) +static void parse_header_digest(struct iscsi_context *iscsi, const char *target, + Error **errp) { QemuOptsList *list; QemuOpts *opts; @@ -928,7 +927,7 @@ static void parse_header_digest(struct iscsi_context *iscsi, const char *target) } else if (!strcmp(digest, "NONE-CRC32C")) { iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C); } else { - error_report("Invalid header-digest setting : %s", digest); + error_setg(errp, "Invalid header-digest setting : %s", digest); } } @@ -986,12 +985,11 @@ static void iscsi_nop_timed_event(void *opaque) } #endif -static int iscsi_readcapacity_sync(IscsiLun *iscsilun) +static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp) { struct scsi_task *task = NULL; struct scsi_readcapacity10 *rc10 = NULL; struct scsi_readcapacity16 *rc16 = NULL; - int ret = 0; int retries = ISCSI_CMD_RETRIES; do { @@ -1006,8 +1004,7 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun) if (task != NULL && task->status == SCSI_STATUS_GOOD) { rc16 = scsi_datain_unmarshall(task); if (rc16 == NULL) { - error_report("iSCSI: Failed to unmarshall readcapacity16 data."); - ret = -EINVAL; + error_setg(errp, "iSCSI: Failed to unmarshall readcapacity16 data."); } else { iscsilun->block_size = rc16->block_length; iscsilun->num_blocks = rc16->returned_lba + 1; @@ -1021,8 +1018,7 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun) if (task != NULL && task->status == SCSI_STATUS_GOOD) { rc10 = scsi_datain_unmarshall(task); if (rc10 == NULL) { - error_report("iSCSI: Failed to unmarshall readcapacity10 data."); - ret = -EINVAL; + error_setg(errp, "iSCSI: Failed to unmarshall readcapacity10 data."); } else { iscsilun->block_size = rc10->block_size; if (rc10->lba == 0) { @@ -1035,20 +1031,18 @@ static int iscsi_readcapacity_sync(IscsiLun *iscsilun) } break; default: - return 0; + return; } } while (task != NULL && task->status == SCSI_STATUS_CHECK_CONDITION && task->sense.key == SCSI_SENSE_UNIT_ATTENTION && retries-- > 0); if (task == NULL || task->status != SCSI_STATUS_GOOD) { - error_report("iSCSI: failed to send readcapacity10 command."); - ret = -EINVAL; + error_setg(errp, "iSCSI: failed to send readcapacity10 command."); } if (task) { scsi_free_scsi_task(task); } - return ret; } /* TODO Convert to fine grained options */ @@ -1066,7 +1060,7 @@ static QemuOptsList runtime_opts = { }; static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun, - int evpd, int pc) + int evpd, int pc, Error **errp) { int full_size; struct scsi_task *task = NULL; @@ -1088,8 +1082,8 @@ static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun, return task; fail: - error_report("iSCSI: Inquiry command failed : %s", - iscsi_get_error(iscsi)); + error_setg(errp, "iSCSI: Inquiry command failed : %s", + iscsi_get_error(iscsi)); if (task) { scsi_free_scsi_task(task); return NULL; @@ -1120,27 +1114,25 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, int ret; if ((BDRV_SECTOR_SIZE % 512) != 0) { - error_report("iSCSI: Invalid BDRV_SECTOR_SIZE. " - "BDRV_SECTOR_SIZE(%lld) is not a multiple " - "of 512", BDRV_SECTOR_SIZE); + error_setg(errp, "iSCSI: Invalid BDRV_SECTOR_SIZE. " + "BDRV_SECTOR_SIZE(%lld) is not a multiple " + "of 512", BDRV_SECTOR_SIZE); return -EINVAL; } opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); ret = -EINVAL; goto out; } filename = qemu_opt_get(opts, "filename"); - iscsi_url = iscsi_parse_full_url(iscsi, filename); if (iscsi_url == NULL) { - error_report("Failed to parse URL : %s", filename); + error_setg(errp, "Failed to parse URL : %s", filename); ret = -EINVAL; goto out; } @@ -1151,13 +1143,13 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, iscsi = iscsi_create_context(initiator_name); if (iscsi == NULL) { - error_report("iSCSI: Failed to create iSCSI context."); + error_setg(errp, "iSCSI: Failed to create iSCSI context."); ret = -ENOMEM; goto out; } if (iscsi_set_targetname(iscsi, iscsi_url->target)) { - error_report("iSCSI: Failed to set target name."); + error_setg(errp, "iSCSI: Failed to set target name."); ret = -EINVAL; goto out; } @@ -1166,21 +1158,22 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, ret = iscsi_set_initiator_username_pwd(iscsi, iscsi_url->user, iscsi_url->passwd); if (ret != 0) { - error_report("Failed to set initiator username and password"); + error_setg(errp, "Failed to set initiator username and password"); ret = -EINVAL; goto out; } } /* check if we got CHAP username/password via the options */ - if (parse_chap(iscsi, iscsi_url->target) != 0) { - error_report("iSCSI: Failed to set CHAP user/password"); + parse_chap(iscsi, iscsi_url->target, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); ret = -EINVAL; goto out; } if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) { - error_report("iSCSI: Failed to set session type to normal."); + error_setg(errp, "iSCSI: Failed to set session type to normal."); ret = -EINVAL; goto out; } @@ -1188,10 +1181,15 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C); /* check if we got HEADER_DIGEST via the options */ - parse_header_digest(iscsi, iscsi_url->target); + parse_header_digest(iscsi, iscsi_url->target, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + ret = -EINVAL; + goto out; + } if (iscsi_full_connect_sync(iscsi, iscsi_url->portal, iscsi_url->lun) != 0) { - error_report("iSCSI: Failed to connect to LUN : %s", + error_setg(errp, "iSCSI: Failed to connect to LUN : %s", iscsi_get_error(iscsi)); ret = -EINVAL; goto out; @@ -1203,14 +1201,14 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, task = iscsi_inquiry_sync(iscsi, iscsilun->lun, 0, 0, 36); if (task == NULL || task->status != SCSI_STATUS_GOOD) { - error_report("iSCSI: failed to send inquiry command."); + error_setg(errp, "iSCSI: failed to send inquiry command."); ret = -EINVAL; goto out; } inq = scsi_datain_unmarshall(task); if (inq == NULL) { - error_report("iSCSI: Failed to unmarshall inquiry data."); + error_setg(errp, "iSCSI: Failed to unmarshall inquiry data."); ret = -EINVAL; goto out; } @@ -1218,7 +1216,9 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, iscsilun->type = inq->periperal_device_type; iscsilun->has_write_same = true; - if ((ret = iscsi_readcapacity_sync(iscsilun)) != 0) { + iscsi_readcapacity_sync(iscsilun, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); goto out; } bs->total_sectors = sector_lun2qemu(iscsilun->num_blocks, iscsilun); @@ -1236,14 +1236,15 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, if (iscsilun->lbpme) { struct scsi_inquiry_logical_block_provisioning *inq_lbp; task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, - SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING); + SCSI_INQUIRY_PAGECODE_LOGICAL_BLOCK_PROVISIONING, + errp); if (task == NULL) { ret = -EINVAL; goto out; } inq_lbp = scsi_datain_unmarshall(task); if (inq_lbp == NULL) { - error_report("iSCSI: failed to unmarshall inquiry datain blob"); + error_setg(errp, "iSCSI: failed to unmarshall inquiry datain blob"); ret = -EINVAL; goto out; } @@ -1256,14 +1257,14 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, if (iscsilun->lbp.lbpu || iscsilun->lbp.lbpws) { struct scsi_inquiry_block_limits *inq_bl; task = iscsi_do_inquiry(iscsilun->iscsi, iscsilun->lun, 1, - SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS); + SCSI_INQUIRY_PAGECODE_BLOCK_LIMITS, errp); if (task == NULL) { ret = -EINVAL; goto out; } inq_bl = scsi_datain_unmarshall(task); if (inq_bl == NULL) { - error_report("iSCSI: failed to unmarshall inquiry datain blob"); + error_setg(errp, "iSCSI: failed to unmarshall inquiry datain blob"); ret = -EINVAL; goto out; } @@ -1354,14 +1355,16 @@ static int iscsi_reopen_prepare(BDRVReopenState *state, static int iscsi_truncate(BlockDriverState *bs, int64_t offset) { IscsiLun *iscsilun = bs->opaque; - int ret = 0; + Error *local_err = NULL; if (iscsilun->type != TYPE_DISK) { return -ENOTSUP; } - if ((ret = iscsi_readcapacity_sync(iscsilun)) != 0) { - return ret; + iscsi_readcapacity_sync(iscsilun, &local_err); + if (local_err != NULL) { + error_free(local_err); + return -EIO; } if (offset > iscsi_getlength(bs)) { -- cgit v1.2.1 From 24897a767bd778fc6a050537d024565f9272cd06 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:54 +0100 Subject: gluster: default scheme to gluster:// and host to localhost. Currently, "gluster:///volname/img" and (using file. options) "file.driver=gluster,file.filename=foo" will segfault. Also, "//host/volname/img" will be rejected, but it is a valid URL that should be accepted just fine with "file.driver=gluster". Accept all of these, by inferring missing transport and host as TCP and localhost respectively. Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/gluster.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'block') diff --git a/block/gluster.c b/block/gluster.c index 58eab07829..89450efd2c 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -127,7 +127,7 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) } /* transport */ - if (!strcmp(uri->scheme, "gluster")) { + if (!uri->scheme || !strcmp(uri->scheme, "gluster")) { gconf->transport = g_strdup("tcp"); } else if (!strcmp(uri->scheme, "gluster+tcp")) { gconf->transport = g_strdup("tcp"); @@ -163,7 +163,7 @@ static int qemu_gluster_parseuri(GlusterConf *gconf, const char *filename) } gconf->server = g_strdup(qp->p[0].value); } else { - gconf->server = g_strdup(uri->server); + gconf->server = g_strdup(uri->server ? uri->server : "localhost"); gconf->port = uri->port; } -- cgit v1.2.1 From a7451cb850d115f257080aff3fbc54f255ebf8f7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:55 +0100 Subject: gluster: correctly propagate errors Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/gluster.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'block') diff --git a/block/gluster.c b/block/gluster.c index 89450efd2c..14d390b4c7 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -175,7 +175,8 @@ out: return ret; } -static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename) +static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename, + Error **errp) { struct glfs *glfs = NULL; int ret; @@ -183,8 +184,8 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename) ret = qemu_gluster_parseuri(gconf, filename); if (ret < 0) { - error_report("Usage: file=gluster[+transport]://[server[:port]]/" - "volname/image[?socket=...]"); + error_setg(errp, "Usage: file=gluster[+transport]://[server[:port]]/" + "volname/image[?socket=...]"); errno = -ret; goto out; } @@ -211,9 +212,11 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename) ret = glfs_init(glfs); if (ret) { - error_report("Gluster connection failed for server=%s port=%d " - "volume=%s image=%s transport=%s", gconf->server, gconf->port, - gconf->volname, gconf->image, gconf->transport); + error_setg_errno(errp, errno, + "Gluster connection failed for server=%s port=%d " + "volume=%s image=%s transport=%s", gconf->server, + gconf->port, gconf->volname, gconf->image, + gconf->transport); goto out; } return glfs; @@ -283,15 +286,14 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options, opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); ret = -EINVAL; goto out; } filename = qemu_opt_get(opts, "filename"); - s->glfs = qemu_gluster_init(gconf, filename); + s->glfs = qemu_gluster_init(gconf, filename, errp); if (!s->glfs) { ret = -errno; goto out; @@ -389,9 +391,9 @@ static int qemu_gluster_create(const char *filename, int64_t total_size = 0; GlusterConf *gconf = g_malloc0(sizeof(GlusterConf)); - glfs = qemu_gluster_init(gconf, filename); + glfs = qemu_gluster_init(gconf, filename, errp); if (!glfs) { - ret = -errno; + ret = -EINVAL; goto out; } -- cgit v1.2.1 From f8d924e48167ec14ec4556441ec7999a30ef6640 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:56 +0100 Subject: cow: correctly propagate errors Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/cow.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'block') diff --git a/block/cow.c b/block/cow.c index d0385be1c5..9e4f624b01 100644 --- a/block/cow.c +++ b/block/cow.c @@ -82,7 +82,7 @@ static int cow_open(BlockDriverState *bs, QDict *options, int flags, char version[64]; snprintf(version, sizeof(version), "COW version %d", cow_header.version); - qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, + error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, bs->device_name, "cow", version); ret = -ENOTSUP; goto fail; @@ -346,8 +346,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options, ret = bdrv_create_file(filename, options, &local_err); if (ret < 0) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); return ret; } @@ -355,8 +354,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options, ret = bdrv_open(&cow_bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err); if (ret < 0) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); return ret; } -- cgit v1.2.1 From 2a94fee3f649bdd2d71c78bb56977284f096f842 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:57 +0100 Subject: curl: correctly propagate errors Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/curl.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'block') diff --git a/block/curl.c b/block/curl.c index bb1fc4ae28..3494c6d662 100644 --- a/block/curl.c +++ b/block/curl.c @@ -456,30 +456,27 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, static int inited = 0; if (flags & BDRV_O_RDWR) { - qerror_report(ERROR_CLASS_GENERIC_ERROR, - "curl block device does not support writes"); + error_setg(errp, "curl block device does not support writes"); return -EROFS; } opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); goto out_noclean; } s->readahead_size = qemu_opt_get_size(opts, "readahead", READ_AHEAD_SIZE); if ((s->readahead_size & 0x1ff) != 0) { - fprintf(stderr, "HTTP_READAHEAD_SIZE %zd is not a multiple of 512\n", - s->readahead_size); + error_setg(errp, "HTTP_READAHEAD_SIZE %zd is not a multiple of 512", + s->readahead_size); goto out_noclean; } file = qemu_opt_get(opts, "url"); if (file == NULL) { - qerror_report(ERROR_CLASS_GENERIC_ERROR, "curl block driver requires " - "an 'url' option"); + error_setg(errp, "curl block driver requires an 'url' option"); goto out_noclean; } -- cgit v1.2.1 From b6d5066d32f9e6c3d7508c1af9ae78327a927120 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:58 +0100 Subject: qcow: correctly propagate errors Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/qcow.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'block') diff --git a/block/qcow.c b/block/qcow.c index 8d00853ab5..ef8920bf07 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -119,17 +119,19 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, if (header.version != QCOW_VERSION) { char version[64]; snprintf(version, sizeof(version), "QCOW version %d", header.version); - qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, - bs->device_name, "qcow", version); + error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, + bs->device_name, "qcow", version); ret = -ENOTSUP; goto fail; } if (header.size <= 1 || header.cluster_bits < 9) { + error_setg(errp, "invalid value in qcow header"); ret = -EINVAL; goto fail; } if (header.crypt_method > QCOW_CRYPT_AES) { + error_setg(errp, "invalid encryption method in qcow header"); ret = -EINVAL; goto fail; } @@ -686,8 +688,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options, ret = bdrv_create_file(filename, options, &local_err); if (ret < 0) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); return ret; } @@ -695,8 +696,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options, ret = bdrv_open(&qcow_bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err); if (ret < 0) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); return ret; } -- cgit v1.2.1 From 0fea6b797202c9efea534a474220a1cf23dd1968 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:43:59 +0100 Subject: qed: correctly propagate errors Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/qed.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'block') diff --git a/block/qed.c b/block/qed.c index 9bc181f041..968028ecbd 100644 --- a/block/qed.c +++ b/block/qed.c @@ -398,7 +398,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, char buf[64]; snprintf(buf, sizeof(buf), "%" PRIx64, s->header.features & ~QED_FEATURE_MASK); - qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, + error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, bs->device_name, "QED", buf); return -ENOTSUP; } @@ -545,7 +545,8 @@ static void bdrv_qed_close(BlockDriverState *bs) static int qed_create(const char *filename, uint32_t cluster_size, uint64_t image_size, uint32_t table_size, - const char *backing_file, const char *backing_fmt) + const char *backing_file, const char *backing_fmt, + Error **errp) { QEDHeader header = { .magic = QED_MAGIC, @@ -566,8 +567,7 @@ static int qed_create(const char *filename, uint32_t cluster_size, ret = bdrv_create_file(filename, NULL, &local_err); if (ret < 0) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); return ret; } @@ -576,8 +576,7 @@ static int qed_create(const char *filename, uint32_t cluster_size, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL, NULL, &local_err); if (ret < 0) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); return ret; } @@ -667,7 +666,7 @@ static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options, } return qed_create(filename, cluster_size, image_size, table_size, - backing_file, backing_fmt); + backing_file, backing_fmt, errp); } typedef struct { -- cgit v1.2.1 From 6890aad46b14849318053fe3ace6109e0f9c5932 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:44:00 +0100 Subject: vhdx: correctly propagate errors Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vhdx.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'block') diff --git a/block/vhdx.c b/block/vhdx.c index 366ff2e627..5390ba6d0f 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -402,9 +402,10 @@ int vhdx_update_headers(BlockDriverState *bs, BDRVVHDXState *s, } /* opens the specified header block from the VHDX file header section */ -static int vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s) +static void vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s, + Error **errp) { - int ret = 0; + int ret; VHDXHeader *header1; VHDXHeader *header2; bool h1_valid = false; @@ -462,7 +463,6 @@ static int vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s) } else if (!h1_valid && h2_valid) { s->curr_header = 1; } else if (!h1_valid && !h2_valid) { - ret = -EINVAL; goto fail; } else { /* If both headers are valid, then we choose the active one by the @@ -473,27 +473,22 @@ static int vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s) } else if (h2_seq > h1_seq) { s->curr_header = 1; } else { - ret = -EINVAL; goto fail; } } vhdx_region_register(s, s->headers[s->curr_header]->log_offset, s->headers[s->curr_header]->log_length); - - ret = 0; - goto exit; fail: - qerror_report(ERROR_CLASS_GENERIC_ERROR, "No valid VHDX header found"); + error_setg_errno(errp, -ret, "No valid VHDX header found"); qemu_vfree(header1); qemu_vfree(header2); s->headers[0] = NULL; s->headers[1] = NULL; exit: qemu_vfree(buffer); - return ret; } @@ -878,7 +873,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags, int ret = 0; uint32_t i; uint64_t signature; - + Error *local_err = NULL; s->bat = NULL; s->first_visible_write = true; @@ -901,8 +896,10 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags, * header update */ vhdx_guid_generate(&s->session_guid); - ret = vhdx_parse_header(bs, s); - if (ret < 0) { + vhdx_parse_header(bs, s, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + ret = -EINVAL; goto fail; } -- cgit v1.2.1 From c0f92b526dbd45fc5b539f51b7379156814dafe9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:44:01 +0100 Subject: vvfat: correctly propagate errors Before: $ ./qemu-io-old qemu-io-old> open -r -o driver=vvfat,fat-type=24,dir=i386-softmmu Valid FAT types are only 12, 16 and 32 qemu-io-old: can't open device (null): Could not open image: Invalid argument After: $ ./qemu-io qemu-io> open -r -o driver=vvfat,fat-type=24,dir=i386-softmmu qemu-io: can't open device (null): Valid FAT types are only 12, 16 and 32 Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vvfat.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'block') diff --git a/block/vvfat.c b/block/vvfat.c index 4bde3bd220..f966ea5da8 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1086,16 +1086,14 @@ DLOG(if (stderr == NULL) { opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); ret = -EINVAL; goto fail; } dirname = qemu_opt_get(opts, "dir"); if (!dirname) { - qerror_report(ERROR_CLASS_GENERIC_ERROR, "vvfat block driver requires " - "a 'dir' option"); + error_setg(errp, "vvfat block driver requires a 'dir' option"); ret = -EINVAL; goto fail; } @@ -1135,8 +1133,7 @@ DLOG(if (stderr == NULL) { case 12: break; default: - qerror_report(ERROR_CLASS_GENERIC_ERROR, "Valid FAT types are only " - "12, 16 and 32"); + error_setg(errp, "Valid FAT types are only 12, 16 and 32"); ret = -EINVAL; goto fail; } -- cgit v1.2.1 From a8842e6d2acc815e9660cc743bd0b0daba18c935 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:44:02 +0100 Subject: vmdk: extract vmdk_read_desc Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) (limited to 'block') diff --git a/block/vmdk.c b/block/vmdk.c index 54a1c59427..b80810802a 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -529,6 +529,32 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, static int vmdk_open_desc_file(BlockDriverState *bs, int flags, uint64_t desc_offset, Error **errp); +static char *vmdk_read_desc(BlockDriverState *file, uint64_t desc_offset, + Error **errp) +{ + int64_t size; + char *buf; + int ret; + + size = bdrv_getlength(file); + if (size < 0) { + error_setg_errno(errp, -size, "Could not access file"); + return NULL; + } + + size = MIN(size, 1 << 20); /* avoid unbounded allocation */ + buf = g_malloc0(size + 1); + + ret = bdrv_pread(file, desc_offset, buf, size); + if (ret < 0) { + error_setg_errno(errp, -ret, "Could not read from file"); + g_free(buf); + return NULL; + } + + return buf; +} + static int vmdk_open_vmdk4(BlockDriverState *bs, BlockDriverState *file, int flags, Error **errp) @@ -823,23 +849,16 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, uint64_t desc_offset, Error **errp) { int ret; - char *buf = NULL; + char *buf; char ct[128]; BDRVVmdkState *s = bs->opaque; - int64_t size; - size = bdrv_getlength(bs->file); - if (size < 0) { + buf = vmdk_read_desc(bs->file, desc_offset, errp); + if (!buf) { return -EINVAL; - } - - size = MIN(size, 1 << 20); /* avoid unbounded allocation */ - buf = g_malloc0(size + 1); - - ret = bdrv_pread(bs->file, desc_offset, buf, size); - if (ret < 0) { goto exit; } + if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) { ret = -EMEDIUMTYPE; goto exit; -- cgit v1.2.1 From d1833ef52be349e41d17e9c5ddaea8bb4ad3a7fb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:44:03 +0100 Subject: vmdk: push vmdk_read_desc up to caller Currently, we just try reading a VMDK file as both image and descriptor. This makes it hard to choose which of the two attempts gave the best error. We'll decide in advance if the file looks like an image or a descriptor, and this patch is the first step to that end. Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 55 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 24 deletions(-) (limited to 'block') diff --git a/block/vmdk.c b/block/vmdk.c index b80810802a..f34c16db7c 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -526,8 +526,8 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, return ret; } -static int vmdk_open_desc_file(BlockDriverState *bs, int flags, - uint64_t desc_offset, Error **errp); +static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, + Error **errp); static char *vmdk_read_desc(BlockDriverState *file, uint64_t desc_offset, Error **errp) @@ -576,7 +576,13 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, if (header.capacity == 0) { uint64_t desc_offset = le64_to_cpu(header.desc_offset); if (desc_offset) { - return vmdk_open_desc_file(bs, flags, desc_offset << 9, errp); + char *buf = vmdk_read_desc(file, desc_offset << 9, errp); + if (!buf) { + return -EINVAL; + } + ret = vmdk_open_desc_file(bs, flags, buf, errp); + g_free(buf); + return ret; } } @@ -727,16 +733,12 @@ static int vmdk_parse_description(const char *desc, const char *opt_name, /* Open an extent file and append to bs array */ static int vmdk_open_sparse(BlockDriverState *bs, - BlockDriverState *file, - int flags, Error **errp) + BlockDriverState *file, int flags, + char *buf, Error **errp) { uint32_t magic; - if (bdrv_pread(file, 0, &magic, sizeof(magic)) != sizeof(magic)) { - return -EIO; - } - - magic = be32_to_cpu(magic); + magic = ldl_be_p(buf); switch (magic) { case VMDK3_MAGIC: return vmdk_open_vmfs_sparse(bs, file, flags, errp); @@ -821,8 +823,14 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, extent->flat_start_offset = flat_offset << 9; } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) { /* SPARSE extent and VMFSSPARSE extent are both "COWD" sparse file*/ - ret = vmdk_open_sparse(bs, extent_file, bs->open_flags, errp); + char *buf = vmdk_read_desc(extent_file, 0, errp); + if (!buf) { + ret = -EINVAL; + } else { + ret = vmdk_open_sparse(bs, extent_file, bs->open_flags, buf, errp); + } if (ret) { + g_free(buf); bdrv_unref(extent_file); return ret; } @@ -845,20 +853,13 @@ next_line: return 0; } -static int vmdk_open_desc_file(BlockDriverState *bs, int flags, - uint64_t desc_offset, Error **errp) +static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, + Error **errp) { int ret; - char *buf; char ct[128]; BDRVVmdkState *s = bs->opaque; - buf = vmdk_read_desc(bs->file, desc_offset, errp); - if (!buf) { - return -EINVAL; - goto exit; - } - if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) { ret = -EMEDIUMTYPE; goto exit; @@ -876,20 +877,25 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, s->desc_offset = 0; ret = vmdk_parse_extents(buf, bs, bs->file->filename, errp); exit: - g_free(buf); return ret; } static int vmdk_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { + char *buf = NULL; int ret; BDRVVmdkState *s = bs->opaque; - if (vmdk_open_sparse(bs, bs->file, flags, errp) == 0) { + buf = vmdk_read_desc(bs->file, 0, errp); + if (!buf) { + return -EINVAL; + } + + if (vmdk_open_sparse(bs, bs->file, flags, buf, errp) == 0) { s->desc_offset = 0x200; } else { - ret = vmdk_open_desc_file(bs, flags, 0, errp); + ret = vmdk_open_desc_file(bs, flags, buf, errp); if (ret) { goto fail; } @@ -908,10 +914,11 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED, "vmdk", bs->device_name, "live migration"); migrate_add_blocker(s->migration_blocker); - + g_free(buf); return 0; fail: + g_free(buf); g_free(s->create_type); s->create_type = NULL; vmdk_free_extents(bs); -- cgit v1.2.1 From 37f09e5e3d206e7c555d28a7755cecfa137dad22 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:44:04 +0100 Subject: vmdk: do not try opening a file as both image and descriptor This prepares for propagating errors from vmdk_open_sparse and vmdk_open_desc_file up to the caller of vmdk_open. Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) (limited to 'block') diff --git a/block/vmdk.c b/block/vmdk.c index f34c16db7c..50a279a8d8 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -886,20 +886,28 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags, char *buf = NULL; int ret; BDRVVmdkState *s = bs->opaque; + uint32_t magic; buf = vmdk_read_desc(bs->file, 0, errp); if (!buf) { return -EINVAL; } - if (vmdk_open_sparse(bs, bs->file, flags, buf, errp) == 0) { - s->desc_offset = 0x200; - } else { - ret = vmdk_open_desc_file(bs, flags, buf, errp); - if (ret) { - goto fail; - } + magic = ldl_be_p(buf); + switch (magic) { + case VMDK3_MAGIC: + case VMDK4_MAGIC: + ret = vmdk_open_sparse(bs, bs->file, flags, buf, errp); + s->desc_offset = 0x200; + break; + default: + ret = vmdk_open_desc_file(bs, flags, buf, errp); + break; } + if (ret) { + goto fail; + } + /* try to open parent images, if exist */ ret = vmdk_parent_open(bs); if (ret) { -- cgit v1.2.1 From 89ac8480a8c7f73dd943dcb1313d6bd984f9a870 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:44:05 +0100 Subject: vmdk: correctly propagate errors Now that we can return the "right" errors, use the Error** parameter to pass them back instead of just printing them. Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'block') diff --git a/block/vmdk.c b/block/vmdk.c index 50a279a8d8..9c3711cbbf 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -572,6 +572,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, error_setg_errno(errp, -ret, "Could not read header from file '%s'", file->filename); + return -EINVAL; } if (header.capacity == 0) { uint64_t desc_offset = le64_to_cpu(header.desc_offset); @@ -641,8 +642,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, char buf[64]; snprintf(buf, sizeof(buf), "VMDK version %d", le32_to_cpu(header.version)); - qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, - bs->device_name, "vmdk", buf); + error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, + bs->device_name, "vmdk", buf); return -ENOTSUP; } else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) { /* VMware KB 2064959 explains that version 3 added support for @@ -654,7 +655,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, } if (le32_to_cpu(header.num_gtes_per_gt) > 512) { - error_report("L2 table size too big"); + error_setg(errp, "L2 table size too big"); return -EINVAL; } @@ -670,8 +671,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, } if (bdrv_getlength(file) < le64_to_cpu(header.grain_offset) * BDRV_SECTOR_SIZE) { - error_report("File truncated, expecting at least %lld bytes", - le64_to_cpu(header.grain_offset) * BDRV_SECTOR_SIZE); + error_setg(errp, "File truncated, expecting at least %lld bytes", + le64_to_cpu(header.grain_offset) * BDRV_SECTOR_SIZE); return -EINVAL; } -- cgit v1.2.1 From 76abe4071d111a9ca6dcc9b9689a831c39ffa718 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:44:06 +0100 Subject: block: do not abuse EMEDIUMTYPE Returning "Wrong medium type" for an image that does not have a valid header is a bit weird. Improve the error by mentioning what format was trying to open it. Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/bochs.c | 3 ++- block/cow.c | 3 ++- block/parallels.c | 3 ++- block/qcow.c | 3 ++- block/qcow2.c | 2 +- block/qed.c | 3 ++- block/vdi.c | 4 ++-- block/vmdk.c | 6 ++++-- block/vpc.c | 3 ++- 9 files changed, 19 insertions(+), 11 deletions(-) (limited to 'block') diff --git a/block/bochs.c b/block/bochs.c index 51d9a90577..4d6403f904 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -129,7 +129,8 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags, strcmp(bochs.subtype, GROWING_TYPE) || ((le32_to_cpu(bochs.version) != HEADER_VERSION) && (le32_to_cpu(bochs.version) != HEADER_V1))) { - return -EMEDIUMTYPE; + error_setg(errp, "Image not in Bochs format"); + return -EINVAL; } if (le32_to_cpu(bochs.version) == HEADER_V1) { diff --git a/block/cow.c b/block/cow.c index 9e4f624b01..30deb88deb 100644 --- a/block/cow.c +++ b/block/cow.c @@ -74,7 +74,8 @@ static int cow_open(BlockDriverState *bs, QDict *options, int flags, } if (be32_to_cpu(cow_header.magic) != COW_MAGIC) { - ret = -EMEDIUMTYPE; + error_setg(errp, "Image not in COW format"); + ret = -EINVAL; goto fail; } diff --git a/block/parallels.c b/block/parallels.c index 2121e43204..3f588f58dc 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -85,7 +85,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, if (memcmp(ph.magic, HEADER_MAGIC, 16) || (le32_to_cpu(ph.version) != HEADER_VERSION)) { - ret = -EMEDIUMTYPE; + error_setg(errp, "Image not in Parallels format"); + ret = -EINVAL; goto fail; } diff --git a/block/qcow.c b/block/qcow.c index ef8920bf07..1e128becf0 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -113,7 +113,8 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, be64_to_cpus(&header.l1_table_offset); if (header.magic != QCOW_MAGIC) { - ret = -EMEDIUMTYPE; + error_setg(errp, "Image not in qcow format"); + ret = -EINVAL; goto fail; } if (header.version != QCOW_VERSION) { diff --git a/block/qcow2.c b/block/qcow2.c index 9dfd90896b..cfe80befa0 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -449,7 +449,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, if (header.magic != QCOW_MAGIC) { error_setg(errp, "Image is not in qcow2 format"); - ret = -EMEDIUMTYPE; + ret = -EINVAL; goto fail; } if (header.version < 2 || header.version > 3) { diff --git a/block/qed.c b/block/qed.c index 968028ecbd..8802ad3845 100644 --- a/block/qed.c +++ b/block/qed.c @@ -391,7 +391,8 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags, qed_header_le_to_cpu(&le_header, &s->header); if (s->header.magic != QED_MAGIC) { - return -EMEDIUMTYPE; + error_setg(errp, "Image not in QED format"); + return -EINVAL; } if (s->header.features & ~QED_FEATURE_MASK) { /* image uses unsupported feature bits */ diff --git a/block/vdi.c b/block/vdi.c index 2d7490f173..f3c6acf3cf 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -395,8 +395,8 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, } if (header.signature != VDI_SIGNATURE) { - logout("bad vdi signature %08x\n", header.signature); - ret = -EMEDIUMTYPE; + error_setg(errp, "Image not in VDI format (bad signature %08x)", header.signature); + ret = -EINVAL; goto fail; } else if (header.version != VDI_VERSION_1_1) { logout("unsupported version %u.%u\n", diff --git a/block/vmdk.c b/block/vmdk.c index 9c3711cbbf..83839f9b7a 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -748,7 +748,8 @@ static int vmdk_open_sparse(BlockDriverState *bs, return vmdk_open_vmdk4(bs, file, flags, errp); break; default: - return -EMEDIUMTYPE; + error_setg(errp, "Image not in VMDK format"); + return -EINVAL; break; } } @@ -862,7 +863,8 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, BDRVVmdkState *s = bs->opaque; if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) { - ret = -EMEDIUMTYPE; + error_setg(errp, "invalid VMDK image descriptor"); + ret = -EINVAL; goto exit; } if (strcmp(ct, "monolithicFlat") && diff --git a/block/vpc.c b/block/vpc.c index 1d326cbf44..82bf2485a5 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -190,7 +190,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } if (strncmp(footer->creator, "conectix", 8)) { - ret = -EMEDIUMTYPE; + error_setg(errp, "invalid VPC image"); + ret = -EINVAL; goto fail; } disk_type = VHD_FIXED; -- cgit v1.2.1 From 5b7aa9b56d1bfc79916262f380c3fc7961becb50 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Feb 2014 14:44:07 +0100 Subject: vdi: say why an image is bad Instead of just putting it in debugging output, we can now put the value in an Error. Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vdi.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'block') diff --git a/block/vdi.c b/block/vdi.c index f3c6acf3cf..ae49cd83ca 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -399,39 +399,46 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, ret = -EINVAL; goto fail; } else if (header.version != VDI_VERSION_1_1) { - logout("unsupported version %u.%u\n", - header.version >> 16, header.version & 0xffff); + error_setg(errp, "unsupported VDI image (version %u.%u)", + header.version >> 16, header.version & 0xffff); ret = -ENOTSUP; goto fail; } else if (header.offset_bmap % SECTOR_SIZE != 0) { /* We only support block maps which start on a sector boundary. */ - logout("unsupported block map offset 0x%x B\n", header.offset_bmap); + error_setg(errp, "unsupported VDI image (unaligned block map offset " + "0x%x)", header.offset_bmap); ret = -ENOTSUP; goto fail; } else if (header.offset_data % SECTOR_SIZE != 0) { /* We only support data blocks which start on a sector boundary. */ - logout("unsupported data offset 0x%x B\n", header.offset_data); + error_setg(errp, "unsupported VDI image (unaligned data offset 0x%x)", + header.offset_data); ret = -ENOTSUP; goto fail; } else if (header.sector_size != SECTOR_SIZE) { - logout("unsupported sector size %u B\n", header.sector_size); + error_setg(errp, "unsupported VDI image (sector size %u is not %u)", + header.sector_size, SECTOR_SIZE); ret = -ENOTSUP; goto fail; } else if (header.block_size != 1 * MiB) { - logout("unsupported block size %u B\n", header.block_size); + error_setg(errp, "unsupported VDI image (sector size %u is not %u)", + header.block_size, 1 * MiB); ret = -ENOTSUP; goto fail; } else if (header.disk_size > (uint64_t)header.blocks_in_image * header.block_size) { - logout("unsupported disk size %" PRIu64 " B\n", header.disk_size); + error_setg(errp, "unsupported VDI image (disk size %" PRIu64 ", " + "image bitmap has room for %" PRIu64 ")", + header.disk_size, + (uint64_t)header.blocks_in_image * header.block_size); ret = -ENOTSUP; goto fail; } else if (!uuid_is_null(header.uuid_link)) { - logout("link uuid != 0, unsupported\n"); + error_setg(errp, "unsupported VDI image (non-NULL link UUID)"); ret = -ENOTSUP; goto fail; } else if (!uuid_is_null(header.uuid_parent)) { - logout("parent uuid != 0, unsupported\n"); + error_setg(errp, "unsupported VDI image (non-NULL parent UUID)"); ret = -ENOTSUP; goto fail; } -- cgit v1.2.1 From 27cec15e4ed4e69155f2499ceb46d22d8425102a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:10 +0100 Subject: quorum: Create quorum.c, add QuorumChildRequest and QuorumAIOCB. Quorum is a block filter mirroring writes to num_children children. For reads quorum reads each children and does a vote. If more than vote_threshold versions are identical the quorum is reached and this winning version is returned to the guest. So quorum prevents bit corruption. For high availability purpose minority errors are reported via QMP but the guest does not see them. This patch creates the driver C source file and introduces the structures that will be used in asynchronous reads and writes. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/Makefile.objs | 1 + block/quorum.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 block/quorum.c (limited to 'block') diff --git a/block/Makefile.objs b/block/Makefile.objs index e254a2180a..716556f6ec 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -3,6 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o block-obj-y += qed-check.o block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o +block-obj-y += quorum.o block-obj-y += parallels.o blkdebug.o blkverify.o block-obj-y += snapshot.o qapi.o block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o diff --git a/block/quorum.c b/block/quorum.c new file mode 100644 index 0000000000..950f5cc990 --- /dev/null +++ b/block/quorum.c @@ -0,0 +1,53 @@ +/* + * Quorum Block filter + * + * Copyright (C) 2012-2014 Nodalink, EURL. + * + * Author: + * Benoît Canet + * + * Based on the design and code of blkverify.c (Copyright (C) 2010 IBM, Corp) + * and blkmirror.c (Copyright (C) 2011 Red Hat, Inc). + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "block/block_int.h" + +typedef struct QuorumAIOCB QuorumAIOCB; + +/* Quorum will create one instance of the following structure per operation it + * performs on its children. + * So for each read/write operation coming from the upper layer there will be + * $children_count QuorumChildRequest. + */ +typedef struct QuorumChildRequest { + BlockDriverAIOCB *aiocb; + QEMUIOVector qiov; + uint8_t *buf; + int ret; + QuorumAIOCB *parent; +} QuorumChildRequest; + +/* Quorum will use the following structure to track progress of each read/write + * operation received by the upper layer. + * This structure hold pointers to the QuorumChildRequest structures instances + * used to do operations on each children and track overall progress. + */ +struct QuorumAIOCB { + BlockDriverAIOCB common; + + /* Request metadata */ + uint64_t sector_num; + int nb_sectors; + + QEMUIOVector *qiov; /* calling IOV */ + + QuorumChildRequest *qcrs; /* individual child requests */ + int count; /* number of completed AIOCB */ + int success_count; /* number of successfully completed AIOCB */ + + bool is_read; + int vote_ret; +}; -- cgit v1.2.1 From cadebd7a2a590c2ac5ced58c2fc207c7ae78fb1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:11 +0100 Subject: quorum: Create BDRVQuorumState and BlkDriver and do init. Create the structure holding the quorum settings and write the minimal block driver instanciation boilerplate. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 950f5cc990..375ffd5963 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -15,6 +15,23 @@ #include "block/block_int.h" +/* the following structure holds the state of one quorum instance */ +typedef struct BDRVQuorumState { + BlockDriverState **bs; /* children BlockDriverStates */ + int num_children; /* children count */ + int threshold; /* if less than threshold children reads gave the + * same result a quorum error occurs. + */ + bool is_blkverify; /* true if the driver is in blkverify mode + * Writes are mirrored on two children devices. + * On reads the two children devices' contents are + * compared and if a difference is spotted its + * location is printed and the code aborts. + * It is useful to debug other block drivers by + * comparing them with a reference one. + */ +} BDRVQuorumState; + typedef struct QuorumAIOCB QuorumAIOCB; /* Quorum will create one instance of the following structure per operation it @@ -51,3 +68,17 @@ struct QuorumAIOCB { bool is_read; int vote_ret; }; + +static BlockDriver bdrv_quorum = { + .format_name = "quorum", + .protocol_name = "quorum", + + .instance_size = sizeof(BDRVQuorumState), +}; + +static void bdrv_quorum_init(void) +{ + bdrv_register(&bdrv_quorum); +} + +block_init(bdrv_quorum_init); -- cgit v1.2.1 From 13e7956e3190b51f02e75374bb9dfdcacfd08829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:12 +0100 Subject: quorum: Add quorum_aio_writev and its dependencies. Writes are mirrored num_children times on num_children devices. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 375ffd5963..99e0e03cdf 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -69,11 +69,114 @@ struct QuorumAIOCB { int vote_ret; }; +static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) +{ + QuorumAIOCB *acb = container_of(blockacb, QuorumAIOCB, common); + BDRVQuorumState *s = acb->common.bs->opaque; + int i; + + /* cancel all callbacks */ + for (i = 0; i < s->num_children; i++) { + bdrv_aio_cancel(acb->qcrs[i].aiocb); + } + + g_free(acb->qcrs); + qemu_aio_release(acb); +} + +static AIOCBInfo quorum_aiocb_info = { + .aiocb_size = sizeof(QuorumAIOCB), + .cancel = quorum_aio_cancel, +}; + +static void quorum_aio_finalize(QuorumAIOCB *acb) +{ + int ret = 0; + + acb->common.cb(acb->common.opaque, ret); + + g_free(acb->qcrs); + qemu_aio_release(acb); +} + +static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, + BlockDriverState *bs, + QEMUIOVector *qiov, + uint64_t sector_num, + int nb_sectors, + BlockDriverCompletionFunc *cb, + void *opaque) +{ + QuorumAIOCB *acb = qemu_aio_get(&quorum_aiocb_info, bs, cb, opaque); + int i; + + acb->common.bs->opaque = s; + acb->sector_num = sector_num; + acb->nb_sectors = nb_sectors; + acb->qiov = qiov; + acb->qcrs = g_new0(QuorumChildRequest, s->num_children); + acb->count = 0; + acb->success_count = 0; + acb->is_read = false; + acb->vote_ret = 0; + + for (i = 0; i < s->num_children; i++) { + acb->qcrs[i].buf = NULL; + acb->qcrs[i].ret = 0; + acb->qcrs[i].parent = acb; + } + + return acb; +} + +static void quorum_aio_cb(void *opaque, int ret) +{ + QuorumChildRequest *sacb = opaque; + QuorumAIOCB *acb = sacb->parent; + BDRVQuorumState *s = acb->common.bs->opaque; + + sacb->ret = ret; + acb->count++; + if (ret == 0) { + acb->success_count++; + } + assert(acb->count <= s->num_children); + assert(acb->success_count <= s->num_children); + if (acb->count < s->num_children) { + return; + } + + quorum_aio_finalize(acb); +} + +static BlockDriverAIOCB *quorum_aio_writev(BlockDriverState *bs, + int64_t sector_num, + QEMUIOVector *qiov, + int nb_sectors, + BlockDriverCompletionFunc *cb, + void *opaque) +{ + BDRVQuorumState *s = bs->opaque; + QuorumAIOCB *acb = quorum_aio_get(s, bs, qiov, sector_num, nb_sectors, + cb, opaque); + int i; + + for (i = 0; i < s->num_children; i++) { + acb->qcrs[i].aiocb = bdrv_aio_writev(s->bs[i], sector_num, qiov, + nb_sectors, &quorum_aio_cb, + &acb->qcrs[i]); + } + + return &acb->common; +} + static BlockDriver bdrv_quorum = { .format_name = "quorum", .protocol_name = "quorum", .instance_size = sizeof(BDRVQuorumState), + + .bdrv_aio_writev = quorum_aio_writev, }; static void bdrv_quorum_init(void) -- cgit v1.2.1 From f70d7f7e4d05b7a7797815afdcb83f4375740838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:13 +0100 Subject: blkverify: Extract qemu_iovec_clone() and qemu_iovec_compare() from blkverify. qemu_iovec_compare() will be used to compare IOs vectors in quorum blkverify mode. The patch extracts these functions in order to factorize the code. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/blkverify.c | 108 +----------------------------------------------------- 1 file changed, 2 insertions(+), 106 deletions(-) (limited to 'block') diff --git a/block/blkverify.c b/block/blkverify.c index 0e28502e8e..b98b08bedf 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -173,110 +173,6 @@ static int64_t blkverify_getlength(BlockDriverState *bs) return bdrv_getlength(s->test_file); } -/** - * Check that I/O vector contents are identical - * - * @a: I/O vector - * @b: I/O vector - * @ret: Offset to first mismatching byte or -1 if match - */ -static ssize_t blkverify_iovec_compare(QEMUIOVector *a, QEMUIOVector *b) -{ - int i; - ssize_t offset = 0; - - assert(a->niov == b->niov); - for (i = 0; i < a->niov; i++) { - size_t len = 0; - uint8_t *p = (uint8_t *)a->iov[i].iov_base; - uint8_t *q = (uint8_t *)b->iov[i].iov_base; - - assert(a->iov[i].iov_len == b->iov[i].iov_len); - while (len < a->iov[i].iov_len && *p++ == *q++) { - len++; - } - - offset += len; - - if (len != a->iov[i].iov_len) { - return offset; - } - } - return -1; -} - -typedef struct { - int src_index; - struct iovec *src_iov; - void *dest_base; -} IOVectorSortElem; - -static int sortelem_cmp_src_base(const void *a, const void *b) -{ - const IOVectorSortElem *elem_a = a; - const IOVectorSortElem *elem_b = b; - - /* Don't overflow */ - if (elem_a->src_iov->iov_base < elem_b->src_iov->iov_base) { - return -1; - } else if (elem_a->src_iov->iov_base > elem_b->src_iov->iov_base) { - return 1; - } else { - return 0; - } -} - -static int sortelem_cmp_src_index(const void *a, const void *b) -{ - const IOVectorSortElem *elem_a = a; - const IOVectorSortElem *elem_b = b; - - return elem_a->src_index - elem_b->src_index; -} - -/** - * Copy contents of I/O vector - * - * The relative relationships of overlapping iovecs are preserved. This is - * necessary to ensure identical semantics in the cloned I/O vector. - */ -static void blkverify_iovec_clone(QEMUIOVector *dest, const QEMUIOVector *src, - void *buf) -{ - IOVectorSortElem sortelems[src->niov]; - void *last_end; - int i; - - /* Sort by source iovecs by base address */ - for (i = 0; i < src->niov; i++) { - sortelems[i].src_index = i; - sortelems[i].src_iov = &src->iov[i]; - } - qsort(sortelems, src->niov, sizeof(sortelems[0]), sortelem_cmp_src_base); - - /* Allocate buffer space taking into account overlapping iovecs */ - last_end = NULL; - for (i = 0; i < src->niov; i++) { - struct iovec *cur = sortelems[i].src_iov; - ptrdiff_t rewind = 0; - - /* Detect overlap */ - if (last_end && last_end > cur->iov_base) { - rewind = last_end - cur->iov_base; - } - - sortelems[i].dest_base = buf - rewind; - buf += cur->iov_len - MIN(rewind, cur->iov_len); - last_end = MAX(cur->iov_base + cur->iov_len, last_end); - } - - /* Sort by source iovec index and build destination iovec */ - qsort(sortelems, src->niov, sizeof(sortelems[0]), sortelem_cmp_src_index); - for (i = 0; i < src->niov; i++) { - qemu_iovec_add(dest, sortelems[i].dest_base, src->iov[i].iov_len); - } -} - static BlkverifyAIOCB *blkverify_aio_get(BlockDriverState *bs, bool is_write, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, @@ -340,7 +236,7 @@ static void blkverify_aio_cb(void *opaque, int ret) static void blkverify_verify_readv(BlkverifyAIOCB *acb) { - ssize_t offset = blkverify_iovec_compare(acb->qiov, &acb->raw_qiov); + ssize_t offset = qemu_iovec_compare(acb->qiov, &acb->raw_qiov); if (offset != -1) { blkverify_err(acb, "contents mismatch in sector %" PRId64, acb->sector_num + (int64_t)(offset / BDRV_SECTOR_SIZE)); @@ -358,7 +254,7 @@ static BlockDriverAIOCB *blkverify_aio_readv(BlockDriverState *bs, acb->verify = blkverify_verify_readv; acb->buf = qemu_blockalign(bs->file, qiov->size); qemu_iovec_init(&acb->raw_qiov, acb->qiov->niov); - blkverify_iovec_clone(&acb->raw_qiov, qiov, acb->buf); + qemu_iovec_clone(&acb->raw_qiov, qiov, acb->buf); bdrv_aio_readv(s->test_file, sector_num, qiov, nb_sectors, blkverify_aio_cb, acb); -- cgit v1.2.1 From 7db6982a19f61e3668397b5e31ebfb16a477c414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:14 +0100 Subject: quorum: Add quorum_aio_readv. Add code to do num_children reads in parallel and cleanup the structures afterwards. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 99e0e03cdf..fd19662bcb 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -91,10 +91,18 @@ static AIOCBInfo quorum_aiocb_info = { static void quorum_aio_finalize(QuorumAIOCB *acb) { - int ret = 0; + BDRVQuorumState *s = acb->common.bs->opaque; + int i, ret = 0; acb->common.cb(acb->common.opaque, ret); + if (acb->is_read) { + for (i = 0; i < s->num_children; i++) { + qemu_vfree(acb->qcrs[i].buf); + qemu_iovec_destroy(&acb->qcrs[i].qiov); + } + } + g_free(acb->qcrs); qemu_aio_release(acb); } @@ -149,6 +157,34 @@ static void quorum_aio_cb(void *opaque, int ret) quorum_aio_finalize(acb); } +static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs, + int64_t sector_num, + QEMUIOVector *qiov, + int nb_sectors, + BlockDriverCompletionFunc *cb, + void *opaque) +{ + BDRVQuorumState *s = bs->opaque; + QuorumAIOCB *acb = quorum_aio_get(s, bs, qiov, sector_num, + nb_sectors, cb, opaque); + int i; + + acb->is_read = true; + + for (i = 0; i < s->num_children; i++) { + acb->qcrs[i].buf = qemu_blockalign(s->bs[i], qiov->size); + qemu_iovec_init(&acb->qcrs[i].qiov, qiov->niov); + qemu_iovec_clone(&acb->qcrs[i].qiov, qiov, acb->qcrs[i].buf); + } + + for (i = 0; i < s->num_children; i++) { + bdrv_aio_readv(s->bs[i], sector_num, qiov, nb_sectors, + quorum_aio_cb, &acb->qcrs[i]); + } + + return &acb->common; +} + static BlockDriverAIOCB *quorum_aio_writev(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, @@ -176,6 +212,7 @@ static BlockDriver bdrv_quorum = { .instance_size = sizeof(BDRVQuorumState), + .bdrv_aio_readv = quorum_aio_readv, .bdrv_aio_writev = quorum_aio_writev, }; -- cgit v1.2.1 From 95c6bff3561eedaf7c7de287bc4a002720605a8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:15 +0100 Subject: quorum: Add quorum mechanism. This patchset enables the core of the quorum mechanism. The num_children reads are compared to get the majority version and if this version exists more than threshold times the guest won't see the error at all. If a block is corrupted or if an error occurs during an IO or if the quorum cannot be established QMP events are used to report to the management. Use gnutls's SHA-256 to compare versions. --enable-quorum must be used to enable the feature. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/Makefile.objs | 2 +- block/quorum.c | 391 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 391 insertions(+), 2 deletions(-) (limited to 'block') diff --git a/block/Makefile.objs b/block/Makefile.objs index 716556f6ec..425d7fbb59 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -3,7 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o block-obj-y += qed-check.o block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o -block-obj-y += quorum.o +block-obj-$(CONFIG_QUORUM) += quorum.o block-obj-y += parallels.o blkdebug.o blkverify.o block-obj-y += snapshot.o qapi.o block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o diff --git a/block/quorum.c b/block/quorum.c index fd19662bcb..4beee96ffa 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -13,7 +13,43 @@ * See the COPYING file in the top-level directory. */ +#include +#include #include "block/block_int.h" +#include "qapi/qmp/qjson.h" + +#define HASH_LENGTH 32 + +/* This union holds a vote hash value */ +typedef union QuorumVoteValue { + char h[HASH_LENGTH]; /* SHA-256 hash */ + int64_t l; /* simpler 64 bits hash */ +} QuorumVoteValue; + +/* A vote item */ +typedef struct QuorumVoteItem { + int index; + QLIST_ENTRY(QuorumVoteItem) next; +} QuorumVoteItem; + +/* this structure is a vote version. A version is the set of votes sharing the + * same vote value. + * The set of votes will be tracked with the items field and its cardinality is + * vote_count. + */ +typedef struct QuorumVoteVersion { + QuorumVoteValue value; + int index; + int vote_count; + QLIST_HEAD(, QuorumVoteItem) items; + QLIST_ENTRY(QuorumVoteVersion) next; +} QuorumVoteVersion; + +/* this structure holds a group of vote versions together */ +typedef struct QuorumVotes { + QLIST_HEAD(, QuorumVoteVersion) vote_list; + bool (*compare)(QuorumVoteValue *a, QuorumVoteValue *b); +} QuorumVotes; /* the following structure holds the state of one quorum instance */ typedef struct BDRVQuorumState { @@ -65,10 +101,14 @@ struct QuorumAIOCB { int count; /* number of completed AIOCB */ int success_count; /* number of successfully completed AIOCB */ + QuorumVotes votes; + bool is_read; int vote_ret; }; +static void quorum_vote(QuorumAIOCB *acb); + static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) { QuorumAIOCB *acb = container_of(blockacb, QuorumAIOCB, common); @@ -94,6 +134,10 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) BDRVQuorumState *s = acb->common.bs->opaque; int i, ret = 0; + if (acb->vote_ret) { + ret = acb->vote_ret; + } + acb->common.cb(acb->common.opaque, ret); if (acb->is_read) { @@ -107,6 +151,16 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) qemu_aio_release(acb); } +static bool quorum_sha256_compare(QuorumVoteValue *a, QuorumVoteValue *b) +{ + return !memcmp(a->h, b->h, HASH_LENGTH); +} + +static bool quorum_64bits_compare(QuorumVoteValue *a, QuorumVoteValue *b) +{ + return a->l == b->l; +} + static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, BlockDriverState *bs, QEMUIOVector *qiov, @@ -125,6 +179,8 @@ static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, acb->qcrs = g_new0(QuorumChildRequest, s->num_children); acb->count = 0; acb->success_count = 0; + acb->votes.compare = quorum_sha256_compare; + QLIST_INIT(&acb->votes.vote_list); acb->is_read = false; acb->vote_ret = 0; @@ -137,6 +193,48 @@ static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, return acb; } +static void quorum_report_bad(QuorumAIOCB *acb, char *node_name, int ret) +{ + QObject *data; + assert(node_name); + data = qobject_from_jsonf("{ 'ret': %d" + ", 'node-name': %s" + ", 'sector-num': %" PRId64 + ", 'sectors-count': %d }", + ret, node_name, acb->sector_num, acb->nb_sectors); + monitor_protocol_event(QEVENT_QUORUM_REPORT_BAD, data); + qobject_decref(data); +} + +static void quorum_report_failure(QuorumAIOCB *acb) +{ + QObject *data; + const char *reference = acb->common.bs->device_name[0] ? + acb->common.bs->device_name : + acb->common.bs->node_name; + data = qobject_from_jsonf("{ 'reference': %s" + ", 'sector-num': %" PRId64 + ", 'sectors-count': %d }", + reference, acb->sector_num, acb->nb_sectors); + monitor_protocol_event(QEVENT_QUORUM_FAILURE, data); + qobject_decref(data); +} + +static int quorum_vote_error(QuorumAIOCB *acb); + +static bool quorum_has_too_much_io_failed(QuorumAIOCB *acb) +{ + BDRVQuorumState *s = acb->common.bs->opaque; + + if (acb->success_count < s->threshold) { + acb->vote_ret = quorum_vote_error(acb); + quorum_report_failure(acb); + return true; + } + + return false; +} + static void quorum_aio_cb(void *opaque, int ret) { QuorumChildRequest *sacb = opaque; @@ -147,6 +245,8 @@ static void quorum_aio_cb(void *opaque, int ret) acb->count++; if (ret == 0) { acb->success_count++; + } else { + quorum_report_bad(acb, sacb->aiocb->bs->node_name, ret); } assert(acb->count <= s->num_children); assert(acb->success_count <= s->num_children); @@ -154,9 +254,298 @@ static void quorum_aio_cb(void *opaque, int ret) return; } + /* Do the vote on read */ + if (acb->is_read) { + quorum_vote(acb); + } else { + quorum_has_too_much_io_failed(acb); + } + quorum_aio_finalize(acb); } +static void quorum_report_bad_versions(BDRVQuorumState *s, + QuorumAIOCB *acb, + QuorumVoteValue *value) +{ + QuorumVoteVersion *version; + QuorumVoteItem *item; + + QLIST_FOREACH(version, &acb->votes.vote_list, next) { + if (acb->votes.compare(&version->value, value)) { + continue; + } + QLIST_FOREACH(item, &version->items, next) { + quorum_report_bad(acb, s->bs[item->index]->node_name, 0); + } + } +} + +static void quorum_copy_qiov(QEMUIOVector *dest, QEMUIOVector *source) +{ + int i; + assert(dest->niov == source->niov); + assert(dest->size == source->size); + for (i = 0; i < source->niov; i++) { + assert(dest->iov[i].iov_len == source->iov[i].iov_len); + memcpy(dest->iov[i].iov_base, + source->iov[i].iov_base, + source->iov[i].iov_len); + } +} + +static void quorum_count_vote(QuorumVotes *votes, + QuorumVoteValue *value, + int index) +{ + QuorumVoteVersion *v = NULL, *version = NULL; + QuorumVoteItem *item; + + /* look if we have something with this hash */ + QLIST_FOREACH(v, &votes->vote_list, next) { + if (votes->compare(&v->value, value)) { + version = v; + break; + } + } + + /* It's a version not yet in the list add it */ + if (!version) { + version = g_new0(QuorumVoteVersion, 1); + QLIST_INIT(&version->items); + memcpy(&version->value, value, sizeof(version->value)); + version->index = index; + version->vote_count = 0; + QLIST_INSERT_HEAD(&votes->vote_list, version, next); + } + + version->vote_count++; + + item = g_new0(QuorumVoteItem, 1); + item->index = index; + QLIST_INSERT_HEAD(&version->items, item, next); +} + +static void quorum_free_vote_list(QuorumVotes *votes) +{ + QuorumVoteVersion *version, *next_version; + QuorumVoteItem *item, *next_item; + + QLIST_FOREACH_SAFE(version, &votes->vote_list, next, next_version) { + QLIST_REMOVE(version, next); + QLIST_FOREACH_SAFE(item, &version->items, next, next_item) { + QLIST_REMOVE(item, next); + g_free(item); + } + g_free(version); + } +} + +static int quorum_compute_hash(QuorumAIOCB *acb, int i, QuorumVoteValue *hash) +{ + int j, ret; + gnutls_hash_hd_t dig; + QEMUIOVector *qiov = &acb->qcrs[i].qiov; + + ret = gnutls_hash_init(&dig, GNUTLS_DIG_SHA256); + + if (ret < 0) { + return ret; + } + + for (j = 0; j < qiov->niov; j++) { + ret = gnutls_hash(dig, qiov->iov[j].iov_base, qiov->iov[j].iov_len); + if (ret < 0) { + break; + } + } + + gnutls_hash_deinit(dig, (void *) hash); + return ret; +} + +static QuorumVoteVersion *quorum_get_vote_winner(QuorumVotes *votes) +{ + int max = 0; + QuorumVoteVersion *candidate, *winner = NULL; + + QLIST_FOREACH(candidate, &votes->vote_list, next) { + if (candidate->vote_count > max) { + max = candidate->vote_count; + winner = candidate; + } + } + + return winner; +} + +/* qemu_iovec_compare is handy for blkverify mode because it returns the first + * differing byte location. Yet it is handcoded to compare vectors one byte + * after another so it does not benefit from the libc SIMD optimizations. + * quorum_iovec_compare is written for speed and should be used in the non + * blkverify mode of quorum. + */ +static bool quorum_iovec_compare(QEMUIOVector *a, QEMUIOVector *b) +{ + int i; + int result; + + assert(a->niov == b->niov); + for (i = 0; i < a->niov; i++) { + assert(a->iov[i].iov_len == b->iov[i].iov_len); + result = memcmp(a->iov[i].iov_base, + b->iov[i].iov_base, + a->iov[i].iov_len); + if (result) { + return false; + } + } + + return true; +} + +static void GCC_FMT_ATTR(2, 3) quorum_err(QuorumAIOCB *acb, + const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + fprintf(stderr, "quorum: sector_num=%" PRId64 " nb_sectors=%d ", + acb->sector_num, acb->nb_sectors); + vfprintf(stderr, fmt, ap); + fprintf(stderr, "\n"); + va_end(ap); + exit(1); +} + +static bool quorum_compare(QuorumAIOCB *acb, + QEMUIOVector *a, + QEMUIOVector *b) +{ + BDRVQuorumState *s = acb->common.bs->opaque; + ssize_t offset; + + /* This driver will replace blkverify in this particular case */ + if (s->is_blkverify) { + offset = qemu_iovec_compare(a, b); + if (offset != -1) { + quorum_err(acb, "contents mismatch in sector %" PRId64, + acb->sector_num + + (uint64_t)(offset / BDRV_SECTOR_SIZE)); + } + return true; + } + + return quorum_iovec_compare(a, b); +} + +/* Do a vote to get the error code */ +static int quorum_vote_error(QuorumAIOCB *acb) +{ + BDRVQuorumState *s = acb->common.bs->opaque; + QuorumVoteVersion *winner = NULL; + QuorumVotes error_votes; + QuorumVoteValue result_value; + int i, ret = 0; + bool error = false; + + QLIST_INIT(&error_votes.vote_list); + error_votes.compare = quorum_64bits_compare; + + for (i = 0; i < s->num_children; i++) { + ret = acb->qcrs[i].ret; + if (ret) { + error = true; + result_value.l = ret; + quorum_count_vote(&error_votes, &result_value, i); + } + } + + if (error) { + winner = quorum_get_vote_winner(&error_votes); + ret = winner->value.l; + } + + quorum_free_vote_list(&error_votes); + + return ret; +} + +static void quorum_vote(QuorumAIOCB *acb) +{ + bool quorum = true; + int i, j, ret; + QuorumVoteValue hash; + BDRVQuorumState *s = acb->common.bs->opaque; + QuorumVoteVersion *winner; + + if (quorum_has_too_much_io_failed(acb)) { + return; + } + + /* get the index of the first successful read */ + for (i = 0; i < s->num_children; i++) { + if (!acb->qcrs[i].ret) { + break; + } + } + + assert(i < s->num_children); + + /* compare this read with all other successful reads stopping at quorum + * failure + */ + for (j = i + 1; j < s->num_children; j++) { + if (acb->qcrs[j].ret) { + continue; + } + quorum = quorum_compare(acb, &acb->qcrs[i].qiov, &acb->qcrs[j].qiov); + if (!quorum) { + break; + } + } + + /* Every successful read agrees */ + if (quorum) { + quorum_copy_qiov(acb->qiov, &acb->qcrs[i].qiov); + return; + } + + /* compute hashes for each successful read, also store indexes */ + for (i = 0; i < s->num_children; i++) { + if (acb->qcrs[i].ret) { + continue; + } + ret = quorum_compute_hash(acb, i, &hash); + /* if ever the hash computation failed */ + if (ret < 0) { + acb->vote_ret = ret; + goto free_exit; + } + quorum_count_vote(&acb->votes, &hash, i); + } + + /* vote to select the most represented version */ + winner = quorum_get_vote_winner(&acb->votes); + + /* if the winner count is smaller than threshold the read fails */ + if (winner->vote_count < s->threshold) { + quorum_report_failure(acb); + acb->vote_ret = -EIO; + goto free_exit; + } + + /* we have a winner: copy it */ + quorum_copy_qiov(acb->qiov, &acb->qcrs[winner->index].qiov); + + /* some versions are bad print them */ + quorum_report_bad_versions(s, acb, &winner->value); + +free_exit: + /* free lists */ + quorum_free_vote_list(&acb->votes); +} + static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, @@ -178,7 +567,7 @@ static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs, } for (i = 0; i < s->num_children; i++) { - bdrv_aio_readv(s->bs[i], sector_num, qiov, nb_sectors, + bdrv_aio_readv(s->bs[i], sector_num, &acb->qcrs[i].qiov, nb_sectors, quorum_aio_cb, &acb->qcrs[i]); } -- cgit v1.2.1 From d55dee2044791a02394a3db7055cedac68dca26b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:16 +0100 Subject: quorum: Add quorum_getlength(). Check that every bs file returns the same length. Otherwise, return -EIO to disable the quorum and avoid length discrepancy. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 4beee96ffa..c6ea862132 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -595,12 +595,38 @@ static BlockDriverAIOCB *quorum_aio_writev(BlockDriverState *bs, return &acb->common; } +static int64_t quorum_getlength(BlockDriverState *bs) +{ + BDRVQuorumState *s = bs->opaque; + int64_t result; + int i; + + /* check that all file have the same length */ + result = bdrv_getlength(s->bs[0]); + if (result < 0) { + return result; + } + for (i = 1; i < s->num_children; i++) { + int64_t value = bdrv_getlength(s->bs[i]); + if (value < 0) { + return value; + } + if (value != result) { + return -EIO; + } + } + + return result; +} + static BlockDriver bdrv_quorum = { .format_name = "quorum", .protocol_name = "quorum", .instance_size = sizeof(BDRVQuorumState), + .bdrv_getlength = quorum_getlength, + .bdrv_aio_readv = quorum_aio_readv, .bdrv_aio_writev = quorum_aio_writev, }; -- cgit v1.2.1 From a28e4c408b28e4d55c5bd327a19290e1da3855dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:17 +0100 Subject: quorum: Add quorum_invalidate_cache(). We really want that live migration works with quorum so implement quorum_invalidate_cache(). Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index c6ea862132..38bc2176e1 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -619,6 +619,16 @@ static int64_t quorum_getlength(BlockDriverState *bs) return result; } +static void quorum_invalidate_cache(BlockDriverState *bs) +{ + BDRVQuorumState *s = bs->opaque; + int i; + + for (i = 0; i < s->num_children; i++) { + bdrv_invalidate_cache(s->bs[i]); + } +} + static BlockDriver bdrv_quorum = { .format_name = "quorum", .protocol_name = "quorum", @@ -629,6 +639,7 @@ static BlockDriver bdrv_quorum = { .bdrv_aio_readv = quorum_aio_readv, .bdrv_aio_writev = quorum_aio_writev, + .bdrv_invalidate_cache = quorum_invalidate_cache, }; static void bdrv_quorum_init(void) -- cgit v1.2.1 From 1c508d174d4b9dfd066c3729a2560afeef5e081f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:18 +0100 Subject: quorum: Add quorum_co_flush(). Makes a vote to select error if any. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 38bc2176e1..840afdaf60 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -629,12 +629,40 @@ static void quorum_invalidate_cache(BlockDriverState *bs) } } +static coroutine_fn int quorum_co_flush(BlockDriverState *bs) +{ + BDRVQuorumState *s = bs->opaque; + QuorumVoteVersion *winner = NULL; + QuorumVotes error_votes; + QuorumVoteValue result_value; + int i; + int result = 0; + + QLIST_INIT(&error_votes.vote_list); + error_votes.compare = quorum_64bits_compare; + + for (i = 0; i < s->num_children; i++) { + result = bdrv_co_flush(s->bs[i]); + result_value.l = result; + quorum_count_vote(&error_votes, &result_value, i); + } + + winner = quorum_get_vote_winner(&error_votes); + result = winner->value.l; + + quorum_free_vote_list(&error_votes); + + return result; +} + static BlockDriver bdrv_quorum = { .format_name = "quorum", .protocol_name = "quorum", .instance_size = sizeof(BDRVQuorumState), + .bdrv_co_flush_to_disk = quorum_co_flush, + .bdrv_getlength = quorum_getlength, .bdrv_aio_readv = quorum_aio_readv, -- cgit v1.2.1 From 98a7a38f81af2b79a134eaa6cbed543aa3ca5fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:19 +0100 Subject: quorum: Implement recursive .bdrv_recurse_is_first_non_filter in quorum. This is used to activate quorum snapshot. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 840afdaf60..1b2b56f8a0 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -655,6 +655,23 @@ static coroutine_fn int quorum_co_flush(BlockDriverState *bs) return result; } +static bool quorum_recurse_is_first_non_filter(BlockDriverState *bs, + BlockDriverState *candidate) +{ + BDRVQuorumState *s = bs->opaque; + int i; + + for (i = 0; i < s->num_children; i++) { + bool perm = bdrv_recurse_is_first_non_filter(s->bs[i], + candidate); + if (perm) { + return true; + } + } + + return false; +} + static BlockDriver bdrv_quorum = { .format_name = "quorum", .protocol_name = "quorum", @@ -668,6 +685,8 @@ static BlockDriver bdrv_quorum = { .bdrv_aio_readv = quorum_aio_readv, .bdrv_aio_writev = quorum_aio_writev, .bdrv_invalidate_cache = quorum_invalidate_cache, + + .bdrv_recurse_is_first_non_filter = quorum_recurse_is_first_non_filter, }; static void bdrv_quorum_init(void) -- cgit v1.2.1 From c88a1de51ab2f26a9a37ffc317249736de8c015c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Fri, 21 Feb 2014 22:21:20 +0100 Subject: quorum: Add quorum_open() and quorum_close(). Example of command line: -drive if=virtio,driver=quorum,\ children.0.file.filename=1.raw,\ children.0.node-name=1.raw,\ children.0.driver=raw,\ children.1.file.filename=2.raw,\ children.1.node-name=2.raw,\ children.1.driver=raw,\ children.2.file.filename=3.raw,\ children.2.node-name=3.raw,\ children.2.driver=raw,\ vote-threshold=2 blkverify=on with vote-threshold=2 and two files can be passed to emulate blkverify. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block/quorum.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 1b2b56f8a0..73dd45b6ff 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -20,6 +20,9 @@ #define HASH_LENGTH 32 +#define QUORUM_OPT_VOTE_THRESHOLD "vote-threshold" +#define QUORUM_OPT_BLKVERIFY "blkverify" + /* This union holds a vote hash value */ typedef union QuorumVoteValue { char h[HASH_LENGTH]; /* SHA-256 hash */ @@ -672,12 +675,170 @@ static bool quorum_recurse_is_first_non_filter(BlockDriverState *bs, return false; } +static int quorum_valid_threshold(int threshold, int num_children, Error **errp) +{ + + if (threshold < 1) { + error_set(errp, QERR_INVALID_PARAMETER_VALUE, + "vote-threshold", "value >= 1"); + return -ERANGE; + } + + if (threshold > num_children) { + error_setg(errp, "threshold may not exceed children count"); + return -ERANGE; + } + + return 0; +} + +static QemuOptsList quorum_runtime_opts = { + .name = "quorum", + .head = QTAILQ_HEAD_INITIALIZER(quorum_runtime_opts.head), + .desc = { + { + .name = QUORUM_OPT_VOTE_THRESHOLD, + .type = QEMU_OPT_NUMBER, + .help = "The number of vote needed for reaching quorum", + }, + { + .name = QUORUM_OPT_BLKVERIFY, + .type = QEMU_OPT_BOOL, + .help = "Trigger block verify mode if set", + }, + { /* end of list */ } + }, +}; + +static int quorum_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) +{ + BDRVQuorumState *s = bs->opaque; + Error *local_err = NULL; + QemuOpts *opts; + bool *opened; + QDict *sub = NULL; + QList *list = NULL; + const QListEntry *lentry; + const QDictEntry *dentry; + int i; + int ret = 0; + + qdict_flatten(options); + qdict_extract_subqdict(options, &sub, "children."); + qdict_array_split(sub, &list); + + /* count how many different children are present and validate + * qdict_size(sub) address the open by reference case + */ + s->num_children = !qlist_size(list) ? qdict_size(sub) : qlist_size(list); + if (s->num_children < 2) { + error_setg(&local_err, + "Number of provided children must be greater than 1"); + ret = -EINVAL; + goto exit; + } + + opts = qemu_opts_create(&quorum_runtime_opts, NULL, 0, &error_abort); + qemu_opts_absorb_qdict(opts, options, &local_err); + if (error_is_set(&local_err)) { + ret = -EINVAL; + goto exit; + } + + s->threshold = qemu_opt_get_number(opts, QUORUM_OPT_VOTE_THRESHOLD, 0); + + /* and validate it against s->num_children */ + ret = quorum_valid_threshold(s->threshold, s->num_children, &local_err); + if (ret < 0) { + goto exit; + } + + /* is the driver in blkverify mode */ + if (qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false) && + s->num_children == 2 && s->threshold == 2) { + s->is_blkverify = true; + } else if (qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false)) { + fprintf(stderr, "blkverify mode is set by setting blkverify=on " + "and using two files with vote_threshold=2\n"); + } + + /* allocate the children BlockDriverState array */ + s->bs = g_new0(BlockDriverState *, s->num_children); + opened = g_new0(bool, s->num_children); + + /* Open by file name or options dict (command line or QMP) */ + if (s->num_children == qlist_size(list)) { + for (i = 0, lentry = qlist_first(list); lentry; + lentry = qlist_next(lentry), i++) { + QDict *d = qobject_to_qdict(lentry->value); + QINCREF(d); + ret = bdrv_open(&s->bs[i], NULL, NULL, d, flags, NULL, &local_err); + if (ret < 0) { + goto close_exit; + } + opened[i] = true; + } + /* Open by QMP references */ + } else { + for (i = 0, dentry = qdict_first(sub); dentry; + dentry = qdict_next(sub, dentry), i++) { + QString *string = qobject_to_qstring(dentry->value); + ret = bdrv_open(&s->bs[i], NULL, qstring_get_str(string), NULL, + flags, NULL, &local_err); + if (ret < 0) { + goto close_exit; + } + opened[i] = true; + } + } + + g_free(opened); + goto exit; + +close_exit: + /* cleanup on error */ + for (i = 0; i < s->num_children; i++) { + if (!opened[i]) { + continue; + } + bdrv_unref(s->bs[i]); + } + g_free(s->bs); + g_free(opened); +exit: + /* propagate error */ + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + } + QDECREF(list); + QDECREF(sub); + return ret; +} + +static void quorum_close(BlockDriverState *bs) +{ + BDRVQuorumState *s = bs->opaque; + int i; + + for (i = 0; i < s->num_children; i++) { + bdrv_unref(s->bs[i]); + } + + g_free(s->bs); +} + static BlockDriver bdrv_quorum = { .format_name = "quorum", .protocol_name = "quorum", .instance_size = sizeof(BDRVQuorumState), + .bdrv_file_open = quorum_open, + .bdrv_close = quorum_close, + + .authorizations = { true, true }, + .bdrv_co_flush_to_disk = quorum_co_flush, .bdrv_getlength = quorum_getlength, -- cgit v1.2.1 From 8a87f3d72279acb89f3d09b28d285d2fb6a7decf Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 21 Feb 2014 22:30:37 +0100 Subject: quorum: Simplify quorum_open() Although it may not look like it, this patch simplifies quorum_open(). qdict_array_split() is now able to return QLists with different objects than only QDicts, therefore it will now do all the work and quorum_open() does not have to handle reference strings by itself. This allows mixing full option dicts and reference strings for specifying the child block devices of quorum; furthermore, it improves handling of malformed specifications. Signed-off-by: Max Reitz Reviewed-by: Benoit Canet Signed-off-by: Kevin Wolf --- block/quorum.c | 66 ++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 27 deletions(-) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 73dd45b6ff..6c28239718 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -720,7 +720,6 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, QDict *sub = NULL; QList *list = NULL; const QListEntry *lentry; - const QDictEntry *dentry; int i; int ret = 0; @@ -728,10 +727,15 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, qdict_extract_subqdict(options, &sub, "children."); qdict_array_split(sub, &list); - /* count how many different children are present and validate - * qdict_size(sub) address the open by reference case - */ - s->num_children = !qlist_size(list) ? qdict_size(sub) : qlist_size(list); + if (qdict_size(sub)) { + error_setg(&local_err, "Invalid option children.%s", + qdict_first(sub)->key); + ret = -EINVAL; + goto exit; + } + + /* count how many different children are present */ + s->num_children = qlist_size(list); if (s->num_children < 2) { error_setg(&local_err, "Number of provided children must be greater than 1"); @@ -767,30 +771,38 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, s->bs = g_new0(BlockDriverState *, s->num_children); opened = g_new0(bool, s->num_children); - /* Open by file name or options dict (command line or QMP) */ - if (s->num_children == qlist_size(list)) { - for (i = 0, lentry = qlist_first(list); lentry; - lentry = qlist_next(lentry), i++) { - QDict *d = qobject_to_qdict(lentry->value); - QINCREF(d); - ret = bdrv_open(&s->bs[i], NULL, NULL, d, flags, NULL, &local_err); - if (ret < 0) { - goto close_exit; - } - opened[i] = true; + for (i = 0, lentry = qlist_first(list); lentry; + lentry = qlist_next(lentry), i++) { + QDict *d; + QString *string; + + switch (qobject_type(lentry->value)) + { + /* List of options */ + case QTYPE_QDICT: + d = qobject_to_qdict(lentry->value); + QINCREF(d); + ret = bdrv_open(&s->bs[i], NULL, NULL, d, flags, NULL, + &local_err); + break; + + /* QMP reference */ + case QTYPE_QSTRING: + string = qobject_to_qstring(lentry->value); + ret = bdrv_open(&s->bs[i], NULL, qstring_get_str(string), NULL, + flags, NULL, &local_err); + break; + + default: + error_setg(&local_err, "Specification of child block device %i " + "is invalid", i); + ret = -EINVAL; } - /* Open by QMP references */ - } else { - for (i = 0, dentry = qdict_first(sub); dentry; - dentry = qdict_next(sub, dentry), i++) { - QString *string = qobject_to_qstring(dentry->value); - ret = bdrv_open(&s->bs[i], NULL, qstring_get_str(string), NULL, - flags, NULL, &local_err); - if (ret < 0) { - goto close_exit; - } - opened[i] = true; + + if (ret < 0) { + goto close_exit; } + opened[i] = true; } g_free(opened); -- cgit v1.2.1