[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 18/40] char: remove class kind field
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 18/40] char: remove class kind field |
Date: |
Thu, 12 Jan 2017 12:32:01 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 |
On 01/11/2017 11:29 AM, Marc-André Lureau wrote:
> The class kind is necessary to lookup the chardev name in
> qmp_chardev_add() after calling qemu_chr_new_from_opts() and to set
> the appropriate ChardevBackend (mainly to free the right
> fields).
>
> qemu_chr_new_from_opts() can be changed to use a non-qmp function
> using the chardev class typename. Introduce qemu_chardev_add() to be
> called from qemu_chr_new_from_opts() and remove the class chardev kind
> field. Set the backend->type in the parse callback (when non-common
> fields are added).
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>
> +static Chardev *qemu_chardev_add(const char *id, const char *typename,
> + ChardevBackend *backend, Error **errp)
> +{
> + Chardev *chr;
> +
> + chr = qemu_chr_find(id);
> + if (chr) {
> + error_setg(errp, "Chardev '%s' already exists", id);
> + return NULL;
> + }
> +
> + chr = qemu_chardev_new(id, typename, backend, errp);
> + if (!chr) {
> + return NULL;
> + }
> +
> + QTAILQ_INSERT_TAIL(&chardevs, chr, next);
> + return chr;
> +}
> +
This part seems okay.
> @@ -4222,22 +4235,22 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
>
> cc = char_get_class(name, errp);
> if (cc == NULL) {
> - goto err;
> + return NULL;
> }
>
> backend = g_new0(ChardevBackend, 1);
> + backend->type = CHARDEV_BACKEND_KIND_NULL;
>
> if (qemu_opt_get_bool(opts, "mux", 0)) {
> bid = g_strdup_printf("%s-base", id);
> }
>
> chr = NULL;
> - backend->type = cc->kind;
I'm not sure I follow this hunk - we used to set backend->type
dynamically and now it is forced to KIND_NULL. Is the point that we
don't need to set backend->type here because the later call to
qemu_chardev_add()...
> @@ -4245,37 +4258,33 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
> backend->u.null.data = ccom; /* Any ChardevCommon member would work
> */
> }
>
> - ret = qmp_chardev_add(bid ? bid : id, backend, errp);
> - if (!ret) {
> - goto qapi_out;
> + chr = qemu_chardev_add(bid ? bid : id,
> + object_class_get_name(OBJECT_CLASS(cc)),
> + backend, errp);
...now passes the Object type which can be used to derive the same same
information? In that case, was the assignment to backend->type =
KIND_NULL dead?
> + if (chr == NULL) {
> + goto out;
> }
>
> if (bid) {
> + Chardev *mux;
> qapi_free_ChardevBackend(backend);
> - qapi_free_ChardevReturn(ret);
> backend = g_new0(ChardevBackend, 1);
> - backend->u.mux.data = g_new0(ChardevMux, 1);
> backend->type = CHARDEV_BACKEND_KIND_MUX;
> + backend->u.mux.data = g_new0(ChardevMux, 1);
Why the churn on the assignment to backend->u.mux.data?
> backend->u.mux.data->chardev = g_strdup(bid);
> - ret = qmp_chardev_add(id, backend, errp);
> - if (!ret) {
> - chr = qemu_chr_find(bid);
> + mux = qemu_chardev_add(id, TYPE_CHARDEV_MUX, backend, errp);
> + if (mux == NULL) {
> qemu_chr_delete(chr);
> chr = NULL;
> - goto qapi_out;
> + goto out;
> }
> + chr = mux;
> }
>
> - chr = qemu_chr_find(id);
> -
> -qapi_out:
> +out:
> qapi_free_ChardevBackend(backend);
> - qapi_free_ChardevReturn(ret);
> g_free(bid);
> return chr;
> -
> -err:
> - return NULL;
> }
>
> @@ -5010,24 +5014,18 @@ Chardev *qemu_chardev_new(const char *id, const char
> *typename,
> ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> Error **errp)
> {
> - const ChardevClass *cc;
> ChardevReturn *ret;
> + const ChardevClass *cc;
Why the churn on this declaration?
> Chardev *chr;
>
> - chr = qemu_chr_find(id);
> - if (chr) {
> - error_setg(errp, "Chardev '%s' already exists", id);
> - return NULL;
> - }
> -
> cc = char_get_class(ChardevBackendKind_lookup[backend->type], errp);
> - if (!cc) {
> + if (cc == NULL) {
Why the churn on this conditional?
> return NULL;
> }
>
> - chr = qemu_chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)),
> + chr = qemu_chardev_add(id, object_class_get_name(OBJECT_CLASS(cc)),
> backend, errp);
> - if (!chr) {
> + if (chr == NULL) {
and again
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH 12/40] char-win: do not override chr_free, (continued)
- [Qemu-devel] [PATCH 15/40] char: remove chr_free, Marc-André Lureau, 2017/01/11
- [Qemu-devel] [PATCH 16/40] char: get rid of CharDriver, Marc-André Lureau, 2017/01/11
- [Qemu-devel] [PATCH 17/40] char: rename remaining CharDriver to Chardev, Marc-André Lureau, 2017/01/11
- [Qemu-devel] [PATCH 18/40] char: remove class kind field, Marc-André Lureau, 2017/01/11
- Re: [Qemu-devel] [PATCH 18/40] char: remove class kind field,
Eric Blake <=
- [Qemu-devel] [PATCH 21/40] char: make null_chr_write() the default method, Marc-André Lureau, 2017/01/11
- [Qemu-devel] [PATCH 22/40] char: move null chardev to its own file, Marc-André Lureau, 2017/01/11
- [Qemu-devel] [PATCH 20/40] char: create chardev-obj-y, Marc-André Lureau, 2017/01/11
- [Qemu-devel] [PATCH 19/40] char: move to chardev/, Marc-André Lureau, 2017/01/11
- [Qemu-devel] [PATCH 24/40] char: move ringbuf/memory to its own file, Marc-André Lureau, 2017/01/11