On 2018-08-15 10:12, Markus Armbruster wrote: > Max Reitz <mre...@redhat.com> 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