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

[Alexander Graf]

On 19.11.2012, at 14:44, Christian Borntraeger wrote:

> 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.

IF_ are internal constants, we can change them to our liking. So if we just 
make IF_IDE=0, all is well, no? Worst case we can always have an IF_DEFAULT=0 
that falls back to IF_IDE.

I really dislike having the same list twice, just with different but almost 
identical names.


Alex