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

[Kevin Wolf]

Am 10.01.2014 um 19:38 hat Max Reitz geschrieben:
> 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. ;-)

You just need to make them storable in images, qcow2 at least.

The way in which I think live snapshots fail today (untested) is that
the VM continues to run happily on the new overlay image, but you can't
restart the VM because the backing file link in the image file is
missing. That's a nasty way of failure and should definitely be fixed.

> >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.

Fair enough. Let's try to cope without a json: protocol if we can.

Kevin