On Thu, Feb 05, 2026 at 08:58:03PM +0100, Ruslan Ruslichenko wrote:
> From: Ruslan Ruslichenko <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Takahiro Nakata <[email protected]>
> Signed-off-by: Ruslan Ruslichenko <[email protected]>
> ---
> 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 :|
