Re: [PATCH v5 04/37] chardev: generate an internal id when none given

[xiaoqiang zhao]

在 2019/12/20 下午9:45, Marc-André Lureau 写道:
> Internally, qemu may create chardev without ID. Those will not be
> looked up with qemu_chr_find(), which prevents using qdev_prop_set_chr().
>
> Use id_generate(), to generate an internal name (prefixed with #), so
> no conflict exist with user-named chardev.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>   chardev/char.c    | 32 ++++++++++++++++++++++++--------
>   include/qemu/id.h |  1 +
>   util/id.c         |  1 +
>   3 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 7b6b2cb123..e7e7492b0e 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -37,6 +37,7 @@
>   #include "qemu/help_option.h"
>   #include "qemu/module.h"
>   #include "qemu/option.h"
> +#include "qemu/id.h"
>   
>   #include "chardev/char-mux.h"
>   
> @@ -944,10 +945,10 @@ void qemu_chr_set_feature(Chardev *chr,
>       return set_bit(feature, chr->features);
>   }
>   
> -Chardev *qemu_chardev_new(const char *id, const char *typename,
> -                          ChardevBackend *backend,
> -                          GMainContext *gcontext,
> -                          Error **errp)
> +static Chardev *chardev_new(const char *id, const char *typename,
> +                            ChardevBackend *backend,
> +                            GMainContext *gcontext,
> +                            Error **errp)
>   {
>       Object *obj;
>       Chardev *chr = NULL;
> @@ -991,6 +992,21 @@ end:
>       return chr;
>   }
>   
> +Chardev *qemu_chardev_new(const char *id, const char *typename,
> +                          ChardevBackend *backend,
> +                          GMainContext *gcontext,
> +                          Error **errp)
> +{
> +    g_autofree char *genid = NULL;
> +
> +    if (!id) {
> +        genid = id_generate(ID_CHR);
> +        id = genid;
> +    }
> +
> +    return chardev_new(id, typename, backend, gcontext, errp);
> +}
> +
>   ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>                                  Error **errp)
>   {
> @@ -1003,8 +1019,8 @@ ChardevReturn *qmp_chardev_add(const char *id, 
> ChardevBackend *backend,
>           return NULL;
>       }
>   
> -    chr = qemu_chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)),
> -                           backend, NULL, errp);
> +    chr = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)),
> +                      backend, NULL, errp);
>       if (!chr) {
>           return NULL;
>       }
> @@ -1061,8 +1077,8 @@ ChardevReturn *qmp_chardev_change(const char *id, 
> ChardevBackend *backend,
>           return NULL;
>       }
>   
> -    chr_new = qemu_chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),
> -                               backend, chr->gcontext, errp);
> +    chr_new = chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),
> +                          backend, chr->gcontext, errp);
>       if (!chr_new) {
>           return NULL;
>       }
> diff --git a/include/qemu/id.h b/include/qemu/id.h
> index 40c70103e4..b55c406e69 100644
> --- a/include/qemu/id.h
> +++ b/include/qemu/id.h
> @@ -4,6 +4,7 @@
>   typedef enum IdSubSystems {
>       ID_QDEV,
>       ID_BLOCK,
> +    ID_CHR,
>       ID_MAX      /* last element, used as array size */
>   } IdSubSystems;
>   
> diff --git a/util/id.c b/util/id.c
> index af1c5f1b81..5addb4460e 100644
> --- a/util/id.c
> +++ b/util/id.c
> @@ -34,6 +34,7 @@ bool id_wellformed(const char *id)
>   static const char *const id_subsys_str[ID_MAX] = {
>       [ID_QDEV]  = "qdev",
>       [ID_BLOCK] = "block",
> +    [ID_CHR] = "chr",
>   };
>   
>   /*

Looks good to me.

Reviewed-by: xiaoqiang zhao <address@hidden>