Message ID | 1350410912-2373-1-git-send-email-coreyb@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On 10/16/2012 12:08 PM, Corey Bryant wrote: > If an fd is added to an fd set via the command line, and it is not > referenced by another command line option (ie. -drive), then clean > it up after QEMU initialization is complete. > > Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com> > --- > v3: > - This patch was split into it's own patch in v3 > (eblake@redhat.com, kwolf@redhat.com) > > monitor.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> > > diff --git a/monitor.c b/monitor.c > index 5d5de41..0dae7ac 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2105,8 +2105,9 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) > MonFdsetFd *mon_fdset_fd_next; > > QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) { > - if (mon_fdset_fd->removed || > - (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) { > + if ((mon_fdset_fd->removed || > + (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) && > + runstate_is_running()) { This condition looks sufficient for command line parsing; however, I have to wonder if there are any long-term ill effects, such as if I pause a guest, then close an fdset, where qemu keeps the fd open until I continue the guest. But thinking a bit more, we never promised that qemu would close fds right away, but only that it wouldn't leak fds by closing them whenever qemu thinks it is convenient. > close(mon_fdset_fd->fd); > g_free(mon_fdset_fd->opaque); > QLIST_REMOVE(mon_fdset_fd, next); >
On 10/17/2012 12:02 AM, Eric Blake wrote: > On 10/16/2012 12:08 PM, Corey Bryant wrote: >> If an fd is added to an fd set via the command line, and it is not >> referenced by another command line option (ie. -drive), then clean >> it up after QEMU initialization is complete. >> >> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com> >> --- >> v3: >> - This patch was split into it's own patch in v3 >> (eblake@redhat.com, kwolf@redhat.com) >> >> monitor.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) > > Reviewed-by: Eric Blake <eblake@redhat.com> > Thanks! >> >> diff --git a/monitor.c b/monitor.c >> index 5d5de41..0dae7ac 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -2105,8 +2105,9 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) >> MonFdsetFd *mon_fdset_fd_next; >> >> QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) { >> - if (mon_fdset_fd->removed || >> - (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) { >> + if ((mon_fdset_fd->removed || >> + (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) && >> + runstate_is_running()) { > > This condition looks sufficient for command line parsing; however, I > have to wonder if there are any long-term ill effects, such as if I > pause a guest, then close an fdset, where qemu keeps the fd open until I > continue the guest. But thinking a bit more, we never promised that > qemu would close fds right away, but only that it wouldn't leak fds by > closing them whenever qemu thinks it is convenient. > I can't think of any ill effects. I think you may be able to remove an fd or fd set followed by adding a drive using that fd set, but that is just a difference in behavior compared to when a guest is running. >> close(mon_fdset_fd->fd); >> g_free(mon_fdset_fd->opaque); >> QLIST_REMOVE(mon_fdset_fd, next); >> >
diff --git a/monitor.c b/monitor.c index 5d5de41..0dae7ac 100644 --- a/monitor.c +++ b/monitor.c @@ -2105,8 +2105,9 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) MonFdsetFd *mon_fdset_fd_next; QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next, mon_fdset_fd_next) { - if (mon_fdset_fd->removed || - (QLIST_EMPTY(&mon_fdset->dup_fds) && mon_refcount == 0)) { + 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);
If an fd is added to an fd set via the command line, and it is not referenced by another command line option (ie. -drive), then clean it up after QEMU initialization is complete. Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com> --- v3: - This patch was split into it's own patch in v3 (eblake@redhat.com, kwolf@redhat.com) monitor.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)