Re: [Qemu-devel] [PATCH 5/5] Add bootindex parameter to net/block/fd device

[Anthony Liguori <anthony@codemonkey.ws>]

On 10/26/2010 05:48 AM, Gleb Natapov wrote:
> If bootindex is specified on command line a string that describes device
> in firmware readable way is added into sorted list. Later this list will
> be passed into firmware to control boot order.
>
> Signed-off-by: Gleb Natapov<gleb@redhat.com>
> ---
>   block_int.h     |    4 +++-
>   hw/fdc.c        |   36 ++++++++++++++++++++++++++++++++++++
>   hw/ide/qdev.c   |   24 ++++++++++++++++++++++++
>   hw/virtio-blk.c |   20 ++++++++++++++++++++
>   hw/virtio-net.c |   20 ++++++++++++++++++++
>   net.h           |    4 +++-
>   sysemu.h        |    9 +++++++++
>   vl.c            |   24 ++++++++++++++++++++++++
>   8 files changed, 139 insertions(+), 2 deletions(-)
>
> diff --git a/block_int.h b/block_int.h
> index e8e7156..60e7be2 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -225,6 +225,7 @@ typedef struct BlockConf {
>       uint16_t logical_block_size;
>       uint16_t min_io_size;
>       uint32_t opt_io_size;
> +    int32_t bootindex;
>   } BlockConf;
>
>   static inline unsigned int get_physical_block_exp(BlockConf *conf)
> @@ -247,6 +248,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
>       DEFINE_PROP_UINT16("physical_block_size", _state,                   \
>                          _conf.physical_block_size, 512),                 \
>       DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
> -    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0)
> +    DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),    \
> +    DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1)         \
>
>   #endif /* BLOCK_INT_H */
> diff --git a/hw/fdc.c b/hw/fdc.c
> index 1f38d0d..9d0dff5 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -35,6 +35,7 @@
>   #include "sysbus.h"
>   #include "qdev-addr.h"
>   #include "blockdev.h"
> +#include "sysemu.h"
>
>   /********************************************************/
>   /* debug Floppy devices */
> @@ -523,6 +524,8 @@ typedef struct FDCtrlSysBus {
>   typedef struct FDCtrlISABus {
>       ISADevice busdev;
>       struct FDCtrl state;
> +    int32_t bootindexA;
> +    int32_t bootindexB;
>   } FDCtrlISABus;
>
>   static uint32_t fdctrl_read (void *opaque, uint32_t reg)
> @@ -1974,6 +1977,7 @@ static int isabus_fdc_init1(ISADevice *dev)
>       int isairq = 6;
>       int dma_chann = 2;
>       int ret;
> +    char devpath[30], *bus_name;
>
>       register_ioport_read(iobase + 0x01, 5, 1,
>                            &fdctrl_read_port, fdctrl);
> @@ -1992,6 +1996,36 @@ static int isabus_fdc_init1(ISADevice *dev)
>       qdev_set_legacy_instance_id(&dev->qdev, iobase, 2);
>       ret = fdctrl_init_common(fdctrl);
>
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    if (isa->bootindexA<  0&&  isa->bootindexB<  0) {
> +        return 0;
> +    }
> +
> +    if (!dev->qdev.parent_bus->info->get_dev_path) {
> +        fprintf(stderr, "Can't create device path for floppy\n");
> +        return 0;
> +    }
> +
> +    bus_name = dev->qdev.parent_bus->info->get_dev_path(&dev->qdev);
> +
> +    if (isa->bootindexA>= 0) {
> +        snprintf(devpath, sizeof(devpath), "%s@%s/fd@a",
> +                 dev->qdev.parent_bus->info->name, bus_name);
> +
> +        add_boot_device_path(isa->bootindexA, strdup(devpath));
> +    }
>    

Even if we're passing a string to SeaBIOS, we should probably keep the 
components structured.  Looks like there's four pieces of data: 1) a bus 
name 2) a bus path 3) a device name 4) a device path.

We derive (1) from the qdev structure, why don't we derive (3) too?

Regards,

Anthony Liguori