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 <marcandre.lur...@redhat.com> > ---
> > +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