diff mbox

[v4,2/7] qapi: Convert getfd and closefd

Message ID 1340390174-7493-3-git-send-email-coreyb@linux.vnet.ibm.com
State New
Headers show

Commit Message

Corey Bryant June 22, 2012, 6:36 p.m. UTC
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
v2:
 -Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
 -Remove changes that returned fd from getfd (lcapitulino@redhat.com)
 -Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
 -Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
 -Drop .user_print lines (lcapitulino@redhat.com)
 -Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
 -Change QMP command existance back to 0.14 (lcapitulino@redhat.com)

v3:
 -Remove unnecessary return statements from qmp_* functions
  (lcapitulino@redhat.com)
 -Add notes to QMP command describing behavior in more detail
  (lcapitulino@redhat.com, eblake@redhat.com)

v4:
 -No changes

 hmp-commands.hx  |    6 ++----
 hmp.c            |   18 ++++++++++++++++++
 hmp.h            |    2 ++
 monitor.c        |   32 ++++++++++++++------------------
 qapi-schema.json |   35 +++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   14 ++++++++++----
 6 files changed, 81 insertions(+), 26 deletions(-)

Comments

Luiz Capitulino June 26, 2012, 8:44 p.m. UTC | #1
On Fri, 22 Jun 2012 14:36:09 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:

> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>

If this patch doesn't change anymore you can add:

 Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
> v2:
>  -Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
>  -Remove changes that returned fd from getfd (lcapitulino@redhat.com)
>  -Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
>  -Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
>  -Drop .user_print lines (lcapitulino@redhat.com)
>  -Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
>  -Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
> 
> v3:
>  -Remove unnecessary return statements from qmp_* functions
>   (lcapitulino@redhat.com)
>  -Add notes to QMP command describing behavior in more detail
>   (lcapitulino@redhat.com, eblake@redhat.com)
> 
> v4:
>  -No changes
> 
>  hmp-commands.hx  |    6 ++----
>  hmp.c            |   18 ++++++++++++++++++
>  hmp.h            |    2 ++
>  monitor.c        |   32 ++++++++++++++------------------
>  qapi-schema.json |   35 +++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   14 ++++++++++----
>  6 files changed, 81 insertions(+), 26 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f5d9d91..eea8b32 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1236,8 +1236,7 @@ ETEXI
>          .args_type  = "fdname:s",
>          .params     = "getfd name",
>          .help       = "receive a file descriptor via SCM rights and assign it a name",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_getfd,
> +        .mhandler.cmd = hmp_getfd,
>      },
>  
>  STEXI
> @@ -1253,8 +1252,7 @@ ETEXI
>          .args_type  = "fdname:s",
>          .params     = "closefd name",
>          .help       = "close a file descriptor previously passed via SCM rights",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_closefd,
> +        .mhandler.cmd = hmp_closefd,
>      },
>  
>  STEXI
> diff --git a/hmp.c b/hmp.c
> index 2ce8cb9..6241856 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>      qmp_netdev_del(id, &err);
>      hmp_handle_error(mon, &err);
>  }
> +
> +void hmp_getfd(Monitor *mon, const QDict *qdict)
> +{
> +    const char *fdname = qdict_get_str(qdict, "fdname");
> +    Error *errp = NULL;
> +
> +    qmp_getfd(fdname, &errp);
> +    hmp_handle_error(mon, &errp);
> +}
> +
> +void hmp_closefd(Monitor *mon, const QDict *qdict)
> +{
> +    const char *fdname = qdict_get_str(qdict, "fdname");
> +    Error *errp = NULL;
> +
> +    qmp_closefd(fdname, &errp);
> +    hmp_handle_error(mon, &errp);
> +}
> diff --git a/hmp.h b/hmp.h
> index 79d138d..8d2b0d7 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
>  void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_getfd(Monitor *mon, const QDict *qdict);
> +void hmp_closefd(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/monitor.c b/monitor.c
> index a3bc2c7..1a7f7e7 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2182,48 +2182,45 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>  }
>  #endif
>  
> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +void qmp_getfd(const char *fdname, Error **errp)
>  {
> -    const char *fdname = qdict_get_str(qdict, "fdname");
>      mon_fd_t *monfd;
>      int fd;
>  
> -    fd = qemu_chr_fe_get_msgfd(mon->chr);
> +    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
>      if (fd == -1) {
> -        qerror_report(QERR_FD_NOT_SUPPLIED);
> -        return -1;
> +        error_set(errp, QERR_FD_NOT_SUPPLIED);
> +        return;
>      }
>  
>      if (qemu_isdigit(fdname[0])) {
> -        qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
> -                      "a name not starting with a digit");
> -        return -1;
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> +                  "a name not starting with a digit");
> +        return;
>      }
>  
> -    QLIST_FOREACH(monfd, &mon->fds, next) {
> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>          if (strcmp(monfd->name, fdname) != 0) {
>              continue;
>          }
>  
>          close(monfd->fd);
>          monfd->fd = fd;
> -        return 0;
> +        return;
>      }
>  
>      monfd = g_malloc0(sizeof(mon_fd_t));
>      monfd->name = g_strdup(fdname);
>      monfd->fd = fd;
>  
> -    QLIST_INSERT_HEAD(&mon->fds, monfd, next);
> -    return 0;
> +    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
>  }
>  
> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +void qmp_closefd(const char *fdname, Error **errp)
>  {
> -    const char *fdname = qdict_get_str(qdict, "fdname");
>      mon_fd_t *monfd;
>  
> -    QLIST_FOREACH(monfd, &mon->fds, next) {
> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>          if (strcmp(monfd->name, fdname) != 0) {
>              continue;
>          }
> @@ -2232,11 +2229,10 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>          close(monfd->fd);
>          g_free(monfd->name);
>          g_free(monfd);
> -        return 0;
> +        return;
>      }
>  
> -    qerror_report(QERR_FD_NOT_FOUND, fdname);
> -    return -1;
> +    error_set(errp, QERR_FD_NOT_FOUND, fdname);
>  }
>  
>  static void do_loadvm(Monitor *mon, const QDict *qdict)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 3b6e346..26a6b84 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1862,3 +1862,38 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'netdev_del', 'data': {'id': 'str'} }
> +
> +##
> +# @getfd:
> +#
> +# Receive a file descriptor via SCM rights and assign it a name
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: Nothing on success
> +#          If file descriptor was not received, FdNotSupplied
> +#          If @fdname is not valid, InvalidParameterType
> +#
> +# Since: 0.14.0
> +#
> +# Notes: If @fdname already exists, the file descriptor assigned to
> +#        it will be closed and replaced by the received file
> +#        descriptor.
> +#        The 'closefd' command can be used to explicitly close the
> +#        file descriptor when it is no longer needed.
> +##
> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
> +
> +##
> +# @closefd:
> +#
> +# Close a file descriptor previously passed via SCM rights
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: Nothing on success
> +#          If @fdname is not found, FdNotFound
> +#
> +# Since: 0.14.0
> +##
> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2e1a38e..e3cf3c5 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -873,8 +873,7 @@ EQMP
>          .args_type  = "fdname:s",
>          .params     = "getfd name",
>          .help       = "receive a file descriptor via SCM rights and assign it a name",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_getfd,
> +        .mhandler.cmd_new = qmp_marshal_input_getfd,
>      },
>  
>  SQMP
> @@ -892,6 +891,14 @@ Example:
>  -> { "execute": "getfd", "arguments": { "fdname": "fd1" } }
>  <- { "return": {} }
>  
> +Notes:
> +
> +(1) If the name specified by the "fdname" argument already exists,
> +    the file descriptor assigned to it will be closed and replaced
> +    by the received file descriptor.
> +(2) The 'closefd' command can be used to explicitly close the file
> +    descriptor when it is no longer needed.
> +
>  EQMP
>  
>      {
> @@ -899,8 +906,7 @@ EQMP
>          .args_type  = "fdname:s",
>          .params     = "closefd name",
>          .help       = "close a file descriptor previously passed via SCM rights",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_closefd,
> +        .mhandler.cmd_new = qmp_marshal_input_closefd,
>      },
>  
>  SQMP
Corey Bryant June 26, 2012, 9:15 p.m. UTC | #2
On 06/26/2012 04:44 PM, Luiz Capitulino wrote:
> On Fri, 22 Jun 2012 14:36:09 -0400
> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>
> If this patch doesn't change anymore you can add:
>
>   Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
>

Thanks!  I'll do that.  I'll assume we still want this patch even if 
pass-fd support is dropped.

>> ---
>> v2:
>>   -Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
>>   -Remove changes that returned fd from getfd (lcapitulino@redhat.com)
>>   -Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
>>   -Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
>>   -Drop .user_print lines (lcapitulino@redhat.com)
>>   -Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
>>   -Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
>>
>> v3:
>>   -Remove unnecessary return statements from qmp_* functions
>>    (lcapitulino@redhat.com)
>>   -Add notes to QMP command describing behavior in more detail
>>    (lcapitulino@redhat.com, eblake@redhat.com)
>>
>> v4:
>>   -No changes
>>
>>   hmp-commands.hx  |    6 ++----
>>   hmp.c            |   18 ++++++++++++++++++
>>   hmp.h            |    2 ++
>>   monitor.c        |   32 ++++++++++++++------------------
>>   qapi-schema.json |   35 +++++++++++++++++++++++++++++++++++
>>   qmp-commands.hx  |   14 ++++++++++----
>>   6 files changed, 81 insertions(+), 26 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index f5d9d91..eea8b32 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1236,8 +1236,7 @@ ETEXI
>>           .args_type  = "fdname:s",
>>           .params     = "getfd name",
>>           .help       = "receive a file descriptor via SCM rights and assign it a name",
>> -        .user_print = monitor_user_noop,
>> -        .mhandler.cmd_new = do_getfd,
>> +        .mhandler.cmd = hmp_getfd,
>>       },
>>
>>   STEXI
>> @@ -1253,8 +1252,7 @@ ETEXI
>>           .args_type  = "fdname:s",
>>           .params     = "closefd name",
>>           .help       = "close a file descriptor previously passed via SCM rights",
>> -        .user_print = monitor_user_noop,
>> -        .mhandler.cmd_new = do_closefd,
>> +        .mhandler.cmd = hmp_closefd,
>>       },
>>
>>   STEXI
>> diff --git a/hmp.c b/hmp.c
>> index 2ce8cb9..6241856 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>>       qmp_netdev_del(id, &err);
>>       hmp_handle_error(mon, &err);
>>   }
>> +
>> +void hmp_getfd(Monitor *mon, const QDict *qdict)
>> +{
>> +    const char *fdname = qdict_get_str(qdict, "fdname");
>> +    Error *errp = NULL;
>> +
>> +    qmp_getfd(fdname, &errp);
>> +    hmp_handle_error(mon, &errp);
>> +}
>> +
>> +void hmp_closefd(Monitor *mon, const QDict *qdict)
>> +{
>> +    const char *fdname = qdict_get_str(qdict, "fdname");
>> +    Error *errp = NULL;
>> +
>> +    qmp_closefd(fdname, &errp);
>> +    hmp_handle_error(mon, &errp);
>> +}
>> diff --git a/hmp.h b/hmp.h
>> index 79d138d..8d2b0d7 100644
>> --- a/hmp.h
>> +++ b/hmp.h
>> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
>>   void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>>   void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>>   void hmp_netdev_del(Monitor *mon, const QDict *qdict);
>> +void hmp_getfd(Monitor *mon, const QDict *qdict);
>> +void hmp_closefd(Monitor *mon, const QDict *qdict);
>>
>>   #endif
>> diff --git a/monitor.c b/monitor.c
>> index a3bc2c7..1a7f7e7 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -2182,48 +2182,45 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>>   }
>>   #endif
>>
>> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> +void qmp_getfd(const char *fdname, Error **errp)
>>   {
>> -    const char *fdname = qdict_get_str(qdict, "fdname");
>>       mon_fd_t *monfd;
>>       int fd;
>>
>> -    fd = qemu_chr_fe_get_msgfd(mon->chr);
>> +    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
>>       if (fd == -1) {
>> -        qerror_report(QERR_FD_NOT_SUPPLIED);
>> -        return -1;
>> +        error_set(errp, QERR_FD_NOT_SUPPLIED);
>> +        return;
>>       }
>>
>>       if (qemu_isdigit(fdname[0])) {
>> -        qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
>> -                      "a name not starting with a digit");
>> -        return -1;
>> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
>> +                  "a name not starting with a digit");
>> +        return;
>>       }
>>
>> -    QLIST_FOREACH(monfd, &mon->fds, next) {
>> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>>           if (strcmp(monfd->name, fdname) != 0) {
>>               continue;
>>           }
>>
>>           close(monfd->fd);
>>           monfd->fd = fd;
>> -        return 0;
>> +        return;
>>       }
>>
>>       monfd = g_malloc0(sizeof(mon_fd_t));
>>       monfd->name = g_strdup(fdname);
>>       monfd->fd = fd;
>>
>> -    QLIST_INSERT_HEAD(&mon->fds, monfd, next);
>> -    return 0;
>> +    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
>>   }
>>
>> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> +void qmp_closefd(const char *fdname, Error **errp)
>>   {
>> -    const char *fdname = qdict_get_str(qdict, "fdname");
>>       mon_fd_t *monfd;
>>
>> -    QLIST_FOREACH(monfd, &mon->fds, next) {
>> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>>           if (strcmp(monfd->name, fdname) != 0) {
>>               continue;
>>           }
>> @@ -2232,11 +2229,10 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>           close(monfd->fd);
>>           g_free(monfd->name);
>>           g_free(monfd);
>> -        return 0;
>> +        return;
>>       }
>>
>> -    qerror_report(QERR_FD_NOT_FOUND, fdname);
>> -    return -1;
>> +    error_set(errp, QERR_FD_NOT_FOUND, fdname);
>>   }
>>
>>   static void do_loadvm(Monitor *mon, const QDict *qdict)
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 3b6e346..26a6b84 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1862,3 +1862,38 @@
>>   # Since: 0.14.0
>>   ##
>>   { 'command': 'netdev_del', 'data': {'id': 'str'} }
>> +
>> +##
>> +# @getfd:
>> +#
>> +# Receive a file descriptor via SCM rights and assign it a name
>> +#
>> +# @fdname: file descriptor name
>> +#
>> +# Returns: Nothing on success
>> +#          If file descriptor was not received, FdNotSupplied
>> +#          If @fdname is not valid, InvalidParameterType
>> +#
>> +# Since: 0.14.0
>> +#
>> +# Notes: If @fdname already exists, the file descriptor assigned to
>> +#        it will be closed and replaced by the received file
>> +#        descriptor.
>> +#        The 'closefd' command can be used to explicitly close the
>> +#        file descriptor when it is no longer needed.
>> +##
>> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
>> +
>> +##
>> +# @closefd:
>> +#
>> +# Close a file descriptor previously passed via SCM rights
>> +#
>> +# @fdname: file descriptor name
>> +#
>> +# Returns: Nothing on success
>> +#          If @fdname is not found, FdNotFound
>> +#
>> +# Since: 0.14.0
>> +##
>> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 2e1a38e..e3cf3c5 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -873,8 +873,7 @@ EQMP
>>           .args_type  = "fdname:s",
>>           .params     = "getfd name",
>>           .help       = "receive a file descriptor via SCM rights and assign it a name",
>> -        .user_print = monitor_user_noop,
>> -        .mhandler.cmd_new = do_getfd,
>> +        .mhandler.cmd_new = qmp_marshal_input_getfd,
>>       },
>>
>>   SQMP
>> @@ -892,6 +891,14 @@ Example:
>>   -> { "execute": "getfd", "arguments": { "fdname": "fd1" } }
>>   <- { "return": {} }
>>
>> +Notes:
>> +
>> +(1) If the name specified by the "fdname" argument already exists,
>> +    the file descriptor assigned to it will be closed and replaced
>> +    by the received file descriptor.
>> +(2) The 'closefd' command can be used to explicitly close the file
>> +    descriptor when it is no longer needed.
>> +
>>   EQMP
>>
>>       {
>> @@ -899,8 +906,7 @@ EQMP
>>           .args_type  = "fdname:s",
>>           .params     = "closefd name",
>>           .help       = "close a file descriptor previously passed via SCM rights",
>> -        .user_print = monitor_user_noop,
>> -        .mhandler.cmd_new = do_closefd,
>> +        .mhandler.cmd_new = qmp_marshal_input_closefd,
>>       },
>>
>>   SQMP
>
Luiz Capitulino June 26, 2012, 9:28 p.m. UTC | #3
On Tue, 26 Jun 2012 17:15:05 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:

> 
> 
> On 06/26/2012 04:44 PM, Luiz Capitulino wrote:
> > On Fri, 22 Jun 2012 14:36:09 -0400
> > Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
> >
> >> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> >
> > If this patch doesn't change anymore you can add:
> >
> >   Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
> >
> 
> Thanks!  I'll do that.  I'll assume we still want this patch even if 
> pass-fd support is dropped.

Yes. Actually, I could cherry pick it but I believe it will cause conflicts
to you if you don't include it?

> 
> >> ---
> >> v2:
> >>   -Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
> >>   -Remove changes that returned fd from getfd (lcapitulino@redhat.com)
> >>   -Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
> >>   -Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
> >>   -Drop .user_print lines (lcapitulino@redhat.com)
> >>   -Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
> >>   -Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
> >>
> >> v3:
> >>   -Remove unnecessary return statements from qmp_* functions
> >>    (lcapitulino@redhat.com)
> >>   -Add notes to QMP command describing behavior in more detail
> >>    (lcapitulino@redhat.com, eblake@redhat.com)
> >>
> >> v4:
> >>   -No changes
> >>
> >>   hmp-commands.hx  |    6 ++----
> >>   hmp.c            |   18 ++++++++++++++++++
> >>   hmp.h            |    2 ++
> >>   monitor.c        |   32 ++++++++++++++------------------
> >>   qapi-schema.json |   35 +++++++++++++++++++++++++++++++++++
> >>   qmp-commands.hx  |   14 ++++++++++----
> >>   6 files changed, 81 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index f5d9d91..eea8b32 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -1236,8 +1236,7 @@ ETEXI
> >>           .args_type  = "fdname:s",
> >>           .params     = "getfd name",
> >>           .help       = "receive a file descriptor via SCM rights and assign it a name",
> >> -        .user_print = monitor_user_noop,
> >> -        .mhandler.cmd_new = do_getfd,
> >> +        .mhandler.cmd = hmp_getfd,
> >>       },
> >>
> >>   STEXI
> >> @@ -1253,8 +1252,7 @@ ETEXI
> >>           .args_type  = "fdname:s",
> >>           .params     = "closefd name",
> >>           .help       = "close a file descriptor previously passed via SCM rights",
> >> -        .user_print = monitor_user_noop,
> >> -        .mhandler.cmd_new = do_closefd,
> >> +        .mhandler.cmd = hmp_closefd,
> >>       },
> >>
> >>   STEXI
> >> diff --git a/hmp.c b/hmp.c
> >> index 2ce8cb9..6241856 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
> >>       qmp_netdev_del(id, &err);
> >>       hmp_handle_error(mon, &err);
> >>   }
> >> +
> >> +void hmp_getfd(Monitor *mon, const QDict *qdict)
> >> +{
> >> +    const char *fdname = qdict_get_str(qdict, "fdname");
> >> +    Error *errp = NULL;
> >> +
> >> +    qmp_getfd(fdname, &errp);
> >> +    hmp_handle_error(mon, &errp);
> >> +}
> >> +
> >> +void hmp_closefd(Monitor *mon, const QDict *qdict)
> >> +{
> >> +    const char *fdname = qdict_get_str(qdict, "fdname");
> >> +    Error *errp = NULL;
> >> +
> >> +    qmp_closefd(fdname, &errp);
> >> +    hmp_handle_error(mon, &errp);
> >> +}
> >> diff --git a/hmp.h b/hmp.h
> >> index 79d138d..8d2b0d7 100644
> >> --- a/hmp.h
> >> +++ b/hmp.h
> >> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
> >>   void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
> >>   void hmp_netdev_add(Monitor *mon, const QDict *qdict);
> >>   void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> >> +void hmp_getfd(Monitor *mon, const QDict *qdict);
> >> +void hmp_closefd(Monitor *mon, const QDict *qdict);
> >>
> >>   #endif
> >> diff --git a/monitor.c b/monitor.c
> >> index a3bc2c7..1a7f7e7 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -2182,48 +2182,45 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
> >>   }
> >>   #endif
> >>
> >> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >> +void qmp_getfd(const char *fdname, Error **errp)
> >>   {
> >> -    const char *fdname = qdict_get_str(qdict, "fdname");
> >>       mon_fd_t *monfd;
> >>       int fd;
> >>
> >> -    fd = qemu_chr_fe_get_msgfd(mon->chr);
> >> +    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
> >>       if (fd == -1) {
> >> -        qerror_report(QERR_FD_NOT_SUPPLIED);
> >> -        return -1;
> >> +        error_set(errp, QERR_FD_NOT_SUPPLIED);
> >> +        return;
> >>       }
> >>
> >>       if (qemu_isdigit(fdname[0])) {
> >> -        qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
> >> -                      "a name not starting with a digit");
> >> -        return -1;
> >> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> >> +                  "a name not starting with a digit");
> >> +        return;
> >>       }
> >>
> >> -    QLIST_FOREACH(monfd, &mon->fds, next) {
> >> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> >>           if (strcmp(monfd->name, fdname) != 0) {
> >>               continue;
> >>           }
> >>
> >>           close(monfd->fd);
> >>           monfd->fd = fd;
> >> -        return 0;
> >> +        return;
> >>       }
> >>
> >>       monfd = g_malloc0(sizeof(mon_fd_t));
> >>       monfd->name = g_strdup(fdname);
> >>       monfd->fd = fd;
> >>
> >> -    QLIST_INSERT_HEAD(&mon->fds, monfd, next);
> >> -    return 0;
> >> +    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
> >>   }
> >>
> >> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >> +void qmp_closefd(const char *fdname, Error **errp)
> >>   {
> >> -    const char *fdname = qdict_get_str(qdict, "fdname");
> >>       mon_fd_t *monfd;
> >>
> >> -    QLIST_FOREACH(monfd, &mon->fds, next) {
> >> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
> >>           if (strcmp(monfd->name, fdname) != 0) {
> >>               continue;
> >>           }
> >> @@ -2232,11 +2229,10 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >>           close(monfd->fd);
> >>           g_free(monfd->name);
> >>           g_free(monfd);
> >> -        return 0;
> >> +        return;
> >>       }
> >>
> >> -    qerror_report(QERR_FD_NOT_FOUND, fdname);
> >> -    return -1;
> >> +    error_set(errp, QERR_FD_NOT_FOUND, fdname);
> >>   }
> >>
> >>   static void do_loadvm(Monitor *mon, const QDict *qdict)
> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> index 3b6e346..26a6b84 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -1862,3 +1862,38 @@
> >>   # Since: 0.14.0
> >>   ##
> >>   { 'command': 'netdev_del', 'data': {'id': 'str'} }
> >> +
> >> +##
> >> +# @getfd:
> >> +#
> >> +# Receive a file descriptor via SCM rights and assign it a name
> >> +#
> >> +# @fdname: file descriptor name
> >> +#
> >> +# Returns: Nothing on success
> >> +#          If file descriptor was not received, FdNotSupplied
> >> +#          If @fdname is not valid, InvalidParameterType
> >> +#
> >> +# Since: 0.14.0
> >> +#
> >> +# Notes: If @fdname already exists, the file descriptor assigned to
> >> +#        it will be closed and replaced by the received file
> >> +#        descriptor.
> >> +#        The 'closefd' command can be used to explicitly close the
> >> +#        file descriptor when it is no longer needed.
> >> +##
> >> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
> >> +
> >> +##
> >> +# @closefd:
> >> +#
> >> +# Close a file descriptor previously passed via SCM rights
> >> +#
> >> +# @fdname: file descriptor name
> >> +#
> >> +# Returns: Nothing on success
> >> +#          If @fdname is not found, FdNotFound
> >> +#
> >> +# Since: 0.14.0
> >> +##
> >> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index 2e1a38e..e3cf3c5 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -873,8 +873,7 @@ EQMP
> >>           .args_type  = "fdname:s",
> >>           .params     = "getfd name",
> >>           .help       = "receive a file descriptor via SCM rights and assign it a name",
> >> -        .user_print = monitor_user_noop,
> >> -        .mhandler.cmd_new = do_getfd,
> >> +        .mhandler.cmd_new = qmp_marshal_input_getfd,
> >>       },
> >>
> >>   SQMP
> >> @@ -892,6 +891,14 @@ Example:
> >>   -> { "execute": "getfd", "arguments": { "fdname": "fd1" } }
> >>   <- { "return": {} }
> >>
> >> +Notes:
> >> +
> >> +(1) If the name specified by the "fdname" argument already exists,
> >> +    the file descriptor assigned to it will be closed and replaced
> >> +    by the received file descriptor.
> >> +(2) The 'closefd' command can be used to explicitly close the file
> >> +    descriptor when it is no longer needed.
> >> +
> >>   EQMP
> >>
> >>       {
> >> @@ -899,8 +906,7 @@ EQMP
> >>           .args_type  = "fdname:s",
> >>           .params     = "closefd name",
> >>           .help       = "close a file descriptor previously passed via SCM rights",
> >> -        .user_print = monitor_user_noop,
> >> -        .mhandler.cmd_new = do_closefd,
> >> +        .mhandler.cmd_new = qmp_marshal_input_closefd,
> >>       },
> >>
> >>   SQMP
> >
>
Corey Bryant June 26, 2012, 10:05 p.m. UTC | #4
On 06/26/2012 05:28 PM, Luiz Capitulino wrote:
> On Tue, 26 Jun 2012 17:15:05 -0400
> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>>
>>
>> On 06/26/2012 04:44 PM, Luiz Capitulino wrote:
>>> On Fri, 22 Jun 2012 14:36:09 -0400
>>> Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>>>
>>>> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
>>>
>>> If this patch doesn't change anymore you can add:
>>>
>>>    Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
>>>
>>
>> Thanks!  I'll do that.  I'll assume we still want this patch even if
>> pass-fd support is dropped.
>
> Yes. Actually, I could cherry pick it but I believe it will cause conflicts
> to you if you don't include it?
>

It might just be easier if I keep it in my series for now, at least 
until we figure out what direction we're going to take regarding pass-fd 
vs passing fd's via SCM_RIGHTS with commands like drive_add, etc.

>>
>>>> ---
>>>> v2:
>>>>    -Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
>>>>    -Remove changes that returned fd from getfd (lcapitulino@redhat.com)
>>>>    -Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
>>>>    -Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
>>>>    -Drop .user_print lines (lcapitulino@redhat.com)
>>>>    -Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
>>>>    -Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
>>>>
>>>> v3:
>>>>    -Remove unnecessary return statements from qmp_* functions
>>>>     (lcapitulino@redhat.com)
>>>>    -Add notes to QMP command describing behavior in more detail
>>>>     (lcapitulino@redhat.com, eblake@redhat.com)
>>>>
>>>> v4:
>>>>    -No changes
>>>>
>>>>    hmp-commands.hx  |    6 ++----
>>>>    hmp.c            |   18 ++++++++++++++++++
>>>>    hmp.h            |    2 ++
>>>>    monitor.c        |   32 ++++++++++++++------------------
>>>>    qapi-schema.json |   35 +++++++++++++++++++++++++++++++++++
>>>>    qmp-commands.hx  |   14 ++++++++++----
>>>>    6 files changed, 81 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>>> index f5d9d91..eea8b32 100644
>>>> --- a/hmp-commands.hx
>>>> +++ b/hmp-commands.hx
>>>> @@ -1236,8 +1236,7 @@ ETEXI
>>>>            .args_type  = "fdname:s",
>>>>            .params     = "getfd name",
>>>>            .help       = "receive a file descriptor via SCM rights and assign it a name",
>>>> -        .user_print = monitor_user_noop,
>>>> -        .mhandler.cmd_new = do_getfd,
>>>> +        .mhandler.cmd = hmp_getfd,
>>>>        },
>>>>
>>>>    STEXI
>>>> @@ -1253,8 +1252,7 @@ ETEXI
>>>>            .args_type  = "fdname:s",
>>>>            .params     = "closefd name",
>>>>            .help       = "close a file descriptor previously passed via SCM rights",
>>>> -        .user_print = monitor_user_noop,
>>>> -        .mhandler.cmd_new = do_closefd,
>>>> +        .mhandler.cmd = hmp_closefd,
>>>>        },
>>>>
>>>>    STEXI
>>>> diff --git a/hmp.c b/hmp.c
>>>> index 2ce8cb9..6241856 100644
>>>> --- a/hmp.c
>>>> +++ b/hmp.c
>>>> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>>>>        qmp_netdev_del(id, &err);
>>>>        hmp_handle_error(mon, &err);
>>>>    }
>>>> +
>>>> +void hmp_getfd(Monitor *mon, const QDict *qdict)
>>>> +{
>>>> +    const char *fdname = qdict_get_str(qdict, "fdname");
>>>> +    Error *errp = NULL;
>>>> +
>>>> +    qmp_getfd(fdname, &errp);
>>>> +    hmp_handle_error(mon, &errp);
>>>> +}
>>>> +
>>>> +void hmp_closefd(Monitor *mon, const QDict *qdict)
>>>> +{
>>>> +    const char *fdname = qdict_get_str(qdict, "fdname");
>>>> +    Error *errp = NULL;
>>>> +
>>>> +    qmp_closefd(fdname, &errp);
>>>> +    hmp_handle_error(mon, &errp);
>>>> +}
>>>> diff --git a/hmp.h b/hmp.h
>>>> index 79d138d..8d2b0d7 100644
>>>> --- a/hmp.h
>>>> +++ b/hmp.h
>>>> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
>>>>    void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>>>>    void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>>>>    void hmp_netdev_del(Monitor *mon, const QDict *qdict);
>>>> +void hmp_getfd(Monitor *mon, const QDict *qdict);
>>>> +void hmp_closefd(Monitor *mon, const QDict *qdict);
>>>>
>>>>    #endif
>>>> diff --git a/monitor.c b/monitor.c
>>>> index a3bc2c7..1a7f7e7 100644
>>>> --- a/monitor.c
>>>> +++ b/monitor.c
>>>> @@ -2182,48 +2182,45 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>>>>    }
>>>>    #endif
>>>>
>>>> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>>> +void qmp_getfd(const char *fdname, Error **errp)
>>>>    {
>>>> -    const char *fdname = qdict_get_str(qdict, "fdname");
>>>>        mon_fd_t *monfd;
>>>>        int fd;
>>>>
>>>> -    fd = qemu_chr_fe_get_msgfd(mon->chr);
>>>> +    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
>>>>        if (fd == -1) {
>>>> -        qerror_report(QERR_FD_NOT_SUPPLIED);
>>>> -        return -1;
>>>> +        error_set(errp, QERR_FD_NOT_SUPPLIED);
>>>> +        return;
>>>>        }
>>>>
>>>>        if (qemu_isdigit(fdname[0])) {
>>>> -        qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
>>>> -                      "a name not starting with a digit");
>>>> -        return -1;
>>>> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
>>>> +                  "a name not starting with a digit");
>>>> +        return;
>>>>        }
>>>>
>>>> -    QLIST_FOREACH(monfd, &mon->fds, next) {
>>>> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>>>>            if (strcmp(monfd->name, fdname) != 0) {
>>>>                continue;
>>>>            }
>>>>
>>>>            close(monfd->fd);
>>>>            monfd->fd = fd;
>>>> -        return 0;
>>>> +        return;
>>>>        }
>>>>
>>>>        monfd = g_malloc0(sizeof(mon_fd_t));
>>>>        monfd->name = g_strdup(fdname);
>>>>        monfd->fd = fd;
>>>>
>>>> -    QLIST_INSERT_HEAD(&mon->fds, monfd, next);
>>>> -    return 0;
>>>> +    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
>>>>    }
>>>>
>>>> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>>> +void qmp_closefd(const char *fdname, Error **errp)
>>>>    {
>>>> -    const char *fdname = qdict_get_str(qdict, "fdname");
>>>>        mon_fd_t *monfd;
>>>>
>>>> -    QLIST_FOREACH(monfd, &mon->fds, next) {
>>>> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>>>>            if (strcmp(monfd->name, fdname) != 0) {
>>>>                continue;
>>>>            }
>>>> @@ -2232,11 +2229,10 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>>>            close(monfd->fd);
>>>>            g_free(monfd->name);
>>>>            g_free(monfd);
>>>> -        return 0;
>>>> +        return;
>>>>        }
>>>>
>>>> -    qerror_report(QERR_FD_NOT_FOUND, fdname);
>>>> -    return -1;
>>>> +    error_set(errp, QERR_FD_NOT_FOUND, fdname);
>>>>    }
>>>>
>>>>    static void do_loadvm(Monitor *mon, const QDict *qdict)
>>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>>> index 3b6e346..26a6b84 100644
>>>> --- a/qapi-schema.json
>>>> +++ b/qapi-schema.json
>>>> @@ -1862,3 +1862,38 @@
>>>>    # Since: 0.14.0
>>>>    ##
>>>>    { 'command': 'netdev_del', 'data': {'id': 'str'} }
>>>> +
>>>> +##
>>>> +# @getfd:
>>>> +#
>>>> +# Receive a file descriptor via SCM rights and assign it a name
>>>> +#
>>>> +# @fdname: file descriptor name
>>>> +#
>>>> +# Returns: Nothing on success
>>>> +#          If file descriptor was not received, FdNotSupplied
>>>> +#          If @fdname is not valid, InvalidParameterType
>>>> +#
>>>> +# Since: 0.14.0
>>>> +#
>>>> +# Notes: If @fdname already exists, the file descriptor assigned to
>>>> +#        it will be closed and replaced by the received file
>>>> +#        descriptor.
>>>> +#        The 'closefd' command can be used to explicitly close the
>>>> +#        file descriptor when it is no longer needed.
>>>> +##
>>>> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
>>>> +
>>>> +##
>>>> +# @closefd:
>>>> +#
>>>> +# Close a file descriptor previously passed via SCM rights
>>>> +#
>>>> +# @fdname: file descriptor name
>>>> +#
>>>> +# Returns: Nothing on success
>>>> +#          If @fdname is not found, FdNotFound
>>>> +#
>>>> +# Since: 0.14.0
>>>> +##
>>>> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
>>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>>>> index 2e1a38e..e3cf3c5 100644
>>>> --- a/qmp-commands.hx
>>>> +++ b/qmp-commands.hx
>>>> @@ -873,8 +873,7 @@ EQMP
>>>>            .args_type  = "fdname:s",
>>>>            .params     = "getfd name",
>>>>            .help       = "receive a file descriptor via SCM rights and assign it a name",
>>>> -        .user_print = monitor_user_noop,
>>>> -        .mhandler.cmd_new = do_getfd,
>>>> +        .mhandler.cmd_new = qmp_marshal_input_getfd,
>>>>        },
>>>>
>>>>    SQMP
>>>> @@ -892,6 +891,14 @@ Example:
>>>>    -> { "execute": "getfd", "arguments": { "fdname": "fd1" } }
>>>>    <- { "return": {} }
>>>>
>>>> +Notes:
>>>> +
>>>> +(1) If the name specified by the "fdname" argument already exists,
>>>> +    the file descriptor assigned to it will be closed and replaced
>>>> +    by the received file descriptor.
>>>> +(2) The 'closefd' command can be used to explicitly close the file
>>>> +    descriptor when it is no longer needed.
>>>> +
>>>>    EQMP
>>>>
>>>>        {
>>>> @@ -899,8 +906,7 @@ EQMP
>>>>            .args_type  = "fdname:s",
>>>>            .params     = "closefd name",
>>>>            .help       = "close a file descriptor previously passed via SCM rights",
>>>> -        .user_print = monitor_user_noop,
>>>> -        .mhandler.cmd_new = do_closefd,
>>>> +        .mhandler.cmd_new = qmp_marshal_input_closefd,
>>>>        },
>>>>
>>>>    SQMP
>>>
>>
>
Luiz Capitulino July 11, 2012, 6:51 p.m. UTC | #5
On Fri, 22 Jun 2012 14:36:09 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:

> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>

I've cherry-picked this one into the qmp tree.

> ---
> v2:
>  -Convert getfd and closefd to QAPI (lcapitulino@redhat.com)
>  -Remove changes that returned fd from getfd (lcapitulino@redhat.com)
>  -Wrap hmp_* functions around qmp_* functions (kwolf@redhat.com)
>  -Move hmp_* functions to hmp.c (lcapitulino@redhat.com)
>  -Drop .user_print lines (lcapitulino@redhat.com)
>  -Use 'cmd' instead of 'cmd_new' for HMP (lcapitulino@redhat.com)
>  -Change QMP command existance back to 0.14 (lcapitulino@redhat.com)
> 
> v3:
>  -Remove unnecessary return statements from qmp_* functions
>   (lcapitulino@redhat.com)
>  -Add notes to QMP command describing behavior in more detail
>   (lcapitulino@redhat.com, eblake@redhat.com)
> 
> v4:
>  -No changes
> 
>  hmp-commands.hx  |    6 ++----
>  hmp.c            |   18 ++++++++++++++++++
>  hmp.h            |    2 ++
>  monitor.c        |   32 ++++++++++++++------------------
>  qapi-schema.json |   35 +++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   14 ++++++++++----
>  6 files changed, 81 insertions(+), 26 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f5d9d91..eea8b32 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1236,8 +1236,7 @@ ETEXI
>          .args_type  = "fdname:s",
>          .params     = "getfd name",
>          .help       = "receive a file descriptor via SCM rights and assign it a name",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_getfd,
> +        .mhandler.cmd = hmp_getfd,
>      },
>  
>  STEXI
> @@ -1253,8 +1252,7 @@ ETEXI
>          .args_type  = "fdname:s",
>          .params     = "closefd name",
>          .help       = "close a file descriptor previously passed via SCM rights",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_closefd,
> +        .mhandler.cmd = hmp_closefd,
>      },
>  
>  STEXI
> diff --git a/hmp.c b/hmp.c
> index 2ce8cb9..6241856 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -999,3 +999,21 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
>      qmp_netdev_del(id, &err);
>      hmp_handle_error(mon, &err);
>  }
> +
> +void hmp_getfd(Monitor *mon, const QDict *qdict)
> +{
> +    const char *fdname = qdict_get_str(qdict, "fdname");
> +    Error *errp = NULL;
> +
> +    qmp_getfd(fdname, &errp);
> +    hmp_handle_error(mon, &errp);
> +}
> +
> +void hmp_closefd(Monitor *mon, const QDict *qdict)
> +{
> +    const char *fdname = qdict_get_str(qdict, "fdname");
> +    Error *errp = NULL;
> +
> +    qmp_closefd(fdname, &errp);
> +    hmp_handle_error(mon, &errp);
> +}
> diff --git a/hmp.h b/hmp.h
> index 79d138d..8d2b0d7 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -64,5 +64,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
>  void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_getfd(Monitor *mon, const QDict *qdict);
> +void hmp_closefd(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/monitor.c b/monitor.c
> index a3bc2c7..1a7f7e7 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2182,48 +2182,45 @@ static void do_inject_mce(Monitor *mon, const QDict *qdict)
>  }
>  #endif
>  
> -static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +void qmp_getfd(const char *fdname, Error **errp)
>  {
> -    const char *fdname = qdict_get_str(qdict, "fdname");
>      mon_fd_t *monfd;
>      int fd;
>  
> -    fd = qemu_chr_fe_get_msgfd(mon->chr);
> +    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
>      if (fd == -1) {
> -        qerror_report(QERR_FD_NOT_SUPPLIED);
> -        return -1;
> +        error_set(errp, QERR_FD_NOT_SUPPLIED);
> +        return;
>      }
>  
>      if (qemu_isdigit(fdname[0])) {
> -        qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
> -                      "a name not starting with a digit");
> -        return -1;
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
> +                  "a name not starting with a digit");
> +        return;
>      }
>  
> -    QLIST_FOREACH(monfd, &mon->fds, next) {
> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>          if (strcmp(monfd->name, fdname) != 0) {
>              continue;
>          }
>  
>          close(monfd->fd);
>          monfd->fd = fd;
> -        return 0;
> +        return;
>      }
>  
>      monfd = g_malloc0(sizeof(mon_fd_t));
>      monfd->name = g_strdup(fdname);
>      monfd->fd = fd;
>  
> -    QLIST_INSERT_HEAD(&mon->fds, monfd, next);
> -    return 0;
> +    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
>  }
>  
> -static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +void qmp_closefd(const char *fdname, Error **errp)
>  {
> -    const char *fdname = qdict_get_str(qdict, "fdname");
>      mon_fd_t *monfd;
>  
> -    QLIST_FOREACH(monfd, &mon->fds, next) {
> +    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>          if (strcmp(monfd->name, fdname) != 0) {
>              continue;
>          }
> @@ -2232,11 +2229,10 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
>          close(monfd->fd);
>          g_free(monfd->name);
>          g_free(monfd);
> -        return 0;
> +        return;
>      }
>  
> -    qerror_report(QERR_FD_NOT_FOUND, fdname);
> -    return -1;
> +    error_set(errp, QERR_FD_NOT_FOUND, fdname);
>  }
>  
>  static void do_loadvm(Monitor *mon, const QDict *qdict)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 3b6e346..26a6b84 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1862,3 +1862,38 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'netdev_del', 'data': {'id': 'str'} }
> +
> +##
> +# @getfd:
> +#
> +# Receive a file descriptor via SCM rights and assign it a name
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: Nothing on success
> +#          If file descriptor was not received, FdNotSupplied
> +#          If @fdname is not valid, InvalidParameterType
> +#
> +# Since: 0.14.0
> +#
> +# Notes: If @fdname already exists, the file descriptor assigned to
> +#        it will be closed and replaced by the received file
> +#        descriptor.
> +#        The 'closefd' command can be used to explicitly close the
> +#        file descriptor when it is no longer needed.
> +##
> +{ 'command': 'getfd', 'data': {'fdname': 'str'} }
> +
> +##
> +# @closefd:
> +#
> +# Close a file descriptor previously passed via SCM rights
> +#
> +# @fdname: file descriptor name
> +#
> +# Returns: Nothing on success
> +#          If @fdname is not found, FdNotFound
> +#
> +# Since: 0.14.0
> +##
> +{ 'command': 'closefd', 'data': {'fdname': 'str'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2e1a38e..e3cf3c5 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -873,8 +873,7 @@ EQMP
>          .args_type  = "fdname:s",
>          .params     = "getfd name",
>          .help       = "receive a file descriptor via SCM rights and assign it a name",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_getfd,
> +        .mhandler.cmd_new = qmp_marshal_input_getfd,
>      },
>  
>  SQMP
> @@ -892,6 +891,14 @@ Example:
>  -> { "execute": "getfd", "arguments": { "fdname": "fd1" } }
>  <- { "return": {} }
>  
> +Notes:
> +
> +(1) If the name specified by the "fdname" argument already exists,
> +    the file descriptor assigned to it will be closed and replaced
> +    by the received file descriptor.
> +(2) The 'closefd' command can be used to explicitly close the file
> +    descriptor when it is no longer needed.
> +
>  EQMP
>  
>      {
> @@ -899,8 +906,7 @@ EQMP
>          .args_type  = "fdname:s",
>          .params     = "closefd name",
>          .help       = "close a file descriptor previously passed via SCM rights",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_closefd,
> +        .mhandler.cmd_new = qmp_marshal_input_closefd,
>      },
>  
>  SQMP
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f5d9d91..eea8b32 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1236,8 +1236,7 @@  ETEXI
         .args_type  = "fdname:s",
         .params     = "getfd name",
         .help       = "receive a file descriptor via SCM rights and assign it a name",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_getfd,
+        .mhandler.cmd = hmp_getfd,
     },
 
 STEXI
@@ -1253,8 +1252,7 @@  ETEXI
         .args_type  = "fdname:s",
         .params     = "closefd name",
         .help       = "close a file descriptor previously passed via SCM rights",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_closefd,
+        .mhandler.cmd = hmp_closefd,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 2ce8cb9..6241856 100644
--- a/hmp.c
+++ b/hmp.c
@@ -999,3 +999,21 @@  void hmp_netdev_del(Monitor *mon, const QDict *qdict)
     qmp_netdev_del(id, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_getfd(Monitor *mon, const QDict *qdict)
+{
+    const char *fdname = qdict_get_str(qdict, "fdname");
+    Error *errp = NULL;
+
+    qmp_getfd(fdname, &errp);
+    hmp_handle_error(mon, &errp);
+}
+
+void hmp_closefd(Monitor *mon, const QDict *qdict)
+{
+    const char *fdname = qdict_get_str(qdict, "fdname");
+    Error *errp = NULL;
+
+    qmp_closefd(fdname, &errp);
+    hmp_handle_error(mon, &errp);
+}
diff --git a/hmp.h b/hmp.h
index 79d138d..8d2b0d7 100644
--- a/hmp.h
+++ b/hmp.h
@@ -64,5 +64,7 @@  void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_getfd(Monitor *mon, const QDict *qdict);
+void hmp_closefd(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/monitor.c b/monitor.c
index a3bc2c7..1a7f7e7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2182,48 +2182,45 @@  static void do_inject_mce(Monitor *mon, const QDict *qdict)
 }
 #endif
 
-static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_getfd(const char *fdname, Error **errp)
 {
-    const char *fdname = qdict_get_str(qdict, "fdname");
     mon_fd_t *monfd;
     int fd;
 
-    fd = qemu_chr_fe_get_msgfd(mon->chr);
+    fd = qemu_chr_fe_get_msgfd(cur_mon->chr);
     if (fd == -1) {
-        qerror_report(QERR_FD_NOT_SUPPLIED);
-        return -1;
+        error_set(errp, QERR_FD_NOT_SUPPLIED);
+        return;
     }
 
     if (qemu_isdigit(fdname[0])) {
-        qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
-                      "a name not starting with a digit");
-        return -1;
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
+                  "a name not starting with a digit");
+        return;
     }
 
-    QLIST_FOREACH(monfd, &mon->fds, next) {
+    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
         if (strcmp(monfd->name, fdname) != 0) {
             continue;
         }
 
         close(monfd->fd);
         monfd->fd = fd;
-        return 0;
+        return;
     }
 
     monfd = g_malloc0(sizeof(mon_fd_t));
     monfd->name = g_strdup(fdname);
     monfd->fd = fd;
 
-    QLIST_INSERT_HEAD(&mon->fds, monfd, next);
-    return 0;
+    QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
 }
 
-static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_closefd(const char *fdname, Error **errp)
 {
-    const char *fdname = qdict_get_str(qdict, "fdname");
     mon_fd_t *monfd;
 
-    QLIST_FOREACH(monfd, &mon->fds, next) {
+    QLIST_FOREACH(monfd, &cur_mon->fds, next) {
         if (strcmp(monfd->name, fdname) != 0) {
             continue;
         }
@@ -2232,11 +2229,10 @@  static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
         close(monfd->fd);
         g_free(monfd->name);
         g_free(monfd);
-        return 0;
+        return;
     }
 
-    qerror_report(QERR_FD_NOT_FOUND, fdname);
-    return -1;
+    error_set(errp, QERR_FD_NOT_FOUND, fdname);
 }
 
 static void do_loadvm(Monitor *mon, const QDict *qdict)
diff --git a/qapi-schema.json b/qapi-schema.json
index 3b6e346..26a6b84 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1862,3 +1862,38 @@ 
 # Since: 0.14.0
 ##
 { 'command': 'netdev_del', 'data': {'id': 'str'} }
+
+##
+# @getfd:
+#
+# Receive a file descriptor via SCM rights and assign it a name
+#
+# @fdname: file descriptor name
+#
+# Returns: Nothing on success
+#          If file descriptor was not received, FdNotSupplied
+#          If @fdname is not valid, InvalidParameterType
+#
+# Since: 0.14.0
+#
+# Notes: If @fdname already exists, the file descriptor assigned to
+#        it will be closed and replaced by the received file
+#        descriptor.
+#        The 'closefd' command can be used to explicitly close the
+#        file descriptor when it is no longer needed.
+##
+{ 'command': 'getfd', 'data': {'fdname': 'str'} }
+
+##
+# @closefd:
+#
+# Close a file descriptor previously passed via SCM rights
+#
+# @fdname: file descriptor name
+#
+# Returns: Nothing on success
+#          If @fdname is not found, FdNotFound
+#
+# Since: 0.14.0
+##
+{ 'command': 'closefd', 'data': {'fdname': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2e1a38e..e3cf3c5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -873,8 +873,7 @@  EQMP
         .args_type  = "fdname:s",
         .params     = "getfd name",
         .help       = "receive a file descriptor via SCM rights and assign it a name",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_getfd,
+        .mhandler.cmd_new = qmp_marshal_input_getfd,
     },
 
 SQMP
@@ -892,6 +891,14 @@  Example:
 -> { "execute": "getfd", "arguments": { "fdname": "fd1" } }
 <- { "return": {} }
 
+Notes:
+
+(1) If the name specified by the "fdname" argument already exists,
+    the file descriptor assigned to it will be closed and replaced
+    by the received file descriptor.
+(2) The 'closefd' command can be used to explicitly close the file
+    descriptor when it is no longer needed.
+
 EQMP
 
     {
@@ -899,8 +906,7 @@  EQMP
         .args_type  = "fdname:s",
         .params     = "closefd name",
         .help       = "close a file descriptor previously passed via SCM rights",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_closefd,
+        .mhandler.cmd_new = qmp_marshal_input_closefd,
     },
 
 SQMP