[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 11/11] QMP/HMP: only actually implement getfd on CONFIG_PO
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v4 11/11] QMP/HMP: only actually implement getfd on CONFIG_POSIX |
Date: |
Mon, 06 Mar 2023 17:01:22 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Regarding the subject line:
1. We use "qmp:" for qmp-only patches, "hmp:" for hmp-only patches, and
"monitor:" for both. "qmp hmp:" would work there, too.
2. We're not implementing anything, we're restricting the existing
implementation to hosts where it is actually useful.
Suggest "monitor: Restrict command getfd to POSIX hosts"
marcandre.lureau@redhat.com writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Currently, the function will simply fail if ancillary fds are not
> provided, for ex on unsupported platforms.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qapi/misc.json | 2 +-
> monitor/fds.c | 2 ++
> monitor/hmp-cmds.c | 2 ++
> hmp-commands.hx | 2 ++
> 4 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 031c94050c..96c053e305 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -273,7 +273,7 @@
> # <- { "return": {} }
> #
> ##
> -{ 'command': 'getfd', 'data': {'fdname': 'str'} }
> +{ 'command': 'getfd', 'data': {'fdname': 'str'}, 'if': 'CONFIG_POSIX' }
>
> ##
> # @get-win32-socket:
This changes the failure from
{"error": {"class": "GenericError", "desc": "No file descriptor supplied
via SCM_RIGHTS"}}
to
{"error": {"class": "CommandNotFound", "desc": "The command getfd has not
been found"}}
when CONFIG_POSIX is off. I think this is fine. But I'd like the
commit message to document it.
> diff --git a/monitor/fds.c b/monitor/fds.c
> index 9ed4197358..d86c2c674c 100644
> --- a/monitor/fds.c
> +++ b/monitor/fds.c
> @@ -98,6 +98,7 @@ static bool monitor_add_fd(Monitor *mon, int fd, const char
> *fdname, Error **err
> return true;
> }
>
> +#ifdef CONFIG_POSIX
> void qmp_getfd(const char *fdname, Error **errp)
> {
> Monitor *cur_mon = monitor_cur();
> @@ -111,6 +112,7 @@ void qmp_getfd(const char *fdname, Error **errp)
>
> monitor_add_fd(cur_mon, fd, fdname, errp);
> }
> +#endif
>
> void qmp_closefd(const char *fdname, Error **errp)
> {
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 34bd8c67d7..6c559b48c8 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -192,6 +192,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
> hmp_handle_error(mon, err);
> }
>
> +#ifdef CONFIG_POSIX
> void hmp_getfd(Monitor *mon, const QDict *qdict)
> {
> const char *fdname = qdict_get_str(qdict, "fdname");
> @@ -200,6 +201,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict)
> qmp_getfd(fdname, &err);
> hmp_handle_error(mon, err);
> }
> +#endif
>
> void hmp_closefd(Monitor *mon, const QDict *qdict)
> {
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index b87c250e23..bb85ee1d26 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1486,6 +1486,7 @@ SRST
> Inject an MCE on the given CPU (x86 only).
> ERST
>
> +#ifdef CONFIG_POSIX
> {
> .name = "getfd",
> .args_type = "fdname:s",
> @@ -1501,6 +1502,7 @@ SRST
> mechanism on unix sockets, it is stored using the name *fdname* for
> later use by other monitor commands.
> ERST
> +#endif
>
> {
> .name = "closefd",
With the commit message brushed up:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
- Re: [PATCH v4 07/11] qapi/gen: run C code through clang-format, if possible, (continued)
[PATCH v4 08/11] qmp: add 'get-win32-socket', marcandre . lureau, 2023/03/06
[PATCH v4 11/11] QMP/HMP: only actually implement getfd on CONFIG_POSIX, marcandre . lureau, 2023/03/06
- Re: [PATCH v4 11/11] QMP/HMP: only actually implement getfd on CONFIG_POSIX,
Markus Armbruster <=
[PATCH v4 09/11] libqtest: make qtest_qmp_add_client work on win32, marcandre . lureau, 2023/03/06
[PATCH v4 10/11] qtest: enable vnc-display test on win32, marcandre . lureau, 2023/03/06