[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH qom-next 2/7] qom: Add get_id

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH qom-next 2/7] qom: Add get_id
Date: Fri, 08 Jun 2012 15:44:06 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1

On 06/08/2012 03:11 PM, Andreas Färber wrote:
Am 08.06.2012 03:22, schrieb Anthony Liguori:
On 06/08/2012 03:31 AM, Andreas Färber wrote:
From: Paolo Bonzini<address@hidden>

Some classes may present objects differently in errors, for example if
are not part of the composition tree or if they are not assigned an id by
the user.  Let them do this with a get_id method on Object, and use the
method consistently where a %(device) appears in the error.

Signed-off-by: Paolo Bonzini<address@hidden>
[AF: Renamed _object_get_id() to object_instance_get_id(), avoid ?:.]
[AF: Use object_property_is_child().]
Signed-off-by: Andreas Färber<address@hidden>


Unfortunately that comment comes a bit late (original submission May
23rd, me specifically cc'ing you in my reply that it's new and not
covered by your carte blanche).

Uh, that was a week before the release. Don't send significant things during the final part of a release and expect to get significant review.

The general idea as I understand it is to have a mechanism for having
devices fill in their ID into %(device) in the error messages once the
code emitting those errors is at Object level. Peter's improved error
message practically becomes "Property '.propertyname' ..." because
without it we'll need to fill in "" in lack of an actual value.

Ambiguity is fundamentally bad. If you want to return the path, return the path. If you want to return the type, return the type. Returning the type because we're too lazy to implement the path properly is not acceptable and makes the error messages useless (because they're ambiguous).

Have a separate 'path' and 'typename' attribute in the errors. With some cleverness, you can probably use '%p' and interpret the pointer an as Object * and automagically generate an embedded 'device': { 'path': '/path/to/device', 'typename': 'FancyType' }.

We should use a canonical path IMHO instead of returning a partial name.

Partial names are ambiguous.

Possibly, but a partial name still is an improvement over the current
situation with no name at all. Also my previous request to not use
%(device) for Object-level API was rejected with reference to existing
QMP users, so by the same argument we cannot stuff a QOM path into
%(device) either and would need a new QMP field %(path) or so. Cc'ing Luiz.

Since qdev->id is NULL 90% of the time, I don't think a user can realistically rely on it. I don't think changing the type of the data in the error is going to be a problem.

Doesn't libvirt ignore the contents of an error object?


Anthony Liguori

There is no guarantee that the object actually has a canonical path at
that point, and object_get_canonical_path() would g_assert() in such a case.

Please advise on how to proceed with this series.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]