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 the > >>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 adjusted > >>to use bdrv_open() with the BDRV_O_PROTOCOL flag instead. > >> > >>Signed-off-by: Max Reitz <mre...@redhat.com> > >>--- > >> 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 check */ > >> #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 operations > >> */ > >>+#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. > > 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=file. > 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 > > 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). > > The current comment for bdrv_file_open() doesn't help much, either: > “Opens a file using a protocol”. Therefore, the easiest way would > just be to state “behaves like the old bdrv_file_open()”, but that > would not be very helpful. Perhaps we could formulate it like “Opens > a single file (no backing chain, etc.) using only a protocol driver > deduced from the filename, if not explicitly specified otherwise.” 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