summaryrefslogtreecommitdiffstats
path: root/chardev
diff options
context:
space:
mode:
authorLukas Straub2021-03-30 20:13:31 +0200
committerMarc-André Lureau2021-04-01 13:27:44 +0200
commitfeb774ca3fc08afc1404f75c06fbaeea5fdbcd19 (patch)
tree3d5cdf7905c4a4c2353d9c8d8295e4f7664514c1 /chardev
parentchardev/char.c: Always pass id to chardev_new (diff)
downloadqemu-feb774ca3fc08afc1404f75c06fbaeea5fdbcd19.tar.gz
qemu-feb774ca3fc08afc1404f75c06fbaeea5fdbcd19.tar.xz
qemu-feb774ca3fc08afc1404f75c06fbaeea5fdbcd19.zip
chardev: Fix yank with the chardev-change case
When changing from chardev-socket (which supports yank) to chardev-socket again, it fails, because the new chardev attempts to register a new yank instance. This in turn fails, as there still is the yank instance from the current chardev. Also, the old chardev shouldn't unregister the yank instance when it is freed. To fix this, now the new chardev only registers a yank instance if the current chardev doesn't support yank and thus hasn't registered one already. Also, when the old chardev is freed, it now only unregisters the yank instance if the new chardev doesn't need it. If the initialization of the new chardev fails, it still has chr->handover_yank_instance set and won't unregister the yank instance when it is freed. s->registered_yank is always true here, as chardev-change only works on user-visible chardevs and those are guraranteed to register a yank instance as they are initialized via chardev_new() qemu_char_open() cc->open() (qmp_chardev_open_socket()). Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Tested-by: Li Zhang <li.zhang@cloud.ionos.com> Message-Id: <9637888d7591d2971975188478bb707299a1dc04.1617127849.git.lukasstraub2@web.de>
Diffstat (limited to 'chardev')
-rw-r--r--chardev/char-socket.c20
-rw-r--r--chardev/char.c35
2 files changed, 45 insertions, 10 deletions
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 1d455ecca4..daa89fe5d1 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1126,7 +1126,13 @@ static void char_socket_finalize(Object *obj)
}
g_free(s->tls_authz);
if (s->registered_yank) {
- yank_unregister_instance(CHARDEV_YANK_INSTANCE(chr->label));
+ /*
+ * In the chardev-change special-case, we shouldn't unregister the yank
+ * instance, as it still may be needed.
+ */
+ if (!chr->handover_yank_instance) {
+ yank_unregister_instance(CHARDEV_YANK_INSTANCE(chr->label));
+ }
}
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
@@ -1424,8 +1430,14 @@ static void qmp_chardev_open_socket(Chardev *chr,
qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
}
- if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label), errp)) {
- return;
+ /*
+ * In the chardev-change special-case, we shouldn't register a new yank
+ * instance, as there already may be one.
+ */
+ if (!chr->handover_yank_instance) {
+ if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label), errp)) {
+ return;
+ }
}
s->registered_yank = true;
@@ -1567,6 +1579,8 @@ static void char_socket_class_init(ObjectClass *oc, void *data)
{
ChardevClass *cc = CHARDEV_CLASS(oc);
+ cc->supports_yank = true;
+
cc->parse = qemu_chr_parse_socket;
cc->open = qmp_chardev_open_socket;
cc->chr_wait_connected = tcp_chr_wait_connected;
diff --git a/chardev/char.c b/chardev/char.c
index 75993f903f..398f09df19 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -39,6 +39,7 @@
#include "qemu/option.h"
#include "qemu/id.h"
#include "qemu/coroutine.h"
+#include "qemu/yank.h"
#include "chardev-internal.h"
@@ -266,6 +267,7 @@ static void char_init(Object *obj)
{
Chardev *chr = CHARDEV(obj);
+ chr->handover_yank_instance = false;
chr->logfd = -1;
qemu_mutex_init(&chr->chr_write_lock);
@@ -959,6 +961,7 @@ void qemu_chr_set_feature(Chardev *chr,
static Chardev *chardev_new(const char *id, const char *typename,
ChardevBackend *backend,
GMainContext *gcontext,
+ bool handover_yank_instance,
Error **errp)
{
Object *obj;
@@ -971,6 +974,7 @@ static Chardev *chardev_new(const char *id, const char *typename,
obj = object_new(typename);
chr = CHARDEV(obj);
+ chr->handover_yank_instance = handover_yank_instance;
chr->label = g_strdup(id);
chr->gcontext = gcontext;
@@ -1004,7 +1008,7 @@ Chardev *qemu_chardev_new(const char *id, const char *typename,
id = genid;
}
- chr = chardev_new(id, typename, backend, gcontext, errp);
+ chr = chardev_new(id, typename, backend, gcontext, false, errp);
if (!chr) {
return NULL;
}
@@ -1032,7 +1036,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
}
chr = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)),
- backend, NULL, errp);
+ backend, NULL, false, errp);
if (!chr) {
return NULL;
}
@@ -1057,9 +1061,10 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
Error **errp)
{
CharBackend *be;
- const ChardevClass *cc;
+ const ChardevClass *cc, *cc_new;
Chardev *chr, *chr_new;
bool closed_sent = false;
+ bool handover_yank_instance;
ChardevReturn *ret;
chr = qemu_chr_find(id);
@@ -1091,13 +1096,20 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
return NULL;
}
- cc = char_get_class(ChardevBackendKind_str(backend->type), errp);
- if (!cc) {
+ cc = CHARDEV_GET_CLASS(chr);
+ cc_new = char_get_class(ChardevBackendKind_str(backend->type), errp);
+ if (!cc_new) {
return NULL;
}
- chr_new = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)),
- backend, chr->gcontext, errp);
+ /*
+ * The new chardev should not register a yank instance if the current
+ * chardev has registered one already.
+ */
+ handover_yank_instance = cc->supports_yank && cc_new->supports_yank;
+
+ chr_new = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc_new)),
+ backend, chr->gcontext, handover_yank_instance, errp);
if (!chr_new) {
return NULL;
}
@@ -1121,6 +1133,15 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
return NULL;
}
+ /* change successfull, clean up */
+ chr_new->handover_yank_instance = false;
+
+ /*
+ * When the old chardev is freed, it should not unregister the yank
+ * instance if the new chardev needs it.
+ */
+ chr->handover_yank_instance = handover_yank_instance;
+
object_unparent(OBJECT(chr));
object_property_add_child(get_chardevs_root(), chr_new->label,
OBJECT(chr_new));