[v7,08/21] multi-process: Initialize communication channel at the remote end

Commit Message

Elena Ufimtseva June 27, 2020, 5:09 p.m. UTC

From: Elena Ufimtseva <[e-mail]>

Initialize the common QIOChannel at remote the end

Signed-off-by: Elena Ufimtseva <[e-mail]>
Signed-off-by: John G Johnson <[e-mail]>
Signed-off-by: Jagannathan Raman <[e-mail]>
---
 hw/i386/remote.c         | 16 ++++++++++++++++
 include/hw/i386/remote.h |  2 ++
 2 files changed, 18 insertions(+)

Comments

Stefan Hajnoczi July 1, 2020, 6:44 a.m. UTC | #1

On Sat, Jun 27, 2020 at 10:09:30AM -0700, [e-mail] wrote:
> @@ -42,6 +43,20 @@ static void remote_machine_init(MachineState *machine)
>      qdev_realize(DEVICE(rem_host), sysbus_get_default(), &error_fatal);
>  }
>  
> +static void remote_set_socket(Object *obj, const char *str, Error **errp)
> +{
> +    RemMachineState *s = REMOTE_MACHINE(obj);
> +    Error *local_err = NULL;
> +    int fd = atoi(str);
> +
> +    s->ioc = qio_channel_new_fd(fd, &local_err);

Missing error handling and local_err is leaked. errp should be set.

> +}
> +
> +static void remote_instance_init(Object *obj)
> +{
> +    object_property_add_str(obj, "socket", NULL, remote_set_socket);
> +}

The name "socket" does not communicate the structure of the value. Is it
a <host>:<port> pair, a UNIX domain socket path, a file descriptor, etc?
It's common to name file descriptor arguments with an "fd" suffix (e.g.
vhostfd) and that would work here too.

Please also include a help string with

  object_property_set_description(obj, property_name, help_text);

The help string when QEMU is invoked like this:

  $ qemu-system-x86_64 -M remote,\?

Patch

diff --git a/hw/i386/remote.c b/hw/i386/remote.c
index 4d13abe9f3..1a1becffe0 100644
--- a/hw/i386/remote.c
+++ b/hw/i386/remote.c
@@ -15,6 +15,7 @@ 
 #include "exec/address-spaces.h"
 #include "exec/memory.h"
 #include "qapi/error.h"
+#include "io/channel-util.h"
 
 static void remote_machine_init(MachineState *machine)
 {
@@ -42,6 +43,20 @@  static void remote_machine_init(MachineState *machine)
     qdev_realize(DEVICE(rem_host), sysbus_get_default(), &error_fatal);
 }
 
+static void remote_set_socket(Object *obj, const char *str, Error **errp)
+{
+    RemMachineState *s = REMOTE_MACHINE(obj);
+    Error *local_err = NULL;
+    int fd = atoi(str);
+
+    s->ioc = qio_channel_new_fd(fd, &local_err);
+}
+
+static void remote_instance_init(Object *obj)
+{
+    object_property_add_str(obj, "socket", NULL, remote_set_socket);
+}
+
 static void remote_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -53,6 +68,7 @@  static const TypeInfo remote_machine = {
     .name = TYPE_REMOTE_MACHINE,
     .parent = TYPE_MACHINE,
     .instance_size = sizeof(RemMachineState),
+    .instance_init = remote_instance_init,
     .class_init = remote_machine_class_init,
 };
 
diff --git a/include/hw/i386/remote.h b/include/hw/i386/remote.h
index d118a940be..0f8b861e7a 100644
--- a/include/hw/i386/remote.h
+++ b/include/hw/i386/remote.h
@@ -17,11 +17,13 @@ 
 #include "qom/object.h"
 #include "hw/boards.h"
 #include "hw/pci-host/remote.h"
+#include "io/channel.h"
 
 typedef struct RemMachineState {
     MachineState parent_obj;
 
     RemotePCIHost *host;
+    QIOChannel *ioc;
 } RemMachineState;
 
 #define TYPE_REMOTE_MACHINE "remote-machine"