[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 06/10] monitor: release the lock before calling close()
From: |
Marc-André Lureau |
Subject: |
Re: [PATCH v3 06/10] monitor: release the lock before calling close() |
Date: |
Tue, 14 Feb 2023 17:36:32 +0400 |
Hi
On Tue, Feb 14, 2023 at 5:34 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> marcandre.lureau@redhat.com writes:
>
> > 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);
> > 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);
> > }
> >
> > void qmp_closefd(const char *fdname, Error **errp)
>
> This confused me. I think I understand now, but let's double-check.
>
> You're reverting commit 0210c3b39bef08 for qmp_getfd() because it
> extended the criticial section beyond the close(), invalidating the
> comment. Correct?
Correct
>
> Did it actually break anything?
Not that I know of (David admitted over IRC that this was not intended)
--
Marc-André Lureau
- [PATCH v3 02/10] char: do not double-close fd when failing to add client, (continued)
- [PATCH v3 02/10] char: do not double-close fd when failing to add client, marcandre . lureau, 2023/02/07
- [PATCH v3 03/10] tests/docker: fix a win32 error due to portability, marcandre . lureau, 2023/02/07
- [PATCH v3 04/10] osdep: implement qemu_socketpair() for win32, marcandre . lureau, 2023/02/07
- [PATCH v3 05/10] qmp: 'add_client' actually expects sockets, marcandre . lureau, 2023/02/07
- [PATCH v3 06/10] monitor: release the lock before calling close(), marcandre . lureau, 2023/02/07
[PATCH v3 07/10] qapi: implement conditional command arguments, marcandre . lureau, 2023/02/07