10:04 a.m.
On 06/15/2012 10:32 AM, Luiz Capitulino wrote:
On Thu, 14 Jun 2012 11:55:02 -0400
Corey Bryant <coreyb(a)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(a)linux.vnet.ibm.com>
> ---
> v2:
> -Introduce new QMP command to pass/return fd (lcapitulino(a)redhat.com)
> -Use passfd as command name (berrange(a)redhat.com)
>
> v3:
> -Use pass-fd as command name (lcapitulino(a)redhat.com)
> -Fail pass-fd if fdname already exists (lcapitulino(a)redhat.com)
> -Add notes to QMP command describing behavior in more detail
> (lcapitulino(a)redhat.com, eblake(a)redhat.com)
> -Add note about fd leakage (eblake(a)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...
I'm not clear on what the problem is with returning the same error class
for two different errors. Could you explain? I don't have a problem
changing it if it's an issue.
> +
> + 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()?
Sure, no problem. I can do that.
> +
> + 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",
--
Regards,
Corey