qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] block: fix backing file segfault


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH] block: fix backing file segfault
Date: Fri, 10 Jan 2014 19:38:18 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0

On 10.01.2014 19:26, Kevin Wolf wrote:
Am 10.01.2014 um 19:05 hat Max Reitz geschrieben:
On 10.01.2014 18:55, Kevin Wolf wrote:
Ok, if you're happy with it, I'll apply it. Can I put your Reviewed-by
there?
Yes, feel free to.
Thanks, applied to the block branch.

Peter, no need for a second version of the patch then. :-)

In the long run, we need to get rid of all this copying anyway. I'm
imagining a BlockDriver function that returns a file name to reproduce
the same setup, and a removal of bs->backing_file and bs->file_name.

For some drivers, the returned filename would be a URL or some other
string that that particular driver can parse.

While doing that, we might also consider a fake protocol that handles
filenames like 'json:{"driver":"qcow2","lazy-refcounts":"on",...}',
because for some drivers this might be the only thing that comes close
to a filename as it is a single string at least...
Urgh. *g*

I'm not sure if we should force every BDS to have a clearly defining
file name. If there are options, which completely change the
behavior of the block driver (I wouldn't consider lazy-refcounts one
of them since it doesn't change the contents of the block device),
I'd rather return NULL when asked for a file name. But then again,
maybe an ugly filename is better than none at all…
We need filenames for backing file relationships. For example, when you
take a live snapshot, we need to reference the old image. If you don't
use the filename, but driver-specific options, I believe this fails
today.

You might also want to set some options for the backing file in images
created with qemu-img.

Yes, I hoped we could use the options instead. But if it fails… Maybe it's worth fixing, I don't know. ;-)

The alternative would be to extend qcow2 to have something more complex
than a string to describe backing files. However, this would mean that
qcow2 is the only possible format for live snapshots.

Well, the problem would arise only for backing files which can't be sufficiently described through a rather simple filename. If there are exceptions where we are indeed forced to specify some options, qemu would be the only program knowing how to interpret those filenames anyway, therefore, there is no point in trying to be compatible.

Max


In general, I'd prefer abandoning filenames* (especially protocol
filenames) altogether. The set of options with which to recreate the
same BDS is already available.

Max

*Of course, we need filenames for, well, opening files, but I'm
referring to have an explicit string "filename" in addition to the
option dicts (nearly) everywhere.
Agreed. The reason why filenames are still passed separately and not
converted to a file.filename QDict entry is the convenience magic that
they enable (at least protocol names) and that file.filename doesn't
have in order to have less special cases with blockdev-add.

So what you'd need to do is to parse the protocol names in the top-level
function bdrv_open() and convert them into the right QDict entries.
Perhaps this is also a better place for the .bdrv_parse_filename()
calls. And then you could call a new bdrv_file_open() that doesn't take
a separate filename argument any more.

Kevin




reply via email to

[Prev in Thread] Current Thread [Next in Thread]