On Sun, Nov 10, 2019 at 10:03:09PM +0300, Denis Plotnikov wrote:
> Some device's property can be changed if the device has been already
> realized. For example, it could be "drive" property of a scsi disk device.
>
> So far, set_pointer could operate only on a relized device. The patch
> extends its interface for operation on an unrealized device.
>
> Signed-off-by: Denis Plotnikov <dplotni...@virtuozzo.com>
> ---
> hw/core/qdev-properties-system.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/hw/core/qdev-properties-system.c
> b/hw/core/qdev-properties-system.c
> index ba412dd2ca..c534590dcd 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -38,9 +38,14 @@ static void get_pointer(Object *obj, Visitor *v, Property
> *prop,
> }
>
> static void set_pointer(Object *obj, Visitor *v, Property *prop,
> - void (*parse)(DeviceState *dev, const char *str,
> - void **ptr, const char *propname,
> - Error **errp),
> + void (*parse_realized)(DeviceState *dev,
> + const char *str, void **ptr,
> + const char *propname,
> + Error **errp),
> + void (*parse_unrealized)(DeviceState *dev,
> + const char *str, void **ptr,
> + const char *propname,
> + Error **errp),
> const char *name, Error **errp)
Wouldn't it be simpler to just add a PropertyInfo::allow_set_after_realize
bool field, and call the same setter function? Then you can
simply change do_parse_drive() to check if realized is true.
> {
> DeviceState *dev = DEVICE(obj);
> @@ -48,11 +53,6 @@ static void set_pointer(Object *obj, Visitor *v, Property
> *prop,
> void **ptr = qdev_get_prop_ptr(dev, prop);
> char *str;
>
> - if (dev->realized) {
> - qdev_prop_set_after_realize(dev, name, errp);
> - return;
> - }
> -
> visit_type_str(v, name, &str, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> @@ -63,7 +63,17 @@ static void set_pointer(Object *obj, Visitor *v, Property
> *prop,
> *ptr = NULL;
> return;
> }
> - parse(dev, str, ptr, prop->name, errp);
> +
> + if (dev->realized) {
> + if (parse_realized) {
> + parse_realized(dev, str, ptr, prop->name, errp);
> + } else {
> + qdev_prop_set_after_realize(dev, name, errp);
> + }
> + } else {
> + parse_unrealized(dev, str, ptr, prop->name, errp);
> + }
> +
> g_free(str);
> }
>
> @@ -178,13 +188,13 @@ static void get_drive(Object *obj, Visitor *v, const
> char *name, void *opaque,
> static void set_drive(Object *obj, Visitor *v, const char *name, void
> *opaque,
> Error **errp)
> {
> - set_pointer(obj, v, opaque, parse_drive, name, errp);
> + set_pointer(obj, v, opaque, NULL, parse_drive, name, errp);
> }
>
> static void set_drive_iothread(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - set_pointer(obj, v, opaque, parse_drive_iothread, name, errp);
> + set_pointer(obj, v, opaque, NULL, parse_drive_iothread, name, errp);
> }
>
> const PropertyInfo qdev_prop_drive = {
> --
> 2.17.0
>
--
Eduardo
