Re: [Qemu-devel] [patch 3/3] mc146818rtc: add "rtc" link to "/machines"

[Paolo Bonzini]

Il 30/05/2014 22:11, address@hidden ha scritto:
> Add a link to rtc under /machines providing a stable
> location for management apps to query "date" field.
>
> {"execute":"qom-get","arguments":{"path":"/machine/rtc","property":"date"} }
>
> Suggested by Paolo Bonzini.
>
> Signed-off-by: Marcelo Tosatti <address@hidden>
>
> Index: qemu/hw/timer/mc146818rtc.c
> ===================================================================
> --- qemu.orig/hw/timer/mc146818rtc.c
> +++ qemu/hw/timer/mc146818rtc.c
> @@ -63,6 +63,7 @@
>  typedef struct RTCState {
>      ISADevice parent_obj;
>
> +    Object *rtc_object;
>      MemoryRegion io;
>      uint8_t cmos_data[128];
>      uint8_t cmos_index;
> @@ -856,6 +857,8 @@ static void rtc_realizefn(DeviceState *d
>      RTCState *s = MC146818_RTC(dev);
>      int base = 0x70;
>
> +    s->rtc_object = OBJECT(s);

Adding a new pointer variable is ugly; let's instead add a new function 
to create the property, that works the intended way.  It can be very 
similar to object_property_add_child, like this:

   object_property_add_alias(qdev_get_machine(), "rtc", OBJECT(s),
                             &error_abort);

Of course you don't need to modify child->parent and you also need to 
change the new function so that the type becomes "link<%s>" (child<%s> 
really means that you have set child->parent); on the other hand you can 
reuse the getter (object_get_child_property).

> +#ifdef TARGET_I386

The #ifdef is not necessary.

> +    object_ref(s->rtc_object);

This means that the RTC cannot be removed anymore unless someone drops 
the link property; but no one except the machine knows that the property 
is there, so there is a catch-22.

So I think it makes sense to not add a reference.  If you do not add a 
reference, you do not need object_property_add_alias to add a finalizer 
either (like object_release_child_property).  However, you need to add 
an instance_finalize callback in mc146818rtc_info, which removes the rtc 
property from /machine.

Paolo

> +    object_property_add_link(qdev_get_machine(), "rtc", TYPE_MC146818_RTC,
> +                            (Object **)&s->rtc_object, NULL, 0, &error_abort);
> +#endif

>  }
>
>  ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)