* [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues
@ 2016-04-26 21:31 Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 01/19] block: Use children list in bdrv_refresh_filename Max Reitz
` (19 more replies)
0 siblings, 20 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:31 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
(Fun fact: This series has been lying around on my disk since last
November. I guess I forgot to send it because I still wanted to review
it before sending it out, and that it what I forgot to do. Well, and
now Berto noticed that we ought to fix something.)
There are some issues regarding filename generation right now:
- You always get a JSON filename if you set even a single qcow2-specific
runtime options (as long as it does not have a dot in it, which is a
bug, too, but here it is working in our favor...). That is not nice
and actually breaks the usage of backing files with relative
filenames with such qcow2 BDS.
- As hinted above, you cannot use relative backing filenames with BDS
that have a JSON filename only, even though qemu might be able to
obtain the directory name by walking through the BDS graph to the
protocol level.
- Overriding the backing file at runtime should invalidate the filename
because it actually changes the BDS's data. Therefore, we need to
force a JSON filename in that case, containing the backing file
override.
- Much of our code assumes paths never to exceed PATH_MAX in length.
This is wrong, at least because of JSON filenames. This should be
fixed wherever the opportunity arises.
This series addresses the first issue by splitting off code from
bdrv_refresh_filename() into bdrv_default_refresh_format_filename() and
bdrv_default_refresh_protocol_filename(). The former will take a boolean
signifying whether the driver-specific runtime options are actually
significant for the BDS data (guest-visible content). For qcow2, they
never are, therefore, the qcow2 driver has to implement
bdrv_refresh_filename() by invoking
bdrv_default_refresh_format_filename() with the boolean set
appropriately (deviating from the actual default implementation which
has to assume that all driver-specific runtime options are significant).
[Patches 4 -- 7]
The second issue is addressed by introducing bdrv_dirname() which
returns the directory of a specific BDS. By default, this is obtained by
walking through the BDS graph to the protocol level and processing that
BDS's filename (exact_filename, to be exact).
This behavior can be overridden by any block driver along the path
implementing bdrv_dirname() itself, or by the user explicitly specifying
the 'base-directory' option for any BDS node.
[Patches 11, 13 -- 18]
The third issue is addressed by paying respect to the backing file
options, and noting whether any of the backing file's options have been
overridden in a way that make opening the backing file implicitly (using
the filename specified in the overlay file) impossible.
[Patches 2 -- 3]
The fourth issue has been addressed as far as it made sense to do it
along fixing the second one.
[Patch 8 -- 10]
Furthermore, there are two fixes to code touched in this series.
[Patches 1 and 12]
Max Reitz (19):
block: Use children list in bdrv_refresh_filename
block: Add BDS.backing_overridden
block: Respect backing bs in bdrv_refresh_filename
block: Add bdrv_default_refresh_format_filename
block: Add bdrv_default_refresh_protocol_filename
block: Make bdrv_default_refresh_format_filename public
qcow2: Implement bdrv_refresh_filename()
block: Make path_combine() return the path
block: bdrv_get_full_backing_filename_from_...'s ret. val.
block: bdrv_get_full_backing_filename's ret. val.
block: Add bdrv_make_absolute_filename()
block: Fix bdrv_find_backing_image()
block: Add bdrv_dirname()
blkverify: Make bdrv_dirname() return NULL
quorum: Make bdrv_dirname() return NULL
block/nbd: Implement bdrv_dirname()
block: Use bdrv_dirname() for relative filenames
block: Add 'base-directory' BDS option
iotests: Add quorum case to test 110
block.c | 340 +++++++++++++++++++++++++++---------------
block/blkverify.c | 13 +-
block/nbd.c | 29 ++++
block/qapi.c | 12 +-
block/qcow2.c | 8 +
block/quorum.c | 12 +-
block/vmdk.c | 11 +-
include/block/block.h | 15 +-
include/block/block_int.h | 7 +
qapi/block-core.json | 9 ++
tests/qemu-iotests/051.out | 8 +-
tests/qemu-iotests/051.pc.out | 8 +-
tests/qemu-iotests/110 | 51 ++++++-
tests/qemu-iotests/110.out | 11 +-
14 files changed, 372 insertions(+), 162 deletions(-)
--
2.8.0
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 01/19] block: Use children list in bdrv_refresh_filename
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-27 0:36 ` Eric Blake
2016-05-31 7:35 ` Alberto Garcia
2016-04-26 21:32 ` [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden Max Reitz
` (18 subsequent siblings)
19 siblings, 2 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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>
---
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 d4939b4..e349e83 100644
--- a/block.c
+++ b/block.c
@@ -3885,16 +3885,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 9414b7a..f445e3e 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -313,9 +313,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 da15465..98c6588 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1028,7 +1028,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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 01/19] block: Use children list in bdrv_refresh_filename Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-27 0:37 ` Eric Blake
2016-05-02 15:35 ` Kevin Wolf
2016-04-26 21:32 ` [Qemu-devel] [PATCH 03/19] block: Respect backing bs in bdrv_refresh_filename Max Reitz
` (17 subsequent siblings)
19 siblings, 2 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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>
---
block.c | 2 ++
include/block/block_int.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/block.c b/block.c
index e349e83..e178488 100644
--- a/block.c
+++ b/block.c
@@ -1299,6 +1299,7 @@ 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")) {
+ bs->backing_overridden = true;
backing_filename[0] = '\0';
} else if (bs->backing_file[0] == '\0' && qdict_size(options) == 0) {
QDECREF(options);
@@ -1589,6 +1590,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 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");
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 10d8759..d73e9ce 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -409,6 +409,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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 03/19] block: Respect backing bs in bdrv_refresh_filename
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 01/19] block: Use children list in bdrv_refresh_filename Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-05-02 15:36 ` Kevin Wolf
2016-04-26 21:32 ` [Qemu-devel] [PATCH 04/19] block: Add bdrv_default_refresh_format_filename Max Reitz
` (16 subsequent siblings)
19 siblings, 1 reply; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 | 14 +++++++++++++-
tests/qemu-iotests/051.out | 8 ++++----
tests/qemu-iotests/051.pc.out | 8 ++++----
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index e178488..aff9ea3 100644
--- a/block.c
+++ b/block.c
@@ -3925,6 +3925,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 */
@@ -3936,13 +3937,24 @@ 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_obj(opts, "backing",
+ QOBJECT(bs->backing->bs->full_open_options));
+ } else if (bs->backing_overridden && !bs->backing) {
+ qdict_put_obj(opts, "backing", QOBJECT(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 408d613..d936798 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 ec6d222..cb899ce 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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 04/19] block: Add bdrv_default_refresh_format_filename
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (2 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 03/19] block: Respect backing bs in bdrv_refresh_filename Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-06-20 13:16 ` Alberto Garcia
2016-04-26 21:32 ` [Qemu-devel] [PATCH 05/19] block: Add bdrv_default_refresh_protocol_filename Max Reitz
` (15 subsequent siblings)
19 siblings, 1 reply; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
Split off the default code for format BDS from bdrv_refresh_filename()
into an own function, first because it is nicer this way, and second
because this will allow block drivers to reuse this function in their
own specific implementation of bdrv_refresh_filename().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 95 +++++++++++++++++++++++++++++++++++------------------------------
1 file changed, 51 insertions(+), 44 deletions(-)
diff --git a/block.c b/block.c
index aff9ea3..447468c 100644
--- a/block.c
+++ b/block.c
@@ -3872,6 +3872,56 @@ static bool append_open_options(QDict *d, BlockDriverState *bs)
return found_any;
}
+static void bdrv_default_refresh_format_filename(BlockDriverState *bs)
+{
+ BlockDriver *drv = bs->drv;
+ QDict *opts;
+ 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_obj(opts, "backing",
+ QOBJECT(bs->backing->bs->full_open_options));
+ } else if (bs->backing_overridden && !bs->backing) {
+ qdict_put_obj(opts, "backing", QOBJECT(qstring_new()));
+ }
+
+ bs->full_open_options = opts;
+ } else {
+ QDECREF(opts);
+ }
+}
+
/* 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
@@ -3915,50 +3965,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
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_obj(opts, "backing",
- QOBJECT(bs->backing->bs->full_open_options));
- } else if (bs->backing_overridden && !bs->backing) {
- qdict_put_obj(opts, "backing", QOBJECT(qstring_new()));
- }
-
- bs->full_open_options = opts;
- } else {
- QDECREF(opts);
- }
+ bdrv_default_refresh_format_filename(bs);
} 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
--
2.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 05/19] block: Add bdrv_default_refresh_protocol_filename
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (3 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 04/19] block: Add bdrv_default_refresh_format_filename Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-06-20 13:18 ` Alberto Garcia
2016-04-26 21:32 ` [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public Max Reitz
` (14 subsequent siblings)
19 siblings, 1 reply; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
Split off the default code for protocol BDS from bdrv_refresh_filename()
into an own function, just as it has been done for format BDS.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 58 +++++++++++++++++++++++++++++++++-------------------------
1 file changed, 33 insertions(+), 25 deletions(-)
diff --git a/block.c b/block.c
index 447468c..511a0b1 100644
--- a/block.c
+++ b/block.c
@@ -3922,6 +3922,38 @@ static void bdrv_default_refresh_format_filename(BlockDriverState *bs)
}
}
+static void bdrv_default_refresh_protocol_filename(BlockDriverState *bs)
+{
+ BlockDriver *drv = bs->drv;
+ QDict *opts;
+
+ /* 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;
+}
+
/* 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
@@ -3967,31 +3999,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
/* Try to reconstruct valid information from the underlying file */
bdrv_default_refresh_format_filename(bs);
} 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;
+ bdrv_default_refresh_protocol_filename(bs);
}
if (bs->exact_filename[0]) {
--
2.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (4 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 05/19] block: Add bdrv_default_refresh_protocol_filename Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-05-02 15:36 ` Kevin Wolf
2016-04-26 21:32 ` [Qemu-devel] [PATCH 07/19] qcow2: Implement bdrv_refresh_filename() Max Reitz
` (13 subsequent siblings)
19 siblings, 1 reply; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
In order to allow block drivers to use that function, it needs to be
public. In order to be useful, it needs to take a parameter which allows
the caller to specify whether the runtime options allowed by the block
driver are actually significant for the guest-visible BDS content.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 9 ++++++---
include/block/block_int.h | 3 +++
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 511a0b1..b3ff08f 100644
--- a/block.c
+++ b/block.c
@@ -3872,7 +3872,10 @@ static bool append_open_options(QDict *d, BlockDriverState *bs)
return found_any;
}
-static void bdrv_default_refresh_format_filename(BlockDriverState *bs)
+/* @options_significant must be true if any of the driver-specific runtime
+ * options may change the guest-visible content of the BDS */
+void bdrv_default_refresh_format_filename(BlockDriverState *bs,
+ bool options_significant)
{
BlockDriver *drv = bs->drv;
QDict *opts;
@@ -3885,7 +3888,7 @@ static void bdrv_default_refresh_format_filename(BlockDriverState *bs)
}
opts = qdict_new();
- has_open_options = append_open_options(opts, bs);
+ has_open_options = append_open_options(opts, bs) && options_significant;
has_open_options |= bs->backing_overridden;
/* If no specific options have been given for this BDS, the filename of
@@ -3997,7 +4000,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
QDECREF(opts);
} else if (bs->file) {
/* Try to reconstruct valid information from the underlying file */
- bdrv_default_refresh_format_filename(bs);
+ bdrv_default_refresh_format_filename(bs, true);
} else if (!bs->full_open_options && qdict_size(bs->options)) {
bdrv_default_refresh_protocol_filename(bs);
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index d73e9ce..eb3665a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -532,6 +532,9 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
void bdrv_set_io_limits(BlockDriverState *bs,
ThrottleConfig *cfg);
+void bdrv_default_refresh_format_filename(BlockDriverState *bs,
+ bool options_significant);
+
/**
* bdrv_add_before_write_notifier:
--
2.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 07/19] qcow2: Implement bdrv_refresh_filename()
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (5 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 08/19] block: Make path_combine() return the path Max Reitz
` (12 subsequent siblings)
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
Implement this function by invoking
bdrv_default_refresh_format_filename(bs, false). None of the qcow2
runtime options change the guest-visible state of a BDS.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index 470734b..bcbf94e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3256,6 +3256,13 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset,
s->signaled_corruption = true;
}
+static void qcow2_refresh_filename(BlockDriverState *bs, QDict *opts)
+{
+ (void)opts;
+
+ bdrv_default_refresh_format_filename(bs, false);
+}
+
static QemuOptsList qcow2_create_opts = {
.name = "qcow2-create-opts",
.head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head),
@@ -3346,6 +3353,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_snapshot_load_tmp = qcow2_snapshot_load_tmp,
.bdrv_get_info = qcow2_get_info,
.bdrv_get_specific_info = qcow2_get_specific_info,
+ .bdrv_refresh_filename = qcow2_refresh_filename,
.bdrv_save_vmstate = qcow2_save_vmstate,
.bdrv_load_vmstate = qcow2_load_vmstate,
--
2.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 08/19] block: Make path_combine() return the path
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (6 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 07/19] qcow2: Implement bdrv_refresh_filename() Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-06-20 13:44 ` Alberto Garcia
2016-04-26 21:32 ` [Qemu-devel] [PATCH 09/19] block: bdrv_get_full_backing_filename_from_...'s ret. val Max Reitz
` (11 subsequent siblings)
19 siblings, 1 reply; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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>
---
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 b3ff08f..41f3c92 100644
--- a/block.c
+++ b/block.c
@@ -146,48 +146,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,
@@ -203,7 +214,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);
}
}
@@ -3147,8 +3158,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)) {
@@ -3157,8 +3168,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 45f9d3c..142de20 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -847,8 +847,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 3a73137..4604465 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -443,9 +443,7 @@ int bdrv_is_snapshot(BlockDriverState *bs);
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_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
--
2.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 09/19] block: bdrv_get_full_backing_filename_from_...'s ret. val.
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (7 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 08/19] block: Make path_combine() return the path Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 10/19] block: bdrv_get_full_backing_filename's " Max Reitz
` (10 subsequent siblings)
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 41f3c92..a12b1d3 100644
--- a/block.c
+++ b/block.c
@@ -201,20 +201,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);
}
}
@@ -222,9 +222,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)
@@ -3551,16 +3557,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 142de20..58fe380 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1934,12 +1934,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 4604465..c6c00dd 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -435,10 +435,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 bdrv_is_snapshot(BlockDriverState *bs);
int path_has_protocol(const char *path);
--
2.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 10/19] block: bdrv_get_full_backing_filename's ret. val.
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (8 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 09/19] block: bdrv_get_full_backing_filename_from_...'s ret. val Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 11/19] block: Add bdrv_make_absolute_filename() Max Reitz
` (9 subsequent siblings)
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 | 24 ++++++++----------------
block/qapi.c | 12 ++----------
include/block/block.h | 3 +--
3 files changed, 11 insertions(+), 28 deletions(-)
diff --git a/block.c b/block.c
index a12b1d3..083f4ae 100644
--- a/block.c
+++ b/block.c
@@ -218,19 +218,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)
@@ -1289,7 +1283,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;
@@ -1317,13 +1311,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")) {
bs->backing_overridden = true;
- 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);
@@ -1344,8 +1337,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
}
backing_hd = NULL;
- ret = bdrv_open_inherit(&backing_hd,
- *backing_filename ? backing_filename : NULL,
+ ret = bdrv_open_inherit(&backing_hd, backing_filename,
reference, options, 0, bs, &child_backing,
errp);
if (ret < 0) {
diff --git a/block/qapi.c b/block/qapi.c
index c5f6ba6..a0fb7fb 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 c6c00dd..83f9f31 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -433,8 +433,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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 11/19] block: Add bdrv_make_absolute_filename()
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (9 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 10/19] block: bdrv_get_full_backing_filename's " Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 12/19] block: Fix bdrv_find_backing_image() Max Reitz
` (8 subsequent siblings)
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 083f4ae..49dc2cb 100644
--- a/block.c
+++ b/block.c
@@ -218,15 +218,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)
{
bdrv_setup_io_funcs(bdrv);
--
2.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 12/19] block: Fix bdrv_find_backing_image()
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (10 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 11/19] block: Add bdrv_make_absolute_filename() Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 13/19] block: Add bdrv_dirname() Max Reitz
` (7 subsequent siblings)
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 49dc2cb..6e89abd 100644
--- a/block.c
+++ b/block.c
@@ -192,15 +192,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)
@@ -3147,7 +3138,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);
@@ -3163,22 +3153,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;
@@ -3189,7 +3188,6 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
g_free(filename_full);
g_free(backing_file_full);
- g_free(filename_tmp);
return retval;
}
--
2.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 13/19] block: Add bdrv_dirname()
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (11 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 12/19] block: Fix bdrv_find_backing_image() Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 14/19] blkverify: Make bdrv_dirname() return NULL Max Reitz
` (6 subsequent siblings)
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 6e89abd..23962ce 100644
--- a/block.c
+++ b/block.c
@@ -4028,3 +4028,29 @@ void bdrv_refresh_filename(BlockDriverState *bs)
QDECREF(json);
}
}
+
+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;
+}
diff --git a/include/block/block.h b/include/block/block.h
index 83f9f31..b0fe5dc 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -437,6 +437,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 bdrv_is_snapshot(BlockDriverState *bs);
int path_has_protocol(const char *path);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index eb3665a..087d9ac 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -137,6 +137,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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 14/19] blkverify: Make bdrv_dirname() return NULL
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (12 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 13/19] block: Add bdrv_dirname() Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 15/19] quorum: " Max Reitz
` (5 subsequent siblings)
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 f445e3e..35d080f 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -338,6 +338,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",
@@ -348,6 +357,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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 15/19] quorum: Make bdrv_dirname() return NULL
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (13 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 14/19] blkverify: Make bdrv_dirname() return NULL Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 16/19] block/nbd: Implement bdrv_dirname() Max Reitz
` (4 subsequent siblings)
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 98c6588..abe2148 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1053,6 +1053,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",
@@ -1062,6 +1072,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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 16/19] block/nbd: Implement bdrv_dirname()
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (14 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 15/19] quorum: " Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-05-02 15:36 ` Kevin Wolf
2016-04-26 21:32 ` [Qemu-devel] [PATCH 17/19] block: Use bdrv_dirname() for relative filenames Max Reitz
` (3 subsequent siblings)
19 siblings, 1 reply; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/block/nbd.c b/block/nbd.c
index f7ea3b3..64534bb 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -469,6 +469,32 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
bs->full_open_options = opts;
}
+static char *nbd_dirname(BlockDriverState *bs, Error **errp)
+{
+ const char *path = qdict_get_try_str(bs->options, "path");
+ const char *host = qdict_get_try_str(bs->options, "host");
+ const char *port = qdict_get_try_str(bs->options, "port");
+ const char *export = qdict_get_try_str(bs->options, "export");
+
+ if (path && export) {
+ return g_strdup_printf("nbd:unix:%s:exportname=%s/", path, export);
+ } else if (path && !export) {
+ return g_strdup_printf("nbd:unix:%s:exportname=", path);
+ } else if (!path && export && port) {
+ return g_strdup_printf("nbd://%s:%s/%s/", host, port, export);
+ } else if (!path && export && !port) {
+ return g_strdup_printf("nbd://%s/%s/", host, export);
+ } else if (!path && !export && port) {
+ return g_strdup_printf("nbd://%s:%s/", host, port);
+ } else if (!path && !export && !port) {
+ return g_strdup_printf("nbd://%s/", host);
+ }
+
+ 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",
@@ -487,6 +513,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 = {
@@ -507,6 +534,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 = {
@@ -527,6 +555,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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 17/19] block: Use bdrv_dirname() for relative filenames
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (15 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 16/19] block/nbd: Implement bdrv_dirname() Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 18/19] block: Add 'base-directory' BDS option Max Reitz
` (2 subsequent siblings)
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 23962ce..a909b7a 100644
--- a/block.c
+++ b/block.c
@@ -212,12 +212,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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 18/19] block: Add 'base-directory' BDS option
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (16 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 17/19] block: Use bdrv_dirname() for relative filenames Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 19/19] iotests: Add quorum case to test 110 Max Reitz
2016-11-02 15:00 ` [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Alberto Garcia
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 | 13 +++++++++++++
include/block/block_int.h | 2 ++
qapi/block-core.json | 9 +++++++++
3 files changed, 24 insertions(+)
diff --git a/block.c b/block.c
index a909b7a..2086c6d 100644
--- a/block.c
+++ b/block.c
@@ -889,6 +889,11 @@ static QemuOptsList bdrv_runtime_opts = {
.type = QEMU_OPT_BOOL,
.help = "Ignore flush requests",
},
+ {
+ .name = "base-directory",
+ .type = QEMU_OPT_STRING,
+ .help = "String to be prepended to filenames relative to this BDS to make them absolute",
+ },
{ /* end of list */ }
},
};
@@ -948,6 +953,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
goto fail_opts;
}
+ bs->dirname = g_strdup(qemu_opt_get(opts, "base-directory"));
+
bs->request_alignment = 512;
bs->zero_beyond_eof = true;
bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
@@ -2359,6 +2366,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);
@@ -4043,6 +4052,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 087d9ac..863caf2 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -415,6 +415,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 1d09079..85b8e06 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2045,6 +2045,14 @@
# @node-name: #optional the name of a block driver state node (Since 2.0).
# This option is required on the top level of blockdev-add if
# the @id option is not given there.
+# @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.7)
# @discard: #optional discard-related options (default: ignore)
# @cache: #optional cache-related options
# @aio: #optional AIO backend (default: threads)
@@ -2073,6 +2081,7 @@
'base': { 'driver': 'BlockdevDriver',
'*id': 'str',
'*node-name': 'str',
+ '*base-directory': 'str',
'*discard': 'BlockdevDiscardOptions',
'*cache': 'BlockdevCacheOptions',
'*aio': 'BlockdevAioOptions',
--
2.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [PATCH 19/19] iotests: Add quorum case to test 110
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (17 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 18/19] block: Add 'base-directory' BDS option Max Reitz
@ 2016-04-26 21:32 ` Max Reitz
2016-11-02 15:00 ` [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Alberto Garcia
19 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-04-26 21:32 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Alberto Garcia, Eric Blake, Kevin Wolf
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 | 9 +++++++++
2 files changed, 57 insertions(+)
diff --git a/tests/qemu-iotests/110 b/tests/qemu-iotests/110
index ba1b3c6..f1b7b08 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 not work
+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
+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..e586538 100644
--- a/tests/qemu-iotests/110.out
+++ b/tests/qemu-iotests/110.out
@@ -19,4 +19,13 @@ 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 ===
+
+qemu-img: Cannot generate a base directory for quorum nodes
+
+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.8.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 01/19] block: Use children list in bdrv_refresh_filename
2016-04-26 21:32 ` [Qemu-devel] [PATCH 01/19] block: Use children list in bdrv_refresh_filename Max Reitz
@ 2016-04-27 0:36 ` Eric Blake
2016-05-31 7:35 ` Alberto Garcia
1 sibling, 0 replies; 41+ messages in thread
From: Eric Blake @ 2016-04-27 0:36 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Alberto Garcia, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 594 bytes --]
On 04/26/2016 03:32 PM, Max Reitz wrote:
> 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>
> ---
> block.c | 9 +++++----
> block/blkverify.c | 3 ---
> block/quorum.c | 1 -
> 3 files changed, 5 insertions(+), 8 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden
2016-04-26 21:32 ` [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden Max Reitz
@ 2016-04-27 0:37 ` Eric Blake
2016-05-02 15:35 ` Kevin Wolf
1 sibling, 0 replies; 41+ messages in thread
From: Eric Blake @ 2016-04-27 0:37 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Alberto Garcia, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 694 bytes --]
On 04/26/2016 03:32 PM, 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>
> ---
> block.c | 2 ++
> include/block/block_int.h | 1 +
> 2 files changed, 3 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden
2016-04-26 21:32 ` [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden Max Reitz
2016-04-27 0:37 ` Eric Blake
@ 2016-05-02 15:35 ` Kevin Wolf
2016-05-03 10:49 ` Max Reitz
1 sibling, 1 reply; 41+ messages in thread
From: Kevin Wolf @ 2016-05-02 15:35 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
> 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>
What about live snapshots or block jobs that manipulate bs->backing?
They don't use bdrv_open_backing_file(), but bdrv_append() or
bdrv_replace_in_backing_chain() in order to assign a backing file, which
may or may not be the same as in the image header.
Also, do we want to consider a backing file overridden when the user
specified it explicitly, but it matches the image header?
Kevin
> block.c | 2 ++
> include/block/block_int.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/block.c b/block.c
> index e349e83..e178488 100644
> --- a/block.c
> +++ b/block.c
> @@ -1299,6 +1299,7 @@ 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")) {
> + bs->backing_overridden = true;
> backing_filename[0] = '\0';
> } else if (bs->backing_file[0] == '\0' && qdict_size(options) == 0) {
> QDECREF(options);
> @@ -1589,6 +1590,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 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");
> }
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 10d8759..d73e9ce 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -409,6 +409,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.8.0
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 03/19] block: Respect backing bs in bdrv_refresh_filename
2016-04-26 21:32 ` [Qemu-devel] [PATCH 03/19] block: Respect backing bs in bdrv_refresh_filename Max Reitz
@ 2016-05-02 15:36 ` Kevin Wolf
2016-05-03 10:50 ` Max Reitz
0 siblings, 1 reply; 41+ messages in thread
From: Kevin Wolf @ 2016-05-02 15:36 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
> 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 | 14 +++++++++++++-
> tests/qemu-iotests/051.out | 8 ++++----
> tests/qemu-iotests/051.pc.out | 8 ++++----
> 3 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/block.c b/block.c
> index e178488..aff9ea3 100644
> --- a/block.c
> +++ b/block.c
> @@ -3925,6 +3925,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 */
> @@ -3936,13 +3937,24 @@ 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_obj(opts, "backing",
> + QOBJECT(bs->backing->bs->full_open_options));
> + } else if (bs->backing_overridden && !bs->backing) {
> + qdict_put_obj(opts, "backing", QOBJECT(qstring_new()));
> + }
I see that the surrounding code does the same, but what's wrong with
qdict_put()?
Kevin
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public
2016-04-26 21:32 ` [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public Max Reitz
@ 2016-05-02 15:36 ` Kevin Wolf
2016-05-03 11:19 ` Max Reitz
0 siblings, 1 reply; 41+ messages in thread
From: Kevin Wolf @ 2016-05-02 15:36 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
> In order to allow block drivers to use that function, it needs to be
> public. In order to be useful, it needs to take a parameter which allows
> the caller to specify whether the runtime options allowed by the block
> driver are actually significant for the guest-visible BDS content.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
Is this actually good enough? I expect that many drivers will have some
options that are significant and other options that aren't. We already
have some (Quorum: children are significant, rewrite-corrupted isn't),
but as we convert more things to proper options, we'll get more of them
(raw-posix: filename is significant, aio=native isn't).
We might actually need to pass a list of significant fields instead that
append_open_options() can use.
Kevin
> block.c | 9 ++++++---
> include/block/block_int.h | 3 +++
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/block.c b/block.c
> index 511a0b1..b3ff08f 100644
> --- a/block.c
> +++ b/block.c
> @@ -3872,7 +3872,10 @@ static bool append_open_options(QDict *d, BlockDriverState *bs)
> return found_any;
> }
>
> -static void bdrv_default_refresh_format_filename(BlockDriverState *bs)
> +/* @options_significant must be true if any of the driver-specific runtime
> + * options may change the guest-visible content of the BDS */
> +void bdrv_default_refresh_format_filename(BlockDriverState *bs,
> + bool options_significant)
> {
> BlockDriver *drv = bs->drv;
> QDict *opts;
> @@ -3885,7 +3888,7 @@ static void bdrv_default_refresh_format_filename(BlockDriverState *bs)
> }
>
> opts = qdict_new();
> - has_open_options = append_open_options(opts, bs);
> + has_open_options = append_open_options(opts, bs) && options_significant;
>
> has_open_options |= bs->backing_overridden;
>
> /* If no specific options have been given for this BDS, the filename of
> @@ -3997,7 +4000,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
> QDECREF(opts);
> } else if (bs->file) {
> /* Try to reconstruct valid information from the underlying file */
> - bdrv_default_refresh_format_filename(bs);
> + bdrv_default_refresh_format_filename(bs, true);
> } else if (!bs->full_open_options && qdict_size(bs->options)) {
> bdrv_default_refresh_protocol_filename(bs);
> }
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index d73e9ce..eb3665a 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -532,6 +532,9 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
> void bdrv_set_io_limits(BlockDriverState *bs,
> ThrottleConfig *cfg);
>
> +void bdrv_default_refresh_format_filename(BlockDriverState *bs,
> + bool options_significant);
> +
>
> /**
> * bdrv_add_before_write_notifier:
> --
> 2.8.0
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 16/19] block/nbd: Implement bdrv_dirname()
2016-04-26 21:32 ` [Qemu-devel] [PATCH 16/19] block/nbd: Implement bdrv_dirname() Max Reitz
@ 2016-05-02 15:36 ` Kevin Wolf
2016-05-03 11:28 ` Max Reitz
0 siblings, 1 reply; 41+ messages in thread
From: Kevin Wolf @ 2016-05-02 15:36 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
> 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 | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index f7ea3b3..64534bb 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -469,6 +469,32 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
> bs->full_open_options = opts;
> }
>
> +static char *nbd_dirname(BlockDriverState *bs, Error **errp)
> +{
> + const char *path = qdict_get_try_str(bs->options, "path");
> + const char *host = qdict_get_try_str(bs->options, "host");
> + const char *port = qdict_get_try_str(bs->options, "port");
> + const char *export = qdict_get_try_str(bs->options, "export");
> +
> + if (path && export) {
> + return g_strdup_printf("nbd:unix:%s:exportname=%s/", path, export);
> + } else if (path && !export) {
> + return g_strdup_printf("nbd:unix:%s:exportname=", path);
> + } else if (!path && export && port) {
> + return g_strdup_printf("nbd://%s:%s/%s/", host, port, export);
> + } else if (!path && export && !port) {
> + return g_strdup_printf("nbd://%s/%s/", host, export);
> + } else if (!path && !export && port) {
> + return g_strdup_printf("nbd://%s:%s/", host, port);
> + } else if (!path && !export && !port) {
> + return g_strdup_printf("nbd://%s/", host);
> + }
Many different cases already, and it's only going to get worse. Wouldn't
it be better to store saddr in BDRVNBDState (so that we don't have to
parse the options a second time here) and to go for the full version
here always? Then it's one case for tcp and one for unix and that's it.
Kevin
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden
2016-05-02 15:35 ` Kevin Wolf
@ 2016-05-03 10:49 ` Max Reitz
2016-05-03 13:34 ` Kevin Wolf
0 siblings, 1 reply; 41+ messages in thread
From: Max Reitz @ 2016-05-03 10:49 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
[-- Attachment #1.1: Type: text/plain, Size: 2746 bytes --]
On 02.05.2016 17:35, Kevin Wolf wrote:
> Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
>> 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>
>
> What about live snapshots or block jobs that manipulate bs->backing?
> They don't use bdrv_open_backing_file(), but bdrv_append() or
> bdrv_replace_in_backing_chain() in order to assign a backing file, which
> may or may not be the same as in the image header.
I'll take a look into each case, thanks.
> Also, do we want to consider a backing file overridden when the user
> specified it explicitly, but it matches the image header?
For simplicity's sake, I would. I think we've realized before that
trying to identify whether to file names point to the same image is
rather difficult.
Max
>
> Kevin
>
>> block.c | 2 ++
>> include/block/block_int.h | 1 +
>> 2 files changed, 3 insertions(+)
>>
>> diff --git a/block.c b/block.c
>> index e349e83..e178488 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1299,6 +1299,7 @@ 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")) {
>> + bs->backing_overridden = true;
>> backing_filename[0] = '\0';
>> } else if (bs->backing_file[0] == '\0' && qdict_size(options) == 0) {
>> QDECREF(options);
>> @@ -1589,6 +1590,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 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");
>> }
>>
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index 10d8759..d73e9ce 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -409,6 +409,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.8.0
>>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 03/19] block: Respect backing bs in bdrv_refresh_filename
2016-05-02 15:36 ` Kevin Wolf
@ 2016-05-03 10:50 ` Max Reitz
0 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-05-03 10:50 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
[-- Attachment #1.1: Type: text/plain, Size: 3269 bytes --]
On 02.05.2016 17:36, Kevin Wolf wrote:
> Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
>> 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 | 14 +++++++++++++-
>> tests/qemu-iotests/051.out | 8 ++++----
>> tests/qemu-iotests/051.pc.out | 8 ++++----
>> 3 files changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index e178488..aff9ea3 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3925,6 +3925,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 */
>> @@ -3936,13 +3937,24 @@ 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_obj(opts, "backing",
>> + QOBJECT(bs->backing->bs->full_open_options));
>> + } else if (bs->backing_overridden && !bs->backing) {
>> + qdict_put_obj(opts, "backing", QOBJECT(qstring_new()));
>> + }
>
> I see that the surrounding code does the same, but what's wrong with
> qdict_put()?
That it's a macro. clang_complete does not tell me about macros.
Will fix and try to keep in mind in the future, thanks.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public
2016-05-02 15:36 ` Kevin Wolf
@ 2016-05-03 11:19 ` Max Reitz
2016-05-03 13:31 ` Kevin Wolf
0 siblings, 1 reply; 41+ messages in thread
From: Max Reitz @ 2016-05-03 11:19 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
[-- Attachment #1.1: Type: text/plain, Size: 2111 bytes --]
On 02.05.2016 17:36, Kevin Wolf wrote:
> Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
>> In order to allow block drivers to use that function, it needs to be
>> public. In order to be useful, it needs to take a parameter which allows
>> the caller to specify whether the runtime options allowed by the block
>> driver are actually significant for the guest-visible BDS content.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>
> Is this actually good enough? I expect that many drivers will have some
> options that are significant and other options that aren't. We already
> have some (Quorum: children are significant, rewrite-corrupted isn't),
> but as we convert more things to proper options, we'll get more of them
> (raw-posix: filename is significant, aio=native isn't).
>
> We might actually need to pass a list of significant fields instead that
> append_open_options() can use.
Well, in theory, every driver with insignificant options would just
implement .bdrv_refresh_filename() however it's needed. Making
bdrv_default_refresh_format_filename() function public is just a way of
keeping that implementation very simple for some drivers that only have
insignificant options.
I'm not opposed to extending this function in the future when it
actually makes sense. Right now I don't think it does. The only thing
that changes if a significant option is detected is that no plain
filename is generated; however, for Quorum we can never generate such a
filename. Therefore, we cannot use this function for Quorum anyway.
However, instead of extending this function, it may make more sense then
to introduce a new field to the BlockDriver struct which is a
NULL-terminated array of significant option names, or something like
that. If .bdrv_refresh_filename is NULL but that array pointer is not,
then the default implementation could behave accordingly. But this is
something I'd defer to the future, too, unless you can point out a
current block driver that would benefit from this functionality (I don't
think Quorum does, as I said above).
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 16/19] block/nbd: Implement bdrv_dirname()
2016-05-02 15:36 ` Kevin Wolf
@ 2016-05-03 11:28 ` Max Reitz
0 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-05-03 11:28 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
[-- Attachment #1.1: Type: text/plain, Size: 2796 bytes --]
On 02.05.2016 17:36, Kevin Wolf wrote:
> Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
>> 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 | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index f7ea3b3..64534bb 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -469,6 +469,32 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
>> bs->full_open_options = opts;
>> }
>>
>> +static char *nbd_dirname(BlockDriverState *bs, Error **errp)
>> +{
>> + const char *path = qdict_get_try_str(bs->options, "path");
>> + const char *host = qdict_get_try_str(bs->options, "host");
>> + const char *port = qdict_get_try_str(bs->options, "port");
>> + const char *export = qdict_get_try_str(bs->options, "export");
>> +
>> + if (path && export) {
>> + return g_strdup_printf("nbd:unix:%s:exportname=%s/", path, export);
>> + } else if (path && !export) {
>> + return g_strdup_printf("nbd:unix:%s:exportname=", path);
>> + } else if (!path && export && port) {
>> + return g_strdup_printf("nbd://%s:%s/%s/", host, port, export);
>> + } else if (!path && export && !port) {
>> + return g_strdup_printf("nbd://%s/%s/", host, export);
>> + } else if (!path && !export && port) {
>> + return g_strdup_printf("nbd://%s:%s/", host, port);
>> + } else if (!path && !export && !port) {
>> + return g_strdup_printf("nbd://%s/", host);
>> + }
>
> Many different cases already, and it's only going to get worse.
Is it? I'm not sure we can support things like TLS here at all, so I
think this should be about it, even in the future.
> Wouldn't
> it be better to store saddr in BDRVNBDState (so that we don't have to
> parse the options a second time here) and to go for the full version
> here always? Then it's one case for tcp and one for unix and that's it.
I think we can do that even without saddr, but I guess I need to see how
this patch interacts with my "blockdev-add for NBD" series anyway.
Thanks for the suggestion, I'll act on it,
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public
2016-05-03 11:19 ` Max Reitz
@ 2016-05-03 13:31 ` Kevin Wolf
2016-05-03 13:48 ` Max Reitz
0 siblings, 1 reply; 41+ messages in thread
From: Kevin Wolf @ 2016-05-03 13:31 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]
Am 03.05.2016 um 13:19 hat Max Reitz geschrieben:
> On 02.05.2016 17:36, Kevin Wolf wrote:
> > Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
> >> In order to allow block drivers to use that function, it needs to be
> >> public. In order to be useful, it needs to take a parameter which allows
> >> the caller to specify whether the runtime options allowed by the block
> >> driver are actually significant for the guest-visible BDS content.
> >>
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >
> > Is this actually good enough? I expect that many drivers will have some
> > options that are significant and other options that aren't. We already
> > have some (Quorum: children are significant, rewrite-corrupted isn't),
> > but as we convert more things to proper options, we'll get more of them
> > (raw-posix: filename is significant, aio=native isn't).
> >
> > We might actually need to pass a list of significant fields instead that
> > append_open_options() can use.
>
> Well, in theory, every driver with insignificant options would just
> implement .bdrv_refresh_filename() however it's needed. Making
> bdrv_default_refresh_format_filename() function public is just a way of
> keeping that implementation very simple for some drivers that only have
> insignificant options.
>
> I'm not opposed to extending this function in the future when it
> actually makes sense. Right now I don't think it does. The only thing
> that changes if a significant option is detected is that no plain
> filename is generated; however, for Quorum we can never generate such a
> filename. Therefore, we cannot use this function for Quorum anyway.
If you integrate it into append_open_options(), I suppose it would also
mean that insignificant options are dropped from the json: description,
i.e. Quorum would return a json: object with all children, but not the
rewrite-corrupted setting. Which I think would be a good thing.
Kevin
> However, instead of extending this function, it may make more sense then
> to introduce a new field to the BlockDriver struct which is a
> NULL-terminated array of significant option names, or something like
> that. If .bdrv_refresh_filename is NULL but that array pointer is not,
> then the default implementation could behave accordingly. But this is
> something I'd defer to the future, too, unless you can point out a
> current block driver that would benefit from this functionality (I don't
> think Quorum does, as I said above).
>
> Max
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden
2016-05-03 10:49 ` Max Reitz
@ 2016-05-03 13:34 ` Kevin Wolf
0 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2016-05-03 13:34 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]
Am 03.05.2016 um 12:49 hat Max Reitz geschrieben:
> On 02.05.2016 17:35, Kevin Wolf wrote:
> > Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
> >> 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>
> >
> > What about live snapshots or block jobs that manipulate bs->backing?
> > They don't use bdrv_open_backing_file(), but bdrv_append() or
> > bdrv_replace_in_backing_chain() in order to assign a backing file, which
> > may or may not be the same as in the image header.
>
> I'll take a look into each case, thanks.
>
> > Also, do we want to consider a backing file overridden when the user
> > specified it explicitly, but it matches the image header?
>
> For simplicity's sake, I would. I think we've realized before that
> trying to identify whether to file names point to the same image is
> rather difficult.
I was just asking because in that case, we probably must consider things
like the backing file of a live snapshot overlay overridden as well (at
least when using an existing image).
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public
2016-05-03 13:31 ` Kevin Wolf
@ 2016-05-03 13:48 ` Max Reitz
2016-05-03 14:34 ` Kevin Wolf
0 siblings, 1 reply; 41+ messages in thread
From: Max Reitz @ 2016-05-03 13:48 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
[-- Attachment #1.1: Type: text/plain, Size: 3548 bytes --]
On 03.05.2016 15:31, Kevin Wolf wrote:
> Am 03.05.2016 um 13:19 hat Max Reitz geschrieben:
>> On 02.05.2016 17:36, Kevin Wolf wrote:
>>> Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
>>>> In order to allow block drivers to use that function, it needs to be
>>>> public. In order to be useful, it needs to take a parameter which allows
>>>> the caller to specify whether the runtime options allowed by the block
>>>> driver are actually significant for the guest-visible BDS content.
>>>>
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>
>>> Is this actually good enough? I expect that many drivers will have some
>>> options that are significant and other options that aren't. We already
>>> have some (Quorum: children are significant, rewrite-corrupted isn't),
>>> but as we convert more things to proper options, we'll get more of them
>>> (raw-posix: filename is significant, aio=native isn't).
>>>
>>> We might actually need to pass a list of significant fields instead that
>>> append_open_options() can use.
>>
>> Well, in theory, every driver with insignificant options would just
>> implement .bdrv_refresh_filename() however it's needed. Making
>> bdrv_default_refresh_format_filename() function public is just a way of
>> keeping that implementation very simple for some drivers that only have
>> insignificant options.
>>
>> I'm not opposed to extending this function in the future when it
>> actually makes sense. Right now I don't think it does. The only thing
>> that changes if a significant option is detected is that no plain
>> filename is generated; however, for Quorum we can never generate such a
>> filename. Therefore, we cannot use this function for Quorum anyway.
>
> If you integrate it into append_open_options(), I suppose it would also
> mean that insignificant options are dropped from the json: description,
> i.e. Quorum would return a json: object with all children, but not the
> rewrite-corrupted setting. Which I think would be a good thing.
I'm not sure I do. :-)
At least right now the JSON version is supposed to contain all options,
be they significant or not. Let me try to remember my reasoning:
Ideally, we want to get a filename which *exactly* results in the same
BDS that we have. This should always be possible if instead of a plain
filename one specifies options, e.g. using a JSON filename. However,
such a JSON filename (or giving options using the dot syntax or as JSON
with blockdev-add) is cumbersome.
In many simple cases, we can (re-)construct a plain filename which
yields exactly the same BDS, though. That's nice so that's what we try
to do first.
In some cases, it is impossible to construct a plain filename which
yields a BDS that will return the same data when accessed. Then, we just
cannot give such a filename and have to stick to a JSON filename,
there's no way around this.
However, sometimes we are in a gray area. We can construct a plain
filename which yields a slightly different BDS than the one we have; but
it will return the same data when accessed and thus it is "close
enough". We then have to make a tradeoff between getting exactly the
same BDS and having a nice and simple filename. I opted for the latter.
However, if we do have to emit a JSON filename at some point in the tree
I think we've basically "lost" already. If we get to that point, we may
as well just emit all the options that have been used to construct the
BDS, even if they don't change the data it yields.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public
2016-05-03 13:48 ` Max Reitz
@ 2016-05-03 14:34 ` Kevin Wolf
2016-05-03 14:52 ` Max Reitz
0 siblings, 1 reply; 41+ messages in thread
From: Kevin Wolf @ 2016-05-03 14:34 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
[-- Attachment #1: Type: text/plain, Size: 5264 bytes --]
Am 03.05.2016 um 15:48 hat Max Reitz geschrieben:
> On 03.05.2016 15:31, Kevin Wolf wrote:
> > Am 03.05.2016 um 13:19 hat Max Reitz geschrieben:
> >> On 02.05.2016 17:36, Kevin Wolf wrote:
> >>> Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
> >>>> In order to allow block drivers to use that function, it needs to be
> >>>> public. In order to be useful, it needs to take a parameter which allows
> >>>> the caller to specify whether the runtime options allowed by the block
> >>>> driver are actually significant for the guest-visible BDS content.
> >>>>
> >>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >>>
> >>> Is this actually good enough? I expect that many drivers will have some
> >>> options that are significant and other options that aren't. We already
> >>> have some (Quorum: children are significant, rewrite-corrupted isn't),
> >>> but as we convert more things to proper options, we'll get more of them
> >>> (raw-posix: filename is significant, aio=native isn't).
> >>>
> >>> We might actually need to pass a list of significant fields instead that
> >>> append_open_options() can use.
> >>
> >> Well, in theory, every driver with insignificant options would just
> >> implement .bdrv_refresh_filename() however it's needed. Making
> >> bdrv_default_refresh_format_filename() function public is just a way of
> >> keeping that implementation very simple for some drivers that only have
> >> insignificant options.
> >>
> >> I'm not opposed to extending this function in the future when it
> >> actually makes sense. Right now I don't think it does. The only thing
> >> that changes if a significant option is detected is that no plain
> >> filename is generated; however, for Quorum we can never generate such a
> >> filename. Therefore, we cannot use this function for Quorum anyway.
> >
> > If you integrate it into append_open_options(), I suppose it would also
> > mean that insignificant options are dropped from the json: description,
> > i.e. Quorum would return a json: object with all children, but not the
> > rewrite-corrupted setting. Which I think would be a good thing.
>
> I'm not sure I do. :-)
>
> At least right now the JSON version is supposed to contain all options,
> be they significant or not. Let me try to remember my reasoning:
>
> Ideally, we want to get a filename which *exactly* results in the same
> BDS that we have.
Here I'm not sure we do. :-)
We already don't do this. We filter out any options that are parsed by
the block layer. For example, we don't include the node name or caching
options. If we really wanted to represend the BDS as exactly as we can,
this wouldn't be right and we'd have to fix it.
But as I see it, what we were really after when we implemented things
this way was that we distinguish options that are conceptually part of
some address that points to the image data (which I thought matches your
"significant" options) and other options that just influence our access
patterns and what we do with the image at this address.
The filename (json: or not) consists then only of the address part, as
the other options can differ between qemu invocations without actually
changing which image we see. I don't expect something called a filename
(json: exists just because a plain filename can't represent everything)
to contain various runtime configuration settings, but just a pure
pointer to the image.
> This should always be possible if instead of a plain
> filename one specifies options, e.g. using a JSON filename. However,
> such a JSON filename (or giving options using the dot syntax or as JSON
> with blockdev-add) is cumbersome.
>
> In many simple cases, we can (re-)construct a plain filename which
> yields exactly the same BDS, though. That's nice so that's what we try
> to do first.
>
> In some cases, it is impossible to construct a plain filename which
> yields a BDS that will return the same data when accessed. Then, we just
> cannot give such a filename and have to stick to a JSON filename,
> there's no way around this.
>
> However, sometimes we are in a gray area. We can construct a plain
> filename which yields a slightly different BDS than the one we have; but
> it will return the same data when accessed and thus it is "close
> enough". We then have to make a tradeoff between getting exactly the
> same BDS and having a nice and simple filename. I opted for the latter.
I can imagine that there are use cases for some mechanism to return the
JSON object that creates exactly the same BDS, like you seem to be
envisioning here. I just doubt that it's useful in those cases where we
really wanted a filename and have to go for JSON because we can't do
anything more user friendly.
> However, if we do have to emit a JSON filename at some point in the tree
> I think we've basically "lost" already. If we get to that point, we may
> as well just emit all the options that have been used to construct the
> BDS, even if they don't change the data it yields.
In places where you want a filename (which is mostly, if not
exclusively, messages for the user), emitting everything may just make
an already unfriendly message even worse.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public
2016-05-03 14:34 ` Kevin Wolf
@ 2016-05-03 14:52 ` Max Reitz
0 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-05-03 14:52 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel, Alberto Garcia, Eric Blake
[-- Attachment #1.1: Type: text/plain, Size: 5823 bytes --]
On 03.05.2016 16:34, Kevin Wolf wrote:
> Am 03.05.2016 um 15:48 hat Max Reitz geschrieben:
>> On 03.05.2016 15:31, Kevin Wolf wrote:
>>> Am 03.05.2016 um 13:19 hat Max Reitz geschrieben:
>>>> On 02.05.2016 17:36, Kevin Wolf wrote:
>>>>> Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
>>>>>> In order to allow block drivers to use that function, it needs to be
>>>>>> public. In order to be useful, it needs to take a parameter which allows
>>>>>> the caller to specify whether the runtime options allowed by the block
>>>>>> driver are actually significant for the guest-visible BDS content.
>>>>>>
>>>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>>>
>>>>> Is this actually good enough? I expect that many drivers will have some
>>>>> options that are significant and other options that aren't. We already
>>>>> have some (Quorum: children are significant, rewrite-corrupted isn't),
>>>>> but as we convert more things to proper options, we'll get more of them
>>>>> (raw-posix: filename is significant, aio=native isn't).
>>>>>
>>>>> We might actually need to pass a list of significant fields instead that
>>>>> append_open_options() can use.
>>>>
>>>> Well, in theory, every driver with insignificant options would just
>>>> implement .bdrv_refresh_filename() however it's needed. Making
>>>> bdrv_default_refresh_format_filename() function public is just a way of
>>>> keeping that implementation very simple for some drivers that only have
>>>> insignificant options.
>>>>
>>>> I'm not opposed to extending this function in the future when it
>>>> actually makes sense. Right now I don't think it does. The only thing
>>>> that changes if a significant option is detected is that no plain
>>>> filename is generated; however, for Quorum we can never generate such a
>>>> filename. Therefore, we cannot use this function for Quorum anyway.
>>>
>>> If you integrate it into append_open_options(), I suppose it would also
>>> mean that insignificant options are dropped from the json: description,
>>> i.e. Quorum would return a json: object with all children, but not the
>>> rewrite-corrupted setting. Which I think would be a good thing.
>>
>> I'm not sure I do. :-)
>>
>> At least right now the JSON version is supposed to contain all options,
>> be they significant or not. Let me try to remember my reasoning:
>>
>> Ideally, we want to get a filename which *exactly* results in the same
>> BDS that we have.
>
> Here I'm not sure we do. :-)
>
> We already don't do this. We filter out any options that are parsed by
> the block layer. For example, we don't include the node name or caching
> options. If we really wanted to represend the BDS as exactly as we can,
> this wouldn't be right and we'd have to fix it.
>
> But as I see it, what we were really after when we implemented things
> this way was that we distinguish options that are conceptually part of
> some address that points to the image data (which I thought matches your
> "significant" options) and other options that just influence our access
> patterns and what we do with the image at this address.
>
> The filename (json: or not) consists then only of the address part, as
> the other options can differ between qemu invocations without actually
> changing which image we see. I don't expect something called a filename
> (json: exists just because a plain filename can't represent everything)
> to contain various runtime configuration settings, but just a pure
> pointer to the image.
>
>> This should always be possible if instead of a plain
>> filename one specifies options, e.g. using a JSON filename. However,
>> such a JSON filename (or giving options using the dot syntax or as JSON
>> with blockdev-add) is cumbersome.
>>
>> In many simple cases, we can (re-)construct a plain filename which
>> yields exactly the same BDS, though. That's nice so that's what we try
>> to do first.
>>
>> In some cases, it is impossible to construct a plain filename which
>> yields a BDS that will return the same data when accessed. Then, we just
>> cannot give such a filename and have to stick to a JSON filename,
>> there's no way around this.
>>
>> However, sometimes we are in a gray area. We can construct a plain
>> filename which yields a slightly different BDS than the one we have; but
>> it will return the same data when accessed and thus it is "close
>> enough". We then have to make a tradeoff between getting exactly the
>> same BDS and having a nice and simple filename. I opted for the latter.
>
> I can imagine that there are use cases for some mechanism to return the
> JSON object that creates exactly the same BDS, like you seem to be
> envisioning here. I just doubt that it's useful in those cases where we
> really wanted a filename and have to go for JSON because we can't do
> anything more user friendly.
>
>> However, if we do have to emit a JSON filename at some point in the tree
>> I think we've basically "lost" already. If we get to that point, we may
>> as well just emit all the options that have been used to construct the
>> BDS, even if they don't change the data it yields.
>
> In places where you want a filename (which is mostly, if not
> exclusively, messages for the user), emitting everything may just make
> an already unfriendly message even worse.
Well, I don't particularly care either way. Thus, I'd be fine with
removing insignificant options even from full_open_options if you deem
that useful, but that is something we don't do already so we'd have to
implement it.
I guess I can implement it in this series if I decide to address the
issue you originally raised here ("Is this good enough?"). If it gets
too much, I'd rather handle it in a follow-up, though.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 01/19] block: Use children list in bdrv_refresh_filename
2016-04-26 21:32 ` [Qemu-devel] [PATCH 01/19] block: Use children list in bdrv_refresh_filename Max Reitz
2016-04-27 0:36 ` Eric Blake
@ 2016-05-31 7:35 ` Alberto Garcia
1 sibling, 0 replies; 41+ messages in thread
From: Alberto Garcia @ 2016-05-31 7:35 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Eric Blake, Kevin Wolf
On Tue 26 Apr 2016 11:32:00 PM CEST, Max Reitz wrote:
> 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: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 04/19] block: Add bdrv_default_refresh_format_filename
2016-04-26 21:32 ` [Qemu-devel] [PATCH 04/19] block: Add bdrv_default_refresh_format_filename Max Reitz
@ 2016-06-20 13:16 ` Alberto Garcia
0 siblings, 0 replies; 41+ messages in thread
From: Alberto Garcia @ 2016-06-20 13:16 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Eric Blake, Kevin Wolf
On Tue 26 Apr 2016 11:32:03 PM CEST, Max Reitz wrote:
> Split off the default code for format BDS from bdrv_refresh_filename()
> into an own function, first because it is nicer this way, and second
> because this will allow block drivers to reuse this function in their
> own specific implementation of bdrv_refresh_filename().
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 05/19] block: Add bdrv_default_refresh_protocol_filename
2016-04-26 21:32 ` [Qemu-devel] [PATCH 05/19] block: Add bdrv_default_refresh_protocol_filename Max Reitz
@ 2016-06-20 13:18 ` Alberto Garcia
0 siblings, 0 replies; 41+ messages in thread
From: Alberto Garcia @ 2016-06-20 13:18 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Eric Blake, Kevin Wolf
On Tue 26 Apr 2016 11:32:04 PM CEST, Max Reitz wrote:
> Split off the default code for protocol BDS from bdrv_refresh_filename()
> into an own function, just as it has been done for format BDS.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 08/19] block: Make path_combine() return the path
2016-04-26 21:32 ` [Qemu-devel] [PATCH 08/19] block: Make path_combine() return the path Max Reitz
@ 2016-06-20 13:44 ` Alberto Garcia
0 siblings, 0 replies; 41+ messages in thread
From: Alberto Garcia @ 2016-06-20 13:44 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Eric Blake, Kevin Wolf
On Tue 26 Apr 2016 11:32:07 PM CEST, Max Reitz wrote:
> 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>
Berto
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
` (18 preceding siblings ...)
2016-04-26 21:32 ` [Qemu-devel] [PATCH 19/19] iotests: Add quorum case to test 110 Max Reitz
@ 2016-11-02 15:00 ` Alberto Garcia
2016-11-02 15:05 ` Max Reitz
19 siblings, 1 reply; 41+ messages in thread
From: Alberto Garcia @ 2016-11-02 15:00 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Eric Blake, Kevin Wolf
On Tue 26 Apr 2016 11:31:59 PM CEST, Max Reitz wrote:
> There are some issues regarding filename generation right now:
>
> - You always get a JSON filename if you set even a single qcow2-specific
> runtime options (as long as it does not have a dot in it, which is a
> bug, too, but here it is working in our favor...). That is not nice
> and actually breaks the usage of backing files with relative
> filenames with such qcow2 BDS.
>
> - As hinted above, you cannot use relative backing filenames with BDS
> that have a JSON filename only, even though qemu might be able to
> obtain the directory name by walking through the BDS graph to the
> protocol level.
>
> - Overriding the backing file at runtime should invalidate the filename
> because it actually changes the BDS's data. Therefore, we need to
> force a JSON filename in that case, containing the backing file
> override.
>
> - Much of our code assumes paths never to exceed PATH_MAX in length.
> This is wrong, at least because of JSON filenames. This should be
> fixed wherever the opportunity arises.
Hi Max,
I'd like to retake the review of this series. It can be rebased easily
and it still seems to work fine. Shall I take a look at the patches as
they are now or shall I better wait for a new version?
Berto
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues
2016-11-02 15:00 ` [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Alberto Garcia
@ 2016-11-02 15:05 ` Max Reitz
0 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2016-11-02 15:05 UTC (permalink / raw)
To: Alberto Garcia, qemu-block; +Cc: qemu-devel, Eric Blake, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]
On 02.11.2016 16:00, Alberto Garcia wrote:
> On Tue 26 Apr 2016 11:31:59 PM CEST, Max Reitz wrote:
>
>> There are some issues regarding filename generation right now:
>>
>> - You always get a JSON filename if you set even a single qcow2-specific
>> runtime options (as long as it does not have a dot in it, which is a
>> bug, too, but here it is working in our favor...). That is not nice
>> and actually breaks the usage of backing files with relative
>> filenames with such qcow2 BDS.
>>
>> - As hinted above, you cannot use relative backing filenames with BDS
>> that have a JSON filename only, even though qemu might be able to
>> obtain the directory name by walking through the BDS graph to the
>> protocol level.
>>
>> - Overriding the backing file at runtime should invalidate the filename
>> because it actually changes the BDS's data. Therefore, we need to
>> force a JSON filename in that case, containing the backing file
>> override.
>>
>> - Much of our code assumes paths never to exceed PATH_MAX in length.
>> This is wrong, at least because of JSON filenames. This should be
>> fixed wherever the opportunity arises.
>
> Hi Max,
>
> I'd like to retake the review of this series. It can be rebased easily
> and it still seems to work fine. Shall I take a look at the patches as
> they are now or shall I better wait for a new version?
Thanks! I think it would be better to wait for a new version, which I'll
get to hopefully sooner than later.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2016-11-02 15:05 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-26 21:31 [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 01/19] block: Use children list in bdrv_refresh_filename Max Reitz
2016-04-27 0:36 ` Eric Blake
2016-05-31 7:35 ` Alberto Garcia
2016-04-26 21:32 ` [Qemu-devel] [PATCH 02/19] block: Add BDS.backing_overridden Max Reitz
2016-04-27 0:37 ` Eric Blake
2016-05-02 15:35 ` Kevin Wolf
2016-05-03 10:49 ` Max Reitz
2016-05-03 13:34 ` Kevin Wolf
2016-04-26 21:32 ` [Qemu-devel] [PATCH 03/19] block: Respect backing bs in bdrv_refresh_filename Max Reitz
2016-05-02 15:36 ` Kevin Wolf
2016-05-03 10:50 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 04/19] block: Add bdrv_default_refresh_format_filename Max Reitz
2016-06-20 13:16 ` Alberto Garcia
2016-04-26 21:32 ` [Qemu-devel] [PATCH 05/19] block: Add bdrv_default_refresh_protocol_filename Max Reitz
2016-06-20 13:18 ` Alberto Garcia
2016-04-26 21:32 ` [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public Max Reitz
2016-05-02 15:36 ` Kevin Wolf
2016-05-03 11:19 ` Max Reitz
2016-05-03 13:31 ` Kevin Wolf
2016-05-03 13:48 ` Max Reitz
2016-05-03 14:34 ` Kevin Wolf
2016-05-03 14:52 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 07/19] qcow2: Implement bdrv_refresh_filename() Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 08/19] block: Make path_combine() return the path Max Reitz
2016-06-20 13:44 ` Alberto Garcia
2016-04-26 21:32 ` [Qemu-devel] [PATCH 09/19] block: bdrv_get_full_backing_filename_from_...'s ret. val Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 10/19] block: bdrv_get_full_backing_filename's " Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 11/19] block: Add bdrv_make_absolute_filename() Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 12/19] block: Fix bdrv_find_backing_image() Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 13/19] block: Add bdrv_dirname() Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 14/19] blkverify: Make bdrv_dirname() return NULL Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 15/19] quorum: " Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 16/19] block/nbd: Implement bdrv_dirname() Max Reitz
2016-05-02 15:36 ` Kevin Wolf
2016-05-03 11:28 ` Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 17/19] block: Use bdrv_dirname() for relative filenames Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 18/19] block: Add 'base-directory' BDS option Max Reitz
2016-04-26 21:32 ` [Qemu-devel] [PATCH 19/19] iotests: Add quorum case to test 110 Max Reitz
2016-11-02 15:00 ` [Qemu-devel] [PATCH 00/19] block: Fix some filename generation issues Alberto Garcia
2016-11-02 15:05 ` Max Reitz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.