[PATCH 08/29] hw/core: Setup Remote Port I/O channels

Ruslan Ruslichenko posted 29 patches 1 day, 11 hours ago
[PATCH 08/29] hw/core: Setup Remote Port I/O channels
Posted by Ruslan Ruslichenko 1 day, 11 hours ago
From: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>

Add initialization of communication channels with
remote peer.

This includes character device backend, which can be
configured based on QOM properties or automatic socket
creation based on the machine path. The patch also
initializes the signaling event pipes.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Signed-off-by: Takahiro Nakata <takahiro.nakata.wr@renesas.com>
Signed-off-by: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
---
 hw/core/remote-port.c         | 187 ++++++++++++++++++++++++++++++++++
 include/hw/core/remote-port.h |  16 +++
 2 files changed, 203 insertions(+)

diff --git a/hw/core/remote-port.c b/hw/core/remote-port.c
index c909a825f3..5154c1bc2a 100644
--- a/hw/core/remote-port.c
+++ b/hw/core/remote-port.c
@@ -52,6 +52,58 @@
 #define REMOTE_PORT_CLASS(klass)    \
      OBJECT_CLASS_CHECK(RemotePortClass, (klass), TYPE_REMOTE_PORT)
 
+static char *rp_sanitize_prefix(RemotePort *s)
+{
+    char *sanitized_name;
+    char *c;
+
+    sanitized_name = g_strdup(s->prefix);
+    for (c = sanitized_name; *c != '\0'; c++) {
+        if (*c == '/') {
+            *c = '_';
+        }
+    }
+    return sanitized_name;
+}
+
+static char *rp_autocreate_chardesc(RemotePort *s, bool server)
+{
+    char *prefix;
+    char *chardesc;
+    int r;
+
+    prefix = rp_sanitize_prefix(s);
+    r = asprintf(&chardesc, "unix:%s/qemu-rport-%s%s",
+                 machine_path, prefix, server ? ",wait,server" : "");
+    assert(r > 0);
+    free(prefix);
+    return chardesc;
+}
+
+static Chardev *rp_autocreate_chardev(RemotePort *s, char *name)
+{
+    Chardev *chr = NULL;
+    char *chardesc;
+    char *s_path;
+    int r;
+
+    r = asprintf(&s_path, "%s/qemu-rport-%s", machine_path,
+                 rp_sanitize_prefix(s));
+    assert(r > 0);
+    if (g_file_test(s_path, G_FILE_TEST_EXISTS)) {
+        chardesc = rp_autocreate_chardesc(s, false);
+        chr = qemu_chr_new_noreplay(name, chardesc, false, NULL);
+        free(chardesc);
+    }
+    free(s_path);
+
+    if (!chr) {
+        chardesc = rp_autocreate_chardesc(s, true);
+        chr = qemu_chr_new_noreplay(name, chardesc, false, NULL);
+        free(chardesc);
+    }
+    return chr;
+}
 
 static void rp_reset(DeviceState *dev)
 {
@@ -66,6 +118,127 @@ static void rp_reset(DeviceState *dev)
 
 static void rp_realize(DeviceState *dev, Error **errp)
 {
+    RemotePort *s = REMOTE_PORT(dev);
+    int r;
+    Error *err = NULL;
+
+    s->prefix = object_get_canonical_path(OBJECT(dev));
+
+    if (!qemu_chr_fe_get_driver(&s->chr)) {
+        char *name;
+        Chardev *chr = NULL;
+        static int nr;
+
+        r = asprintf(&name, "rport%d", nr);
+        nr++;
+        assert(r > 0);
+
+        if (s->chrdev_id) {
+            chr = qemu_chr_find(s->chrdev_id);
+        }
+
+        if (chr) {
+            /* Found the chardev via commandline */
+        } else if (s->chardesc) {
+            chr = qemu_chr_new(name, s->chardesc, NULL);
+        } else {
+            if (!machine_path) {
+                error_report("%s: Missing chardesc prop."
+                             " Forgot -machine-path?",
+                             s->prefix);
+                exit(EXIT_FAILURE);
+            }
+            chr = rp_autocreate_chardev(s, name);
+        }
+
+        free(name);
+        if (!chr) {
+            error_report("%s: Unable to create remort-port channel %s",
+                         s->prefix, s->chardesc);
+            exit(EXIT_FAILURE);
+        }
+
+        qdev_prop_set_chr(dev, "chardev", chr);
+        s->chrdev = chr;
+    }
+
+#ifdef _WIN32
+    /*
+     * Create a socket connection between two sockets. We auto-bind
+     * and read out the port selected by the kernel.
+     */
+    {
+        char *name;
+        SocketAddress *sock;
+        int port;
+        int listen_sk;
+
+        sock = socket_parse("127.0.0.1:0", &error_abort);
+        listen_sk = socket_listen(sock, 1, &error_abort);
+
+        if (s->event.pipe.read < 0) {
+            perror("socket read");
+            exit(EXIT_FAILURE);
+        }
+
+        {
+            struct sockaddr_in saddr;
+            socklen_t slen = sizeof saddr;
+            int r;
+
+            r = getsockname(listen_sk, (struct sockaddr *) &saddr, &slen);
+            if (r < 0) {
+                perror("getsockname");
+                exit(EXIT_FAILURE);
+            }
+            port = htons(saddr.sin_port);
+        }
+
+        name = g_strdup_printf("127.0.0.1:%d", port);
+        s->event.pipe.write = inet_connect(name, &error_abort);
+        g_free(name);
+        if (s->event.pipe.write < 0) {
+            perror("socket write");
+            exit(EXIT_FAILURE);
+        }
+
+        for (;;) {
+            struct sockaddr_in saddr;
+            socklen_t slen = sizeof saddr;
+            int fd;
+
+            slen = sizeof(saddr);
+            fd = qemu_accept(listen_sk, (struct sockaddr *)&saddr, &slen);
+            if (fd < 0 && errno != EINTR) {
+                close(listen_sk);
+                return;
+            } else if (fd >= 0) {
+                close(listen_sk);
+                s->event.pipe.read = fd;
+                break;
+            }
+        }
+
+        if (!qemu_set_blocking(s->event.pipe.read, false, &err)) {
+            error_report("%s: Unable to set non-block for internal pipes",
+                        s->prefix);
+            exit(EXIT_FAILURE);
+        }
+    }
+#else
+    if (!g_unix_open_pipe(s->event.pipes, FD_CLOEXEC, NULL)) {
+        error_report("%s: Unable to create remort-port internal pipes",
+                    s->prefix);
+        exit(EXIT_FAILURE);
+    }
+
+    if (!qemu_set_blocking(s->event.pipe.read, false, &err)) {
+        error_report("%s: Unable to set non-block for internal pipes",
+                    s->prefix);
+        exit(EXIT_FAILURE);
+    }
+
+#endif
 }
 
 static void rp_unrealize(DeviceState *dev)
@@ -73,6 +246,13 @@ static void rp_unrealize(DeviceState *dev)
     RemotePort *s = REMOTE_PORT(dev);
 
     s->finalizing = true;
+
+    info_report("%s: Wait for remote-port to disconnect", s->prefix);
+    qemu_chr_fe_disconnect(&s->chr);
+
+    close(s->event.pipe.read);
+    close(s->event.pipe.write);
+    object_unparent(OBJECT(s->chrdev));
 }
 
 static const VMStateDescription vmstate_rp = {
@@ -84,6 +264,12 @@ static const VMStateDescription vmstate_rp = {
     }
 };
 
+static Property rp_properties[] = {
+    DEFINE_PROP_CHR("chardev", RemotePort, chr),
+    DEFINE_PROP_STRING("chardesc", RemotePort, chardesc),
+    DEFINE_PROP_STRING("chrdev-id", RemotePort, chrdev_id),
+};
+
 static void rp_prop_allow_set_link(const Object *obj, const char *name,
                                    Object *val, Error **errp)
 {
@@ -112,6 +298,7 @@ static void rp_class_init(ObjectClass *klass, const void *data)
     dc->realize = rp_realize;
     dc->unrealize = rp_unrealize;
     dc->vmsd = &vmstate_rp;
+    device_class_set_props_n(dc, rp_properties, ARRAY_SIZE(rp_properties));
 }
 
 static const TypeInfo rp_info = {
diff --git a/include/hw/core/remote-port.h b/include/hw/core/remote-port.h
index db71071c8e..0f40018cdb 100644
--- a/include/hw/core/remote-port.h
+++ b/include/hw/core/remote-port.h
@@ -56,8 +56,24 @@ typedef struct RemotePortDeviceClass {
 struct RemotePort {
     DeviceState parent;
 
+    union {
+       int pipes[2];
+       struct {
+           int read;
+           int write;
+       } pipe;
+    } event;
+    Chardev *chrdev;
+    CharFrontend chr;
     bool finalizing;
 
+    char *chardesc;
+    char *chrdev_id;
+
+    const char *prefix;
+    const char *remote_prefix;
+
+    uint32_t current_id;
     bool reset_done;
 
 #define REMOTE_PORT_MAX_DEVS 1024
-- 
2.43.0
Re: [PATCH 08/29] hw/core: Setup Remote Port I/O channels
Posted by Daniel P. Berrangé 1 day, 11 hours ago
On Thu, Feb 05, 2026 at 08:58:03PM +0100, Ruslan Ruslichenko wrote:
> From: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
> 
> Add initialization of communication channels with
> remote peer.
> 
> This includes character device backend, which can be
> configured based on QOM properties or automatic socket
> creation based on the machine path. The patch also
> initializes the signaling event pipes.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> Signed-off-by: Takahiro Nakata <takahiro.nakata.wr@renesas.com>
> Signed-off-by: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
> ---
>  hw/core/remote-port.c         | 187 ++++++++++++++++++++++++++++++++++
>  include/hw/core/remote-port.h |  16 +++
>  2 files changed, 203 insertions(+)
> 
> diff --git a/hw/core/remote-port.c b/hw/core/remote-port.c
> index c909a825f3..5154c1bc2a 100644
> --- a/hw/core/remote-port.c
> +++ b/hw/core/remote-port.c
> @@ -52,6 +52,58 @@
>  #define REMOTE_PORT_CLASS(klass)    \
>       OBJECT_CLASS_CHECK(RemotePortClass, (klass), TYPE_REMOTE_PORT)
>  
> +static char *rp_sanitize_prefix(RemotePort *s)
> +{
> +    char *sanitized_name;
> +    char *c;
> +
> +    sanitized_name = g_strdup(s->prefix);
> +    for (c = sanitized_name; *c != '\0'; c++) {
> +        if (*c == '/') {
> +            *c = '_';
> +        }
> +    }
> +    return sanitized_name;
> +}
> +
> +static char *rp_autocreate_chardesc(RemotePort *s, bool server)
> +{
> +    char *prefix;
> +    char *chardesc;
> +    int r;
> +
> +    prefix = rp_sanitize_prefix(s);
> +    r = asprintf(&chardesc, "unix:%s/qemu-rport-%s%s",
> +                 machine_path, prefix, server ? ",wait,server" : "");
> +    assert(r > 0);
> +    free(prefix);
> +    return chardesc;
> +}
> +
> +static Chardev *rp_autocreate_chardev(RemotePort *s, char *name)
> +{
> +    Chardev *chr = NULL;
> +    char *chardesc;
> +    char *s_path;
> +    int r;
> +
> +    r = asprintf(&s_path, "%s/qemu-rport-%s", machine_path,
> +                 rp_sanitize_prefix(s));
> +    assert(r > 0);
> +    if (g_file_test(s_path, G_FILE_TEST_EXISTS)) {
> +        chardesc = rp_autocreate_chardesc(s, false);
> +        chr = qemu_chr_new_noreplay(name, chardesc, false, NULL);
> +        free(chardesc);
> +    }
> +    free(s_path);
> +
> +    if (!chr) {
> +        chardesc = rp_autocreate_chardesc(s, true);
> +        chr = qemu_chr_new_noreplay(name, chardesc, false, NULL);
> +        free(chardesc);
> +    }
> +    return chr;
> +}
>  
>  static void rp_reset(DeviceState *dev)
>  {
> @@ -66,6 +118,127 @@ static void rp_reset(DeviceState *dev)
>  
>  static void rp_realize(DeviceState *dev, Error **errp)
>  {
> +    RemotePort *s = REMOTE_PORT(dev);
> +    int r;
> +    Error *err = NULL;
> +
> +    s->prefix = object_get_canonical_path(OBJECT(dev));
> +
> +    if (!qemu_chr_fe_get_driver(&s->chr)) {
> +        char *name;
> +        Chardev *chr = NULL;
> +        static int nr;
> +
> +        r = asprintf(&name, "rport%d", nr);
> +        nr++;
> +        assert(r > 0);
> +
> +        if (s->chrdev_id) {
> +            chr = qemu_chr_find(s->chrdev_id);
> +        }
> +
> +        if (chr) {
> +            /* Found the chardev via commandline */
> +        } else if (s->chardesc) {
> +            chr = qemu_chr_new(name, s->chardesc, NULL);
> +        } else {
> +            if (!machine_path) {
> +                error_report("%s: Missing chardesc prop."
> +                             " Forgot -machine-path?",
> +                             s->prefix);
> +                exit(EXIT_FAILURE);
> +            }
> +            chr = rp_autocreate_chardev(s, name);
> +        }

Having three different ways to configure the chardev rather
feels like overkill. IMHO it should be sufficient to just
accept thue chardev ID as a mandatory argument, and not
attempt to create chardevs from this code. That would
in turn seem to avoid the need for the -machine-path arg
to exist ?


> +#ifdef _WIN32
> +    /*
> +     * Create a socket connection between two sockets. We auto-bind
> +     * and read out the port selected by the kernel.
> +     */
> +    {
> +        char *name;
> +        SocketAddress *sock;
> +        int port;
> +        int listen_sk;
> +
> +        sock = socket_parse("127.0.0.1:0", &error_abort);
> +        listen_sk = socket_listen(sock, 1, &error_abort);

Please avoid introducing new usage of low level sockets APIs. The higher
level QIOChannelSocket APIs is preferred, that said.....

> +
> +        if (s->event.pipe.read < 0) {
> +            perror("socket read");
> +            exit(EXIT_FAILURE);
> +        }
> +
> +        {
> +            struct sockaddr_in saddr;
> +            socklen_t slen = sizeof saddr;
> +            int r;
> +
> +            r = getsockname(listen_sk, (struct sockaddr *) &saddr, &slen);
> +            if (r < 0) {
> +                perror("getsockname");
> +                exit(EXIT_FAILURE);
> +            }
> +            port = htons(saddr.sin_port);
> +        }
> +
> +        name = g_strdup_printf("127.0.0.1:%d", port);
> +        s->event.pipe.write = inet_connect(name, &error_abort);
> +        g_free(name);
> +        if (s->event.pipe.write < 0) {
> +            perror("socket write");
> +            exit(EXIT_FAILURE);
> +        }
> +
> +        for (;;) {
> +            struct sockaddr_in saddr;
> +            socklen_t slen = sizeof saddr;
> +            int fd;
> +
> +            slen = sizeof(saddr);
> +            fd = qemu_accept(listen_sk, (struct sockaddr *)&saddr, &slen);
> +            if (fd < 0 && errno != EINTR) {
> +                close(listen_sk);
> +                return;
> +            } else if (fd >= 0) {
> +                close(listen_sk);
> +                s->event.pipe.read = fd;
> +                break;
> +            }
> +        }
> +
> +        if (!qemu_set_blocking(s->event.pipe.read, false, &err)) {
> +            error_report("%s: Unable to set non-block for internal pipes",
> +                        s->prefix);
> +            exit(EXIT_FAILURE);
> +        }
> +    }
> +#else
> +    if (!g_unix_open_pipe(s->event.pipes, FD_CLOEXEC, NULL)) {
> +        error_report("%s: Unable to create remort-port internal pipes",
> +                    s->prefix);
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    if (!qemu_set_blocking(s->event.pipe.read, false, &err)) {
> +        error_report("%s: Unable to set non-block for internal pipes",
> +                    s->prefix);
> +        exit(EXIT_FAILURE);
> +    }
> +
> +#endif

... I can't help thinking we have this code "self event pipe" design
somewhere else in QEMU. Ideally we would have a helper API for this
task and avoid OS conditional code in this device impl.


>  }
>  
>  static void rp_unrealize(DeviceState *dev)
> @@ -73,6 +246,13 @@ static void rp_unrealize(DeviceState *dev)
>      RemotePort *s = REMOTE_PORT(dev);
>  
>      s->finalizing = true;
> +
> +    info_report("%s: Wait for remote-port to disconnect", s->prefix);
> +    qemu_chr_fe_disconnect(&s->chr);
> +
> +    close(s->event.pipe.read);
> +    close(s->event.pipe.write);
> +    object_unparent(OBJECT(s->chrdev));
>  }
>  
>  static const VMStateDescription vmstate_rp = {
> @@ -84,6 +264,12 @@ static const VMStateDescription vmstate_rp = {
>      }
>  };
>  
> +static Property rp_properties[] = {
> +    DEFINE_PROP_CHR("chardev", RemotePort, chr),
> +    DEFINE_PROP_STRING("chardesc", RemotePort, chardesc),
> +    DEFINE_PROP_STRING("chrdev-id", RemotePort, chrdev_id),
> +};

This really feels lik

> +
>  static void rp_prop_allow_set_link(const Object *obj, const char *name,
>                                     Object *val, Error **errp)
>  {
> @@ -112,6 +298,7 @@ static void rp_class_init(ObjectClass *klass, const void *data)
>      dc->realize = rp_realize;
>      dc->unrealize = rp_unrealize;
>      dc->vmsd = &vmstate_rp;
> +    device_class_set_props_n(dc, rp_properties, ARRAY_SIZE(rp_properties));
>  }
>  
>  static const TypeInfo rp_info = {
> diff --git a/include/hw/core/remote-port.h b/include/hw/core/remote-port.h
> index db71071c8e..0f40018cdb 100644
> --- a/include/hw/core/remote-port.h
> +++ b/include/hw/core/remote-port.h
> @@ -56,8 +56,24 @@ typedef struct RemotePortDeviceClass {
>  struct RemotePort {
>      DeviceState parent;
>  
> +    union {
> +       int pipes[2];
> +       struct {
> +           int read;
> +           int write;
> +       } pipe;
> +    } event;
> +    Chardev *chrdev;
> +    CharFrontend chr;
>      bool finalizing;
>  
> +    char *chardesc;
> +    char *chrdev_id;
> +
> +    const char *prefix;
> +    const char *remote_prefix;
> +
> +    uint32_t current_id;
>      bool reset_done;
>  
>  #define REMOTE_PORT_MAX_DEVS 1024
> -- 
> 2.43.0
> 
> 

With 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 :|
Re: [PATCH 08/29] hw/core: Setup Remote Port I/O channels
Posted by Ruslan Ruslichenko 13 hours ago
Hello Daniel,
Thank you for your review.

On Thu, Feb 5, 2026 at 9:28 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Feb 05, 2026 at 08:58:03PM +0100, Ruslan Ruslichenko wrote:
> > From: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
> >
> > Add initialization of communication channels with
> > remote peer.
> >
> > This includes character device backend, which can be
> > configured based on QOM properties or automatic socket
> > creation based on the machine path. The patch also
> > initializes the signaling event pipes.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > Signed-off-by: Takahiro Nakata <takahiro.nakata.wr@renesas.com>
> > Signed-off-by: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
> > ---
> >  hw/core/remote-port.c         | 187 ++++++++++++++++++++++++++++++++++
> >  include/hw/core/remote-port.h |  16 +++
> >  2 files changed, 203 insertions(+)
> >
> > diff --git a/hw/core/remote-port.c b/hw/core/remote-port.c
> > index c909a825f3..5154c1bc2a 100644
> > --- a/hw/core/remote-port.c
> > +++ b/hw/core/remote-port.c
> > @@ -52,6 +52,58 @@
> >  #define REMOTE_PORT_CLASS(klass)    \
> >       OBJECT_CLASS_CHECK(RemotePortClass, (klass), TYPE_REMOTE_PORT)
> >
> > +static char *rp_sanitize_prefix(RemotePort *s)
> > +{
> > +    char *sanitized_name;
> > +    char *c;
> > +
> > +    sanitized_name = g_strdup(s->prefix);
> > +    for (c = sanitized_name; *c != '\0'; c++) {
> > +        if (*c == '/') {
> > +            *c = '_';
> > +        }
> > +    }
> > +    return sanitized_name;
> > +}
> > +
> > +static char *rp_autocreate_chardesc(RemotePort *s, bool server)
> > +{
> > +    char *prefix;
> > +    char *chardesc;
> > +    int r;
> > +
> > +    prefix = rp_sanitize_prefix(s);
> > +    r = asprintf(&chardesc, "unix:%s/qemu-rport-%s%s",
> > +                 machine_path, prefix, server ? ",wait,server" : "");
> > +    assert(r > 0);
> > +    free(prefix);
> > +    return chardesc;
> > +}
> > +
> > +static Chardev *rp_autocreate_chardev(RemotePort *s, char *name)
> > +{
> > +    Chardev *chr = NULL;
> > +    char *chardesc;
> > +    char *s_path;
> > +    int r;
> > +
> > +    r = asprintf(&s_path, "%s/qemu-rport-%s", machine_path,
> > +                 rp_sanitize_prefix(s));
> > +    assert(r > 0);
> > +    if (g_file_test(s_path, G_FILE_TEST_EXISTS)) {
> > +        chardesc = rp_autocreate_chardesc(s, false);
> > +        chr = qemu_chr_new_noreplay(name, chardesc, false, NULL);
> > +        free(chardesc);
> > +    }
> > +    free(s_path);
> > +
> > +    if (!chr) {
> > +        chardesc = rp_autocreate_chardesc(s, true);
> > +        chr = qemu_chr_new_noreplay(name, chardesc, false, NULL);
> > +        free(chardesc);
> > +    }
> > +    return chr;
> > +}
> >
> >  static void rp_reset(DeviceState *dev)
> >  {
> > @@ -66,6 +118,127 @@ static void rp_reset(DeviceState *dev)
> >
> >  static void rp_realize(DeviceState *dev, Error **errp)
> >  {
> > +    RemotePort *s = REMOTE_PORT(dev);
> > +    int r;
> > +    Error *err = NULL;
> > +
> > +    s->prefix = object_get_canonical_path(OBJECT(dev));
> > +
> > +    if (!qemu_chr_fe_get_driver(&s->chr)) {
> > +        char *name;
> > +        Chardev *chr = NULL;
> > +        static int nr;
> > +
> > +        r = asprintf(&name, "rport%d", nr);
> > +        nr++;
> > +        assert(r > 0);
> > +
> > +        if (s->chrdev_id) {
> > +            chr = qemu_chr_find(s->chrdev_id);
> > +        }
> > +
> > +        if (chr) {
> > +            /* Found the chardev via commandline */
> > +        } else if (s->chardesc) {
> > +            chr = qemu_chr_new(name, s->chardesc, NULL);
> > +        } else {
> > +            if (!machine_path) {
> > +                error_report("%s: Missing chardesc prop."
> > +                             " Forgot -machine-path?",
> > +                             s->prefix);
> > +                exit(EXIT_FAILURE);
> > +            }
> > +            chr = rp_autocreate_chardev(s, name);
> > +        }
>
> Having three different ways to configure the chardev rather
> feels like overkill. IMHO it should be sufficient to just
> accept thue chardev ID as a mandatory argument, and not
> attempt to create chardevs from this code. That would
> in turn seem to avoid the need for the -machine-path arg
> to exist ?
>

I think we can consider using only chrdev_id, for now. This loses some
flexibility, but definitely simplifies things a lot.

>
> > +#ifdef _WIN32
> > +    /*
> > +     * Create a socket connection between two sockets. We auto-bind
> > +     * and read out the port selected by the kernel.
> > +     */
> > +    {
> > +        char *name;
> > +        SocketAddress *sock;
> > +        int port;
> > +        int listen_sk;
> > +
> > +        sock = socket_parse("127.0.0.1:0", &error_abort);
> > +        listen_sk = socket_listen(sock, 1, &error_abort);
>
> Please avoid introducing new usage of low level sockets APIs. The higher
> level QIOChannelSocket APIs is preferred, that said.....
>
> > +
> > +        if (s->event.pipe.read < 0) {
> > +            perror("socket read");
> > +            exit(EXIT_FAILURE);
> > +        }
> > +
> > +        {
> > +            struct sockaddr_in saddr;
> > +            socklen_t slen = sizeof saddr;
> > +            int r;
> > +
> > +            r = getsockname(listen_sk, (struct sockaddr *) &saddr, &slen);
> > +            if (r < 0) {
> > +                perror("getsockname");
> > +                exit(EXIT_FAILURE);
> > +            }
> > +            port = htons(saddr.sin_port);
> > +        }
> > +
> > +        name = g_strdup_printf("127.0.0.1:%d", port);
> > +        s->event.pipe.write = inet_connect(name, &error_abort);
> > +        g_free(name);
> > +        if (s->event.pipe.write < 0) {
> > +            perror("socket write");
> > +            exit(EXIT_FAILURE);
> > +        }
> > +
> > +        for (;;) {
> > +            struct sockaddr_in saddr;
> > +            socklen_t slen = sizeof saddr;
> > +            int fd;
> > +
> > +            slen = sizeof(saddr);
> > +            fd = qemu_accept(listen_sk, (struct sockaddr *)&saddr, &slen);
> > +            if (fd < 0 && errno != EINTR) {
> > +                close(listen_sk);
> > +                return;
> > +            } else if (fd >= 0) {
> > +                close(listen_sk);
> > +                s->event.pipe.read = fd;
> > +                break;
> > +            }
> > +        }
> > +
> > +        if (!qemu_set_blocking(s->event.pipe.read, false, &err)) {
> > +            error_report("%s: Unable to set non-block for internal pipes",
> > +                        s->prefix);
> > +            exit(EXIT_FAILURE);
> > +        }
> > +    }
> > +#else
> > +    if (!g_unix_open_pipe(s->event.pipes, FD_CLOEXEC, NULL)) {
> > +        error_report("%s: Unable to create remort-port internal pipes",
> > +                    s->prefix);
> > +        exit(EXIT_FAILURE);
> > +    }
> > +
> > +    if (!qemu_set_blocking(s->event.pipe.read, false, &err)) {
> > +        error_report("%s: Unable to set non-block for internal pipes",
> > +                    s->prefix);
> > +        exit(EXIT_FAILURE);
> > +    }
> > +
> > +#endif
>
> ... I can't help thinking we have this code "self event pipe" design
> somewhere else in QEMU. Ideally we would have a helper API for this
> task and avoid OS conditional code in this device impl.
>
>
Thanks! Looks like EventNotifier may work here and it supports both platforms.
I'll try to use it instead then.

> >  }
> >
> >  static void rp_unrealize(DeviceState *dev)
> > @@ -73,6 +246,13 @@ static void rp_unrealize(DeviceState *dev)
> >      RemotePort *s = REMOTE_PORT(dev);
> >
> >      s->finalizing = true;
> > +
> > +    info_report("%s: Wait for remote-port to disconnect", s->prefix);
> > +    qemu_chr_fe_disconnect(&s->chr);
> > +
> > +    close(s->event.pipe.read);
> > +    close(s->event.pipe.write);
> > +    object_unparent(OBJECT(s->chrdev));
> >  }
> >
> >  static const VMStateDescription vmstate_rp = {
> > @@ -84,6 +264,12 @@ static const VMStateDescription vmstate_rp = {
> >      }
> >  };
> >
> > +static Property rp_properties[] = {
> > +    DEFINE_PROP_CHR("chardev", RemotePort, chr),
> > +    DEFINE_PROP_STRING("chardesc", RemotePort, chardesc),
> > +    DEFINE_PROP_STRING("chrdev-id", RemotePort, chrdev_id),
> > +};
>
> This really feels lik
>
> > +
> >  static void rp_prop_allow_set_link(const Object *obj, const char *name,
> >                                     Object *val, Error **errp)
> >  {
> > @@ -112,6 +298,7 @@ static void rp_class_init(ObjectClass *klass, const void *data)
> >      dc->realize = rp_realize;
> >      dc->unrealize = rp_unrealize;
> >      dc->vmsd = &vmstate_rp;
> > +    device_class_set_props_n(dc, rp_properties, ARRAY_SIZE(rp_properties));
> >  }
> >
> >  static const TypeInfo rp_info = {
> > diff --git a/include/hw/core/remote-port.h b/include/hw/core/remote-port.h
> > index db71071c8e..0f40018cdb 100644
> > --- a/include/hw/core/remote-port.h
> > +++ b/include/hw/core/remote-port.h
> > @@ -56,8 +56,24 @@ typedef struct RemotePortDeviceClass {
> >  struct RemotePort {
> >      DeviceState parent;
> >
> > +    union {
> > +       int pipes[2];
> > +       struct {
> > +           int read;
> > +           int write;
> > +       } pipe;
> > +    } event;
> > +    Chardev *chrdev;
> > +    CharFrontend chr;
> >      bool finalizing;
> >
> > +    char *chardesc;
> > +    char *chrdev_id;
> > +
> > +    const char *prefix;
> > +    const char *remote_prefix;
> > +
> > +    uint32_t current_id;
> >      bool reset_done;
> >
> >  #define REMOTE_PORT_MAX_DEVS 1024
> > --
> > 2.43.0
> >
> >
>
> With 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 :|
>

--
BR,
Ruslan