From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45669) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScuNc-00036v-If for qemu-devel@nongnu.org; Fri, 08 Jun 2012 04:17:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ScuNa-0003qH-DN for qemu-devel@nongnu.org; Fri, 08 Jun 2012 04:17:12 -0400 Received: from cantor2.suse.de ([195.135.220.15]:57276 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScuNa-0003qC-3U for qemu-devel@nongnu.org; Fri, 08 Jun 2012 04:17:10 -0400 Message-ID: <4FD1B501.5050804@suse.de> Date: Fri, 08 Jun 2012 10:17:05 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1339097465-22977-1-git-send-email-afaerber@suse.de> <1339097465-22977-3-git-send-email-afaerber@suse.de> <4FD153CC.6040304@codemonkey.ws> <4FD1A5B8.20700@suse.de> <4FD1AD46.9040607@codemonkey.ws> In-Reply-To: <4FD1AD46.9040607@codemonkey.ws> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH qom-next 2/7] qom: Add get_id List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori , Luiz Capitulino , libvir-list@redhat.com Cc: Paolo Bonzini , qemu-devel@nongnu.org Am 08.06.2012 09:44, schrieb Anthony Liguori: > On 06/08/2012 03:11 PM, Andreas F=C3=A4rber wrote: >> Am 08.06.2012 03:22, schrieb Anthony Liguori: >>> On 06/08/2012 03:31 AM, Andreas F=C3=A4rber wrote: >>>> From: Paolo Bonzini >>>> >>>> Some classes may present objects differently in errors, for example = if >>>> they >>>> 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 >>>> [AF: Renamed _object_get_id() to object_instance_get_id(), avoid ?:.= ] >>>> [AF: Use object_property_is_child().] >>>> Signed-off-by: Andreas F=C3=A4rber >>> >>> Nack. >> >> 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). >=20 > 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= . Well, obviously we were hoping to get this series committed immediately after the release, so we needed to get review before that. :) >> 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. >=20 > 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). >=20 > 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' }. >=20 >>> >>> We should use a canonical path IMHO instead of returning a partial na= me. >>> >>> 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. >=20 > 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. >=20 > Doesn't libvirt ignore the contents of an error object? I'm out of my field there, those questions are for Luiz and the libvirt guys to answer. (Context is ongoing DeviceState -> Object transition on qom-next branch, properties being moved to Object and what info to include in Error objects then) If you reach consensus how to handle this, I can refactor accordingly, or Paolo could pick up my tweaked series again and refactor himself. Regards, Andreas >> 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. --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg