* [PATCH 0/4] chardev fixes
@ 2021-07-23 10:28 marcandre.lureau
2021-07-23 10:28 ` [PATCH 1/4] chardev: fix qemu_chr_open_fd() being called with fd=-1 marcandre.lureau
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: marcandre.lureau @ 2021-07-23 10:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, berrange, Paolo Bonzini
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
Two small fixes related to fd handling for "serial" and "file" backend and some
API comments and minor improvement.
Marc-André Lureau (4):
chardev: fix qemu_chr_open_fd() being called with fd=-1
chardev: fix qemu_chr_open_fd() with fd_in==fd_out
chardev: remove needless class method
chardev: add some comments about the class methods
include/chardev/char.h | 34 +++++++++++++++++++++++++++++++++-
chardev/char-fd.c | 31 +++++++++++++++++++++++--------
chardev/char-mux.c | 6 ++----
3 files changed, 58 insertions(+), 13 deletions(-)
--
2.32.0.264.g75ae10bc75
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] chardev: fix qemu_chr_open_fd() being called with fd=-1
2021-07-23 10:28 [PATCH 0/4] chardev fixes marcandre.lureau
@ 2021-07-23 10:28 ` marcandre.lureau
2021-08-03 8:40 ` Daniel P. Berrangé
2021-07-23 10:28 ` [PATCH 2/4] chardev: fix qemu_chr_open_fd() with fd_in==fd_out marcandre.lureau
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: marcandre.lureau @ 2021-07-23 10:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, berrange, Paolo Bonzini
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The "file" chardev may call qemu_chr_open_fd() with fd_in=-1. This may
cause invalid system calls, as the QIOChannel is assumed to be properly
initialized later on.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
chardev/char-fd.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index 1cd62f2779..ee85a52c06 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -133,15 +133,19 @@ void qemu_chr_open_fd(Chardev *chr,
FDChardev *s = FD_CHARDEV(chr);
char *name;
- s->ioc_in = QIO_CHANNEL(qio_channel_file_new_fd(fd_in));
- name = g_strdup_printf("chardev-file-in-%s", chr->label);
- qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
- g_free(name);
- s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out));
- name = g_strdup_printf("chardev-file-out-%s", chr->label);
- qio_channel_set_name(QIO_CHANNEL(s->ioc_out), name);
- g_free(name);
- qemu_set_nonblock(fd_out);
+ if (fd_in >= 0) {
+ s->ioc_in = QIO_CHANNEL(qio_channel_file_new_fd(fd_in));
+ name = g_strdup_printf("chardev-file-in-%s", chr->label);
+ qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
+ g_free(name);
+ }
+ if (fd_out >= 0) {
+ s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out));
+ name = g_strdup_printf("chardev-file-out-%s", chr->label);
+ qio_channel_set_name(QIO_CHANNEL(s->ioc_out), name);
+ g_free(name);
+ qemu_set_nonblock(fd_out);
+ }
}
static void char_fd_class_init(ObjectClass *oc, void *data)
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] chardev: fix qemu_chr_open_fd() with fd_in==fd_out
2021-07-23 10:28 [PATCH 0/4] chardev fixes marcandre.lureau
2021-07-23 10:28 ` [PATCH 1/4] chardev: fix qemu_chr_open_fd() being called with fd=-1 marcandre.lureau
@ 2021-07-23 10:28 ` marcandre.lureau
2021-08-03 8:44 ` Daniel P. Berrangé
2021-07-23 10:28 ` [PATCH 3/4] chardev: remove needless class method marcandre.lureau
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: marcandre.lureau @ 2021-07-23 10:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, berrange, Paolo Bonzini
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The "serial" chardev calls qemu_chr_open_fd() with the same fd. This
may lead to double-close as each QIOChannel owns the fd.
Instead, share the reference to the same QIOChannel.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
chardev/char-fd.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index ee85a52c06..32166182bf 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -139,13 +139,24 @@ void qemu_chr_open_fd(Chardev *chr,
qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
g_free(name);
}
- if (fd_out >= 0) {
+
+ if (fd_out < 0) {
+ return;
+ }
+
+ if (fd_out == fd_in) {
+ s->ioc_out = QIO_CHANNEL(object_ref(s->ioc_in));
+ name = g_strdup_printf("chardev-file-%s", chr->label);
+ qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
+ g_free(name);
+ } else {
s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out));
name = g_strdup_printf("chardev-file-out-%s", chr->label);
qio_channel_set_name(QIO_CHANNEL(s->ioc_out), name);
g_free(name);
- qemu_set_nonblock(fd_out);
}
+
+ qemu_set_nonblock(fd_out);
}
static void char_fd_class_init(ObjectClass *oc, void *data)
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] chardev: remove needless class method
2021-07-23 10:28 [PATCH 0/4] chardev fixes marcandre.lureau
2021-07-23 10:28 ` [PATCH 1/4] chardev: fix qemu_chr_open_fd() being called with fd=-1 marcandre.lureau
2021-07-23 10:28 ` [PATCH 2/4] chardev: fix qemu_chr_open_fd() with fd_in==fd_out marcandre.lureau
@ 2021-07-23 10:28 ` marcandre.lureau
2021-08-02 18:53 ` Philippe Mathieu-Daudé
2021-08-03 8:45 ` Daniel P. Berrangé
2021-07-23 10:28 ` [PATCH 4/4] chardev: add some comments about the class methods marcandre.lureau
2021-08-02 18:49 ` [PATCH 0/4] chardev fixes Marc-André Lureau
4 siblings, 2 replies; 12+ messages in thread
From: marcandre.lureau @ 2021-07-23 10:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, berrange, Paolo Bonzini
From: Marc-André Lureau <marcandre.lureau@redhat.com>
"chr_option_parsed" is only implemented by the "mux" chardev, we can
specialize the code there to avoid the needless generic class method.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/chardev/char.h | 1 -
chardev/char-mux.c | 6 ++----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 7c0444f90d..589e7fe46d 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -273,7 +273,6 @@ struct ChardevClass {
void (*chr_set_echo)(Chardev *chr, bool echo);
void (*chr_set_fe_open)(Chardev *chr, int fe_open);
void (*chr_be_event)(Chardev *s, QEMUChrEvent event);
- void (*chr_options_parsed)(Chardev *chr);
};
Chardev *qemu_chardev_new(const char *id, const char *typename,
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 5baf419010..ada0c6866f 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -386,10 +386,9 @@ void suspend_mux_open(void)
static int chardev_options_parsed_cb(Object *child, void *opaque)
{
Chardev *chr = (Chardev *)child;
- ChardevClass *class = CHARDEV_GET_CLASS(chr);
- if (!chr->be_open && class->chr_options_parsed) {
- class->chr_options_parsed(chr);
+ if (!chr->be_open && CHARDEV_IS_MUX(chr)) {
+ open_muxes(chr);
}
return 0;
@@ -412,7 +411,6 @@ static void char_mux_class_init(ObjectClass *oc, void *data)
cc->chr_accept_input = mux_chr_accept_input;
cc->chr_add_watch = mux_chr_add_watch;
cc->chr_be_event = mux_chr_be_event;
- cc->chr_options_parsed = open_muxes;
cc->chr_update_read_handler = mux_chr_update_read_handlers;
}
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] chardev: add some comments about the class methods
2021-07-23 10:28 [PATCH 0/4] chardev fixes marcandre.lureau
` (2 preceding siblings ...)
2021-07-23 10:28 ` [PATCH 3/4] chardev: remove needless class method marcandre.lureau
@ 2021-07-23 10:28 ` marcandre.lureau
2021-08-03 8:46 ` Daniel P. Berrangé
2021-08-02 18:49 ` [PATCH 0/4] chardev fixes Marc-André Lureau
4 siblings, 1 reply; 12+ messages in thread
From: marcandre.lureau @ 2021-07-23 10:28 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, berrange, Paolo Bonzini
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/chardev/char.h | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 589e7fe46d..2e4c16f82f 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -254,24 +254,57 @@ struct ChardevClass {
bool internal; /* TODO: eventually use TYPE_USER_CREATABLE */
bool supports_yank;
+
+ /* parse command line options and populate QAPI @backend */
void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
+ /* called after construction, open/starts the backend */
void (*open)(Chardev *chr, ChardevBackend *backend,
bool *be_opened, Error **errp);
+ /* write buf to the backend */
int (*chr_write)(Chardev *s, const uint8_t *buf, int len);
+
+ /*
+ * Read from the backend (blocking). A typical front-end will instead rely
+ * on char_can_read/chr_read being called when polling/looping.
+ */
int (*chr_sync_read)(Chardev *s, const uint8_t *buf, int len);
+
+ /* create a watch on the backend */
GSource *(*chr_add_watch)(Chardev *s, GIOCondition cond);
+
+ /* update the backend internal sources */
void (*chr_update_read_handler)(Chardev *s);
+
+ /* send an ioctl to the backend */
int (*chr_ioctl)(Chardev *s, int cmd, void *arg);
+
+ /* get ancillary-received fds during last read */
int (*get_msgfds)(Chardev *s, int* fds, int num);
+
+ /* set ancillary fds to be sent with next write */
int (*set_msgfds)(Chardev *s, int *fds, int num);
+
+ /* accept the given fd */
int (*chr_add_client)(Chardev *chr, int fd);
+
+ /* wait for a connection */
int (*chr_wait_connected)(Chardev *chr, Error **errp);
+
+ /* disconnect a connection */
void (*chr_disconnect)(Chardev *chr);
+
+ /* called by frontend when it can read */
void (*chr_accept_input)(Chardev *chr);
+
+ /* set terminal echo */
void (*chr_set_echo)(Chardev *chr, bool echo);
+
+ /* notify the backend of frontend open state */
void (*chr_set_fe_open)(Chardev *chr, int fe_open);
+
+ /* handle various events */
void (*chr_be_event)(Chardev *s, QEMUChrEvent event);
};
--
2.32.0.264.g75ae10bc75
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] chardev fixes
2021-07-23 10:28 [PATCH 0/4] chardev fixes marcandre.lureau
` (3 preceding siblings ...)
2021-07-23 10:28 ` [PATCH 4/4] chardev: add some comments about the class methods marcandre.lureau
@ 2021-08-02 18:49 ` Marc-André Lureau
4 siblings, 0 replies; 12+ messages in thread
From: Marc-André Lureau @ 2021-08-02 18:49 UTC (permalink / raw)
To: QEMU; +Cc: Paolo Bonzini, Daniel P. Berrange
[-- Attachment #1: Type: text/plain, Size: 917 bytes --]
Hi
On Fri, Jul 23, 2021 at 2:29 PM <marcandre.lureau@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> Two small fixes related to fd handling for "serial" and "file" backend and
> some
> API comments and minor improvement.
>
> Marc-André Lureau (4):
> chardev: fix qemu_chr_open_fd() being called with fd=-1
> chardev: fix qemu_chr_open_fd() with fd_in==fd_out
>
I think I could queue those 2 patches for 6.1 as fixes. Daniel, care to
review?
thanks
chardev: remove needless class method
> chardev: add some comments about the class methods
>
> include/chardev/char.h | 34 +++++++++++++++++++++++++++++++++-
> chardev/char-fd.c | 31 +++++++++++++++++++++++--------
> chardev/char-mux.c | 6 ++----
> 3 files changed, 58 insertions(+), 13 deletions(-)
>
> --
> 2.32.0.264.g75ae10bc75
>
>
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 1613 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] chardev: remove needless class method
2021-07-23 10:28 ` [PATCH 3/4] chardev: remove needless class method marcandre.lureau
@ 2021-08-02 18:53 ` Philippe Mathieu-Daudé
2021-08-03 8:45 ` Daniel P. Berrangé
1 sibling, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-02 18:53 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Paolo Bonzini, berrange
On 7/23/21 12:28 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> "chr_option_parsed" is only implemented by the "mux" chardev, we can
> specialize the code there to avoid the needless generic class method.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/chardev/char.h | 1 -
> chardev/char-mux.c | 6 ++----
> 2 files changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] chardev: fix qemu_chr_open_fd() being called with fd=-1
2021-07-23 10:28 ` [PATCH 1/4] chardev: fix qemu_chr_open_fd() being called with fd=-1 marcandre.lureau
@ 2021-08-03 8:40 ` Daniel P. Berrangé
2021-08-04 11:11 ` Marc-André Lureau
0 siblings, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2021-08-03 8:40 UTC (permalink / raw)
To: marcandre.lureau; +Cc: Paolo Bonzini, qemu-devel
On Fri, Jul 23, 2021 at 02:28:22PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The "file" chardev may call qemu_chr_open_fd() with fd_in=-1. This may
> cause invalid system calls, as the QIOChannel is assumed to be properly
> initialized later on.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> chardev/char-fd.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/chardev/char-fd.c b/chardev/char-fd.c
> index 1cd62f2779..ee85a52c06 100644
> --- a/chardev/char-fd.c
> +++ b/chardev/char-fd.c
> @@ -133,15 +133,19 @@ void qemu_chr_open_fd(Chardev *chr,
> FDChardev *s = FD_CHARDEV(chr);
> char *name;
>
> - s->ioc_in = QIO_CHANNEL(qio_channel_file_new_fd(fd_in));
> - name = g_strdup_printf("chardev-file-in-%s", chr->label);
> - qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
> - g_free(name);
> - s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out));
> - name = g_strdup_printf("chardev-file-out-%s", chr->label);
> - qio_channel_set_name(QIO_CHANNEL(s->ioc_out), name);
> - g_free(name);
> - qemu_set_nonblock(fd_out);
> + if (fd_in >= 0) {
> + s->ioc_in = QIO_CHANNEL(qio_channel_file_new_fd(fd_in));
> + name = g_strdup_printf("chardev-file-in-%s", chr->label);
> + qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
> + g_free(name);
> + }
> + if (fd_out >= 0) {
> + s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out));
> + name = g_strdup_printf("chardev-file-out-%s", chr->label);
> + qio_channel_set_name(QIO_CHANNEL(s->ioc_out), name);
> + g_free(name);
> + qemu_set_nonblock(fd_out);
> + }
Other code in this file assumes ioc_out is non-NULL, so this looks
like an incomplete fix.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] chardev: fix qemu_chr_open_fd() with fd_in==fd_out
2021-07-23 10:28 ` [PATCH 2/4] chardev: fix qemu_chr_open_fd() with fd_in==fd_out marcandre.lureau
@ 2021-08-03 8:44 ` Daniel P. Berrangé
0 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2021-08-03 8:44 UTC (permalink / raw)
To: marcandre.lureau; +Cc: Paolo Bonzini, qemu-devel
On Fri, Jul 23, 2021 at 02:28:23PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The "serial" chardev calls qemu_chr_open_fd() with the same fd. This
> may lead to double-close as each QIOChannel owns the fd.
>
> Instead, share the reference to the same QIOChannel.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> chardev/char-fd.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char-fd.c b/chardev/char-fd.c
> index ee85a52c06..32166182bf 100644
> --- a/chardev/char-fd.c
> +++ b/chardev/char-fd.c
> @@ -139,13 +139,24 @@ void qemu_chr_open_fd(Chardev *chr,
> qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
> g_free(name);
> }
> - if (fd_out >= 0) {
> +
> + if (fd_out < 0) {
> + return;
> + }
> +
> + if (fd_out == fd_in) {
> + s->ioc_out = QIO_CHANNEL(object_ref(s->ioc_in));
> + name = g_strdup_printf("chardev-file-%s", chr->label);
> + qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
> + g_free(name);
This is overwriting the name set a few lines earlier. I think the
code ought to be refactor to eliminate this duplication.
ie
if (fd_out == fd_in) {
s->ioc_out = s->ioc_in = QIO_CHANNEL(qio_channel_file_new_fd(fd_in));
....
} else {
s->ioc_in = QIO_CHANNEL(qio_channel_file_new_fd(fd_in));
...
s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out));
...
}
> + } else {
> s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out));
> name = g_strdup_printf("chardev-file-out-%s", chr->label);
> qio_channel_set_name(QIO_CHANNEL(s->ioc_out), name);
> g_free(name);
> - qemu_set_nonblock(fd_out);
> }
> +
> + qemu_set_nonblock(fd_out);
> }
>
> static void char_fd_class_init(ObjectClass *oc, void *data)
> --
> 2.32.0.264.g75ae10bc75
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] chardev: remove needless class method
2021-07-23 10:28 ` [PATCH 3/4] chardev: remove needless class method marcandre.lureau
2021-08-02 18:53 ` Philippe Mathieu-Daudé
@ 2021-08-03 8:45 ` Daniel P. Berrangé
1 sibling, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2021-08-03 8:45 UTC (permalink / raw)
To: marcandre.lureau; +Cc: Paolo Bonzini, qemu-devel
On Fri, Jul 23, 2021 at 02:28:24PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> "chr_option_parsed" is only implemented by the "mux" chardev, we can
> specialize the code there to avoid the needless generic class method.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/chardev/char.h | 1 -
> chardev/char-mux.c | 6 ++----
> 2 files changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] chardev: add some comments about the class methods
2021-07-23 10:28 ` [PATCH 4/4] chardev: add some comments about the class methods marcandre.lureau
@ 2021-08-03 8:46 ` Daniel P. Berrangé
0 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2021-08-03 8:46 UTC (permalink / raw)
To: marcandre.lureau; +Cc: Paolo Bonzini, qemu-devel
On Fri, Jul 23, 2021 at 02:28:25PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/chardev/char.h | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index 589e7fe46d..2e4c16f82f 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -254,24 +254,57 @@ struct ChardevClass {
>
> bool internal; /* TODO: eventually use TYPE_USER_CREATABLE */
> bool supports_yank;
> +
> + /* parse command line options and populate QAPI @backend */
> void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
>
> + /* called after construction, open/starts the backend */
> void (*open)(Chardev *chr, ChardevBackend *backend,
> bool *be_opened, Error **errp);
>
> + /* write buf to the backend */
> int (*chr_write)(Chardev *s, const uint8_t *buf, int len);
> +
> + /*
> + * Read from the backend (blocking). A typical front-end will instead rely
> + * on char_can_read/chr_read being called when polling/looping.
> + */
chr_can_read
> int (*chr_sync_read)(Chardev *s, const uint8_t *buf, int len);
> +
> + /* create a watch on the backend */
> GSource *(*chr_add_watch)(Chardev *s, GIOCondition cond);
> +
> + /* update the backend internal sources */
> void (*chr_update_read_handler)(Chardev *s);
> +
> + /* send an ioctl to the backend */
> int (*chr_ioctl)(Chardev *s, int cmd, void *arg);
> +
> + /* get ancillary-received fds during last read */
> int (*get_msgfds)(Chardev *s, int* fds, int num);
> +
> + /* set ancillary fds to be sent with next write */
> int (*set_msgfds)(Chardev *s, int *fds, int num);
> +
> + /* accept the given fd */
> int (*chr_add_client)(Chardev *chr, int fd);
> +
> + /* wait for a connection */
> int (*chr_wait_connected)(Chardev *chr, Error **errp);
> +
> + /* disconnect a connection */
> void (*chr_disconnect)(Chardev *chr);
> +
> + /* called by frontend when it can read */
> void (*chr_accept_input)(Chardev *chr);
> +
> + /* set terminal echo */
> void (*chr_set_echo)(Chardev *chr, bool echo);
> +
> + /* notify the backend of frontend open state */
> void (*chr_set_fe_open)(Chardev *chr, int fe_open);
> +
> + /* handle various events */
> void (*chr_be_event)(Chardev *s, QEMUChrEvent event);
> };
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] chardev: fix qemu_chr_open_fd() being called with fd=-1
2021-08-03 8:40 ` Daniel P. Berrangé
@ 2021-08-04 11:11 ` Marc-André Lureau
0 siblings, 0 replies; 12+ messages in thread
From: Marc-André Lureau @ 2021-08-04 11:11 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Paolo Bonzini, QEMU
[-- Attachment #1: Type: text/plain, Size: 2243 bytes --]
Hi
On Tue, Aug 3, 2021 at 12:41 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> On Fri, Jul 23, 2021 at 02:28:22PM +0400, marcandre.lureau@redhat.com
> wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The "file" chardev may call qemu_chr_open_fd() with fd_in=-1. This may
> > cause invalid system calls, as the QIOChannel is assumed to be properly
> > initialized later on.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > chardev/char-fd.c | 22 +++++++++++++---------
> > 1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/chardev/char-fd.c b/chardev/char-fd.c
> > index 1cd62f2779..ee85a52c06 100644
> > --- a/chardev/char-fd.c
> > +++ b/chardev/char-fd.c
> > @@ -133,15 +133,19 @@ void qemu_chr_open_fd(Chardev *chr,
> > FDChardev *s = FD_CHARDEV(chr);
> > char *name;
> >
> > - s->ioc_in = QIO_CHANNEL(qio_channel_file_new_fd(fd_in));
> > - name = g_strdup_printf("chardev-file-in-%s", chr->label);
> > - qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
> > - g_free(name);
> > - s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out));
> > - name = g_strdup_printf("chardev-file-out-%s", chr->label);
> > - qio_channel_set_name(QIO_CHANNEL(s->ioc_out), name);
> > - g_free(name);
> > - qemu_set_nonblock(fd_out);
> > + if (fd_in >= 0) {
> > + s->ioc_in = QIO_CHANNEL(qio_channel_file_new_fd(fd_in));
> > + name = g_strdup_printf("chardev-file-in-%s", chr->label);
> > + qio_channel_set_name(QIO_CHANNEL(s->ioc_in), name);
> > + g_free(name);
> > + }
> > + if (fd_out >= 0) {
> > + s->ioc_out = QIO_CHANNEL(qio_channel_file_new_fd(fd_out));
> > + name = g_strdup_printf("chardev-file-out-%s", chr->label);
> > + qio_channel_set_name(QIO_CHANNEL(s->ioc_out), name);
> > + g_free(name);
> > + qemu_set_nonblock(fd_out);
> > + }
>
> Other code in this file assumes ioc_out is non-NULL, so this looks
> like an incomplete fix.
>
>
Right, I am adding a patch to correct the source watch creation, and fixing
this one handling !ioc_out condition.
thanks
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 3260 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-08-04 11:13 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-23 10:28 [PATCH 0/4] chardev fixes marcandre.lureau
2021-07-23 10:28 ` [PATCH 1/4] chardev: fix qemu_chr_open_fd() being called with fd=-1 marcandre.lureau
2021-08-03 8:40 ` Daniel P. Berrangé
2021-08-04 11:11 ` Marc-André Lureau
2021-07-23 10:28 ` [PATCH 2/4] chardev: fix qemu_chr_open_fd() with fd_in==fd_out marcandre.lureau
2021-08-03 8:44 ` Daniel P. Berrangé
2021-07-23 10:28 ` [PATCH 3/4] chardev: remove needless class method marcandre.lureau
2021-08-02 18:53 ` Philippe Mathieu-Daudé
2021-08-03 8:45 ` Daniel P. Berrangé
2021-07-23 10:28 ` [PATCH 4/4] chardev: add some comments about the class methods marcandre.lureau
2021-08-03 8:46 ` Daniel P. Berrangé
2021-08-02 18:49 ` [PATCH 0/4] chardev fixes Marc-André Lureau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).