Re: [Qemu-devel] [PATCH v2] hw/pci: fixed crash when using rombar=0 with romfile=path for hotplugged devices

[Michael S. Tsirkin]

On Mon, Oct 27, 2014 at 03:44:04PM +0200, Marcel Apfelbaum wrote:
> Combining rombar=0 with romfile=<path> is an user error,
> silently dropping the romfile is a reasonable thing to do.
> 
> Signed-off-by: Marcel Apfelbaum <address@hidden>

How about failing adding the device instead?
Return error from pci_add_option_rom, and check at
calling sites?

> ---
> v1 -> v2:
>  After a discussion with Michael, Paolo and Alex, this
>  patch silent drops the romfile instead of not allowing
>  the hotplug.
>  
>  An OK from libvirt will be nice.
> 
>  hw/pci/pci.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 6ce75aa..cd7a403 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1776,7 +1776,12 @@ static int pci_qdev_init(DeviceState *qdev)
>          pci_dev->romfile = g_strdup(pc->romfile);
>          is_default_rom = true;
>      }
> -    pci_add_option_rom(pci_dev, is_default_rom);
> +
> +    rc = pci_add_option_rom(pci_dev, is_default_rom);
> +    if (rc != 0) {
> +        pci_unregister_device(DEVICE(pci_dev));
> +        return rc;
> +    }
>  
>      return 0;
>  }
> @@ -1937,6 +1942,15 @@ static int pci_add_option_rom(PCIDevice *pdev, bool 
> is_default_rom)
>           * for 0.11 compatibility.
>           */
>          int class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
> +
> +        /*
> +         * For hot-plugged devices silently ignore the option ROM
> +         * if the rom bar is disabled.
> +         */
> +        if (DEVICE(pdev)->hotplugged) {
> +            return 0;
> +        }
> +
>          if (class == 0x0300) {
>              rom_add_vga(pdev->romfile);
>          } else {
> -- 
> 1.8.3.1