From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56787) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VoeDr-0007SY-OC for qemu-devel@nongnu.org; Thu, 05 Dec 2013 14:04:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VoeDl-0000SW-N4 for qemu-devel@nongnu.org; Thu, 05 Dec 2013 14:04:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41566) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VoeDl-0000SS-FF for qemu-devel@nongnu.org; Thu, 05 Dec 2013 14:04:21 -0500 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rB5J4JOj021032 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 5 Dec 2013 14:04:19 -0500 Message-ID: <52A0CE5A.5050309@redhat.com> Date: Thu, 05 Dec 2013 20:04:58 +0100 From: Max Reitz MIME-Version: 1.0 References: <52A0BAD8.90001@redhat.com> <52A0C77F.8060200@redhat.com> In-Reply-To: <52A0C77F.8060200@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC] Using BlockdevRef in the block layer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "qemu-devel@nongnu.org" Cc: Kevin Wolf , Stefan Hajnoczi On 05.12.2013 19:35, Max Reitz wrote: > On 05.12.2013 18:41, Max Reitz wrote: >> [=85] >> >> Second, if specifying a reference to an existing device should really=20 >> be supported, bdrv_open() should ideally not call bdrv_file_open()=20 >> anymore, but a function bdrv_find_ref() instead which resolves a=20 >> BlockdevRef structure (for simplicity, it appears to be easier to use=20 >> a QDict equivalent to a BlockdevRef instead of the latter itself=20 >> (since that results in many effectively redundant conversions to and=20 >> from those representations)). However, bdrv_file_open() supports=20 >> parsing protocol filenames, which bdrv_find_ref() would not. As a=20 >> result, it is probably best to call bdrv_find_ref() from=20 >> bdrv_file_open() instead and leave bdrv_open() generally the way it=20 >> is right now =96 yes, this is a question. ;-) (=93Do you agree?=94) > > I noticed only just now that the current design does not seem to allow=20 > nesting of files (i.e., driver=3Dblkdebug-qmp, file.driver=3Dqcow2,=20 > file.file.driver=3Dfile). Perhaps I do have to call bdrv_find_ref() in=20 > bdrv_open() and only resort to bdrv_file_open() if a filename that=20 > must be parsed was given...? Okay, I removed bdrv_find_ref() in favor of letting bdrv_file_open()=20 handle everything. If a reference to an existing block device is given=20 (as an additional parameter, since the QDict cannot handle this because=20 of the fact that a blockdev reference will replace the QDict instead of=20 being part of it), it is used. Otherwise, if the block driver found does=20 not support bdrv_file_open(), it will recursively call bdrv_open()=20 instead of bdrv_open_common(). Concept-wise, this is perhaps a pretty=20 ugly solution; but code-wise, it looks pretty well, especially=20 considering that this does not change the current code much in terms of=20 LoC changed (and, more importantly, it just works; at least apparently,=20 so far). On the other hand, as far as I know, the current =93file=94 concept is no= t=20 determined to stay anyway, is it? At some point, we will have to=20 implement something to actually chain multiple block drivers. Allowing=20 this through =93file=94 is as far as the current concept goes and there i= s=20 no way of getting around recursion. The only question to me here is whether it is a good idea to allow calls=20 to bdrv_file_open() on block drivers which don't actually support that=20 function; but I don't know how to solve this problem in any other way,=20 and I personally don't want to put too much effort into fixing the=20 current model if we want to have a new one as soon as possible anyway. Max