[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 03/10] block: Make bdrv_file_open() static
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 03/10] block: Make bdrv_file_open() static |
Date: |
Mon, 3 Feb 2014 11:05:21 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> >>---
> >> 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH 03/10] block: Make bdrv_file_open() static,
Kevin Wolf <=