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