[libvirt] [PATCH v3 2/5] qapi: Add pass-fd QMP command

[Luiz Capitulino]

On Thu, 14 Jun 2012 11:55:02 -0400
Corey Bryant <coreyb at linux.vnet.ibm.com> wrote:

> This patch adds the pass-fd QMP command using the QAPI framework.
> Like the getfd command, it is used to pass a file descriptor via
> SCM_RIGHTS.  However, the pass-fd command also returns the received
> file descriptor, which is a difference in behavior from the getfd
> command, which returns nothing.
> 
> The closefd command can be used to close a file descriptor that was
> passed with the pass-fd command.
> 
> Note that when using getfd or pass-fd, there are some commands
> (e.g. migrate with fd:name) that implicitly close the named fd.
> When this is not the case, closefd must be used to avoid fd leaks.
> 
> Signed-off-by: Corey Bryant <coreyb at linux.vnet.ibm.com>
> ---
> v2:
>  -Introduce new QMP command to pass/return fd (lcapitulino at redhat.com)
>  -Use passfd as command name (berrange at redhat.com)
> 
> v3:
>  -Use pass-fd as command name (lcapitulino at redhat.com)
>  -Fail pass-fd if fdname already exists (lcapitulino at redhat.com)
>  -Add notes to QMP command describing behavior in more detail
>   (lcapitulino at redhat.com, eblake at redhat.com)
>  -Add note about fd leakage (eblake at redhat.com)
> 
>  monitor.c        |   33 +++++++++++++++++++++++++++++++++
>  qapi-schema.json |   19 +++++++++++++++++++
>  qmp-commands.hx  |   34 ++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+)
> 
> diff --git a/monitor.c b/monitor.c
> index 1a7f7e7..6d99368 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2182,6 +2182,39 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>  }
>  #endif
>  
> +int64_t qmp_pass_fd(const char *fdname, Error **errp)
> +{
> +    mon_fd_t *monfd;
> +    int fd;
> +
> +    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
> +    if (fd == -1) {
> +        error_set(errp, QERR_FD_NOT_SUPPLIED);
> +        return -1;
> +    }
> +
> +    if (qemu_isdigit(fdname[0])) {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> +                  "a name not starting with a digit");
> +        return -1;
> +    }
> +
> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> +        if (strcmp(monfd->name, fdname) == 0) {
> +            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> +                      "a name that does not already exist");
> +            return -1;
> +        }
> +    }

Returning the same error class for two different errors is not a good idea.
I think you have two options here. You could return QERR_INVALID_PARAMETER
for the "already exists" case or introduce QERR_FD_EXISTS. The later is
certainly nicer, but we were trying to avoid having too specific errors...

> +
> +    monfd = g_malloc0(sizeof(mon_fd_t));
> +    monfd->name = g_strdup(fdname);
> +    monfd->fd = fd;

Maybe you could try to move this to a separate function to share code with
qmp_getfd()?

> +
> +    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
> +    return fd;
> +}
> +
>  void qmp_getfd(const char *fdname, Error **errp)
>  {
>      mon_fd_t *monfd;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 26a6b84..ed99f23 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1864,6 +1864,25 @@
>  { 'command': 'netdev_del', 'data': {'id': 'str'} }
>  
>  ##
> +# @pass-fd:
> +#
> +# Pass a file descriptor via SCM rights and assign it a name
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: The QEMU file descriptor that was received
> +#          If file descriptor was not received, FdNotSupplied
> +#          If @fdname is not valid, InvalidParameterType
> +#
> +# Since: 1.2.0
> +#
> +# Notes: If @fdname already exists, the command will fail.
> +#        The 'closefd' command can be used to explicitly close the
> +#        file descriptor when it is no longer needed.
> +##
> +{ 'command': 'pass-fd', 'data': {'fdname': 'str'}, 'returns': 'int' }
> +
> +##
>  # @getfd:
>  #
>  # Receive a file descriptor via SCM rights and assign it a name
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index e3cf3c5..c039947 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -869,6 +869,40 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "pass-fd",
> +        .args_type  = "fdname:s",
> +        .params     = "pass-fd name",
> +        .help       = "pass a file descriptor via SCM rights and assign it a name",
> +        .mhandler.cmd_new = qmp_marshal_input_pass_fd,
> +    },
> +
> +SQMP
> +pass-fd
> +-------
> +
> +Pass a file descriptor via SCM rights and assign it a name.
> +
> +Arguments:
> +
> +- "fdname": file descriptor name (json-string)
> +
> +Return a json-int with the QEMU file descriptor that was received.
> +
> +Example:
> +
> +-> { "execute": "pass-fd", "arguments": { "fdname": "fd1" } }
> +<- { "return": 42 }
> +
> +Notes:
> +
> +(1) If the name specified by the "fdname" argument already exists,
> +    the command will fail.
> +(2) The 'closefd' command can be used to explicitly close the file
> +    descriptor when it is no longer needed.
> +
> +EQMP
> +
> +    {
>          .name       = "getfd",
>          .args_type  = "fdname:s",
>          .params     = "getfd name",