From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:37577) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuPUW-0000yb-RM for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:24:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RuPUQ-0001Ja-SW for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:24:24 -0500 Received: from mail-pw0-f45.google.com ([209.85.160.45]:56608) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuPUQ-0001JP-Nt for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:24:18 -0500 Received: by pbaa11 with SMTP id a11so6785475pba.4 for ; Mon, 06 Feb 2012 06:24:18 -0800 (PST) Message-ID: <4F2FE28E.7080901@codemonkey.ws> Date: Mon, 06 Feb 2012 08:24:14 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1328342577-25732-1-git-send-email-pbonzini@redhat.com> <1328342577-25732-11-git-send-email-pbonzini@redhat.com> In-Reply-To: <1328342577-25732-11-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 10/27] qom: use object_resolve_path_type for links List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 02/04/2012 02:02 AM, Paolo Bonzini wrote: > This allows to restrict partial matches to objects of the expected > type. It will let people use bare names to reference drives > even though their name might be the same as a device's (e.g. > -drive id=hd0,if=none,... -device ...,drive=hd0,id=hd0). > > As a useful byproduct, this fixes a problem with links of interface > type. When a link property's type is an interface, the code expects > the implementation object (not the parent object) to be stored in the > variable. The parent object does not contain the right vtable. > > Signed-off-by: Paolo Bonzini Reviewed-by: Anthony Liguori I'm really happy with how this turned out. Regards, Anthony Liguori > --- > qerror.c | 4 ++++ > qerror.h | 3 +++ > qom/object.c | 32 ++++++++++++++++---------------- > 3 files changed, 23 insertions(+), 16 deletions(-) > > diff --git a/qerror.c b/qerror.c > index 3d179c8..8e6efaf 100644 > --- a/qerror.c > +++ b/qerror.c > @@ -48,6 +48,10 @@ static const QErrorStringTable qerror_table[] = { > .desc = "Could not add client", > }, > { > + .error_fmt = QERR_AMBIGUOUS_PATH, > + .desc = "Path '%(path)' does not uniquely identify a %(object)" > + }, > + { > .error_fmt = QERR_BAD_BUS_FOR_DEVICE, > .desc = "Device '%(device)' can't go on a %(bad_bus_type) bus", > }, > diff --git a/qerror.h b/qerror.h > index 8c36ddb..e8718bf 100644 > --- a/qerror.h > +++ b/qerror.h > @@ -54,6 +54,9 @@ QError *qobject_to_qerror(const QObject *obj); > #define QERR_ADD_CLIENT_FAILED \ > "{ 'class': 'AddClientFailed', 'data': {} }" > > +#define QERR_AMBIGUOUS_PATH \ > + "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }" > + > #define QERR_BAD_BUS_FOR_DEVICE \ > "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }" > > diff --git a/qom/object.c b/qom/object.c > index ea4a1f5..75be582 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -854,6 +854,7 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, > bool ambiguous = false; > const char *type; > char *path; > + gchar *target_type; > > type = object_property_get_type(obj, name, NULL); > > @@ -861,31 +862,30 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, > > if (*child) { > object_unref(*child); > + *child = NULL; > } > > if (strcmp(path, "") != 0) { > Object *target; > > - target = object_resolve_path(path,&ambiguous); > - if (target) { > - gchar *target_type; > - > - target_type = g_strdup(&type[5]); > - *strchr(target_type, '>') = 0; > + target_type = g_strdup(&type[5]); > + *strchr(target_type, '>') = 0; > + target = object_resolve_path_type(path, target_type,&ambiguous); > > - if (object_dynamic_cast(target, target_type)) { > - object_ref(target); > - *child = target; > + if (ambiguous) { > + error_set(errp, QERR_AMBIGUOUS_PATH, path); > + } else if (target) { > + object_ref(target); > + *child = target; > + } else { > + target = object_resolve_path(path,&ambiguous); > + if (target || ambiguous) { > + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, target_type); > } else { > - error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, type); > + error_set(errp, QERR_DEVICE_NOT_FOUND, path); > } > - > - g_free(target_type); > - } else { > - error_set(errp, QERR_DEVICE_NOT_FOUND, path); > } > - } else { > - *child = NULL; > + g_free(target_type); > } > > g_free(path);