| Message ID | 1495198042-124203-4-git-send-email-anton.nefedov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi On Fri, May 19, 2017 at 4:51 PM Anton Nefedov <anton.nefedov@virtuozzo.com> wrote: > This patch adds a possibility to change a char device without a frontend > removal. > > 1. Ideally, it would have to happen transparently to a frontend, i.e. > frontend would continue its regular operation. > However, backends are not stateless and are set up by the frontends > via qemu_chr_fe_<> functions, and it's not (generally) possible to replay > that setup entirely in a backend code, as different chardevs respond > to the setup calls differently, so do frontends work differently basing > on those setup responses. > Moreover, some frontend can generally get and save the backend pointer > (qemu_chr_fe_get_driver()), and it will become invalid after backend > change. > > So, a frontend which would like to support chardev hotswap has to register > a "backend change" handler, and redo its backend setup there. > > 2. Write path can be used by multiple threads and thus protected with > chr_write_lock. > So hotswap also has to be protected so write functions won't access > a backend being replaced. > > Why not use the chr_write_lock then? (rename it chr_lock?) > 3. Hotswap function can be called from e.g. a read handler of a monitor > socket. This can cause troubles so it's safer to defer execution to > a bottom-half (however, it means we cannot return some of the errors > synchronously - but most of them we can) > What kind of troubles? > > Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > chardev/char.c | 147 > ++++++++++++++++++++++++++++++++++++++++++++++---- > include/sysemu/char.h | 10 ++++ > qapi-schema.json | 40 ++++++++++++++ > 3 files changed, 187 insertions(+), 10 deletions(-) > > diff --git a/chardev/char.c b/chardev/char.c > index ae60950..bac5e1c 100644 > --- a/chardev/char.c > +++ b/chardev/char.c > @@ -132,12 +132,16 @@ static bool qemu_chr_replay(Chardev *chr) > > int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len) > { > - Chardev *s = be->chr; > + Chardev *s; > ChardevClass *cc; > int ret; > > + qemu_mutex_lock(&be->chr_lock); > + s = be->chr; > + > if (!s) { > - return 0; > + ret = 0; > + goto end; > } > > if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_PLAY) { > @@ -145,7 +149,7 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t > *buf, int len) > replay_char_write_event_load(&ret, &offset); > assert(offset <= len); > qemu_chr_fe_write_buffer(s, buf, offset, &offset); > - return ret; > + goto end; > } > > cc = CHARDEV_GET_CLASS(s); > @@ -161,7 +165,9 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t > *buf, int len) > if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) { > replay_char_write_event_save(ret, ret < 0 ? 0 : ret); > } > - > + > +end: > + qemu_mutex_unlock(&be->chr_lock); > return ret; > } > > @@ -191,13 +197,16 @@ int qemu_chr_write_all(Chardev *s, const uint8_t > *buf, int len) > > int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len) > { > - Chardev *s = be->chr; > + Chardev *s; > + int ret; > > - if (!s) { > - return 0; > - } > + qemu_mutex_lock(&be->chr_lock); > > - return qemu_chr_write_all(s, buf, len); > + s = be->chr; > + ret = s ? qemu_chr_write_all(s, buf, len) : 0; > + > + qemu_mutex_unlock(&be->chr_lock); > + return ret; > } > > int qemu_chr_fe_read_all(CharBackend *be, uint8_t *buf, int len) > @@ -478,7 +487,7 @@ Chardev *qemu_chr_fe_get_driver(CharBackend *be) > return be->chr; > } > > -bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) > +static bool fe_connect(CharBackend *b, Chardev *s, Error **errp) > { > I would rather keep a qemu_chr prefix for consistency > int tag = 0; > > @@ -507,6 +516,17 @@ unavailable: > return false; > } > > +bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) > +{ > + if (!fe_connect(b, s, errp)) { > + return false; > + } > + > + qemu_mutex_init(&b->chr_lock); > + b->hotswap_bh = NULL; > + return true; > +} > + > static bool qemu_chr_is_busy(Chardev *s) > { > if (CHARDEV_IS_MUX(s)) { > @@ -531,6 +551,10 @@ void qemu_chr_fe_deinit(CharBackend *b) > d->backends[b->tag] = NULL; > } > b->chr = NULL; > + qemu_mutex_destroy(&b->chr_lock); > + if (b->hotswap_bh) { > + qemu_bh_delete(b->hotswap_bh); > Could there be a chr_new leak here? > + } > } > } > > @@ -1308,6 +1332,109 @@ ChardevReturn *qmp_chardev_add(const char *id, > ChardevBackend *backend, > return ret; > } > > +static void chardev_change_bh(void *opaque) > +{ > + Chardev *chr_new = opaque; > + const char *id = chr_new->label; > + Chardev *chr = qemu_chr_find(id); > Could chr be gone in the meantime? potentially > + CharBackend *be = chr->be; > + bool closed_sent = false; > + > + if (!be) { > + /* disconnected since we checked: ok, less work for us */ > + goto end; > + } > + > + if (chr->be_open && !chr_new->be_open) { > + qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > + closed_sent = true; > + } > + > + qemu_mutex_lock(&be->chr_lock); > + chr->be = NULL; > + fe_connect(be, chr_new, &error_abort); > + > + if (be->chr_be_change(be->opaque) < 0) { > + error_report("Chardev '%s' change failed", id); > + fe_connect(be, chr, &error_abort); > + qemu_mutex_unlock(&be->chr_lock); > + if (closed_sent) { > + qemu_chr_be_event(chr, CHR_EVENT_OPENED); > + } > + object_unref(OBJECT(chr_new)); > + return; > + } > + qemu_mutex_unlock(&be->chr_lock); > + > +end: > + object_unparent(OBJECT(chr)); > + object_property_add_child(get_chardevs_root(), id, OBJECT(chr_new), > + &error_abort); > + object_unref(OBJECT(chr_new)); > +} > + > +ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, > + Error **errp) > +{ > + const ChardevClass *cc; > + Chardev *chr, *chr_new; > + ChardevReturn *ret; > + > + chr = qemu_chr_find(id); > + if (!chr) { > + error_setg(errp, "Chardev '%s' does not exist", id); > + return NULL; > + } > + > + if (CHARDEV_IS_MUX(chr)) { > + error_setg(errp, "Mux device hotswap not supported yet"); > + return NULL; > + } > + > + if (qemu_chr_replay(chr)) { > + error_setg(errp, > + "Chardev '%s' cannot be changed in record/replay mode", id); > + return NULL; > + } > + > + if (!chr->be) { > + /* easy case */ > + object_unparent(OBJECT(chr)); > + return qmp_chardev_add(id, backend, errp); > + } > + > + if (!chr->be->chr_be_change) { > + error_setg(errp, "Chardev user does not support chardev hotswap"); > + return NULL; > + } > + > + cc = char_get_class(ChardevBackendKind_lookup[backend->type], errp); > + if (!cc) { > + return NULL; > + } > + > + chr_new = qemu_chardev_new(NULL, > object_class_get_name(OBJECT_CLASS(cc)), > + backend, errp); > + chr_new->label = g_strdup(id); > move it below the check for !chr_new > + if (!chr_new) { > + return NULL; > + } > + > + if (chr->be->hotswap_bh) { > + qemu_bh_delete(chr->be->hotswap_bh); > + } > Is it necessary to delete and recreate the bh? Could there be a chr_new leak if the bh didn't run? It's probably best to deny backend change while one is going on. > + chr->be->hotswap_bh = qemu_bh_new(chardev_change_bh, chr_new); > + qemu_bh_schedule(chr->be->hotswap_bh); > + > + ret = g_new0(ChardevReturn, 1); > + if (CHARDEV_IS_PTY(chr_new)) { > + ret->pty = g_strdup(chr_new->filename + 4); > + ret->has_pty = true; > + } > + > + return ret; > ah, potentially a case where qmp-async would be better.. +} > + > void qmp_chardev_remove(const char *id, Error **errp) > { > Chardev *chr; > diff --git a/include/sysemu/char.h b/include/sysemu/char.h > index 9f8df07..68c7876 100644 > --- a/include/sysemu/char.h > +++ b/include/sysemu/char.h > @@ -92,6 +92,8 @@ typedef struct CharBackend { > void *opaque; > int tag; > int fe_open; > + QemuMutex chr_lock; > + QEMUBH *hotswap_bh; > } CharBackend; > > struct Chardev { > @@ -141,6 +143,14 @@ void qemu_chr_parse_common(QemuOpts *opts, > ChardevCommon *backend); > */ > Chardev *qemu_chr_new(const char *label, const char *filename); > > +/** > + * @qemu_chr_change: > + * > + * Change an existing character backend > + * > + * @opts the new backend options > + */ > +void qemu_chr_change(QemuOpts *opts, Error **errp); > > This patch needs some test-char.c tests. > /** > * @qemu_chr_fe_disconnect: > diff --git a/qapi-schema.json b/qapi-schema.json > index 80603cf..bf72166 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -5075,6 +5075,46 @@ > 'returns': 'ChardevReturn' } > > ## > +# @chardev-change: > +# > +# Change a character device backend > +# > +# @id: the chardev's ID, must exist > +# @backend: new backend type and parameters > +# > +# Returns: ChardevReturn. > +# > +# Since: 2.10 > +# > +# Example: > +# > +# -> { "execute" : "chardev-change", > +# "arguments" : { "id" : "baz", > +# "backend" : { "type" : "pty", "data" : {} } } } > +# <- { "return": { "pty" : "/dev/pty/42" } } > +# > +# -> {"execute" : "chardev-change", > +# "arguments" : { > +# "id" : "charchannel2", > +# "backend" : { > +# "type" : "socket", > +# "data" : { > +# "addr" : { > +# "type" : "unix" , > +# "data" : { > +# "path" : "/tmp/charchannel2.socket" > +# } > +# }, > +# "server" : true, > +# "wait" : false }}}} > +# <- {"return": {}} > +# > +## > +{ 'command': 'chardev-change', 'data': {'id' : 'str', > + 'backend' : 'ChardevBackend' }, > + 'returns': 'ChardevReturn' } > + > +## > # @chardev-remove: > # > # Remove a character device backend > -- > 2.7.4 > > > -- Marc-André Lureau
> Hi > Hi Marc-André, thanks for reviewing, pls see answers below > On Fri, May 19, 2017 at 4:51 PM Anton Nefedov <address@hidden> > wrote: > >> This patch adds a possibility to change a char device without a frontend >> removal. >> >> 1. Ideally, it would have to happen transparently to a frontend, i.e. >> frontend would continue its regular operation. >> However, backends are not stateless and are set up by the frontends >> via qemu_chr_fe_<> functions, and it's not (generally) possible to replay >> that setup entirely in a backend code, as different chardevs respond >> to the setup calls differently, so do frontends work differently basing >> on those setup responses. >> Moreover, some frontend can generally get and save the backend pointer >> (qemu_chr_fe_get_driver()), and it will become invalid after backend >> change. >> >> So, a frontend which would like to support chardev hotswap has to register >> a "backend change" handler, and redo its backend setup there. >> >> 2. Write path can be used by multiple threads and thus protected with >> chr_write_lock. >> So hotswap also has to be protected so write functions won't access >> a backend being replaced. >> >> > Why not use the chr_write_lock then? (rename it chr_lock?) > chr_write_lock is a Chardev property, it will be gone together with a swapped device. We could remove chr_write_lock and leave only the new be->chr_lock, since it covers everything; I guess the only problem is mux, where multiple CharBackends share the same Chardev. Would smth like this be better?: static void char_lock(CharBackend *be) { qemu_mutex_lock(be->chr && CHARDEV_IS_MUX(be->chr) ? &be->chr->chr_lock : &be->chr_lock); } and use this instead of lock(be->chr_lock) and remove chr_write_lock. (This would rely on a fact that mux is never hotswapped so be->chr_lock is not needed). > >> 3. Hotswap function can be called from e.g. a read handler of a monitor >> socket. This can cause troubles so it's safer to defer execution to >> a bottom-half (however, it means we cannot return some of the errors >> synchronously - but most of them we can) >> > > What kind of troubles? > if someone tried to hotswap monitor, it would look like: (gdb) bt #0 qmp_chardev_change (id=id@entry=0x7fc24bee7a60 "charmonitor", backend=backend@entry=0x7fc24cc28290, errp=errp@entry=0x7ffde4d8d4f0) at /mnt/code/us-qemu/chardev/char.c:1438 #1 0x00007fc249fd914b in hmp_chardev_change (mon=<optimized out>, qdict=<optimized out>) at /mnt/code/us-qemu/hmp.c:2252 #2 0x00007fc249edeece in handle_hmp_command (mon=mon@entry=0x7fc24bef87c0, cmdline=0x7fc24bf2345f "charmonitor socket,path=/vz/vmprivate/centosos/mon.socket,server,nowait") at /mnt/code/us-qemu/monitor.c:3100 #3 0x00007fc249ee02fa in monitor_command_cb (opaque=0x7fc24bef87c0, cmdline=<optimized out>, readline_opaque=<optimized out>) at /mnt/code/us-qemu/monitor.c:3897 #4 0x00007fc24a242428 in readline_handle_byte (rs=0x7fc24bf23450, ch=<optimized out>) at /mnt/code/us-qemu/util/readline.c:393 #5 0x00007fc249edf0e2 in monitor_read (opaque=<optimized out>, buf=<optimized out>, size=<optimized out>) at /mnt/code/us-qemu/monitor.c:3880 #6 0x00007fc24a1deb2b in tcp_chr_read (chan=<optimized out>, cond=<optimized out>, opaque=<optimized out>) at /mnt/code/us-qemu/chardev/char-socket.c:441 #7 0x00007fc240997d7a in g_main_dispatch (context=0x7fc24beda950) at gmain.c:3152 #8 g_main_context_dispatch (context=context@entry=0x7fc24beda950) at gmain.c:3767 #9 0x00007fc24a22fe7c in glib_pollfds_poll () at /mnt/code/us-qemu/util/main-loop.c:213 #10 os_host_main_loop_wait (timeout=<optimized out>) at /mnt/code/us-qemu/util/main-loop.c:261 #11 main_loop_wait (nonblocking=nonblocking@entry=0) at /mnt/code/us-qemu/util/main-loop.c:517 #12 0x00007fc249e9a19a in main_loop () at /mnt/code/us-qemu/vl.c:1900 #13 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at /mnt/code/us-qemu/vl.c:4732 i.e. hotswap the device that is in tcp_chr_read (frame #6), and can potentially be accessed (with old pointer on a stack) after it is destroyed by qmp_chardev_change() however, there is no support for monitor hotswap in the patchset. Maybe it's not worth to mess with bottom-halves at all? It causes problems, like those mentioned in your further comments >> >> Signed-off-by: Anton Nefedov <address@hidden> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden> >> --- >> chardev/char.c | 147 >> ++++++++++++++++++++++++++++++++++++++++++++++---- >> include/sysemu/char.h | 10 ++++ >> qapi-schema.json | 40 ++++++++++++++ >> 3 files changed, 187 insertions(+), 10 deletions(-) >> >> -bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) >> +static bool fe_connect(CharBackend *b, Chardev *s, Error **errp) >> { >> > > I would rather keep a qemu_chr prefix for consistency > Done. > >> int tag = 0; >> >> @@ -507,6 +516,17 @@ unavailable: >> return false; >> } >> >> +bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) >> +{ >> + if (!fe_connect(b, s, errp)) { >> + return false; >> + } >> + >> + qemu_mutex_init(&b->chr_lock); >> + b->hotswap_bh = NULL; >> + return true; >> +} >> + >> static bool qemu_chr_is_busy(Chardev *s) >> { >> if (CHARDEV_IS_MUX(s)) { >> @@ -531,6 +551,10 @@ void qemu_chr_fe_deinit(CharBackend *b) >> d->backends[b->tag] = NULL; >> } >> b->chr = NULL; >> + qemu_mutex_destroy(&b->chr_lock); >> + if (b->hotswap_bh) { >> + qemu_bh_delete(b->hotswap_bh); >> > > Could there be a chr_new leak here? > > Yes you're right it can leak. Maybe we don't need this asynchrony, see above, otherwise I will think how to fix >> + } >> } >> } >> >> @@ -1308,6 +1332,109 @@ ChardevReturn *qmp_chardev_add(const char *id, >> ChardevBackend *backend, >> return ret; >> } >> >> +static void chardev_change_bh(void *opaque) >> +{ >> + Chardev *chr_new = opaque; >> + const char *id = chr_new->label; >> + Chardev *chr = qemu_chr_find(id); >> > > Could chr be gone in the meantime? potentially > > Potentially yes. Will fix (clean chr_new and bail out). >> + >> + chr_new = qemu_chardev_new(NULL, >> object_class_get_name(OBJECT_CLASS(cc)), >> + backend, errp); >> + chr_new->label = g_strdup(id); >> > > move it below the check for !chr_new > > my bad, thanks >> + if (!chr_new) { >> + return NULL; >> + } >> + >> + if (chr->be->hotswap_bh) { >> + qemu_bh_delete(chr->be->hotswap_bh); >> + } >> > > Is it necessary to delete and recreate the bh? Could there be a chr_new > leak if the bh didn't run? It's probably best to deny backend change while > one is going on. > > Hmm probably not, but new() is the only function that sets the argument. Will think how to fix this if we decide to keep the bh. >> + chr->be->hotswap_bh = qemu_bh_new(chardev_change_bh, chr_new); >> + qemu_bh_schedule(chr->be->hotswap_bh); >> + >> + ret = g_new0(ChardevReturn, 1); >> + if (CHARDEV_IS_PTY(chr_new)) { >> + ret->pty = g_strdup(chr_new->filename + 4); >> + ret->has_pty = true; >> + } >> + >> + return ret; >> > > ah, potentially a case where qmp-async would be better.. > > +} >> + >> void qmp_chardev_remove(const char *id, Error **errp) >> { >> Chardev *chr; >> diff --git a/include/sysemu/char.h b/include/sysemu/char.h >> index 9f8df07..68c7876 100644 >> --- a/include/sysemu/char.h >> +++ b/include/sysemu/char.h >> @@ -92,6 +92,8 @@ typedef struct CharBackend { >> void *opaque; >> int tag; >> int fe_open; >> + QemuMutex chr_lock; >> + QEMUBH *hotswap_bh; >> } CharBackend; >> >> struct Chardev { >> @@ -141,6 +143,14 @@ void qemu_chr_parse_common(QemuOpts *opts, >> ChardevCommon *backend); >> */ >> Chardev *qemu_chr_new(const char *label, const char *filename); >> >> +/** >> + * @qemu_chr_change: >> + * >> + * Change an existing character backend >> + * >> + * @opts the new backend options >> + */ >> +void qemu_chr_change(QemuOpts *opts, Error **errp); >> >> > This patch needs some test-char.c tests. > > Will do /Anton
diff --git a/chardev/char.c b/chardev/char.c index ae60950..bac5e1c 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -132,12 +132,16 @@ static bool qemu_chr_replay(Chardev *chr) int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len) { - Chardev *s = be->chr; + Chardev *s; ChardevClass *cc; int ret; + qemu_mutex_lock(&be->chr_lock); + s = be->chr; + if (!s) { - return 0; + ret = 0; + goto end; } if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_PLAY) { @@ -145,7 +149,7 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len) replay_char_write_event_load(&ret, &offset); assert(offset <= len); qemu_chr_fe_write_buffer(s, buf, offset, &offset); - return ret; + goto end; } cc = CHARDEV_GET_CLASS(s); @@ -161,7 +165,9 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len) if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) { replay_char_write_event_save(ret, ret < 0 ? 0 : ret); } - + +end: + qemu_mutex_unlock(&be->chr_lock); return ret; } @@ -191,13 +197,16 @@ int qemu_chr_write_all(Chardev *s, const uint8_t *buf, int len) int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len) { - Chardev *s = be->chr; + Chardev *s; + int ret; - if (!s) { - return 0; - } + qemu_mutex_lock(&be->chr_lock); - return qemu_chr_write_all(s, buf, len); + s = be->chr; + ret = s ? qemu_chr_write_all(s, buf, len) : 0; + + qemu_mutex_unlock(&be->chr_lock); + return ret; } int qemu_chr_fe_read_all(CharBackend *be, uint8_t *buf, int len) @@ -478,7 +487,7 @@ Chardev *qemu_chr_fe_get_driver(CharBackend *be) return be->chr; } -bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) +static bool fe_connect(CharBackend *b, Chardev *s, Error **errp) { int tag = 0; @@ -507,6 +516,17 @@ unavailable: return false; } +bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) +{ + if (!fe_connect(b, s, errp)) { + return false; + } + + qemu_mutex_init(&b->chr_lock); + b->hotswap_bh = NULL; + return true; +} + static bool qemu_chr_is_busy(Chardev *s) { if (CHARDEV_IS_MUX(s)) { @@ -531,6 +551,10 @@ void qemu_chr_fe_deinit(CharBackend *b) d->backends[b->tag] = NULL; } b->chr = NULL; + qemu_mutex_destroy(&b->chr_lock); + if (b->hotswap_bh) { + qemu_bh_delete(b->hotswap_bh); + } } } @@ -1308,6 +1332,109 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, return ret; } +static void chardev_change_bh(void *opaque) +{ + Chardev *chr_new = opaque; + const char *id = chr_new->label; + Chardev *chr = qemu_chr_find(id); + CharBackend *be = chr->be; + bool closed_sent = false; + + if (!be) { + /* disconnected since we checked: ok, less work for us */ + goto end; + } + + if (chr->be_open && !chr_new->be_open) { + qemu_chr_be_event(chr, CHR_EVENT_CLOSED); + closed_sent = true; + } + + qemu_mutex_lock(&be->chr_lock); + chr->be = NULL; + fe_connect(be, chr_new, &error_abort); + + if (be->chr_be_change(be->opaque) < 0) { + error_report("Chardev '%s' change failed", id); + fe_connect(be, chr, &error_abort); + qemu_mutex_unlock(&be->chr_lock); + if (closed_sent) { + qemu_chr_be_event(chr, CHR_EVENT_OPENED); + } + object_unref(OBJECT(chr_new)); + return; + } + qemu_mutex_unlock(&be->chr_lock); + +end: + object_unparent(OBJECT(chr)); + object_property_add_child(get_chardevs_root(), id, OBJECT(chr_new), + &error_abort); + object_unref(OBJECT(chr_new)); +} + +ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend, + Error **errp) +{ + const ChardevClass *cc; + Chardev *chr, *chr_new; + ChardevReturn *ret; + + chr = qemu_chr_find(id); + if (!chr) { + error_setg(errp, "Chardev '%s' does not exist", id); + return NULL; + } + + if (CHARDEV_IS_MUX(chr)) { + error_setg(errp, "Mux device hotswap not supported yet"); + return NULL; + } + + if (qemu_chr_replay(chr)) { + error_setg(errp, + "Chardev '%s' cannot be changed in record/replay mode", id); + return NULL; + } + + if (!chr->be) { + /* easy case */ + object_unparent(OBJECT(chr)); + return qmp_chardev_add(id, backend, errp); + } + + if (!chr->be->chr_be_change) { + error_setg(errp, "Chardev user does not support chardev hotswap"); + return NULL; + } + + cc = char_get_class(ChardevBackendKind_lookup[backend->type], errp); + if (!cc) { + return NULL; + } + + chr_new = qemu_chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)), + backend, errp); + chr_new->label = g_strdup(id); + if (!chr_new) { + return NULL; + } + + if (chr->be->hotswap_bh) { + qemu_bh_delete(chr->be->hotswap_bh); + } + chr->be->hotswap_bh = qemu_bh_new(chardev_change_bh, chr_new); + qemu_bh_schedule(chr->be->hotswap_bh); + + ret = g_new0(ChardevReturn, 1); + if (CHARDEV_IS_PTY(chr_new)) { + ret->pty = g_strdup(chr_new->filename + 4); + ret->has_pty = true; + } + + return ret; +} + void qmp_chardev_remove(const char *id, Error **errp) { Chardev *chr; diff --git a/include/sysemu/char.h b/include/sysemu/char.h index 9f8df07..68c7876 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -92,6 +92,8 @@ typedef struct CharBackend { void *opaque; int tag; int fe_open; + QemuMutex chr_lock; + QEMUBH *hotswap_bh; } CharBackend; struct Chardev { @@ -141,6 +143,14 @@ void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend); */ Chardev *qemu_chr_new(const char *label, const char *filename); +/** + * @qemu_chr_change: + * + * Change an existing character backend + * + * @opts the new backend options + */ +void qemu_chr_change(QemuOpts *opts, Error **errp); /** * @qemu_chr_fe_disconnect: diff --git a/qapi-schema.json b/qapi-schema.json index 80603cf..bf72166 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -5075,6 +5075,46 @@ 'returns': 'ChardevReturn' } ## +# @chardev-change: +# +# Change a character device backend +# +# @id: the chardev's ID, must exist +# @backend: new backend type and parameters +# +# Returns: ChardevReturn. +# +# Since: 2.10 +# +# Example: +# +# -> { "execute" : "chardev-change", +# "arguments" : { "id" : "baz", +# "backend" : { "type" : "pty", "data" : {} } } } +# <- { "return": { "pty" : "/dev/pty/42" } } +# +# -> {"execute" : "chardev-change", +# "arguments" : { +# "id" : "charchannel2", +# "backend" : { +# "type" : "socket", +# "data" : { +# "addr" : { +# "type" : "unix" , +# "data" : { +# "path" : "/tmp/charchannel2.socket" +# } +# }, +# "server" : true, +# "wait" : false }}}} +# <- {"return": {}} +# +## +{ 'command': 'chardev-change', 'data': {'id' : 'str', + 'backend' : 'ChardevBackend' }, + 'returns': 'ChardevReturn' } + +## # @chardev-remove: # # Remove a character device backend