* [Qemu-devel] [PATCH v14 01/20] block: Add, parse and store "force-shared-write" option
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 8:34 ` Kevin Wolf
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 02/20] qapi: Add 'force-shared-write' to blockdev-add arguments Fam Zheng
` (18 subsequent siblings)
19 siblings, 1 reply; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 9 +++++++++
include/block/block.h | 2 ++
include/block/block_int.h | 1 +
3 files changed, 12 insertions(+)
diff --git a/block.c b/block.c
index 1e668fb..f5c4e97 100644
--- a/block.c
+++ b/block.c
@@ -763,6 +763,7 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,
* the parent. */
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
+ qdict_copy_default(child_options, parent_options, BDRV_OPT_FORCE_SHARED_WRITE);
/* Inherit the read-only option from the parent if it's not set */
qdict_copy_default(child_options, parent_options, BDRV_OPT_READ_ONLY);
@@ -871,6 +872,7 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,
* which is only applied on the top level (BlockBackend) */
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
+ qdict_copy_default(child_options, parent_options, BDRV_OPT_FORCE_SHARED_WRITE);
/* backing files always opened read-only */
qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "on");
@@ -1115,6 +1117,11 @@ QemuOptsList bdrv_runtime_opts = {
.type = QEMU_OPT_STRING,
.help = "discard operation (ignore/off, unmap/on)",
},
+ {
+ .name = BDRV_OPT_FORCE_SHARED_WRITE,
+ .type = QEMU_OPT_BOOL,
+ .help = "always accept other writers (default: off)",
+ },
{ /* end of list */ }
},
};
@@ -1154,6 +1161,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file,
drv = bdrv_find_format(driver_name);
assert(drv != NULL);
+ bs->force_shared_write = qemu_opt_get_bool(opts, BDRV_OPT_FORCE_SHARED_WRITE, false);
+
if (file != NULL) {
filename = blk_bs(file)->filename;
} else {
diff --git a/include/block/block.h b/include/block/block.h
index 5ddc0cf..d3afb75 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -109,6 +109,8 @@ typedef struct HDGeometry {
#define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush"
#define BDRV_OPT_READ_ONLY "read-only"
#define BDRV_OPT_DISCARD "discard"
+#define BDRV_OPT_FORCE_SHARED_WRITE \
+ "force-shared-write"
#define BDRV_SECTOR_BITS 9
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 59400bd..fb81692 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -518,6 +518,7 @@ struct BlockDriverState {
bool valid_key; /* if true, a valid encryption key has been set */
bool sg; /* if true, the device is a /dev/sg* */
bool probed; /* if true, format was probed rather than specified */
+ bool force_shared_write; /* if true, always allow shared write perm. */
BlockDriver *drv; /* NULL means no media */
void *opaque;
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 02/20] qapi: Add 'force-shared-write' to blockdev-add arguments
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 01/20] block: Add, parse and store "force-shared-write" option Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 8:35 ` Kevin Wolf
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 03/20] block: Respect "force-shared-write" in perm propagating Fam Zheng
` (17 subsequent siblings)
19 siblings, 1 reply; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
qapi/block-core.json | 3 +++
1 file changed, 3 insertions(+)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 033457c..b9b8002 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2880,6 +2880,8 @@
# (default: false)
# @detect-zeroes: detect and optimize zero writes (Since 2.1)
# (default: off)
+# @force-shared-write: enforce shared write permission on added nodes
+# (Since 2.10)
#
# Remaining options are determined by the block driver.
#
@@ -2891,6 +2893,7 @@
'*discard': 'BlockdevDiscardOptions',
'*cache': 'BlockdevCacheOptions',
'*read-only': 'bool',
+ '*force-shared-write': 'bool',
'*detect-zeroes': 'BlockdevDetectZeroesOptions' },
'discriminator': 'driver',
'data': {
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 02/20] qapi: Add 'force-shared-write' to blockdev-add arguments
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 02/20] qapi: Add 'force-shared-write' to blockdev-add arguments Fam Zheng
@ 2017-04-21 8:35 ` Kevin Wolf
2017-04-21 8:42 ` Fam Zheng
0 siblings, 1 reply; 36+ messages in thread
From: Kevin Wolf @ 2017-04-21 8:35 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
As far as I am concerned, this could be squashed into patch 1, though.
Adding a new field to the schema is an integral part of adding a new
option, I think.
Kevin
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v14 02/20] qapi: Add 'force-shared-write' to blockdev-add arguments
2017-04-21 8:35 ` Kevin Wolf
@ 2017-04-21 8:42 ` Fam Zheng
0 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 8:42 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
On Fri, 04/21 10:35, Kevin Wolf wrote:
> Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>
> As far as I am concerned, this could be squashed into patch 1, though.
> Adding a new field to the schema is an integral part of adding a new
> option, I think.
Makes sense.
Fam
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v14 03/20] block: Respect "force-shared-write" in perm propagating
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 01/20] block: Add, parse and store "force-shared-write" option Fam Zheng
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 02/20] qapi: Add 'force-shared-write' to blockdev-add arguments Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 8:38 ` Kevin Wolf
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 04/20] qemu-img: Add --share-rw option to subcommands Fam Zheng
` (16 subsequent siblings)
19 siblings, 1 reply; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/block.c b/block.c
index f5c4e97..6acf618 100644
--- a/block.c
+++ b/block.c
@@ -1422,6 +1422,21 @@ static int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
static void bdrv_child_abort_perm_update(BdrvChild *c);
static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
+static void bdrv_child_perm(BlockDriverState *bs, BdrvChild *c,
+ const BdrvChildRole *role,
+ uint64_t parent_perm, uint64_t parent_shared,
+ uint64_t *nperm, uint64_t *nshared)
+{
+ if (bs->drv && bs->drv->bdrv_child_perm) {
+ bs->drv->bdrv_child_perm(bs, c, role,
+ parent_perm, parent_shared,
+ nperm, nshared);
+ }
+ if (bs->force_shared_write) {
+ *nshared |= BLK_PERM_WRITE;
+ }
+}
+
/*
* Check whether permissions on this node can be changed in a way that
* @cumulative_perms and @cumulative_shared_perms are the new cumulative
@@ -1466,9 +1481,9 @@ static int bdrv_check_perm(BlockDriverState *bs, uint64_t cumulative_perms,
/* Check all children */
QLIST_FOREACH(c, &bs->children, next) {
uint64_t cur_perm, cur_shared;
- drv->bdrv_child_perm(bs, c, c->role,
- cumulative_perms, cumulative_shared_perms,
- &cur_perm, &cur_shared);
+ bdrv_child_perm(bs, c, c->role,
+ cumulative_perms, cumulative_shared_perms,
+ &cur_perm, &cur_shared);
ret = bdrv_child_check_perm(c, cur_perm, cur_shared, ignore_children,
errp);
if (ret < 0) {
@@ -1528,9 +1543,9 @@ static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms,
/* Update all children */
QLIST_FOREACH(c, &bs->children, next) {
uint64_t cur_perm, cur_shared;
- drv->bdrv_child_perm(bs, c, c->role,
- cumulative_perms, cumulative_shared_perms,
- &cur_perm, &cur_shared);
+ bdrv_child_perm(bs, c, c->role,
+ cumulative_perms, cumulative_shared_perms,
+ &cur_perm, &cur_shared);
bdrv_child_set_perm(c, cur_perm, cur_shared);
}
}
@@ -1865,8 +1880,8 @@ BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
assert(parent_bs->drv);
assert(bdrv_get_aio_context(parent_bs) == bdrv_get_aio_context(child_bs));
- parent_bs->drv->bdrv_child_perm(parent_bs, NULL, child_role,
- perm, shared_perm, &perm, &shared_perm);
+ bdrv_child_perm(parent_bs, NULL, child_role,
+ perm, shared_perm, &perm, &shared_perm);
child = bdrv_root_attach_child(child_bs, child_name, child_role,
perm, shared_perm, parent_bs, errp);
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 03/20] block: Respect "force-shared-write" in perm propagating
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 03/20] block: Respect "force-shared-write" in perm propagating Fam Zheng
@ 2017-04-21 8:38 ` Kevin Wolf
0 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2017-04-21 8:38 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block.c | 31 +++++++++++++++++++++++--------
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/block.c b/block.c
> index f5c4e97..6acf618 100644
> --- a/block.c
> +++ b/block.c
> @@ -1422,6 +1422,21 @@ static int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
> static void bdrv_child_abort_perm_update(BdrvChild *c);
> static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
>
> +static void bdrv_child_perm(BlockDriverState *bs, BdrvChild *c,
> + const BdrvChildRole *role,
> + uint64_t parent_perm, uint64_t parent_shared,
> + uint64_t *nperm, uint64_t *nshared)
> +{
> + if (bs->drv && bs->drv->bdrv_child_perm) {
> + bs->drv->bdrv_child_perm(bs, c, role,
> + parent_perm, parent_shared,
> + nperm, nshared);
> + }
> + if (bs->force_shared_write) {
As discussed on IRC, this should be c->bs->force_shared_write, so that
the option set for a given node refers to all BdrvChild objects pointing
to that node, not to all children of the node.
This means that if you configure this manually and you just want to get
rid of the file locking, setting force-shared-write = true for the
file-posix node is enough (without the change you would have to set it
for all parents of the file-posix node).
> + *nshared |= BLK_PERM_WRITE;
> + }
> +}
Kevin
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v14 04/20] qemu-img: Add --share-rw option to subcommands
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (2 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 03/20] block: Respect "force-shared-write" in perm propagating Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 13:25 ` Kevin Wolf
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 05/20] qemu-img: Update documentation for --share-rw Fam Zheng
` (15 subsequent siblings)
19 siblings, 1 reply; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Similar to share-rw qdev property, this will force the opened images to
allow shared write permission of other programs.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
qemu-img.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 119 insertions(+), 36 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index ed24371..df88a79 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -28,6 +28,7 @@
#include "qapi/qobject-output-visitor.h"
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qbool.h"
#include "qemu/cutils.h"
#include "qemu/config-file.h"
#include "qemu/option.h"
@@ -283,12 +284,15 @@ static int img_open_password(BlockBackend *blk, const char *filename,
static BlockBackend *img_open_opts(const char *optstr,
QemuOpts *opts, int flags, bool writethrough,
- bool quiet)
+ bool quiet, bool share_rw)
{
QDict *options;
Error *local_err = NULL;
BlockBackend *blk;
options = qemu_opts_to_qdict(opts, NULL);
+ if (share_rw) {
+ qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
+ }
blk = blk_new_open(NULL, NULL, options, flags, &local_err);
if (!blk) {
error_reportf_err(local_err, "Could not open '%s': ", optstr);
@@ -305,17 +309,19 @@ static BlockBackend *img_open_opts(const char *optstr,
static BlockBackend *img_open_file(const char *filename,
const char *fmt, int flags,
- bool writethrough, bool quiet)
+ bool writethrough, bool quiet, bool share_rw)
{
BlockBackend *blk;
Error *local_err = NULL;
- QDict *options = NULL;
+ QDict *options = qdict_new();
if (fmt) {
- options = qdict_new();
qdict_put(options, "driver", qstring_from_str(fmt));
}
+ if (share_rw) {
+ qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
+ }
blk = blk_new_open(filename, NULL, options, flags, &local_err);
if (!blk) {
error_reportf_err(local_err, "Could not open '%s': ", filename);
@@ -334,7 +340,7 @@ static BlockBackend *img_open_file(const char *filename,
static BlockBackend *img_open(bool image_opts,
const char *filename,
const char *fmt, int flags, bool writethrough,
- bool quiet)
+ bool quiet, bool share_rw)
{
BlockBackend *blk;
if (image_opts) {
@@ -348,9 +354,9 @@ static BlockBackend *img_open(bool image_opts,
if (!opts) {
return NULL;
}
- blk = img_open_opts(filename, opts, flags, writethrough, quiet);
+ blk = img_open_opts(filename, opts, flags, writethrough, quiet, share_rw);
} else {
- blk = img_open_file(filename, fmt, flags, writethrough, quiet);
+ blk = img_open_file(filename, fmt, flags, writethrough, quiet, share_rw);
}
return blk;
}
@@ -650,6 +656,7 @@ static int img_check(int argc, char **argv)
ImageCheck *check;
bool quiet = false;
bool image_opts = false;
+ bool share_rw = false;
fmt = NULL;
output = NULL;
@@ -664,9 +671,10 @@ static int img_check(int argc, char **argv)
{"output", required_argument, 0, OPTION_OUTPUT},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":hf:r:T:q",
+ c = getopt_long(argc, argv, ":hf:r:T:qU",
long_options, &option_index);
if (c == -1) {
break;
@@ -705,6 +713,9 @@ static int img_check(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OBJECT: {
QemuOpts *opts;
opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -744,7 +755,8 @@ static int img_check(int argc, char **argv)
return 1;
}
- blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
+ blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet,
+ share_rw);
if (!blk) {
return 1;
}
@@ -864,6 +876,7 @@ static int img_commit(int argc, char **argv)
CommonBlockJobCBInfo cbi;
bool image_opts = false;
AioContext *aio_context;
+ bool share_rw = false;
fmt = NULL;
cache = BDRV_DEFAULT_CACHE;
@@ -873,9 +886,10 @@ static int img_commit(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":f:ht:b:dpq",
+ c = getopt_long(argc, argv, ":f:ht:b:dpqU",
long_options, NULL);
if (c == -1) {
break;
@@ -910,6 +924,9 @@ static int img_commit(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OBJECT: {
QemuOpts *opts;
opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -947,7 +964,8 @@ static int img_commit(int argc, char **argv)
return 1;
}
- blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
+ blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet,
+ share_rw);
if (!blk) {
return 1;
}
@@ -1206,6 +1224,7 @@ static int img_compare(int argc, char **argv)
int c, pnum;
uint64_t progress_base;
bool image_opts = false;
+ bool share_rw = false;
cache = BDRV_DEFAULT_CACHE;
for (;;) {
@@ -1213,9 +1232,10 @@ static int img_compare(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":hf:F:T:pqs",
+ c = getopt_long(argc, argv, ":hf:F:T:pqsU",
long_options, NULL);
if (c == -1) {
break;
@@ -1248,6 +1268,9 @@ static int img_compare(int argc, char **argv)
case 's':
strict = true;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OBJECT: {
QemuOpts *opts;
opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -1293,13 +1316,15 @@ static int img_compare(int argc, char **argv)
goto out3;
}
- blk1 = img_open(image_opts, filename1, fmt1, flags, writethrough, quiet);
+ blk1 = img_open(image_opts, filename1, fmt1, flags, writethrough, quiet,
+ share_rw);
if (!blk1) {
ret = 2;
goto out3;
}
- blk2 = img_open(image_opts, filename2, fmt2, flags, writethrough, quiet);
+ blk2 = img_open(image_opts, filename2, fmt2, flags, writethrough, quiet,
+ share_rw);
if (!blk2) {
ret = 2;
goto out2;
@@ -1934,6 +1959,7 @@ static int img_convert(int argc, char **argv)
bool writethrough, src_writethrough, quiet = false, image_opts = false,
compress = false, skip_create = false, progress = false;
int64_t ret = -EINVAL;
+ bool share_rw = false;
ImgConvertState s = (ImgConvertState) {
/* Need at least 4k of zeros for sparse detection */
@@ -1948,9 +1974,10 @@ static int img_convert(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":hf:O:B:ce6o:s:l:S:pt:T:qnm:W",
+ c = getopt_long(argc, argv, ":hf:O:B:ce6o:s:l:S:pt:T:qnm:WU",
long_options, NULL);
if (c == -1) {
break;
@@ -2054,6 +2081,9 @@ static int img_convert(int argc, char **argv)
case 'W':
s.wr_in_order = false;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OBJECT:
opts = qemu_opts_parse_noisily(&qemu_object_opts,
optarg, true);
@@ -2117,7 +2147,8 @@ static int img_convert(int argc, char **argv)
for (bs_i = 0; bs_i < s.src_num; bs_i++) {
s.src[bs_i] = img_open(image_opts, argv[optind + bs_i],
- fmt, src_flags, src_writethrough, quiet);
+ fmt, src_flags, src_writethrough, quiet,
+ share_rw);
if (!s.src[bs_i]) {
ret = -1;
goto out;
@@ -2262,7 +2293,8 @@ static int img_convert(int argc, char **argv)
* the bdrv_create() call which takes different params.
* Not critical right now, so fix can wait...
*/
- s.target = img_open_file(out_filename, out_fmt, flags, writethrough, quiet);
+ s.target = img_open_file(out_filename, out_fmt, flags, writethrough, quiet,
+ share_rw);
if (!s.target) {
ret = -1;
goto out;
@@ -2413,7 +2445,7 @@ static gboolean str_equal_func(gconstpointer a, gconstpointer b)
static ImageInfoList *collect_image_info_list(bool image_opts,
const char *filename,
const char *fmt,
- bool chain)
+ bool chain, bool share_rw)
{
ImageInfoList *head = NULL;
ImageInfoList **last = &head;
@@ -2436,7 +2468,8 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
g_hash_table_insert(filenames, (gpointer)filename, NULL);
blk = img_open(image_opts, filename, fmt,
- BDRV_O_NO_BACKING | BDRV_O_NO_IO, false, false);
+ BDRV_O_NO_BACKING | BDRV_O_NO_IO, false, false,
+ share_rw);
if (!blk) {
goto err;
}
@@ -2488,6 +2521,7 @@ static int img_info(int argc, char **argv)
const char *filename, *fmt, *output;
ImageInfoList *list;
bool image_opts = false;
+ bool share_rw = false;
fmt = NULL;
output = NULL;
@@ -2500,9 +2534,10 @@ static int img_info(int argc, char **argv)
{"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":f:h",
+ c = getopt_long(argc, argv, ":f:hU",
long_options, &option_index);
if (c == -1) {
break;
@@ -2520,6 +2555,9 @@ static int img_info(int argc, char **argv)
case 'f':
fmt = optarg;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OUTPUT:
output = optarg;
break;
@@ -2559,7 +2597,8 @@ static int img_info(int argc, char **argv)
return 1;
}
- list = collect_image_info_list(image_opts, filename, fmt, chain);
+ list = collect_image_info_list(image_opts, filename, fmt, chain,
+ share_rw);
if (!list) {
return 1;
}
@@ -2705,6 +2744,7 @@ static int img_map(int argc, char **argv)
MapEntry curr = { .length = 0 }, next;
int ret = 0;
bool image_opts = false;
+ bool share_rw = false;
fmt = NULL;
output = NULL;
@@ -2716,9 +2756,10 @@ static int img_map(int argc, char **argv)
{"output", required_argument, 0, OPTION_OUTPUT},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":f:h",
+ c = getopt_long(argc, argv, ":f:hU",
long_options, &option_index);
if (c == -1) {
break;
@@ -2736,6 +2777,9 @@ static int img_map(int argc, char **argv)
case 'f':
fmt = optarg;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OUTPUT:
output = optarg;
break;
@@ -2772,7 +2816,7 @@ static int img_map(int argc, char **argv)
return 1;
}
- blk = img_open(image_opts, filename, fmt, 0, false, false);
+ blk = img_open(image_opts, filename, fmt, 0, false, false, share_rw);
if (!blk) {
return 1;
}
@@ -2835,6 +2879,7 @@ static int img_snapshot(int argc, char **argv)
bool quiet = false;
Error *err = NULL;
bool image_opts = false;
+ bool share_rw = false;
bdrv_oflags = BDRV_O_RDWR;
/* Parse commandline parameters */
@@ -2843,9 +2888,10 @@ static int img_snapshot(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":la:c:d:hq",
+ c = getopt_long(argc, argv, ":la:c:d:hqU",
long_options, NULL);
if (c == -1) {
break;
@@ -2895,6 +2941,9 @@ static int img_snapshot(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OBJECT: {
QemuOpts *opts;
opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -2921,7 +2970,8 @@ static int img_snapshot(int argc, char **argv)
}
/* Open the image */
- blk = img_open(image_opts, filename, NULL, bdrv_oflags, false, quiet);
+ blk = img_open(image_opts, filename, NULL, bdrv_oflags, false, quiet,
+ share_rw);
if (!blk) {
return 1;
}
@@ -2985,6 +3035,7 @@ static int img_rebase(int argc, char **argv)
int c, flags, src_flags, ret;
bool writethrough, src_writethrough;
int unsafe = 0;
+ bool share_rw = 0;
int progress = 0;
bool quiet = false;
Error *local_err = NULL;
@@ -3001,9 +3052,10 @@ static int img_rebase(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":hf:F:b:upt:T:q",
+ c = getopt_long(argc, argv, ":hf:F:b:upt:T:qU",
long_options, NULL);
if (c == -1) {
break;
@@ -3053,6 +3105,9 @@ static int img_rebase(int argc, char **argv)
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
+ case 'U':
+ share_rw = true;
+ break;
}
}
@@ -3101,7 +3156,8 @@ static int img_rebase(int argc, char **argv)
* Ignore the old backing file for unsafe rebase in case we want to correct
* the reference to a renamed or moved backing file.
*/
- blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
+ blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet,
+ share_rw);
if (!blk) {
ret = -1;
goto out;
@@ -3126,6 +3182,9 @@ static int img_rebase(int argc, char **argv)
qdict_put(options, "driver", qstring_from_str(bs->backing_format));
}
+ if (share_rw) {
+ qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
+ }
bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
blk_old_backing = blk_new_open(backing_name, NULL,
options, src_flags, &local_err);
@@ -3341,6 +3400,7 @@ static int img_resize(int argc, char **argv)
bool quiet = false;
BlockBackend *blk = NULL;
QemuOpts *param;
+ bool share_rw = false;
static QemuOptsList resize_options = {
.name = "resize_options",
@@ -3373,9 +3433,10 @@ static int img_resize(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":f:hq",
+ c = getopt_long(argc, argv, ":f:hqU",
long_options, NULL);
if (c == -1) {
break;
@@ -3396,6 +3457,9 @@ static int img_resize(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OBJECT: {
QemuOpts *opts;
opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -3448,7 +3512,8 @@ static int img_resize(int argc, char **argv)
qemu_opts_del(param);
blk = img_open(image_opts, filename, fmt,
- BDRV_O_RDWR | BDRV_O_RESIZE, false, quiet);
+ BDRV_O_RDWR | BDRV_O_RESIZE, false, quiet,
+ share_rw);
if (!blk) {
ret = -1;
goto out;
@@ -3509,6 +3574,7 @@ static int img_amend(int argc, char **argv)
BlockBackend *blk = NULL;
BlockDriverState *bs = NULL;
bool image_opts = false;
+ bool share_rw = false;
cache = BDRV_DEFAULT_CACHE;
for (;;) {
@@ -3516,9 +3582,10 @@ static int img_amend(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":ho:f:t:pq",
+ c = getopt_long(argc, argv, ":ho:f:t:pqU",
long_options, NULL);
if (c == -1) {
break;
@@ -3560,6 +3627,9 @@ static int img_amend(int argc, char **argv)
case 'q':
quiet = true;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OBJECT:
opts = qemu_opts_parse_noisily(&qemu_object_opts,
optarg, true);
@@ -3611,7 +3681,8 @@ static int img_amend(int argc, char **argv)
goto out;
}
- blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
+ blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet,
+ share_rw);
if (!blk) {
ret = -1;
goto out;
@@ -3779,6 +3850,7 @@ static int img_bench(int argc, char **argv)
bool writethrough = false;
struct timeval t1, t2;
int i;
+ bool share_rw = false;
for (;;) {
static const struct option long_options[] = {
@@ -3787,9 +3859,10 @@ static int img_bench(int argc, char **argv)
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{"pattern", required_argument, 0, OPTION_PATTERN},
{"no-drain", no_argument, 0, OPTION_NO_DRAIN},
+ {"share-rw", no_argument, 0, 'U'},
{0, 0, 0, 0}
};
- c = getopt_long(argc, argv, ":hc:d:f:no:qs:S:t:w", long_options, NULL);
+ c = getopt_long(argc, argv, ":hc:d:f:no:qs:S:t:wU", long_options, NULL);
if (c == -1) {
break;
}
@@ -3883,6 +3956,9 @@ static int img_bench(int argc, char **argv)
flags |= BDRV_O_RDWR;
is_write = true;
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_PATTERN:
{
unsigned long res;
@@ -3930,7 +4006,8 @@ static int img_bench(int argc, char **argv)
goto out;
}
- blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
+ blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet,
+ share_rw);
if (!blk) {
ret = -1;
goto out;
@@ -4097,6 +4174,7 @@ static int img_dd(int argc, char **argv)
const char *fmt = NULL;
int64_t size = 0;
int64_t block_count = 0, out_pos, in_pos;
+ bool share_rw = false;
struct DdInfo dd = {
.flags = 0,
.count = 0,
@@ -4125,10 +4203,11 @@ static int img_dd(int argc, char **argv)
const struct option long_options[] = {
{ "help", no_argument, 0, 'h'},
{ "image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ { "share-rw", no_argument, 0, 'U'},
{ 0, 0, 0, 0 }
};
- while ((c = getopt_long(argc, argv, ":hf:O:", long_options, NULL))) {
+ while ((c = getopt_long(argc, argv, ":hf:O:U", long_options, NULL))) {
if (c == EOF) {
break;
}
@@ -4148,6 +4227,9 @@ static int img_dd(int argc, char **argv)
case 'h':
help();
break;
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
@@ -4192,7 +4274,8 @@ static int img_dd(int argc, char **argv)
ret = -1;
goto out;
}
- blk1 = img_open(image_opts, in.filename, fmt, 0, false, false);
+ blk1 = img_open(image_opts, in.filename, fmt, 0, false, false,
+ share_rw);
if (!blk1) {
ret = -1;
@@ -4260,7 +4343,7 @@ static int img_dd(int argc, char **argv)
}
blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
- false, false);
+ false, false, share_rw);
if (!blk2) {
ret = -1;
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 04/20] qemu-img: Add --share-rw option to subcommands
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 04/20] qemu-img: Add --share-rw option to subcommands Fam Zheng
@ 2017-04-21 13:25 ` Kevin Wolf
2017-04-21 15:35 ` Eric Blake
2017-04-24 6:10 ` Fam Zheng
0 siblings, 2 replies; 36+ messages in thread
From: Kevin Wolf @ 2017-04-21 13:25 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> Similar to share-rw qdev property, this will force the opened images to
> allow shared write permission of other programs.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
General observation: We were considering to make share-rw require
read-only. Some of the commands converted here always open the image
read-write, so if we go ahead with the restriction, will the option
become useless in many of the subcommands?
> qemu-img.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 119 insertions(+), 36 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index ed24371..df88a79 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -28,6 +28,7 @@
> #include "qapi/qobject-output-visitor.h"
> #include "qapi/qmp/qerror.h"
> #include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qbool.h"
> #include "qemu/cutils.h"
> #include "qemu/config-file.h"
> #include "qemu/option.h"
> @@ -283,12 +284,15 @@ static int img_open_password(BlockBackend *blk, const char *filename,
>
> static BlockBackend *img_open_opts(const char *optstr,
> QemuOpts *opts, int flags, bool writethrough,
> - bool quiet)
> + bool quiet, bool share_rw)
> {
> QDict *options;
> Error *local_err = NULL;
> BlockBackend *blk;
> options = qemu_opts_to_qdict(opts, NULL);
> + if (share_rw) {
> + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
> + }
It's interesting that you chose a conditional qdict_put for true rather
than an unconditional one for share_rw here. The difference becomes
visible when someone sets both -U and share-rw=off; we need to decide
which one should take precedence.
Generally, we always give explicit options the precedence, so if we were
to follow suit here, we would set share-rw here only if the option isn't
already set. For strings, we have qdict_set_default_str() to achieve
this, for bools we probably need a new function (or does Eric's series
which introduces qdict_put_bool() also introduce a similar function,
like some qdict_set_default_bool?)
> blk = blk_new_open(NULL, NULL, options, flags, &local_err);
> if (!blk) {
> error_reportf_err(local_err, "Could not open '%s': ", optstr);
> @@ -2985,6 +3035,7 @@ static int img_rebase(int argc, char **argv)
> int c, flags, src_flags, ret;
> bool writethrough, src_writethrough;
> int unsafe = 0;
> + bool share_rw = 0;
Not false?
> int progress = 0;
> bool quiet = false;
> Error *local_err = NULL;
> @@ -3001,9 +3052,10 @@ static int img_rebase(int argc, char **argv)
> {"help", no_argument, 0, 'h'},
> {"object", required_argument, 0, OPTION_OBJECT},
> {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
> + {"share-rw", no_argument, 0, 'U'},
> {0, 0, 0, 0}
> };
> - c = getopt_long(argc, argv, ":hf:F:b:upt:T:q",
> + c = getopt_long(argc, argv, ":hf:F:b:upt:T:qU",
> long_options, NULL);
> if (c == -1) {
> break;
> @@ -3053,6 +3105,9 @@ static int img_rebase(int argc, char **argv)
> case OPTION_IMAGE_OPTS:
> image_opts = true;
> break;
> + case 'U':
> + share_rw = true;
> + break;
> }
> }
>
> @@ -3101,7 +3156,8 @@ static int img_rebase(int argc, char **argv)
> * Ignore the old backing file for unsafe rebase in case we want to correct
> * the reference to a renamed or moved backing file.
> */
> - blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
> + blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet,
> + share_rw);
> if (!blk) {
> ret = -1;
> goto out;
> @@ -3126,6 +3182,9 @@ static int img_rebase(int argc, char **argv)
> qdict_put(options, "driver", qstring_from_str(bs->backing_format));
> }
>
> + if (share_rw) {
> + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
This is longer than 80 lines and wrapping wouldn't make it unreadable. I
think there are more similar instances in this series (even though you
replied to the patchew mail that they are intentional).
> + }
> bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
> blk_old_backing = blk_new_open(backing_name, NULL,
> options, src_flags, &local_err);
Why don't you apply share_rw to blk_new_backing, which is opened a few
lines down from here?
Kevin
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 04/20] qemu-img: Add --share-rw option to subcommands
2017-04-21 13:25 ` Kevin Wolf
@ 2017-04-21 15:35 ` Eric Blake
2017-04-24 6:10 ` Fam Zheng
1 sibling, 0 replies; 36+ messages in thread
From: Eric Blake @ 2017-04-21 15:35 UTC (permalink / raw)
To: Kevin Wolf, Fam Zheng; +Cc: qemu-devel, Max Reitz, qemu-block
[-- Attachment #1: Type: text/plain, Size: 2554 bytes --]
On 04/21/2017 08:25 AM, Kevin Wolf wrote:
> Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
>> Similar to share-rw qdev property, this will force the opened images to
>> allow shared write permission of other programs.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> General observation: We were considering to make share-rw require
> read-only. Some of the commands converted here always open the image
> read-write, so if we go ahead with the restriction, will the option
> become useless in many of the subcommands?
>
>> static BlockBackend *img_open_opts(const char *optstr,
>> QemuOpts *opts, int flags, bool writethrough,
>> - bool quiet)
>> + bool quiet, bool share_rw)
>> {
>> QDict *options;
>> Error *local_err = NULL;
>> BlockBackend *blk;
>> options = qemu_opts_to_qdict(opts, NULL);
>> + if (share_rw) {
>> + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
>> + }
>
> It's interesting that you chose a conditional qdict_put for true rather
> than an unconditional one for share_rw here. The difference becomes
> visible when someone sets both -U and share-rw=off; we need to decide
> which one should take precedence.
>
> Generally, we always give explicit options the precedence, so if we were
> to follow suit here, we would set share-rw here only if the option isn't
> already set. For strings, we have qdict_set_default_str() to achieve
> this, for bools we probably need a new function (or does Eric's series
> which introduces qdict_put_bool() also introduce a similar function,
> like some qdict_set_default_bool?)
It doesn't (yet), but I could add one if it is a common pattern that we
envision using.
>> @@ -3126,6 +3182,9 @@ static int img_rebase(int argc, char **argv)
>> qdict_put(options, "driver", qstring_from_str(bs->backing_format));
>> }
>>
>> + if (share_rw) {
>> + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
>
> This is longer than 80 lines and wrapping wouldn't make it unreadable. I
> think there are more similar instances in this series (even though you
> replied to the patchew mail that they are intentional).
This one would indeed benefit from my patch for qdict_put_bool().
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 04/20] qemu-img: Add --share-rw option to subcommands
2017-04-21 13:25 ` Kevin Wolf
2017-04-21 15:35 ` Eric Blake
@ 2017-04-24 6:10 ` Fam Zheng
2017-04-24 10:13 ` Kevin Wolf
1 sibling, 1 reply; 36+ messages in thread
From: Fam Zheng @ 2017-04-24 6:10 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
On Fri, 04/21 15:25, Kevin Wolf wrote:
> Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> > Similar to share-rw qdev property, this will force the opened images to
> > allow shared write permission of other programs.
> >
> > Signed-off-by: Fam Zheng <famz@redhat.com>
>
> General observation: We were considering to make share-rw require
> read-only. Some of the commands converted here always open the image
> read-write, so if we go ahead with the restriction, will the option
> become useless in many of the subcommands?
share-rw on qdev doesn't require read-only, so I personally perfer we follow
that manner. Because even with --share-rw for the read-write commands,
the image is still protected from corruption by the fact that the other side
probably uses non-share-rw.
But on the other hand, we can always add the option when necessary, so it's okay
to leave them as is. If you insist, I can remove them in next version.
>
> > qemu-img.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++--------------
> > 1 file changed, 119 insertions(+), 36 deletions(-)
> >
> > diff --git a/qemu-img.c b/qemu-img.c
> > index ed24371..df88a79 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -28,6 +28,7 @@
> > #include "qapi/qobject-output-visitor.h"
> > #include "qapi/qmp/qerror.h"
> > #include "qapi/qmp/qjson.h"
> > +#include "qapi/qmp/qbool.h"
> > #include "qemu/cutils.h"
> > #include "qemu/config-file.h"
> > #include "qemu/option.h"
> > @@ -283,12 +284,15 @@ static int img_open_password(BlockBackend *blk, const char *filename,
> >
> > static BlockBackend *img_open_opts(const char *optstr,
> > QemuOpts *opts, int flags, bool writethrough,
> > - bool quiet)
> > + bool quiet, bool share_rw)
> > {
> > QDict *options;
> > Error *local_err = NULL;
> > BlockBackend *blk;
> > options = qemu_opts_to_qdict(opts, NULL);
> > + if (share_rw) {
> > + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
> > + }
>
> It's interesting that you chose a conditional qdict_put for true rather
> than an unconditional one for share_rw here. The difference becomes
> visible when someone sets both -U and share-rw=off; we need to decide
> which one should take precedence.
I don't have a preference here. Setting both -U and share-rw=off is
inconsistent, it's not a problem to yield an "undefined" result.
>
> Generally, we always give explicit options the precedence, so if we were
> to follow suit here, we would set share-rw here only if the option isn't
> already set. For strings, we have qdict_set_default_str() to achieve
> this, for bools we probably need a new function (or does Eric's series
> which introduces qdict_put_bool() also introduce a similar function,
> like some qdict_set_default_bool?)
>
> > blk = blk_new_open(NULL, NULL, options, flags, &local_err);
> > if (!blk) {
> > error_reportf_err(local_err, "Could not open '%s': ", optstr);
>
> > @@ -2985,6 +3035,7 @@ static int img_rebase(int argc, char **argv)
> > int c, flags, src_flags, ret;
> > bool writethrough, src_writethrough;
> > int unsafe = 0;
> > + bool share_rw = 0;
>
> Not false?
Will fix.
>
> > int progress = 0;
> > bool quiet = false;
> > Error *local_err = NULL;
> > @@ -3001,9 +3052,10 @@ static int img_rebase(int argc, char **argv)
> > {"help", no_argument, 0, 'h'},
> > {"object", required_argument, 0, OPTION_OBJECT},
> > {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
> > + {"share-rw", no_argument, 0, 'U'},
> > {0, 0, 0, 0}
> > };
> > - c = getopt_long(argc, argv, ":hf:F:b:upt:T:q",
> > + c = getopt_long(argc, argv, ":hf:F:b:upt:T:qU",
> > long_options, NULL);
> > if (c == -1) {
> > break;
> > @@ -3053,6 +3105,9 @@ static int img_rebase(int argc, char **argv)
> > case OPTION_IMAGE_OPTS:
> > image_opts = true;
> > break;
> > + case 'U':
> > + share_rw = true;
> > + break;
> > }
> > }
> >
> > @@ -3101,7 +3156,8 @@ static int img_rebase(int argc, char **argv)
> > * Ignore the old backing file for unsafe rebase in case we want to correct
> > * the reference to a renamed or moved backing file.
> > */
> > - blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
> > + blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet,
> > + share_rw);
> > if (!blk) {
> > ret = -1;
> > goto out;
> > @@ -3126,6 +3182,9 @@ static int img_rebase(int argc, char **argv)
> > qdict_put(options, "driver", qstring_from_str(bs->backing_format));
> > }
> >
> > + if (share_rw) {
> > + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
>
> This is longer than 80 lines and wrapping wouldn't make it unreadable. I
> think there are more similar instances in this series (even though you
> replied to the patchew mail that they are intentional).
OK, I can wrap it.
>
> > + }
> > bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
> > blk_old_backing = blk_new_open(backing_name, NULL,
> > options, src_flags, &local_err);
>
> Why don't you apply share_rw to blk_new_backing, which is opened a few
> lines down from here?
I missed that one, will fix.
Fam
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 04/20] qemu-img: Add --share-rw option to subcommands
2017-04-24 6:10 ` Fam Zheng
@ 2017-04-24 10:13 ` Kevin Wolf
2017-04-24 11:28 ` Fam Zheng
0 siblings, 1 reply; 36+ messages in thread
From: Kevin Wolf @ 2017-04-24 10:13 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
Am 24.04.2017 um 08:10 hat Fam Zheng geschrieben:
> On Fri, 04/21 15:25, Kevin Wolf wrote:
> > Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> > > Similar to share-rw qdev property, this will force the opened images to
> > > allow shared write permission of other programs.
> > >
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> >
> > General observation: We were considering to make share-rw require
> > read-only. Some of the commands converted here always open the image
> > read-write, so if we go ahead with the restriction, will the option
> > become useless in many of the subcommands?
>
> share-rw on qdev doesn't require read-only, so I personally perfer we
> follow that manner.
It's not really completely comparable to qdev's share-rw because qdev
only shares writes on the top level (and the qcow2 driver restricts this
again down the tree), while this option propagates all the way down.
Which is why you called the block layer option "force-shared-write".
Maybe that would be the better name here as well.
> Because even with --share-rw for the read-write commands, the image is
> still protected from corruption by the fact that the other side
> probably uses non-share-rw.
If the other side "probably" uses non-share-rw, then the image is only
"probably" protected. I think you're right about the common case, but if
two qemu instances use force-shared-write=on, then we get actual image
corruption.
As far as I know, our real use cases for the option are read-only: We
want to inspect images which are in use by a VM. Do we have any use
cases for read-write access?
Note that this is different from qdev in that share-rw on the qdev level
affects only the user data and can work e.g. if the guest uses a cluster
file system. But this option affects metadata as well and qcow2 never
supports this, so opening two images read-write at the same time is
guaranteed to corrupt the image.
> But on the other hand, we can always add the option when necessary, so
> it's okay to leave them as is. If you insist, I can remove them in
> next version.
Yes, I think we really need a check in bdrv_open_common() that forbids
force-shared-write=on on writable images. And then, the options in
qemu-img become useless when applied to writable images because they
would only produce errors.
> > > qemu-img.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++--------------
> > > 1 file changed, 119 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/qemu-img.c b/qemu-img.c
> > > index ed24371..df88a79 100644
> > > --- a/qemu-img.c
> > > +++ b/qemu-img.c
> > > @@ -28,6 +28,7 @@
> > > #include "qapi/qobject-output-visitor.h"
> > > #include "qapi/qmp/qerror.h"
> > > #include "qapi/qmp/qjson.h"
> > > +#include "qapi/qmp/qbool.h"
> > > #include "qemu/cutils.h"
> > > #include "qemu/config-file.h"
> > > #include "qemu/option.h"
> > > @@ -283,12 +284,15 @@ static int img_open_password(BlockBackend *blk, const char *filename,
> > >
> > > static BlockBackend *img_open_opts(const char *optstr,
> > > QemuOpts *opts, int flags, bool writethrough,
> > > - bool quiet)
> > > + bool quiet, bool share_rw)
> > > {
> > > QDict *options;
> > > Error *local_err = NULL;
> > > BlockBackend *blk;
> > > options = qemu_opts_to_qdict(opts, NULL);
> > > + if (share_rw) {
> > > + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
> > > + }
> >
> > It's interesting that you chose a conditional qdict_put for true rather
> > than an unconditional one for share_rw here. The difference becomes
> > visible when someone sets both -U and share-rw=off; we need to decide
> > which one should take precedence.
>
> I don't have a preference here. Setting both -U and share-rw=off is
> inconsistent, it's not a problem to yield an "undefined" result.
We could just check whether the entry already exists and error out.
Maybe that's the best option.
Kevin
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 04/20] qemu-img: Add --share-rw option to subcommands
2017-04-24 10:13 ` Kevin Wolf
@ 2017-04-24 11:28 ` Fam Zheng
0 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-24 11:28 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
On Mon, 04/24 12:13, Kevin Wolf wrote:
> Am 24.04.2017 um 08:10 hat Fam Zheng geschrieben:
> > On Fri, 04/21 15:25, Kevin Wolf wrote:
> > > Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> > > > Similar to share-rw qdev property, this will force the opened images to
> > > > allow shared write permission of other programs.
> > > >
> > > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > >
> > > General observation: We were considering to make share-rw require
> > > read-only. Some of the commands converted here always open the image
> > > read-write, so if we go ahead with the restriction, will the option
> > > become useless in many of the subcommands?
> >
> > share-rw on qdev doesn't require read-only, so I personally perfer we
> > follow that manner.
>
> It's not really completely comparable to qdev's share-rw because qdev
> only shares writes on the top level (and the qcow2 driver restricts this
> again down the tree), while this option propagates all the way down.
> Which is why you called the block layer option "force-shared-write".
> Maybe that would be the better name here as well.
Makes sense to me.
>
> > Because even with --share-rw for the read-write commands, the image is
> > still protected from corruption by the fact that the other side
> > probably uses non-share-rw.
>
> If the other side "probably" uses non-share-rw, then the image is only
> "probably" protected. I think you're right about the common case, but if
> two qemu instances use force-shared-write=on, then we get actual image
> corruption.
>
> As far as I know, our real use cases for the option are read-only: We
> want to inspect images which are in use by a VM. Do we have any use
> cases for read-write access?
>
> Note that this is different from qdev in that share-rw on the qdev level
> affects only the user data and can work e.g. if the guest uses a cluster
> file system. But this option affects metadata as well and qcow2 never
> supports this, so opening two images read-write at the same time is
> guaranteed to corrupt the image.
OK, I think that makes sense too.
>
> > But on the other hand, we can always add the option when necessary, so
> > it's okay to leave them as is. If you insist, I can remove them in
> > next version.
>
> Yes, I think we really need a check in bdrv_open_common() that forbids
> force-shared-write=on on writable images. And then, the options in
> qemu-img become useless when applied to writable images because they
> would only produce errors.
>
> > > > qemu-img.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++--------------
> > > > 1 file changed, 119 insertions(+), 36 deletions(-)
> > > >
> > > > diff --git a/qemu-img.c b/qemu-img.c
> > > > index ed24371..df88a79 100644
> > > > --- a/qemu-img.c
> > > > +++ b/qemu-img.c
> > > > @@ -28,6 +28,7 @@
> > > > #include "qapi/qobject-output-visitor.h"
> > > > #include "qapi/qmp/qerror.h"
> > > > #include "qapi/qmp/qjson.h"
> > > > +#include "qapi/qmp/qbool.h"
> > > > #include "qemu/cutils.h"
> > > > #include "qemu/config-file.h"
> > > > #include "qemu/option.h"
> > > > @@ -283,12 +284,15 @@ static int img_open_password(BlockBackend *blk, const char *filename,
> > > >
> > > > static BlockBackend *img_open_opts(const char *optstr,
> > > > QemuOpts *opts, int flags, bool writethrough,
> > > > - bool quiet)
> > > > + bool quiet, bool share_rw)
> > > > {
> > > > QDict *options;
> > > > Error *local_err = NULL;
> > > > BlockBackend *blk;
> > > > options = qemu_opts_to_qdict(opts, NULL);
> > > > + if (share_rw) {
> > > > + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
> > > > + }
> > >
> > > It's interesting that you chose a conditional qdict_put for true rather
> > > than an unconditional one for share_rw here. The difference becomes
> > > visible when someone sets both -U and share-rw=off; we need to decide
> > > which one should take precedence.
> >
> > I don't have a preference here. Setting both -U and share-rw=off is
> > inconsistent, it's not a problem to yield an "undefined" result.
>
> We could just check whether the entry already exists and error out.
> Maybe that's the best option.
Sounds good, will add the check.
Fam
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v14 05/20] qemu-img: Update documentation for --share-rw
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (3 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 04/20] qemu-img: Add --share-rw option to subcommands Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 15:37 ` Eric Blake
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 06/20] qemu-io: Add --share-rw option Fam Zheng
` (14 subsequent siblings)
19 siblings, 1 reply; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
qemu-img-cmds.hx | 48 ++++++++++++++++++++++++------------------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 8ac7822..1b00bb8 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -10,15 +10,15 @@ STEXI
ETEXI
DEF("bench", img_bench,
- "bench [-c count] [-d depth] [-f fmt] [--flush-interval=flush_interval] [-n] [--no-drain] [-o offset] [--pattern=pattern] [-q] [-s buffer_size] [-S step_size] [-t cache] [-w] filename")
+ "bench [-c count] [-d depth] [-f fmt] [--flush-interval=flush_interval] [-n] [--no-drain] [-o offset] [--pattern=pattern] [-q] [-s buffer_size] [-S step_size] [-t cache] [-w] [--share-rw] filename")
STEXI
-@item bench [-c @var{count}] [-d @var{depth}] [-f @var{fmt}] [--flush-interval=@var{flush_interval}] [-n] [--no-drain] [-o @var{offset}] [--pattern=@var{pattern}] [-q] [-s @var{buffer_size}] [-S @var{step_size}] [-t @var{cache}] [-w] @var{filename}
+@item bench [-c @var{count}] [-d @var{depth}] [-f @var{fmt}] [--flush-interval=@var{flush_interval}] [-n] [--no-drain] [-o @var{offset}] [--pattern=@var{pattern}] [-q] [-s @var{buffer_size}] [-S @var{step_size}] [-t @var{cache}] [-w] [--share-rw] @var{filename}
ETEXI
DEF("check", img_check,
- "check [-q] [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] filename")
+ "check [-q] [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] [--share-rw] filename")
STEXI
-@item check [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] @var{filename}
+@item check [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] [--share-rw] @var{filename}
ETEXI
DEF("create", img_create,
@@ -28,62 +28,62 @@ STEXI
ETEXI
DEF("commit", img_commit,
- "commit [-q] [--object objectdef] [--image-opts] [-f fmt] [-t cache] [-b base] [-d] [-p] filename")
+ "commit [-q] [--object objectdef] [--image-opts] [-f fmt] [-t cache] [-b base] [-d] [-p] [--share-rw] filename")
STEXI
-@item commit [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] @var{filename}
+@item commit [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] [--share-rw] @var{filename}
ETEXI
DEF("compare", img_compare,
- "compare [--object objectdef] [--image-opts] [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] filename1 filename2")
+ "compare [--object objectdef] [--image-opts] [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] [--share-rw] filename1 filename2")
STEXI
-@item compare [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-q] [-s] @var{filename1} @var{filename2}
+@item compare [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-q] [-s] [--share-rw] @var{filename1} @var{filename2}
ETEXI
DEF("convert", img_convert,
- "convert [--object objectdef] [--image-opts] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+ "convert [--object objectdef] [--image-opts] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--share-rw] filename [filename2 [...]] output_filename")
STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--share-rw] @var{filename} [@var{filename2} [...]] @var{output_filename}
ETEXI
DEF("dd", img_dd,
- "dd [--image-opts] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] [skip=blocks] if=input of=output")
+ "dd [--image-opts] [--share-rw] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] [skip=blocks] if=input of=output")
STEXI
-@item dd [--image-opts] [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output}
+@item dd [--image-opts] [--share-rw] [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output}
ETEXI
DEF("info", img_info,
- "info [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [--backing-chain] filename")
+ "info [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [--backing-chain] [--share-rw] filename")
STEXI
-@item info [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] @var{filename}
+@item info [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] [--share-rw] @var{filename}
ETEXI
DEF("map", img_map,
- "map [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] filename")
+ "map [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [--share-rw] filename")
STEXI
-@item map [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
+@item map [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [--output=@var{ofmt}] [--share-rw] @var{filename}
ETEXI
DEF("snapshot", img_snapshot,
- "snapshot [--object objectdef] [--image-opts] [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
+ "snapshot [--object objectdef] [--image-opts] [--share-rw] [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
STEXI
-@item snapshot [--object @var{objectdef}] [--image-opts] [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
+@item snapshot [--object @var{objectdef}] [--image-opts] [--share-rw] [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
ETEXI
DEF("rebase", img_rebase,
- "rebase [--object objectdef] [--image-opts] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
+ "rebase [--object objectdef] [--image-opts] [--share-rw] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
STEXI
-@item rebase [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
+@item rebase [--object @var{objectdef}] [--image-opts] [--share-rw] [-q] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
ETEXI
DEF("resize", img_resize,
- "resize [--object objectdef] [--image-opts] [-q] filename [+ | -]size")
+ "resize [--object objectdef] [--image-opts] [--share-rw] [-q] filename [+ | -]size")
STEXI
-@item resize [--object @var{objectdef}] [--image-opts] [-q] @var{filename} [+ | -]@var{size}
+@item resize [--object @var{objectdef}] [--image-opts] [--share-rw] [-q] @var{filename} [+ | -]@var{size}
ETEXI
DEF("amend", img_amend,
- "amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] -o options filename")
+ "amend [--object objectdef] [--image-opts] [--share-rw] [-p] [-q] [-f fmt] [-t cache] -o options filename")
STEXI
-@item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
+@item amend [--object @var{objectdef}] [--image-opts] [--share-rw] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
@end table
ETEXI
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 05/20] qemu-img: Update documentation for --share-rw
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 05/20] qemu-img: Update documentation for --share-rw Fam Zheng
@ 2017-04-21 15:37 ` Eric Blake
2017-04-24 5:44 ` Fam Zheng
0 siblings, 1 reply; 36+ messages in thread
From: Eric Blake @ 2017-04-21 15:37 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: Kevin Wolf, Max Reitz, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1387 bytes --]
On 04/20/2017 10:55 PM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> qemu-img-cmds.hx | 48 ++++++++++++++++++++++++------------------------
> 1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 8ac7822..1b00bb8 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -10,15 +10,15 @@ STEXI
> ETEXI
>
> DEF("bench", img_bench,
> - "bench [-c count] [-d depth] [-f fmt] [--flush-interval=flush_interval] [-n] [--no-drain] [-o offset] [--pattern=pattern] [-q] [-s buffer_size] [-S step_size] [-t cache] [-w] filename")
> + "bench [-c count] [-d depth] [-f fmt] [--flush-interval=flush_interval] [-n] [--no-drain] [-o offset] [--pattern=pattern] [-q] [-s buffer_size] [-S step_size] [-t cache] [-w] [--share-rw] filename")
General comment - it seems that we favor the short-option spelling where
one exists; should all of these updates mention -U instead of --share-rw?
Also, why did you rename it from --unsafe-reads in an earlier revision?
After all, if I'm understanding this flag correctly, what you are asking
for is the ability to read the image in spite of other simultaneous
writers that may make your reads inconsistent.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 05/20] qemu-img: Update documentation for --share-rw
2017-04-21 15:37 ` Eric Blake
@ 2017-04-24 5:44 ` Fam Zheng
0 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-24 5:44 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Kevin Wolf, qemu-block, Max Reitz
On Fri, 04/21 10:37, Eric Blake wrote:
> On 04/20/2017 10:55 PM, Fam Zheng wrote:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> > qemu-img-cmds.hx | 48 ++++++++++++++++++++++++------------------------
> > 1 file changed, 24 insertions(+), 24 deletions(-)
> >
> > diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> > index 8ac7822..1b00bb8 100644
> > --- a/qemu-img-cmds.hx
> > +++ b/qemu-img-cmds.hx
> > @@ -10,15 +10,15 @@ STEXI
> > ETEXI
> >
> > DEF("bench", img_bench,
> > - "bench [-c count] [-d depth] [-f fmt] [--flush-interval=flush_interval] [-n] [--no-drain] [-o offset] [--pattern=pattern] [-q] [-s buffer_size] [-S step_size] [-t cache] [-w] filename")
> > + "bench [-c count] [-d depth] [-f fmt] [--flush-interval=flush_interval] [-n] [--no-drain] [-o offset] [--pattern=pattern] [-q] [-s buffer_size] [-S step_size] [-t cache] [-w] [--share-rw] filename")
>
> General comment - it seems that we favor the short-option spelling where
> one exists; should all of these updates mention -U instead of --share-rw?
OK, I can change it.
>
> Also, why did you rename it from --unsafe-reads in an earlier revision?
> After all, if I'm understanding this flag correctly, what you are asking
> for is the ability to read the image in spite of other simultaneous
> writers that may make your reads inconsistent.
It was a result of discussion with Kevin on IRC - consistent read as in the new
op blocker API is specifically for the state of the intermediate nodes in commit
job, and is orthogonal to the share-rw semantics as added to qdev. This option
here for qemu-img/qemu-io, is more close to the latter, thus the name is updated
to reflect its use case better.
Fam
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v14 06/20] qemu-io: Add --share-rw option
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (4 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 05/20] qemu-img: Update documentation for --share-rw Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 13:45 ` Kevin Wolf
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 07/20] iotests: 030: Prepare for image locking Fam Zheng
` (13 subsequent siblings)
19 siblings, 1 reply; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Add --share-rw/-U to program options and -U to open subcommand.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
qemu-io.c | 35 +++++++++++++++++++++++++++--------
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 427cbae..c4d824f 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -20,6 +20,7 @@
#include "qemu/readline.h"
#include "qemu/log.h"
#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qbool.h"
#include "qom/object_interfaces.h"
#include "sysemu/block-backend.h"
#include "block/block_int.h"
@@ -53,7 +54,8 @@ static const cmdinfo_t close_cmd = {
.oneline = "close the current open file",
};
-static int openfile(char *name, int flags, bool writethrough, QDict *opts)
+static int openfile(char *name, int flags, bool writethrough, bool share_rw,
+ QDict *opts)
{
Error *local_err = NULL;
BlockDriverState *bs;
@@ -64,6 +66,12 @@ static int openfile(char *name, int flags, bool writethrough, QDict *opts)
return 1;
}
+ if (share_rw) {
+ if (!opts) {
+ opts = qdict_new();
+ }
+ qdict_put(opts, BDRV_OPT_FORCE_SHARED_WRITE, qbool_from_bool(true));
+ }
qemuio_blk = blk_new_open(name, NULL, opts, flags, &local_err);
if (!qemuio_blk) {
error_reportf_err(local_err, "can't open%s%s: ",
@@ -108,6 +116,7 @@ static void open_help(void)
" -r, -- open file read-only\n"
" -s, -- use snapshot file\n"
" -n, -- disable host cache, short for -t none\n"
+" -U, -- allow shared write, don't lock image\n"
" -k, -- use kernel AIO implementation (on Linux only)\n"
" -t, -- use the given cache mode for the image\n"
" -d, -- use the given discard mode for the image\n"
@@ -124,7 +133,7 @@ static const cmdinfo_t open_cmd = {
.argmin = 1,
.argmax = -1,
.flags = CMD_NOFILE_OK,
- .args = "[-rsnk] [-t cache] [-d discard] [-o options] [path]",
+ .args = "[-rsnkU] [-t cache] [-d discard] [-o options] [path]",
.oneline = "open the file specified by path",
.help = open_help,
};
@@ -147,8 +156,9 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
int c;
QemuOpts *qopts;
QDict *opts;
+ bool share_rw = false;
- while ((c = getopt(argc, argv, "snro:kt:d:")) != -1) {
+ while ((c = getopt(argc, argv, "snro:kt:d:U")) != -1) {
switch (c) {
case 's':
flags |= BDRV_O_SNAPSHOT;
@@ -188,6 +198,9 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
return 0;
}
break;
+ case 'U':
+ share_rw = true;
+ break;
default:
qemu_opts_reset(&empty_opts);
return qemuio_command_usage(&open_cmd);
@@ -211,9 +224,9 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
qemu_opts_reset(&empty_opts);
if (optind == argc - 1) {
- return openfile(argv[optind], flags, writethrough, opts);
+ return openfile(argv[optind], flags, writethrough, share_rw, opts);
} else if (optind == argc) {
- return openfile(NULL, flags, writethrough, opts);
+ return openfile(NULL, flags, writethrough, share_rw, opts);
} else {
QDECREF(opts);
return qemuio_command_usage(&open_cmd);
@@ -257,6 +270,7 @@ static void usage(const char *name)
" -T, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
" specify tracing options\n"
" see qemu-img(1) man page for full description\n"
+" -U, --share-rw allow shared write\n"
" -h, --help display this help and exit\n"
" -V, --version output version information and exit\n"
"\n"
@@ -436,7 +450,7 @@ static QemuOptsList file_opts = {
int main(int argc, char **argv)
{
int readonly = 0;
- const char *sopt = "hVc:d:f:rsnmkt:T:";
+ const char *sopt = "hVc:d:f:rsnmkt:T:U";
const struct option lopt[] = {
{ "help", no_argument, NULL, 'h' },
{ "version", no_argument, NULL, 'V' },
@@ -452,6 +466,7 @@ int main(int argc, char **argv)
{ "trace", required_argument, NULL, 'T' },
{ "object", required_argument, NULL, OPTION_OBJECT },
{ "image-opts", no_argument, NULL, OPTION_IMAGE_OPTS },
+ { "share-rw", no_argument, 0, 'U'},
{ NULL, 0, NULL, 0 }
};
int c;
@@ -462,6 +477,7 @@ int main(int argc, char **argv)
QDict *opts = NULL;
const char *format = NULL;
char *trace_file = NULL;
+ bool share_rw = false;
#ifdef CONFIG_POSIX
signal(SIGPIPE, SIG_IGN);
@@ -524,6 +540,9 @@ int main(int argc, char **argv)
case 'h':
usage(progname);
exit(0);
+ case 'U':
+ share_rw = true;
+ break;
case OPTION_OBJECT: {
QemuOpts *qopts;
qopts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -595,7 +614,7 @@ int main(int argc, char **argv)
exit(1);
}
opts = qemu_opts_to_qdict(qopts, NULL);
- if (openfile(NULL, flags, writethrough, opts)) {
+ if (openfile(NULL, flags, writethrough, share_rw, opts)) {
exit(1);
}
} else {
@@ -603,7 +622,7 @@ int main(int argc, char **argv)
opts = qdict_new();
qdict_put(opts, "driver", qstring_from_str(format));
}
- if (openfile(argv[optind], flags, writethrough, opts)) {
+ if (openfile(argv[optind], flags, writethrough, share_rw, opts)) {
exit(1);
}
}
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 07/20] iotests: 030: Prepare for image locking
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (5 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 06/20] qemu-io: Add --share-rw option Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 13:51 ` Kevin Wolf
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 08/20] iotests: 046: " Fam Zheng
` (12 subsequent siblings)
19 siblings, 1 reply; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
qemu-img and qemu-io commands when guest is running need "-U" option,
add it.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/qemu-iotests/030 | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 0d472d5..5f1dce8 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -63,8 +63,8 @@ class TestSingleDrive(iotests.QMPTestCase):
def test_stream_intermediate(self):
self.assert_no_active_block_jobs()
- self.assertNotEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
+ self.assertNotEqual(qemu_io('-f', 'raw', '-rU', '-c', 'map', backing_img),
+ qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', mid_img),
'image file map matches backing file before streaming')
result = self.vm.qmp('block-stream', device='mid', job_id='stream-mid')
@@ -114,7 +114,7 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
# The image map is empty before the operation
- empty_map = qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img)
+ empty_map = qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', test_img)
# This is a no-op: no data should ever be copied from the base image
result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
@@ -125,7 +125,7 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
+ self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', '-r', test_img),
empty_map, 'image file map changed after a no-op')
def test_stream_partial(self):
@@ -197,8 +197,8 @@ class TestParallelOps(iotests.QMPTestCase):
# Check that the maps don't match before the streaming operations
for i in range(2, self.num_imgs, 2):
- self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i]),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i-1]),
+ self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[i]),
+ qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[i-1]),
'image file map matches backing file before streaming')
# Create all streaming jobs
@@ -351,8 +351,8 @@ class TestParallelOps(iotests.QMPTestCase):
def test_stream_base_node_name(self):
self.assert_no_active_block_jobs()
- self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[4]),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[3]),
+ self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[4]),
+ qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[3]),
'image file map matches backing file before streaming')
# Error: the base node does not exist
@@ -422,8 +422,8 @@ class TestQuorum(iotests.QMPTestCase):
if not iotests.supports_quorum():
return
- self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]),
+ self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.children[0]),
+ qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.backing[0]),
'image file map matches backing file before streaming')
self.assert_no_active_block_jobs()
@@ -436,8 +436,8 @@ class TestQuorum(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]),
+ self.assertEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.children[0]),
+ qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.backing[0]),
'image file map does not match backing file after streaming')
class TestSmallerBackingFile(iotests.QMPTestCase):
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 07/20] iotests: 030: Prepare for image locking
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 07/20] iotests: 030: Prepare for image locking Fam Zheng
@ 2017-04-21 13:51 ` Kevin Wolf
2017-04-24 6:15 ` Fam Zheng
0 siblings, 1 reply; 36+ messages in thread
From: Kevin Wolf @ 2017-04-21 13:51 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> qemu-img and qemu-io commands when guest is running need "-U" option,
> add it.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> tests/qemu-iotests/030 | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index 0d472d5..5f1dce8 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -63,8 +63,8 @@ class TestSingleDrive(iotests.QMPTestCase):
> def test_stream_intermediate(self):
> self.assert_no_active_block_jobs()
>
> - self.assertNotEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
> - qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
> + self.assertNotEqual(qemu_io('-f', 'raw', '-rU', '-c', 'map', backing_img),
> + qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', mid_img),
> 'image file map matches backing file before streaming')
>
> result = self.vm.qmp('block-stream', device='mid', job_id='stream-mid')
> @@ -114,7 +114,7 @@ class TestSingleDrive(iotests.QMPTestCase):
> self.assert_no_active_block_jobs()
>
> # The image map is empty before the operation
> - empty_map = qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img)
> + empty_map = qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', test_img)
>
> # This is a no-op: no data should ever be copied from the base image
> result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
> @@ -125,7 +125,7 @@ class TestSingleDrive(iotests.QMPTestCase):
> self.assert_no_active_block_jobs()
> self.vm.shutdown()
>
> - self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> + self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', '-r', test_img),
> empty_map, 'image file map changed after a no-op')
This one doesn't seem strictly necessary, as you're only adding -r
without -U. I still think it's a good idea to use -r where we can, but
if we decide to do this, there are more places in this test that could
be updated.
Maybe a separate patch for adding -r without -U to the cases where
qemu-io is run after shutting down the VM?
> def test_stream_partial(self):
> @@ -197,8 +197,8 @@ class TestParallelOps(iotests.QMPTestCase):
>
> # Check that the maps don't match before the streaming operations
> for i in range(2, self.num_imgs, 2):
> - self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i]),
> - qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i-1]),
> + self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[i]),
> + qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[i-1]),
> 'image file map matches backing file before streaming')
>
> # Create all streaming jobs
> @@ -351,8 +351,8 @@ class TestParallelOps(iotests.QMPTestCase):
> def test_stream_base_node_name(self):
> self.assert_no_active_block_jobs()
>
> - self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[4]),
> - qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[3]),
> + self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[4]),
> + qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.imgs[3]),
> 'image file map matches backing file before streaming')
>
> # Error: the base node does not exist
> @@ -422,8 +422,8 @@ class TestQuorum(iotests.QMPTestCase):
> if not iotests.supports_quorum():
> return
>
> - self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]),
> - qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]),
> + self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.children[0]),
> + qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.backing[0]),
> 'image file map matches backing file before streaming')
>
> self.assert_no_active_block_jobs()
> @@ -436,8 +436,8 @@ class TestQuorum(iotests.QMPTestCase):
> self.assert_no_active_block_jobs()
> self.vm.shutdown()
>
> - self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]),
> - qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]),
> + self.assertEqual(qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.children[0]),
> + qemu_io('-f', iotests.imgfmt, '-U', '-c', 'map', self.backing[0]),
> 'image file map does not match backing file after streaming')
These hunks all have -U without -r, which I think we wanted to make an
error.
Kevin
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 07/20] iotests: 030: Prepare for image locking
2017-04-21 13:51 ` Kevin Wolf
@ 2017-04-24 6:15 ` Fam Zheng
0 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-24 6:15 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
On Fri, 04/21 15:51, Kevin Wolf wrote:
> Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben:
> > qemu-img and qemu-io commands when guest is running need "-U" option,
> > add it.
> >
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> > tests/qemu-iotests/030 | 24 ++++++++++++------------
> > 1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> > index 0d472d5..5f1dce8 100755
> > --- a/tests/qemu-iotests/030
> > +++ b/tests/qemu-iotests/030
> > @@ -63,8 +63,8 @@ class TestSingleDrive(iotests.QMPTestCase):
> > def test_stream_intermediate(self):
> > self.assert_no_active_block_jobs()
> >
> > - self.assertNotEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
> > - qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
> > + self.assertNotEqual(qemu_io('-f', 'raw', '-rU', '-c', 'map', backing_img),
> > + qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', mid_img),
> > 'image file map matches backing file before streaming')
> >
> > result = self.vm.qmp('block-stream', device='mid', job_id='stream-mid')
> > @@ -114,7 +114,7 @@ class TestSingleDrive(iotests.QMPTestCase):
> > self.assert_no_active_block_jobs()
> >
> > # The image map is empty before the operation
> > - empty_map = qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img)
> > + empty_map = qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', test_img)
> >
> > # This is a no-op: no data should ever be copied from the base image
> > result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
> > @@ -125,7 +125,7 @@ class TestSingleDrive(iotests.QMPTestCase):
> > self.assert_no_active_block_jobs()
> > self.vm.shutdown()
> >
> > - self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> > + self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', '-r', test_img),
> > empty_map, 'image file map changed after a no-op')
>
> This one doesn't seem strictly necessary, as you're only adding -r
> without -U. I still think it's a good idea to use -r where we can, but
> if we decide to do this, there are more places in this test that could
> be updated.
>
> Maybe a separate patch for adding -r without -U to the cases where
> qemu-io is run after shutting down the VM?
Yes, will split. (In a previous version -U is implied by -r, and the patch was
"add -r to all qemu-io invocations where writing is not needed.)
Fam
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v14 08/20] iotests: 046: Prepare for image locking
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (6 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 07/20] iotests: 030: Prepare for image locking Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 09/20] iotests: 055: Don't attach the target image already for drive-backup Fam Zheng
` (11 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
The qemu-img info command is executed while VM is running, add -U option
to avoid the image locking error.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/qemu-iotests/046 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/046 b/tests/qemu-iotests/046
index e528b67..f2ebecf 100755
--- a/tests/qemu-iotests/046
+++ b/tests/qemu-iotests/046
@@ -192,7 +192,7 @@ echo "== Verify image content =="
function verify_io()
{
- if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | grep "compat: 0.10" > /dev/null); then
+ if ($QEMU_IMG info -U -f "$IMGFMT" "$TEST_IMG" | grep "compat: 0.10" > /dev/null); then
# For v2 images, discarded clusters are read from the backing file
# Keep the variable empty so that the backing file value can be used as
# the default below
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 09/20] iotests: 055: Don't attach the target image already for drive-backup
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (7 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 08/20] iotests: 046: " Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 10/20] iotests: 085: Avoid image locking conflict Fam Zheng
` (10 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Double attach is not a valid usage of the target image, drive-backup
will open the blockdev itself so skip the add_drive call in this case.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/055 | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index aafcd24..ba4da65 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -458,17 +458,18 @@ class TestDriveCompression(iotests.QMPTestCase):
except OSError:
pass
- def do_prepare_drives(self, fmt, args):
+ def do_prepare_drives(self, fmt, args, attach_target):
self.vm = iotests.VM().add_drive(test_img)
qemu_img('create', '-f', fmt, blockdev_target_img,
str(TestDriveCompression.image_len), *args)
- self.vm.add_drive(blockdev_target_img, format=fmt, interface="none")
+ if attach_target:
+ self.vm.add_drive(blockdev_target_img, format=fmt, interface="none")
self.vm.launch()
- def do_test_compress_complete(self, cmd, format, **args):
- self.do_prepare_drives(format['type'], format['args'])
+ def do_test_compress_complete(self, cmd, format, attach_target, **args):
+ self.do_prepare_drives(format['type'], format['args'], attach_target)
self.assert_no_active_block_jobs()
@@ -484,15 +485,16 @@ class TestDriveCompression(iotests.QMPTestCase):
def test_complete_compress_drive_backup(self):
for format in TestDriveCompression.fmt_supports_compression:
- self.do_test_compress_complete('drive-backup', format,
+ self.do_test_compress_complete('drive-backup', format, False,
target=blockdev_target_img, mode='existing')
def test_complete_compress_blockdev_backup(self):
for format in TestDriveCompression.fmt_supports_compression:
- self.do_test_compress_complete('blockdev-backup', format, target='drive1')
+ self.do_test_compress_complete('blockdev-backup', format, True,
+ target='drive1')
- def do_test_compress_cancel(self, cmd, format, **args):
- self.do_prepare_drives(format['type'], format['args'])
+ def do_test_compress_cancel(self, cmd, format, attach_target, **args):
+ self.do_prepare_drives(format['type'], format['args'], attach_target)
self.assert_no_active_block_jobs()
@@ -506,15 +508,16 @@ class TestDriveCompression(iotests.QMPTestCase):
def test_compress_cancel_drive_backup(self):
for format in TestDriveCompression.fmt_supports_compression:
- self.do_test_compress_cancel('drive-backup', format,
+ self.do_test_compress_cancel('drive-backup', format, False,
target=blockdev_target_img, mode='existing')
def test_compress_cancel_blockdev_backup(self):
for format in TestDriveCompression.fmt_supports_compression:
- self.do_test_compress_cancel('blockdev-backup', format, target='drive1')
+ self.do_test_compress_cancel('blockdev-backup', format, True,
+ target='drive1')
- def do_test_compress_pause(self, cmd, format, **args):
- self.do_prepare_drives(format['type'], format['args'])
+ def do_test_compress_pause(self, cmd, format, attach_target, **args):
+ self.do_prepare_drives(format['type'], format['args'], attach_target)
self.assert_no_active_block_jobs()
@@ -546,12 +549,13 @@ class TestDriveCompression(iotests.QMPTestCase):
def test_compress_pause_drive_backup(self):
for format in TestDriveCompression.fmt_supports_compression:
- self.do_test_compress_pause('drive-backup', format,
+ self.do_test_compress_pause('drive-backup', format, False,
target=blockdev_target_img, mode='existing')
def test_compress_pause_blockdev_backup(self):
for format in TestDriveCompression.fmt_supports_compression:
- self.do_test_compress_pause('blockdev-backup', format, target='drive1')
+ self.do_test_compress_pause('blockdev-backup', format, True,
+ target='drive1')
if __name__ == '__main__':
iotests.main(supported_fmts=['raw', 'qcow2'])
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 10/20] iotests: 085: Avoid image locking conflict
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (8 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 09/20] iotests: 055: Don't attach the target image already for drive-backup Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 11/20] iotests: 087: Don't attach test image twice Fam Zheng
` (9 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
In the case where we test the expected error when a blockdev-snapshot
target already has a backing image, the backing chain is opened multiple
times. This will be a problem when we use image locking, so use a
different backing file that is not already open.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/qemu-iotests/085 | 34 ++++++++++++++++++++--------------
tests/qemu-iotests/085.out | 3 ++-
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index c53e97f..cc6efd8 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -45,7 +45,7 @@ _cleanup()
rm -f "${TEST_DIR}/${i}-${snapshot_virt0}"
rm -f "${TEST_DIR}/${i}-${snapshot_virt1}"
done
- rm -f "${TEST_IMG}.1" "${TEST_IMG}.2"
+ rm -f "${TEST_IMG}" "${TEST_IMG}.1" "${TEST_IMG}.2" "${TEST_IMG}.base"
}
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -87,24 +87,26 @@ function create_group_snapshot()
}
# ${1}: unique identifier for the snapshot filename
-# ${2}: true: open backing images; false: don't open them (default)
+# ${2}: extra_params to the blockdev-add command
+# ${3}: filename
+function do_blockdev_add()
+{
+ cmd="{ 'execute': 'blockdev-add', 'arguments':
+ { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${2}
+ 'file':
+ { 'driver': 'file', 'filename': '${3}',
+ 'node-name': 'file_${1}' } } }"
+ _send_qemu_cmd $h "${cmd}" "return"
+}
+
+# ${1}: unique identifier for the snapshot filename
function add_snapshot_image()
{
- if [ "${2}" = "true" ]; then
- extra_params=""
- else
- extra_params="'backing': '', "
- fi
base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}"
snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"
_make_test_img -b "${base_image}" "$size"
mv "${TEST_IMG}" "${snapshot_file}"
- cmd="{ 'execute': 'blockdev-add', 'arguments':
- { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
- 'file':
- { 'driver': 'file', 'filename': '${snapshot_file}',
- 'node-name': 'file_${1}' } } }"
- _send_qemu_cmd $h "${cmd}" "return"
+ do_blockdev_add "$1" "'backing': '', " "${snapshot_file}"
}
# ${1}: unique identifier for the snapshot filename
@@ -222,7 +224,11 @@ echo === Invalid command - snapshot node has a backing image ===
echo
SNAPSHOTS=$((${SNAPSHOTS}+1))
-add_snapshot_image ${SNAPSHOTS} true
+
+_make_test_img "$size"
+mv "${TEST_IMG}" "${TEST_IMG}.base"
+_make_test_img -b "${TEST_IMG}.base" "$size"
+do_blockdev_add ${SNAPSHOTS} "" "${TEST_IMG}"
blockdev_snapshot ${SNAPSHOTS} error
echo
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 182acb4..65b0f68 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -78,7 +78,8 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
=== Invalid command - snapshot node has a backing image ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/12-snapshot-v0.IMGFMT
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
{"return": {}}
{"error": {"class": "GenericError", "desc": "The snapshot already has a backing image"}}
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 11/20] iotests: 087: Don't attach test image twice
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (9 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 10/20] iotests: 085: Avoid image locking conflict Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 12/20] iotests: 091: Quit QEMU before checking image Fam Zheng
` (8 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
The test scenario doesn't require the same image, instead it focuses on
the duplicated node-name, so use null-co to avoid locking conflict.
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/qemu-iotests/087 | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index 9de57dd..6d52f7d 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -82,8 +82,7 @@ run_qemu -drive driver=$IMGFMT,id=disk,node-name=test-node,file="$TEST_IMG" <<EO
"driver": "$IMGFMT",
"node-name": "disk",
"file": {
- "driver": "file",
- "filename": "$TEST_IMG"
+ "driver": "null-co"
}
}
}
@@ -92,8 +91,7 @@ run_qemu -drive driver=$IMGFMT,id=disk,node-name=test-node,file="$TEST_IMG" <<EO
"driver": "$IMGFMT",
"node-name": "test-node",
"file": {
- "driver": "file",
- "filename": "$TEST_IMG"
+ "driver": "null-co"
}
}
}
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 12/20] iotests: 091: Quit QEMU before checking image
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (10 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 11/20] iotests: 087: Don't attach test image twice Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 13/20] iotests: 172: Use separate images for multiple devices Fam Zheng
` (7 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/091 | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index 32bbd56..10ac4a8 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -95,7 +95,9 @@ echo "vm2: qemu process running successfully"
echo "vm2: flush io, and quit"
_send_qemu_cmd $h2 'qemu-io disk flush' "(qemu)"
_send_qemu_cmd $h2 'quit' ""
+_send_qemu_cmd $h1 'quit' ""
+wait
echo "Check image pattern"
${QEMU_IO} -c "read -P 0x22 0 4M" "${TEST_IMG}" | _filter_testdir | _filter_qemu_io
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 13/20] iotests: 172: Use separate images for multiple devices
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (11 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 12/20] iotests: 091: Quit QEMU before checking image Fam Zheng
@ 2017-04-21 3:55 ` Fam Zheng
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 14/20] tests: Use null-co:// instead of /dev/null as the dummy image Fam Zheng
` (6 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:55 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
To avoid image lock failures.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/qemu-iotests/172 | 55 +++++++++++++++++++++++++---------------------
tests/qemu-iotests/172.out | 50 +++++++++++++++++++++--------------------
2 files changed, 56 insertions(+), 49 deletions(-)
diff --git a/tests/qemu-iotests/172 b/tests/qemu-iotests/172
index 1b7d3a1..826d6fe 100755
--- a/tests/qemu-iotests/172
+++ b/tests/qemu-iotests/172
@@ -30,6 +30,8 @@ status=1 # failure is the default!
_cleanup()
{
_cleanup_test_img
+ rm -f "$TEST_IMG.2"
+ rm -f "$TEST_IMG.3"
}
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -86,6 +88,9 @@ size=720k
_make_test_img $size
+TEST_IMG="$TEST_IMG.2" _make_test_img $size
+TEST_IMG="$TEST_IMG.3" _make_test_img $size
+
# Default drive semantics:
#
# By default you get a single empty floppy drive. You can override it with
@@ -105,7 +110,7 @@ echo === Using -fda/-fdb options ===
check_floppy_qtree -fda "$TEST_IMG"
check_floppy_qtree -fdb "$TEST_IMG"
-check_floppy_qtree -fda "$TEST_IMG" -fdb "$TEST_IMG"
+check_floppy_qtree -fda "$TEST_IMG" -fdb "$TEST_IMG.2"
echo
@@ -114,7 +119,7 @@ echo === Using -drive options ===
check_floppy_qtree -drive if=floppy,file="$TEST_IMG"
check_floppy_qtree -drive if=floppy,file="$TEST_IMG",index=1
-check_floppy_qtree -drive if=floppy,file="$TEST_IMG" -drive if=floppy,file="$TEST_IMG",index=1
+check_floppy_qtree -drive if=floppy,file="$TEST_IMG" -drive if=floppy,file="$TEST_IMG.2",index=1
echo
echo
@@ -122,7 +127,7 @@ echo === Using -drive if=none and -global ===
check_floppy_qtree -drive if=none,file="$TEST_IMG" -global isa-fdc.driveA=none0
check_floppy_qtree -drive if=none,file="$TEST_IMG" -global isa-fdc.driveB=none0
-check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" \
+check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveA=none0 -global isa-fdc.driveB=none1
echo
@@ -131,7 +136,7 @@ echo === Using -drive if=none and -device ===
check_floppy_qtree -drive if=none,file="$TEST_IMG" -device floppy,drive=none0
check_floppy_qtree -drive if=none,file="$TEST_IMG" -device floppy,drive=none0,unit=1
-check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" \
+check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-device floppy,drive=none0 -device floppy,drive=none1,unit=1
echo
@@ -139,58 +144,58 @@ echo
echo === Mixing -fdX and -global ===
# Working
-check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG" -global isa-fdc.driveB=none0
-check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG" -global isa-fdc.driveA=none0
+check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveB=none0
+check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveA=none0
# Conflicting (-fdX wins)
-check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG" -global isa-fdc.driveA=none0
-check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG" -global isa-fdc.driveB=none0
+check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveA=none0
+check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveB=none0
echo
echo
echo === Mixing -fdX and -device ===
# Working
-check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG" -device floppy,drive=none0
-check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG" -device floppy,drive=none0,unit=1
+check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -device floppy,drive=none0
+check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -device floppy,drive=none0,unit=1
-check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG" -device floppy,drive=none0
-check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG" -device floppy,drive=none0,unit=0
+check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -device floppy,drive=none0
+check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -device floppy,drive=none0,unit=0
# Conflicting
-check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG" -device floppy,drive=none0,unit=0
-check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG" -device floppy,drive=none0,unit=1
+check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -device floppy,drive=none0,unit=0
+check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -device floppy,drive=none0,unit=1
echo
echo
echo === Mixing -drive and -device ===
# Working
-check_floppy_qtree -drive if=floppy,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" -device floppy,drive=none0
-check_floppy_qtree -drive if=floppy,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" -device floppy,drive=none0,unit=1
+check_floppy_qtree -drive if=floppy,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -device floppy,drive=none0
+check_floppy_qtree -drive if=floppy,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -device floppy,drive=none0,unit=1
# Conflicting
-check_floppy_qtree -drive if=floppy,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" -device floppy,drive=none0,unit=0
+check_floppy_qtree -drive if=floppy,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -device floppy,drive=none0,unit=0
echo
echo
echo === Mixing -global and -device ===
# Working
-check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" \
+check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveA=none0 -device floppy,drive=none1
-check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" \
+check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=1
-check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" \
+check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveB=none0 -device floppy,drive=none1
-check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" \
+check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=0
# Conflicting
-check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" \
+check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=0
-check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG" \
+check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=1
echo
@@ -199,8 +204,8 @@ echo === Too many floppy drives ===
# Working
check_floppy_qtree -drive if=floppy,file="$TEST_IMG" \
- -drive if=none,file="$TEST_IMG" \
- -drive if=none,file="$TEST_IMG" \
+ -drive if=none,file="$TEST_IMG.2" \
+ -drive if=none,file="$TEST_IMG.3" \
-global isa-fdc.driveB=none0 \
-device floppy,drive=none1
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index 54b5329..2732966 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -1,5 +1,7 @@
QA output created by 172
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=737280
+Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=737280
+Formatting 'TEST_DIR/t.IMGFMT.3', fmt=IMGFMT size=737280
=== Default ===
@@ -99,7 +101,7 @@ Testing: -fdb TEST_DIR/t.qcow2
share-rw = false
drive-type = "288"
-Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2
+Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -205,7 +207,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
share-rw = false
drive-type = "288"
-Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
+Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t.qcow2.2,index=1
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -300,7 +302,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0
share-rw = false
drive-type = "144"
-Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 -global isa-fdc.driveB=none1
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -global isa-fdc.driveB=none1
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -395,7 +397,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
share-rw = false
drive-type = "144"
-Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0 -device floppy,drive=none1,unit=1
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0 -device floppy,drive=none1,unit=1
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -436,7 +438,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
=== Mixing -fdX and -global ===
-Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0
+Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -474,7 +476,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-
share-rw = false
drive-type = "144"
-Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0
+Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -512,7 +514,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-
share-rw = false
drive-type = "144"
-Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0
+Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -539,7 +541,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-
share-rw = false
drive-type = "144"
-Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0
+Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -569,7 +571,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-
=== Mixing -fdX and -device ===
-Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0
+Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -607,7 +609,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop
share-rw = false
drive-type = "144"
-Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
+Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=1
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -645,7 +647,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop
share-rw = false
drive-type = "144"
-Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0
+Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -683,7 +685,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop
share-rw = false
drive-type = "144"
-Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=0
+Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=0
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -721,18 +723,18 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop
share-rw = false
drive-type = "144"
-Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=0
+Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=0
QEMU_PROG: -device floppy,drive=none0,unit=0: Floppy unit 0 is in use
QEMU_PROG: -device floppy,drive=none0,unit=0: Device initialization failed.
-Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
+Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=1
QEMU_PROG: -device floppy,drive=none0,unit=1: Floppy unit 1 is in use
QEMU_PROG: -device floppy,drive=none0,unit=1: Device initialization failed.
=== Mixing -drive and -device ===
-Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0
+Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -770,7 +772,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
share-rw = false
drive-type = "144"
-Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
+Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=1
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -808,14 +810,14 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
share-rw = false
drive-type = "144"
-Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=0
+Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=0
QEMU_PROG: -device floppy,drive=none0,unit=0: Floppy unit 0 is in use
QEMU_PROG: -device floppy,drive=none0,unit=0: Device initialization failed.
=== Mixing -global and -device ===
-Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 -device floppy,drive=none1
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device floppy,drive=none1
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -853,7 +855,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
share-rw = false
drive-type = "144"
-Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=1
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=1
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -891,7 +893,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
share-rw = false
drive-type = "144"
-Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0 -device floppy,drive=none1
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device floppy,drive=none1
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -929,7 +931,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
share-rw = false
drive-type = "144"
-Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=0
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=0
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -967,18 +969,18 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
share-rw = false
drive-type = "144"
-Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=0
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=0
QEMU_PROG: -device floppy,drive=none1,unit=0: Floppy unit 0 is in use
QEMU_PROG: -device floppy,drive=none1,unit=0: Device initialization failed.
-Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=1
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=1
QEMU_PROG: -device floppy,drive=none1,unit=1: Floppy unit 1 is in use
QEMU_PROG: -device floppy,drive=none1,unit=1: Device initialization failed.
=== Too many floppy drives ===
-Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0 -device floppy,drive=none1
+Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -drive if=none,file=TEST_DIR/t.qcow2.3 -global isa-fdc.driveB=none0 -device floppy,drive=none1
QEMU_PROG: -device floppy,drive=none1: Can't create floppy unit 2, bus supports only 2 units
QEMU_PROG: -device floppy,drive=none1: Device initialization failed.
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 14/20] tests: Use null-co:// instead of /dev/null as the dummy image
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (12 preceding siblings ...)
2017-04-21 3:55 ` [Qemu-devel] [PATCH v14 13/20] iotests: 172: Use separate images for multiple devices Fam Zheng
@ 2017-04-21 3:56 ` Fam Zheng
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 15/20] file-posix: Add 'locking' option Fam Zheng
` (5 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:56 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
tests/drive_del-test.c | 2 +-
tests/nvme-test.c | 2 +-
tests/usb-hcd-uhci-test.c | 2 +-
tests/usb-hcd-xhci-test.c | 2 +-
tests/virtio-blk-test.c | 2 +-
tests/virtio-scsi-test.c | 4 ++--
6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c
index 121b9c9..2175139 100644
--- a/tests/drive_del-test.c
+++ b/tests/drive_del-test.c
@@ -92,7 +92,7 @@ static void test_after_failed_device_add(void)
static void test_drive_del_device_del(void)
{
/* Start with a drive used by a device that unplugs instantaneously */
- qtest_start("-drive if=none,id=drive0,file=/dev/null,format=raw"
+ qtest_start("-drive if=none,id=drive0,file=null-co://,format=raw"
" -device virtio-scsi-pci"
" -device scsi-hd,drive=drive0,id=dev0");
diff --git a/tests/nvme-test.c b/tests/nvme-test.c
index c8bece4..7674a44 100644
--- a/tests/nvme-test.c
+++ b/tests/nvme-test.c
@@ -22,7 +22,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
qtest_add_func("/nvme/nop", nop);
- qtest_start("-drive id=drv0,if=none,file=/dev/null,format=raw "
+ qtest_start("-drive id=drv0,if=none,file=null-co://,format=raw "
"-device nvme,drive=drv0,serial=foo");
ret = g_test_run();
diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index f25bae5..5b500fe 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -79,7 +79,7 @@ int main(int argc, char **argv)
{
const char *arch = qtest_get_arch();
const char *cmd = "-device piix3-usb-uhci,id=uhci,addr=1d.0"
- " -drive id=drive0,if=none,file=/dev/null,format=raw"
+ " -drive id=drive0,if=none,file=null-co://,format=raw"
" -device usb-tablet,bus=uhci.0,port=1";
int ret;
diff --git a/tests/usb-hcd-xhci-test.c b/tests/usb-hcd-xhci-test.c
index 22513e9..031764d 100644
--- a/tests/usb-hcd-xhci-test.c
+++ b/tests/usb-hcd-xhci-test.c
@@ -89,7 +89,7 @@ int main(int argc, char **argv)
qtest_add_func("/xhci/pci/hotplug/usb-uas", test_usb_uas_hotplug);
qtest_start("-device nec-usb-xhci,id=xhci"
- " -drive id=drive0,if=none,file=/dev/null,format=raw");
+ " -drive id=drive0,if=none,file=null-co://,format=raw");
ret = g_test_run();
qtest_end();
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 1eee95d..fd2078c 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -63,7 +63,7 @@ static QOSState *pci_test_start(void)
const char *arch = qtest_get_arch();
char *tmp_path;
const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
- "-drive if=none,id=drive1,file=/dev/null,format=raw "
+ "-drive if=none,id=drive1,file=null-co://,format=raw "
"-device virtio-blk-pci,id=drv0,drive=drive0,"
"addr=%x.%x";
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 0eabd56..5e2ff14 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -35,7 +35,7 @@ typedef struct {
static QOSState *qvirtio_scsi_start(const char *extra_opts)
{
const char *arch = qtest_get_arch();
- const char *cmd = "-drive id=drv0,if=none,file=/dev/null,format=raw "
+ const char *cmd = "-drive id=drv0,if=none,file=null-co://,format=raw "
"-device virtio-scsi-pci,id=vs0 "
"-device scsi-hd,bus=vs0.0,drive=drv0 %s";
@@ -195,7 +195,7 @@ static void hotplug(void)
QDict *response;
QOSState *qs;
- qs = qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
+ qs = qvirtio_scsi_start("-drive id=drv1,if=none,file=null-co://,format=raw");
response = qmp("{\"execute\": \"device_add\","
" \"arguments\": {"
" \"driver\": \"scsi-hd\","
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 15/20] file-posix: Add 'locking' option
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (13 preceding siblings ...)
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 14/20] tests: Use null-co:// instead of /dev/null as the dummy image Fam Zheng
@ 2017-04-21 3:56 ` Fam Zheng
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 16/20] tests: Disable image lock in test-replication Fam Zheng
` (4 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:56 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Making this option available even before implementing it will let
converting tests easier: in coming patches they can specify the option
already when necessary, before we actually write code to lock the
images.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/file-posix.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/block/file-posix.c b/block/file-posix.c
index ade71db..2114720 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -392,6 +392,11 @@ static QemuOptsList raw_runtime_opts = {
.type = QEMU_OPT_STRING,
.help = "host AIO implementation (threads, native)",
},
+ {
+ .name = "locking",
+ .type = QEMU_OPT_BOOL,
+ .help = "lock the file",
+ },
{ /* end of list */ }
},
};
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 16/20] tests: Disable image lock in test-replication
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (14 preceding siblings ...)
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 15/20] file-posix: Add 'locking' option Fam Zheng
@ 2017-04-21 3:56 ` Fam Zheng
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 17/20] block: Reuse bs as backing hd for drive-backup sync=none Fam Zheng
` (3 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:56 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
The COLO block replication architecture requires one disk to be shared
between primary and secondary, in the test both processes use posix file
protocol (instead of over NBD) so it is affected by image locking.
Disable the lock.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/test-replication.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/test-replication.c b/tests/test-replication.c
index fac2da3..e1c4235 100644
--- a/tests/test-replication.c
+++ b/tests/test-replication.c
@@ -179,7 +179,8 @@ static BlockBackend *start_primary(void)
char *cmdline;
cmdline = g_strdup_printf("driver=replication,mode=primary,node-name=xxx,"
- "file.driver=qcow2,file.file.filename=%s"
+ "file.driver=qcow2,file.file.filename=%s,"
+ "file.file.locking=off"
, p_local_disk);
opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
g_free(cmdline);
@@ -310,7 +311,9 @@ static BlockBackend *start_secondary(void)
Error *local_err = NULL;
/* add s_local_disk and forge S_LOCAL_DISK_ID */
- cmdline = g_strdup_printf("file.filename=%s,driver=qcow2", s_local_disk);
+ cmdline = g_strdup_printf("file.filename=%s,driver=qcow2,"
+ "file.locking=off",
+ s_local_disk);
opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
g_free(cmdline);
@@ -331,8 +334,10 @@ static BlockBackend *start_secondary(void)
/* add S_(ACTIVE/HIDDEN)_DISK and forge S_ID */
cmdline = g_strdup_printf("driver=replication,mode=secondary,top-id=%s,"
"file.driver=qcow2,file.file.filename=%s,"
+ "file.file.locking=off,"
"file.backing.driver=qcow2,"
"file.backing.file.filename=%s,"
+ "file.backing.file.locking=off,"
"file.backing.backing=%s"
, S_ID, s_active_disk, s_hidden_disk
, S_LOCAL_DISK_ID);
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 17/20] block: Reuse bs as backing hd for drive-backup sync=none
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (15 preceding siblings ...)
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 16/20] tests: Disable image lock in test-replication Fam Zheng
@ 2017-04-21 3:56 ` Fam Zheng
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 18/20] osdep: Add qemu_lock_fd and qemu_unlock_fd Fam Zheng
` (2 subsequent siblings)
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:56 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
blockdev.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 4927914..4e04dec 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3174,6 +3174,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
Error *local_err = NULL;
int flags;
int64_t size;
+ bool set_backing_hd = false;
if (!backup->has_speed) {
backup->speed = 0;
@@ -3224,6 +3225,8 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
}
if (backup->sync == MIRROR_SYNC_MODE_NONE) {
source = bs;
+ flags |= BDRV_O_NO_BACKING;
+ set_backing_hd = true;
}
size = bdrv_getlength(bs);
@@ -3250,7 +3253,9 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
}
if (backup->format) {
- options = qdict_new();
+ if (!options) {
+ options = qdict_new();
+ }
qdict_put(options, "driver", qstring_from_str(backup->format));
}
@@ -3261,6 +3266,14 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
bdrv_set_aio_context(target_bs, aio_context);
+ if (set_backing_hd) {
+ bdrv_set_backing_hd(target_bs, source, &local_err);
+ if (local_err) {
+ bdrv_unref(target_bs);
+ goto out;
+ }
+ }
+
if (backup->has_bitmap) {
bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
if (!bmap) {
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 18/20] osdep: Add qemu_lock_fd and qemu_unlock_fd
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (16 preceding siblings ...)
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 17/20] block: Reuse bs as backing hd for drive-backup sync=none Fam Zheng
@ 2017-04-21 3:56 ` Fam Zheng
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 19/20] file-posix: Add image locking in perm operations Fam Zheng
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 20/20] qemu-iotests: Add test case 153 for image locking Fam Zheng
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:56 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
They are wrappers of POSIX fcntl "file private locking", with a
convenient "try lock" wrapper implemented with F_OFD_GETLK.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/qemu/osdep.h | 3 +++
util/osdep.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 122ff06..1c9f5e2 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -341,6 +341,9 @@ int qemu_close(int fd);
#ifndef _WIN32
int qemu_dup(int fd);
#endif
+int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive);
+int qemu_unlock_fd(int fd, int64_t start, int64_t len);
+int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive);
#if defined(__HAIKU__) && defined(__i386__)
#define FMT_pid "%ld"
diff --git a/util/osdep.c b/util/osdep.c
index 06fb1cf..3de4a18 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -140,6 +140,54 @@ static int qemu_parse_fdset(const char *param)
{
return qemu_parse_fd(param);
}
+
+static int qemu_lock_fcntl(int fd, int64_t start, int64_t len, int fl_type)
+{
+#ifdef F_OFD_SETLK
+ int ret;
+ struct flock fl = {
+ .l_whence = SEEK_SET,
+ .l_start = start,
+ .l_len = len,
+ .l_type = fl_type,
+ };
+ ret = fcntl(fd, F_OFD_SETLK, &fl);
+ return ret == -1 ? -errno : 0;
+#else
+ return -ENOTSUP;
+#endif
+}
+
+int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive)
+{
+ return qemu_lock_fcntl(fd, start, len, exclusive ? F_WRLCK : F_RDLCK);
+}
+
+int qemu_unlock_fd(int fd, int64_t start, int64_t len)
+{
+ return qemu_lock_fcntl(fd, start, len, F_UNLCK);
+}
+
+int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive)
+{
+#ifdef F_OFD_SETLK
+ int ret;
+ struct flock fl = {
+ .l_whence = SEEK_SET,
+ .l_start = start,
+ .l_len = len,
+ .l_type = exclusive ? F_WRLCK : F_RDLCK,
+ };
+ ret = fcntl(fd, F_OFD_GETLK, &fl);
+ if (ret == -1) {
+ return -errno;
+ } else {
+ return fl.l_type == F_UNLCK ? 0 : -EAGAIN;
+ }
+#else
+ return -ENOTSUP;
+#endif
+}
#endif
/*
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v14 19/20] file-posix: Add image locking in perm operations
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (17 preceding siblings ...)
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 18/20] osdep: Add qemu_lock_fd and qemu_unlock_fd Fam Zheng
@ 2017-04-21 3:56 ` Fam Zheng
2017-04-24 13:39 ` Kevin Wolf
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 20/20] qemu-iotests: Add test case 153 for image locking Fam Zheng
19 siblings, 1 reply; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:56 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
virtlockd in libvirt locks the first byte, so we start looking at the
file bytes from 0x10.
The complication is in the transactional interface. To make the reopen
logic managable, and allow better reuse, the code is internally
organized with a table from old mode to the new one.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/file-posix.c | 739 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 736 insertions(+), 3 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 2114720..c307493 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -129,8 +129,54 @@ do { \
#define MAX_BLOCKSIZE 4096
+/* Posix file locking bytes. Libvirt takes byte 0, we start from byte 0x10,
+ * leaving a few more bytes for its future use. */
+#define RAW_LOCK_BYTE_MIN 0x10
+#define RAW_LOCK_BYTE_NO_OTHER_WRITER 0x10
+#define RAW_LOCK_BYTE_WRITE 0x11
+#ifdef F_OFD_SETLK
+#define RAW_LOCK_SUPPORTED 1
+#else
+#define RAW_LOCK_SUPPORTED 0
+#endif
+
+/*
+ ** reader that can tolerate writers: Don't do anything
+ *
+ ** reader that can't tolerate writers: Take shared lock on byte 0x10. Test
+ * byte 0x11 is unlocked.
+ *
+ ** shared writer: Take shared lock on byte 0x11. Test byte 0x10 is unlocked.
+ *
+ ** exclusive writer: Take exclusive locks on both bytes.
+ */
+
+typedef enum {
+ /* Read only and accept other writers. */
+ RAW_L_READ_SHARE_RW,
+ /* Read only and try to forbid other writers. */
+ RAW_L_READ,
+ /* Read/write and accept other writers. */
+ RAW_L_WRITE_SHARE_RW,
+ /* Read/write and try to forbid other writers. */
+ RAW_L_WRITE,
+} BDRVRawLockMode;
+
+typedef struct BDRVRawLockUpdateState {
+ /* A dup of @fd used for acquiring lock. */
+ int image_fd;
+ int lock_fd;
+ int open_flags;
+ BDRVRawLockMode new_lock;
+ bool use_lock;
+} BDRVRawLockUpdateState;
+
typedef struct BDRVRawState {
int fd;
+ /* A dup of @fd to make manipulating lock easier, especially during reopen,
+ * where this will accept BDRVRawReopenState.lock_fd. */
+ int lock_fd;
+ bool use_lock;
int type;
int open_flags;
size_t buf_align;
@@ -145,6 +191,11 @@ typedef struct BDRVRawState {
bool page_cache_inconsistent:1;
bool has_fallocate;
bool needs_alignment;
+ /* The current lock mode we are in. Note that in incoming migration this is
+ * the "desired" mode to be applied at bdrv_invalidate_cache. */
+ BDRVRawLockMode cur_lock_mode;
+ /* Used by raw_check_perm/raw_set_perm. */
+ BDRVRawLockUpdateState *lock_update;
} BDRVRawState;
typedef struct BDRVRawReopenState {
@@ -367,6 +418,59 @@ static void raw_parse_flags(int bdrv_flags, int *open_flags)
}
}
+static int raw_lock_fd(int fd, BDRVRawLockMode mode, Error **errp)
+{
+ int ret;
+ assert(fd >= 0);
+ assert(RAW_LOCK_SUPPORTED);
+ switch (mode) {
+ case RAW_L_READ_SHARE_RW:
+ ret = qemu_unlock_fd(fd, RAW_LOCK_BYTE_MIN, 2);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to unlock fd");
+ goto fail;
+ }
+ break;
+ case RAW_L_READ:
+ ret = qemu_lock_fd(fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock share byte");
+ goto fail;
+ }
+ ret = qemu_lock_fd_test(fd, RAW_LOCK_BYTE_WRITE, 1, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Write byte lock is taken");
+ goto fail;
+ }
+ break;
+ case RAW_L_WRITE_SHARE_RW:
+ ret = qemu_lock_fd(fd, RAW_LOCK_BYTE_WRITE, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock write byte");
+ goto fail;
+ }
+ ret = qemu_lock_fd_test(fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Share byte lock is taken");
+ goto fail;
+ }
+ break;
+ case RAW_L_WRITE:
+ ret = qemu_lock_fd(fd, RAW_LOCK_BYTE_MIN, 2, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock image");
+ goto fail;
+ }
+ break;
+ default:
+ abort();
+ }
+ return 0;
+fail:
+ qemu_unlock_fd(fd, RAW_LOCK_BYTE_MIN, 2);
+ return ret;
+}
+
static void raw_parse_filename(const char *filename, QDict *options,
Error **errp)
{
@@ -401,6 +505,23 @@ static QemuOptsList raw_runtime_opts = {
},
};
+static BDRVRawLockMode raw_get_lock_mode(bool write, bool shared)
+{
+ if (write) {
+ if (shared) {
+ return RAW_L_WRITE_SHARE_RW;
+ } else {
+ return RAW_L_WRITE;
+ }
+ } else {
+ if (shared) {
+ return RAW_L_READ_SHARE_RW;
+ } else {
+ return RAW_L_READ;
+ }
+ }
+}
+
static int raw_open_common(BlockDriverState *bs, QDict *options,
int bdrv_flags, int open_flags, Error **errp)
{
@@ -440,10 +561,13 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
}
s->use_linux_aio = (aio == BLOCKDEV_AIO_OPTIONS_NATIVE);
+ s->use_lock = qemu_opt_get_bool(opts, "locking", true);
+
s->open_flags = open_flags;
raw_parse_flags(bdrv_flags, &s->open_flags);
s->fd = -1;
+ s->lock_fd = -1;
fd = qemu_open(filename, s->open_flags, 0644);
if (fd < 0) {
ret = -errno;
@@ -542,6 +666,509 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
return raw_open_common(bs, options, flags, 0, errp);
}
+typedef enum {
+ RAW_LT_PREPARE,
+ RAW_LT_COMMIT,
+ RAW_LT_ABORT
+} RawLockTransOp;
+
+typedef int (*RawLockTransFunc)(RawLockTransOp op,
+ int old_lock_fd, int new_lock_fd,
+ BDRVRawLockMode old_lock,
+ BDRVRawLockMode new_lock,
+ Error **errp);
+
+static int raw_lt_nop(RawLockTransOp op,
+ int old_lock_fd, int new_lock_fd,
+ BDRVRawLockMode old_lock,
+ BDRVRawLockMode new_lock,
+ Error **errp)
+{
+ assert(old_lock == new_lock || new_lock == RAW_L_READ_SHARE_RW);
+ return 0;
+}
+
+static int raw_lt_from_unlock(RawLockTransOp op,
+ int old_lock_fd, int new_lock_fd,
+ BDRVRawLockMode old_lock,
+ BDRVRawLockMode new_lock,
+ Error **errp)
+{
+ assert(old_lock != new_lock);
+ assert(old_lock == RAW_L_READ_SHARE_RW);
+ switch (op) {
+ case RAW_LT_PREPARE:
+ return raw_lock_fd(new_lock_fd, new_lock, errp);
+ case RAW_LT_COMMIT:
+ break;
+ case RAW_LT_ABORT:
+ break;
+ }
+
+ return 0;
+}
+
+static int raw_lt_read_to_write_share(RawLockTransOp op,
+ int old_lock_fd, int new_lock_fd,
+ BDRVRawLockMode old_lock,
+ BDRVRawLockMode new_lock,
+ Error **errp)
+{
+ int ret = 0;
+
+ assert(old_lock == RAW_L_READ);
+ assert(new_lock == RAW_L_WRITE_SHARE_RW);
+
+ /*
+ * lock byte "no other writer" lock byte "write"
+ * old S 0
+ * new 0 S
+ *
+ * (0 = unlocked; S = shared; X = exclusive.)
+ */
+ switch (op) {
+ case RAW_LT_PREPARE:
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_WRITE, 1, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock new fd (write byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock new fd (share byte)");
+ break;
+ }
+ ret = qemu_unlock_fd(old_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to unlock old fd (share byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to upgrade new fd (share byte)");
+ break;
+ }
+ ret = qemu_unlock_fd(new_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1);
+ if (ret) {
+ /* This is very unlikely, but catch it anyway. */
+ error_setg_errno(errp, -ret, "Failed to unlock new fd (share byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_WRITE, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to downgrade new fd (write byte)");
+ break;
+ }
+ break;
+ case RAW_LT_COMMIT:
+ break;
+ case RAW_LT_ABORT:
+ ret = qemu_lock_fd(old_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, false);
+ if (ret) {
+ /* As unlikely as above unlock failure, but report it anyway. */
+ error_report("Failed to restore lock on old fd (share byte)");
+ }
+ break;
+ }
+ return ret;
+}
+
+static int raw_lt_read_to_write(RawLockTransOp op,
+ int old_lock_fd, int new_lock_fd,
+ BDRVRawLockMode old_lock,
+ BDRVRawLockMode new_lock,
+ Error **errp)
+{
+ int ret = 0;
+
+ assert(old_lock == RAW_L_READ);
+ assert(new_lock == RAW_L_WRITE);
+ /*
+ * lock byte "no other writer" lock byte "write"
+ * old S 0
+ * new X X
+ *
+ * (0 = unlocked; S = shared; X = exclusive.)
+ */
+ switch (op) {
+ case RAW_LT_PREPARE:
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_WRITE, 1, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock new fd (write byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock new fd (share byte)");
+ break;
+ }
+ ret = qemu_unlock_fd(old_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to unlock old fd (share byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to upgrade new fd (share byte)");
+ break;
+ }
+ break;
+ case RAW_LT_COMMIT:
+ break;
+ case RAW_LT_ABORT:
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to downgrade new fd (share byte)");
+ break;
+ }
+ ret = qemu_lock_fd(old_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, false);
+ if (ret) {
+ error_report("Failed to restore lock on old fd (share byte)");
+ }
+ break;
+ }
+ return ret;
+}
+
+static int raw_lt_write_share_to_read(RawLockTransOp op,
+ int old_lock_fd, int new_lock_fd,
+ BDRVRawLockMode old_lock,
+ BDRVRawLockMode new_lock,
+ Error **errp)
+{
+ int ret = 0;
+
+ assert(old_lock == RAW_L_WRITE_SHARE_RW);
+ assert(new_lock == RAW_L_READ);
+ /*
+ * lock byte "no other writer" lock byte "write"
+ * old 0 S
+ * new S 0
+ *
+ * (0 = unlocked; S = shared; X = exclusive.)
+ */
+ switch (op) {
+ case RAW_LT_PREPARE:
+ /* Make sure there are no other writers. */
+ ret = qemu_lock_fd(old_lock_fd, RAW_LOCK_BYTE_WRITE, 1, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock old fd (write byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, false);
+ if (!ret) {
+ break;
+ }
+ error_setg_errno(errp, -ret, "Failed to lock new fd (share byte)");
+ /* fall through */
+ case RAW_LT_ABORT:
+ ret = qemu_lock_fd(old_lock_fd, RAW_LOCK_BYTE_WRITE, 1, false);
+ if (ret) {
+ error_report("Failed to downgrade old fd (write byte)");
+ }
+ break;
+ case RAW_LT_COMMIT:
+ break;
+ }
+ return ret;
+}
+
+static int raw_lt_write_share_to_write(RawLockTransOp op,
+ int old_lock_fd, int new_lock_fd,
+ BDRVRawLockMode old_lock,
+ BDRVRawLockMode new_lock,
+ Error **errp)
+{
+ int ret = 0;
+
+ assert(old_lock == RAW_L_WRITE_SHARE_RW);
+ assert(new_lock == RAW_L_WRITE);
+ /*
+ * lock byte "no other writer" lock byte "write"
+ * old 0 S
+ * new X X
+ *
+ * (0 = unlocked; S = shared; X = exclusive.)
+ */
+ switch (op) {
+ case RAW_LT_PREPARE:
+ /* Make sure there are no other writers. */
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock new fd (share byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_WRITE, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock new fd (write byte)");
+ break;
+ }
+ ret = qemu_unlock_fd(old_lock_fd, RAW_LOCK_BYTE_WRITE, 1);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to unlock old fd (write byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_WRITE, 1, true);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to upgrade new fd (write byte)");
+ break;
+ }
+ break;
+ case RAW_LT_COMMIT:
+ break;
+ case RAW_LT_ABORT:
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_WRITE, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to downgrade new fd (write byte)");
+ break;
+ }
+ ret = qemu_lock_fd(old_lock_fd, RAW_LOCK_BYTE_WRITE, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to restore old fd (write byte)");
+ break;
+ }
+ break;
+ }
+ return ret;
+}
+
+static int raw_lt_write_to_read(RawLockTransOp op,
+ int old_lock_fd, int new_lock_fd,
+ BDRVRawLockMode old_lock,
+ BDRVRawLockMode new_lock,
+ Error **errp)
+{
+ int ret = 0;
+
+ assert(old_lock == RAW_L_WRITE);
+ assert(new_lock == RAW_L_READ);
+ /*
+ * lock byte "no other writer" lock byte "write"
+ * old X X
+ * new S 0
+ *
+ * (0 = unlocked; S = shared; X = exclusive.)
+ */
+ switch (op) {
+ case RAW_LT_PREPARE:
+ break;
+ case RAW_LT_COMMIT:
+ ret = qemu_lock_fd(old_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to downgrade old fd (share byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_NO_OTHER_WRITER, 1, false);
+ if (ret) {
+ error_setg_errno(errp, -ret, "Failed to lock new fd (share byte)");
+ break;
+ }
+ break;
+ case RAW_LT_ABORT:
+ break;
+ }
+ return ret;
+}
+
+static int raw_lt_write_to_write_share(RawLockTransOp op,
+ int old_lock_fd, int new_lock_fd,
+ BDRVRawLockMode old_lock,
+ BDRVRawLockMode new_lock,
+ Error **errp)
+{
+ int ret = 0;
+
+ assert(old_lock == RAW_L_WRITE);
+ assert(new_lock == RAW_L_WRITE_SHARE_RW);
+ /*
+ * lock byte "no other writer" lock byte "write"
+ * old X X
+ * new 0 S
+ *
+ * (0 = unlocked; S = shared; X = exclusive.)
+ */
+ switch (op) {
+ case RAW_LT_PREPARE:
+ break;
+ case RAW_LT_COMMIT:
+ ret = qemu_lock_fd(old_lock_fd, RAW_LOCK_BYTE_WRITE, 1, false);
+ if (ret) {
+ error_report("Failed to downgrade old fd (share byte)");
+ break;
+ }
+ ret = qemu_lock_fd(new_lock_fd, RAW_LOCK_BYTE_WRITE, 1, false);
+ if (ret) {
+ error_report("Failed to lock new fd (write byte)");
+ }
+ break;
+ case RAW_LT_ABORT:
+ break;
+ }
+ return ret;
+}
+
+/**
+ * Transactionally moving between possible locking states is tricky and must be
+ * done carefully. That is mostly because downgrading an exclusive lock to
+ * shared or unlocked is not guaranteed to be revertible. As a result, in such
+ * cases we have to defer the downgrading to "commit", given that no revert will
+ * happen after that point, and that downgrading a lock should never fail.
+ *
+ * On the other hand, upgrading a lock (e.g. from unlocked or shared to
+ * exclusive lock) must happen in "prepare" because it may fail.
+ *
+ * Manage the operation matrix with this state transition table to make
+ * fulfilling above conditions easier.
+ */
+static const struct RawLockTransOp {
+ BDRVRawLockMode old_lock;
+ BDRVRawLockMode new_lock;
+ RawLockTransFunc func;
+ bool need_lock_fd;
+ bool close_old_lock_fd;
+} raw_lock_trans_ops[] = {
+
+ {RAW_L_READ_SHARE_RW, RAW_L_READ_SHARE_RW, raw_lt_nop, false, false},
+ {RAW_L_READ_SHARE_RW, RAW_L_READ, raw_lt_from_unlock, true},
+ {RAW_L_READ_SHARE_RW, RAW_L_WRITE_SHARE_RW, raw_lt_from_unlock, true},
+ {RAW_L_READ_SHARE_RW, RAW_L_WRITE, raw_lt_from_unlock, true},
+
+ {RAW_L_READ, RAW_L_READ_SHARE_RW, raw_lt_nop, false, true},
+ {RAW_L_READ, RAW_L_READ, raw_lt_nop, false, false},
+ {RAW_L_READ, RAW_L_WRITE_SHARE_RW, raw_lt_read_to_write_share, true},
+ {RAW_L_READ, RAW_L_WRITE, raw_lt_read_to_write, true},
+
+ {RAW_L_WRITE_SHARE_RW, RAW_L_READ_SHARE_RW, raw_lt_nop, false, true},
+ {RAW_L_WRITE_SHARE_RW, RAW_L_READ, raw_lt_write_share_to_read, true},
+ {RAW_L_WRITE_SHARE_RW, RAW_L_WRITE_SHARE_RW, raw_lt_nop, false, false},
+ {RAW_L_WRITE_SHARE_RW, RAW_L_WRITE, raw_lt_write_share_to_write, true},
+
+ {RAW_L_WRITE, RAW_L_READ_SHARE_RW, raw_lt_nop, false, true},
+ {RAW_L_WRITE, RAW_L_READ, raw_lt_write_to_read, true},
+ {RAW_L_WRITE, RAW_L_WRITE_SHARE_RW, raw_lt_write_to_write_share, true},
+ {RAW_L_WRITE, RAW_L_WRITE, raw_lt_nop, false, false},
+};
+
+static int raw_handle_lock_update(BlockDriverState *bs,
+ RawLockTransOp op,
+ Error **errp)
+{
+ BDRVRawState *s = bs->opaque;
+ BDRVRawLockMode old_lock, new_lock;
+ const struct RawLockTransOp *rec;
+ int ret = 0;
+ Error *local_err = NULL;
+ BDRVRawLockUpdateState *lu = s->lock_update;
+ int lock_fd = -1;
+
+ if (!RAW_LOCK_SUPPORTED) {
+ goto cleanup;
+ }
+
+ if (bdrv_get_flags(bs) & BDRV_O_INACTIVE) {
+ /* leave the work to bdrv_invalidate_cache. */
+ return 0;
+ }
+
+ if (op == RAW_LT_PREPARE) {
+ lock_fd = qemu_open(bs->filename, lu->open_flags);
+ if (lock_fd == -1) {
+ if (errno == ENOENT) {
+ /* The file is gone, probably BDRV_O_SNAPSHOT? Skip locking. */
+ lu->use_lock = false;
+ } else {
+ /* other errors handled later. */
+ }
+ }
+ }
+
+ old_lock = s->cur_lock_mode;
+ new_lock = lu->use_lock ? lu->new_lock : RAW_L_READ_SHARE_RW;
+ for (rec = &raw_lock_trans_ops[0];
+ rec < &raw_lock_trans_ops[ARRAY_SIZE(raw_lock_trans_ops)];
+ rec++) {
+ if (rec->old_lock == old_lock && rec->new_lock == new_lock) {
+ break;
+ }
+ }
+ assert(rec != &raw_lock_trans_ops[ARRAY_SIZE(raw_lock_trans_ops)]);
+
+ assert(old_lock == RAW_L_READ_SHARE_RW || s->lock_fd >= 0);
+
+ DPRINTF("handle lock %p old lock %d new lock %d op %d func %p\n", bs,
+ old_lock, new_lock, op, rec->func);
+ switch (op) {
+ case RAW_LT_PREPARE:
+ if (rec->need_lock_fd) {
+ if (lock_fd >= 0) {
+ lu->lock_fd = lock_fd;
+ } else {
+ error_setg(errp, "Failed to initialize lock fd");
+ }
+ } else {
+ if (lock_fd >= 0) {
+ qemu_close(lock_fd);
+ lock_fd = -1;
+ }
+ }
+ ret = rec->func(op, s->lock_fd, lu->lock_fd, old_lock, new_lock, errp);
+ if (!ret) {
+ break;
+ }
+ /* Only succeeded preparation will be reverted by block layer, we
+ * need to clean up this failure manually. */
+ op = RAW_LT_ABORT;
+ /* fall through */
+ case RAW_LT_ABORT:
+ rec->func(op, s->lock_fd, lu->lock_fd, old_lock, new_lock, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ }
+ if (lu->lock_fd >= 0) {
+ qemu_close(lu->lock_fd);
+ lu->lock_fd = -1;
+ }
+ goto cleanup;
+ case RAW_LT_COMMIT:
+ rec->func(op, s->lock_fd, lu->lock_fd, old_lock, new_lock, &error_abort);
+ if ((rec->need_lock_fd || rec->close_old_lock_fd) && s->lock_fd >= 0) {
+ raw_lock_fd(s->lock_fd, RAW_L_READ_SHARE_RW, NULL);
+ qemu_close(s->lock_fd);
+ s->lock_fd = -1;
+ }
+ if (rec->need_lock_fd) {
+ s->lock_fd = lu->lock_fd;
+ }
+ assert(s->lock_fd >= 0 || new_lock == RAW_L_READ_SHARE_RW);
+ s->cur_lock_mode = new_lock;
+ s->use_lock = lu->use_lock;
+ goto cleanup;
+ }
+ return ret;
+cleanup:
+ g_free(s->lock_update);
+ s->lock_update = NULL;
+ return ret;
+}
+
+static void raw_init_lock_update(BlockDriverState *bs,
+ int image_fd,
+ bool write, bool shared,
+ bool use_lock)
+{
+ BDRVRawState *s = bs->opaque;
+
+ assert(!s->lock_update);
+ s->lock_update = g_new0(BDRVRawLockUpdateState, 1);
+ *s->lock_update = (BDRVRawLockUpdateState) {
+ .image_fd = image_fd,
+ .new_lock = raw_get_lock_mode(write, shared),
+ .use_lock = use_lock,
+ .open_flags = (s->open_flags & ~(O_RDWR | O_RDONLY)) |
+ (write ? O_RDWR : O_RDONLY),
+ };
+}
+
static int raw_reopen_prepare(BDRVReopenState *state,
BlockReopenQueue *queue, Error **errp)
{
@@ -549,6 +1176,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
BDRVRawReopenState *rs;
int ret = 0;
Error *local_err = NULL;
+ bool shared;
assert(state != NULL);
assert(state->bs != NULL);
@@ -613,13 +1241,27 @@ static int raw_reopen_prepare(BDRVReopenState *state,
if (rs->fd != -1) {
raw_probe_alignment(state->bs, rs->fd, &local_err);
if (local_err) {
- qemu_close(rs->fd);
- rs->fd = -1;
error_propagate(errp, local_err);
ret = -EINVAL;
+ goto fail;
}
}
+ shared = s->cur_lock_mode == RAW_L_READ_SHARE_RW ||
+ s->cur_lock_mode == RAW_L_WRITE_SHARE_RW;
+ /* Shared perm doesn't change during reopen. */
+ raw_init_lock_update(state->bs, rs->fd, state->flags & BDRV_O_RDWR, shared,
+ s->use_lock);
+ qdict_del(state->options, "locking");
+ ret = raw_handle_lock_update(state->bs, RAW_LT_PREPARE, errp);
+ if (ret) {
+ goto fail;
+ }
+
+ return 0;
+fail:
+ qemu_close(rs->fd);
+ rs->fd = -1;
return ret;
}
@@ -630,6 +1272,8 @@ static void raw_reopen_commit(BDRVReopenState *state)
s->open_flags = rs->open_flags;
+ raw_handle_lock_update(state->bs, RAW_LT_COMMIT, &error_abort);
+
qemu_close(s->fd);
s->fd = rs->fd;
@@ -647,6 +1291,8 @@ static void raw_reopen_abort(BDRVReopenState *state)
return;
}
+ raw_handle_lock_update(state->bs, RAW_LT_ABORT, &error_abort);
+
if (rs->fd >= 0) {
qemu_close(rs->fd);
rs->fd = -1;
@@ -1410,6 +2056,10 @@ static void raw_close(BlockDriverState *bs)
qemu_close(s->fd);
s->fd = -1;
}
+ if (s->lock_fd >= 0) {
+ qemu_close(s->lock_fd);
+ s->lock_fd = -1;
+ }
}
static int raw_truncate(BlockDriverState *bs, int64_t offset)
@@ -1947,6 +2597,85 @@ static QemuOptsList raw_create_opts = {
}
};
+static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared,
+ Error **errp)
+{
+ bool is_shared;
+ BDRVRawState *s = bs->opaque;
+
+ if (!RAW_LOCK_SUPPORTED) {
+ return 0;
+ }
+ if (s->lock_update) {
+ /* Override the previously stashed update. */
+ g_free(s->lock_update);
+ s->lock_update = NULL;
+ }
+ is_shared = shared & BLK_PERM_WRITE;
+ DPRINTF("raw check perm %p rw %d shared %d\n",
+ bs, perm & BLK_PERM_WRITE ? 1 : 0,
+ is_shared);
+ raw_init_lock_update(bs, s->fd,
+ perm & BLK_PERM_WRITE, is_shared, s->use_lock);
+
+ return raw_handle_lock_update(bs, RAW_LT_PREPARE, errp);
+}
+
+static void raw_set_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared)
+{
+ BDRVRawState *s = bs->opaque;
+
+ if (!RAW_LOCK_SUPPORTED) {
+ return;
+ }
+ assert(s->lock_update);
+
+ raw_handle_lock_update(bs, RAW_LT_COMMIT, NULL);
+}
+
+static void raw_abort_perm_update(BlockDriverState *bs)
+{
+ BDRVRawState *s = bs->opaque;
+
+ if (!RAW_LOCK_SUPPORTED) {
+ return;
+ }
+ if (!s->lock_update) {
+ return;
+ }
+ raw_handle_lock_update(bs, RAW_LT_ABORT, NULL);
+}
+
+static int raw_inactivate(BlockDriverState *bs)
+{
+ BDRVRawState *s = bs->opaque;
+ int r = 0;
+
+ if (RAW_LOCK_SUPPORTED && s->cur_lock_mode != RAW_L_READ_SHARE_RW) {
+ r = raw_lock_fd(s->lock_fd, RAW_L_READ_SHARE_RW, NULL);
+ }
+ return r;
+}
+
+static void raw_invalidate_cache(BlockDriverState *bs, Error **errp)
+{
+ int r;
+ BDRVRawState *s = bs->opaque;
+
+ if (!RAW_LOCK_SUPPORTED) {
+ return;
+ }
+ if (s->lock_update) {
+ /* Apply the pending lock update from perm or reopen. */
+ r = raw_handle_lock_update(bs, RAW_LT_PREPARE, errp);
+ if (r) {
+ return;
+ }
+ raw_handle_lock_update(bs, RAW_LT_COMMIT, errp);
+ assert(!s->lock_update);
+ }
+}
+
BlockDriver bdrv_file = {
.format_name = "file",
.protocol_name = "file",
@@ -1977,7 +2706,11 @@ BlockDriver bdrv_file = {
.bdrv_get_info = raw_get_info,
.bdrv_get_allocated_file_size
= raw_get_allocated_file_size,
-
+ .bdrv_inactivate = raw_inactivate,
+ .bdrv_invalidate_cache = raw_invalidate_cache,
+ .bdrv_check_perm = raw_check_perm,
+ .bdrv_set_perm = raw_set_perm,
+ .bdrv_abort_perm_update = raw_abort_perm_update,
.create_opts = &raw_create_opts,
};
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v14 19/20] file-posix: Add image locking in perm operations
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 19/20] file-posix: Add image locking in perm operations Fam Zheng
@ 2017-04-24 13:39 ` Kevin Wolf
0 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2017-04-24 13:39 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, eblake, Max Reitz, qemu-block
Am 21.04.2017 um 05:56 hat Fam Zheng geschrieben:
> virtlockd in libvirt locks the first byte, so we start looking at the
> file bytes from 0x10.
>
> The complication is in the transactional interface. To make the reopen
> logic managable, and allow better reuse, the code is internally
> organized with a table from old mode to the new one.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block/file-posix.c | 739 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 736 insertions(+), 3 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 2114720..c307493 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -129,8 +129,54 @@ do { \
>
> #define MAX_BLOCKSIZE 4096
>
> +/* Posix file locking bytes. Libvirt takes byte 0, we start from byte 0x10,
> + * leaving a few more bytes for its future use. */
> +#define RAW_LOCK_BYTE_MIN 0x10
> +#define RAW_LOCK_BYTE_NO_OTHER_WRITER 0x10
> +#define RAW_LOCK_BYTE_WRITE 0x11
> +#ifdef F_OFD_SETLK
> +#define RAW_LOCK_SUPPORTED 1
> +#else
> +#define RAW_LOCK_SUPPORTED 0
> +#endif
> +
> +/*
> + ** reader that can tolerate writers: Don't do anything
> + *
> + ** reader that can't tolerate writers: Take shared lock on byte 0x10. Test
> + * byte 0x11 is unlocked.
> + *
> + ** shared writer: Take shared lock on byte 0x11. Test byte 0x10 is unlocked.
> + *
> + ** exclusive writer: Take exclusive locks on both bytes.
> + */
There are few things that made this patch basically degenerate into a
ton of special cases, the most important of them being that we thought
that we'd need acquire exclusive locks to avoid races and that read-only
file descriptors can't take an exclusive lock and therefore can't test
whether anyone else is holding a shared lock on the file descriptor.
After looking a bit more into this and discussing it on IRC with Fam who
found the last missing puzzle piece, it turned out that while it's true
that F_OFD_SETLK for an exclusive lock doesn't work for read-only FDs,
in fact F_OFD_GETLK (which tests if a lock could be set) does work, so
we can use it to check whether anyone else is holding a shared lock.
We also never need to actually acquire any exclusive locks, just
acquiring shared locks everywhere and testing whether we could set
exclusive locks is enough for our needs. Both of these operations work
fine with read-only FDs.
With these observations, it should be easy to even map _all_ 64 bits of
perm and ~shared to file locks. I think the algorithm looks roughly like
this:
1. Acquire shared @perm locks for (old_perm | new_perm)
2. Acquire shared @~shared locks for (~old_shared | ~new_shared)
3. Test that nobody else holds @perm locks for ~new_shared
4. Test that nobody else holds @~shared locks for new_perm
5. Drop @perm locks for (old_perm & ~new_perm)
6. Drop @~shared locks for (~old_shared & new_shared)
Having one universal algorithm for all state transitions allows us to
greatly simplify the code in this patch.
Kevin
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v14 20/20] qemu-iotests: Add test case 153 for image locking
2017-04-21 3:55 [Qemu-devel] [PATCH v14 00/20] block: Image locking series Fam Zheng
` (18 preceding siblings ...)
2017-04-21 3:56 ` [Qemu-devel] [PATCH v14 19/20] file-posix: Add image locking in perm operations Fam Zheng
@ 2017-04-21 3:56 ` Fam Zheng
19 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2017-04-21 3:56 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/qemu-iotests/153 | 219 ++++++++++++++++
tests/qemu-iotests/153.out | 627 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 847 insertions(+)
create mode 100755 tests/qemu-iotests/153
create mode 100644 tests/qemu-iotests/153.out
diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
new file mode 100755
index 0000000..ff22615
--- /dev/null
+++ b/tests/qemu-iotests/153
@@ -0,0 +1,219 @@
+#!/bin/bash
+#
+# Test image locking
+#
+# Copyright 2016, 2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=famz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+ rm -f "${TEST_IMG}.base"
+ rm -f "${TEST_IMG}.convert"
+ rm -f "${TEST_IMG}.a"
+ rm -f "${TEST_IMG}.b"
+ rm -f "${TEST_IMG}.lnk"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+size=32M
+
+_run_cmd()
+{
+ echo
+ (echo "$@"; "$@" 2>&1 1>/dev/null) | _filter_testdir
+}
+
+function _do_run_qemu()
+{
+ (
+ if ! test -t 0; then
+ while read cmd; do
+ echo $cmd
+ done
+ fi
+ echo quit
+ ) | $QEMU -nographic -monitor stdio -serial none "$@" 1>/dev/null
+}
+
+function _run_qemu_with_images()
+{
+ _do_run_qemu \
+ $(for i in $@; do echo "-drive if=none,file=$i"; done) 2>&1 \
+ | _filter_testdir | _filter_qemu
+}
+
+test_opts="$(echo readonly={on,off},force-shared-write={on,off})"
+
+for opts1 in "" $test_opts; do
+ echo
+ echo "== Creating base image =="
+ TEST_IMG="${TEST_IMG}.base" _make_test_img $size
+
+ echo
+ echo "== Creating test image =="
+ $QEMU_IMG create -f $IMGFMT "${TEST_IMG}" -b ${TEST_IMG}.base | _filter_img_create
+
+ echo
+ echo "== Launching QEMU, opts: '$opts1' =="
+ _launch_qemu -drive file="${TEST_IMG}",if=none,$opts1
+ h=$QEMU_HANDLE
+
+ for opts2 in "" $test_opts; do
+ echo
+ echo "== Launching another QEMU, opts: '$opts2' =="
+ echo "quit" | \
+ $QEMU -nographic -monitor stdio \
+ -drive file="${TEST_IMG}",if=none,$opts2 2>&1 1>/dev/null | \
+ _filter_testdir | _filter_qemu
+ done
+
+ for L in "" "-U"; do
+
+ echo
+ echo "== Running utility commands $L =="
+ _run_cmd $QEMU_IO $L -c "read 0 512" "${TEST_IMG}"
+ _run_cmd $QEMU_IO $L -r -c "read 0 512" "${TEST_IMG}"
+ _run_cmd $QEMU_IO -c "open $L ${TEST_IMG}" -c "read 0 512"
+ _run_cmd $QEMU_IO -c "open -r $L ${TEST_IMG}" -c "read 0 512"
+ _run_cmd $QEMU_IMG info $L "${TEST_IMG}"
+ _run_cmd $QEMU_IMG check $L "${TEST_IMG}"
+ _run_cmd $QEMU_IMG compare $L "${TEST_IMG}" "${TEST_IMG}"
+ _run_cmd $QEMU_IMG map $L "${TEST_IMG}"
+ _run_cmd $QEMU_IMG amend -o "" $L "${TEST_IMG}"
+ _run_cmd $QEMU_IMG commit $L "${TEST_IMG}"
+ _run_cmd $QEMU_IMG resize $L "${TEST_IMG}" $size
+ _run_cmd $QEMU_IMG rebase $L "${TEST_IMG}" -b "${TEST_IMG}.base"
+ _run_cmd $QEMU_IMG snapshot -l $L "${TEST_IMG}"
+ _run_cmd $QEMU_IMG convert $L "${TEST_IMG}" "${TEST_IMG}.convert"
+ _run_cmd $QEMU_IMG dd $L if="${TEST_IMG}" of="${TEST_IMG}.convert" bs=512 count=1
+ _run_cmd $QEMU_IMG bench $L -c 1 "${TEST_IMG}"
+ _run_cmd $QEMU_IMG bench $L -w -c 1 "${TEST_IMG}"
+ done
+ _send_qemu_cmd $h "{ 'execute': 'quit', }" ""
+ echo
+ echo "Round done"
+ _cleanup_qemu
+done
+
+for opt1 in $test_opts; do
+ for opt2 in $test_opts; do
+ echo
+ echo "== Two devices with the same image ($opt1 - $opt2) =="
+ _run_qemu_with_images "${TEST_IMG},$opt1" "${TEST_IMG},$opt2"
+ done
+done
+
+echo "== Creating ${TEST_IMG}.[abc] =="
+(
+ $QEMU_IMG create -f qcow2 "${TEST_IMG}.a" -b "${TEST_IMG}"
+ $QEMU_IMG create -f qcow2 "${TEST_IMG}.b" -b "${TEST_IMG}"
+ $QEMU_IMG create -f qcow2 "${TEST_IMG}.c" -b "${TEST_IMG}.b"
+) | _filter_img_create
+
+echo
+echo "== Two devices sharing the same file in backing chain =="
+_run_qemu_with_images "${TEST_IMG}.a" "${TEST_IMG}.b"
+_run_qemu_with_images "${TEST_IMG}.a" "${TEST_IMG}.c"
+
+echo
+echo "== Backing image also as an active device =="
+_run_qemu_with_images "${TEST_IMG}.a" "${TEST_IMG}"
+
+echo
+echo "== Backing image also as an active device (ro) =="
+_run_qemu_with_images "${TEST_IMG}.a" "${TEST_IMG},readonly=on"
+
+echo
+echo "== Symbolic link =="
+rm -f "${TEST_IMG}.lnk" &>/dev/null
+ln -s ${TEST_IMG} "${TEST_IMG}.lnk" || echo "Failed to create link"
+_run_qemu_with_images "${TEST_IMG}.lnk" "${TEST_IMG}"
+
+echo
+echo "== Closing an image should unlock it =="
+_launch_qemu
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'qmp_capabilities' }" \
+ 'return'
+
+echo "Adding drive"
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line': 'drive_add 0 if=none,id=d0,file=${TEST_IMG}' } }" \
+ ""
+
+_run_cmd $QEMU_IO "${TEST_IMG}" -c 'write 0 512'
+
+echo "Closing drive"
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line': 'drive_del d0' } }" \
+ ""
+
+_run_cmd $QEMU_IO "${TEST_IMG}" -c 'write 0 512'
+
+echo "Adding two and closing one"
+for d in d0 d1; do
+ _send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line': 'drive_add 0 if=none,id=$d,file=${TEST_IMG},readonly=on' } }" \
+ ""
+done
+
+_run_cmd $QEMU_IMG info "${TEST_IMG}"
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line': 'drive_del d0' } }" \
+ ""
+
+_run_cmd $QEMU_IO "${TEST_IMG}" -c 'write 0 512'
+
+echo "Closing the other"
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line': 'drive_del d1' } }" \
+ ""
+
+_run_cmd $QEMU_IO "${TEST_IMG}" -c 'write 0 512'
+
+_cleanup_qemu
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out
new file mode 100644
index 0000000..786ff0b
--- /dev/null
+++ b/tests/qemu-iotests/153.out
@@ -0,0 +1,627 @@
+QA output created by 153
+
+== Creating base image ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
+
+== Creating test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base
+
+== Launching QEMU, opts: '' ==
+
+== Launching another QEMU, opts: '' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,: Failed to lock image: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=on' ==
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=off' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=on,force-shared-write=off: Failed to lock share byte: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=on' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=off,force-shared-write=on: Failed to lock write byte: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=off' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=off,force-shared-write=off: Failed to lock image: Resource temporarily unavailable
+
+== Running utility commands ==
+
+_qemu_io_wrapper -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+_qemu_io_wrapper -r -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open -r TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper check TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper map TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper amend -o TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper commit TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper resize TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper rebase TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper snapshot -l TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper convert TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper dd if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper bench -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper bench -w -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+== Running utility commands -U ==
+
+_qemu_io_wrapper -U -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_io_wrapper -U -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -c open -U TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open -r -U TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_img_wrapper info -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o -U TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_img_wrapper commit -U TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_img_wrapper snapshot -l -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+_qemu_img_wrapper dd -U if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+
+_qemu_img_wrapper bench -U -c 1 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper bench -U -w -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+Round done
+
+== Creating base image ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
+
+== Creating test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base
+
+== Launching QEMU, opts: 'readonly=on,force-shared-write=on' ==
+
+== Launching another QEMU, opts: '' ==
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=on' ==
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=off' ==
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=on' ==
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=off' ==
+
+== Running utility commands ==
+
+_qemu_io_wrapper -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -c open TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_io_wrapper -c open -r TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o TEST_DIR/t.qcow2
+
+_qemu_img_wrapper commit TEST_DIR/t.qcow2
+
+_qemu_img_wrapper resize TEST_DIR/t.qcow2 32M
+
+_qemu_img_wrapper rebase TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+
+_qemu_img_wrapper snapshot -l TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+_qemu_img_wrapper dd if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+
+_qemu_img_wrapper bench -c 1 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper bench -w -c 1 TEST_DIR/t.qcow2
+
+== Running utility commands -U ==
+
+_qemu_io_wrapper -U -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -U -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -c open -U TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_io_wrapper -c open -r -U TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_img_wrapper info -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper commit -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M
+
+_qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+
+_qemu_img_wrapper snapshot -l -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+_qemu_img_wrapper dd -U if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+
+_qemu_img_wrapper bench -U -c 1 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper bench -U -w -c 1 TEST_DIR/t.qcow2
+
+Round done
+
+== Creating base image ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
+
+== Creating test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base
+
+== Launching QEMU, opts: 'readonly=on,force-shared-write=off' ==
+
+== Launching another QEMU, opts: '' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,: Failed to lock image: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=on' ==
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=off' ==
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=on' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=off,force-shared-write=on: Share byte lock is taken: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=off' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=off,force-shared-write=off: Failed to lock image: Resource temporarily unavailable
+
+== Running utility commands ==
+
+_qemu_io_wrapper -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+_qemu_io_wrapper -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -c open TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open -r TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper commit TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper resize TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper rebase TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper snapshot -l TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+_qemu_img_wrapper dd if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+
+_qemu_img_wrapper bench -c 1 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper bench -w -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+== Running utility commands -U ==
+
+_qemu_io_wrapper -U -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Share byte lock is taken: Resource temporarily unavailable
+
+_qemu_io_wrapper -U -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -c open -U TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Share byte lock is taken: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open -r -U TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_img_wrapper info -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o -U TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Share byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper commit -U TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Share byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Share byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Share byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper snapshot -l -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+_qemu_img_wrapper dd -U if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+
+_qemu_img_wrapper bench -U -c 1 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper bench -U -w -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Share byte lock is taken: Resource temporarily unavailable
+
+Round done
+
+== Creating base image ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
+
+== Creating test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base
+
+== Launching QEMU, opts: 'readonly=off,force-shared-write=on' ==
+
+== Launching another QEMU, opts: '' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,: Failed to lock image: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=on' ==
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=off' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=on,force-shared-write=off: Write byte lock is taken: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=on' ==
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=off' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=off,force-shared-write=off: Failed to lock image: Resource temporarily unavailable
+
+== Running utility commands ==
+
+_qemu_io_wrapper -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+_qemu_io_wrapper -r -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open -r TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper check TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper map TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper amend -o TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper commit TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper resize TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper rebase TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper snapshot -l TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper convert TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper dd if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper bench -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Write byte lock is taken: Resource temporarily unavailable
+
+_qemu_img_wrapper bench -w -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+== Running utility commands -U ==
+
+_qemu_io_wrapper -U -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -U -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -c open -U TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_io_wrapper -c open -r -U TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_img_wrapper info -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper commit -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M
+
+_qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+
+_qemu_img_wrapper snapshot -l -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+_qemu_img_wrapper dd -U if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+
+_qemu_img_wrapper bench -U -c 1 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper bench -U -w -c 1 TEST_DIR/t.qcow2
+
+Round done
+
+== Creating base image ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
+
+== Creating test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base
+
+== Launching QEMU, opts: 'readonly=off,force-shared-write=off' ==
+
+== Launching another QEMU, opts: '' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,: Failed to lock image: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=on' ==
+
+== Launching another QEMU, opts: 'readonly=on,force-shared-write=off' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=on,force-shared-write=off: Failed to lock share byte: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=on' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=off,force-shared-write=on: Failed to lock write byte: Resource temporarily unavailable
+
+== Launching another QEMU, opts: 'readonly=off,force-shared-write=off' ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,readonly=off,force-shared-write=off: Failed to lock image: Resource temporarily unavailable
+
+== Running utility commands ==
+
+_qemu_io_wrapper -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+_qemu_io_wrapper -r -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open -r TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper check TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper map TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper amend -o TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper commit TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper resize TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper rebase TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+_qemu_img_wrapper snapshot -l TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper convert TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper dd if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper bench -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock share byte: Resource temporarily unavailable
+
+_qemu_img_wrapper bench -w -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image: Resource temporarily unavailable
+
+== Running utility commands -U ==
+
+_qemu_io_wrapper -U -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_io_wrapper -U -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -c open -U TEST_DIR/t.qcow2 -c read 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_io_wrapper -c open -r -U TEST_DIR/t.qcow2 -c read 0 512
+
+_qemu_img_wrapper info -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o -U TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_img_wrapper commit -U TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+_qemu_img_wrapper snapshot -l -U TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+_qemu_img_wrapper dd -U if=TEST_DIR/t.qcow2 of=TEST_DIR/t.qcow2.convert bs=512 count=1
+
+_qemu_img_wrapper bench -U -c 1 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper bench -U -w -c 1 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock write byte: Resource temporarily unavailable
+
+Round done
+
+== Two devices with the same image (readonly=on,force-shared-write=on - readonly=on,force-shared-write=on) ==
+
+== Two devices with the same image (readonly=on,force-shared-write=on - readonly=on,force-shared-write=off) ==
+
+== Two devices with the same image (readonly=on,force-shared-write=on - readonly=off,force-shared-write=on) ==
+
+== Two devices with the same image (readonly=on,force-shared-write=on - readonly=off,force-shared-write=off) ==
+
+== Two devices with the same image (readonly=on,force-shared-write=off - readonly=on,force-shared-write=on) ==
+
+== Two devices with the same image (readonly=on,force-shared-write=off - readonly=on,force-shared-write=off) ==
+
+== Two devices with the same image (readonly=on,force-shared-write=off - readonly=off,force-shared-write=on) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,readonly=off,force-shared-write=on: Share byte lock is taken: Resource temporarily unavailable
+
+== Two devices with the same image (readonly=on,force-shared-write=off - readonly=off,force-shared-write=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,readonly=off,force-shared-write=off: Failed to lock image: Resource temporarily unavailable
+
+== Two devices with the same image (readonly=off,force-shared-write=on - readonly=on,force-shared-write=on) ==
+
+== Two devices with the same image (readonly=off,force-shared-write=on - readonly=on,force-shared-write=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,readonly=on,force-shared-write=off: Write byte lock is taken: Resource temporarily unavailable
+
+== Two devices with the same image (readonly=off,force-shared-write=on - readonly=off,force-shared-write=on) ==
+
+== Two devices with the same image (readonly=off,force-shared-write=on - readonly=off,force-shared-write=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,readonly=off,force-shared-write=off: Failed to lock image: Resource temporarily unavailable
+
+== Two devices with the same image (readonly=off,force-shared-write=off - readonly=on,force-shared-write=on) ==
+
+== Two devices with the same image (readonly=off,force-shared-write=off - readonly=on,force-shared-write=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,readonly=on,force-shared-write=off: Failed to lock share byte: Resource temporarily unavailable
+
+== Two devices with the same image (readonly=off,force-shared-write=off - readonly=off,force-shared-write=on) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,readonly=off,force-shared-write=on: Failed to lock write byte: Resource temporarily unavailable
+
+== Two devices with the same image (readonly=off,force-shared-write=off - readonly=off,force-shared-write=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,readonly=off,force-shared-write=off: Failed to lock image: Resource temporarily unavailable
+== Creating /var/tmp/q/build/tests/qemu-iotests/scratch/t.qcow2.[abc] ==
+Formatting 'TEST_DIR/t.IMGFMT.a', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT
+Formatting 'TEST_DIR/t.IMGFMT.b', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT
+Formatting 'TEST_DIR/t.IMGFMT.c', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.b
+
+== Two devices sharing the same file in backing chain ==
+
+== Backing image also as an active device ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+== Backing image also as an active device (ro) ==
+
+== Symbolic link ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+
+== Closing an image should unlock it ==
+{"return": {}}
+Adding drive
+
+_qemu_io_wrapper TEST_DIR/t.qcow2 -c write 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+Closing drive
+
+_qemu_io_wrapper TEST_DIR/t.qcow2 -c write 0 512
+Adding two and closing one
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+
+_qemu_io_wrapper TEST_DIR/t.qcow2 -c write 0 512
+can't open device TEST_DIR/t.qcow2: Failed to lock image: Resource temporarily unavailable
+Closing the other
+
+_qemu_io_wrapper TEST_DIR/t.qcow2 -c write 0 512
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 893962d..c8ae60e 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -154,6 +154,7 @@
149 rw auto sudo
150 rw auto quick
152 rw auto quick
+153 rw auto quick
154 rw auto backing quick
155 rw auto
156 rw auto quick
--
2.9.3
^ permalink raw reply related [flat|nested] 36+ messages in thread