| Message ID | 20240523190548.23977-6-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | migration/mapped-ram: Add direct-io support | expand |
On Thu, May 23, 2024 at 04:05:35PM -0300, Fabiano Rosas wrote: > Introduce two new functions to remove and free no longer used fds and > fdsets. > > We need those to decouple the remove/free routines from > monitor_fdset_cleanup() which will go away in the next patches. > > The new functions: > > - monitor_fdset_free() will be used when a monitor connection closes > and when an fd is removed to cleanup any fdset that is now empty. > > - monitor_fdset_fd_free() will be used to remove one or more fds that > have been explicitly targeted by qmp_remove_fd(). > > Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> One nitpick below. > --- > monitor/fds.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/monitor/fds.c b/monitor/fds.c > index fb9f58c056..101e21720d 100644 > --- a/monitor/fds.c > +++ b/monitor/fds.c > @@ -167,6 +167,22 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp) > return -1; > } > > +static void monitor_fdset_free(MonFdset *mon_fdset) > +{ > + if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) { > + QLIST_REMOVE(mon_fdset, next); > + g_free(mon_fdset); > + } > +} Would monitor_fdset_free_if_empty() (or similar) slightly better? static void monitor_fdset_free(MonFdset *mon_fdset) { QLIST_REMOVE(mon_fdset, next); g_free(mon_fdset); } static void monitor_fdset_free_if_empty(MonFdset *mon_fdset) { if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) { monitor_fdset_free(mon_fdset); } } > + > +static void monitor_fdset_fd_free(MonFdsetFd *mon_fdset_fd) > +{ > + close(mon_fdset_fd->fd); > + g_free(mon_fdset_fd->opaque); > + QLIST_REMOVE(mon_fdset_fd, next); > + g_free(mon_fdset_fd); > +} > + > static void monitor_fdset_cleanup(MonFdset *mon_fdset) > { > MonFdsetFd *mon_fdset_fd; > @@ -176,17 +192,11 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) > if ((mon_fdset_fd->removed || > (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) && > runstate_is_running()) { > - close(mon_fdset_fd->fd); > - g_free(mon_fdset_fd->opaque); > - QLIST_REMOVE(mon_fdset_fd, next); > - g_free(mon_fdset_fd); > + monitor_fdset_fd_free(mon_fdset_fd); > } > } > > - if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) { > - QLIST_REMOVE(mon_fdset, next); > - g_free(mon_fdset); > - } > + monitor_fdset_free(mon_fdset); > } > > void monitor_fdsets_cleanup(void) > -- > 2.35.3 >
Peter Xu <peterx@redhat.com> writes: > On Thu, May 23, 2024 at 04:05:35PM -0300, Fabiano Rosas wrote: >> Introduce two new functions to remove and free no longer used fds and >> fdsets. >> >> We need those to decouple the remove/free routines from >> monitor_fdset_cleanup() which will go away in the next patches. >> >> The new functions: >> >> - monitor_fdset_free() will be used when a monitor connection closes >> and when an fd is removed to cleanup any fdset that is now empty. >> >> - monitor_fdset_fd_free() will be used to remove one or more fds that >> have been explicitly targeted by qmp_remove_fd(). >> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> > > Reviewed-by: Peter Xu <peterx@redhat.com> > > One nitpick below. > >> --- >> monitor/fds.c | 26 ++++++++++++++++++-------- >> 1 file changed, 18 insertions(+), 8 deletions(-) >> >> diff --git a/monitor/fds.c b/monitor/fds.c >> index fb9f58c056..101e21720d 100644 >> --- a/monitor/fds.c >> +++ b/monitor/fds.c >> @@ -167,6 +167,22 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp) >> return -1; >> } >> >> +static void monitor_fdset_free(MonFdset *mon_fdset) >> +{ >> + if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) { >> + QLIST_REMOVE(mon_fdset, next); >> + g_free(mon_fdset); >> + } >> +} > > Would monitor_fdset_free_if_empty() (or similar) slightly better? Yes, I'll use that. > > static void monitor_fdset_free(MonFdset *mon_fdset) > { > QLIST_REMOVE(mon_fdset, next); > g_free(mon_fdset); > } > > static void monitor_fdset_free_if_empty(MonFdset *mon_fdset) > { > if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) { > monitor_fdset_free(mon_fdset); > } > } > >> + >> +static void monitor_fdset_fd_free(MonFdsetFd *mon_fdset_fd) >> +{ >> + close(mon_fdset_fd->fd); >> + g_free(mon_fdset_fd->opaque); >> + QLIST_REMOVE(mon_fdset_fd, next); >> + g_free(mon_fdset_fd); >> +} >> + >> static void monitor_fdset_cleanup(MonFdset *mon_fdset) >> { >> MonFdsetFd *mon_fdset_fd; >> @@ -176,17 +192,11 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) >> if ((mon_fdset_fd->removed || >> (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) && >> runstate_is_running()) { >> - close(mon_fdset_fd->fd); >> - g_free(mon_fdset_fd->opaque); >> - QLIST_REMOVE(mon_fdset_fd, next); >> - g_free(mon_fdset_fd); >> + monitor_fdset_fd_free(mon_fdset_fd); >> } >> } >> >> - if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) { >> - QLIST_REMOVE(mon_fdset, next); >> - g_free(mon_fdset); >> - } >> + monitor_fdset_free(mon_fdset); >> } >> >> void monitor_fdsets_cleanup(void) >> -- >> 2.35.3 >>
diff --git a/monitor/fds.c b/monitor/fds.c index fb9f58c056..101e21720d 100644 --- a/monitor/fds.c +++ b/monitor/fds.c @@ -167,6 +167,22 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp) return -1; } +static void monitor_fdset_free(MonFdset *mon_fdset) +{ + if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) { + QLIST_REMOVE(mon_fdset, next); + g_free(mon_fdset); + } +} + +static void monitor_fdset_fd_free(MonFdsetFd *mon_fdset_fd) +{ + close(mon_fdset_fd->fd); + g_free(mon_fdset_fd->opaque); + QLIST_REMOVE(mon_fdset_fd, next); + g_free(mon_fdset_fd); +} + static void monitor_fdset_cleanup(MonFdset *mon_fdset) { MonFdsetFd *mon_fdset_fd; @@ -176,17 +192,11 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) if ((mon_fdset_fd->removed || (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) && runstate_is_running()) { - close(mon_fdset_fd->fd); - g_free(mon_fdset_fd->opaque); - QLIST_REMOVE(mon_fdset_fd, next); - g_free(mon_fdset_fd); + monitor_fdset_fd_free(mon_fdset_fd); } } - if (QLIST_EMPTY(&mon_fdset->fds) && QLIST_EMPTY(&mon_fdset->dup_fds)) { - QLIST_REMOVE(mon_fdset, next); - g_free(mon_fdset); - } + monitor_fdset_free(mon_fdset); } void monitor_fdsets_cleanup(void)
Introduce two new functions to remove and free no longer used fds and fdsets. We need those to decouple the remove/free routines from monitor_fdset_cleanup() which will go away in the next patches. The new functions: - monitor_fdset_free() will be used when a monitor connection closes and when an fd is removed to cleanup any fdset that is now empty. - monitor_fdset_fd_free() will be used to remove one or more fds that have been explicitly targeted by qmp_remove_fd(). Signed-off-by: Fabiano Rosas <farosas@suse.de> --- monitor/fds.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)