Re: [PATCH v3 06/10] monitor: release the lock before calling close()

[Philippe Mathieu-Daudé]

On 7/2/23 15:25, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> As per comment, presumably to avoid syscall in critical section.
>
> Fixes: 0210c3b39bef08 ("monitor: Use LOCK_GUARD macros")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   monitor/fds.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/monitor/fds.c b/monitor/fds.c
> index 26b39a0ce6..03c5e97c35 100644
> --- a/monitor/fds.c
> +++ b/monitor/fds.c
> @@ -80,7 +80,7 @@ void qmp_getfd(const char *fdname, Error **errp)
>           return;
>       }
>   
> -    QEMU_LOCK_GUARD(&cur_mon->mon_lock);
> +    qemu_mutex_lock(&cur_mon->mon_lock);

If you respin, please add /* See close() call below. */ comment.

>       QLIST_FOREACH(monfd, &cur_mon->fds, next) {
>           if (strcmp(monfd->name, fdname) != 0) {
>               continue;
> @@ -88,6 +88,7 @@ void qmp_getfd(const char *fdname, Error **errp)
>   
>           tmp_fd = monfd->fd;
>           monfd->fd = fd;
> +        qemu_mutex_unlock(&cur_mon->mon_lock);
>           /* Make sure close() is outside critical section */
>           close(tmp_fd);
>           return;
> @@ -98,6 +99,7 @@ void qmp_getfd(const char *fdname, Error **errp)
>       monfd->fd = fd;
>   
>       QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
> +    qemu_mutex_unlock(&cur_mon->mon_lock);
>   }

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>