[Qemu-devel] [PATCH] q35: Re-enable FDC on pc-q35-2.3 and older

[Qemu-devel] [PATCH] q35: Re-enable FDC on pc-q35-2.3 and older

[Eduardo Habkost]

commit ea96bc629cbd52be98b2967a4b4f72e91dfc3ee4 doesn't match the patch
submitted by Laszlo to qemu-devel. We reuse pc_q35_2_4_machine_options()
inside pc_q35_2_3_machine_options(), so we need to undo the no_floppy
change in pc_q35_2_3_machine_options().

This restores the previous behavior where all the 2.3 and older machines
had no_floppy=0.

Reported-by: Ján Tomko <jtomko@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_q35.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b68263d..082cd93 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -403,6 +403,7 @@ DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
 static void pc_q35_2_3_machine_options(MachineClass *m)
 {
     pc_q35_2_4_machine_options(m);
+    m->no_floppy = 0;
     m->alias = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
 }
-- 
2.1.0

Re: [Qemu-devel] [PATCH] q35: Re-enable FDC on pc-q35-2.3 and older

[Laszlo Ersek]

On 06/18/15 18:49, Eduardo Habkost wrote:
> commit ea96bc629cbd52be98b2967a4b4f72e91dfc3ee4 doesn't match the patch
> submitted by Laszlo to qemu-devel. We reuse pc_q35_2_4_machine_options()
> inside pc_q35_2_3_machine_options(), so we need to undo the no_floppy
> change in pc_q35_2_3_machine_options().
> 
> This restores the previous behavior where all the 2.3 and older machines
> had no_floppy=0.
> 
> Reported-by: Ján Tomko <jtomko@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/i386/pc_q35.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index b68263d..082cd93 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -403,6 +403,7 @@ DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
>  static void pc_q35_2_3_machine_options(MachineClass *m)
>  {
>      pc_q35_2_4_machine_options(m);
> +    m->no_floppy = 0;
>      m->alias = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
>  }
> 

Looks good to me, thanks! In fact I'm just learning that under the new
"machine type knobs" infrastructure, it's idiomatic to override settings
(ie. to assign the same field several times). Resetting m->alias to NULL
is an example.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Cheers
Laszlo

Re: [Qemu-devel] [PATCH] q35: Re-enable FDC on pc-q35-2.3 and older

[Markus Armbruster]

Laszlo Ersek <lersek@redhat.com> writes:

> On 06/18/15 18:49, Eduardo Habkost wrote:
>> commit ea96bc629cbd52be98b2967a4b4f72e91dfc3ee4 doesn't match the patch
>> submitted by Laszlo to qemu-devel. We reuse pc_q35_2_4_machine_options()
>> inside pc_q35_2_3_machine_options(), so we need to undo the no_floppy
>> change in pc_q35_2_3_machine_options().
>> 
>> This restores the previous behavior where all the 2.3 and older machines
>> had no_floppy=0.

Mentioning it was a botched merge, not a review failure wouldn't hurt.

>> 
>> Reported-by: Ján Tomko <jtomko@redhat.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>  hw/i386/pc_q35.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index b68263d..082cd93 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -403,6 +403,7 @@ DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
>>  static void pc_q35_2_3_machine_options(MachineClass *m)
>>  {
>>      pc_q35_2_4_machine_options(m);
>> +    m->no_floppy = 0;
>>      m->alias = NULL;
>>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
>>  }
>> 
>
> Looks good to me, thanks! In fact I'm just learning that under the new
> "machine type knobs" infrastructure, it's idiomatic to override settings
> (ie. to assign the same field several times). Resetting m->alias to NULL
> is an example.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Peter, could you pick this up directly as a regression fix?

Re: [Qemu-devel] [PATCH] q35: Re-enable FDC on pc-q35-2.3 and older

[Peter Maydell]

On 19 June 2015 at 07:43, Markus Armbruster <armbru@redhat.com> wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
>
>> On 06/18/15 18:49, Eduardo Habkost wrote:
>>> commit ea96bc629cbd52be98b2967a4b4f72e91dfc3ee4 doesn't match the patch
>>> submitted by Laszlo to qemu-devel. We reuse pc_q35_2_4_machine_options()
>>> inside pc_q35_2_3_machine_options(), so we need to undo the no_floppy
>>> change in pc_q35_2_3_machine_options().
>>>
>>> This restores the previous behavior where all the 2.3 and older machines
>>> had no_floppy=0.
>
> Mentioning it was a botched merge, not a review failure wouldn't hurt.
>
>>>
>>> Reported-by: Ján Tomko <jtomko@redhat.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>> ---
>>>  hw/i386/pc_q35.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>>> index b68263d..082cd93 100644
>>> --- a/hw/i386/pc_q35.c
>>> +++ b/hw/i386/pc_q35.c
>>> @@ -403,6 +403,7 @@ DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
>>>  static void pc_q35_2_3_machine_options(MachineClass *m)
>>>  {
>>>      pc_q35_2_4_machine_options(m);
>>> +    m->no_floppy = 0;
>>>      m->alias = NULL;
>>>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
>>>  }
>>>
>>
>> Looks good to me, thanks! In fact I'm just learning that under the new
>> "machine type knobs" infrastructure, it's idiomatic to override settings
>> (ie. to assign the same field several times). Resetting m->alias to NULL
>> is an example.
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> Peter, could you pick this up directly as a regression fix?

Sure; applied to master (with a note about it being a bad merge).

thanks
-- PMM

Re: [Qemu-devel] [PATCH] q35: Re-enable FDC on pc-q35-2.3 and older

[Eduardo Habkost]

On Fri, Jun 19, 2015 at 08:43:29AM +0200, Markus Armbruster wrote:
> Laszlo Ersek <lersek@redhat.com> writes:
> 
> > On 06/18/15 18:49, Eduardo Habkost wrote:
> >> commit ea96bc629cbd52be98b2967a4b4f72e91dfc3ee4 doesn't match the patch
> >> submitted by Laszlo to qemu-devel. We reuse pc_q35_2_4_machine_options()
> >> inside pc_q35_2_3_machine_options(), so we need to undo the no_floppy
> >> change in pc_q35_2_3_machine_options().
> >> 
> >> This restores the previous behavior where all the 2.3 and older machines
> >> had no_floppy=0.
> 
> Mentioning it was a botched merge, not a review failure wouldn't hurt.

That's what I tried to say when I wrote that the commit doesn't match
the patch that Laszlo submitted to qemu-devel.

-- 
Eduardo