Re: [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option

[John Snow <jsnow@redhat.com>]

On 12/07/2015 06:56 PM, Eric Blake wrote:
> On 12/07/2015 04:34 PM, John Snow wrote:
>> This patch adds a new explicit Floppy Drive Type option. The existing
>> behavior in QEMU is to automatically guess a drive type based on the
>> media inserted, or if a diskette is not present, arbitrarily assign one.
>>
>> This behavior can be described as "auto." This patch adds explicit
>> behaviors: 120, 144, 288, auto, and none. The new "auto" behavior
>> is intended to mimick current behavior, while the other types pick
> 
> s/mimick/mimic/ (one of those weird 'ic' verbs where the 'k' is
> necessary in past tense but not present tense)
> 

>> one explicitly.
>>
>> In a future patch, the goal is to change the FDC's default drive type
>> from auto (falling back to 1.44MB) to auto (falling back to 2.88MB).
>>
>> In order to allow users to obtain the old behaviors, though, a mechanism
>> for specifying the exact type of drive we want is needed.
>>
>> This patch adds the properties, but it is not acted on yet in favor of
>> making those changes a little more explicitly clear in standalone patches
>> later in this patch set.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
> 
>> +++ b/qapi/block.json
>> @@ -40,6 +40,22 @@
>>    'data': ['auto', 'none', 'lba', 'large', 'rechs']}
>>  
>>  ##
>> +# @FloppyDriveType
>> +#
>> +# Type of Floppy drive to be emulated by the Floppy Disk Controller.
>> +#
>> +# @144:  1.44MB 3.5" drive
>> +# @288:  2.88MB 3.5" drive
>> +# @120:  1.5MB 5.25" drive
> 
> Names start with a digit - not the prettiest, but also not the first
> instance, so qapi handles it just fine.  And I don't have any
> suggestions for a better yet still concise name.
> 
>> +# @none: No drive connected
>> +# @auto: Automatically determined by inserted media at boot
>> +#
>> +# Since: 2.6
>> +##
>> +{ 'enum': 'FloppyDriveType',
>> +  'data': ['144', '288', '120', 'none', 'auto']}
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

I was actually contemplating re-spinning this for a v3:

Instead of having a "typeA" and "typeB" properties of the FDC, I'll just
spin the properties in such a way that they write directly to
FDCtrl.drives[n].drive, which avoids the need for two new members and a
method designed to "fetch" the default from the controller.

I also want to add a "fallback" member to the FDC as a CLI configurable
parameter such that when using "auto" you can configure directly what
type of drive you'll get.

This way the default behavior will be "auto" (but configurable) and the
fallback if the drive is empty or it cannot make a confident guess will
be whatever the user chose as "fallback" -- presumed "144" before this
series and "288" afterwards.

I believe this also opens up the possibility for having "fallback"
default to "144" for machine types created prior to 2.6 ... I think.
(I'm less familiar with machine compat code as of yet.)

Sound reasonable?

--js