Hi, we got a user report about bootindex for an 'usb-storage' device not working anymore [0] and I reproduced it and bisected it to this patch.
Am 31.01.24 um 14:06 schrieb Kevin Wolf: > @@ -399,11 +397,10 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, > BlockBackend *blk, > object_property_add_child(OBJECT(bus), name, OBJECT(dev)); > g_free(name); > > + s = SCSI_DEVICE(dev); > + s->conf = *conf; > + > qdev_prop_set_uint32(dev, "scsi-id", unit); > - if (bootindex >= 0) { > - object_property_set_int(OBJECT(dev), "bootindex", bootindex, > - &error_abort); > - } The fact that this is not called anymore means that the 'set' method for the property is also not called. Here, that method is device_set_bootindex() (as configured by scsi_dev_instance_init() -> device_add_bootindex_property()). Therefore, the device is never registered via add_boot_device_path() meaning that the bootindex property does not have the desired effect anymore. Is it necessary to keep the object_property_set_{bool,int} and qdev_prop_set_enum calls around for these potential side effects? Would it even be necessary to introduce new similar calls for the newly supported properties? Or is there an easy alternative to s->conf = *conf; that does trigger the side effects? > if (object_property_find(OBJECT(dev), "removable")) { > qdev_prop_set_bit(dev, "removable", removable); > } > @@ -414,19 +411,12 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, > BlockBackend *blk, > object_unparent(OBJECT(dev)); > return NULL; > } > - if (!object_property_set_bool(OBJECT(dev), "share-rw", share_rw, errp)) { > - object_unparent(OBJECT(dev)); > - return NULL; > - } > - > - qdev_prop_set_enum(dev, "rerror", rerror); > - qdev_prop_set_enum(dev, "werror", werror); > > if (!qdev_realize_and_unref(dev, &bus->qbus, errp)) { > object_unparent(OBJECT(dev)); > return NULL; > } > - return SCSI_DEVICE(dev); > + return s; > } > > void scsi_bus_legacy_handle_cmdline(SCSIBus *bus) [0]: https://forum.proxmox.com/threads/149772/post-679433 Best Regards, Fiona