[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 24/28] ide: add bootindex to qom property
From: |
Gonglei (Arei) |
Subject: |
Re: [Qemu-devel] [PATCH v7 24/28] ide: add bootindex to qom property |
Date: |
Wed, 10 Sep 2014 01:21:15 +0000 |
Hi,
> From: Eduardo Habkost [mailto:address@hidden
> > > > Subject: Re: [Qemu-devel] [PATCH v7 24/28] ide: add bootindex to qom
> > > property
> > > >
> > > > On Fri, Sep 05, 2014 at 04:37:32PM +0800, address@hidden
> wrote:
> > > > > From: Gonglei <address@hidden>
> > > > >
> > > > > Add a qom property with the same name 'bootindex',
> > > > > when we remove it form qdev property, things will
> > > > > continue to work just fine, and we can use qom features
> > > > > which are not supported by qdev property.
> > > > >
> > > > > Signed-off-by: Gonglei <address@hidden>
> > > > > ---
> > > > > hw/ide/qdev.c | 14 ++++++++++++++
> > > > > 1 file changed, 14 insertions(+)
> > > > >
> > > > > diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> > > > > index efab95b..9e2ed40 100644
> > > > > --- a/hw/ide/qdev.c
> > > > > +++ b/hw/ide/qdev.c
> > > > > @@ -191,6 +191,17 @@ static int ide_dev_initfn(IDEDevice *dev,
> > > > IDEDriveKind kind)
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static void ide_dev_instance_init(Object *obj)
> > > > > +{
> > > > > + DeviceState *dev = DEVICE(obj);
> > > > > + IDEDevice *d = DO_UPCAST(IDEDevice, qdev, dev);
> > > > > +
> > > > > + device_add_bootindex_property(obj, &d->conf.bootindex,
> > > > > + "bootindex",
> > > > > + d->unit ? "/address@hidden" :
> > > "/address@hidden",
> > > > > + &d->qdev, NULL);
> > > > > +}
> > > > > +
> > Oops, I found a thorny issue that the d->unit parameter had not been
> initialized
> > in ide_dev_instance_init(). d->unit maybe is a random value, which will
> against
> > the original purpose in this situation.
> >
> > What's your opinion, Eduardo? Thanks!
>
> It looks like you can call add_boot_device_path() only on realize, on
> IDE. If IDE is the only case, we could just change IDE to not use
> device_add_bootindex_property(), having its own getter/setter and a call
> to add_boot_device_path() on realize().
>
> Or, to keep the code generic, we could just make get_boot_devices_list()
> query the device for the suffix, instead of requiring the suffix to be
> set before device_add_bootindex_property() is called. With something
> like:
>
> typedef struct BootDeviceInfo {
> int bootindex;
> const char *suffix;
> } BootDeviceInfo;
>
> struct FWBootEntry {
> QTAILQ_ENTRY(FWBootEntry) link;
> DeviceState *dev;
> BootDeviceInfo *bootdev;
> };
>
> void add_boot_device_path(DeviceState *dev, BootDeviceInfo *info);
> void del_boot_device_path(BootDeviceInfo *info);
> void device_add_bootindex_property(Object *obj, const char *property,
> DeviceState *dev, BootDeviceInfo
> *info,
> Error **errp);
>
> This way, the suffix can be set between device_add_bootindex_property()
> and get_boot_devices_list() if necessary:
>
> static void somedevice_instance_init(Object *obj)
> {
> SomeDevice *somedev = SOME_DEVICE(obj);
> device_add_bootindex_property(obj, DEVICE(obj), &somedev->bootinfo,
> &error_abort);
> }
>
> static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
> {
> [...]
> dev->bootinfo.suffix = dev->unit ? "/address@hidden" : "/address@hidden";
> }
>
Thank you so much!
I agree more with #1, because only IDE device has this special case.
This is not worth add a property to DeviceState struct for a special
device IMO.
Best regards,
-Gonglei
- [Qemu-devel] [PATCH v7 22/28] isa-fdc: remove bootindexA/B property from qdev to qom, (continued)
- [Qemu-devel] [PATCH v7 22/28] isa-fdc: remove bootindexA/B property from qdev to qom, arei.gonglei, 2014/09/05
- [Qemu-devel] [PATCH v7 23/28] scsi: add bootindex to qom property, arei.gonglei, 2014/09/05
- [Qemu-devel] [PATCH v7 09/28] e1000: add bootindex to qom property, arei.gonglei, 2014/09/05
- [Qemu-devel] [PATCH v7 26/28] block: remove bootindex property from qdev to qom, arei.gonglei, 2014/09/05
- [Qemu-devel] [PATCH v7 21/28] redirect: remove bootindex property from qdev to qom, arei.gonglei, 2014/09/05
- [Qemu-devel] [PATCH v7 24/28] ide: add bootindex to qom property, arei.gonglei, 2014/09/05
[Qemu-devel] [PATCH v7 25/28] virtio-blk: add bootindex to qom property, arei.gonglei, 2014/09/05
[Qemu-devel] [PATCH v7 28/28] bootindex: delete bootindex when device is removed, arei.gonglei, 2014/09/05
[Qemu-devel] [PATCH v7 02/28] bootindex: add check bootindex function, arei.gonglei, 2014/09/05
[Qemu-devel] [PATCH v7 04/28] fw_cfg: add fw_cfg_machine_reset function, arei.gonglei, 2014/09/05
[Qemu-devel] [PATCH v7 08/28] virtio-net: add bootindex to qom property, arei.gonglei, 2014/09/05
[Qemu-devel] [PATCH v7 06/28] bootindex: support to set a existent device's bootindex to -1, arei.gonglei, 2014/09/05
[Qemu-devel] [PATCH v7 01/28] bootdevice: move bootdevice related code to new file bootdevice.c, arei.gonglei, 2014/09/05