From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39267) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1gnR-0007pr-Nw for qemu-devel@nongnu.org; Fri, 10 Jan 2014 13:27:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W1gnL-0004TD-Mm for qemu-devel@nongnu.org; Fri, 10 Jan 2014 13:27:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52920) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1gnL-0004T8-EL for qemu-devel@nongnu.org; Fri, 10 Jan 2014 13:26:59 -0500 Date: Fri, 10 Jan 2014 19:26:54 +0100 From: Kevin Wolf Message-ID: <20140110182654.GH4276@dhcp-200-207.str.redhat.com> References: <1389210205-10787-1-git-send-email-peter@gridcentric.ca> <20140109105930.GA2862@dhcp-200-207.str.redhat.com> <52D02D96.3010901@redhat.com> <20140110175518.GG4276@dhcp-200-207.str.redhat.com> <52D03651.5010603@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <52D03651.5010603@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] block: fix backing file segfault List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: famz@redhat.com, Peter Feiner , qemu-devel@nongnu.org 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? >=20 > 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... >=20 > Urgh. *g* >=20 > 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=E2=80=A6 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. >=20 > Max >=20 > *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