[Qemu-devel] [PATCHv2 0/2] introduce bitmap_try_new

[Qemu-devel] [PATCHv2 0/2] introduce bitmap_try_new

[Peter Lieven]

this adds a new helper to handle errors when allocation a bitmap and
also introduces its first user.

v1->v2: - derive bitmap_new from bitmap_try_new (Eric)
        - fail if allocation fails in iscsi_open (Paolo)

Peter Lieven (2):
  util: introduce bitmap_try_new
  block/iscsi: handle failure on malloc of the allocationmap

 block/iscsi.c         |   19 ++++++++++++-------
 include/qemu/bitmap.h |   13 +++++++++++--
 2 files changed, 23 insertions(+), 9 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCHv2 1/2] util: introduce bitmap_try_new

[Peter Lieven]

regular bitmap_new simply aborts if the memory allocation fails.
bitmap_try_new returns NULL on failure and allows for proper
error handling.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 include/qemu/bitmap.h |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 1babd5d..edf4f17 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -88,10 +88,19 @@ int slow_bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
 int slow_bitmap_intersects(const unsigned long *bitmap1,
                            const unsigned long *bitmap2, long bits);
 
-static inline unsigned long *bitmap_new(long nbits)
+static inline unsigned long *bitmap_try_new(long nbits)
 {
     long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
-    return g_malloc0(len);
+    return g_try_malloc0(len);
+}
+
+static inline unsigned long *bitmap_new(long nbits)
+{
+    unsigned long *ptr = bitmap_try_new(nbits);
+    if (ptr == NULL) {
+        abort();
+    }
+    return ptr;
 }
 
 static inline void bitmap_zero(unsigned long *dst, long nbits)
-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv2 1/2] util: introduce bitmap_try_new

[Dr. David Alan Gilbert]

* Peter Lieven (pl@kamp.de) wrote:
> regular bitmap_new simply aborts if the memory allocation fails.
> bitmap_try_new returns NULL on failure and allows for proper
> error handling.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  include/qemu/bitmap.h |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
> index 1babd5d..edf4f17 100644
> --- a/include/qemu/bitmap.h
> +++ b/include/qemu/bitmap.h
> @@ -88,10 +88,19 @@ int slow_bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
>  int slow_bitmap_intersects(const unsigned long *bitmap1,
>                             const unsigned long *bitmap2, long bits);
>  
> -static inline unsigned long *bitmap_new(long nbits)
> +static inline unsigned long *bitmap_try_new(long nbits)
>  {
>      long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
> -    return g_malloc0(len);
> +    return g_try_malloc0(len);
> +}
> +
> +static inline unsigned long *bitmap_new(long nbits)
> +{
> +    unsigned long *ptr = bitmap_try_new(nbits);
> +    if (ptr == NULL) {
> +        abort();
> +    }
> +    return ptr;
>  }

This seems like a good idea; it's probably a good idea to deprecate
use of bitmap_new and get rid of the other uses in the longterm
(there aren't that many).

size_t would be a nice type for the nbits; could you make your bitmap_try_new
use that and then fix up those users as we convert them to use the try?

Dave

>  
>  static inline void bitmap_zero(unsigned long *dst, long nbits)
> -- 
> 1.7.9.5
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCHv2 2/2] block/iscsi: handle failure on malloc of the allocationmap

[Peter Lieven]

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/iscsi.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 5c72ffe..3a01de0 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -316,6 +316,13 @@ static bool is_request_lun_aligned(int64_t sector_num, int nb_sectors,
     return 1;
 }
 
+static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun)
+{
+    return bitmap_try_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
+                                                       iscsilun),
+                                       iscsilun->cluster_sectors));
+}
+
 static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num,
                                     int nb_sectors)
 {
@@ -1402,9 +1409,10 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
         iscsilun->cluster_sectors = (iscsilun->bl.opt_unmap_gran *
                                      iscsilun->block_size) >> BDRV_SECTOR_BITS;
         if (iscsilun->lbprz && !(bs->open_flags & BDRV_O_NOCACHE)) {
-            iscsilun->allocationmap =
-                bitmap_new(DIV_ROUND_UP(bs->total_sectors,
-                                        iscsilun->cluster_sectors));
+            iscsilun->allocationmap = iscsi_allocationmap_init(iscsilun);
+            if (iscsilun->allocationmap == NULL) {
+                ret = -ENOMEM;
+            }
         }
     }
 
@@ -1497,10 +1505,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
 
     if (iscsilun->allocationmap != NULL) {
         g_free(iscsilun->allocationmap);
-        iscsilun->allocationmap =
-            bitmap_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
-                                                    iscsilun),
-                                    iscsilun->cluster_sectors));
+        iscsilun->allocationmap = iscsi_allocationmap_init(iscsilun);
     }
 
     return 0;
-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv2 0/2] introduce bitmap_try_new

[Paolo Bonzini]

Il 30/09/2014 09:09, Peter Lieven ha scritto:
> this adds a new helper to handle errors when allocation a bitmap and
> also introduces its first user.
> 
> v1->v2: - derive bitmap_new from bitmap_try_new (Eric)
>         - fail if allocation fails in iscsi_open (Paolo)

Thanks, applied to scsi-next.

Paolo