From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:34772)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <marcel.apfelbaum@gmail.com>) id 1XikhX-0006lT-Ow
	for qemu-devel@nongnu.org; Mon, 27 Oct 2014 09:51:24 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <marcel.apfelbaum@gmail.com>) id 1XikhO-0005xC-NA
	for qemu-devel@nongnu.org; Mon, 27 Oct 2014 09:51:15 -0400
Received: from mail-lb0-x231.google.com ([2a00:1450:4010:c04::231]:37201)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <marcel.apfelbaum@gmail.com>) id 1XikhO-0005wt-FG
	for qemu-devel@nongnu.org; Mon, 27 Oct 2014 09:51:06 -0400
Received: by mail-lb0-f177.google.com with SMTP id b6so1885480lbj.36
	for <qemu-devel@nongnu.org>; Mon, 27 Oct 2014 06:51:04 -0700 (PDT)
Message-ID: <1414417814.29756.50.camel@localhost.localdomain>
From: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Date: Mon, 27 Oct 2014 15:50:14 +0200
In-Reply-To: <1414417444-32158-1-git-send-email-marcel.a@redhat.com>
References: <1414417444-32158-1-git-send-email-marcel.a@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2] hw/pci: fixed crash when using rombar=0
 with romfile=path for hotplugged devices
Reply-To: marcel.a@redhat.com
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: pbonzini@redhat.com, alex.williamson@redhat.com, qemu-devel@nongnu.org, mst@redhat.com

On Mon, 2014-10-27 at 15:44 +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 <marcel.a@redhat.com>
> ---
> 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.
CC: Erik Blake <eblake@redhat.com>

Thanks,
Marcel

> 
>  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 {