Re: [Qemu-devel] [PATCH v2 33/35] blockdev: Convert drive_new() to Error

[Markus Armbruster]

Philippe Mathieu-Daudé <address@hidden> writes:

> On 15/10/2018 16:48, Max Reitz wrote:
>> On 15.10.18 13:53, Markus Armbruster wrote:
>>> Calling error_report() from within a function that takes an Error **
>>> argument is suspicious.  drive_new() calls error_report() even though
>>> it can run within drive_init_func(), which takes an Error ** argument.
>>> drive_init_func()'s caller main(), via qemu_opts_foreach(), is fine
>>> with it, but clean it up anyway:
>>>
>>> * Convert drive_new() to Error
>>>
>>> * Update add_init_drive() to report the error received from
>>>   drive_new()
>>>
>>> * Make main() pass &error_fatal through qemu_opts_foreach(),
>>>   drive_init_func() to drive_new()
>>>
>>> * Make default_drive() pass &error_abort through qemu_opts_foreach(),
>>>   drive_init_func() to drive_new()
>>>
>>> Cc: Kevin Wolf <address@hidden>
>>> Cc: Max Reitz <address@hidden>
>>> Signed-off-by: Markus Armbruster <address@hidden>
>>> ---
>>>  blockdev.c                | 27 ++++++++++++++-------------
>>>  device-hotplug.c          |  5 ++++-
>>>  include/sysemu/blockdev.h |  3 ++-
>>>  vl.c                      |  8 ++++----
>>>  4 files changed, 24 insertions(+), 19 deletions(-)
>> 
>> [...]
>> 
>>> diff --git a/vl.c b/vl.c
>>> index 65366b961e..22beca29d1 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>> 
>> [...]
>>> @@ -4396,7 +4395,8 @@ int main(int argc, char **argv, char **envp)
>>>                            NULL, NULL);
>>>      }
>>>      if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
>>> -                          &machine_class->block_default_type, NULL)) {
>>> +                          &machine_class->block_default_type, 
>>> &error_fatal)) {
>>> +        /* We printed help */
>>>          exit(1);
>>>      }
>> 
>> I thought you wanted it to become an exit(0)?  I don't care either way,
>
> Markus did it in the next patch ;)

I noticed I was about to start a commit message paragraph with "Also",
and split the patch :)

> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

Thanks!