[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: allow best-effort query
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH] block: allow best-effort query |
Date: |
Mon, 26 Oct 2015 17:34:02 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 10/26/2015 05:18 PM, Max Reitz wrote:
> On 26.10.2015 19:12, John Snow wrote:
>> For more complex BDS trees that can be created under normal circumstances,
>> we lose the ability to issue query commands because of our inability to
>> re-construct the absolute filename.
>>
>> Instead, omit this field when it is a problem and present as much information
>> as we can.
>>
>> This will change the expected output in iotest 110, where we will now see a
>> json filename and the lack of an absolute filename instead of an error.
>>
>> Signed-off-by: John Snow <address@hidden>
>> ---
>> block/qapi.c | 10 ++++++----
>> tests/qemu-iotests/110.out | 5 ++++-
>> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> One problem I see now is that qemu-img --backing-chain uses the full
> backing name if it is available and if it isn't, it resorts to the
> non-full backing name (which was good until this patch, as far as I can
> see). But now that's not necessarily a valid substitution anymore:
>
> $ mkdir foo
> $ ./qemu-img create -f qcow2 foo/base.qcow2 64M
> $ ./qemu-img create -f qcow2 -b base.qcow2 foo/top.qcow2
> $ ./qemu-img create -f qcow2 base.qcow2 64M
> $ ./qemu-img info --backing-chain foo/top.qcow2
> image: foo/top.qcow2
> file format: qcow2
> virtual size: 64M (67108864 bytes)
> disk size: 196K
> cluster_size: 65536
> backing file: base.qcow2 (actual path: foo/base.qcow2)
> Format specific information:
> compat: 1.1
> lazy refcounts: false
> refcount bits: 16
> corrupt: false
>
> image: foo/base.qcow2
> file format: qcow2
> virtual size: 64M (67108864 bytes)
> disk size: 196K
> cluster_size: 65536
> Format specific information:
> compat: 1.1
> lazy refcounts: false
> refcount bits: 16
> corrupt: false
> $ ./qemu-img info --backing-chain \
> "json:{'file.filename':'foo/top.qcow2',
> 'driver':'qcow2','lazy-refcounts':true}"
> image: json:{"lazy-refcounts": true, "driver": "qcow2", "file":
> {"driver": "file", "filename": "foo/top.qcow2"}}
> file format: qcow2
> virtual size: 64M (67108864 bytes)
> disk size: 196K
> cluster_size: 65536
> backing file: base.qcow2
> Format specific information:
> compat: 1.1
> lazy refcounts: false
> refcount bits: 16
> corrupt: false
>
> image: base.qcow2
> file format: qcow2
> virtual size: 32M (33554432 bytes)
> disk size: 196K
> cluster_size: 65536
> Format specific information:
> compat: 1.1
> lazy refcounts: false
> refcount bits: 16
> corrupt: false
>
> As you can see, in the second case, the wrong base.qcow2 was used. I
> think qemu-img info --backing-chain should abort once it hits a point
> where has_full_backing_filename is false; but for that to work, we need
> to set info->full_backing_filename even if the "relative" and the
> absolute backing filename (backing_filename and backing_filename2) are
> equal.
>
> (By the way: It's actually some kind of a bug that you can open images
> with JSON filenames and backing files; this only works because
> bdrv_open_backing_file() is called before bdrv_refresh_filename().
> Actually, after my "Drop BDS.filename" series it will no longer work.
> This is bad. I need to fix this before I can continue the series...)
>
> ((It is bad because that means:
> $ x86_64-softmmu/qemu-system-x86_64 \
> -drive if=none,file.filename=foo/top.qcow2,lazy-refcounts=on
> qemu-system-x86_64: -drive
> if=none,file.filename=foo/top.qcow2,lazy-refcounts=on: Cannot use
> relative backing file names for 'json:{"lazy-refcounts": "on", "driver":
> "qcow2", "file": {"driver": "file", "filename": "foo/top.qcow2"}}'
>
> And that's bad.))
>
> (((So for this patch that means I guess we should just fix
> bdrv_get_full_backing_filename() instead.)))
>
> Max
>
Sigh, I guess there's no easy fixes within ten miles of this fifty car
pileup.
What's your opinion for what a meaningful fix to
bdrv_get_full_backing_filename might be?
It's not really rational for query to ever _fail_; I would be tempted to
fix any users of query information to understand what to do if that
field is absent.
--js