Patchwork [2/3] Update the blkmirror block driver

login
register
mail settings
Submitter Federico Simoncelli
Date Feb. 22, 2012, 5:13 p.m.
Message ID <1329930815-7995-3-git-send-email-fsimonce@redhat.com>
Download mbox | patch
Permalink /patch/142507/
State New
Headers show

Comments

Federico Simoncelli - Feb. 22, 2012, 5:13 p.m.
Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
---
 block/blkmirror.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)
Paolo Bonzini - Feb. 23, 2012, 7:18 a.m.
On 02/22/2012 06:13 PM, Federico Simoncelli wrote:
> @@ -46,7 +46,7 @@ static int blkmirror_open(BlockDriverState *bs, const char *filename, int flags)
>      filename += strlen("blkmirror:");
>  
>      /* Parse the raw image filename */
> -    filename2 = qemu_malloc(strlen(filename)+1);
> +    filename2 = qemu_vmalloc(strlen(filename)+1);
>      escape = 0;
>      for (i = n = 0; i < strlen(filename); i++) {
>          if (!escape && filename[i] == ':') {
> @@ -66,11 +66,11 @@ static int blkmirror_open(BlockDriverState *bs, const char *filename, int flags)
>  
>      m->bs[0] = bdrv_new("");
>      if (m->bs[0] == NULL) {
> -        free(filename2);
> +        qemu_vfree(filename2);
>          return -ENOMEM;
>      }
>      ret = bdrv_open(m->bs[0], filename2, flags, NULL);
> -    free(filename2);
> +    qemu_vfree(filename2);
>      if (ret < 0) {
>          return ret;
>      }

These should be g_malloc and g_free.

Also, please squash this patch in part 1.

Paolo
Federico Simoncelli - Feb. 23, 2012, 9:44 a.m.
----- Original Message -----
> From: "Paolo Bonzini" <pbonzini@redhat.com>
> To: "Federico Simoncelli" <fsimonce@redhat.com>
> Cc: qemu-devel@nongnu.org, kwolf@redhat.com, mtosatti@redhat.com
> Sent: Thursday, February 23, 2012 8:18:28 AM
> Subject: Re: [PATCH 2/3] Update the blkmirror block driver
> 
> On 02/22/2012 06:13 PM, Federico Simoncelli wrote:
> > @@ -46,7 +46,7 @@ static int blkmirror_open(BlockDriverState *bs,
> > const char *filename, int flags)
> >      filename += strlen("blkmirror:");
> >  
> >      /* Parse the raw image filename */
> > -    filename2 = qemu_malloc(strlen(filename)+1);
> > +    filename2 = qemu_vmalloc(strlen(filename)+1);
> >      escape = 0;
> >      for (i = n = 0; i < strlen(filename); i++) {
> >          if (!escape && filename[i] == ':') {
> > @@ -66,11 +66,11 @@ static int blkmirror_open(BlockDriverState *bs,
> > const char *filename, int flags)
> >  
> >      m->bs[0] = bdrv_new("");
> >      if (m->bs[0] == NULL) {
> > -        free(filename2);
> > +        qemu_vfree(filename2);
> >          return -ENOMEM;
> >      }
> >      ret = bdrv_open(m->bs[0], filename2, flags, NULL);
> > -    free(filename2);
> > +    qemu_vfree(filename2);
> >      if (ret < 0) {
> >          return ret;
> >      }
> 
> These should be g_malloc and g_free.

Thanks!

> Also, please squash this patch in part 1.

Yes, I sent it as a separate patch to make it easier to review my
changes (if someone already reviewed Marcelo's patch).
Any comment on the BDRV_O_NO_BACKING flag I added? That's the real
trick I'm pulling here. It basically allows to open the second image
only for writing and it doesn't complain if the backing file is not
there yet (it will be copied during step 4).
Paolo Bonzini - Feb. 23, 2012, 9:45 a.m.
On 02/23/2012 10:44 AM, Federico Simoncelli wrote:
> Any comment on the BDRV_O_NO_BACKING flag I added? That's the real
> trick I'm pulling here. It basically allows to open the second image
> only for writing and it doesn't complain if the backing file is not
> there yet (it will be copied during step 4).

Yes, it makes sense to me.

Paolo

Patch

diff --git a/block/blkmirror.c b/block/blkmirror.c
index 1c02710..1cfd2fb 100644
--- a/block/blkmirror.c
+++ b/block/blkmirror.c
@@ -46,7 +46,7 @@  static int blkmirror_open(BlockDriverState *bs, const char *filename, int flags)
     filename += strlen("blkmirror:");
 
     /* Parse the raw image filename */
-    filename2 = qemu_malloc(strlen(filename)+1);
+    filename2 = qemu_vmalloc(strlen(filename)+1);
     escape = 0;
     for (i = n = 0; i < strlen(filename); i++) {
         if (!escape && filename[i] == ':') {
@@ -66,11 +66,11 @@  static int blkmirror_open(BlockDriverState *bs, const char *filename, int flags)
 
     m->bs[0] = bdrv_new("");
     if (m->bs[0] == NULL) {
-        free(filename2);
+        qemu_vfree(filename2);
         return -ENOMEM;
     }
     ret = bdrv_open(m->bs[0], filename2, flags, NULL);
-    free(filename2);
+    qemu_vfree(filename2);
     if (ret < 0) {
         return ret;
     }
@@ -81,7 +81,7 @@  static int blkmirror_open(BlockDriverState *bs, const char *filename, int flags)
         bdrv_delete(m->bs[0]);
         return -ENOMEM;
     }
-    ret = bdrv_open(m->bs[1], filename, flags, NULL);
+    ret = bdrv_open(m->bs[1], filename, flags | BDRV_O_NO_BACKING, NULL);
     if (ret < 0) {
         bdrv_delete(m->bs[0]);
         return ret;
@@ -101,17 +101,17 @@  static void blkmirror_close(BlockDriverState *bs)
     }
 }
 
-static int blkmirror_flush(BlockDriverState *bs)
+static coroutine_fn int blkmirror_co_flush(BlockDriverState *bs)
 {
     BdrvMirrorState *m = bs->opaque;
     int ret;
 
-    ret = bdrv_flush(m->bs[0]);
+    ret = bdrv_co_flush(m->bs[0]);
     if (ret < 0) {
         return ret;
     }
 
-    return bdrv_flush(m->bs[1]);
+    return bdrv_co_flush(m->bs[1]);
 }
 
 static int64_t blkmirror_getlength(BlockDriverState *bs)
@@ -242,18 +242,18 @@  static BlockDriverAIOCB *blkmirror_aio_flush(BlockDriverState *bs,
     return &dcb->common;
 }
 
-static int blkmirror_discard(BlockDriverState *bs, int64_t sector_num,
-                             int nb_sectors)
+static coroutine_fn int blkmirror_co_discard(BlockDriverState *bs,
+                             int64_t sector_num, int nb_sectors)
 {
     BdrvMirrorState *m = bs->opaque;
     int ret;
 
-    ret = bdrv_discard(m->bs[0], sector_num, nb_sectors);
+    ret = bdrv_co_discard(m->bs[0], sector_num, nb_sectors);
     if (ret < 0) {
         return ret;
     }
 
-    return bdrv_discard(m->bs[1], sector_num, nb_sectors);
+    return bdrv_co_discard(m->bs[1], sector_num, nb_sectors);
 }
 
 
@@ -266,8 +266,8 @@  static BlockDriver bdrv_blkmirror = {
 
     .bdrv_file_open     = blkmirror_open,
     .bdrv_close         = blkmirror_close,
-    .bdrv_flush         = blkmirror_flush,
-    .bdrv_discard       = blkmirror_discard,
+    .bdrv_co_flush_to_disk = blkmirror_co_flush,
+    .bdrv_co_discard    = blkmirror_co_discard,
 
     .bdrv_aio_readv     = blkmirror_aio_readv,
     .bdrv_aio_writev    = blkmirror_aio_writev,