* [Qemu-devel] [PATCH v2 01/24] block/mirror: Small absolute-paths simplification
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
@ 2016-11-27 1:55 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 02/24] block: Use children list in bdrv_refresh_filename Max Reitz
` (22 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:55 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
When invoking drive-mirror in absolute-paths mode, the target's backing
BDS is assigned to it in mirror_complete(). The current logic only does
so if the target does not have that backing BDS already; but it actually
cannot have a backing BDS at all (the BDS is opened with O_NO_BACKING in
qmp_drive_mirror()), so just assert that and assign the new backing BDS
unconditionally.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/mirror.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 301ba92..4ece624 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -888,9 +888,9 @@ static void mirror_complete(BlockJob *job, Error **errp)
if (s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
BlockDriverState *backing = s->is_none_mode ? src : s->base;
- if (backing_bs(target) != backing) {
- bdrv_set_backing_hd(target, backing);
- }
+
+ assert(!target->backing);
+ bdrv_set_backing_hd(target, backing);
}
s->should_complete = true;
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 02/24] block: Use children list in bdrv_refresh_filename
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
2016-11-27 1:55 ` [Qemu-devel] [PATCH v2 01/24] block/mirror: Small absolute-paths simplification Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 03/24] block: Add BDS.backing_overridden Max Reitz
` (21 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
bdrv_refresh_filename() should invoke itself recursively on all
children, not just on file.
With that change, we can remove the manual invocations in blkverify and
quorum.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block.c | 9 +++++----
block/blkverify.c | 3 ---
block/quorum.c | 1 -
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/block.c b/block.c
index 39ddea3..1615f5d 100644
--- a/block.c
+++ b/block.c
@@ -3948,16 +3948,17 @@ static bool append_open_options(QDict *d, BlockDriverState *bs)
void bdrv_refresh_filename(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
+ BdrvChild *child;
QDict *opts;
if (!drv) {
return;
}
- /* This BDS's file name will most probably depend on its file's name, so
- * refresh that first */
- if (bs->file) {
- bdrv_refresh_filename(bs->file->bs);
+ /* This BDS's file name may depend on any of its children's file names, so
+ * refresh those first */
+ QLIST_FOREACH(child, &bs->children, next) {
+ bdrv_refresh_filename(child->bs);
}
if (drv->bdrv_refresh_filename) {
diff --git a/block/blkverify.c b/block/blkverify.c
index 28f9af6..e0f9275 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -293,9 +293,6 @@ static void blkverify_refresh_filename(BlockDriverState *bs, QDict *options)
{
BDRVBlkverifyState *s = bs->opaque;
- /* bs->file->bs has already been refreshed */
- bdrv_refresh_filename(s->test_file->bs);
-
if (bs->file->bs->full_open_options
&& s->test_file->bs->full_open_options)
{
diff --git a/block/quorum.c b/block/quorum.c
index d122299..45e61bb 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1058,7 +1058,6 @@ static void quorum_refresh_filename(BlockDriverState *bs, QDict *options)
int i;
for (i = 0; i < s->num_children; i++) {
- bdrv_refresh_filename(s->children[i]->bs);
if (!s->children[i]->bs->full_open_options) {
return;
}
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 03/24] block: Add BDS.backing_overridden
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
2016-11-27 1:55 ` [Qemu-devel] [PATCH v2 01/24] block/mirror: Small absolute-paths simplification Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 02/24] block: Use children list in bdrv_refresh_filename Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 2:33 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 04/24] block: Respect backing bs in bdrv_refresh_filename Max Reitz
` (20 subsequent siblings)
23 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
If the backing file is overridden, this most probably does change the
guest-visible data of a BDS. Therefore, we will need to consider this in
bdrv_refresh_filename().
Adding a new field to the BDS is not nice, but it is very simple and
exactly keeps track of whether the backing file has been overridden.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
While this patch adds a TODO comment that sounds like it could be
resolved in a later patch in this series, it is not.
Checking whether @options contains significant runtime options is not
trivial, because we not only care about significant options at that
level but also at levels below: Normally, it should be fine to check
that for backing_hd and backing_hd->file->bs, but that's not for sure.
So I decided a false positive is better than a false negative here,
which is why the TODO persists throughout the series.
---
block.c | 14 ++++++++++++++
block/mirror.c | 4 ++++
blockdev.c | 16 ++++++++++++++++
include/block/block_int.h | 1 +
4 files changed, 35 insertions(+)
diff --git a/block.c b/block.c
index 1615f5d..c6c62b2 100644
--- a/block.c
+++ b/block.c
@@ -1516,6 +1516,12 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
goto free_exit;
}
+ if (reference || qdict_size(options)) {
+ /* TODO: Should only be set to true if @options contains actually
+ * significant runtime options */
+ bs->backing_overridden = true;
+ }
+
if (bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) {
qdict_put(options, "driver", qstring_from_str(bs->backing_format));
}
@@ -1659,6 +1665,9 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs,
bdrv_ref(bs_snapshot);
bdrv_append(bs_snapshot, bs);
+ bs_snapshot->backing_overridden = true;
+ bdrv_refresh_filename(bs_snapshot);
+
g_free(tmp_filename);
return bs_snapshot;
@@ -1786,6 +1795,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
backing = qdict_get_try_str(options, "backing");
if (backing && *backing == '\0') {
flags |= BDRV_O_NO_BACKING;
+ bs->backing_overridden = true;
qdict_del(options, "backing");
}
@@ -3959,6 +3969,10 @@ void bdrv_refresh_filename(BlockDriverState *bs)
* refresh those first */
QLIST_FOREACH(child, &bs->children, next) {
bdrv_refresh_filename(child->bs);
+
+ if (child->role == &child_backing && child->bs->backing_overridden) {
+ bs->backing_overridden = true;
+ }
}
if (drv->bdrv_refresh_filename) {
diff --git a/block/mirror.c b/block/mirror.c
index 4ece624..6cb6cbd 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -891,6 +891,10 @@ static void mirror_complete(BlockJob *job, Error **errp)
assert(!target->backing);
bdrv_set_backing_hd(target, backing);
+
+ /* The target image's file already has been created with the backing
+ * file we just set, so there is no need to set backing_overridden or
+ * call bdrv_refresh_filename(). */
}
s->should_complete = true;
diff --git a/blockdev.c b/blockdev.c
index 245e1e1..7889bab 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1771,6 +1771,8 @@ static void external_snapshot_commit(BlkActionState *common)
{
ExternalSnapshotState *state =
DO_UPCAST(ExternalSnapshotState, common, common);
+ TransactionAction *action = common->action;
+ bool image_was_existing = false;
bdrv_set_aio_context(state->new_bs, state->aio_context);
@@ -1783,6 +1785,20 @@ static void external_snapshot_commit(BlkActionState *common)
bdrv_reopen(state->old_bs, state->old_bs->open_flags & ~BDRV_O_RDWR,
NULL);
}
+
+ if (action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) {
+ BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync.data;
+ if (s->has_mode && s->mode == NEW_IMAGE_MODE_EXISTING) {
+ image_was_existing = true;
+ }
+ } else {
+ image_was_existing = true;
+ }
+
+ if (image_was_existing) {
+ state->new_bs->backing_overridden = true;
+ bdrv_refresh_filename(state->new_bs);
+ }
}
static void external_snapshot_abort(BlkActionState *common)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 83a423c..555f3c2 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -461,6 +461,7 @@ struct BlockDriverState {
char backing_file[PATH_MAX]; /* if non zero, the image is a diff of
this file image */
char backing_format[16]; /* if non-zero and backing_file exists */
+ bool backing_overridden; /* backing file has been specified by the user */
QDict *full_open_options;
char exact_filename[PATH_MAX];
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [Qemu-devel] [PATCH v2 03/24] block: Add BDS.backing_overridden
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 03/24] block: Add BDS.backing_overridden Max Reitz
@ 2016-11-27 2:33 ` Max Reitz
0 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 2:33 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Kevin Wolf, Alberto Garcia, Eric Blake
[-- Attachment #1: Type: text/plain, Size: 1838 bytes --]
On 27.11.2016 02:56, Max Reitz wrote:
> If the backing file is overridden, this most probably does change the
> guest-visible data of a BDS. Therefore, we will need to consider this in
> bdrv_refresh_filename().
>
> Adding a new field to the BDS is not nice, but it is very simple and
> exactly keeps track of whether the backing file has been overridden.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> While this patch adds a TODO comment that sounds like it could be
> resolved in a later patch in this series, it is not.
>
> Checking whether @options contains significant runtime options is not
> trivial, because we not only care about significant options at that
> level but also at levels below: Normally, it should be fine to check
> that for backing_hd and backing_hd->file->bs, but that's not for sure.
> So I decided a false positive is better than a false negative here,
> which is why the TODO persists throughout the series.
Of course I only noticed after I had sent this series that this was
wrong: It breaks iotest 030, and that is because even just giving a node
name to a backing node will now result in the backed file getting a JSON
filename. That is very unfortunate if one then tries to address files in
the backing chain by filename (which, very unfortunately, we do for
block-stream and block-commit).
So I guess in v3 I will do what I hinted at above: Shallow-clone the
options before @driver is set and after backing_hd has been opened,
check whether there are any significant options for either backing_hd or
backing_hd->file->bs.
Max
> ---
> block.c | 14 ++++++++++++++
> block/mirror.c | 4 ++++
> blockdev.c | 16 ++++++++++++++++
> include/block/block_int.h | 1 +
> 4 files changed, 35 insertions(+)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 04/24] block: Respect backing bs in bdrv_refresh_filename
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (2 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 03/24] block: Add BDS.backing_overridden Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 05/24] block: Make path_combine() return the path Max Reitz
` (19 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Basically, bdrv_refresh_filename() should respect all children of a
BlockDriverState. However, generally those children are driver-specific,
so this function cannot handle the general case. On the other hand,
there are only few drivers which use other children than @file and
@backing (that being vmdk, quorum, and blkverify).
Most block drivers only use @file and/or @backing (if they use any
children at all). Both can be implemented directly in
bdrv_refresh_filename.
The user overriding the file's filename is already handled, however, the
user overriding the backing file is not. If this is done, opening the
BDS with the plain filename of its file will not be correct, so we may
not set bs->exact_filename in that case.
iotest 051 contains test cases for overwriting the backing file, and so
its output changes with this patch applied (which I consider a good
thing).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 12 +++++++++++-
tests/qemu-iotests/051.out | 8 ++++----
tests/qemu-iotests/051.pc.out | 8 ++++----
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index c6c62b2..0b37c2e 100644
--- a/block.c
+++ b/block.c
@@ -4000,6 +4000,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
opts = qdict_new();
has_open_options = append_open_options(opts, bs);
+ has_open_options |= bs->backing_overridden;
/* If no specific options have been given for this BDS, the filename of
* the underlying file should suffice for this one as well */
@@ -4011,13 +4012,22 @@ void bdrv_refresh_filename(BlockDriverState *bs)
* file BDS. The full options QDict of that file BDS should somehow
* contain a representation of the filename, therefore the following
* suffices without querying the (exact_)filename of this BDS. */
- if (bs->file->bs->full_open_options) {
+ if (bs->file->bs->full_open_options &&
+ (!bs->backing || bs->backing->bs->full_open_options))
+ {
qdict_put_obj(opts, "driver",
QOBJECT(qstring_from_str(drv->format_name)));
QINCREF(bs->file->bs->full_open_options);
qdict_put_obj(opts, "file",
QOBJECT(bs->file->bs->full_open_options));
+ if (bs->backing) {
+ QINCREF(bs->backing->bs->full_open_options);
+ qdict_put(opts, "backing", bs->backing->bs->full_open_options);
+ } else if (bs->backing_overridden && !bs->backing) {
+ qdict_put(opts, "backing", qstring_new());
+ }
+
bs->full_open_options = opts;
} else {
QDECREF(opts);
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 42bf416..0d0b3b1 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -59,7 +59,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
Testing: -drive file=TEST_DIR/t.qcow2,driver=qcow2,backing.file.filename=TEST_DIR/t.qcow2.orig,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2.orig"}}, "driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
Removable device: not locked, tray closed
Cache mode: writeback
Backing file: TEST_DIR/t.qcow2.orig (chain depth: 1)
@@ -148,7 +148,7 @@ QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2.base"}}, "driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
Removable device: not locked, tray closed
Cache mode: writeback
Backing file: TEST_DIR/t.qcow2.base (chain depth: 1)
@@ -168,7 +168,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2.base"}}, "driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
Removable device: not locked, tray closed
Cache mode: writethrough
Backing file: TEST_DIR/t.qcow2.base (chain depth: 1)
@@ -188,7 +188,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2.base"}}, "driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
Removable device: not locked, tray closed
Cache mode: writeback, ignore flushes
Backing file: TEST_DIR/t.qcow2.base (chain depth: 1)
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index 603bb76..4b830bc 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -59,7 +59,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
Testing: -drive file=TEST_DIR/t.qcow2,driver=qcow2,backing.file.filename=TEST_DIR/t.qcow2.orig,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2.orig"}}, "driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
Removable device: not locked, tray closed
Cache mode: writeback
Backing file: TEST_DIR/t.qcow2.orig (chain depth: 1)
@@ -242,7 +242,7 @@ QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2.base"}}, "driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
Removable device: not locked, tray closed
Cache mode: writeback
Backing file: TEST_DIR/t.qcow2.base (chain depth: 1)
@@ -262,7 +262,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2.base"}}, "driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
Removable device: not locked, tray closed
Cache mode: writethrough
Backing file: TEST_DIR/t.qcow2.base (chain depth: 1)
@@ -282,7 +282,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2.base"}}, "driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
Removable device: not locked, tray closed
Cache mode: writeback, ignore flushes
Backing file: TEST_DIR/t.qcow2.base (chain depth: 1)
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 05/24] block: Make path_combine() return the path
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (3 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 04/24] block: Respect backing bs in bdrv_refresh_filename Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 06/24] block: bdrv_get_full_backing_filename_from_...'s ret. val Max Reitz
` (18 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Besides being safe for arbitrary path lengths, after some follow-up
patches all callers will want a freshly allocated buffer anyway.
In the meantime, path_combine_deprecated() is added which has the same
interface as path_combine() had before this patch. All callers to that
function will be converted in follow-up patches.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block.c | 81 +++++++++++++++++++++++++++++----------------------
block/vmdk.c | 3 +-
include/block/block.h | 4 +--
3 files changed, 48 insertions(+), 40 deletions(-)
diff --git a/block.c b/block.c
index 0b37c2e..70076f4 100644
--- a/block.c
+++ b/block.c
@@ -148,48 +148,59 @@ int path_is_absolute(const char *path)
#endif
}
-/* if filename is absolute, just copy it to dest. Otherwise, build a
+/* if filename is absolute, just return its duplicate. Otherwise, build a
path to it by considering it is relative to base_path. URL are
supported. */
-void path_combine(char *dest, int dest_size,
- const char *base_path,
- const char *filename)
+char *path_combine(const char *base_path, const char *filename)
{
const char *p, *p1;
+ char *result;
int len;
- if (dest_size <= 0)
- return;
if (path_is_absolute(filename)) {
- pstrcpy(dest, dest_size, filename);
+ return g_strdup(filename);
+ }
+
+ p = strchr(base_path, ':');
+ if (p) {
+ p++;
} else {
- p = strchr(base_path, ':');
- if (p)
- p++;
- else
- p = base_path;
- p1 = strrchr(base_path, '/');
+ p = base_path;
+ }
+ p1 = strrchr(base_path, '/');
#ifdef _WIN32
- {
- const char *p2;
- p2 = strrchr(base_path, '\\');
- if (!p1 || p2 > p1)
- p1 = p2;
+ {
+ const char *p2;
+ p2 = strrchr(base_path, '\\');
+ if (!p1 || p2 > p1) {
+ p1 = p2;
}
+ }
#endif
- if (p1)
- p1++;
- else
- p1 = base_path;
- if (p1 > p)
- p = p1;
- len = p - base_path;
- if (len > dest_size - 1)
- len = dest_size - 1;
- memcpy(dest, base_path, len);
- dest[len] = '\0';
- pstrcat(dest, dest_size, filename);
+ if (p1) {
+ p1++;
+ } else {
+ p1 = base_path;
+ }
+ if (p1 > p) {
+ p = p1;
}
+ len = p - base_path;
+
+ result = g_malloc(len + strlen(filename) + 1);
+ memcpy(result, base_path, len);
+ strcpy(result + len, filename);
+
+ return result;
+}
+
+static void path_combine_deprecated(char *dest, int dest_size,
+ const char *base_path,
+ const char *filename)
+{
+ char *combined = path_combine(base_path, filename);
+ pstrcpy(dest, dest_size, combined);
+ g_free(combined);
}
void bdrv_get_full_backing_filename_from_filename(const char *backed,
@@ -205,7 +216,7 @@ void bdrv_get_full_backing_filename_from_filename(const char *backed,
error_setg(errp, "Cannot use relative backing file names for '%s'",
backed);
} else {
- path_combine(dest, sz, backed, backing);
+ path_combine_deprecated(dest, sz, backed, backing);
}
}
@@ -3178,8 +3189,8 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
} else {
/* If not an absolute filename path, make it relative to the current
* image's filename path */
- path_combine(filename_tmp, PATH_MAX, curr_bs->filename,
- backing_file);
+ path_combine_deprecated(filename_tmp, PATH_MAX, curr_bs->filename,
+ backing_file);
/* We are going to compare absolute pathnames */
if (!realpath(filename_tmp, filename_full)) {
@@ -3188,8 +3199,8 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
/* We need to make sure the backing filename we are comparing against
* is relative to the current image filename (or absolute) */
- path_combine(filename_tmp, PATH_MAX, curr_bs->filename,
- curr_bs->backing_file);
+ path_combine_deprecated(filename_tmp, PATH_MAX, curr_bs->filename,
+ curr_bs->backing_file);
if (!realpath(filename_tmp, backing_file_full)) {
continue;
diff --git a/block/vmdk.c b/block/vmdk.c
index a11c27a..aad2d7d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -846,8 +846,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
return -EINVAL;
}
- extent_path = g_malloc0(PATH_MAX);
- path_combine(extent_path, PATH_MAX, desc_file_path, fname);
+ extent_path = path_combine(desc_file_path, fname);
ret = snprintf(extent_opt_prefix, 32, "extents.%d", s->num_extents);
assert(ret < 32);
diff --git a/include/block/block.h b/include/block/block.h
index 49bb0b2..6e5d4b2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -444,9 +444,7 @@ void bdrv_get_full_backing_filename_from_filename(const char *backed,
int path_has_protocol(const char *path);
int path_is_absolute(const char *path);
-void path_combine(char *dest, int dest_size,
- const char *base_path,
- const char *filename);
+char *path_combine(const char *base_path, const char *filename);
int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 06/24] block: bdrv_get_full_backing_filename_from_...'s ret. val.
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (4 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 05/24] block: Make path_combine() return the path Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 07/24] block: bdrv_get_full_backing_filename's " Max Reitz
` (17 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Make bdrv_get_full_backing_filename_from_filename() return an allocated
string instead of placing the result in a caller-provided buffer.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 32 +++++++++++++++++++-------------
block/vmdk.c | 8 +++-----
include/block/block.h | 7 +++----
3 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/block.c b/block.c
index 70076f4..45013dc 100644
--- a/block.c
+++ b/block.c
@@ -203,20 +203,20 @@ static void path_combine_deprecated(char *dest, int dest_size,
g_free(combined);
}
-void bdrv_get_full_backing_filename_from_filename(const char *backed,
- const char *backing,
- char *dest, size_t sz,
- Error **errp)
+char *bdrv_get_full_backing_filename_from_filename(const char *backed,
+ const char *backing,
+ Error **errp)
{
if (backing[0] == '\0' || path_has_protocol(backing) ||
path_is_absolute(backing))
{
- pstrcpy(dest, sz, backing);
+ return g_strdup(backing);
} else if (backed[0] == '\0' || strstart(backed, "json:", NULL)) {
error_setg(errp, "Cannot use relative backing file names for '%s'",
backed);
+ return NULL;
} else {
- path_combine_deprecated(dest, sz, backed, backing);
+ return path_combine(backed, backing);
}
}
@@ -224,9 +224,15 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz,
Error **errp)
{
char *backed = bs->exact_filename[0] ? bs->exact_filename : bs->filename;
+ char *full_name;
- bdrv_get_full_backing_filename_from_filename(backed, bs->backing_file,
- dest, sz, errp);
+ full_name = bdrv_get_full_backing_filename_from_filename(backed,
+ bs->backing_file,
+ errp);
+ if (full_name) {
+ pstrcpy(dest, sz, full_name);
+ g_free(full_name);
+ }
}
void bdrv_register(BlockDriver *bdrv)
@@ -3608,16 +3614,16 @@ void bdrv_img_create(const char *filename, const char *fmt,
if (size == -1) {
if (backing_file) {
BlockDriverState *bs;
- char *full_backing = g_new0(char, PATH_MAX);
+ char *full_backing;
int64_t size;
int back_flags;
QDict *backing_options = NULL;
- bdrv_get_full_backing_filename_from_filename(filename, backing_file,
- full_backing, PATH_MAX,
- &local_err);
+ full_backing =
+ bdrv_get_full_backing_filename_from_filename(filename,
+ backing_file,
+ &local_err);
if (local_err) {
- g_free(full_backing);
goto out;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index aad2d7d..6f6932a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1973,12 +1973,10 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
}
if (backing_file) {
BlockBackend *blk;
- char *full_backing = g_new0(char, PATH_MAX);
- bdrv_get_full_backing_filename_from_filename(filename, backing_file,
- full_backing, PATH_MAX,
- &local_err);
+ char *full_backing =
+ bdrv_get_full_backing_filename_from_filename(filename, backing_file,
+ &local_err);
if (local_err) {
- g_free(full_backing);
error_propagate(errp, local_err);
ret = -ENOENT;
goto exit;
diff --git a/include/block/block.h b/include/block/block.h
index 6e5d4b2..fe16e45 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -437,10 +437,9 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
char *filename, int filename_size);
void bdrv_get_full_backing_filename(BlockDriverState *bs,
char *dest, size_t sz, Error **errp);
-void bdrv_get_full_backing_filename_from_filename(const char *backed,
- const char *backing,
- char *dest, size_t sz,
- Error **errp);
+char *bdrv_get_full_backing_filename_from_filename(const char *backed,
+ const char *backing,
+ Error **errp);
int path_has_protocol(const char *path);
int path_is_absolute(const char *path);
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 07/24] block: bdrv_get_full_backing_filename's ret. val.
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (5 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 06/24] block: bdrv_get_full_backing_filename_from_...'s ret. val Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 08/24] block: Add bdrv_make_absolute_filename() Max Reitz
` (16 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Make bdrv_get_full_backing_filename() return an allocated string instead
of placing the result in a caller-provided buffer.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 26 +++++++++-----------------
block/qapi.c | 12 ++----------
include/block/block.h | 3 +--
3 files changed, 12 insertions(+), 29 deletions(-)
diff --git a/block.c b/block.c
index 45013dc..d6b3a5a 100644
--- a/block.c
+++ b/block.c
@@ -220,19 +220,13 @@ char *bdrv_get_full_backing_filename_from_filename(const char *backed,
}
}
-void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz,
- Error **errp)
+char *bdrv_get_full_backing_filename(BlockDriverState *bs, Error **errp)
{
char *backed = bs->exact_filename[0] ? bs->exact_filename : bs->filename;
- char *full_name;
- full_name = bdrv_get_full_backing_filename_from_filename(backed,
- bs->backing_file,
- errp);
- if (full_name) {
- pstrcpy(dest, sz, full_name);
- g_free(full_name);
- }
+ return bdrv_get_full_backing_filename_from_filename(backed,
+ bs->backing_file,
+ errp);
}
void bdrv_register(BlockDriver *bdrv)
@@ -1484,7 +1478,7 @@ out:
int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
const char *bdref_key, Error **errp)
{
- char *backing_filename = g_malloc0(PATH_MAX);
+ char *backing_filename = NULL;
char *bdref_key_dot;
const char *reference = NULL;
int ret = 0;
@@ -1511,13 +1505,12 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
reference = qdict_get_try_str(parent_options, bdref_key);
if (reference || qdict_haskey(options, "file.filename")) {
- backing_filename[0] = '\0';
+ backing_filename = NULL;
} else if (bs->backing_file[0] == '\0' && qdict_size(options) == 0) {
QDECREF(options);
goto free_exit;
} else {
- bdrv_get_full_backing_filename(bs, backing_filename, PATH_MAX,
- &local_err);
+ backing_filename = bdrv_get_full_backing_filename(bs, &local_err);
if (local_err) {
ret = -EINVAL;
error_propagate(errp, local_err);
@@ -1543,9 +1536,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
qdict_put(options, "driver", qstring_from_str(bs->backing_format));
}
- backing_hd = bdrv_open_inherit(*backing_filename ? backing_filename : NULL,
- reference, options, 0, bs, &child_backing,
- errp);
+ backing_hd = bdrv_open_inherit(backing_filename, reference, options, 0, bs,
+ &child_backing, errp);
if (!backing_hd) {
bs->open_flags |= BDRV_O_NO_BACKING;
error_prepend(errp, "Could not open backing file: ");
diff --git a/block/qapi.c b/block/qapi.c
index a62e862..1543dd6 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -265,18 +265,10 @@ void bdrv_query_image_info(BlockDriverState *bs,
backing_filename = bs->backing_file;
if (backing_filename[0] != '\0') {
- char *backing_filename2 = g_malloc0(PATH_MAX);
+ char *backing_filename2;
info->backing_filename = g_strdup(backing_filename);
info->has_backing_filename = true;
- bdrv_get_full_backing_filename(bs, backing_filename2, PATH_MAX, &err);
- if (err) {
- /* Can't reconstruct the full backing filename, so we must omit
- * this field and apply a Best Effort to this query. */
- g_free(backing_filename2);
- backing_filename2 = NULL;
- error_free(err);
- err = NULL;
- }
+ backing_filename2 = bdrv_get_full_backing_filename(bs, NULL);
/* Always report the full_backing_filename if present, even if it's the
* same as backing_filename. That they are same is useful info. */
diff --git a/include/block/block.h b/include/block/block.h
index fe16e45..eb41956 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -435,8 +435,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
void bdrv_get_backing_filename(BlockDriverState *bs,
char *filename, int filename_size);
-void bdrv_get_full_backing_filename(BlockDriverState *bs,
- char *dest, size_t sz, Error **errp);
+char *bdrv_get_full_backing_filename(BlockDriverState *bs, Error **errp);
char *bdrv_get_full_backing_filename_from_filename(const char *backed,
const char *backing,
Error **errp);
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 08/24] block: Add bdrv_make_absolute_filename()
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (6 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 07/24] block: bdrv_get_full_backing_filename's " Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 09/24] block: Fix bdrv_find_backing_image() Max Reitz
` (15 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
This is a general function for making a filename that is relative to a
certain BDS absolute.
It calls bdrv_get_full_backing_filename_from_filename() for now, but
that will be changed in a follow-up patch.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index d6b3a5a..a384e28 100644
--- a/block.c
+++ b/block.c
@@ -220,15 +220,22 @@ char *bdrv_get_full_backing_filename_from_filename(const char *backed,
}
}
-char *bdrv_get_full_backing_filename(BlockDriverState *bs, Error **errp)
+static char *bdrv_make_absolute_filename(BlockDriverState *relative_to,
+ const char *filename, Error **errp)
{
- char *backed = bs->exact_filename[0] ? bs->exact_filename : bs->filename;
+ char *bs_filename = relative_to->exact_filename[0]
+ ? relative_to->exact_filename
+ : relative_to->filename;
- return bdrv_get_full_backing_filename_from_filename(backed,
- bs->backing_file,
+ return bdrv_get_full_backing_filename_from_filename(bs_filename, filename,
errp);
}
+char *bdrv_get_full_backing_filename(BlockDriverState *bs, Error **errp)
+{
+ return bdrv_make_absolute_filename(bs, bs->backing_file, errp);
+}
+
void bdrv_register(BlockDriver *bdrv)
{
QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 09/24] block: Fix bdrv_find_backing_image()
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (7 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 08/24] block: Add bdrv_make_absolute_filename() Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 10/24] block: Add bdrv_dirname() Max Reitz
` (14 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
bdrv_find_backing_image() should use bdrv_get_full_backing_filename() or
bdrv_make_absolute_filename() instead of trying to do what those
functions do by itself.
path_combine_deprecated() can now be dropped, so let's do that.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/block.c b/block.c
index a384e28..e2e05e0 100644
--- a/block.c
+++ b/block.c
@@ -194,15 +194,6 @@ char *path_combine(const char *base_path, const char *filename)
return result;
}
-static void path_combine_deprecated(char *dest, int dest_size,
- const char *base_path,
- const char *filename)
-{
- char *combined = path_combine(base_path, filename);
- pstrcpy(dest, dest_size, combined);
- g_free(combined);
-}
-
char *bdrv_get_full_backing_filename_from_filename(const char *backed,
const char *backing,
Error **errp)
@@ -3178,7 +3169,6 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
filename_full = g_malloc(PATH_MAX);
backing_file_full = g_malloc(PATH_MAX);
- filename_tmp = g_malloc(PATH_MAX);
is_protocol = path_has_protocol(backing_file);
@@ -3194,22 +3184,31 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
} else {
/* If not an absolute filename path, make it relative to the current
* image's filename path */
- path_combine_deprecated(filename_tmp, PATH_MAX, curr_bs->filename,
- backing_file);
+ filename_tmp = bdrv_make_absolute_filename(curr_bs, backing_file,
+ NULL);
+ if (!filename_tmp) {
+ continue;
+ }
/* We are going to compare absolute pathnames */
if (!realpath(filename_tmp, filename_full)) {
+ g_free(filename_tmp);
continue;
}
+ g_free(filename_tmp);
/* We need to make sure the backing filename we are comparing against
* is relative to the current image filename (or absolute) */
- path_combine_deprecated(filename_tmp, PATH_MAX, curr_bs->filename,
- curr_bs->backing_file);
+ filename_tmp = bdrv_get_full_backing_filename(curr_bs, NULL);
+ if (!filename_tmp) {
+ continue;
+ }
if (!realpath(filename_tmp, backing_file_full)) {
+ g_free(filename_tmp);
continue;
}
+ g_free(filename_tmp);
if (strcmp(backing_file_full, filename_full) == 0) {
retval = curr_bs->backing->bs;
@@ -3220,7 +3219,6 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
g_free(filename_full);
g_free(backing_file_full);
- g_free(filename_tmp);
return retval;
}
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 10/24] block: Add bdrv_dirname()
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (8 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 09/24] block: Fix bdrv_find_backing_image() Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 11/24] blkverify: Make bdrv_dirname() return NULL Max Reitz
` (13 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
This function may be implemented by block drivers to derive a directory
name from a BDS. Concatenating this g_free()-able string with a relative
filename must result in a valid (not necessarily existing) filename, so
this is a function that should generally be not implemented by format
drivers, because this is protocol-specific.
If a BDS's driver does not implement this function, bdrv_dirname() will
fall through to the BDS's file if it exists. If it does not, the
exact_filename field will be used to generate a directory name.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 26 ++++++++++++++++++++++++++
include/block/block.h | 1 +
include/block/block_int.h | 1 +
3 files changed, 28 insertions(+)
diff --git a/block.c b/block.c
index e2e05e0..5108555 100644
--- a/block.c
+++ b/block.c
@@ -4084,6 +4084,32 @@ void bdrv_refresh_filename(BlockDriverState *bs)
}
}
+char *bdrv_dirname(BlockDriverState *bs, Error **errp)
+{
+ BlockDriver *drv = bs->drv;
+
+ if (!drv) {
+ error_setg(errp, "Node '%s' is ejected", bs->node_name);
+ return NULL;
+ }
+
+ if (drv->bdrv_dirname) {
+ return drv->bdrv_dirname(bs, errp);
+ }
+
+ if (bs->file) {
+ return bdrv_dirname(bs->file->bs, errp);
+ }
+
+ if (bs->exact_filename[0] != '\0') {
+ return path_combine(bs->exact_filename, "");
+ }
+
+ error_setg(errp, "Cannot generate a base directory for %s nodes",
+ drv->format_name);
+ return NULL;
+}
+
/*
* Hot add/remove a BDS's child. So the user can take a child offline when
* it is broken and take a new child online
diff --git a/include/block/block.h b/include/block/block.h
index eb41956..7bcbd05 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -439,6 +439,7 @@ char *bdrv_get_full_backing_filename(BlockDriverState *bs, Error **errp);
char *bdrv_get_full_backing_filename_from_filename(const char *backed,
const char *backing,
Error **errp);
+char *bdrv_dirname(BlockDriverState *bs, Error **errp);
int path_has_protocol(const char *path);
int path_is_absolute(const char *path);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 555f3c2..65d6f53 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -130,6 +130,7 @@ struct BlockDriver {
int (*bdrv_make_empty)(BlockDriverState *bs);
void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
+ char *(*bdrv_dirname)(BlockDriverState *bs, Error **errp);
/* aio */
BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 11/24] blkverify: Make bdrv_dirname() return NULL
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (9 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 10/24] block: Add bdrv_dirname() Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 12/24] quorum: " Max Reitz
` (12 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
blkverify's BDSs have a file BDS, but we do not want this to be
preferred over the raw node. There is no way to decide between the two
(and not really a reason to, either), so just return NULL in blkverify's
implementation of bdrv_dirname().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/blkverify.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/block/blkverify.c b/block/blkverify.c
index e0f9275..d6e57e0 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -318,6 +318,15 @@ static void blkverify_refresh_filename(BlockDriverState *bs, QDict *options)
}
}
+static char *blkverify_dirname(BlockDriverState *bs, Error **errp)
+{
+ /* In general, there are two BDSs with different dirnames below this one;
+ * so there is no unique dirname we could return (unless both are equal by
+ * chance). Therefore, to be consistent, just always return NULL. */
+ error_setg(errp, "Cannot generate a base directory for blkverify nodes");
+ return NULL;
+}
+
static BlockDriver bdrv_blkverify = {
.format_name = "blkverify",
.protocol_name = "blkverify",
@@ -328,6 +337,7 @@ static BlockDriver bdrv_blkverify = {
.bdrv_close = blkverify_close,
.bdrv_getlength = blkverify_getlength,
.bdrv_refresh_filename = blkverify_refresh_filename,
+ .bdrv_dirname = blkverify_dirname,
.bdrv_aio_readv = blkverify_aio_readv,
.bdrv_aio_writev = blkverify_aio_writev,
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 12/24] quorum: Make bdrv_dirname() return NULL
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (10 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 11/24] blkverify: Make bdrv_dirname() return NULL Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 13/24] block/nbd: Implement bdrv_dirname() Max Reitz
` (11 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
While the common implementation for bdrv_dirname() should return NULL
for quorum BDSs already (because they do not have a file node and their
exact_filename field should be empty), there is no reason not to make
that explicit.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/quorum.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/block/quorum.c b/block/quorum.c
index 45e61bb..07c943c 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1083,6 +1083,16 @@ static void quorum_refresh_filename(BlockDriverState *bs, QDict *options)
bs->full_open_options = opts;
}
+static char *quorum_dirname(BlockDriverState *bs, Error **errp)
+{
+ /* In general, there are multiple BDSs with different dirnames below this
+ * one; so there is no unique dirname we could return (unless all are equal
+ * by chance, or there is only one). Therefore, to be consistent, just
+ * always return NULL. */
+ error_setg(errp, "Cannot generate a base directory for quorum nodes");
+ return NULL;
+}
+
static BlockDriver bdrv_quorum = {
.format_name = "quorum",
.protocol_name = "quorum",
@@ -1092,6 +1102,7 @@ static BlockDriver bdrv_quorum = {
.bdrv_file_open = quorum_open,
.bdrv_close = quorum_close,
.bdrv_refresh_filename = quorum_refresh_filename,
+ .bdrv_dirname = quorum_dirname,
.bdrv_co_flush_to_disk = quorum_co_flush,
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 13/24] block/nbd: Implement bdrv_dirname()
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (11 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 12/24] quorum: " Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 14/24] block/nfs: " Max Reitz
` (10 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
The idea behind this implementation is that the export name might be
interpreted as a path (which is the only apparent interpretation of
relative filenames for NBD paths).
The default implementation of bdrv_dirname() would handle that just fine
for nbd+tcp, but not for nbd+unix, because in that case, the last
element of the path is the Unix socket path and not the export name.
Therefore, we need to implement an own bdrv_dirname() which uses the
legacy NBD URL which has the export name at the end.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/nbd.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/block/nbd.c b/block/nbd.c
index 35f24be..42f0cd6 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -552,6 +552,34 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
bs->full_open_options = opts;
}
+static char *nbd_dirname(BlockDriverState *bs, Error **errp)
+{
+ BDRVNBDState *s = bs->opaque;
+ const char *host = NULL, *port = NULL, *path = NULL;
+
+ if (s->saddr->type == SOCKET_ADDRESS_KIND_INET) {
+ const InetSocketAddress *inet = s->saddr->u.inet.data;
+ if (!inet->has_ipv4 && !inet->has_ipv6 && !inet->has_to) {
+ host = inet->host;
+ port = inet->port;
+ }
+ } else if (s->saddr->type == SOCKET_ADDRESS_KIND_UNIX) {
+ path = s->saddr->u.q_unix.data->path;
+ }
+
+ if (path) {
+ return g_strdup_printf("nbd:unix:%s:exportname=%s%s", path,
+ s->export ?: "", s->export ? "/" : "");
+ } else if (host) {
+ return g_strdup_printf("nbd://%s:%s/%s%s", host, port,
+ s->export ?: "", s->export ? "/" : "");
+ }
+
+ error_setg(errp, "Cannot generate a base directory for NBD node '%s'",
+ bs->filename);
+ return NULL;
+}
+
static BlockDriver bdrv_nbd = {
.format_name = "nbd",
.protocol_name = "nbd",
@@ -569,6 +597,7 @@ static BlockDriver bdrv_nbd = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_dirname = nbd_dirname,
};
static BlockDriver bdrv_nbd_tcp = {
@@ -588,6 +617,7 @@ static BlockDriver bdrv_nbd_tcp = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_dirname = nbd_dirname,
};
static BlockDriver bdrv_nbd_unix = {
@@ -607,6 +637,7 @@ static BlockDriver bdrv_nbd_unix = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_dirname = nbd_dirname,
};
static void bdrv_nbd_init(void)
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 14/24] block/nfs: Implement bdrv_dirname()
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (12 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 13/24] block/nbd: Implement bdrv_dirname() Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 15/24] block: Use bdrv_dirname() for relative filenames Max Reitz
` (9 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
While the basic idea is obvious and could be handled by the default
bdrv_dirname() implementation, we cannot generate a directory name if
the gid or uid are set, so we have to explicitly return NULL in those
cases.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/nfs.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/block/nfs.c b/block/nfs.c
index d082783..81ce98a 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -828,6 +828,19 @@ static void nfs_refresh_filename(BlockDriverState *bs, QDict *options)
bs->full_open_options = opts;
}
+static char *nfs_dirname(BlockDriverState *bs, Error **errp)
+{
+ NFSClient *client = bs->opaque;
+
+ if (client->uid || client->gid) {
+ error_setg(errp, "Cannot generate a base directory for NBD node '%s'",
+ bs->filename);
+ return NULL;
+ }
+
+ return g_strdup_printf("nfs://%s%s/", client->server->host, client->path);
+}
+
#ifdef LIBNFS_FEATURE_PAGECACHE
static void nfs_invalidate_cache(BlockDriverState *bs,
Error **errp)
@@ -861,6 +874,7 @@ static BlockDriver bdrv_nfs = {
.bdrv_detach_aio_context = nfs_detach_aio_context,
.bdrv_attach_aio_context = nfs_attach_aio_context,
.bdrv_refresh_filename = nfs_refresh_filename,
+ .bdrv_dirname = nfs_dirname,
#ifdef LIBNFS_FEATURE_PAGECACHE
.bdrv_invalidate_cache = nfs_invalidate_cache,
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 15/24] block: Use bdrv_dirname() for relative filenames
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (13 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 14/24] block/nfs: " Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 16/24] block: Add 'base-directory' BDS option Max Reitz
` (8 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
bdrv_get_full_backing_filename_from_filename() breaks down when it comes
to JSON filenames. Using bdrv_dirname() as the basis is better because
since we have BDS, we can descend through the BDS tree to the protocol
layer, which gives us a greater probability of finding a non-JSON name;
also, bdrv_dirname() is more correct as it allows block drivers to
override the generation of that directory name in a protocol-specific
way.
We still need to keep bdrv_get_full_backing_filename_from_filename(),
though, because it has valid callers which need it during image creation
when no BDS is available yet.
This makes a test case in qemu-iotest 110, which was supposed to fail,
work. That is actually good, but we need to change the reference output
(and the comment in 110) accordingly.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 20 +++++++++++++++-----
tests/qemu-iotests/110 | 3 ++-
tests/qemu-iotests/110.out | 2 +-
3 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index 5108555..b878d66 100644
--- a/block.c
+++ b/block.c
@@ -214,12 +214,22 @@ char *bdrv_get_full_backing_filename_from_filename(const char *backed,
static char *bdrv_make_absolute_filename(BlockDriverState *relative_to,
const char *filename, Error **errp)
{
- char *bs_filename = relative_to->exact_filename[0]
- ? relative_to->exact_filename
- : relative_to->filename;
+ char *dir, *full_name;
- return bdrv_get_full_backing_filename_from_filename(bs_filename, filename,
- errp);
+ if (filename[0] == '\0' || path_has_protocol(filename) ||
+ path_is_absolute(filename))
+ {
+ return g_strdup(filename);
+ }
+
+ dir = bdrv_dirname(relative_to, errp);
+ if (!dir) {
+ return NULL;
+ }
+
+ full_name = g_strconcat(dir, filename, NULL);
+ g_free(dir);
+ return full_name;
}
char *bdrv_get_full_backing_filename(BlockDriverState *bs, Error **errp)
diff --git a/tests/qemu-iotests/110 b/tests/qemu-iotests/110
index 9de7369..ba1b3c6 100755
--- a/tests/qemu-iotests/110
+++ b/tests/qemu-iotests/110
@@ -61,7 +61,8 @@ echo '=== Non-reconstructable filename ==='
echo
# Across blkdebug without a config file, you cannot reconstruct filenames, so
-# qemu is incapable of knowing the directory of the top image
+# qemu is incapable of knowing the directory of the top image from the filename
+# alone. However, using bdrv_dirname(), it should still work.
TEST_IMG="json:{
'driver': '$IMGFMT',
'file': {
diff --git a/tests/qemu-iotests/110.out b/tests/qemu-iotests/110.out
index b3584ff..5370bc1 100644
--- a/tests/qemu-iotests/110.out
+++ b/tests/qemu-iotests/110.out
@@ -14,7 +14,7 @@ backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
image: json:{"driver": "IMGFMT", "file": {"set-state.0.event": "read_aio", "image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "set-state.0.new_state": 42}}
file format: IMGFMT
virtual size: 64M (67108864 bytes)
-backing file: t.IMGFMT.base (cannot determine actual path)
+backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
=== Backing name is always relative to the backed image ===
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 16/24] block: Add 'base-directory' BDS option
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (14 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 15/24] block: Use bdrv_dirname() for relative filenames Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 17/24] iotests: Add quorum case to test 110 Max Reitz
` (7 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Using this option, one can directly override what bdrv_dirname() will
return. This is useful if one uses e.g. qcow2 on top of quorum (with
only protocol BDSs under the quorum BDS) and wants to be able to use
relative backing filenames.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 14 ++++++++++++++
include/block/block_int.h | 2 ++
qapi/block-core.json | 9 +++++++++
3 files changed, 25 insertions(+)
diff --git a/block.c b/block.c
index b878d66..44412e1 100644
--- a/block.c
+++ b/block.c
@@ -984,6 +984,12 @@ QemuOptsList bdrv_runtime_opts = {
.type = QEMU_OPT_STRING,
.help = "discard operation (ignore/off, unmap/on)",
},
+ {
+ .name = "base-directory",
+ .type = QEMU_OPT_STRING,
+ .help = "String to prepend to filenames relative to this BDS for "
+ "making them absolute",
+ },
{ /* end of list */ }
},
};
@@ -1047,6 +1053,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
goto fail_opts;
}
+ bs->dirname = g_strdup(qemu_opt_get(opts, "base-directory"));
+
bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
@@ -2491,6 +2499,8 @@ static void bdrv_delete(BlockDriverState *bs)
bdrv_close(bs);
+ g_free(bs->dirname);
+
/* remove from list, if necessary */
if (bs->node_name[0] != '\0') {
QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
@@ -4098,6 +4108,10 @@ char *bdrv_dirname(BlockDriverState *bs, Error **errp)
{
BlockDriver *drv = bs->drv;
+ if (bs->dirname) {
+ return g_strdup(bs->dirname);
+ }
+
if (!drv) {
error_setg(errp, "Node '%s' is ejected", bs->node_name);
return NULL;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 65d6f53..7143737 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -467,6 +467,8 @@ struct BlockDriverState {
QDict *full_open_options;
char exact_filename[PATH_MAX];
+ char *dirname;
+
BdrvChild *backing;
BdrvChild *file;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c29bef7..a677483 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2366,6 +2366,14 @@
# (default: false)
# @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
# (default: off)
+# @base-directory #optional May be specified for any node. Normally, whenever a
+# filename is specified which is supposed to be relative to this
+# node (such as relative backing filenames), the base directory
+# to be used is the directory the image file of this node is in,
+# which is simply prepended to the relative filename. Using this
+# option, the string which is prepended (i.e. the base
+# directory) can be overridden.
+# (Since 2.9)
#
# Remaining options are determined by the block driver.
#
@@ -2374,6 +2382,7 @@
{ 'union': 'BlockdevOptions',
'base': { 'driver': 'BlockdevDriver',
'*node-name': 'str',
+ '*base-directory': 'str',
'*discard': 'BlockdevDiscardOptions',
'*cache': 'BlockdevCacheOptions',
'*read-only': 'bool',
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 17/24] iotests: Add quorum case to test 110
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (15 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 16/24] block: Add 'base-directory' BDS option Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 18/24] block: Add sgfnt_runtime_opts to BlockDriver Max Reitz
` (6 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Test 110 tests relative backing filenames for complex BDS trees. Add
quorum as an example that can never work automatically (without
special-casing if all child nodes have the same base directory), and an
example on how to make it work manually (using the base-directory
option).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/110 | 48 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/110.out | 12 ++++++++++++
2 files changed, 60 insertions(+)
diff --git a/tests/qemu-iotests/110 b/tests/qemu-iotests/110
index ba1b3c6..d96b656 100755
--- a/tests/qemu-iotests/110
+++ b/tests/qemu-iotests/110
@@ -30,6 +30,7 @@ status=1 # failure is the default!
_cleanup()
{
_cleanup_test_img
+ rm -f "$TEST_IMG.copy"
}
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -87,6 +88,53 @@ echo
# omit the image size; it should work anyway
_make_test_img -b "$TEST_IMG_REL.base"
+echo
+echo '=== Nodes without a common directory ==='
+echo
+
+cp "$TEST_IMG" "$TEST_IMG.copy"
+
+# Should inform us that the actual path of the backing file cannot be determined
+TEST_IMG="json:{
+ 'driver': '$IMGFMT',
+ 'file': {
+ 'driver': 'quorum',
+ 'vote-threshold': 1,
+ 'children': [
+ {
+ 'driver': 'file',
+ 'filename': '$TEST_IMG'
+ },
+ {
+ 'driver': 'file',
+ 'filename': '$TEST_IMG.copy'
+ }
+ ]
+ }
+}" _img_info | _filter_img_info
+
+echo
+
+# Should work fine
+TEST_IMG="json:{
+ 'driver': '$IMGFMT',
+ 'file': {
+ 'driver': 'quorum',
+ 'base-directory': '$TEST_DIR/',
+ 'vote-threshold': 1,
+ 'children': [
+ {
+ 'driver': 'file',
+ 'filename': '$TEST_IMG'
+ },
+ {
+ 'driver': 'file',
+ 'filename': '$TEST_IMG.copy'
+ }
+ ]
+ }
+}" _img_info | _filter_img_info
+
# success, all done
echo '*** done'
diff --git a/tests/qemu-iotests/110.out b/tests/qemu-iotests/110.out
index 5370bc1..e1845d8 100644
--- a/tests/qemu-iotests/110.out
+++ b/tests/qemu-iotests/110.out
@@ -19,4 +19,16 @@ backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
=== Backing name is always relative to the backed image ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=t.IMGFMT.base
+
+=== Nodes without a common directory ===
+
+image: json:{"driver": "IMGFMT", "file": {"children": [{"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, {"driver": "file", "filename": "TEST_DIR/t.IMGFMT.copy"}], "driver": "quorum", "blkverify": false, "rewrite-corrupted": false, "vote-threshold": 1}}
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+backing file: t.IMGFMT.base (cannot determine actual path)
+
+image: json:{"driver": "IMGFMT", "file": {"children": [{"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, {"driver": "file", "filename": "TEST_DIR/t.IMGFMT.copy"}], "driver": "quorum", "blkverify": false, "rewrite-corrupted": false, "vote-threshold": 1}}
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
*** done
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 18/24] block: Add sgfnt_runtime_opts to BlockDriver
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (16 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 17/24] iotests: Add quorum case to test 110 Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 19/24] block: Add BlockDriver.bdrv_gather_child_options Max Reitz
` (5 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
This new field can be set by block drivers to list the runtime options
they accept that may influence the contents of the respective BDS. As of
a follow-up patch, this list will be used by the common
bdrv_refresh_filename() implementation to decide which options to put
into BDS.full_open_options (and consequently whether a JSON filename has
to be created), thus freeing the drivers of having to implement that
logic themselves.
Additionally, this patch adds the field to all of the block drivers that
need it and sets it accordingly.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/blkdebug.c | 6 ++++++
block/curl.c | 20 ++++++++++++++++++++
block/gluster.c | 19 +++++++++++++++++++
block/nbd.c | 8 ++++++++
block/nfs.c | 3 +++
block/null.c | 9 +++++++++
block/quorum.c | 8 ++++++++
block/rbd.c | 2 ++
block/vvfat.c | 4 ++++
include/block/block_int.h | 7 +++++++
10 files changed, 86 insertions(+)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 4127571..5de6fad 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -762,6 +762,12 @@ static BlockDriver bdrv_blkdebug = {
= blkdebug_debug_remove_breakpoint,
.bdrv_debug_resume = blkdebug_debug_resume,
.bdrv_debug_is_suspended = blkdebug_debug_is_suspended,
+
+ .sgfnt_runtime_opts = (const char *const[]) { "config",
+ "inject-error.",
+ "set-state.",
+ "suspend.",
+ NULL },
};
static void bdrv_blkdebug_init(void)
diff --git a/block/curl.c b/block/curl.c
index 0404c1b..0777d5b 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -872,6 +872,18 @@ static int64_t curl_getlength(BlockDriverState *bs)
return s->len;
}
+static const char *const curl_sgfnt_runtime_opts[] = {
+ CURL_BLOCK_OPT_URL,
+ CURL_BLOCK_OPT_SSLVERIFY,
+ CURL_BLOCK_OPT_COOKIE,
+ CURL_BLOCK_OPT_USERNAME,
+ CURL_BLOCK_OPT_PASSWORD_SECRET,
+ CURL_BLOCK_OPT_PROXY_USERNAME,
+ CURL_BLOCK_OPT_PROXY_PASSWORD_SECRET,
+
+ NULL
+};
+
static BlockDriver bdrv_http = {
.format_name = "http",
.protocol_name = "http",
@@ -886,6 +898,8 @@ static BlockDriver bdrv_http = {
.bdrv_detach_aio_context = curl_detach_aio_context,
.bdrv_attach_aio_context = curl_attach_aio_context,
+
+ .sgfnt_runtime_opts = curl_sgfnt_runtime_opts,
};
static BlockDriver bdrv_https = {
@@ -902,6 +916,8 @@ static BlockDriver bdrv_https = {
.bdrv_detach_aio_context = curl_detach_aio_context,
.bdrv_attach_aio_context = curl_attach_aio_context,
+
+ .sgfnt_runtime_opts = curl_sgfnt_runtime_opts,
};
static BlockDriver bdrv_ftp = {
@@ -918,6 +934,8 @@ static BlockDriver bdrv_ftp = {
.bdrv_detach_aio_context = curl_detach_aio_context,
.bdrv_attach_aio_context = curl_attach_aio_context,
+
+ .sgfnt_runtime_opts = curl_sgfnt_runtime_opts,
};
static BlockDriver bdrv_ftps = {
@@ -934,6 +952,8 @@ static BlockDriver bdrv_ftps = {
.bdrv_detach_aio_context = curl_detach_aio_context,
.bdrv_attach_aio_context = curl_attach_aio_context,
+
+ .sgfnt_runtime_opts = curl_sgfnt_runtime_opts,
};
static void curl_block_init(void)
diff --git a/block/gluster.c b/block/gluster.c
index 891c13b..3b286c7 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1402,6 +1402,21 @@ static int64_t coroutine_fn qemu_gluster_co_get_block_status(
}
+static const char *const gluster_sgfnt_open_opts[] = {
+ GLUSTER_OPT_VOLUME,
+ GLUSTER_OPT_PATH,
+ GLUSTER_OPT_TYPE,
+ GLUSTER_OPT_SERVER_PATTERN,
+ GLUSTER_OPT_HOST,
+ GLUSTER_OPT_PORT,
+ GLUSTER_OPT_TO,
+ GLUSTER_OPT_IPV4,
+ GLUSTER_OPT_IPV6,
+ GLUSTER_OPT_SOCKET,
+
+ NULL
+};
+
static BlockDriver bdrv_gluster = {
.format_name = "gluster",
.protocol_name = "gluster",
@@ -1428,6 +1443,7 @@ static BlockDriver bdrv_gluster = {
#endif
.bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
.create_opts = &qemu_gluster_create_opts,
+ .sgfnt_runtime_opts = gluster_sgfnt_open_opts,
};
static BlockDriver bdrv_gluster_tcp = {
@@ -1456,6 +1472,7 @@ static BlockDriver bdrv_gluster_tcp = {
#endif
.bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
.create_opts = &qemu_gluster_create_opts,
+ .sgfnt_runtime_opts = gluster_sgfnt_open_opts,
};
static BlockDriver bdrv_gluster_unix = {
@@ -1484,6 +1501,7 @@ static BlockDriver bdrv_gluster_unix = {
#endif
.bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
.create_opts = &qemu_gluster_create_opts,
+ .sgfnt_runtime_opts = gluster_sgfnt_open_opts,
};
/* rdma is deprecated (actually never supported for volfile fetch).
@@ -1518,6 +1536,7 @@ static BlockDriver bdrv_gluster_rdma = {
#endif
.bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
.create_opts = &qemu_gluster_create_opts,
+ .sgfnt_runtime_opts = gluster_sgfnt_open_opts,
};
static void bdrv_gluster_init(void)
diff --git a/block/nbd.c b/block/nbd.c
index 42f0cd6..1307989 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -580,6 +580,11 @@ static char *nbd_dirname(BlockDriverState *bs, Error **errp)
return NULL;
}
+static const char *const nbd_sgfnt_runtime_opts[] = {
+ "path", "host", "port", "export", "tls-creds", "server.",
+ NULL
+};
+
static BlockDriver bdrv_nbd = {
.format_name = "nbd",
.protocol_name = "nbd",
@@ -598,6 +603,7 @@ static BlockDriver bdrv_nbd = {
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
.bdrv_dirname = nbd_dirname,
+ .sgfnt_runtime_opts = nbd_sgfnt_runtime_opts,
};
static BlockDriver bdrv_nbd_tcp = {
@@ -618,6 +624,7 @@ static BlockDriver bdrv_nbd_tcp = {
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
.bdrv_dirname = nbd_dirname,
+ .sgfnt_runtime_opts = nbd_sgfnt_runtime_opts,
};
static BlockDriver bdrv_nbd_unix = {
@@ -638,6 +645,7 @@ static BlockDriver bdrv_nbd_unix = {
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
.bdrv_dirname = nbd_dirname,
+ .sgfnt_runtime_opts = nbd_sgfnt_runtime_opts,
};
static void bdrv_nbd_init(void)
diff --git a/block/nfs.c b/block/nfs.c
index 81ce98a..6ffdede 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -876,6 +876,9 @@ static BlockDriver bdrv_nfs = {
.bdrv_refresh_filename = nfs_refresh_filename,
.bdrv_dirname = nfs_dirname,
+ .sgfnt_runtime_opts = (const char *const[]) { "path", "uid",
+ "gid", NULL },
+
#ifdef LIBNFS_FEATURE_PAGECACHE
.bdrv_invalidate_cache = nfs_invalidate_cache,
#endif
diff --git a/block/null.c b/block/null.c
index b300390..16ef8b2 100644
--- a/block/null.c
+++ b/block/null.c
@@ -236,6 +236,13 @@ static void null_refresh_filename(BlockDriverState *bs, QDict *opts)
bs->full_open_options = opts;
}
+static const char *const null_sgfnt_runtime_opts[] = {
+ BLOCK_OPT_SIZE,
+ NULL_OPT_ZEROES,
+
+ NULL
+};
+
static BlockDriver bdrv_null_co = {
.format_name = "null-co",
.protocol_name = "null-co",
@@ -253,6 +260,7 @@ static BlockDriver bdrv_null_co = {
.bdrv_co_get_block_status = null_co_get_block_status,
.bdrv_refresh_filename = null_refresh_filename,
+ .sgfnt_runtime_opts = null_sgfnt_runtime_opts,
};
static BlockDriver bdrv_null_aio = {
@@ -272,6 +280,7 @@ static BlockDriver bdrv_null_aio = {
.bdrv_co_get_block_status = null_co_get_block_status,
.bdrv_refresh_filename = null_refresh_filename,
+ .sgfnt_runtime_opts = null_sgfnt_runtime_opts,
};
static void bdrv_null_init(void)
diff --git a/block/quorum.c b/block/quorum.c
index 07c943c..0fa803b 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1116,6 +1116,14 @@ static BlockDriver bdrv_quorum = {
.is_filter = true,
.bdrv_recurse_is_first_non_filter = quorum_recurse_is_first_non_filter,
+
+ .sgfnt_runtime_opts = (const char *const[]) {
+ QUORUM_OPT_VOTE_THRESHOLD,
+ QUORUM_OPT_BLKVERIFY,
+ QUORUM_OPT_REWRITE,
+ QUORUM_OPT_READ_PATTERN,
+ NULL
+ },
};
static void bdrv_quorum_init(void)
diff --git a/block/rbd.c b/block/rbd.c
index a57b3e3..b09a18f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1005,6 +1005,8 @@ static BlockDriver bdrv_rbd = {
#ifdef LIBRBD_SUPPORTS_INVALIDATE
.bdrv_invalidate_cache = qemu_rbd_invalidate_cache,
#endif
+
+ .sgfnt_runtime_opts = (const char *const[]) { "password-secret", NULL },
};
static void bdrv_rbd_init(void)
diff --git a/block/vvfat.c b/block/vvfat.c
index ded2109..94f2860 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3077,6 +3077,10 @@ static BlockDriver bdrv_vvfat = {
.bdrv_co_preadv = vvfat_co_preadv,
.bdrv_co_pwritev = vvfat_co_pwritev,
.bdrv_co_get_block_status = vvfat_co_get_block_status,
+
+ .sgfnt_runtime_opts = (const char *const[]) { "dir", "fat-type",
+ "floppy", "label", "rw",
+ NULL },
};
static void bdrv_vvfat_init(void)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7143737..5885d9f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -322,6 +322,13 @@ struct BlockDriver {
Error **errp);
QLIST_ENTRY(BlockDriver) list;
+
+ /* Pointer to a NULL-terminated array of names of significant options that
+ * can be specified for bdrv_open(). A significant option is one that
+ * changes the data of a BDS.
+ * If this pointer is NULL, the array is considered empty.
+ * "filename" and "driver" are always considered significant. */
+ const char *const *sgfnt_runtime_opts;
};
typedef struct BlockLimits {
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 19/24] block: Add BlockDriver.bdrv_gather_child_options
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (17 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 18/24] block: Add sgfnt_runtime_opts to BlockDriver Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 20/24] block: Generically refresh runtime options Max Reitz
` (4 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Some follow-up patches will rework the way bs->full_open_options is
refreshed in bdrv_refresh_filename(). The new implementation will remove
the need for the block drivers' bdrv_refresh_filename() implementations
to set bs->full_open_options; instead, it will be generic and use static
information from each block driver.
However, by implementing bdrv_gather_child_options(), block drivers will
still be able to override the way the full_open_options of their
children are incorporated into their own.
We need to implement this function for VMDK because we have to prevent
the generic implementation from gathering the options of all children:
It is not possible to specify options for the extents through the
runtime options.
For quorum, the child names that would be used by the generic
implementation and the ones that we actually want to use differ. See
quorum_gather_child_options() for more information.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/quorum.c | 30 ++++++++++++++++++++++++++++++
block/vmdk.c | 13 +++++++++++++
include/block/block_int.h | 1 +
3 files changed, 44 insertions(+)
diff --git a/block/quorum.c b/block/quorum.c
index 0fa803b..126127c 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1083,6 +1083,35 @@ static void quorum_refresh_filename(BlockDriverState *bs, QDict *options)
bs->full_open_options = opts;
}
+static void quorum_gather_child_options(BlockDriverState *bs, QDict *target)
+{
+ BDRVQuorumState *s = bs->opaque;
+ QList *children_list;
+ int i;
+
+ /* The generic implementation for gathering child options in
+ * bdrv_refresh_filename() would use the names of the children as specified
+ * for bdrv_open_child() or bdrv_attach_child(), which is "children.%u" with
+ * %u being a value (s->next_child_index) that is incremented each time a
+ * new child is added (and never decremented). Since children can be deleted
+ * at runtime, there may be gaps in that enumeration. When creating a new
+ * quorum BDS and specifying the children for it through runtime options,
+ * the enumeration used there may not have any gaps, though.
+ *
+ * Therefore, we have to create a new gap-less enumeration here (which we
+ * can achieve by simply putting all of the children's full_open_options
+ * into a QList).
+ */
+
+ children_list = qlist_new();
+ qdict_put(target, "children", children_list);
+
+ for (i = 0; i < s->num_children; i++) {
+ QINCREF(s->children[i]->bs->full_open_options);
+ qlist_append(children_list, s->children[i]->bs->full_open_options);
+ }
+}
+
static char *quorum_dirname(BlockDriverState *bs, Error **errp)
{
/* In general, there are multiple BDSs with different dirnames below this
@@ -1102,6 +1131,7 @@ static BlockDriver bdrv_quorum = {
.bdrv_file_open = quorum_open,
.bdrv_close = quorum_close,
.bdrv_refresh_filename = quorum_refresh_filename,
+ .bdrv_gather_child_options = quorum_gather_child_options,
.bdrv_dirname = quorum_dirname,
.bdrv_co_flush_to_disk = quorum_co_flush,
diff --git a/block/vmdk.c b/block/vmdk.c
index 6f6932a..87ac4f0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2287,6 +2287,18 @@ static int vmdk_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
return 0;
}
+static void vmdk_gather_child_options(BlockDriverState *bs, QDict *target)
+{
+ /* No children but file and backing can be explicitly specified */
+ QINCREF(bs->file->bs->full_open_options);
+ qdict_put(target, "file", bs->file->bs->full_open_options);
+
+ if (bs->backing_overridden) {
+ QINCREF(bs->backing->bs->full_open_options);
+ qdict_put(target, "backing", bs->backing->bs->full_open_options);
+ }
+}
+
static QemuOptsList vmdk_create_opts = {
.name = "vmdk-create-opts",
.head = QTAILQ_HEAD_INITIALIZER(vmdk_create_opts.head),
@@ -2356,6 +2368,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_get_specific_info = vmdk_get_specific_info,
.bdrv_refresh_limits = vmdk_refresh_limits,
.bdrv_get_info = vmdk_get_info,
+ .bdrv_gather_child_options = vmdk_gather_child_options,
.supports_backing = true,
.create_opts = &vmdk_create_opts,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 5885d9f..cfb8cf3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -130,6 +130,7 @@ struct BlockDriver {
int (*bdrv_make_empty)(BlockDriverState *bs);
void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
+ void (*bdrv_gather_child_options)(BlockDriverState *bs, QDict *target);
char *(*bdrv_dirname)(BlockDriverState *bs, Error **errp);
/* aio */
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 20/24] block: Generically refresh runtime options
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (18 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 19/24] block: Add BlockDriver.bdrv_gather_child_options Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 21/24] block: Purify .bdrv_refresh_filename() Max Reitz
` (3 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Instead of having every block driver which implements
bdrv_refresh_filename() copy all of the significant runtime options over
to bs->full_open_options, implement this process generically in
bdrv_refresh_filename().
This patch only adds this new generic implementation, it does not remove
the old functionality. This is done in a follow-up patch.
With this patch, some superfluous information (that should never have
been there) may be removed from some JSON filenames, as can be seen in
the change to iotest 110's reference output.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 116 ++++++++++++++++++++++++++++++++++++++++++++-
tests/qemu-iotests/110.out | 4 +-
2 files changed, 117 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 44412e1..9a84506 100644
--- a/block.c
+++ b/block.c
@@ -3936,6 +3936,92 @@ out:
return to_replace_bs;
}
+/**
+ * Iterates through the list of runtime option keys that are said to be
+ * "significant" for a BDS. An option is called "significant" if it changes a
+ * BDS's data. For example, the null block driver's "size" and "read-zeroes"
+ * options are significant, but its "latency-ns" option is not.
+ *
+ * If a key returned by this function ends with a dot, all options starting with
+ * that prefix are significant.
+ */
+static const char *const *significant_options(BlockDriverState *bs,
+ const char *const *curopt)
+{
+ static const char *const global_options[] = {
+ "driver", "filename", "base-directory", NULL
+ };
+
+ if (!curopt) {
+ return &global_options[0];
+ }
+
+ curopt++;
+ if (curopt == &global_options[ARRAY_SIZE(global_options) - 1] && bs->drv) {
+ curopt = bs->drv->sgfnt_runtime_opts;
+ }
+
+ return (curopt && *curopt) ? curopt : NULL;
+}
+
+/**
+ * Copies all significant runtime options from bs->options to the given QDict.
+ * The set of significant option keys is determined by invoking
+ * significant_options().
+ *
+ * Returns true iff any significant option was present in bs->options (and thus
+ * copied to the target QDict) with the exception of "filename" and "driver".
+ * The caller is expected to use this value to decide whether the existence of
+ * significant options prevents the generation of a plain filename.
+ */
+static bool append_significant_runtime_options(QDict *d, BlockDriverState *bs)
+{
+ bool found_any = false;
+ const char *const *option_name = NULL;
+
+ if (!bs->drv) {
+ return false;
+ }
+
+ while ((option_name = significant_options(bs, option_name))) {
+ bool option_given = false;
+
+ assert(strlen(*option_name) > 0);
+ if ((*option_name)[strlen(*option_name) - 1] != '.') {
+ QObject *entry = qdict_get(bs->options, *option_name);
+ if (!entry) {
+ continue;
+ }
+
+ qobject_incref(entry);
+ qdict_put_obj(d, *option_name, entry);
+ option_given = true;
+ } else {
+ const QDictEntry *entry;
+ for (entry = qdict_first(bs->options); entry;
+ entry = qdict_next(bs->options, entry))
+ {
+ if (strstart(qdict_entry_key(entry), *option_name, NULL)) {
+ qobject_incref(qdict_entry_value(entry));
+ qdict_put_obj(d, qdict_entry_key(entry),
+ qdict_entry_value(entry));
+ option_given = true;
+ }
+ }
+ }
+
+ /* While "driver" and "filename" need to be included in a JSON filename,
+ * their existence does not prohibit generation of a plain filename. */
+ if (!found_any && option_given &&
+ strcmp(*option_name, "driver") && strcmp(*option_name, "filename"))
+ {
+ found_any = true;
+ }
+ }
+
+ return found_any;
+}
+
static bool append_open_options(QDict *d, BlockDriverState *bs)
{
const QDictEntry *entry;
@@ -4094,9 +4180,37 @@ void bdrv_refresh_filename(BlockDriverState *bs)
bs->full_open_options = opts;
}
+ /* Gather the options QDict */
+ opts = qdict_new();
+ append_significant_runtime_options(opts, bs);
+
+ if (drv->bdrv_gather_child_options) {
+ /* Some block drivers may not want to present all of their children's
+ * options, or name them differently from BdrvChild.name */
+ drv->bdrv_gather_child_options(bs, opts);
+ } else {
+ QLIST_FOREACH(child, &bs->children, next) {
+ if (child->role == &child_backing && !bs->backing_overridden) {
+ /* We can skip the backing BDS if it has not been overridden */
+ continue;
+ }
+
+ QINCREF(child->bs->full_open_options);
+ qdict_put(opts, child->name, child->bs->full_open_options);
+ }
+
+ if (bs->backing_overridden && !bs->backing) {
+ /* Force no backing file */
+ qdict_put(opts, "backing", qstring_new());
+ }
+ }
+
+ QDECREF(bs->full_open_options);
+ bs->full_open_options = opts;
+
if (bs->exact_filename[0]) {
pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename);
- } else if (bs->full_open_options) {
+ } else {
QString *json = qobject_to_json(QOBJECT(bs->full_open_options));
snprintf(bs->filename, sizeof(bs->filename), "json:%s",
qstring_get_str(json));
diff --git a/tests/qemu-iotests/110.out b/tests/qemu-iotests/110.out
index e1845d8..7eb199d 100644
--- a/tests/qemu-iotests/110.out
+++ b/tests/qemu-iotests/110.out
@@ -22,12 +22,12 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=t.IMGFMT.b
=== Nodes without a common directory ===
-image: json:{"driver": "IMGFMT", "file": {"children": [{"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, {"driver": "file", "filename": "TEST_DIR/t.IMGFMT.copy"}], "driver": "quorum", "blkverify": false, "rewrite-corrupted": false, "vote-threshold": 1}}
+image: json:{"driver": "IMGFMT", "file": {"children": [{"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, {"driver": "file", "filename": "TEST_DIR/t.IMGFMT.copy"}], "driver": "quorum", "vote-threshold": 1}}
file format: IMGFMT
virtual size: 64M (67108864 bytes)
backing file: t.IMGFMT.base (cannot determine actual path)
-image: json:{"driver": "IMGFMT", "file": {"children": [{"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, {"driver": "file", "filename": "TEST_DIR/t.IMGFMT.copy"}], "driver": "quorum", "blkverify": false, "rewrite-corrupted": false, "vote-threshold": 1}}
+image: json:{"driver": "IMGFMT", "file": {"children": [{"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, {"driver": "file", "filename": "TEST_DIR/t.IMGFMT.copy"}], "driver": "quorum", "base-directory": "TEST_DIR/", "vote-threshold": 1}}
file format: IMGFMT
virtual size: 64M (67108864 bytes)
backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 21/24] block: Purify .bdrv_refresh_filename()
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (19 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 20/24] block: Generically refresh runtime options Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 22/24] block: Do not copy exact_filename from format file Max Reitz
` (2 subsequent siblings)
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Currently, BlockDriver.bdrv_refresh_filename() is supposed to both
refresh the filename (BDS.exact_filename) and set BDS.full_open_options.
Now that we have generic code in the central bdrv_refresh_filename() for
creating BDS.full_open_options, we can drop the latter part from all
BlockDriver.bdrv_refresh_filename() implementations.
This also means that we can drop all of the existing default code for
this from the global bdrv_refresh_filename() itself.
Furthermore, we now have to call BlockDriver.bdrv_refresh_filename()
after having set BDS.full_open_options, because the block driver's
implementation should now be allowed to depend on BDS.full_open_options
being set correctly.
Finally, with this patch we can drop the @options parameter from
BlockDriver.bdrv_refresh_filename().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 149 +++++++---------------------------------------
block/blkdebug.c | 44 ++++----------
block/blkverify.c | 17 +-----
block/nbd.c | 25 +-------
block/nfs.c | 41 +------------
block/null.c | 23 ++++---
block/quorum.c | 34 -----------
include/block/block_int.h | 2 +-
8 files changed, 52 insertions(+), 283 deletions(-)
diff --git a/block.c b/block.c
index 9a84506..187ce10 100644
--- a/block.c
+++ b/block.c
@@ -4022,47 +4022,6 @@ static bool append_significant_runtime_options(QDict *d, BlockDriverState *bs)
return found_any;
}
-static bool append_open_options(QDict *d, BlockDriverState *bs)
-{
- const QDictEntry *entry;
- QemuOptDesc *desc;
- BdrvChild *child;
- bool found_any = false;
- const char *p;
-
- for (entry = qdict_first(bs->options); entry;
- entry = qdict_next(bs->options, entry))
- {
- /* Exclude options for children */
- QLIST_FOREACH(child, &bs->children, next) {
- if (strstart(qdict_entry_key(entry), child->name, &p)
- && (!*p || *p == '.'))
- {
- break;
- }
- }
- if (child) {
- continue;
- }
-
- /* And exclude all non-driver-specific options */
- for (desc = bdrv_runtime_opts.desc; desc->name; desc++) {
- if (!strcmp(qdict_entry_key(entry), desc->name)) {
- break;
- }
- }
- if (desc->name) {
- continue;
- }
-
- qobject_incref(qdict_entry_value(entry));
- qdict_put_obj(d, qdict_entry_key(entry), qdict_entry_value(entry));
- found_any = true;
- }
-
- return found_any;
-}
-
/* Updates the following BDS fields:
* - exact_filename: A filename which may be used for opening a block device
* which (mostly) equals the given BDS (even without any
@@ -4080,6 +4039,8 @@ void bdrv_refresh_filename(BlockDriverState *bs)
BlockDriver *drv = bs->drv;
BdrvChild *child;
QDict *opts;
+ bool generate_json_filename; /* Whether our default implementation should
+ fill exact_filename (false) or not (true) */
if (!drv) {
return;
@@ -4095,94 +4056,10 @@ void bdrv_refresh_filename(BlockDriverState *bs)
}
}
- if (drv->bdrv_refresh_filename) {
- /* Obsolete information is of no use here, so drop the old file name
- * information before refreshing it */
- bs->exact_filename[0] = '\0';
- if (bs->full_open_options) {
- QDECREF(bs->full_open_options);
- bs->full_open_options = NULL;
- }
-
- opts = qdict_new();
- append_open_options(opts, bs);
- drv->bdrv_refresh_filename(bs, opts);
- QDECREF(opts);
- } else if (bs->file) {
- /* Try to reconstruct valid information from the underlying file */
- bool has_open_options;
-
- bs->exact_filename[0] = '\0';
- if (bs->full_open_options) {
- QDECREF(bs->full_open_options);
- bs->full_open_options = NULL;
- }
-
- opts = qdict_new();
- has_open_options = append_open_options(opts, bs);
- has_open_options |= bs->backing_overridden;
-
- /* If no specific options have been given for this BDS, the filename of
- * the underlying file should suffice for this one as well */
- if (bs->file->bs->exact_filename[0] && !has_open_options) {
- strcpy(bs->exact_filename, bs->file->bs->exact_filename);
- }
- /* Reconstructing the full options QDict is simple for most format block
- * drivers, as long as the full options are known for the underlying
- * file BDS. The full options QDict of that file BDS should somehow
- * contain a representation of the filename, therefore the following
- * suffices without querying the (exact_)filename of this BDS. */
- if (bs->file->bs->full_open_options &&
- (!bs->backing || bs->backing->bs->full_open_options))
- {
- qdict_put_obj(opts, "driver",
- QOBJECT(qstring_from_str(drv->format_name)));
- QINCREF(bs->file->bs->full_open_options);
- qdict_put_obj(opts, "file",
- QOBJECT(bs->file->bs->full_open_options));
-
- if (bs->backing) {
- QINCREF(bs->backing->bs->full_open_options);
- qdict_put(opts, "backing", bs->backing->bs->full_open_options);
- } else if (bs->backing_overridden && !bs->backing) {
- qdict_put(opts, "backing", qstring_new());
- }
-
- bs->full_open_options = opts;
- } else {
- QDECREF(opts);
- }
- } else if (!bs->full_open_options && qdict_size(bs->options)) {
- /* There is no underlying file BDS (at least referenced by BDS.file),
- * so the full options QDict should be equal to the options given
- * specifically for this block device when it was opened (plus the
- * driver specification).
- * Because those options don't change, there is no need to update
- * full_open_options when it's already set. */
-
- opts = qdict_new();
- append_open_options(opts, bs);
- qdict_put_obj(opts, "driver",
- QOBJECT(qstring_from_str(drv->format_name)));
-
- if (bs->exact_filename[0]) {
- /* This may not work for all block protocol drivers (some may
- * require this filename to be parsed), but we have to find some
- * default solution here, so just include it. If some block driver
- * does not support pure options without any filename at all or
- * needs some special format of the options QDict, it needs to
- * implement the driver-specific bdrv_refresh_filename() function.
- */
- qdict_put_obj(opts, "filename",
- QOBJECT(qstring_from_str(bs->exact_filename)));
- }
-
- bs->full_open_options = opts;
- }
-
/* Gather the options QDict */
opts = qdict_new();
- append_significant_runtime_options(opts, bs);
+ generate_json_filename = append_significant_runtime_options(opts, bs);
+ generate_json_filename |= bs->backing_overridden;
if (drv->bdrv_gather_child_options) {
/* Some block drivers may not want to present all of their children's
@@ -4208,6 +4085,24 @@ void bdrv_refresh_filename(BlockDriverState *bs)
QDECREF(bs->full_open_options);
bs->full_open_options = opts;
+ if (drv->bdrv_refresh_filename) {
+ /* Obsolete information is of no use here, so drop the old file name
+ * information before refreshing it */
+ bs->exact_filename[0] = '\0';
+
+ drv->bdrv_refresh_filename(bs);
+ } else if (bs->file) {
+ /* Try to reconstruct valid information from the underlying file */
+
+ bs->exact_filename[0] = '\0';
+
+ /* If no specific options have been given for this BDS, the filename of
+ * the underlying file should suffice for this one as well */
+ if (bs->file->bs->exact_filename[0] && !generate_json_filename) {
+ strcpy(bs->exact_filename, bs->file->bs->exact_filename);
+ }
+ }
+
if (bs->exact_filename[0]) {
pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename);
} else {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 5de6fad..6b18120 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -679,48 +679,28 @@ static int blkdebug_truncate(BlockDriverState *bs, int64_t offset)
return bdrv_truncate(bs->file->bs, offset);
}
-static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
+static void blkdebug_refresh_filename(BlockDriverState *bs)
{
BDRVBlkdebugState *s = bs->opaque;
- QDict *opts;
const QDictEntry *e;
- bool force_json = false;
- for (e = qdict_first(options); e; e = qdict_next(options, e)) {
- if (strcmp(qdict_entry_key(e), "config") &&
- strcmp(qdict_entry_key(e), "x-image"))
- {
- force_json = true;
- break;
- }
- }
-
- if (force_json && !bs->file->bs->full_open_options) {
- /* The config file cannot be recreated, so creating a plain filename
- * is impossible */
+ if (!bs->file->bs->exact_filename[0]) {
return;
}
- if (!force_json && bs->file->bs->exact_filename[0]) {
- snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "blkdebug:%s:%s", s->config_file ?: "",
- bs->file->bs->exact_filename);
- }
-
- opts = qdict_new();
- qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("blkdebug")));
-
- QINCREF(bs->file->bs->full_open_options);
- qdict_put_obj(opts, "image", QOBJECT(bs->file->bs->full_open_options));
-
- for (e = qdict_first(options); e; e = qdict_next(options, e)) {
- if (strcmp(qdict_entry_key(e), "x-image")) {
- qobject_incref(qdict_entry_value(e));
- qdict_put_obj(opts, qdict_entry_key(e), qdict_entry_value(e));
+ for (e = qdict_first(bs->full_open_options); e;
+ e = qdict_next(bs->full_open_options, e))
+ {
+ if (strcmp(qdict_entry_key(e), "config") &&
+ strcmp(qdict_entry_key(e), "image") &&
+ strcmp(qdict_entry_key(e), "driver"))
+ {
+ return;
}
}
- bs->full_open_options = opts;
+ snprintf(bs->exact_filename, sizeof(bs->exact_filename), "blkdebug:%s:%s",
+ s->config_file ?: "", bs->file->bs->exact_filename);
}
static void blkdebug_refresh_limits(BlockDriverState *bs, Error **errp)
diff --git a/block/blkverify.c b/block/blkverify.c
index d6e57e0..caf267c 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -289,25 +289,10 @@ static bool blkverify_recurse_is_first_non_filter(BlockDriverState *bs,
return bdrv_recurse_is_first_non_filter(s->test_file->bs, candidate);
}
-static void blkverify_refresh_filename(BlockDriverState *bs, QDict *options)
+static void blkverify_refresh_filename(BlockDriverState *bs)
{
BDRVBlkverifyState *s = bs->opaque;
- if (bs->file->bs->full_open_options
- && s->test_file->bs->full_open_options)
- {
- QDict *opts = qdict_new();
- qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("blkverify")));
-
- QINCREF(bs->file->bs->full_open_options);
- qdict_put_obj(opts, "raw", QOBJECT(bs->file->bs->full_open_options));
- QINCREF(s->test_file->bs->full_open_options);
- qdict_put_obj(opts, "test",
- QOBJECT(s->test_file->bs->full_open_options));
-
- bs->full_open_options = opts;
- }
-
if (bs->file->bs->exact_filename[0]
&& s->test_file->bs->exact_filename[0])
{
diff --git a/block/nbd.c b/block/nbd.c
index 1307989..ea5d7a8 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -499,12 +499,9 @@ static void nbd_attach_aio_context(BlockDriverState *bs,
nbd_client_attach_aio_context(bs, new_context);
}
-static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
+static void nbd_refresh_filename(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
- QDict *opts = qdict_new();
- QObject *saddr_qdict;
- Visitor *ov;
const char *host = NULL, *port = NULL, *path = NULL;
if (s->saddr->type == SOCKET_ADDRESS_KIND_INET) {
@@ -517,8 +514,6 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
path = s->saddr->u.q_unix.data->path;
}
- qdict_put(opts, "driver", qstring_from_str("nbd"));
-
if (path && s->export) {
snprintf(bs->exact_filename, sizeof(bs->exact_filename),
"nbd+unix:///%s?socket=%s", s->export, path);
@@ -532,24 +527,6 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
snprintf(bs->exact_filename, sizeof(bs->exact_filename),
"nbd://%s:%s", host, port);
}
-
- ov = qobject_output_visitor_new(&saddr_qdict);
- visit_type_SocketAddress(ov, NULL, &s->saddr, &error_abort);
- visit_complete(ov, &saddr_qdict);
- visit_free(ov);
- assert(qobject_type(saddr_qdict) == QTYPE_QDICT);
-
- qdict_put_obj(opts, "server", saddr_qdict);
-
- if (s->export) {
- qdict_put(opts, "export", qstring_from_str(s->export));
- }
- if (s->tlscredsid) {
- qdict_put(opts, "tls-creds", qstring_from_str(s->tlscredsid));
- }
-
- qdict_flatten(opts);
- bs->full_open_options = opts;
}
static char *nbd_dirname(BlockDriverState *bs, Error **errp)
diff --git a/block/nfs.c b/block/nfs.c
index 6ffdede..eac4010 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -767,14 +767,9 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
return 0;
}
-static void nfs_refresh_filename(BlockDriverState *bs, QDict *options)
+static void nfs_refresh_filename(BlockDriverState *bs)
{
NFSClient *client = bs->opaque;
- QDict *opts = qdict_new();
- QObject *server_qdict;
- Visitor *ov;
-
- qdict_put(opts, "driver", qstring_from_str("nfs"));
if (client->uid && !client->gid) {
snprintf(bs->exact_filename, sizeof(bs->exact_filename),
@@ -792,40 +787,6 @@ static void nfs_refresh_filename(BlockDriverState *bs, QDict *options)
snprintf(bs->exact_filename, sizeof(bs->exact_filename),
"nfs://%s%s", client->server->host, client->path);
}
-
- ov = qobject_output_visitor_new(&server_qdict);
- visit_type_NFSServer(ov, NULL, &client->server, &error_abort);
- visit_complete(ov, &server_qdict);
- assert(qobject_type(server_qdict) == QTYPE_QDICT);
-
- qdict_put_obj(opts, "server", server_qdict);
- qdict_put(opts, "path", qstring_from_str(client->path));
-
- if (client->uid) {
- qdict_put(opts, "uid", qint_from_int(client->uid));
- }
- if (client->gid) {
- qdict_put(opts, "gid", qint_from_int(client->gid));
- }
- if (client->tcp_syncnt) {
- qdict_put(opts, "tcp-syncnt",
- qint_from_int(client->tcp_syncnt));
- }
- if (client->readahead) {
- qdict_put(opts, "readahead",
- qint_from_int(client->readahead));
- }
- if (client->pagecache) {
- qdict_put(opts, "pagecache",
- qint_from_int(client->pagecache));
- }
- if (client->debug) {
- qdict_put(opts, "debug", qint_from_int(client->debug));
- }
-
- visit_free(ov);
- qdict_flatten(opts);
- bs->full_open_options = opts;
}
static char *nfs_dirname(BlockDriverState *bs, Error **errp)
diff --git a/block/null.c b/block/null.c
index 16ef8b2..fb03fb9 100644
--- a/block/null.c
+++ b/block/null.c
@@ -222,18 +222,23 @@ static int64_t coroutine_fn null_co_get_block_status(BlockDriverState *bs,
}
}
-static void null_refresh_filename(BlockDriverState *bs, QDict *opts)
+static void null_refresh_filename(BlockDriverState *bs)
{
- QINCREF(opts);
- qdict_del(opts, "filename");
-
- if (!qdict_size(opts)) {
- snprintf(bs->exact_filename, sizeof(bs->exact_filename), "%s://",
- bs->drv->format_name);
+ const QDictEntry *e;
+
+ for (e = qdict_first(bs->full_open_options); e;
+ e = qdict_next(bs->full_open_options, e))
+ {
+ /* These options can be ignored */
+ if (strcmp(qdict_entry_key(e), "filename") &&
+ strcmp(qdict_entry_key(e), "driver"))
+ {
+ return;
+ }
}
- qdict_put(opts, "driver", qstring_from_str(bs->drv->format_name));
- bs->full_open_options = opts;
+ snprintf(bs->exact_filename, sizeof(bs->exact_filename), "%s://",
+ bs->drv->format_name);
}
static const char *const null_sgfnt_runtime_opts[] = {
diff --git a/block/quorum.c b/block/quorum.c
index 126127c..09c4add 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1050,39 +1050,6 @@ static void quorum_del_child(BlockDriverState *bs, BdrvChild *child,
bdrv_drained_end(bs);
}
-static void quorum_refresh_filename(BlockDriverState *bs, QDict *options)
-{
- BDRVQuorumState *s = bs->opaque;
- QDict *opts;
- QList *children;
- int i;
-
- for (i = 0; i < s->num_children; i++) {
- if (!s->children[i]->bs->full_open_options) {
- return;
- }
- }
-
- children = qlist_new();
- for (i = 0; i < s->num_children; i++) {
- QINCREF(s->children[i]->bs->full_open_options);
- qlist_append_obj(children,
- QOBJECT(s->children[i]->bs->full_open_options));
- }
-
- opts = qdict_new();
- qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("quorum")));
- qdict_put_obj(opts, QUORUM_OPT_VOTE_THRESHOLD,
- QOBJECT(qint_from_int(s->threshold)));
- qdict_put_obj(opts, QUORUM_OPT_BLKVERIFY,
- QOBJECT(qbool_from_bool(s->is_blkverify)));
- qdict_put_obj(opts, QUORUM_OPT_REWRITE,
- QOBJECT(qbool_from_bool(s->rewrite_corrupted)));
- qdict_put_obj(opts, "children", QOBJECT(children));
-
- bs->full_open_options = opts;
-}
-
static void quorum_gather_child_options(BlockDriverState *bs, QDict *target)
{
BDRVQuorumState *s = bs->opaque;
@@ -1130,7 +1097,6 @@ static BlockDriver bdrv_quorum = {
.bdrv_file_open = quorum_open,
.bdrv_close = quorum_close,
- .bdrv_refresh_filename = quorum_refresh_filename,
.bdrv_gather_child_options = quorum_gather_child_options,
.bdrv_dirname = quorum_dirname,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index cfb8cf3..0c65c58 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -129,7 +129,7 @@ struct BlockDriver {
int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
int (*bdrv_make_empty)(BlockDriverState *bs);
- void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
+ void (*bdrv_refresh_filename)(BlockDriverState *bs);
void (*bdrv_gather_child_options)(BlockDriverState *bs, QDict *target);
char *(*bdrv_dirname)(BlockDriverState *bs, Error **errp);
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 22/24] block: Do not copy exact_filename from format file
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (20 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 21/24] block: Purify .bdrv_refresh_filename() Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 23/24] block/curl: Implement bdrv_refresh_filename() Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 24/24] block/null: Generate filename even with latency-ns Max Reitz
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
If the a format BDS's file BDS is in turn a format BDS, we cannot simply
use the same filename, because when opening a BDS tree based on a
filename alone, qemu will create only one format node on top of one
protocol node (disregarding a potential backing file).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 187ce10..d2917f1 100644
--- a/block.c
+++ b/block.c
@@ -4096,9 +4096,20 @@ void bdrv_refresh_filename(BlockDriverState *bs)
bs->exact_filename[0] = '\0';
- /* If no specific options have been given for this BDS, the filename of
- * the underlying file should suffice for this one as well */
- if (bs->file->bs->exact_filename[0] && !generate_json_filename) {
+ /* We can use the underlying file's filename if:
+ * - it has a filename,
+ * - the file is a protocol BDS, and
+ * - opening that file (as this BDS's format) will automatically create
+ * the BDS tree we have right now, that is:
+ * - the user did not significantly change this BDS's behavior with
+ * some explicit options
+ * - no non-file child of this BDS has been overridden by the user
+ * Both of these conditions are represented by generate_json_filename.
+ */
+ if (bs->file->bs->exact_filename[0] &&
+ (bs->file->bs->open_flags & BDRV_O_PROTOCOL) &&
+ !generate_json_filename)
+ {
strcpy(bs->exact_filename, bs->file->bs->exact_filename);
}
}
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 23/24] block/curl: Implement bdrv_refresh_filename()
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (21 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 22/24] block: Do not copy exact_filename from format file Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 24/24] block/null: Generate filename even with latency-ns Max Reitz
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/curl.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/block/curl.c b/block/curl.c
index 0777d5b..4725caf 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -872,6 +872,20 @@ static int64_t curl_getlength(BlockDriverState *bs)
return s->len;
}
+static void curl_refresh_filename(BlockDriverState *bs)
+{
+ BDRVCURLState *s = bs->opaque;
+
+ if (!s->sslverify || s->cookie ||
+ s->username || s->password || s->proxyusername || s->proxypassword)
+ {
+ return;
+ }
+
+ pstrcpy(bs->exact_filename, sizeof(bs->exact_filename), s->url);
+}
+
+
static const char *const curl_sgfnt_runtime_opts[] = {
CURL_BLOCK_OPT_URL,
CURL_BLOCK_OPT_SSLVERIFY,
@@ -899,6 +913,7 @@ static BlockDriver bdrv_http = {
.bdrv_detach_aio_context = curl_detach_aio_context,
.bdrv_attach_aio_context = curl_attach_aio_context,
+ .bdrv_refresh_filename = curl_refresh_filename,
.sgfnt_runtime_opts = curl_sgfnt_runtime_opts,
};
@@ -917,6 +932,7 @@ static BlockDriver bdrv_https = {
.bdrv_detach_aio_context = curl_detach_aio_context,
.bdrv_attach_aio_context = curl_attach_aio_context,
+ .bdrv_refresh_filename = curl_refresh_filename,
.sgfnt_runtime_opts = curl_sgfnt_runtime_opts,
};
@@ -935,6 +951,7 @@ static BlockDriver bdrv_ftp = {
.bdrv_detach_aio_context = curl_detach_aio_context,
.bdrv_attach_aio_context = curl_attach_aio_context,
+ .bdrv_refresh_filename = curl_refresh_filename,
.sgfnt_runtime_opts = curl_sgfnt_runtime_opts,
};
@@ -953,6 +970,7 @@ static BlockDriver bdrv_ftps = {
.bdrv_detach_aio_context = curl_detach_aio_context,
.bdrv_attach_aio_context = curl_attach_aio_context,
+ .bdrv_refresh_filename = curl_refresh_filename,
.sgfnt_runtime_opts = curl_sgfnt_runtime_opts,
};
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH v2 24/24] block/null: Generate filename even with latency-ns
2016-11-27 1:55 [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues Max Reitz
` (22 preceding siblings ...)
2016-11-27 1:56 ` [Qemu-devel] [PATCH v2 23/24] block/curl: Implement bdrv_refresh_filename() Max Reitz
@ 2016-11-27 1:56 ` Max Reitz
23 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2016-11-27 1:56 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Alberto Garcia, Eric Blake
While we cannot represent the latency-ns option in a filename, it is not
a significant option so not being able to should not stop us from
generating a filename nonetheless.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/null.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/null.c b/block/null.c
index fb03fb9..f665bcf 100644
--- a/block/null.c
+++ b/block/null.c
@@ -231,7 +231,8 @@ static void null_refresh_filename(BlockDriverState *bs)
{
/* These options can be ignored */
if (strcmp(qdict_entry_key(e), "filename") &&
- strcmp(qdict_entry_key(e), "driver"))
+ strcmp(qdict_entry_key(e), "driver") &&
+ strcmp(qdict_entry_key(e), NULL_OPT_LATENCY))
{
return;
}
--
2.10.2
^ permalink raw reply related [flat|nested] 26+ messages in thread