Re: [Qemu-devel] [PATCH v2] hw: fix error reporting for missing option ROMs

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2139 bytes --]

On 03/11/2016 04:18 AM, Daniel P. Berrange wrote:
> If QEMU fails to load any of the VGA ROMs, it prints a message
> to stderr and then carries on as if everything was fine, despite
> the VGA interface not being functional. This extends the the
> various rom_add_*() methods in loader.h to accept a 'Error **errp'
> parameter. The VGA device realizefn() impls can now pass in the
> errp they already have and get errors reported as fatal problems.
> 
> Addition of 'Error **errp' to the load_*() methods in loader.h is
> left as an exercise for future interested developers, since it will
> require fixing up a great many callers to propagate errors correctly.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> 
> Changed in v2:
> 
>  - Use error_fatal instead of NULL in places lacking an
>    Error **errp to propagate to
>  - Use error_setg_file_open instead of error_setg_errno
>  - Mention that load_*() methods are intentionally not converted
> 


> +++ b/hw/i386/pc_sysfw.c
> @@ -178,6 +178,7 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
>      MemoryRegion *bios, *isa_bios;
>      int bios_size, isa_bios_size;
>      int ret;
> +    Error *err = NULL;
>  
>      /* BIOS load */
>      if (bios_name == NULL) {
> @@ -199,10 +200,10 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
>      if (!isapc_ram_fw) {
>          memory_region_set_readonly(bios, true);
>      }
> -    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
> +    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1, &err);
>      if (ret != 0) {
>      bios_error:
> -        fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
> +        error_report_err(err);

You can get here through the 'goto bios_error' flow, at which point err
is not set.  You'll want to do an error_setg() just before that goto.

With that fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]