From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45848) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAGPG-0000o9-OV for qemu-devel@nongnu.org; Mon, 03 Feb 2014 05:05:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WAGPA-000677-Bg for qemu-devel@nongnu.org; Mon, 03 Feb 2014 05:05:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10833) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAGPA-00066y-3C for qemu-devel@nongnu.org; Mon, 03 Feb 2014 05:05:28 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s13A5O5A027104 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 3 Feb 2014 05:05:27 -0500 Date: Mon, 3 Feb 2014 11:05:21 +0100 From: Kevin Wolf Message-ID: <20140203100521.GF3643@dhcp-200-207.str.redhat.com> References: <1390762963-25538-1-git-send-email-mreitz@redhat.com> <1390762963-25538-4-git-send-email-mreitz@redhat.com> <20140129132605.GG2726@dhcp-200-207.str.redhat.com> <52EC059B.10209@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <52EC059B.10209@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 03/10] block: Make bdrv_file_open() static List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Am 31.01.2014 um 21:20 hat Max Reitz geschrieben: > On 29.01.2014 14:26, Kevin Wolf wrote: > >Am 26.01.2014 um 20:02 hat Max Reitz geschrieben: > >>Add the bdrv_open() option BDRV_O_PROTOCOL which results in passing t= he > >>call to bdrv_file_open(). Additionally, make bdrv_file_open() static = and > >>therefore bdrv_open() the only way to call it. > >> > >>Consequently, all existing calls to bdrv_file_open() have to be adjus= ted > >>to use bdrv_open() with the BDRV_O_PROTOCOL flag instead. > >> > >>Signed-off-by: Max Reitz > >>--- > >> block.c | 17 ++++++++++++----- > >> block/cow.c | 6 +++--- > >> block/qcow.c | 6 +++--- > >> block/qcow2.c | 5 +++-- > >> block/qed.c | 5 +++-- > >> block/sheepdog.c | 8 +++++--- > >> block/vhdx.c | 5 +++-- > >> block/vmdk.c | 11 +++++++---- > >> include/block/block.h | 5 ++--- > >> qemu-io.c | 4 +++- > >> 10 files changed, 44 insertions(+), 28 deletions(-) > >>diff --git a/include/block/block.h b/include/block/block.h > >>index a421041..396f9ed 100644 > >>--- a/include/block/block.h > >>+++ b/include/block/block.h > >>@@ -102,6 +102,8 @@ typedef enum { > >> #define BDRV_O_CHECK 0x1000 /* open solely for consistency c= heck */ > >> #define BDRV_O_ALLOW_RDWR 0x2000 /* allow reopen to change from r= /o to r/w */ > >> #define BDRV_O_UNMAP 0x4000 /* execute guest UNMAP/TRIM oper= ations */ > >>+#define BDRV_O_PROTOCOL 0x8000 /* open the file using a protocol= instead of > >>+ a block driver */ > >Protocol drivers are a subset of block drivers, so this description > >doesn't make sense. >=20 > Hm, technically they probably are but it always seemed to me that > bdrv_open() would never directly use a protocol, instead using raw > as the format if no format was found. Except if you explicitly specify something like format=3Dfile. > More importantly, it is actually possible to use a non-protocol > block driver with BDRV_O_PROTOCOL; it just needs to be explicitly > specified. Yes, I think with explicit specification of the driver you get mostly the same results with bdrv_open() and bdrv_file_open(). > >I guess we need to list the differences between bdrv_open() and > >bdrv_file_open() in order to define what this flag really changes. I > >think this includes: > > > >- Disables format probing > >- BDRV_O_SNAPSHOT is ignored > >- No backing files are opened > >- Probably a few more >=20 > So, to me, the main difference is that bdrv_open() always uses some > non-protocol block driver, whereas bdrv_file_open() only probes for > protocol block drivers (if a non-protocol driver should be used, it > has to be explicitly specified). >=20 > The current comment for bdrv_file_open() doesn't help much, either: > =E2=80=9COpens a file using a protocol=E2=80=9D. Therefore, the easiest= way would > just be to state =E2=80=9Cbehaves like the old bdrv_file_open()=E2=80=9D= , but that > would not be very helpful. Perhaps we could formulate it like =E2=80=9C= Opens > a single file (no backing chain, etc.) using only a protocol driver > deduced from the filename, if not explicitly specified otherwise.=E2=80= =9D Perhaps we should really specify it as the difference in probing: - bdrv_open() adds a probed format layer on top of bs - bdrv_file_open() probes protocols for bs All other differences can probably go away without breaking anything: BDRV_O_SNAPSHOT can be hopefully be moved to drive_init() (the tricky one here is qmp_change_blockdev), and supporting backing files in bdrv_file_open() by moving their handling to common code shouldn't hurt. Any other differences that need to be eliminated? Once we know what the differences should be, I guess we can greatly simplify the implementation. Kevin