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

[Stefan Hajnoczi]

On Sat, Jun 27, 2020 at 10:09:30AM -0700, elena.ufimtseva@oracle.com 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,\?

signature.asc
Description: PGP signature