This patch allows the socket chardev async connection be setup with
non-default gcontext. We do it by postponing the setup to machine done,
since until then we can know which context we should run the async
operation on.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
chardev/char-socket.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 1ce5adad9a..165612845a 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1004,9 +1004,8 @@ static void qmp_chardev_open_socket(Chardev *chr,
s->reconnect_time = reconnect;
}
- if (s->reconnect_time) {
- tcp_chr_connect_async(chr);
- } else {
+ /* If reconnect_time is set, will do that in chr_machine_done. */
+ if (!s->reconnect_time) {
if (s->is_listen) {
char *name;
s->listener = qio_net_listener_new();
@@ -1138,6 +1137,17 @@ char_socket_get_connected(Object *obj, Error **errp)
return s->connected;
}
+static int tcp_chr_machine_done_hook(Chardev *chr)
+{
+ SocketChardev *s = SOCKET_CHARDEV(chr);
+
+ if (s->reconnect_time) {
+ tcp_chr_connect_async(chr);
+ }
+
+ return 0;
+}
+
static void char_socket_class_init(ObjectClass *oc, void *data)
{
ChardevClass *cc = CHARDEV_CLASS(oc);
@@ -1153,6 +1163,7 @@ static void char_socket_class_init(ObjectClass *oc, void *data)
cc->chr_add_client = tcp_chr_add_client;
cc->chr_add_watch = tcp_chr_add_watch;
cc->chr_update_read_handler = tcp_chr_update_read_handler;
+ cc->chr_machine_done = tcp_chr_machine_done_hook;
object_class_property_add(oc, "addr", "SocketAddress",
char_socket_get_addr, NULL,
--
2.14.3
On Tue, Mar 06, 2018 at 01:33:18PM +0800, Peter Xu wrote: > This patch allows the socket chardev async connection be setup with > non-default gcontext. We do it by postponing the setup to machine done, > since until then we can know which context we should run the async > operation on. > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > chardev/char-socket.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 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 :|
Hi
On Tue, Mar 6, 2018 at 6:41 AM Peter Xu <peterx@redhat.com> wrote:
>
> This patch allows the socket chardev async connection be setup with
> non-default gcontext. We do it by postponing the setup to machine done,
> since until then we can know which context we should run the async
> operation on.
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> chardev/char-socket.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 1ce5adad9a..165612845a 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1004,9 +1004,8 @@ static void qmp_chardev_open_socket(Chardev *chr,
> s->reconnect_time = reconnect;
> }
>
> - if (s->reconnect_time) {
> - tcp_chr_connect_async(chr);
> - } else {
> + /* If reconnect_time is set, will do that in chr_machine_done. */
> + if (!s->reconnect_time) {
> if (s->is_listen) {
> char *name;
> s->listener = qio_net_listener_new();
> @@ -1138,6 +1137,17 @@ char_socket_get_connected(Object *obj, Error **errp)
> return s->connected;
> }
>
> +static int tcp_chr_machine_done_hook(Chardev *chr)
> +{
> + SocketChardev *s = SOCKET_CHARDEV(chr);
> +
> + if (s->reconnect_time) {
> + tcp_chr_connect_async(chr);
> + }
> +
> + return 0;
> +}
This patch broke /vhost-user/reconnect test, since it is using
reconnect chardev sockets under a test program, where no machine done
hook exist.
> +
> static void char_socket_class_init(ObjectClass *oc, void *data)
> {
> ChardevClass *cc = CHARDEV_CLASS(oc);
> @@ -1153,6 +1163,7 @@ static void char_socket_class_init(ObjectClass *oc, void *data)
> cc->chr_add_client = tcp_chr_add_client;
> cc->chr_add_watch = tcp_chr_add_watch;
> cc->chr_update_read_handler = tcp_chr_update_read_handler;
> + cc->chr_machine_done = tcp_chr_machine_done_hook;
>
> object_class_property_add(oc, "addr", "SocketAddress",
> char_socket_get_addr, NULL,
> --
> 2.14.3
>
>
--
Marc-André Lureau
Hi
On Thu, Aug 16, 2018 at 7:49 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
> On Tue, Mar 6, 2018 at 6:41 AM Peter Xu <peterx@redhat.com> wrote:
> >
> > This patch allows the socket chardev async connection be setup with
> > non-default gcontext. We do it by postponing the setup to machine done,
> > since until then we can know which context we should run the async
> > operation on.
> >
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > chardev/char-socket.c | 17 ++++++++++++++---
> > 1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> > index 1ce5adad9a..165612845a 100644
> > --- a/chardev/char-socket.c
> > +++ b/chardev/char-socket.c
> > @@ -1004,9 +1004,8 @@ static void qmp_chardev_open_socket(Chardev *chr,
> > s->reconnect_time = reconnect;
> > }
> >
> > - if (s->reconnect_time) {
> > - tcp_chr_connect_async(chr);
> > - } else {
> > + /* If reconnect_time is set, will do that in chr_machine_done. */
> > + if (!s->reconnect_time) {
> > if (s->is_listen) {
> > char *name;
> > s->listener = qio_net_listener_new();
> > @@ -1138,6 +1137,17 @@ char_socket_get_connected(Object *obj, Error **errp)
> > return s->connected;
> > }
> >
> > +static int tcp_chr_machine_done_hook(Chardev *chr)
> > +{
> > + SocketChardev *s = SOCKET_CHARDEV(chr);
> > +
> > + if (s->reconnect_time) {
> > + tcp_chr_connect_async(chr);
> > + }
> > +
> > + return 0;
> > +}
>
> This patch broke /vhost-user/reconnect test, since it is using
> reconnect chardev sockets under a test program, where no machine done
> hook exist.
>
What about chardev created after machine done?
> > +
> > static void char_socket_class_init(ObjectClass *oc, void *data)
> > {
> > ChardevClass *cc = CHARDEV_CLASS(oc);
> > @@ -1153,6 +1163,7 @@ static void char_socket_class_init(ObjectClass *oc, void *data)
> > cc->chr_add_client = tcp_chr_add_client;
> > cc->chr_add_watch = tcp_chr_add_watch;
> > cc->chr_update_read_handler = tcp_chr_update_read_handler;
> > + cc->chr_machine_done = tcp_chr_machine_done_hook;
> >
> > object_class_property_add(oc, "addr", "SocketAddress",
> > char_socket_get_addr, NULL,
> > --
> > 2.14.3
> >
> >
>
>
> --
> Marc-André Lureau
--
Marc-André Lureau
On Thu, Aug 16, 2018 at 08:27:40PM +0200, Marc-André Lureau wrote:
> Hi
> On Thu, Aug 16, 2018 at 7:49 PM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> > On Tue, Mar 6, 2018 at 6:41 AM Peter Xu <peterx@redhat.com> wrote:
> > >
> > > This patch allows the socket chardev async connection be setup with
> > > non-default gcontext. We do it by postponing the setup to machine done,
> > > since until then we can know which context we should run the async
> > > operation on.
> > >
> > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > ---
> > > chardev/char-socket.c | 17 ++++++++++++++---
> > > 1 file changed, 14 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> > > index 1ce5adad9a..165612845a 100644
> > > --- a/chardev/char-socket.c
> > > +++ b/chardev/char-socket.c
> > > @@ -1004,9 +1004,8 @@ static void qmp_chardev_open_socket(Chardev *chr,
> > > s->reconnect_time = reconnect;
> > > }
> > >
> > > - if (s->reconnect_time) {
> > > - tcp_chr_connect_async(chr);
> > > - } else {
> > > + /* If reconnect_time is set, will do that in chr_machine_done. */
> > > + if (!s->reconnect_time) {
> > > if (s->is_listen) {
> > > char *name;
> > > s->listener = qio_net_listener_new();
> > > @@ -1138,6 +1137,17 @@ char_socket_get_connected(Object *obj, Error **errp)
> > > return s->connected;
> > > }
> > >
> > > +static int tcp_chr_machine_done_hook(Chardev *chr)
> > > +{
> > > + SocketChardev *s = SOCKET_CHARDEV(chr);
> > > +
> > > + if (s->reconnect_time) {
> > > + tcp_chr_connect_async(chr);
> > > + }
> > > +
> > > + return 0;
> > > +}
> >
> > This patch broke /vhost-user/reconnect test, since it is using
> > reconnect chardev sockets under a test program, where no machine done
> > hook exist.
> >
>
> What about chardev created after machine done?
Hmm, so binding that with machine done might be problematic...
I'm thinking whether we can do that in ->chr_update_read_handler()
instead.
Btw, is there a short answer on why we disabled the reconnection test
in vhost-user-test? Could I just verify the problem using:
QTEST_VHOST_USER_FIXME=1 QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 ./tests/vhost-user-test
? Or any easy way to reproduce/verify the problem?
Regards,
--
Peter Xu
© 2016 - 2025 Red Hat, Inc.