[libvirt] [PATCH v3 3/4] monitor: Prevent removing fd from set during init
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(-) 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); -- 1.7.11.4
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);
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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);
-- Regards, Corey Bryant
participants (2)
-
Corey Bryant -
Eric Blake