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

[Eric Blake <eblake@redhat.com>]

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

On 10/27/2014 10:03 AM, Michael S. Tsirkin wrote:
> On Mon, Oct 27, 2014 at 05:04:03PM +0200, Marcel Apfelbaum wrote:
>> On Mon, 2014-10-27 at 16:52 +0200, Michael S. Tsirkin wrote:
>>> 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 <marcel.a@redhat.com>
>>>
>>> How about failing adding the device instead?
>>> Return error from pci_add_option_rom, and check at
>>> calling sites?
>>
>> This was was the prev version of this patch has done,
>> we have only one calling site:  pci_qdev_init.
>> I could tweak the prev version to return error on both
>> rom_add_vga/rom_add_option, but I was under the impression
>> that silently drop the romfile was discussion's decision.
>>
>> I am fine both ways, as it is a user error and hopefully
>> used correctly by libvirt. I only want to avoid the crash.
>>
>> Thanks,
>> Marcel
> 
> Let's try the non silent error.
> If someone is unhappy we can make it silent again, but
> if it succeeds we'll have to keep it working forever.

I agree - from the libvirt perspective, it is better to be conservative
and have a noisy failure on an incompatible combination, to prove no one
is using it (or give us reason to relax it later), than to accidentally
silently accept it and change from what was requested, and then find out
people were relying on the silent change in semantics.


-- 
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: 539 bytes --]