Re: [Xen-devel] [Qemu-devel] [PATCH v3 13/13] xen-hvm: Mark inappropriate error handling FIXME

[Markus Armbruster <armbru@xxxxxxxxxx>]

Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> writes:

> On Thu, 17 Dec 2015, Markus Armbruster wrote:
>> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
>> Signed-off-by: Markus Armbruster <armbru@xxxxxxxxxx>
>> ---
>>  xen-hvm.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>> 
>> diff --git a/xen-hvm.c b/xen-hvm.c
>> index 3d78a0c..2a93390 100644
>> --- a/xen-hvm.c
>> +++ b/xen-hvm.c
>> @@ -240,6 +240,7 @@ static void xen_ram_init(PCMachineState *pcms,
>>  
>>  void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr)
>>  {
>> +    /* FIXME caller ram_block_add() wants error_setg() on failure */
>>      unsigned long nr_pfn;
>>      xen_pfn_t *pfn_list;
>>      int i;
>> @@ -1192,6 +1193,12 @@ static void xen_wakeup_notifier(Notifier *notifier, 
>> void *data)
>>  int xen_hvm_init(PCMachineState *pcms,
>>                   MemoryRegion **ram_memory)
>>  {
>> +    /*
>> +     * FIXME Returns -1 without cleaning up on some errors (harmless
>> +     * as long as the caller exit()s on error), dies with hw_error()
>
> Could you please stop the comment here and just replace hw_error() with
> either return -1 or exit(1) within xen_hvm_init?

If compile-testing is okay, I can either

(1) replace hw_error() by return -1, or

(2) replace both hw_error() and return -1 by exit(1), and make
xen_hvm_init() return void.

Your pick?

>
>> +     * on others.  hw_error() isn't approprate here.  Should probably
>> +     * simply exit() on all errors.
>> +     */
>>      int i, rc;
>>      xen_pfn_t ioreq_pfn;
>>      xen_pfn_t bufioreq_pfn;

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel