[Qemu-devel] [PATCH]: block: get rid of the BDRV_O_FILE flag

[Qemu-devel] [PATCH]: block: get rid of the BDRV_O_FILE flag

[Christoph Hellwig]

BDRV_O_FILE is only used to communicate between bdrv_file_open and bdrv_open.
It affects two things:  first bdrv_open only searches for protocols using
find_protocol instead of all image formats and host drivers.  We can easily
move that to the caller and pass the found driver to bdrv_open.  Second
it is used to not force a read-write open of a snapshot file.  But we never
use bdrv_file_open to open snapshots and this behaviour doesn't make sense
to start with.

qemu-io abused the BDRV_O_FILE for it's growable option, switch it to
using bdrv_file_open to make sure we only open files as growable were
we can actually support that.

This patch requires Kevin's "[PATCH] Replace calls of old bdrv_open" to
be applied first.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: qemu/block.c
===================================================================
--- qemu.orig/block.c	2010-04-05 16:30:39.370254354 +0200
+++ qemu/block.c	2010-04-05 16:32:40.992005645 +0200
@@ -335,10 +335,16 @@ static BlockDriver *find_image_format(co
 int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags)
 {
     BlockDriverState *bs;
+    BlockDriver *drv;
     int ret;
 
+    drv = find_protocol(filename);
+    if (!drv) {
+        return -ENOENT;
+    }
+
     bs = bdrv_new("");
-    ret = bdrv_open(bs, filename, flags | BDRV_O_FILE, NULL);
+    ret = bdrv_open(bs, filename, flags, drv);
     if (ret < 0) {
         bdrv_delete(bs);
         return ret;
@@ -416,9 +422,8 @@ int bdrv_open(BlockDriverState *bs, cons
     }
 
     pstrcpy(bs->filename, sizeof(bs->filename), filename);
-    if (flags & BDRV_O_FILE) {
-        drv = find_protocol(filename);
-    } else if (!drv) {
+
+    if (!drv) {
         drv = find_hdev_driver(filename);
         if (!drv) {
             drv = find_image_format(filename);
@@ -450,14 +455,12 @@ int bdrv_open(BlockDriverState *bs, cons
      * Clear flags that are internal to the block layer before opening the
      * image.
      */
-    open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+    open_flags = flags & ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
 
     /*
      * Snapshots should be writeable.
-     *
-     * XXX(hch): and what is the point of a snapshot during a read-only open?
      */
-    if (!(flags & BDRV_O_FILE) && bs->is_temporary) {
+    if (bs->is_temporary) {
         open_flags |= BDRV_O_RDWR;
     }
 
Index: qemu/block.h
===================================================================
--- qemu.orig/block.h	2010-04-05 16:30:39.371254214 +0200
+++ qemu/block.h	2010-04-05 16:31:29.944004249 +0200
@@ -29,10 +29,6 @@ typedef struct QEMUSnapshotInfo {
 
 #define BDRV_O_RDWR        0x0002
 #define BDRV_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
-#define BDRV_O_FILE        0x0010 /* open as a raw file (do not try to
-                                     use a disk image format on top of
-                                     it (default for
-                                     bdrv_file_open()) */
 #define BDRV_O_NOCACHE     0x0020 /* do not use the host page cache */
 #define BDRV_O_CACHE_WB    0x0040 /* use write-back caching */
 #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
Index: qemu/qemu-io.c
===================================================================
--- qemu.orig/qemu-io.c	2010-04-05 16:30:39.381254145 +0200
+++ qemu/qemu-io.c	2010-04-05 16:31:29.946004598 +0200
@@ -1276,23 +1276,23 @@ static int openfile(char *name, int flag
 		return 1;
 	}
 
-	bs = bdrv_new("hda");
-	if (!bs)
-		return 1;
-
 	if (growable) {
-		flags |= BDRV_O_FILE;
-	}
+		if (bdrv_file_open(&bs, name, flags)) {
+			fprintf(stderr, "%s: can't open device %s\n", progname, name);
+			return 1;
+		}
+	} else {
+		bs = bdrv_new("hda");
+		if (!bs)
+			return 1;
 
-	if (bdrv_open(bs, name, flags, NULL) < 0) {
-		fprintf(stderr, "%s: can't open device %s\n", progname, name);
-		bs = NULL;
-		return 1;
+		if (bdrv_open(bs, name, flags, NULL) < 0) {
+			fprintf(stderr, "%s: can't open device %s\n", progname, name);
+			bs = NULL;
+			return 1;
+		}
 	}
 
-	if (growable) {
-		bs->growable = 1;
-	}
 	return 0;
 }
 

Re: [Qemu-devel] [PATCH]: block: get rid of the BDRV_O_FILE flag

[Kevin Wolf]

Am 05.04.2010 16:53, schrieb Christoph Hellwig:
> BDRV_O_FILE is only used to communicate between bdrv_file_open and bdrv_open.
> It affects two things:  first bdrv_open only searches for protocols using
> find_protocol instead of all image formats and host drivers.  We can easily
> move that to the caller and pass the found driver to bdrv_open.  Second
> it is used to not force a read-write open of a snapshot file.  But we never
> use bdrv_file_open to open snapshots and this behaviour doesn't make sense
> to start with.
> 
> qemu-io abused the BDRV_O_FILE for it's growable option, switch it to
> using bdrv_file_open to make sure we only open files as growable were
> we can actually support that.
> 
> This patch requires Kevin's "[PATCH] Replace calls of old bdrv_open" to
> be applied first.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks like a step in the right direction.

Acked-by: Kevin Wolf <kwolf@redhat.com>

[Qemu-devel] Re: [PATCH]: block: get rid of the BDRV_O_FILE flag

[Juan Quintela]

Christoph Hellwig <hch@lst.de> wrote:
> BDRV_O_FILE is only used to communicate between bdrv_file_open and bdrv_open.
> It affects two things:  first bdrv_open only searches for protocols using
> find_protocol instead of all image formats and host drivers.  We can easily
> move that to the caller and pass the found driver to bdrv_open.  Second
> it is used to not force a read-write open of a snapshot file.  But we never
> use bdrv_file_open to open snapshots and this behaviour doesn't make sense
> to start with.
>
> qemu-io abused the BDRV_O_FILE for it's growable option, switch it to
> using bdrv_file_open to make sure we only open files as growable were
> we can actually support that.
>
> This patch requires Kevin's "[PATCH] Replace calls of old bdrv_open" to
> be applied first.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH]: block: get rid of the BDRV_O_FILE flag

[Kevin Wolf]

Am 05.04.2010 16:53, schrieb Christoph Hellwig:
> This patch requires Kevin's "[PATCH] Replace calls of old bdrv_open" to
> be applied first.

Now that this is starting again, let's avoid the annoyance we had the
last time when patches were held back because they depended on patches
which depended on other patches... I have applied your patches to my
work branch at git://repo.or.cz/qemu/kevin.git block, let's base the
next patches on that branch.

Maybe I should even give that branch some testing and send a pull
request for Anthony.

Kevin