Re: [Qemu-devel] [PATCH v2 0/4] block: allow partial info-block query

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3125 bytes --]

On 12.12.2015 02:25, John Snow wrote:
> Max: Did you have in mind something like this?

What I had in mind was fixing the whole thing (the test case in 110
should actually work just fine, because you can easily determine the
base directory to be used). ;-)

I've been working on that myself anyway, so yes, regarding not aborting
block-status if some full backing filename cannot be reconstructed, this
looks good (by preventing qemu-img from resorting to the relative
filename when querying the backing chain).

There is a single thing I don't find quite perfect:

$ mkdir -p foo
$ ./qemu-img create -f qcow2 foo/base.qcow2 64M
$ ./qemu-img create -f qcow2 -b base.qcow2 foo/top.qcow2
$ ./qemu-img info foo/top.qcow2
[...]
backing file: base.qcow2 (actual path: foo/base.qcow2)
[...]

$ ./qemu-img info \
    "json:{'lazy-refcounts':true,'file.filename':'foo/top.qcow2'}"
[...]
backing file: base.qcow2
[...]

$ cd foo
$ ../qemu-img info top.qcow2
[...]
backing file: base.qcow2
[...]

Before this series, there was only one case where the actual path
information was missing: If it equals the raw relative filename. Now,
there are two cases: As can be seen in the last invokation, it still
will not be emitted if actual and raw filename are equal. But as you can
see in the second invokation, it will be hidden also if it simply cannot
be determined.

Therefore, if the actual path information is missing, the user cannot
determine[1] whether this is due to the actual path (relative to the
CWD) is equal to the (raw) relative path to the overlay; or whether this
is due to some error in determining the absolute filename.

[1] You can by testing whether --backing-chain emits an error. But that
is a bit crude.

I think we should put a note there if an error occurred while
determining the absolute backing filename.

(Doing that is pretty simple:
if (!info->has_full_backing_filename) {
    func_fprintf(f, " (cannot determine actual path)");
} else if (strcmp(...)) {
    func_fprintf(f, " (actual path: %s)", ...);
}
)

I think that should be an additional patch between 2 and 4.

Max

> 
> v2:
>  - Fix qemu-img from now choking when it gets a partial response.
> 
> ________________________________________________________________________________
> 
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch block-allow-partial-query
> https://github.com/jnsnow/qemu/tree/block-allow-partial-query
> 
> This version is tagged block-allow-partial-query-v2:
> https://github.com/jnsnow/qemu/releases/tag/block-allow-partial-query-v2
> 
> John Snow (4):
>   block/qapi: do not redundantly print "actual path"
>   block/qapi: always report full_backing_filename
>   qemu-img: abort when full_backing_filename not present
>   block/qapi: allow best-effort query
> 
>  block/qapi.c               | 18 +++++++++++-------
>  qemu-img.c                 |  5 ++++-
>  tests/qemu-iotests/043.out |  2 ++
>  tests/qemu-iotests/110.out |  5 ++++-
>  4 files changed, 21 insertions(+), 9 deletions(-)
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]