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

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Stefano Stabellini <address@hidden> writes:
>
>> On Thu, 17 Dec 2015, Markus Armbruster wrote:
>>> Cc: Stefano Stabellini <address@hidden>
>>> Cc: address@hidden
>>> Signed-off-by: Markus Armbruster <address@hidden>
>>> ---
>>>  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?

If you don't mind, I'll do (2) as a follow-up patch.  (2) because I like
it better, and follow-up patch because I'd prefer not to delay my error
pull request any longer.