10:14 a.m.
On Fri, 15 Jun 2012 11:04:16 -0400
Corey Bryant <coreyb(a)linux.vnet.ibm.com> wrote:
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.
Because an mngt app/user won't know if what has happened was that the fd name
is invalid or if the fd name already exists.
>> +
>> + 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",
>
>