* [Qemu-devel] [PATCH 0/2] introduce bitmap_try_new
@ 2014-08-22 9:26 Peter Lieven
2014-08-22 9:26 ` [Qemu-devel] [PATCH 1/2] util: " Peter Lieven
2014-08-22 9:26 ` [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap Peter Lieven
0 siblings, 2 replies; 11+ messages in thread
From: Peter Lieven @ 2014-08-22 9:26 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, Peter Lieven, stefanha
this adds a new helper to handle errors when allocation a bitmap and
also introduces its first user.
Peter Lieven (2):
util: introduce bitmap_try_new
block/iscsi: handle failure on malloc of the allocationmap
block/iscsi.c | 22 +++++++++++++++-------
include/qemu/bitmap.h | 6 ++++++
2 files changed, 21 insertions(+), 7 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 1/2] util: introduce bitmap_try_new
2014-08-22 9:26 [Qemu-devel] [PATCH 0/2] introduce bitmap_try_new Peter Lieven
@ 2014-08-22 9:26 ` Peter Lieven
2014-08-25 15:09 ` Eric Blake
2014-08-22 9:26 ` [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap Peter Lieven
1 sibling, 1 reply; 11+ messages in thread
From: Peter Lieven @ 2014-08-22 9:26 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, Peter Lieven, stefanha
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 | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 1babd5d..51b430f 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -94,6 +94,12 @@ static inline unsigned long *bitmap_new(long nbits)
return g_malloc0(len);
}
+static inline unsigned long *bitmap_try_new(long nbits)
+{
+ long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
+ return g_try_malloc0(len);
+}
+
static inline void bitmap_zero(unsigned long *dst, long nbits)
{
if (small_nbits(nbits)) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap
2014-08-22 9:26 [Qemu-devel] [PATCH 0/2] introduce bitmap_try_new Peter Lieven
2014-08-22 9:26 ` [Qemu-devel] [PATCH 1/2] util: " Peter Lieven
@ 2014-08-22 9:26 ` Peter Lieven
2014-08-25 10:37 ` Paolo Bonzini
1 sibling, 1 reply; 11+ messages in thread
From: Peter Lieven @ 2014-08-22 9:26 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, Peter Lieven, stefanha
Signed-off-by: Peter Lieven <pl@kamp.de>
---
block/iscsi.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index ed883c3..131357c 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -325,6 +325,19 @@ static bool is_request_lun_aligned(int64_t sector_num, int nb_sectors,
return 1;
}
+static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun)
+{
+ unsigned long *ptr;
+ ptr = bitmap_try_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
+ iscsilun),
+ iscsilun->cluster_sectors));
+ if (ptr == NULL) {
+ error_report("iSCSI: could not initialize allocationmap. "
+ "Out of memory.");
+ }
+ return ptr;
+}
+
static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num,
int nb_sectors)
{
@@ -1413,9 +1426,7 @@ 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);
}
}
@@ -1508,10 +1519,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap
2014-08-22 9:26 ` [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap Peter Lieven
@ 2014-08-25 10:37 ` Paolo Bonzini
2014-08-25 11:10 ` Peter Lieven
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2014-08-25 10:37 UTC (permalink / raw)
To: Peter Lieven, qemu-devel; +Cc: kwolf, stefanha
Il 22/08/2014 11:26, Peter Lieven ha scritto:
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
> block/iscsi.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index ed883c3..131357c 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -325,6 +325,19 @@ static bool is_request_lun_aligned(int64_t sector_num, int nb_sectors,
> return 1;
> }
>
> +static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun)
> +{
> + unsigned long *ptr;
> + ptr = bitmap_try_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
> + iscsilun),
> + iscsilun->cluster_sectors));
> + if (ptr == NULL) {
> + error_report("iSCSI: could not initialize allocationmap. "
> + "Out of memory.");
> + }
> + return ptr;
> +}
> +
> static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num,
> int nb_sectors)
> {
> @@ -1413,9 +1426,7 @@ 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);
> }
> }
iscsi_open has an Error ** argument. Please pass it to
iscsi_allocationmap_init and use error_setg instead of error_report.
> @@ -1508,10 +1519,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);
Here you may have to use qerror_report_err, though I guess the failure
need not be fatal and you can leave the allocationmap set to NULL.
Paolo
> }
>
> return 0;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap
2014-08-25 10:37 ` Paolo Bonzini
@ 2014-08-25 11:10 ` Peter Lieven
2014-08-25 11:20 ` Paolo Bonzini
0 siblings, 1 reply; 11+ messages in thread
From: Peter Lieven @ 2014-08-25 11:10 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: kwolf, stefanha
On 25.08.2014 12:37, Paolo Bonzini wrote:
> Il 22/08/2014 11:26, Peter Lieven ha scritto:
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>> block/iscsi.c | 22 +++++++++++++++-------
>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index ed883c3..131357c 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -325,6 +325,19 @@ static bool is_request_lun_aligned(int64_t sector_num, int nb_sectors,
>> return 1;
>> }
>>
>> +static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun)
>> +{
>> + unsigned long *ptr;
>> + ptr = bitmap_try_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
>> + iscsilun),
>> + iscsilun->cluster_sectors));
>> + if (ptr == NULL) {
>> + error_report("iSCSI: could not initialize allocationmap. "
>> + "Out of memory.");
>> + }
>> + return ptr;
>> +}
>> +
>> static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num,
>> int nb_sectors)
>> {
>> @@ -1413,9 +1426,7 @@ 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);
>> }
>> }
> iscsi_open has an Error ** argument. Please pass it to
> iscsi_allocationmap_init and use error_setg instead of error_report.
I could pass the Error argument and use error_report only if the pointer is null.
>
>
>> @@ -1508,10 +1519,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);
> Here you may have to use qerror_report_err, though I guess the failure
> need not be fatal and you can leave the allocationmap set to NULL.
That was the plan. I would also not fail on iscsi_open or would you?
Peter
>
> Paolo
>
>> }
>>
>> return 0;
>>
--
Mit freundlichen Grüßen
Peter Lieven
...........................................................
KAMP Netzwerkdienste GmbH
Vestische Str. 89-91 | 46117 Oberhausen
Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
pl@kamp.de | http://www.kamp.de
Geschäftsführer: Heiner Lante | Michael Lante
Amtsgericht Duisburg | HRB Nr. 12154
USt-Id-Nr.: DE 120607556
...........................................................
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap
2014-08-25 11:10 ` Peter Lieven
@ 2014-08-25 11:20 ` Paolo Bonzini
2014-08-25 11:26 ` Peter Lieven
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2014-08-25 11:20 UTC (permalink / raw)
To: Peter Lieven, qemu-devel; +Cc: kwolf, stefanha
Il 25/08/2014 13:10, Peter Lieven ha scritto:
> On 25.08.2014 12:37, Paolo Bonzini wrote:
>> Il 22/08/2014 11:26, Peter Lieven ha scritto:
>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>> ---
>>> block/iscsi.c | 22 +++++++++++++++-------
>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>> index ed883c3..131357c 100644
>>> --- a/block/iscsi.c
>>> +++ b/block/iscsi.c
>>> @@ -325,6 +325,19 @@ static bool is_request_lun_aligned(int64_t
>>> sector_num, int nb_sectors,
>>> return 1;
>>> }
>>> +static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun)
>>> +{
>>> + unsigned long *ptr;
>>> + ptr =
>>> bitmap_try_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
>>> + iscsilun),
>>> + iscsilun->cluster_sectors));
>>> + if (ptr == NULL) {
>>> + error_report("iSCSI: could not initialize allocationmap. "
>>> + "Out of memory.");
>>> + }
>>> + return ptr;
>>> +}
>>> +
>>> static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t
>>> sector_num,
>>> int nb_sectors)
>>> {
>>> @@ -1413,9 +1426,7 @@ 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);
>>> }
>>> }
>> iscsi_open has an Error ** argument. Please pass it to
>> iscsi_allocationmap_init and use error_setg instead of error_report.
>
> I could pass the Error argument and use error_report only if the pointer
> is null.
No, NULL means "I don't care about errors, or I don't care about precise
error messages and will use the return value to check for errors".
>>> @@ -1508,10 +1519,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);
>> Here you may have to use qerror_report_err, though I guess the failure
>> need not be fatal and you can leave the allocationmap set to NULL.
>
> That was the plan. I would also not fail on iscsi_open or would you?
Hmm, good question. If so, the patch is correct with error_report. I
thought a failure in iscsi_open would be less surprising. The question
then is, do we need an option to disable allocationmap queries?
Paolo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap
2014-08-25 11:20 ` Paolo Bonzini
@ 2014-08-25 11:26 ` Peter Lieven
2014-08-25 11:42 ` Paolo Bonzini
0 siblings, 1 reply; 11+ messages in thread
From: Peter Lieven @ 2014-08-25 11:26 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: kwolf, stefanha
On 25.08.2014 13:20, Paolo Bonzini wrote:
> Il 25/08/2014 13:10, Peter Lieven ha scritto:
>> On 25.08.2014 12:37, Paolo Bonzini wrote:
>>> Il 22/08/2014 11:26, Peter Lieven ha scritto:
>>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>>> ---
>>>> block/iscsi.c | 22 +++++++++++++++-------
>>>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>>> index ed883c3..131357c 100644
>>>> --- a/block/iscsi.c
>>>> +++ b/block/iscsi.c
>>>> @@ -325,6 +325,19 @@ static bool is_request_lun_aligned(int64_t
>>>> sector_num, int nb_sectors,
>>>> return 1;
>>>> }
>>>> +static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun)
>>>> +{
>>>> + unsigned long *ptr;
>>>> + ptr =
>>>> bitmap_try_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
>>>> + iscsilun),
>>>> + iscsilun->cluster_sectors));
>>>> + if (ptr == NULL) {
>>>> + error_report("iSCSI: could not initialize allocationmap. "
>>>> + "Out of memory.");
>>>> + }
>>>> + return ptr;
>>>> +}
>>>> +
>>>> static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t
>>>> sector_num,
>>>> int nb_sectors)
>>>> {
>>>> @@ -1413,9 +1426,7 @@ 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);
>>>> }
>>>> }
>>> iscsi_open has an Error ** argument. Please pass it to
>>> iscsi_allocationmap_init and use error_setg instead of error_report.
>> I could pass the Error argument and use error_report only if the pointer
>> is null.
> No, NULL means "I don't care about errors, or I don't care about precise
> error messages and will use the return value to check for errors".
>
>>>> @@ -1508,10 +1519,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);
>>> Here you may have to use qerror_report_err, though I guess the failure
>>> need not be fatal and you can leave the allocationmap set to NULL.
>> That was the plan. I would also not fail on iscsi_open or would you?
> Hmm, good question. If so, the patch is correct with error_report. I
> thought a failure in iscsi_open would be less surprising. The question
> then is, do we need an option to disable allocationmap queries?
We have cache=none... Otherwise if allocationmap is NULL the code
can already handle it.
Peter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap
2014-08-25 11:26 ` Peter Lieven
@ 2014-08-25 11:42 ` Paolo Bonzini
0 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2014-08-25 11:42 UTC (permalink / raw)
To: Peter Lieven, qemu-devel; +Cc: kwolf, stefanha
Il 25/08/2014 13:26, Peter Lieven ha scritto:
>>>
>> Hmm, good question. If so, the patch is correct with error_report. I
>> thought a failure in iscsi_open would be less surprising. The question
>> then is, do we need an option to disable allocationmap queries?
>
> We have cache=none...
Doh, right. Then I think iscsi_open should fail if it cannot allocate
the map.
Paolo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] util: introduce bitmap_try_new
2014-08-22 9:26 ` [Qemu-devel] [PATCH 1/2] util: " Peter Lieven
@ 2014-08-25 15:09 ` Eric Blake
2014-09-05 20:00 ` Peter Lieven
0 siblings, 1 reply; 11+ messages in thread
From: Eric Blake @ 2014-08-25 15:09 UTC (permalink / raw)
To: Peter Lieven, qemu-devel; +Cc: kwolf, pbonzini, stefanha
[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]
On 08/22/2014 03:26 AM, Peter Lieven 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 | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
> index 1babd5d..51b430f 100644
> --- a/include/qemu/bitmap.h
> +++ b/include/qemu/bitmap.h
> @@ -94,6 +94,12 @@ static inline unsigned long *bitmap_new(long nbits)
> return g_malloc0(len);
> }
>
> +static inline unsigned long *bitmap_try_new(long nbits)
> +{
> + long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
> + return g_try_malloc0(len);
> +}
> +
What you have works, but I personally would have reimplemented
bitmap_new as the first caller of bitmap_try_new in this patch, where
bitmap_new handles a NULL bitmap_try_new return by abort()ing, so that
it has the same behavior. By routing the one function to use the other,
we are future-proofing: if initialization of a bitmap ever needs
modification, we only update bitmap_try_new, rather than copying the
updates to both functions.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] util: introduce bitmap_try_new
2014-08-25 15:09 ` Eric Blake
@ 2014-09-05 20:00 ` Peter Lieven
2014-09-05 22:17 ` Eric Blake
0 siblings, 1 reply; 11+ messages in thread
From: Peter Lieven @ 2014-09-05 20:00 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: kwolf, pbonzini, stefanha
Am 25.08.2014 um 17:09 schrieb Eric Blake:
> On 08/22/2014 03:26 AM, Peter Lieven 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 | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
>> index 1babd5d..51b430f 100644
>> --- a/include/qemu/bitmap.h
>> +++ b/include/qemu/bitmap.h
>> @@ -94,6 +94,12 @@ static inline unsigned long *bitmap_new(long nbits)
>> return g_malloc0(len);
>> }
>>
>> +static inline unsigned long *bitmap_try_new(long nbits)
>> +{
>> + long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
>> + return g_try_malloc0(len);
>> +}
>> +
> What you have works, but I personally would have reimplemented
> bitmap_new as the first caller of bitmap_try_new in this patch, where
> bitmap_new handles a NULL bitmap_try_new return by abort()ing, so that
> it has the same behavior. By routing the one function to use the other,
> we are future-proofing: if initialization of a bitmap ever needs
> modification, we only update bitmap_try_new, rather than copying the
> updates to both functions.
>
Good point. What would be the right exit if we receive a NULL from
bitmap_try_new? abort() ?
Peter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] util: introduce bitmap_try_new
2014-09-05 20:00 ` Peter Lieven
@ 2014-09-05 22:17 ` Eric Blake
0 siblings, 0 replies; 11+ messages in thread
From: Eric Blake @ 2014-09-05 22:17 UTC (permalink / raw)
To: Peter Lieven, qemu-devel; +Cc: kwolf, pbonzini, stefanha
[-- Attachment #1: Type: text/plain, Size: 807 bytes --]
On 09/05/2014 02:00 PM, Peter Lieven wrote:
>>> +
>> What you have works, but I personally would have reimplemented
>> bitmap_new as the first caller of bitmap_try_new in this patch, where
>> bitmap_new handles a NULL bitmap_try_new return by abort()ing, so that
>> it has the same behavior. By routing the one function to use the other,
>> we are future-proofing: if initialization of a bitmap ever needs
>> modification, we only update bitmap_try_new, rather than copying the
>> updates to both functions.
>>
>
> Good point. What would be the right exit if we receive a NULL from
> bitmap_try_new? abort() ?
I think that g_malloc() calls abort(), so yes, that sounds right.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-09-05 22:18 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-22 9:26 [Qemu-devel] [PATCH 0/2] introduce bitmap_try_new Peter Lieven
2014-08-22 9:26 ` [Qemu-devel] [PATCH 1/2] util: " Peter Lieven
2014-08-25 15:09 ` Eric Blake
2014-09-05 20:00 ` Peter Lieven
2014-09-05 22:17 ` Eric Blake
2014-08-22 9:26 ` [Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap Peter Lieven
2014-08-25 10:37 ` Paolo Bonzini
2014-08-25 11:10 ` Peter Lieven
2014-08-25 11:20 ` Paolo Bonzini
2014-08-25 11:26 ` Peter Lieven
2014-08-25 11:42 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.