[Qemu-devel] [PATCH] block: Use bdrv functions to replace file operation in cow.c

[Qemu-devel] [PATCH] block: Use bdrv functions to replace file operation in cow.c

[Li Zhi Hui]

Since common file operation functions lack of error detection, 
so change them to bdrv series functions.

Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
---
 block/cow.c |   63 +++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index 707c0aa..46649c2 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -138,8 +138,8 @@ static int cow_is_allocated(BlockDriverState *bs, int64_t sector_num,
     int changed;
 
     if (nb_sectors == 0) {
-	*num_same = nb_sectors;
-	return 0;
+        *num_same = nb_sectors;
+        return 0;
     }
 
     changed = is_bit_set(bs, sector_num);
@@ -148,8 +148,9 @@ static int cow_is_allocated(BlockDriverState *bs, int64_t sector_num,
     }
 
     for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) {
-	if (is_bit_set(bs, sector_num + *num_same) != changed)
-	    break;
+        if (is_bit_set(bs, sector_num + *num_same) != changed) {
+            break;
+        }
     }
 
     return changed;
@@ -243,12 +244,13 @@ static void cow_close(BlockDriverState *bs)
 
 static int cow_create(const char *filename, QEMUOptionParameter *options)
 {
-    int fd, cow_fd;
     struct cow_header_v2 cow_header;
     struct stat st;
     int64_t image_sectors = 0;
     const char *image_filename = NULL;
     int ret;
+    BlockDriverState *cow_bs;
+    BlockDriverState *image_bs;
 
     /* Read out options */
     while (options && options->name) {
@@ -260,10 +262,16 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
         options++;
     }
 
-    cow_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
-              0644);
-    if (cow_fd < 0)
-        return -errno;
+    ret = bdrv_create_file(filename, options);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = bdrv_file_open(&cow_bs, filename, BDRV_O_RDWR);
+    if (ret < 0) {
+        return ret;
+    }
+
     memset(&cow_header, 0, sizeof(cow_header));
     cow_header.magic = cpu_to_be32(COW_MAGIC);
     cow_header.version = cpu_to_be32(COW_VERSION);
@@ -271,16 +279,16 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
         /* Note: if no file, we put a dummy mtime */
         cow_header.mtime = cpu_to_be32(0);
 
-        fd = open(image_filename, O_RDONLY | O_BINARY);
-        if (fd < 0) {
-            close(cow_fd);
+        ret = bdrv_file_open(&image_bs, image_filename, BDRV_O_RDWR);
+        if (ret < 0) {
+            bdrv_close(cow_bs);
             goto mtime_fail;
         }
-        if (fstat(fd, &st) != 0) {
-            close(fd);
+        if (stat(image_filename, &st) != 0) {
+            bdrv_close(image_bs);
             goto mtime_fail;
         }
-        close(fd);
+        bdrv_close(image_bs);
         cow_header.mtime = cpu_to_be32(st.st_mtime);
     mtime_fail:
         pstrcpy(cow_header.backing_file, sizeof(cow_header.backing_file),
@@ -288,21 +296,22 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
     }
     cow_header.sectorsize = cpu_to_be32(512);
     cow_header.size = cpu_to_be64(image_sectors * 512);
-    ret = qemu_write_full(cow_fd, &cow_header, sizeof(cow_header));
+    ret = bdrv_pwrite(cow_bs, 0, &cow_header, sizeof(cow_header));
     if (ret != sizeof(cow_header)) {
         ret = -errno;
         goto exit;
     }
 
     /* resize to include at least all the bitmap */
-    ret = ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) >> 3));
+    ret = bdrv_truncate(cow_bs,
+        sizeof(cow_header) + ((image_sectors + 7) >> 3));
     if (ret) {
         ret = -errno;
         goto exit;
     }
 
 exit:
-    close(cow_fd);
+    bdrv_close(cow_bs);
     return ret;
 }
 
@@ -326,16 +335,16 @@ static QEMUOptionParameter cow_create_options[] = {
 };
 
 static BlockDriver bdrv_cow = {
-    .format_name	= "cow",
-    .instance_size	= sizeof(BDRVCowState),
-    .bdrv_probe		= cow_probe,
-    .bdrv_open		= cow_open,
-    .bdrv_read          = cow_co_read,
-    .bdrv_write         = cow_co_write,
-    .bdrv_close		= cow_close,
-    .bdrv_create	= cow_create,
+    .format_name    = "cow",
+    .instance_size  = sizeof(BDRVCowState),
+    .bdrv_probe     = cow_probe,
+    .bdrv_open      = cow_open,
+    .bdrv_read      = cow_co_read,
+    .bdrv_write     = cow_co_write,
+    .bdrv_close     = cow_close,
+    .bdrv_create    = cow_create,
     .bdrv_co_flush      = cow_co_flush,
-    .bdrv_is_allocated	= cow_is_allocated,
+    .bdrv_is_allocated  = cow_is_allocated,
 
     .create_options = cow_create_options,
 };
-- 
1.7.4.1

Re: [Qemu-devel] [PATCH] block: Use bdrv functions to replace file operation in cow.c

[Markus Armbruster]

Li Zhi Hui <zhihuili@linux.vnet.ibm.com> writes:

> Since common file operation functions lack of error detection, 
> so change them to bdrv series functions.

Looks like a few indentation fixes crept in as well.  Best to keep such
style cleanups well separated from functional changes.

Re: [Qemu-devel] [PATCH] block: Use bdrv functions to replace file operation in cow.c

[Stefan Hajnoczi]

On Fri, Nov 04, 2011 at 03:43:04PM +0800, Li Zhi Hui wrote:
> Since common file operation functions lack of error detection, 
> so change them to bdrv series functions.
> 
> Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
> ---
>  block/cow.c |   63 +++++++++++++++++++++++++++++++++-------------------------
>  1 files changed, 36 insertions(+), 27 deletions(-)

Please always send separate patches for coding style/whitespace changes.
Note that coding style changes are usually only worth it if you are
going to modify that function.  They make it harder for people trying to
follow the source history or backport commits because they perturb code.

I also suggest CCing Kevin Wolf <kwolf@redhat.com> on any block layer
changes because he is the block layer maintainer.  It's a good idea to
check ./MAINTAINERS before sending a patch so you know the right people
to CC - it helps get their attention.

Stefan