[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] char: chardevice hotswap
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] char: chardevice hotswap |
Date: |
Thu, 9 Feb 2017 16:09:22 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Thu, Feb 09, 2017 at 06:25:33PM +0300, Denis V. Lunev wrote:
> From: Anton Nefedov <address@hidden>
>
> This patch adds a possibility to change a char device without a frontend
> removal.
>
> Ideally, it would have to happen transparently to a frontend, i.e. frontend
> would continue its regular operation. However, backends are not stateles
> 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.
>
> 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.
>
> 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)
>
> Signed-off-by: Anton Nefedov <address@hidden>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Paolo Bonzini <address@hidden>
> CC: "Marc-André Lureau" <address@hidden>
> CC: "Dr. David Alan Gilbert" <address@hidden>
> ---
> chardev/char.c | 161
> +++++++++++++++++++++++++++++++++++++++++++++++---
> hmp.c | 14 +++++
IIRC we required new commands to have a QMP addition and the new HMP
function written by invoking the QMP handler.
> diff --git a/hmp.c b/hmp.c
> index 2bc4f06..70252df 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1522,6 +1522,20 @@ void hmp_change(Monitor *mon, const QDict *qdict)
> }
> }
> qmp_change("vnc", target, !!arg, arg, &err);
> + } else if (strcmp(device, "chardev") == 0) {
> + QemuOpts *opts;
> +
> + if (arg == NULL) {
> + arg = "";
> + }
> + opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"), arg, true);
> + if (opts == NULL) {
> + error_setg(&err, "Parsing chardev args failed");
> + } else {
> + qemu_opts_set_id(opts, g_strdup(target));
> + qemu_chr_change(opts, &err);
> + qemu_opts_del(opts);
> + }
The hmp 'change' command is/was a huge mistake. We shouldn't continue
to add stuff to it - create dedicated commands for any new functionality
instead of over-loading it one command todo many different things.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|