qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 04/11] fdc: add default drive type option


From: John Snow
Subject: Re: [Qemu-devel] [PATCH v3 04/11] fdc: add default drive type option
Date: Fri, 18 Dec 2015 12:20:04 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0


On 12/18/2015 10:54 AM, Markus Armbruster wrote:
> John Snow <address@hidden> writes:
> 
>> 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
>> 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).
> 
> Err, will we have two different kinds of auto then?
> 

"No," the auto behavior will follow the same rules, but the fallback
type is configurable. Not that it's sane to do it, but it lets me
configure the new behavior in a way that's pretty solidly backwards
compatible using HW_COMPAT.

It's four steps:

1) Codify the old behavior as type=auto,fallback=144
2) Add drive sizes
3) Adjust pick_geometry to allow 1.44MB diskettes to be used in 2.88MB
drives
4) Set new behavior for 2.6+ to type=auto,fallback=288


"Why not just set the new default to type=288?"

Concerns and lack of testing that I will break esoteric targets. This
change is the safest, most minimal change I can make that accomplishes
my goal (2.88MB drives in most cases on x86 platforms to allow virtio
driver diskettes to be inserted post-boot.)

>> In order to allow users to obtain the old behaviors, though, a mechanism
>> for specifying the exact type of drive we want is needed.
> 
> I'd expect users who understand drive types sufficiently to pick one not
> to pick the old behavior, because it's *nuts*.
> 

Yes, I meant mostly if they wanted to force a 1.44MB drive type. libvirt
installations can just have you choose 120, 144 or 288 and be done with
it and always explicitly state which it wants. Easy.

Manual executions of QEMU still get the magic. The magic isn't *so* bad
now, honestly.

>> 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 <address@hidden>
>> ---
>>  hw/block/fdc.c               | 107 
>> ++++++++++++++++++++++++++-----------------
>>  hw/core/qdev-properties.c    |  11 +++++
>>  hw/i386/pc.c                 |  17 +++----
>>  include/hw/block/fdc.h       |   9 +---
>>  include/hw/qdev-properties.h |   1 +
>>  qapi/block.json              |  16 +++++++
>>  6 files changed, 102 insertions(+), 59 deletions(-)
>>
>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>> index 13fef23..ad0e052 100644
>> --- a/hw/block/fdc.c
>> +++ b/hw/block/fdc.c
>> @@ -60,58 +60,60 @@ typedef enum FDriveRate {
>>  } FDriveRate;
>>  
>>  typedef struct FDFormat {
>> -    FDriveType drive;
>> +    FloppyDriveType drive;
>>      uint8_t last_sect;
>>      uint8_t max_track;
>>      uint8_t max_head;
>>      FDriveRate rate;
>>  } FDFormat;
>>  
>> +#define FDRIVE_DEFAULT FLOPPY_DRIVE_TYPE_AUTO
>> +
>>  static const FDFormat fd_formats[] = {
>>      /* First entry is default format */
>>      /* 1.44 MB 3"1/2 floppy disks */
>> -    { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 21, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 21, 82, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 21, 83, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 22, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 23, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 24, 80, 1, FDRIVE_RATE_500K, },
>>      /* 2.88 MB 3"1/2 floppy disks */
>> -    { FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
>> -    { FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
>> -    { FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
>> -    { FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
>> -    { FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
>> +    { FLOPPY_DRIVE_TYPE_288, 36, 80, 1, FDRIVE_RATE_1M, },
>> +    { FLOPPY_DRIVE_TYPE_288, 39, 80, 1, FDRIVE_RATE_1M, },
>> +    { FLOPPY_DRIVE_TYPE_288, 40, 80, 1, FDRIVE_RATE_1M, },
>> +    { FLOPPY_DRIVE_TYPE_288, 44, 80, 1, FDRIVE_RATE_1M, },
>> +    { FLOPPY_DRIVE_TYPE_288, 48, 80, 1, FDRIVE_RATE_1M, },
>>      /* 720 kB 3"1/2 floppy disks */
>> -    { FDRIVE_DRV_144,  9, 80, 1, FDRIVE_RATE_250K, },
>> -    { FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
>> -    { FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
>> -    { FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
>> -    { FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
>> -    { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_144,  9, 80, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 10, 80, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 10, 82, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 10, 83, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 13, 80, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 14, 80, 1, FDRIVE_RATE_250K, },
>>      /* 1.2 MB 5"1/4 floppy disks */
>> -    { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
>> -    { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_120, 15, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_120, 18, 82, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_120, 18, 83, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, },
>>      /* 720 kB 5"1/4 floppy disks */
>> -    { FDRIVE_DRV_120,  9, 80, 1, FDRIVE_RATE_250K, },
>> -    { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_120,  9, 80, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_120, 11, 80, 1, FDRIVE_RATE_250K, },
>>      /* 360 kB 5"1/4 floppy disks */
>> -    { FDRIVE_DRV_120,  9, 40, 1, FDRIVE_RATE_300K, },
>> -    { FDRIVE_DRV_120,  9, 40, 0, FDRIVE_RATE_300K, },
>> -    { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
>> -    { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },
>> +    { FLOPPY_DRIVE_TYPE_120,  9, 40, 1, FDRIVE_RATE_300K, },
>> +    { FLOPPY_DRIVE_TYPE_120,  9, 40, 0, FDRIVE_RATE_300K, },
>> +    { FLOPPY_DRIVE_TYPE_120, 10, 41, 1, FDRIVE_RATE_300K, },
>> +    { FLOPPY_DRIVE_TYPE_120, 10, 42, 1, FDRIVE_RATE_300K, },
>>      /* 320 kB 5"1/4 floppy disks */
>> -    { FDRIVE_DRV_120,  8, 40, 1, FDRIVE_RATE_250K, },
>> -    { FDRIVE_DRV_120,  8, 40, 0, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_120,  8, 40, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_120,  8, 40, 0, FDRIVE_RATE_250K, },
>>      /* 360 kB must match 5"1/4 better than 3"1/2... */
>> -    { FDRIVE_DRV_144,  9, 80, 0, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_144,  9, 80, 0, FDRIVE_RATE_250K, },
>>      /* end */
>> -    { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
>> +    { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, },
>>  };
>>  
>>  #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
>> @@ -133,8 +135,9 @@ typedef struct FDrive {
>>      FDCtrl *fdctrl;
>>      BlockBackend *blk;
>>      /* Drive status */
>> -    FDriveType drive;         /* CMOS drive type */
>> -    FDriveType disk;          /* Current disk type */
>> +    FloppyDriveType drive;         /* CMOS drive type */
>> +    FloppyDriveType disk;          /* Current disk type */
>> +
>>      uint8_t perpendicular;    /* 2.88 MB access mode    */
>>      /* Position */
>>      uint8_t head;
>> @@ -155,10 +158,10 @@ typedef struct FDrive {
>>  static void fd_init(FDrive *drv)
>>  {
>>      /* Drive */
>> -    drv->drive = FDRIVE_DRV_NONE;
>> +    drv->drive = FLOPPY_DRIVE_TYPE_NONE; /* FIXME: Obey CLI properties */
>>      drv->perpendicular = 0;
>>      /* Disk */
>> -    drv->disk = FDRIVE_DRV_NONE;
>> +    drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>>      drv->last_sect = 0;
>>      drv->max_track = 0;
>>  }
>> @@ -255,11 +258,11 @@ static void pick_geometry(FDrive *drv)
>>      first_match = -1;
>>      for (i = 0; ; i++) {
>>          parse = &fd_formats[i];
>> -        if (parse->drive == FDRIVE_DRV_NONE) {
>> +        if (parse->drive == FLOPPY_DRIVE_TYPE_NONE) {
>>              break;
>>          }
>>          if (drv->drive == parse->drive ||
>> -            drv->drive == FDRIVE_DRV_NONE) {
>> +            drv->drive == FLOPPY_DRIVE_TYPE_NONE) {
>>              size = (parse->max_head + 1) * parse->max_track *
>>                  parse->last_sect;
>>              if (nb_sectors == size) {
>> @@ -288,7 +291,7 @@ static void pick_geometry(FDrive *drv)
>>      drv->max_track = parse->max_track;
>>      drv->last_sect = parse->last_sect;
>>      drv->drive = parse->drive;
>> -    drv->disk = drv->media_inserted ? parse->drive : FDRIVE_DRV_NONE;
>> +    drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE;
>>      drv->media_rate = parse->rate;
>>  
>>      if (drv->media_inserted) {
>> @@ -566,6 +569,7 @@ struct FDCtrl {
>>      /* Timers state */
>>      uint8_t timer0;
>>      uint8_t timer1;
>> +
>>  };
>>  
>>  #define TYPE_SYSBUS_FDC "base-sysbus-fdc"
>> @@ -2224,6 +2228,8 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, 
>> Error **errp)
>>          if (drive->blk) {
>>              blk_set_dev_ops(drive->blk, &fdctrl_block_ops, drive);
>>              drive->media_inserted = blk_is_inserted(drive->blk);
>> +        } else {
>> +            drive->drive = FLOPPY_DRIVE_TYPE_NONE;
>>          }
>>      }
>>  }
>> @@ -2396,7 +2402,7 @@ static void sysbus_fdc_common_realize(DeviceState 
>> *dev, Error **errp)
>>      fdctrl_realize_common(fdctrl, errp);
>>  }
>>  
>> -FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
>> +FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
>>  {
>>      FDCtrlISABus *isa = ISA_FDC(fdc);
>>  
>> @@ -2421,6 +2427,12 @@ static Property isa_fdc_properties[] = {
>>      DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].blk),
>>      DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, 
>> state.check_media_rate,
>>                      0, true),
>> +    DEFINE_PROP_DEFAULT("fdtypeA", FDCtrlISABus, state.drives[0].drive,
>> +                        FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
>> +                        FloppyDriveType),
>> +    DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlISABus, state.drives[1].drive,
>> +                        FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
>> +                        FloppyDriveType),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> @@ -2469,6 +2481,12 @@ static const VMStateDescription vmstate_sysbus_fdc ={
>>  static Property sysbus_fdc_properties[] = {
>>      DEFINE_PROP_DRIVE("driveA", FDCtrlSysBus, state.drives[0].blk),
>>      DEFINE_PROP_DRIVE("driveB", FDCtrlSysBus, state.drives[1].blk),
>> +    DEFINE_PROP_DEFAULT("fdtypeA", FDCtrlSysBus, state.drives[0].drive,
>> +                        FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
>> +                        FloppyDriveType),
>> +    DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlSysBus, state.drives[1].drive,
>> +                        FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
>> +                        FloppyDriveType),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> @@ -2489,6 +2507,9 @@ static const TypeInfo sysbus_fdc_info = {
>>  
>>  static Property sun4m_fdc_properties[] = {
>>      DEFINE_PROP_DRIVE("drive", FDCtrlSysBus, state.drives[0].blk),
>> +    DEFINE_PROP_DEFAULT("fdtype", FDCtrlSysBus, state.drives[0].drive,
>> +                        FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
>> +                        FloppyDriveType),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>> index 33e245e..b43943e 100644
>> --- a/hw/core/qdev-properties.c
>> +++ b/hw/core/qdev-properties.c
>> @@ -541,6 +541,17 @@ PropertyInfo qdev_prop_bios_chs_trans = {
>>      .set = set_enum,
>>  };
>>  
>> +/* --- FDC default drive types */
>> +
>> +PropertyInfo qdev_prop_fdc_drive_type = {
>> +    .name = "FdcDriveType",
>> +    .description = "FDC drive type, "
>> +                   "144/288/120/none/auto",
>> +    .enum_table = FloppyDriveType_lookup,
>> +    .get = get_enum,
>> +    .set = set_enum
>> +};
>> +
>>  /* --- pci address --- */
>>  
>>  /*
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 72d9b9c..02bfc74 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -207,24 +207,24 @@ static void pic_irq_request(void *opaque, int irq, int 
>> level)
>>  
>>  #define REG_EQUIPMENT_BYTE          0x14
>>  
>> -static int cmos_get_fd_drive_type(FDriveType fd0)
>> +static int cmos_get_fd_drive_type(FloppyDriveType fd0)
>>  {
>>      int val;
>>  
>>      switch (fd0) {
>> -    case FDRIVE_DRV_144:
>> +    case FLOPPY_DRIVE_TYPE_144:
>>          /* 1.44 Mb 3"5 drive */
>>          val = 4;
>>          break;
>> -    case FDRIVE_DRV_288:
>> +    case FLOPPY_DRIVE_TYPE_288:
>>          /* 2.88 Mb 3"5 drive */
>>          val = 5;
>>          break;
>> -    case FDRIVE_DRV_120:
>> +    case FLOPPY_DRIVE_TYPE_120:
>>          /* 1.2 Mb 5"5 drive */
>>          val = 2;
>>          break;
>> -    case FDRIVE_DRV_NONE:
>> +    case FLOPPY_DRIVE_TYPE_NONE:
>>      default:
>>          val = 0;
>>          break;
>> @@ -295,7 +295,8 @@ static void pc_boot_set(void *opaque, const char 
>> *boot_device, Error **errp)
>>  static void pc_cmos_init_floppy(ISADevice *rtc_state, ISADevice *floppy)
>>  {
>>      int val, nb, i;
>> -    FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
>> +    FloppyDriveType fd_type[2] = { FLOPPY_DRIVE_TYPE_NONE,
>> +                                   FLOPPY_DRIVE_TYPE_NONE };
>>  
>>      /* floppy type */
>>      if (floppy) {
>> @@ -309,10 +310,10 @@ static void pc_cmos_init_floppy(ISADevice *rtc_state, 
>> ISADevice *floppy)
>>  
>>      val = rtc_get_memory(rtc_state, REG_EQUIPMENT_BYTE);
>>      nb = 0;
>> -    if (fd_type[0] < FDRIVE_DRV_NONE) {
>> +    if (fd_type[0] != FLOPPY_DRIVE_TYPE_NONE) {
>>          nb++;
>>      }
>> -    if (fd_type[1] < FDRIVE_DRV_NONE) {
>> +    if (fd_type[1] != FLOPPY_DRIVE_TYPE_NONE) {
>>          nb++;
>>      }
>>      switch (nb) {
>> diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
>> index d48b2f8..adce14f 100644
>> --- a/include/hw/block/fdc.h
>> +++ b/include/hw/block/fdc.h
>> @@ -6,13 +6,6 @@
>>  /* fdc.c */
>>  #define MAX_FD 2
>>  
>> -typedef enum FDriveType {
>> -    FDRIVE_DRV_144  = 0x00,   /* 1.44 MB 3"5 drive      */
>> -    FDRIVE_DRV_288  = 0x01,   /* 2.88 MB 3"5 drive      */
>> -    FDRIVE_DRV_120  = 0x02,   /* 1.2  MB 5"25 drive     */
>> -    FDRIVE_DRV_NONE = 0x03,   /* No drive connected     */
>> -} FDriveType;
>> -
>>  #define TYPE_ISA_FDC "isa-fdc"
>>  
>>  ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
>> @@ -21,6 +14,6 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
>>  void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
>>                         DriveInfo **fds, qemu_irq *fdc_tc);
>>  
>> -FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
>> +FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
>>  
>>  #endif
>> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
>> index 77538a8..c26797e 100644
>> --- a/include/hw/qdev-properties.h
>> +++ b/include/hw/qdev-properties.h
>> @@ -20,6 +20,7 @@ extern PropertyInfo qdev_prop_ptr;
>>  extern PropertyInfo qdev_prop_macaddr;
>>  extern PropertyInfo qdev_prop_losttickpolicy;
>>  extern PropertyInfo qdev_prop_bios_chs_trans;
>> +extern PropertyInfo qdev_prop_fdc_drive_type;
>>  extern PropertyInfo qdev_prop_drive;
>>  extern PropertyInfo qdev_prop_netdev;
>>  extern PropertyInfo qdev_prop_vlan;
>> diff --git a/qapi/block.json b/qapi/block.json
>> index 84022f1..36d4bac 100644
>> --- a/qapi/block.json
>> +++ 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
>> +# @none: No drive connected
>> +# @auto: Automatically determined by inserted media at boot
>> +#
>> +# Since: 2.6
>> +##
>> +{ 'enum': 'FloppyDriveType',
>> +  'data': ['144', '288', '120', 'none', 'auto']}
> 
> We have to permit members names starting with a digit, but that can't
> make me like them.
> 

I wasn't aware they were such a problem. FLOPPY_DRIVE_TYPE_144 doesn't
look too bad and neither does type=144. Does it get weird somewhere else
that I am unaware of?

>> +
>> +##
>>  # @BlockdevSnapshotInternal
>>  #
>>  # @device: the name of the device to generate the snapshot from
> 
> Massive churn because you chose to rename FDriveType and its members.
> Could be avoided if anyone cares.
> 
> However, the churn makes the other changes in this patch hard to see.
> Can you split it into a pure rename (including the new QAPI type) and
> the rest?
> 

Yes, no problem. Apologies and thank you.

--js



reply via email to

[Prev in Thread] Current Thread [Next in Thread]