[Qemu-devel] [PATCH 0/3] block and qemu-img: Further cleanups

[Qemu-devel] [PATCH 0/3] block and qemu-img: Further cleanups

[Stefan Hajnoczi]

These patches apply to Kevin's block branch.

[Qemu-devel] [PATCH 1/3] block: Convert first_drv to QLIST

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c     |   22 ++++++++++++----------
 block_int.h |    2 +-
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/block.c b/block.c
index 5d087de..12cf434 100644
--- a/block.c
+++ b/block.c
@@ -58,7 +58,8 @@ static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
 static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
     QTAILQ_HEAD_INITIALIZER(bdrv_states);
 
-static BlockDriver *first_drv;
+static QLIST_HEAD(, BlockDriver) bdrv_drivers =
+    QLIST_HEAD_INITIALIZER(bdrv_drivers);
 
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
@@ -142,8 +143,7 @@ void bdrv_register(BlockDriver *bdrv)
     if (!bdrv->bdrv_aio_flush)
         bdrv->bdrv_aio_flush = bdrv_aio_flush_em;
 
-    bdrv->next = first_drv;
-    first_drv = bdrv;
+    QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
 }
 
 /* create a new block device (by default it is empty) */
@@ -162,9 +162,10 @@ BlockDriverState *bdrv_new(const char *device_name)
 BlockDriver *bdrv_find_format(const char *format_name)
 {
     BlockDriver *drv1;
-    for(drv1 = first_drv; drv1 != NULL; drv1 = drv1->next) {
-        if (!strcmp(drv1->format_name, format_name))
+    QLIST_FOREACH(drv1, &bdrv_drivers, list) {
+        if (!strcmp(drv1->format_name, format_name)) {
             return drv1;
+        }
     }
     return NULL;
 }
@@ -265,10 +266,11 @@ static BlockDriver *find_protocol(const char *filename)
         len = sizeof(protocol) - 1;
     memcpy(protocol, filename, len);
     protocol[len] = '\0';
-    for(drv1 = first_drv; drv1 != NULL; drv1 = drv1->next) {
+    QLIST_FOREACH(drv1, &bdrv_drivers, list) {
         if (drv1->protocol_name &&
-            !strcmp(drv1->protocol_name, protocol))
+            !strcmp(drv1->protocol_name, protocol)) {
             return drv1;
+        }
     }
     return NULL;
 }
@@ -282,7 +284,7 @@ static BlockDriver *find_hdev_driver(const char *filename)
     int score_max = 0, score;
     BlockDriver *drv = NULL, *d;
 
-    for (d = first_drv; d; d = d->next) {
+    QLIST_FOREACH(d, &bdrv_drivers, list) {
         if (d->bdrv_probe_device) {
             score = d->bdrv_probe_device(filename);
             if (score > score_max) {
@@ -317,7 +319,7 @@ static BlockDriver *find_image_format(const char *filename)
     }
 
     score_max = 0;
-    for(drv1 = first_drv; drv1 != NULL; drv1 = drv1->next) {
+    QLIST_FOREACH(drv1, &bdrv_drivers, list) {
         if (drv1->bdrv_probe) {
             score = drv1->bdrv_probe(buf, ret, filename);
             if (score > score_max) {
@@ -1157,7 +1159,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
 {
     BlockDriver *drv;
 
-    for (drv = first_drv; drv != NULL; drv = drv->next) {
+    QLIST_FOREACH(drv, &bdrv_drivers, list) {
         it(opaque, drv->format_name);
     }
 }
diff --git a/block_int.h b/block_int.h
index 466a38c..d4067ff 100644
--- a/block_int.h
+++ b/block_int.h
@@ -126,7 +126,7 @@ struct BlockDriver {
     /* Set if newly created images are not guaranteed to contain only zeros */
     int no_zero_init;
 
-    struct BlockDriver *next;
+    QLIST_ENTRY(BlockDriver) list;
 };
 
 struct BlockDriverState {
-- 
1.7.0

[Qemu-devel] [PATCH 2/3] qemu-img: Eliminate bdrv_new_open() code duplication

[Stefan Hajnoczi]

Several commands have code to create a BlockDriverState and open a file.
The bdrv_new_open() function can be used to perform these steps.  This
patch converts the qemu-img commands to actually use bdrv_new_open().

Replaced the bdrv_new_open() 'readonly' argument with bdrv_open()-style
flags to support generic flags like BDRV_O_NO_BACKING.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 qemu-img.c |   83 +++++++----------------------------------------------------
 1 files changed, 10 insertions(+), 73 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 18e43a0..b30effa 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -190,12 +190,11 @@ static int read_password(char *buf, int buf_size)
 
 static BlockDriverState *bdrv_new_open(const char *filename,
                                        const char *fmt,
-                                       int readonly)
+                                       int flags)
 {
     BlockDriverState *bs;
     BlockDriver *drv;
     char password[256];
-    int flags = BRDV_O_FLAGS;
 
     bs = bdrv_new("");
     if (!bs)
@@ -207,9 +206,6 @@ static BlockDriverState *bdrv_new_open(const char *filename,
     } else {
         drv = NULL;
     }
-    if (!readonly) {
-        flags |= BDRV_O_RDWR;
-    }
     if (bdrv_open(bs, filename, flags, drv) < 0) {
         error("Could not open '%s'", filename);
     }
@@ -349,7 +345,7 @@ static int img_create(int argc, char **argv)
                 }
             }
 
-            bs = bdrv_new_open(backing_file->value.s, fmt, 1);
+            bs = bdrv_new_open(backing_file->value.s, fmt, BRDV_O_FLAGS);
             bdrv_get_geometry(bs, &size);
             size *= 512;
             bdrv_delete(bs);
@@ -384,7 +380,6 @@ static int img_check(int argc, char **argv)
 {
     int c, ret;
     const char *filename, *fmt;
-    BlockDriver *drv;
     BlockDriverState *bs;
 
     fmt = NULL;
@@ -405,19 +400,7 @@ static int img_check(int argc, char **argv)
         help();
     filename = argv[optind++];
 
-    bs = bdrv_new("");
-    if (!bs)
-        error("Not enough memory");
-    if (fmt) {
-        drv = bdrv_find_format(fmt);
-        if (!drv)
-            error("Unknown file format '%s'", fmt);
-    } else {
-        drv = NULL;
-    }
-    if (bdrv_open(bs, filename, BRDV_O_FLAGS, drv) < 0) {
-        error("Could not open '%s'", filename);
-    }
+    bs = bdrv_new_open(filename, fmt, BRDV_O_FLAGS);
     ret = bdrv_check(bs);
     switch(ret) {
     case 0:
@@ -443,7 +426,6 @@ static int img_commit(int argc, char **argv)
 {
     int c, ret;
     const char *filename, *fmt;
-    BlockDriver *drv;
     BlockDriverState *bs;
 
     fmt = NULL;
@@ -464,19 +446,7 @@ static int img_commit(int argc, char **argv)
         help();
     filename = argv[optind++];
 
-    bs = bdrv_new("");
-    if (!bs)
-        error("Not enough memory");
-    if (fmt) {
-        drv = bdrv_find_format(fmt);
-        if (!drv)
-            error("Unknown file format '%s'", fmt);
-    } else {
-        drv = NULL;
-    }
-    if (bdrv_open(bs, filename, BRDV_O_FLAGS | BDRV_O_RDWR, drv) < 0) {
-        error("Could not open '%s'", filename);
-    }
+    bs = bdrv_new_open(filename, fmt, BRDV_O_FLAGS | BDRV_O_RDWR);
     ret = bdrv_commit(bs);
     switch(ret) {
     case 0:
@@ -633,7 +603,7 @@ static int img_convert(int argc, char **argv)
 
     total_sectors = 0;
     for (bs_i = 0; bs_i < bs_n; bs_i++) {
-        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, 1);
+        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BRDV_O_FLAGS);
         if (!bs[bs_i])
             error("Could not open '%s'", argv[optind + bs_i]);
         bdrv_get_geometry(bs[bs_i], &bs_sectors);
@@ -691,7 +661,7 @@ static int img_convert(int argc, char **argv)
         }
     }
 
-    out_bs = bdrv_new_open(out_filename, out_fmt, 0);
+    out_bs = bdrv_new_open(out_filename, out_fmt, BRDV_O_FLAGS | BDRV_O_RDWR);
 
     bs_i = 0;
     bs_offset = 0;
@@ -889,7 +859,6 @@ static int img_info(int argc, char **argv)
 {
     int c;
     const char *filename, *fmt;
-    BlockDriver *drv;
     BlockDriverState *bs;
     char fmt_name[128], size_buf[128], dsize_buf[128];
     uint64_t total_sectors;
@@ -916,19 +885,7 @@ static int img_info(int argc, char **argv)
         help();
     filename = argv[optind++];
 
-    bs = bdrv_new("");
-    if (!bs)
-        error("Not enough memory");
-    if (fmt) {
-        drv = bdrv_find_format(fmt);
-        if (!drv)
-            error("Unknown file format '%s'", fmt);
-    } else {
-        drv = NULL;
-    }
-    if (bdrv_open(bs, filename, BRDV_O_FLAGS | BDRV_O_NO_BACKING, drv) < 0) {
-        error("Could not open '%s'", filename);
-    }
+    bs = bdrv_new_open(filename, fmt, BRDV_O_FLAGS | BDRV_O_NO_BACKING);
     bdrv_get_format(bs, fmt_name, sizeof(fmt_name));
     bdrv_get_geometry(bs, &total_sectors);
     get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
@@ -1028,13 +985,7 @@ static int img_snapshot(int argc, char **argv)
     filename = argv[optind++];
 
     /* Open the image */
-    bs = bdrv_new("");
-    if (!bs)
-        error("Not enough memory");
-
-    if (bdrv_open(bs, filename, bdrv_oflags, NULL) < 0) {
-        error("Could not open '%s'", filename);
-    }
+    bs = bdrv_new_open(filename, NULL, bdrv_oflags);
 
     /* Perform the requested action */
     switch(action) {
@@ -1080,7 +1031,7 @@ static int img_snapshot(int argc, char **argv)
 static int img_rebase(int argc, char **argv)
 {
     BlockDriverState *bs, *bs_old_backing, *bs_new_backing;
-    BlockDriver *drv, *old_backing_drv, *new_backing_drv;
+    BlockDriver *old_backing_drv, *new_backing_drv;
     char *filename;
     const char *fmt, *out_basefmt, *out_baseimg;
     int c, flags, ret;
@@ -1124,22 +1075,8 @@ static int img_rebase(int argc, char **argv)
      * Ignore the old backing file for unsafe rebase in case we want to correct
      * the reference to a renamed or moved backing file.
      */
-    bs = bdrv_new("");
-    if (!bs)
-        error("Not enough memory");
-
-    drv = NULL;
-    if (fmt) {
-        drv = bdrv_find_format(fmt);
-        if (drv == NULL) {
-            error("Invalid format name: '%s'", fmt);
-        }
-    }
-
     flags = BRDV_O_FLAGS | BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
-    if (bdrv_open(bs, filename, flags, drv) < 0) {
-        error("Could not open '%s'", filename);
-    }
+    bs = bdrv_new_open(filename, fmt, flags);
 
     /* Find the right drivers for the backing files */
     old_backing_drv = NULL;
-- 
1.7.0

[Qemu-devel] [PATCH 3/3] qemu-img: Fix BRDV_O_FLAGS typo

[Stefan Hajnoczi]

It should be BDRV_O_FLAGS instead of BRDV_O_FLAGS.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 qemu-img.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index b30effa..7203b8b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -37,7 +37,7 @@ typedef struct img_cmd_t {
 } img_cmd_t;
 
 /* Default to cache=writeback as data integrity is not important for qemu-tcg. */
-#define BRDV_O_FLAGS BDRV_O_CACHE_WB
+#define BDRV_O_FLAGS BDRV_O_CACHE_WB
 
 static void QEMU_NORETURN error(const char *fmt, ...)
 {
@@ -345,7 +345,7 @@ static int img_create(int argc, char **argv)
                 }
             }
 
-            bs = bdrv_new_open(backing_file->value.s, fmt, BRDV_O_FLAGS);
+            bs = bdrv_new_open(backing_file->value.s, fmt, BDRV_O_FLAGS);
             bdrv_get_geometry(bs, &size);
             size *= 512;
             bdrv_delete(bs);
@@ -400,7 +400,7 @@ static int img_check(int argc, char **argv)
         help();
     filename = argv[optind++];
 
-    bs = bdrv_new_open(filename, fmt, BRDV_O_FLAGS);
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS);
     ret = bdrv_check(bs);
     switch(ret) {
     case 0:
@@ -446,7 +446,7 @@ static int img_commit(int argc, char **argv)
         help();
     filename = argv[optind++];
 
-    bs = bdrv_new_open(filename, fmt, BRDV_O_FLAGS | BDRV_O_RDWR);
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
     ret = bdrv_commit(bs);
     switch(ret) {
     case 0:
@@ -603,7 +603,7 @@ static int img_convert(int argc, char **argv)
 
     total_sectors = 0;
     for (bs_i = 0; bs_i < bs_n; bs_i++) {
-        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BRDV_O_FLAGS);
+        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS);
         if (!bs[bs_i])
             error("Could not open '%s'", argv[optind + bs_i]);
         bdrv_get_geometry(bs[bs_i], &bs_sectors);
@@ -661,7 +661,7 @@ static int img_convert(int argc, char **argv)
         }
     }
 
-    out_bs = bdrv_new_open(out_filename, out_fmt, BRDV_O_FLAGS | BDRV_O_RDWR);
+    out_bs = bdrv_new_open(out_filename, out_fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
 
     bs_i = 0;
     bs_offset = 0;
@@ -885,7 +885,7 @@ static int img_info(int argc, char **argv)
         help();
     filename = argv[optind++];
 
-    bs = bdrv_new_open(filename, fmt, BRDV_O_FLAGS | BDRV_O_NO_BACKING);
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
     bdrv_get_format(bs, fmt_name, sizeof(fmt_name));
     bdrv_get_geometry(bs, &total_sectors);
     get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
@@ -1075,7 +1075,7 @@ static int img_rebase(int argc, char **argv)
      * Ignore the old backing file for unsafe rebase in case we want to correct
      * the reference to a renamed or moved backing file.
      */
-    flags = BRDV_O_FLAGS | BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
+    flags = BDRV_O_FLAGS | BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
     bs = bdrv_new_open(filename, fmt, flags);
 
     /* Find the right drivers for the backing files */
@@ -1106,7 +1106,7 @@ static int img_rebase(int argc, char **argv)
 
         bs_old_backing = bdrv_new("old_backing");
         bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
-        if (bdrv_open(bs_old_backing, backing_name, BRDV_O_FLAGS,
+        if (bdrv_open(bs_old_backing, backing_name, BDRV_O_FLAGS,
             old_backing_drv))
         {
             error("Could not open old backing file '%s'", backing_name);
@@ -1114,7 +1114,7 @@ static int img_rebase(int argc, char **argv)
         }
 
         bs_new_backing = bdrv_new("new_backing");
-        if (bdrv_open(bs_new_backing, out_baseimg, BRDV_O_FLAGS | BDRV_O_RDWR,
+        if (bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS | BDRV_O_RDWR,
             new_backing_drv))
         {
             error("Could not open new backing file '%s'", out_baseimg);
-- 
1.7.0

[Qemu-devel] Re: [PATCH 0/3] block and qemu-img: Further cleanups

[Kevin Wolf]

Am 13.04.2010 11:29, schrieb Stefan Hajnoczi:
> These patches apply to Kevin's block branch.

Thanks. Looks good, applied all to the block branch.

Kevin