On Tue, Sep 09, 2014 at 07:51:49AM +0000, Gonglei (Arei) wrote: > Hi, > > > Subject: RE: [Qemu-devel] [PATCH v7 24/28] ide: add bootindex to qom > > property > > > > > Subject: Re: [Qemu-devel] [PATCH v7 24/28] ide: add bootindex to qom > > property > > > > > > On Fri, Sep 05, 2014 at 04:37:32PM +0800, arei.gong...@huawei.com wrote: > > > > From: Gonglei <arei.gong...@huawei.com> > > > > > > > > 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 <arei.gong...@huawei.com> > > > > --- > > > > 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 ? "/disk@1" : > > "/disk@0", > > > > + &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 ? "/disk@1" : "/disk@0"; } -- Eduardo