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

[Peter Lieven <pl@kamp.de>]

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

...........................................................