On Wed, Jun 18, 2014 at 12:12 AM, Paolo Bonzini <pbonz...@redhat.com> wrote:
> Il 17/06/2014 16:09, Peter Crosthwaite ha scritto:
>
>>> > +
>>> > + object_property_add_alias(qdev_get_machine(), "rtc",
>>> > + OBJECT(s), NULL, &error_abort);
>>
>> This will fail if anyone wants to add two such devices to a machine
>> model. It seems a bit board specific to assume that this device is
>> only valid as a singleton. Perhaps s/&error_abort/local_err/ and
>> raising a warning explaining that only the first RTC in the system
>> gets the alias?
>
>
> &error_warn? :) But then, /machine/rtc.tm is the only supported interface,
I have thought about &error_warn in the past, but thought the better
of it. Warnings need user digestable message and I can't think of a
sane implementation where you populate a message for error framework
to raise while maintaining the needed errp == NULL || *errp == NULL
semantic at the same time.
> and it should be the same for all RTC devices so perhaps a NULL error
> pointer is enough.
>
NULL Works for me. But other than source verbosity is there any reason
to not catch the error?:
object_property_add_alias(qdev_get_machine(), "rtc",
OBJECT(s), NULL, &local_err);
if (local_err) {
error_report("WARNING: Multiple RTCs in system, not aliasing %s",
object_get_canonical_path(OBJECT(s)));
/* NOTE: no exit(1) */
}
Regards,
Peter
>
>> The other options is arrayification using your (hot off the press)
>> "[*]" proposal:
>>
>> + object_property_add_alias(qdev_get_machine(), "rtc[*]",
>> + OBJECT(s), NULL, &error_abort);
>
>
> This could make sense, but would leave /machine/rtc[0] in the common case.
> I prefer raising a warning.
>
> Paolo
>
