Re: [Qemu-devel] [Xen-devel] [PATCH v3 13/13] xen-hvm: Mark inappropriate error handling FIXME
On Wed, 13 Jan 2016, Markus Armbruster wrote:
> Markus Armbruster writes:
>
> > Stefano Stabellini writes:
> >
> >> On Thu, 17 Dec 2015, Markus Armbruster wrote:
> >>> Cc: Stefano Stabellini
> >>> Cc: xen-de...@lists.xensource.com
> >>> Signed-off-by: Markus Armbruster
> >>> ---
> >>> 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.
That's fine. Sorry, I lost your previous email.
Re: [Qemu-devel] [Xen-devel] [PATCH v3 13/13] xen-hvm: Mark inappropriate error handling FIXME
Markus Armbruster writes:
> Stefano Stabellini writes:
>
>> On Thu, 17 Dec 2015, Markus Armbruster wrote:
>>> Cc: Stefano Stabellini
>>> Cc: xen-de...@lists.xensource.com
>>> Signed-off-by: Markus Armbruster
>>> ---
>>> 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.
Re: [Qemu-devel] [Xen-devel] [PATCH v3 13/13] xen-hvm: Mark inappropriate error handling FIXME
Stefano Stabellini writes:
> On Thu, 17 Dec 2015, Markus Armbruster wrote:
>> Cc: Stefano Stabellini
>> Cc: xen-de...@lists.xensource.com
>> Signed-off-by: Markus Armbruster
>> ---
>> 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;
Re: [Qemu-devel] [Xen-devel] [PATCH v3 13/13] xen-hvm: Mark inappropriate error handling FIXME
On Thu, 17 Dec 2015, Markus Armbruster wrote:
> Cc: Stefano Stabellini
> Cc: xen-de...@lists.xensource.com
> Signed-off-by: Markus Armbruster
> ---
> 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?
> + * 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;
> --
> 2.4.3
>
>
> ___
> Xen-devel mailing list
> xen-de...@lists.xen.org
> http://lists.xen.org/xen-devel
>
