qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] block: Use bdrv functions to replace file operation in cow.c
@ 2011-11-07  6:52 Li Zhi Hui
  2011-11-07  8:34 ` Stefan Hajnoczi
  2011-11-07 11:27 ` Kevin Wolf
  0 siblings, 2 replies; 3+ messages in thread
From: Li Zhi Hui @ 2011-11-07  6:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, zhihuili, stefanha

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

v2: Only contains the function modified.
v1: Fix coding style and convert file operation functions to bdrv functions.

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

diff --git a/block/cow.c b/block/cow.c
index 707c0aa..808fe31 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -243,12 +243,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 +261,15 @@ 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 +277,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 +294,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;
 }
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH v2] block: Use bdrv functions to replace file operation in cow.c
  2011-11-07  6:52 [Qemu-devel] [PATCH v2] block: Use bdrv functions to replace file operation in cow.c Li Zhi Hui
@ 2011-11-07  8:34 ` Stefan Hajnoczi
  2011-11-07 11:27 ` Kevin Wolf
  1 sibling, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2011-11-07  8:34 UTC (permalink / raw)
  To: Li Zhi Hui; +Cc: kwolf, qemu-devel

On Mon, Nov 07, 2011 at 02:52:17PM +0800, Li Zhi Hui wrote:
> @@ -260,10 +261,15 @@ 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 +277,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);

This should be bdrv_delete() instead of bdrv_close() because
bdrv_file_open() does bdrv_new() to allocate the BlockDriverState - it
needs to be freed.

The same applies to the other hunks in this patch.

Stefan

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH v2] block: Use bdrv functions to replace file operation in cow.c
  2011-11-07  6:52 [Qemu-devel] [PATCH v2] block: Use bdrv functions to replace file operation in cow.c Li Zhi Hui
  2011-11-07  8:34 ` Stefan Hajnoczi
@ 2011-11-07 11:27 ` Kevin Wolf
  1 sibling, 0 replies; 3+ messages in thread
From: Kevin Wolf @ 2011-11-07 11:27 UTC (permalink / raw)
  To: Li Zhi Hui; +Cc: qemu-devel, stefanha

Am 07.11.2011 07:52, schrieb Li Zhi Hui:
> Since common file operation functions lack of error detection, 
> so change them to bdrv series functions.
> 
> v2: Only contains the function modified.
> v1: Fix coding style and convert file operation functions to bdrv functions.
> 
> Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
> ---
>  block/cow.c |   35 +++++++++++++++++++++--------------
>  1 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/block/cow.c b/block/cow.c
> index 707c0aa..808fe31 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -243,12 +243,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 +261,15 @@ 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 +277,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);

Why do you open image_bs? The only thing you do with it is closing it.

>          cow_header.mtime = cpu_to_be32(st.st_mtime);
>      mtime_fail:
>          pstrcpy(cow_header.backing_file, sizeof(cow_header.backing_file),
> @@ -288,21 +294,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;

This is wrong, ret already has the right value, and errno might be
different.

>          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;

Same here.

>          goto exit;
>      }
>  
>  exit:
> -    close(cow_fd);
> +    bdrv_close(cow_bs);
>      return ret;
>  }

This should be bdrv_delete.

Kevin

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-11-07 11:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-07  6:52 [Qemu-devel] [PATCH v2] block: Use bdrv functions to replace file operation in cow.c Li Zhi Hui
2011-11-07  8:34 ` Stefan Hajnoczi
2011-11-07 11:27 ` Kevin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).