Re: [Qemu-devel] [PATCH 1/1] Support default block interfaces per QEMUMachine

[Christian Borntraeger]

On 19/11/12 14:36, Alexander Graf wrote:
> 
> On 12.11.2012, at 09:22, Christian Borntraeger wrote:
> 
>> There are QEMUMachines that have neither IF_IDE nor IF_SCSI as a
>> default/standard interface to their block devices / drives. Therfore,
>> this patch introduces a new field default_block per QEMUMachine struct.
>> The prior use_scsi field becomes thereby obsolete and is replaced through
>> .default_block = DEFAULT_SCSI.
>>
>> Based on an initial patch from Einar Lueck <address@hidden>
>>
>> Signed-off-by: Christian Borntraeger <address@hidden>
>> ---
>> blockdev.c          |    4 ++--
>> blockdev.h          |   29 ++++++++++++++++++++++++++++-
>> hw/boards.h         |    3 ++-
>> hw/device-hotplug.c |    2 +-
>> hw/highbank.c       |    2 +-
>> hw/leon3.c          |    1 -
>> hw/mips_jazz.c      |    4 ++--
>> hw/pc_sysfw.c       |    2 +-
>> hw/puv3.c           |    1 -
>> hw/realview.c       |    7 ++++---
>> hw/s390-virtio.c    |   16 +---------------
>> hw/spapr.c          |    2 +-
>> hw/sun4m.c          |   24 ++++++++++++------------
>> hw/versatilepb.c    |    4 ++--
>> hw/vexpress.c       |    4 ++--
>> hw/xilinx_zynq.c    |    2 +-
>> vl.c                |   20 +++++++++++---------
>> 17 files changed, 71 insertions(+), 56 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index e73fd6e..aca3c14 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -275,7 +275,7 @@ static bool do_check_io_limits(BlockIOLimit *io_limits)
>>     return true;
>> }
>>
>> -DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>> +DriveInfo *drive_init(QemuOpts *opts, BlockDefault block_default)
>> {
>>     const char *buf;
>>     const char *file = NULL;
>> @@ -325,7 +325,7 @@ DriveInfo *drive_init(QemuOpts *opts, int 
>> default_to_scsi)
>>             return NULL;
>>      }
>>     } else {
>> -        type = default_to_scsi ? IF_SCSI : IF_IDE;
>> +        type = block_if_from_default(block_default);
>>     }
>>
>>     max_devs = if_max_devs[type];
>> diff --git a/blockdev.h b/blockdev.h
>> index 5f27b64..aba6d77 100644
>> --- a/blockdev.h
>> +++ b/blockdev.h
>> @@ -24,6 +24,33 @@ typedef enum {
>>     IF_COUNT
>> } BlockInterfaceType;
>>
>> +/* For machine default interface. */
>> +typedef enum {
>> +    DEFAULT_IDE = 0,
>> +    DEFAULT_SCSI,
>> +    DEFAULT_FLOPPY,
>> +    DEFAULT_PFLASH,
>> +    DEFAULT_MTD,
>> +    DEFAULT_SD,
>> +    DEFAULT_VIRTIO,
>> +    DEFAULT_XEN
>> +} BlockDefault;
> 
> Why a new enum? Can't we just reuse the IF_ ones?
> 
> Also, traditionally Markus has had very strong opinions on anything IF_ and 
> -drive related. It's probably a good idea to get him in the loop :).
> 
> Alex

Review feedback from the first cycle. Anthony wanted to make the default (field 
is not specified at all) a
sane default, which means IDE must be 0.