Re: [Qemu-devel] [PATCH v2 10/27] qom: use object_resolve_path_type for links

[Anthony Liguori <anthony@codemonkey.ws>]

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<pbonzini@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

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);