Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On Sat, Jul 31, 2010 at 05:21:34PM +0200, Jes Sorensen wrote: > On 07/31/10 17:13, Aurelien Jarno wrote: > > On Sat, Jul 31, 2010 at 04:40:05PM +0200, Jes Sorensen wrote: > >> If you want that, please do it in a separate patch for the entire file, > >> otherwise it will never become consistent. However it doesn't change the > >> issue either that putting braces around a single line like this is bad > >> coding style. > > > > We got this discussions numerous times, and I am not going to start it > > again. If you want to see the patch applied, just follow the rules. > > > > Then I would like to know why you do not apply this silly rule to every > patch submitted. There's patches going in almost every day that doesn't > use this hopeless formatting. > Because not everyone try to enforce the rules. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On 07/31/10 17:13, Aurelien Jarno wrote: > On Sat, Jul 31, 2010 at 04:40:05PM +0200, Jes Sorensen wrote: >> If you want that, please do it in a separate patch for the entire file, >> otherwise it will never become consistent. However it doesn't change the >> issue either that putting braces around a single line like this is bad >> coding style. > > We got this discussions numerous times, and I am not going to start it > again. If you want to see the patch applied, just follow the rules. > Then I would like to know why you do not apply this silly rule to every patch submitted. There's patches going in almost every day that doesn't use this hopeless formatting. Jes
Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On Sat, Jul 31, 2010 at 04:40:05PM +0200, Jes Sorensen wrote: > On 07/31/10 16:19, Aurelien Jarno wrote: > > On Sat, Jul 31, 2010 at 11:16:45AM +0200, Jes Sorensen wrote: > >> On 07/30/10 23:08, Aurelien Jarno wrote: > >>> Missing braces around the return 0 line. > >> > >> Half the QEMU code base doesn't have braces around single line if > >> statements, including in hw/pci.c. Adding braces here would be > >> inconsistent. > >> > > > > If we follow the coding style in new patches, it will eventually become > > consistent. > > > > If you want that, please do it in a separate patch for the entire file, > otherwise it will never become consistent. However it doesn't change the > issue either that putting braces around a single line like this is bad > coding style. We got this discussions numerous times, and I am not going to start it again. If you want to see the patch applied, just follow the rules. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On 07/31/10 16:19, Aurelien Jarno wrote: > On Sat, Jul 31, 2010 at 11:16:45AM +0200, Jes Sorensen wrote: >> On 07/30/10 23:08, Aurelien Jarno wrote: >>> Missing braces around the return 0 line. >> >> Half the QEMU code base doesn't have braces around single line if >> statements, including in hw/pci.c. Adding braces here would be inconsistent. >> > > If we follow the coding style in new patches, it will eventually become > consistent. > If you want that, please do it in a separate patch for the entire file, otherwise it will never become consistent. However it doesn't change the issue either that putting braces around a single line like this is bad coding style. Jes
Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On Sat, Jul 31, 2010 at 11:16:45AM +0200, Jes Sorensen wrote:
> On 07/30/10 23:08, Aurelien Jarno wrote:
> > On Fri, Jul 23, 2010 at 05:56:38PM +0200, jes.soren...@redhat.com wrote:
> >> From: Jes Sorensen
> >>
> >> pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
> >> at init time, it is not possible to load an option ROM for a hotplug
> >> device when running in compat mode.
> >>
> >> v2: Alex Williamson pointed out that one can get to qdev directly from
> >> pci_dev, so no need to pass it down.
> >>
> >> Signed-off-by: Jes Sorensen
> >> ---
> >> hw/pci.c |8 ++--
> >> 1 files changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/pci.c b/hw/pci.c
> >> index a98d6f3..2d38643 100644
> >> --- a/hw/pci.c
> >> +++ b/hw/pci.c
> >> @@ -1810,11 +1810,15 @@ static int pci_add_option_rom(PCIDevice *pdev)
> >> return 0;
> >>
> >> if (!pdev->rom_bar) {
> >> +int class;
> >> /*
> >> * Load rom via fw_cfg instead of creating a rom bar,
> >> - * for 0.11 compatibility.
> >> + * for 0.11 compatibility. fw_cfg is initialized at boot, so
> >> + * we cannot do hotplug load of option roms.
> >> */
> >> -int class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
> >> +if (pdev->qdev.hotplugged)
> >> +return 0;
> >
> > Missing braces around the return 0 line.
>
> Half the QEMU code base doesn't have braces around single line if
> statements, including in hw/pci.c. Adding braces here would be inconsistent.
>
If we follow the coding style in new patches, it will eventually become
consistent.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On 07/30/10 23:08, Aurelien Jarno wrote:
> On Fri, Jul 23, 2010 at 05:56:38PM +0200, jes.soren...@redhat.com wrote:
>> From: Jes Sorensen
>>
>> pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
>> at init time, it is not possible to load an option ROM for a hotplug
>> device when running in compat mode.
>>
>> v2: Alex Williamson pointed out that one can get to qdev directly from
>> pci_dev, so no need to pass it down.
>>
>> Signed-off-by: Jes Sorensen
>> ---
>> hw/pci.c |8 ++--
>> 1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/pci.c b/hw/pci.c
>> index a98d6f3..2d38643 100644
>> --- a/hw/pci.c
>> +++ b/hw/pci.c
>> @@ -1810,11 +1810,15 @@ static int pci_add_option_rom(PCIDevice *pdev)
>> return 0;
>>
>> if (!pdev->rom_bar) {
>> +int class;
>> /*
>> * Load rom via fw_cfg instead of creating a rom bar,
>> - * for 0.11 compatibility.
>> + * for 0.11 compatibility. fw_cfg is initialized at boot, so
>> + * we cannot do hotplug load of option roms.
>> */
>> -int class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
>> +if (pdev->qdev.hotplugged)
>> +return 0;
>
> Missing braces around the return 0 line.
Half the QEMU code base doesn't have braces around single line if
statements, including in hw/pci.c. Adding braces here would be inconsistent.
Jes
Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On Fri, Jul 23, 2010 at 05:56:38PM +0200, jes.soren...@redhat.com wrote:
> From: Jes Sorensen
>
> pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
> at init time, it is not possible to load an option ROM for a hotplug
> device when running in compat mode.
>
> v2: Alex Williamson pointed out that one can get to qdev directly from
> pci_dev, so no need to pass it down.
>
> Signed-off-by: Jes Sorensen
> ---
> hw/pci.c |8 ++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index a98d6f3..2d38643 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1810,11 +1810,15 @@ static int pci_add_option_rom(PCIDevice *pdev)
> return 0;
>
> if (!pdev->rom_bar) {
> +int class;
> /*
> * Load rom via fw_cfg instead of creating a rom bar,
> - * for 0.11 compatibility.
> + * for 0.11 compatibility. fw_cfg is initialized at boot, so
> + * we cannot do hotplug load of option roms.
> */
> -int class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
> +if (pdev->qdev.hotplugged)
> +return 0;
Missing braces around the return 0 line.
> +class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
> if (class == 0x0300) {
> rom_add_vga(pdev->romfile);
> } else {
> --
> 1.7.1.1
Otherwise looks good.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
jes.soren...@redhat.com writes: > From: Jes Sorensen > > pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup > at init time, it is not possible to load an option ROM for a hotplug > device when running in compat mode. Example: $ qemu -M pc-0.11 -S -monitor stdio QEMU 0.12.50 monitor - type 'help' for more information (qemu) device_add e1000 qemu: hardware error: ROM images must be loaded at startup [...] Aborted (core dumped) The fix silently omits the option ROM when we can't load it. Works for me.
