I am replacing the last instances of close() I found with VIR_CLOSE() /
VIR_FORCE_CLOSE respectively.
The first patch of virsh I missed out on previously.
The 2nd patch I had left out intentionally to look at it more carefully:
The 'closed' variable could be easily removed since it wasn't used
anywhere else. The possible race condition that could result from the
filedescriptor being closed and not set to -1 (and possibly let us write
into 'something' totally different if the fd was allocated by another
thread) seems to be prevented by the qemuMonitorLock() already placed
around the code that reads from or writes to the fd. So the change of
this code as shown in the patch should not have any side-effects.
Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>
---
src/qemu/qemu_monitor.c | 14 +++-----------
tools/virsh.c | 4 ++--
2 files changed, 5 insertions(+), 13 deletions(-)
Index: libvirt-acl/tools/virsh.c
===================================================================
--- libvirt-acl.orig/tools/virsh.c
+++ libvirt-acl/tools/virsh.c
@@ -8977,12 +8977,12 @@ editWriteToTempFile (vshControl *ctl, co
if (safewrite (fd, doc, strlen (doc)) == -1) {
vshError(ctl, _("write: %s: failed to write to temporary file:
%s"),
ret, strerror(errno));
- close (fd);
+ VIR_FORCE_CLOSE(fd);
unlink (ret);
VIR_FREE(ret);
return NULL;
}
- if (close (fd) == -1) {
+ if (VIR_CLOSE(fd) < 0) {
vshError(ctl, _("close: %s: failed to write or close temporary
file: %s"),
ret, strerror(errno));
unlink (ret);
Index: libvirt-acl/src/qemu/qemu_monitor.c
===================================================================
--- libvirt-acl.orig/src/qemu/qemu_monitor.c
+++ libvirt-acl/src/qemu/qemu_monitor.c
@@ -73,8 +73,6 @@ struct _qemuMonitor {
/* If the monitor EOF callback is currently active (stops more
commands being run) */
unsigned eofcb: 1;
- /* If the monitor is in process of shutting down */
- unsigned closed: 1;
unsigned json: 1;
};
@@ -692,17 +690,11 @@ void qemuMonitorClose(qemuMonitorPtr mon
VIR_DEBUG("mon=%p", mon);
qemuMonitorLock(mon);
- if (!mon->closed) {
+
+ if (mon->fd >= 0) {
if (mon->watch)
virEventRemoveHandle(mon->watch);
- if (mon->fd != -1)
- close(mon->fd);
- /* NB: ordinarily one might immediately set mon->watch to -1
- * and mon->fd to -1, but there may be a callback active
- * that is still relying on these fields being valid. So
- * we merely close them, but not clear their values and
- * use this explicit 'closed' flag to track this state */
- mon->closed = 1;
+ VIR_FORCE_CLOSE(mon->fd);
}
if (qemuMonitorUnref(mon) > 0)