[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [RFC PATCH] rbd: Don't convert keypairs to
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [Qemu-devel] [RFC PATCH] rbd: Don't convert keypairs to JSON and back |
Date: |
Thu, 16 Aug 2018 01:55:36 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-08-15 10:12, Markus Armbruster wrote:
> Max Reitz <address@hidden> writes:
[...]
>> To me personally the issue is that if you can specify a plain filename,
>> bdrv_refresh_filename() should give you that plain filename back. So
>> rbd's implementation of that is lacking. Well, it just doesn't exist.
>
> I'm not even sure I understand what you're talking about.
We have this bdrv_refresh_filename() thing which can do this:
$ qemu-img info \
"json:{'driver':'raw',
'file':{'driver':'nbd','host':'localhost'}}"
image: nbd://localhost:10809
[...]
So it can reconstruct a plain filename even if you specify it as options
instead of just using a plain filename.
Now here's my fault: I thought it might be necessary for a driver to
implement that function (which rbd doesn't) so that you'd get a nice
filename back (instead of just json:{} garbled things). But you don't.
For protocol drivers, you'll just get the initial filename back. (So
my comment was just wrong.)
So what I was thinking about was some case where you specified a normal
plain filename and qemu would give you back json:{}. (If rbd
implemented bdrv_refresh_filename(), that wouldn't happen, because it
would reconstruct a nice normal filename.) It turns out, I don't think
that can happen so easily. You'll just get your filename back.
Because here's what I'm thinking: If someone uses an option that is
undocumented and starts with =, well, too bad. If someone uses a normal
filename, but gets back a json:{} filename... Then they are free to use
that anywhere, and their use of "=" is legitimized.
Now that issue kind of reappears when you open an RBD volume, and then
e.g. take a blockdev-snapshot. Then your overlay has an overridden
backing file (one that may be different from what its image header says)
and its filename may well become a json:{} one (to arrange for the
overridden backing file options). Of course, if you opened the RBD
volume with a filename with some of the options warranting
=keyvalue-pairs, then your json:{} filename will contain those options
under =keyvalue-pairs.
So... I'm not quite sure what I want to say? I think there are edge
cases where the user may not have put any weird option into qemu, but
they do get a json:{} filename with =keyvalue-pairs out of it. And I
think users are free to use json:{} filenames qemu spews at them, and we
can't blame them for it.
>>> If so, and we are comfortable changing the output the way this patch does
>>> (technically altering ABI anyway), we might as well go all the way and
>>> filter it out completely. That would be preferable to cleaning up the json
>>> output of the internal key/value pairs, IMO.
>>
>> Well, this filtering at least is done by my "Fix some filename
>> generation issues" series.
>
> Likewise.
The series overhauls quite a bit of the bdrv_refresh_filename()
infrastructure. That function is also responsible for generating
json:{} filenames.
One thing it introduces is a BlockDriver field where a driver can
specify which of the runtime options are actually important. The rest
is omitted from the generated json:{} filename.
I may have taken the liberty not to include =keyvalue-pairs in RBD's
"strong runtime options" list.
Max
signature.asc
Description: OpenPGP digital signature