- Devel - libvirt List Archives

[libvirt] race condtion in getting VNC port for libvirt
by Guangya Liu 10 Nov '10

10 Nov '10

Hi, We are encountering a problem of race conditions for getting VNC port when VM start up. In a very powerful hypervisor, if we try to start up more than 1 VM concurrently, then there might be some VMs failed to start up due to VNC port confilict. We searched libvirt bugs and found that some one report the problem and we have give a fix, but the fix can not resolve the problem. http://osdir.com/ml/libvir-list/2010-05/msg00530.html https://build.opensuse.org/package/view_file?file=vnc-race-3.patch&packa ge=libvirt&project=Virtualization:openSUSE11.3 The bitmap test-and-set operation is not atomic, so that there still is a time-window in which a VNC port could be reserved for two caller. Your help is highly appreciated! Thanks, Guangya Liu Cloud Developer Platform Computing direct: +86-29-87607400-333 www.platform.com <http://www.platform.com/>

2 3

[libvirt] [PATCH v3] bye to close(), welcome to VIR_(FORCE_)CLOSE()
by Stefan Berger 10 Nov '10

10 Nov '10

Now that 0.8.5 is out, here is another posting of this big cleanup patch. If there are some dubious hunks in it, please let me know and I'd rather cut them out and we fix them with in separate patches... There were some problems with Eric's suggested cfg.mk extension (showing contents of error messages with 'close' as text), so I could not add the 'sc_avoid_close' to it. I again put the diff of tree+v3 against tree+v1 to the end of the patch. I forward-ported V1 for that. V3: - reaction to Daniel's comments on V2 V2: - forgot to convert close ( occurrences (with whitespace) - following Eric's and Laine's comments Using automated replacement with sed and editing I have now replaced all occurrences of close() with VIR_(FORCE_)CLOSE() except for one, of course. Some replacements were straight forward, others I needed to pay attention. I hope I payed attention in all the right places... Please have a look. This should have at least solved one more double-close error. Signed-off-by: Stefan Berger <stefanB(a)us.ibm.com> --- daemon/libvirtd.c | 48 ++++++--------- proxy/libvirt_proxy.c | 16 ++--- src/libvirt.c | 10 +-- src/lxc/lxc_container.c | 13 ++-- src/lxc/lxc_controller.c | 27 +++----- src/lxc/lxc_driver.c | 27 +++----- src/node_device/node_device_linux_sysfs.c | 5 - src/nwfilter/nwfilter_ebiptables_driver.c | 8 +- src/openvz/openvz_conf.c | 35 ++++------- src/openvz/openvz_driver.c | 3 src/phyp/phyp_driver.c | 21 ++++-- src/qemu/qemu_conf.c | 30 ++++----- src/qemu/qemu_driver.c | 94 ++++++++++++------------------ src/qemu/qemu_monitor.c | 3 src/remote/remote_driver.c | 34 ++++------ src/secret/secret_driver.c | 12 +-- src/security/security_apparmor.c | 9 +- src/security/security_selinux.c | 9 +- src/security/virt-aa-helper.c | 9 +- src/storage/storage_backend.c | 27 +++----- src/storage/storage_backend_fs.c | 11 +-- src/storage/storage_backend_iscsi.c | 5 - src/storage/storage_backend_mpath.c | 5 - src/storage/storage_backend_scsi.c | 8 +- src/storage/storage_driver.c | 5 - src/test/test_driver.c | 20 ++---- src/uml/uml_conf.c | 3 src/uml/uml_driver.c | 24 +++---- src/util/bridge.c | 14 ++-- src/util/conf.c | 3 src/util/hooks.c | 22 +++---- src/util/interface.c | 12 +-- src/util/logging.c | 6 - src/util/macvtap.c | 15 +--- src/util/pci.c | 6 - src/util/storage_file.c | 5 - src/util/util.c | 71 ++++++++++------------ src/util/uuid.c | 9 +- src/util/virtaudit.c | 3 src/xen/proxy_internal.c | 14 ++-- src/xen/xen_hypervisor.c | 12 +-- src/xen/xen_inotify.c | 3 src/xen/xend_internal.c | 17 ++--- tests/testutils.c | 19 +++--- tools/console.c | 3 45 files changed, 352 insertions(+), 403 deletions(-) Index: libvirt-acl/src/libvirt.c =================================================================== --- libvirt-acl.orig/src/libvirt.c +++ libvirt-acl/src/libvirt.c @@ -10794,7 +10794,7 @@ virStreamRef(virStreamPtr stream) * ... report an error .... * done: * virStreamFree(st); - * close(fd); + * VIR_FORCE_CLOSE(fd); * * Returns the number of bytes written, which may be less * than requested. @@ -10884,8 +10884,8 @@ error: * ... report an error .... * done: * virStreamFree(st); - * close(fd); - * + * if (VIR_CLOSE(fd) < 0) + * virReportSystemError(errno, "%s", _("failed to close file")); * * Returns the number of bytes read, which may be less * than requested. @@ -10964,7 +10964,7 @@ error: * if (virStreamFinish(st) < 0) * ...report an error... * virStreamFree(st); - * close(fd); + * VIR_FORCE_CLOSE(fd); * * Returns 0 if all the data was successfully sent. The caller * should invoke virStreamFinish(st) to flush the stream upon @@ -11061,7 +11061,7 @@ cleanup: * if (virStreamFinish(st) < 0) * ...report an error... * virStreamFree(st); - * close(fd); + * VIR_FORCE_CLOSE(fd); * * Returns 0 if all the data was successfully received. The caller * should invoke virStreamFinish(st) to flush the stream upon Index: libvirt-acl/src/lxc/lxc_container.c =================================================================== --- libvirt-acl.orig/src/lxc/lxc_container.c +++ libvirt-acl/src/lxc/lxc_container.c @@ -52,6 +52,7 @@ #include "util.h" #include "memory.h" #include "veth.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -145,8 +146,10 @@ static int lxcContainerSetStdio(int cont * close all FDs before executing the container */ open_max = sysconf (_SC_OPEN_MAX); for (i = 0; i < open_max; i++) - if (i != ttyfd && i != control) - close(i); + if (i != ttyfd && i != control) { + int tmpfd = i; + VIR_FORCE_CLOSE(tmpfd); + } if (dup2(ttyfd, 0) < 0) { virReportSystemError(errno, "%s", @@ -222,7 +225,7 @@ static int lxcContainerWaitForContinue(i _("Failed to read the container continue message")); return -1; } - close(control); + VIR_FORCE_CLOSE(control); DEBUG0("Received container continue message"); @@ -776,10 +779,10 @@ static int lxcContainerChild( void *data VIR_FREE(ttyPath); if (lxcContainerSetStdio(argv->monitor, ttyfd) < 0) { - close(ttyfd); + VIR_FORCE_CLOSE(ttyfd); return -1; } - close(ttyfd); + VIR_FORCE_CLOSE(ttyfd); if (lxcContainerSetupMounts(vmDef, root) < 0) return -1; Index: libvirt-acl/src/lxc/lxc_controller.c =================================================================== --- libvirt-acl.orig/src/lxc/lxc_controller.c +++ libvirt-acl/src/lxc/lxc_controller.c @@ -48,6 +48,7 @@ #include "veth.h" #include "memory.h" #include "util.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -233,8 +234,7 @@ static int lxcMonitorServer(const char * return fd; error: - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } @@ -409,7 +409,7 @@ static int lxcControllerMain(int monitor goto cleanup; } if (client != -1) { /* Already connected, so kick new one out */ - close(fd); + VIR_FORCE_CLOSE(fd); continue; } client = fd; @@ -426,8 +426,7 @@ static int lxcControllerMain(int monitor _("epoll_ctl(client) failed")); goto cleanup; } - close(client); - client = -1; + VIR_FORCE_CLOSE(client); } else { if (epollEvent.events & EPOLLIN) { curFdOff = epollEvent.data.fd == appPty ? 0 : 1; @@ -485,9 +484,9 @@ static int lxcControllerMain(int monitor rc = 0; cleanup: - close(appPty); - close(contPty); - close(epollFd); + VIR_FORCE_CLOSE(appPty); + VIR_FORCE_CLOSE(contPty); + VIR_FORCE_CLOSE(epollFd); return rc; } @@ -660,8 +659,7 @@ lxcControllerRun(virDomainDefPtr def, control[1], containerPtyPath)) < 0) goto cleanup; - close(control[1]); - control[1] = -1; + VIR_FORCE_CLOSE(control[1]); if (lxcControllerMoveInterfaces(nveths, veths, container) < 0) goto cleanup; @@ -679,13 +677,10 @@ lxcControllerRun(virDomainDefPtr def, cleanup: VIR_FREE(devptmx); VIR_FREE(devpts); - if (control[0] != -1) - close(control[0]); - if (control[1] != -1) - close(control[1]); + VIR_FORCE_CLOSE(control[0]); + VIR_FORCE_CLOSE(control[1]); VIR_FREE(containerPtyPath); - if (containerPty != -1) - close(containerPty); + VIR_FORCE_CLOSE(containerPty); if (container > 1) { int status; Index: libvirt-acl/src/lxc/lxc_driver.c =================================================================== --- libvirt-acl.orig/src/lxc/lxc_driver.c +++ libvirt-acl/src/lxc/lxc_driver.c @@ -51,6 +51,7 @@ #include "uuid.h" #include "stats_linux.h" #include "hooks.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -974,7 +975,7 @@ static int lxcVmCleanup(lxc_driver_t *dr } virEventRemoveHandle(priv->monitorWatch); - close(priv->monitor); + VIR_FORCE_CLOSE(priv->monitor); virFileDeletePid(driver->stateDir, vm->def->name); virDomainDeleteConfig(driver->stateDir, NULL, vm); @@ -1156,8 +1157,7 @@ static int lxcMonitorClient(lxc_driver_t error: VIR_FREE(sockpath); - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } @@ -1539,19 +1539,18 @@ static int lxcVmStart(virConnectPtr conn rc = 0; cleanup: + if (VIR_CLOSE(logfd) < 0) { + virReportSystemError(errno, "%s", _("could not close logfile")); + rc = -1; + } for (i = 0 ; i < nveths ; i++) { if (rc != 0) vethDelete(veths[i]); VIR_FREE(veths[i]); } - if (rc != 0 && priv->monitor != -1) { - close(priv->monitor); - priv->monitor = -1; - } - if (parentTty != -1) - close(parentTty); - if (logfd != -1) - close(logfd); + if (rc != 0) + VIR_FORCE_CLOSE(priv->monitor); + VIR_FORCE_CLOSE(parentTty); VIR_FREE(logfile); return rc; } @@ -2011,8 +2010,7 @@ lxcReconnectVM(void *payload, const char /* Read pid from controller */ if ((virFileReadPid(lxc_driver->stateDir, vm->def->name, &vm->pid)) != 0) { - close(priv->monitor); - priv->monitor = -1; + VIR_FORCE_CLOSE(priv->monitor); goto cleanup; } @@ -2042,8 +2040,7 @@ lxcReconnectVM(void *payload, const char } } else { vm->def->id = -1; - close(priv->monitor); - priv->monitor = -1; + VIR_FORCE_CLOSE(priv->monitor); } cleanup: Index: libvirt-acl/src/node_device/node_device_linux_sysfs.c =================================================================== --- libvirt-acl.orig/src/node_device/node_device_linux_sysfs.c +++ libvirt-acl/src/node_device/node_device_linux_sysfs.c @@ -31,6 +31,7 @@ #include "virterror_internal.h" #include "memory.h" #include "logging.h" +#include "files.h" #include <dirent.h> #define VIR_FROM_THIS VIR_FROM_NODEDEV @@ -104,9 +105,7 @@ int read_wwn_linux(int host, const char } out: - if (fd != -1) { - close(fd); - } + VIR_FORCE_CLOSE(fd); return retval; } Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -37,6 +37,7 @@ #include "nwfilter_conf.h" #include "nwfilter_gentech_driver.h" #include "nwfilter_ebiptables_driver.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_NWFILTER @@ -2493,13 +2494,12 @@ ebiptablesWriteToTempFile(const char *st } VIR_FREE(header); - close(fd); + VIR_FORCE_CLOSE(fd); return filnam; err_exit: VIR_FREE(header); - if (fd >= 0) - close(fd); + VIR_FORCE_CLOSE(fd); unlink(filename); return NULL; } @@ -3259,7 +3259,7 @@ iptablesCheckBridgeNFCallEnabled(bool is lastReport = now; } } - close(fd); + VIR_FORCE_CLOSE(fd); } } } Index: libvirt-acl/src/openvz/openvz_conf.c =================================================================== --- libvirt-acl.orig/src/openvz/openvz_conf.c +++ libvirt-acl/src/openvz/openvz_conf.c @@ -50,6 +50,7 @@ #include "memory.h" #include "util.h" #include "nodeinfo.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_OPENVZ @@ -109,7 +110,7 @@ openvzExtractVersionInfo(const char *cmd cleanup2: VIR_FREE(help); - if (close(newstdout) < 0) + if (VIR_CLOSE(newstdout) < 0) ret = -1; rewait: @@ -569,7 +570,7 @@ openvzWriteConfigParam(const char * conf goto error; temp_fd = open(temp_file, O_WRONLY | O_CREAT | O_TRUNC, 0644); if (temp_fd == -1) { - close(fd); + VIR_FORCE_CLOSE(fd); goto error; } @@ -590,12 +591,10 @@ openvzWriteConfigParam(const char * conf safewrite(temp_fd, "\"\n", 2) < 0) goto error; - if (close(fd) < 0) + if (VIR_CLOSE(fd) < 0) goto error; - fd = -1; - if (close(temp_fd) < 0) + if (VIR_CLOSE(temp_fd) < 0) goto error; - temp_fd = -1; if (rename(temp_file, conf_file) < 0) goto error; @@ -603,10 +602,8 @@ openvzWriteConfigParam(const char * conf return 0; error: - if (fd != -1) - close(fd); - if (temp_fd != -1) - close(temp_fd); + VIR_FORCE_CLOSE(fd); + VIR_FORCE_CLOSE(temp_fd); if (temp_file) unlink(temp_file); VIR_FREE(temp_file); @@ -662,7 +659,7 @@ openvzReadConfigParam(const char * conf_ } } } - close(fd); + VIR_FORCE_CLOSE(fd); if (ret == 0 && found) ret = 1; @@ -703,7 +700,7 @@ openvz_copyfile(char* from_path, char* t return -1; copy_fd = open(to_path, O_WRONLY | O_CREAT | O_TRUNC, 0644); if (copy_fd == -1) { - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } @@ -716,19 +713,16 @@ openvz_copyfile(char* from_path, char* t goto error; } - if (close(fd) < 0) + if (VIR_CLOSE(fd) < 0) goto error; - fd = -1; - if (close(copy_fd) < 0) + if (VIR_CLOSE(copy_fd) < 0) goto error; return 0; error: - if (fd != -1) - close(fd); - if (copy_fd != -1) - close(copy_fd); + VIR_FORCE_CLOSE(fd); + VIR_FORCE_CLOSE(copy_fd); return -1; } @@ -880,8 +874,7 @@ openvzGetVPSUUID(int vpsid, char *uuidst } retval = 0; cleanup: - if (fd >= 0) - close(fd); + VIR_FORCE_CLOSE(fd); VIR_FREE(conf_file); return retval; Index: libvirt-acl/src/openvz/openvz_driver.c =================================================================== --- libvirt-acl.orig/src/openvz/openvz_driver.c +++ libvirt-acl/src/openvz/openvz_driver.c @@ -57,6 +57,7 @@ #include "nodeinfo.h" #include "memory.h" #include "bridge.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_OPENVZ @@ -1540,7 +1541,7 @@ Version: 2.2 } } - close(fd); + VIR_FORCE_CLOSE(fd); if (ret < 0) return -1; Index: libvirt-acl/src/phyp/phyp_driver.c =================================================================== --- libvirt-acl.orig/src/phyp/phyp_driver.c +++ libvirt-acl/src/phyp/phyp_driver.c @@ -58,6 +58,7 @@ #include "domain_conf.h" #include "storage_conf.h" #include "nodeinfo.h" +#include "files.h" #include "phyp_driver.h" @@ -457,11 +458,15 @@ phypUUIDTable_WriteFile(virConnectPtr co } } - close(fd); + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, _("Could not close %s\n"), + local_file); + goto err; + } return 0; err: - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } @@ -672,11 +677,11 @@ phypUUIDTable_ReadFile(virConnectPtr con } else virReportOOMError(); - close(fd); + VIR_FORCE_CLOSE(fd); return 0; err: - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } @@ -764,7 +769,11 @@ phypUUIDTable_Pull(virConnectPtr conn) } break; } - close(fd); + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, _("Could not close %s\n"), + local_file); + goto err; + } goto exit; exit: @@ -1001,7 +1010,7 @@ openSSHSession(virConnectPtr conn, virCo if (connect(sock, cur->ai_addr, cur->ai_addrlen) == 0) { goto connected; } - close(sock); + VIR_FORCE_CLOSE(sock); } cur = cur->ai_next; } Index: libvirt-acl/src/qemu/qemu_conf.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_conf.c +++ libvirt-acl/src/qemu/qemu_conf.c @@ -55,6 +55,7 @@ #include "macvtap.h" #include "cpu/cpu.h" #include "domain_nwfilter.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -541,7 +542,7 @@ qemudProbeMachineTypes(const char *binar cleanup2: VIR_FREE(output); cleanup: - if (close(newstdout) < 0) + if (VIR_CLOSE(newstdout) < 0) ret = -1; rewait: @@ -791,7 +792,7 @@ qemudProbeCPUModels(const char *qemu, cleanup: VIR_FREE(output); - if (close(newstdout) < 0) + if (VIR_CLOSE(newstdout) < 0) ret = -1; rewait: @@ -1440,7 +1441,7 @@ static void qemudParsePCIDeviceStrs(cons cleanup: VIR_FREE(pciassign); - close(newstderr); + VIR_FORCE_CLOSE(newstderr); rewait: if (waitpid(child, &status, 0) != child) { if (errno == EINTR) @@ -1500,7 +1501,7 @@ int qemudExtractVersionInfo(const char * cleanup2: VIR_FREE(help); - if (close(newstdout) < 0) + if (VIR_CLOSE(newstdout) < 0) ret = -1; rewait: @@ -1615,8 +1616,7 @@ qemudPhysIfaceConnect(virConnectPtr conn if ((net->filter) && (net->ifname)) { err = virDomainConfNWFilterInstantiate(conn, net); if (err) { - close(rc); - rc = -1; + VIR_FORCE_CLOSE(rc); delMacvtap(net->ifname, net->mac, net->data.direct.linkdev, &net->data.direct.virtPortProfile); VIR_FREE(net->ifname); @@ -1761,10 +1761,8 @@ qemudNetworkIfaceConnect(virConnectPtr c if (tapfd >= 0) { if ((net->filter) && (net->ifname)) { err = virDomainConfNWFilterInstantiate(conn, net); - if (err) { - close(tapfd); - tapfd = -1; - } + if (err) + VIR_FORCE_CLOSE(tapfd); } } @@ -4576,7 +4574,7 @@ int qemudBuildCommandLine(virConnectPtr if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { virDomainConfNWFilterTeardown(net); - close(tapfd); + VIR_FORCE_CLOSE(tapfd); goto no_memory; } @@ -4595,7 +4593,7 @@ int qemudBuildCommandLine(virConnectPtr if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { virDomainConfNWFilterTeardown(net); - close(tapfd); + VIR_FORCE_CLOSE(tapfd); goto no_memory; } @@ -4615,7 +4613,7 @@ int qemudBuildCommandLine(virConnectPtr int vhostfd = qemudOpenVhostNet(net, qemuCmdFlags); if (vhostfd >= 0) { if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { - close(vhostfd); + VIR_FORCE_CLOSE(vhostfd); goto no_memory; } @@ -5115,14 +5113,14 @@ int qemudBuildCommandLine(virConnectPtr if (configfd >= 0) { if (virAsprintf(&configfd_name, "%d", configfd) < 0) { - close(configfd); + VIR_FORCE_CLOSE(configfd); virReportOOMError(); goto no_memory; } if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { VIR_FREE(configfd_name); - close(configfd); + VIR_FORCE_CLOSE(configfd); goto no_memory; } @@ -5215,7 +5213,7 @@ int qemudBuildCommandLine(virConnectPtr if (vmfds && *vmfds) { for (i = 0; i < *nvmfds; i++) - close((*vmfds)[i]); + VIR_FORCE_CLOSE((*vmfds)[i]); VIR_FREE(*vmfds); *nvmfds = 0; } Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -81,6 +81,7 @@ #include "hooks.h" #include "storage_file.h" #include "virtaudit.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -767,7 +768,7 @@ qemudLogFD(struct qemud_driver *driver, if (virSetCloseExec(fd) < 0) { virReportSystemError(errno, "%s", _("Unable to set VM logfile close-on-exec flag")); - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } return fd; @@ -799,14 +800,14 @@ qemudLogReadFD(const char* logDir, const if (virSetCloseExec(fd) < 0) { virReportSystemError(errno, "%s", _("Unable to set VM logfile close-on-exec flag")); - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } if (pos < 0 || lseek(fd, pos, SEEK_SET) < 0) { - virReportSystemError(pos < 0 ? 0 : errno, + virReportSystemError(pos < 0 ? 0 : errno, _("Unable to seek to %lld in %s"), (long long) pos, logfile); - close(fd); + VIR_FORCE_CLOSE(fd); } return fd; } @@ -2394,7 +2395,7 @@ cleanup: } closelog: - if (close(logfd) < 0) { + if (VIR_CLOSE(logfd) < 0) { char ebuf[4096]; VIR_WARN("Unable to close logfile: %s", virStrerror(errno, ebuf, sizeof ebuf)); @@ -2973,13 +2974,13 @@ static int qemudNextFreeVNCPort(struct q return -1; if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)&reuse, sizeof(reuse)) < 0) { - close(fd); + VIR_FORCE_CLOSE(fd); break; } if (bind(fd, (struct sockaddr*)&addr, sizeof(addr)) == 0) { /* Not in use, lets grab it */ - close(fd); + VIR_FORCE_CLOSE(fd); /* Add port to bitmap of reserved ports */ if (virBitmapSetBit(driver->reservedVNCPorts, i - QEMU_VNC_PORT_MIN) < 0) { @@ -2988,7 +2989,7 @@ static int qemudNextFreeVNCPort(struct q } return i; } - close(fd); + VIR_FORCE_CLOSE(fd); if (errno == EADDRINUSE) { /* In use, try next */ @@ -3240,7 +3241,7 @@ qemuPrepareChardevDevice(virDomainDefPtr return -1; } - close(fd); + VIR_FORCE_CLOSE(fd); return 0; } @@ -4098,7 +4099,7 @@ static int qemudStartVMDaemon(virConnect if (vmfds) { for (i = 0 ; i < nvmfds ; i++) { - close(vmfds[i]); + VIR_FORCE_CLOSE(vmfds[i]); } VIR_FREE(vmfds); } @@ -4151,8 +4152,7 @@ static int qemudStartVMDaemon(virConnect if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto cleanup; - if (logfile != -1) - close(logfile); + VIR_FORCE_CLOSE(logfile); return 0; @@ -4162,8 +4162,7 @@ cleanup: * pretend we never started it */ qemudShutdownVMDaemon(driver, vm, 0); - if (logfile != -1) - close(logfile); + VIR_FORCE_CLOSE(logfile); return -1; } @@ -4438,7 +4437,7 @@ static int kvmGetMaxVCPUs(void) { if (r > 0) maxvcpus = r; - close(fd); + VIR_FORCE_CLOSE(fd); return maxvcpus; } @@ -5543,10 +5542,10 @@ static int qemudDomainSaveFlag(struct qe goto endjob; } if (qemudDomainSaveFileOpHook(fd, &hdata) < 0) { - close(fd); + VIR_FORCE_CLOSE(fd); goto endjob; } - if (close(fd) < 0) { + if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("unable to close %s"), path); goto endjob; } @@ -5982,7 +5981,7 @@ static int qemudDomainCoreDump(virDomain goto endjob; } - if (close(fd) < 0) { + if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("unable to save file %s"), path); @@ -6627,8 +6626,7 @@ static int qemudOpenAsUID(const char *pa /* parent */ /* parent doesn't need the write side of the pipe */ - close(pipefd[1]); - pipefd[1] = -1; + VIR_FORCE_CLOSE(pipefd[1]); if (forkRet < 0) { virReportSystemError(errno, @@ -6640,10 +6638,8 @@ static int qemudOpenAsUID(const char *pa fd = pipefd[0]; pipefd[0] = -1; parent_cleanup: - if (pipefd[0] != -1) - close(pipefd[0]); - if (pipefd[1] != -1) - close(pipefd[1]); + VIR_FORCE_CLOSE(pipefd[0]); + VIR_FORCE_CLOSE(pipefd[1]); if ((fd < 0) && (*child_pid > 0)) { /* a child process was started and subsequently an error occurred in the parent, so we need to wait for it to @@ -6669,7 +6665,7 @@ parent_cleanup: struct passwd pwd, *pwd_result; /* child doesn't need the read side of the pipe */ - close(pipefd[0]); + VIR_FORCE_CLOSE(pipefd[0]); if (forkRet < 0) { exit_code = errno; @@ -6734,10 +6730,8 @@ parent_cleanup: child_cleanup: VIR_FREE(buf); - if (fd != -1) - close(fd); - if (pipefd[1] != -1) - close(pipefd[1]); + VIR_FORCE_CLOSE(fd); + VIR_FORCE_CLOSE(pipefd[1]); _exit(exit_code); } @@ -6745,8 +6739,10 @@ static int qemudDomainSaveImageClose(int { int ret = 0; - if (fd != -1) - close(fd); + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, "%s", + _("cannot close file")); + } if (read_pid != -1) { /* reap the process that read the file */ @@ -6902,8 +6898,7 @@ qemudDomainSaveImageStartVM(virConnectPt /* empty */ } } - if (intermediatefd != -1) - close(intermediatefd); + VIR_FORCE_CLOSE(intermediatefd); wait_ret = qemudDomainSaveImageClose(fd, read_pid, &status); fd = -1; @@ -8282,9 +8277,7 @@ static int qemudDomainAttachNetDevice(vi } qemuDomainObjExitMonitorWithDriver(driver, vm); - if (tapfd != -1) - close(tapfd); - tapfd = -1; + VIR_FORCE_CLOSE(tapfd); if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -8338,8 +8331,7 @@ cleanup: VIR_FREE(nicstr); VIR_FREE(netstr); VIR_FREE(tapfd_name); - if (tapfd != -1) - close(tapfd); + VIR_FORCE_CLOSE(tapfd); return ret; @@ -8468,8 +8460,7 @@ static int qemudDomainAttachHostPciDevic VIR_FREE(devstr); VIR_FREE(configfd_name); - if (configfd >= 0) - close(configfd); + VIR_FORCE_CLOSE(configfd); return 0; @@ -8483,8 +8474,7 @@ error: VIR_FREE(devstr); VIR_FREE(configfd_name); - if (configfd >= 0) - close(configfd); + VIR_FORCE_CLOSE(configfd); return -1; } @@ -10346,8 +10336,7 @@ qemudDomainBlockPeek (virDomainPtr dom, } cleanup: - if (fd >= 0) - close (fd); + VIR_FORCE_CLOSE(fd); if (vm) virDomainObjUnlock(vm); return ret; @@ -10434,7 +10423,7 @@ endjob: cleanup: VIR_FREE(tmp); - if (fd >= 0) close (fd); + VIR_FORCE_CLOSE(fd); unlink (tmp); if (vm) virDomainObjUnlock(vm); @@ -10590,8 +10579,7 @@ static int qemuDomainGetBlockInfo(virDom } cleanup: - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); if (vm) virDomainObjUnlock(vm); return ret; @@ -10906,8 +10894,7 @@ cleanup: static void qemuStreamMigFree(struct qemuStreamMigFile *qemust) { - if (qemust->fd != -1) - close(qemust->fd); + VIR_FORCE_CLOSE(qemust->fd); VIR_FREE(qemust); } @@ -11748,10 +11735,8 @@ finish: qemuDomainObjExitRemoteWithDriver(driver, vm); cleanup: - if (client_sock != -1) - close(client_sock); - if (qemu_sock != -1) - close(qemu_sock); + VIR_FORCE_CLOSE(client_sock); + VIR_FORCE_CLOSE(qemu_sock); if (ddomain) virUnrefDomain(ddomain); @@ -12532,8 +12517,7 @@ cleanup: VIR_FREE(snapFile); VIR_FREE(snapDir); VIR_FREE(newxml); - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); return ret; } Index: libvirt-acl/src/qemu/qemu_monitor.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_monitor.c +++ libvirt-acl/src/qemu/qemu_monitor.c @@ -36,6 +36,7 @@ #include "virterror_internal.h" #include "memory.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -283,7 +284,7 @@ qemuMonitorOpenUnix(const char *monitor) return monfd; error: - close(monfd); + VIR_FORCE_CLOSE(monfd); return -1; } Index: libvirt-acl/src/remote/remote_driver.c =================================================================== --- libvirt-acl.orig/src/remote/remote_driver.c +++ libvirt-acl/src/remote/remote_driver.c @@ -82,6 +82,7 @@ #include "util.h" #include "event.h" #include "ignore-value.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_REMOTE @@ -621,7 +622,7 @@ doRemoteOpen (virConnectPtr conn, if (connect (priv->sock, r->ai_addr, r->ai_addrlen) == -1) { saved_errno = errno; - close (priv->sock); + VIR_FORCE_CLOSE(priv->sock); continue; } @@ -630,8 +631,7 @@ doRemoteOpen (virConnectPtr conn, negotiate_gnutls_on_connection (conn, priv, no_verify); if (!priv->session) { - close (priv->sock); - priv->sock = -1; + VIR_FORCE_CLOSE(priv->sock); goto failed; } } @@ -711,8 +711,7 @@ doRemoteOpen (virConnectPtr conn, if (errno == ECONNREFUSED && flags & VIR_DRV_OPEN_REMOTE_AUTOSTART && trials < 20) { - close(priv->sock); - priv->sock = -1; + VIR_FORCE_CLOSE(priv->sock); if (trials > 0 || remoteForkDaemon() == 0) { trials++; @@ -806,8 +805,8 @@ doRemoteOpen (virConnectPtr conn, goto failed; /* Parent continues here. */ - close (sv[1]); - close (errfd[1]); + VIR_FORCE_CLOSE(sv[1]); + VIR_FORCE_CLOSE(errfd[1]); priv->sock = sv[0]; priv->errfd = errfd[0]; priv->pid = pid; @@ -955,15 +954,14 @@ doRemoteOpen (virConnectPtr conn, failed: /* Close the socket if we failed. */ - if (priv->errfd >= 0) - close(priv->errfd); + VIR_FORCE_CLOSE(priv->errfd); if (priv->sock >= 0) { if (priv->uses_tls && priv->session) { gnutls_bye (priv->session, GNUTLS_SHUT_RDWR); gnutls_deinit (priv->session); } - close (priv->sock); + VIR_FORCE_CLOSE(priv->sock); #ifndef WIN32 if (priv->pid > 0) { pid_t reap; @@ -977,10 +975,8 @@ retry: #endif } - if (wakeupFD[0] >= 0) { - close(wakeupFD[0]); - close(wakeupFD[1]); - } + VIR_FORCE_CLOSE(wakeupFD[0]); + VIR_FORCE_CLOSE(wakeupFD[1]); VIR_FREE(priv->hostname); goto cleanup; @@ -1442,8 +1438,8 @@ doRemoteClose (virConnectPtr conn, struc if (priv->saslconn) sasl_dispose (&priv->saslconn); #endif - close (priv->sock); - close (priv->errfd); + VIR_FORCE_CLOSE(priv->sock); + VIR_FORCE_CLOSE(priv->errfd); #ifndef WIN32 if (priv->pid > 0) { @@ -1456,10 +1452,8 @@ retry: } while (reap != -1 && reap != priv->pid); } #endif - if (priv->wakeupReadFD >= 0) { - close(priv->wakeupReadFD); - close(priv->wakeupSendFD); - } + VIR_FORCE_CLOSE(priv->wakeupReadFD); + VIR_FORCE_CLOSE(priv->wakeupSendFD); /* Free hostname copy */ Index: libvirt-acl/src/secret/secret_driver.c =================================================================== --- libvirt-acl.orig/src/secret/secret_driver.c +++ libvirt-acl/src/secret/secret_driver.c @@ -41,6 +41,7 @@ #include "util.h" #include "uuid.h" #include "virterror_internal.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_SECRET @@ -181,7 +182,7 @@ replaceFile(const char *filename, void * tmp_path); goto cleanup; } - if (close(fd) < 0) { + if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("error closing '%s'"), tmp_path); goto cleanup; } @@ -196,8 +197,7 @@ replaceFile(const char *filename, void * ret = 0; cleanup: - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); if (tmp_path != NULL) { unlink(tmp_path); VIR_FREE(tmp_path); @@ -394,8 +394,7 @@ secretLoadValue(virSecretDriverStatePtr virReportSystemError(errno, _("cannot read '%s'"), filename); goto cleanup; } - close(fd); - fd = -1; + VIR_FORCE_CLOSE(fd); if (!base64_decode_alloc(contents, st.st_size, &value, &value_size)) { virSecretReportError(VIR_ERR_INTERNAL_ERROR, @@ -422,8 +421,7 @@ cleanup: memset(contents, 0, st.st_size); VIR_FREE(contents); } - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); VIR_FREE(filename); return ret; } Index: libvirt-acl/src/security/security_apparmor.c =================================================================== --- libvirt-acl.orig/src/security/security_apparmor.c +++ libvirt-acl/src/security/security_apparmor.c @@ -37,6 +37,7 @@ #include "uuid.h" #include "pci.h" #include "hostusb.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_SECURITY #define SECURITY_APPARMOR_VOID_DOI "0" @@ -215,7 +216,7 @@ load_profile(virSecurityDriverPtr drv, virReportSystemError(errno, "%s", _("unable to write to pipe")); goto clean; } - close(pipefd[1]); + VIR_FORCE_CLOSE(pipefd[1]); rc = 0; rewait: @@ -233,10 +234,8 @@ load_profile(virSecurityDriverPtr drv, clean: VIR_FREE(xml); - if (pipefd[0] > 0) - close(pipefd[0]); - if (pipefd[1] > 0) - close(pipefd[1]); + VIR_FORCE_CLOSE(pipefd[0]); + VIR_FORCE_CLOSE(pipefd[1]); return rc; } Index: libvirt-acl/src/security/security_selinux.c =================================================================== --- libvirt-acl.orig/src/security/security_selinux.c +++ libvirt-acl/src/security/security_selinux.c @@ -28,6 +28,7 @@ #include "pci.h" #include "hostusb.h" #include "storage_file.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_SECURITY @@ -120,10 +121,10 @@ SELinuxInitialize(void) virReportSystemError(errno, _("cannot read SELinux virtual domain context file %s"), selinux_virtual_domain_context_path()); - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } - close(fd); + VIR_FORCE_CLOSE(fd); ptr = strchrnul(default_domain_context, '\n'); *ptr = '\0'; @@ -139,10 +140,10 @@ SELinuxInitialize(void) virReportSystemError(errno, _("cannot read SELinux virtual image context file %s"), selinux_virtual_image_context_path()); - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } - close(fd); + VIR_FORCE_CLOSE(fd); ptr = strchrnul(default_image_context, '\n'); if (*ptr == '\n') { Index: libvirt-acl/src/security/virt-aa-helper.c =================================================================== --- libvirt-acl.orig/src/security/virt-aa-helper.c +++ libvirt-acl/src/security/virt-aa-helper.c @@ -37,6 +37,7 @@ #include "uuid.h" #include "hostusb.h" #include "pci.h" +#include "files.h" static char *progname; @@ -278,12 +279,12 @@ update_include_file(const char *include_ } if (safewrite(fd, pcontent, plen) < 0) { /* don't write the '\0' */ - close(fd); + VIR_FORCE_CLOSE(fd); vah_error(NULL, 0, "failed to write to profile"); goto clean; } - if (close(fd) != 0) { + if (VIR_CLOSE(fd) != 0) { vah_error(NULL, 0, "failed to close or write to profile"); goto clean; } @@ -385,12 +386,12 @@ create_profile(const char *profile, cons } if (safewrite(fd, pcontent, plen - 1) < 0) { /* don't write the '\0' */ - close(fd); + VIR_FORCE_CLOSE(fd); vah_error(NULL, 0, "failed to write to profile"); goto clean_all; } - if (close(fd) != 0) { + if (VIR_CLOSE(fd) != 0) { vah_error(NULL, 0, "failed to close or write to profile"); goto clean_all; } Index: libvirt-acl/src/storage/storage_backend.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend.c +++ libvirt-acl/src/storage/storage_backend.c @@ -51,6 +51,7 @@ #include "storage_file.h" #include "storage_backend.h" #include "logging.h" +#include "files.h" #if WITH_STORAGE_LVM # include "storage_backend_logical.h" @@ -181,7 +182,7 @@ virStorageBackendCopyToFD(virStorageVolD } while ((amtleft -= 512) > 0); } - if (inputfd != -1 && close(inputfd) < 0) { + if (VIR_CLOSE(inputfd) < 0) { ret = -errno; virReportSystemError(errno, _("cannot close file '%s'"), @@ -193,8 +194,7 @@ virStorageBackendCopyToFD(virStorageVolD *total -= remain; cleanup: - if (inputfd != -1) - close(inputfd); + VIR_FORCE_CLOSE(inputfd); VIR_FREE(buf); @@ -251,7 +251,7 @@ virStorageBackendCreateBlockFrom(virConn vol->target.path, vol->target.perms.mode); goto cleanup; } - if (close(fd) < 0) { + if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("cannot close file '%s'"), vol->target.path); @@ -261,8 +261,7 @@ virStorageBackendCreateBlockFrom(virConn ret = 0; cleanup: - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); return ret; } @@ -608,7 +607,7 @@ static int virStorageBackendQEMUImgBacki cleanup: VIR_FREE(help); - close(newstdout); + VIR_FORCE_CLOSE(newstdout); rewait: if (child) { if (waitpid(child, &status, 0) != child) { @@ -997,7 +996,7 @@ virStorageBackendVolOpenCheckMode(const virReportSystemError(errno, _("cannot stat file '%s'"), path); - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } @@ -1009,7 +1008,7 @@ virStorageBackendVolOpenCheckMode(const mode = VIR_STORAGE_VOL_OPEN_BLOCK; if (!(mode & flags)) { - close(fd); + VIR_FORCE_CLOSE(fd); if (mode & VIR_STORAGE_VOL_OPEN_ERROR) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, @@ -1045,7 +1044,7 @@ virStorageBackendUpdateVolTargetInfo(vir allocation, capacity); - close(fd); + VIR_FORCE_CLOSE(fd); return ret; } @@ -1461,10 +1460,8 @@ virStorageBackendRunProgRegex(virStorage if (list) fclose(list); - else { - if (fd >= 0) - close(fd); - } + else + VIR_FORCE_CLOSE(fd); while ((err = waitpid(child, &exitstatus, 0) == -1) && errno == EINTR); @@ -1579,7 +1576,7 @@ virStorageBackendRunProgNul(virStoragePo if (fp) fclose (fp); else - close (fd); + VIR_FORCE_CLOSE(fd); while ((w_err = waitpid (child, &exitstatus, 0) == -1) && errno == EINTR) /* empty */ ; Index: libvirt-acl/src/storage/storage_backend_fs.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_fs.c +++ libvirt-acl/src/storage/storage_backend_fs.c @@ -45,6 +45,7 @@ #include "util.h" #include "memory.h" #include "xml.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -72,25 +73,25 @@ virStorageBackendProbeTarget(virStorageV if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, allocation, capacity)) < 0) { - close(fd); + VIR_FORCE_CLOSE(fd); return ret; } memset(&meta, 0, sizeof(meta)); if ((target->format = virStorageFileProbeFormatFromFD(target->path, fd)) < 0) { - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } if (virStorageFileGetMetadataFromFD(target->path, fd, target->format, &meta) < 0) { - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } - close(fd); + VIR_FORCE_CLOSE(fd); if (meta.backingStore) { *backingStore = meta.backingStore; @@ -98,7 +99,7 @@ virStorageBackendProbeTarget(virStorageV if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) { if ((*backingStoreFormat = virStorageFileProbeFormat(*backingStore)) < 0) { - close(fd); + VIR_FORCE_CLOSE(fd); goto cleanup; } } else { Index: libvirt-acl/src/storage/storage_backend_iscsi.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_iscsi.c +++ libvirt-acl/src/storage/storage_backend_iscsi.c @@ -41,6 +41,7 @@ #include "util.h" #include "memory.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -237,9 +238,7 @@ out: if (fp != NULL) { fclose(fp); } else { - if (fd != -1) { - close(fd); - } + VIR_FORCE_CLOSE(fd); } return ret; Index: libvirt-acl/src/storage/storage_backend_mpath.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_mpath.c +++ libvirt-acl/src/storage/storage_backend_mpath.c @@ -35,6 +35,7 @@ #include "storage_backend.h" #include "memory.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -61,9 +62,7 @@ virStorageBackendMpathUpdateVolTargetInf ret = 0; out: - if (fd != -1) { - close(fd); - } + VIR_FORCE_CLOSE(fd); return ret; } Index: libvirt-acl/src/storage/storage_backend_scsi.c =================================================================== --- libvirt-acl.orig/src/storage/storage_backend_scsi.c +++ libvirt-acl/src/storage/storage_backend_scsi.c @@ -32,6 +32,7 @@ #include "storage_backend_scsi.h" #include "memory.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -154,8 +155,7 @@ virStorageBackendSCSIUpdateVolTargetInfo ret = 0; cleanup: - if (fd >= 0) - close(fd); + VIR_FORCE_CLOSE(fd); return ret; } @@ -572,14 +572,14 @@ virStorageBackendSCSITriggerRescan(uint3 if (safewrite(fd, LINUX_SYSFS_SCSI_HOST_SCAN_STRING, sizeof(LINUX_SYSFS_SCSI_HOST_SCAN_STRING)) < 0) { - + VIR_FORCE_CLOSE(fd); virReportSystemError(errno, _("Write to '%s' to trigger host scan failed"), path); retval = -1; } - close(fd); + VIR_FORCE_CLOSE(fd); free_path: VIR_FREE(path); out: Index: libvirt-acl/src/storage/storage_driver.c =================================================================== --- libvirt-acl.orig/src/storage/storage_driver.c +++ libvirt-acl/src/storage/storage_driver.c @@ -45,6 +45,7 @@ #include "memory.h" #include "storage_backend.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -1664,9 +1665,7 @@ storageVolumeWipeInternal(virStorageVolD out: VIR_FREE(writebuf); - if (fd != -1) { - close(fd); - } + VIR_FORCE_CLOSE(fd); return ret; } Index: libvirt-acl/src/test/test_driver.c =================================================================== --- libvirt-acl.orig/src/test/test_driver.c +++ libvirt-acl/src/test/test_driver.c @@ -50,6 +50,7 @@ #include "xml.h" #include "threads.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_TEST @@ -788,8 +789,7 @@ static int testOpenFromFile(virConnectPt _("Invalid XML in file '%s'"), file); goto error; } - close(fd); - fd = -1; + VIR_FORCE_CLOSE(fd); root = xmlDocGetRootElement(xml); if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "node"))) { @@ -1101,8 +1101,7 @@ static int testOpenFromFile(virConnectPt VIR_FREE(networks); VIR_FREE(ifaces); VIR_FREE(pools); - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); virDomainObjListDeinit(&privconn->domains); virNetworkObjListFree(&privconn->networks); virInterfaceObjListFree(&privconn->ifaces); @@ -1752,7 +1751,7 @@ static int testDomainSave(virDomainPtr d goto cleanup; } - if (close(fd) < 0) { + if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("saving domain '%s' to '%s': write failed"), domain->name, path); @@ -1779,8 +1778,7 @@ cleanup: * in either case we're already in a failure scenario * and have reported a earlier error */ if (ret != 0) { - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); unlink(path); } if (privdom) @@ -1870,8 +1868,7 @@ static int testDomainRestore(virConnectP cleanup: virDomainDefFree(def); VIR_FREE(xml); - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); if (dom) virDomainObjUnlock(dom); if (event) @@ -1911,7 +1908,7 @@ static int testDomainCoreDump(virDomainP domain->name, to); goto cleanup; } - if (close(fd) < 0) { + if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("domain '%s' coredump: write failed: %s"), domain->name, to); @@ -1932,8 +1929,7 @@ static int testDomainCoreDump(virDomainP ret = 0; cleanup: - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); if (privdom) virDomainObjUnlock(privdom); if (event) Index: libvirt-acl/src/uml/uml_conf.c =================================================================== --- libvirt-acl.orig/src/uml/uml_conf.c +++ libvirt-acl/src/uml/uml_conf.c @@ -47,6 +47,7 @@ #include "bridge.h" #include "logging.h" #include "domain_nwfilter.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_UML @@ -367,7 +368,7 @@ umlBuildCommandLineChr(virDomainChrDefPt } if (virAsprintf(&ret, "%s%d=null,fd:%d", dev, def->target.port, fd_out) < 0) { virReportOOMError(); - close(fd_out); + VIR_FORCE_CLOSE(fd_out); return NULL; } FD_SET(fd_out, keepfd); Index: libvirt-acl/src/uml/uml_driver.c =================================================================== --- libvirt-acl.orig/src/uml/uml_driver.c +++ libvirt-acl/src/uml/uml_driver.c @@ -59,6 +59,7 @@ #include "datatypes.h" #include "logging.h" #include "domain_nwfilter.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_UML @@ -533,7 +534,7 @@ umlShutdown(void) { umlDriverLock(uml_driver); if (uml_driver->inotifyWatch != -1) virEventRemoveHandle(uml_driver->inotifyWatch); - close(uml_driver->inotifyFD); + VIR_FORCE_CLOSE(uml_driver->inotifyFD); virCapabilitiesFree(uml_driver->caps); /* shutdown active VMs @@ -659,8 +660,7 @@ restat: if (bind(priv->monitor, (struct sockaddr *)&addr, sizeof addr) < 0) { virReportSystemError(errno, "%s", _("cannot bind socket")); - close(priv->monitor); - priv->monitor = -1; + VIR_FORCE_CLOSE(priv->monitor); return -1; } @@ -870,13 +870,13 @@ static int umlStartVMDaemon(virConnectPt if (umlSetCloseExec(logfd) < 0) { virReportSystemError(errno, "%s", _("Unable to set VM logfile close-on-exec flag")); - close(logfd); + VIR_FORCE_CLOSE(logfd); return -1; } if (umlBuildCommandLine(conn, driver, vm, &keepfd, &argv, &progenv) < 0) { - close(logfd); + VIR_FORCE_CLOSE(logfd); virDomainConfVMNWFilterTeardown(vm); umlCleanupTapDevices(conn, vm); return -1; @@ -912,15 +912,17 @@ static int umlStartVMDaemon(virConnectPt -1, &logfd, &logfd, VIR_EXEC_CLEAR_CAPS, NULL, NULL, NULL); - close(logfd); + VIR_FORCE_CLOSE(logfd); /* * At the moment, the only thing that populates keepfd is * umlBuildCommandLineChr. We want to close every fd it opens. */ for (i = 0; i < FD_SETSIZE; i++) - if (FD_ISSET(i, &keepfd)) - close(i); + if (FD_ISSET(i, &keepfd)) { + int tmpfd = i; + VIR_FORCE_CLOSE(tmpfd); + } for (i = 0 ; argv[i] ; i++) VIR_FREE(argv[i]); @@ -957,9 +959,7 @@ static void umlShutdownVMDaemon(virConne virKillProcess(vm->pid, SIGTERM); - if (priv->monitor != -1) - close(priv->monitor); - priv->monitor = -1; + VIR_FORCE_CLOSE(priv->monitor); if ((ret = waitpid(vm->pid, NULL, 0)) != vm->pid) { VIR_WARN("Got unexpected pid %d != %d", @@ -2088,7 +2088,7 @@ umlDomainBlockPeek (virDomainPtr dom, } cleanup: - if (fd >= 0) close (fd); + VIR_FORCE_CLOSE(fd); if (vm) virDomainObjUnlock(vm); return ret; Index: libvirt-acl/src/util/bridge.c =================================================================== --- libvirt-acl.orig/src/util/bridge.c +++ libvirt-acl/src/util/bridge.c @@ -24,6 +24,7 @@ #if defined(WITH_BRIDGE) # include "bridge.h" +# include "files.h" # include <stdlib.h> # include <stdio.h> @@ -82,12 +83,12 @@ brInit(brControl **ctlp) if ((flags = fcntl(fd, F_GETFD)) < 0 || fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0) { int err = errno; - close(fd); + VIR_FORCE_CLOSE(fd); return err; } if (VIR_ALLOC(*ctlp) < 0) { - close(fd); + VIR_FORCE_CLOSE(fd); return ENOMEM; } @@ -108,8 +109,7 @@ brShutdown(brControl *ctl) if (!ctl) return; - close(ctl->fd); - ctl->fd = 0; + VIR_FORCE_CLOSE(ctl->fd); VIR_FREE(ctl); } @@ -540,11 +540,11 @@ brAddTap(brControl *ctl, if (tapfd) *tapfd = fd; else - close(fd); + VIR_FORCE_CLOSE(fd); return 0; error: - close(fd); + VIR_FORCE_CLOSE(fd); return errno; } @@ -575,7 +575,7 @@ int brDeleteTap(brControl *ctl, } error: - close(fd); + VIR_FORCE_CLOSE(fd); return errno; } Index: libvirt-acl/src/util/conf.c =================================================================== --- libvirt-acl.orig/src/util/conf.c +++ libvirt-acl/src/util/conf.c @@ -24,6 +24,7 @@ #include "util.h" #include "c-ctype.h" #include "memory.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_CONF @@ -954,7 +955,7 @@ virConfWriteFile(const char *filename, v content = virBufferContentAndReset(&buf); ret = safewrite(fd, content, use); VIR_FREE(content); - close(fd); + VIR_FORCE_CLOSE(fd); if (ret != (int)use) { virConfError(NULL, VIR_ERR_WRITE_FAILED, _("failed to save content")); return -1; Index: libvirt-acl/src/util/interface.c =================================================================== --- libvirt-acl.orig/src/util/interface.c +++ libvirt-acl/src/util/interface.c @@ -39,6 +39,7 @@ #include "util.h" #include "interface.h" #include "virterror_internal.h" +#include "files.h" #define ifaceError(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_NET, code, __FILE__, \ @@ -82,7 +83,7 @@ ifaceGetFlags(const char *ifname, short *flags = ifr.ifr_flags; - close(fd); + VIR_FORCE_CLOSE(fd); return rc; } @@ -161,7 +162,7 @@ static int chgIfaceFlags(const char *ifn } err_exit: - close(fd); + VIR_FORCE_CLOSE(fd); return rc; } @@ -259,8 +260,7 @@ ifaceCheck(bool reportError, const char } err_exit: - if (fd >= 0) - close(fd); + VIR_FORCE_CLOSE(fd); return rc; } @@ -326,7 +326,7 @@ ifaceGetIndex(bool reportError, const ch } err_exit: - close(fd); + VIR_FORCE_CLOSE(fd); return rc; } @@ -373,7 +373,7 @@ ifaceGetVlanID(const char *vlanifname, i *vlanid = vlanargs.u.VID; err_exit: - close(fd); + VIR_FORCE_CLOSE(fd); return rc; } Index: libvirt-acl/src/util/logging.c =================================================================== --- libvirt-acl.orig/src/util/logging.c +++ libvirt-acl/src/util/logging.c @@ -40,6 +40,7 @@ #include "util.h" #include "buf.h" #include "threads.h" +#include "files.h" /* * Macro used to format the message as a string in virLogMessage @@ -603,8 +604,7 @@ static int virLogOutputToFd(const char * static void virLogCloseFd(void *data) { int fd = (long) data; - if (fd >= 0) - close(fd); + VIR_FORCE_CLOSE(fd); } static int virLogAddOutputToStderr(int priority) { @@ -622,7 +622,7 @@ static int virLogAddOutputToFile(int pri return(-1); if (virLogDefineOutput(virLogOutputToFd, virLogCloseFd, (void *)(long)fd, priority, VIR_LOG_TO_FILE, file, 0) < 0) { - close(fd); + VIR_FORCE_CLOSE(fd); return(-1); } return(0); Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/src/util/macvtap.c @@ -52,6 +52,7 @@ # include "conf/domain_conf.h" # include "virterror_internal.h" # include "uuid.h" +# include "files.h" # define VIR_FROM_THIS VIR_FROM_NET @@ -92,12 +93,6 @@ static int nlOpen(void) } -static void nlClose(int fd) -{ - close(fd); -} - - /** * nlComm: * @nlmsg: pointer to netlink message @@ -191,7 +186,7 @@ err_exit: *respbuflen = 0; } - nlClose(fd); + VIR_FORCE_CLOSE(fd); return rc; } @@ -689,8 +684,7 @@ create_name: if (rc >= 0) { if (configMacvtapTap(rc, vnet_hdr) < 0) { - close(rc); - rc = -1; + VIR_FORCE_CLOSE(rc); /* sets rc to -1 */ goto disassociate_exit; } *res_ifname = strdup(cr_ifname); @@ -778,8 +772,7 @@ getLldpadPid(void) { _("Error opening file %s"), LLDPAD_PID_FILE); } - if (fd >= 0) - close(fd); + VIR_FORCE_CLOSE(fd); return pid; } Index: libvirt-acl/src/util/pci.c =================================================================== --- libvirt-acl.orig/src/util/pci.c +++ libvirt-acl/src/util/pci.c @@ -37,6 +37,7 @@ #include "memory.h" #include "util.h" #include "virterror_internal.h" +#include "files.h" /* avoid compilation breakage on some systems */ #ifndef MODPROBE @@ -188,10 +189,7 @@ pciCloseConfig(pciDevice *dev) if (!dev) return; - if (dev->fd >= 0) { - close(dev->fd); - dev->fd = -1; - } + VIR_FORCE_CLOSE(dev->fd); } static int Index: libvirt-acl/src/util/storage_file.c =================================================================== --- libvirt-acl.orig/src/util/storage_file.c +++ libvirt-acl/src/util/storage_file.c @@ -36,6 +36,7 @@ #include "memory.h" #include "virterror_internal.h" #include "logging.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -688,7 +689,7 @@ virStorageFileProbeFormat(const char *pa ret = virStorageFileProbeFormatFromFD(path, fd); - close(fd); + VIR_FORCE_CLOSE(fd); return ret; } @@ -782,7 +783,7 @@ virStorageFileGetMetadata(const char *pa ret = virStorageFileGetMetadataFromFD(path, fd, format, meta); - close(fd); + VIR_FORCE_CLOSE(fd); return ret; } Index: libvirt-acl/src/util/util.c =================================================================== --- libvirt-acl.orig/src/util/util.c +++ libvirt-acl/src/util/util.c @@ -71,6 +71,7 @@ #include "memory.h" #include "threads.h" #include "verify.h" +#include "files.h" #ifndef NSIG # define NSIG 32 @@ -461,6 +462,7 @@ __virExec(const char *const*argv, int pipeerr[2] = {-1,-1}; int childout = -1; int childerr = -1; + int tmpfd; if ((null = open("/dev/null", O_RDONLY)) < 0) { virReportSystemError(errno, @@ -534,13 +536,13 @@ __virExec(const char *const*argv, } if (pid) { /* parent */ - close(null); + VIR_FORCE_CLOSE(null); if (outfd && *outfd == -1) { - close(pipeout[1]); + VIR_FORCE_CLOSE(pipeout[1]); *outfd = pipeout[0]; } if (errfd && *errfd == -1) { - close(pipeerr[1]); + VIR_FORCE_CLOSE(pipeerr[1]); *errfd = pipeerr[0]; } @@ -568,8 +570,10 @@ __virExec(const char *const*argv, i != childout && i != childerr && (!keepfd || - !FD_ISSET(i, keepfd))) - close(i); + !FD_ISSET(i, keepfd))) { + tmpfd = i; + VIR_FORCE_CLOSE(tmpfd); + } if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0) { virReportSystemError(errno, @@ -589,14 +593,15 @@ __virExec(const char *const*argv, goto fork_error; } - if (infd > 0) - close(infd); - close(null); - if (childout > 0) - close(childout); + VIR_FORCE_CLOSE(infd); + VIR_FORCE_CLOSE(null); + tmpfd = childout; /* preserve childout value */ + VIR_FORCE_CLOSE(tmpfd); if (childerr > 0 && - childerr != childout) - close(childerr); + childerr != childout) { + VIR_FORCE_CLOSE(childerr); + childout = -1; + } /* Daemonize as late as possible, so the parent process can detect * the above errors with wait* */ @@ -666,16 +671,11 @@ __virExec(const char *const*argv, /* NB we don't virUtilError() on any failures here because the code which jumped hre already raised an error condition which we must not overwrite */ - if (pipeerr[0] > 0) - close(pipeerr[0]); - if (pipeerr[1] > 0) - close(pipeerr[1]); - if (pipeout[0] > 0) - close(pipeout[0]); - if (pipeout[1] > 0) - close(pipeout[1]); - if (null > 0) - close(null); + VIR_FORCE_CLOSE(pipeerr[0]); + VIR_FORCE_CLOSE(pipeerr[1]); + VIR_FORCE_CLOSE(pipeout[0]); + VIR_FORCE_CLOSE(pipeout[1]); + VIR_FORCE_CLOSE(null); return -1; } @@ -865,10 +865,8 @@ virRunWithHook(const char *const*argv, VIR_FREE(outbuf); VIR_FREE(errbuf); VIR_FREE(argv_str); - if (outfd != -1) - close(outfd); - if (errfd != -1) - close(errfd); + VIR_FORCE_CLOSE(outfd); + VIR_FORCE_CLOSE(errfd); return ret; } @@ -1108,7 +1106,7 @@ int virFileReadAll(const char *path, int } int len = virFileReadLimFD(fd, maxlen, buf); - close(fd); + VIR_FORCE_CLOSE(fd); if (len < 0) { virReportSystemError(errno, _("Failed to read file '%s'"), path); return -1; @@ -1129,13 +1127,13 @@ int virFileWriteStr(const char *path, co if (safewrite(fd, str, strlen(str)) < 0) { int saved_errno = errno; - close (fd); + VIR_FORCE_CLOSE(fd); errno = saved_errno; return -1; } /* Use errno from failed close only if there was no write error. */ - if (close (fd) != 0) + if (VIR_CLOSE(fd) != 0) return -1; return 0; @@ -1314,7 +1312,7 @@ static int virFileOperationNoFork(const if ((hook) && ((ret = hook(fd, hookdata)) != 0)) { goto error; } - if (close(fd) < 0) { + if (VIR_CLOSE(fd) < 0) { ret = -errno; virReportSystemError(errno, _("failed to close new file '%s'"), path); @@ -1323,8 +1321,7 @@ static int virFileOperationNoFork(const } fd = -1; error: - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); return ret; } @@ -1475,7 +1472,7 @@ parenterror: if ((hook) && ((ret = hook(fd, hookdata)) != 0)) { goto childerror; } - if (close(fd) < 0) { + if (VIR_CLOSE(fd) < 0) { ret = -errno; virReportSystemError(errno, _("child failed to close new file '%s'"), path); @@ -1752,10 +1749,8 @@ int virFileOpenTtyAt(const char *ptmx, rc = 0; cleanup: - if (rc != 0 && - *ttymaster != -1) { - close(*ttymaster); - } + if (rc != 0) + VIR_FORCE_CLOSE(*ttymaster); return rc; @@ -1821,7 +1816,7 @@ int virFileWritePidPath(const char *pidf if (!(file = fdopen(fd, "w"))) { rc = errno; - close(fd); + VIR_FORCE_CLOSE(fd); goto cleanup; } Index: libvirt-acl/src/util/uuid.c =================================================================== --- libvirt-acl.orig/src/util/uuid.c +++ libvirt-acl/src/util/uuid.c @@ -39,6 +39,7 @@ #include "virterror_internal.h" #include "logging.h" #include "memory.h" +#include "files.h" #ifndef ENODATA # define ENODATA EIO @@ -61,7 +62,7 @@ virUUIDGenerateRandomBytes(unsigned char if ((n = read(fd, buf, buflen)) <= 0) { if (errno == EINTR) continue; - close(fd); + VIR_FORCE_CLOSE(fd); return n < 0 ? errno : ENODATA; } @@ -69,7 +70,7 @@ virUUIDGenerateRandomBytes(unsigned char buflen -= n; } - close(fd); + VIR_FORCE_CLOSE(fd); return 0; } @@ -240,10 +241,10 @@ getDMISystemUUID(char *uuid, int len) int fd = open(paths[i], O_RDONLY); if (fd > 0) { if (saferead(fd, uuid, len) == len) { - close(fd); + VIR_FORCE_CLOSE(fd); return 0; } - close(fd); + VIR_FORCE_CLOSE(fd); } i++; } Index: libvirt-acl/src/util/virtaudit.c =================================================================== --- libvirt-acl.orig/src/util/virtaudit.c +++ libvirt-acl/src/util/virtaudit.c @@ -31,6 +31,7 @@ #include "logging.h" #include "virtaudit.h" #include "util.h" +#include "files.h" /* Provide the macros in case the header file is old. FIXME: should be removed. */ @@ -134,7 +135,7 @@ void virAuditSend(const char *file ATTRI void virAuditClose(void) { #if HAVE_AUDIT - close(auditfd); + VIR_FORCE_CLOSE(auditfd); #endif } Index: libvirt-acl/src/xen/proxy_internal.c =================================================================== --- libvirt-acl.orig/src/xen/proxy_internal.c +++ libvirt-acl/src/xen/proxy_internal.c @@ -30,6 +30,7 @@ #include "util.h" #include "xen_driver.h" #include "memory.h" +#include "files.h" #define STANDALONE @@ -196,7 +197,7 @@ retry: addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } @@ -204,7 +205,7 @@ retry: * now bind the socket to that address and listen on it */ if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { - close(fd); + VIR_FORCE_CLOSE(fd); if (trials < 3) { if (virProxyForkServer() < 0) return(-1); @@ -232,16 +233,17 @@ retry: static int virProxyCloseSocket(xenUnifiedPrivatePtr priv) { int ret; + int tmpfd; if (priv->proxy < 0) return(-1); - ret = close(priv->proxy); + tmpfd = priv->proxy; + ret = VIR_CLOSE(priv->proxy); if (ret != 0) - VIR_WARN("Failed to close socket %d", priv->proxy); + VIR_WARN("Failed to close socket %d", tmpfd); else - VIR_DEBUG("Closed socket %d", priv->proxy); - priv->proxy = -1; + VIR_DEBUG("Closed socket %d", tmpfd); return(ret); } Index: libvirt-acl/src/xen/xen_hypervisor.c =================================================================== --- libvirt-acl.orig/src/xen/xen_hypervisor.c +++ libvirt-acl/src/xen/xen_hypervisor.c @@ -65,6 +65,7 @@ #include "buf.h" #include "capabilities.h" #include "memory.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_XEN @@ -2036,7 +2037,7 @@ xenHypervisorInit(void) hypervisor_version = -1; virXenError(VIR_ERR_XEN_CALL, " ioctl %lu", (unsigned long) IOCTL_PRIVCMD_HYPERCALL); - close(fd); + VIR_FORCE_CLOSE(fd); in_init = 0; return(-1); @@ -2122,13 +2123,13 @@ xenHypervisorInit(void) hypervisor_version = -1; virXenError(VIR_ERR_XEN_CALL, " ioctl %lu", (unsigned long)IOCTL_PRIVCMD_HYPERCALL); - close(fd); + VIR_FORCE_CLOSE(fd); in_init = 0; VIR_FREE(ipt); return(-1); done: - close(fd); + VIR_FORCE_CLOSE(fd); in_init = 0; VIR_FREE(ipt); return(0); @@ -2191,7 +2192,7 @@ xenHypervisorClose(virConnectPtr conn) if (priv->handle < 0) return -1; - ret = close(priv->handle); + ret = VIR_CLOSE(priv->handle); if (ret < 0) return (-1); @@ -2396,8 +2397,7 @@ get_cpu_flags(virConnectPtr conn, const ret = 1; out: - if (fd != -1) - close(fd); + VIR_FORCE_CLOSE(fd); return ret; } Index: libvirt-acl/src/xen/xen_inotify.c =================================================================== --- libvirt-acl.orig/src/xen/xen_inotify.c +++ libvirt-acl/src/xen/xen_inotify.c @@ -39,6 +39,7 @@ #include "xend_internal.h" #include "logging.h" #include "uuid.h" +#include "files.h" #include "xm_internal.h" /* for xenXMDomainConfigParse */ @@ -483,7 +484,7 @@ xenInotifyClose(virConnectPtr conn) if (priv->inotifyWatch != -1) virEventRemoveHandle(priv->inotifyWatch); - close(priv->inotifyFD); + VIR_FORCE_CLOSE(priv->inotifyFD); return 0; } Index: libvirt-acl/src/xen/xend_internal.c =================================================================== --- libvirt-acl.orig/src/xen/xend_internal.c +++ libvirt-acl/src/xen/xend_internal.c @@ -45,6 +45,7 @@ #include "xs_internal.h" /* To extract VNC port & Serial console TTY */ #include "memory.h" #include "count-one-bits.h" +#include "files.h" /* required for cpumap_t */ #include <xen/dom0_ops.h> @@ -118,7 +119,6 @@ static int do_connect(virConnectPtr xend) { int s; - int serrno; int no_slow_start = 1; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) xend->privateData; @@ -137,10 +137,7 @@ do_connect(virConnectPtr xend) if (connect(s, (struct sockaddr *)&priv->addr, priv->addrlen) == -1) { - serrno = errno; - close(s); - errno = serrno; - s = -1; + VIR_FORCE_CLOSE(s); /* preserves errno */ /* * Connecting to XenD when privileged is mandatory, so log this @@ -387,7 +384,7 @@ xend_get(virConnectPtr xend, const char "Content-Type: application/x-www-form-urlencoded\r\n" "\r\n"); ret = xend_req(s, content); - close(s); + VIR_FORCE_CLOSE(s); if (((ret < 0) || (ret >= 300)) && ((ret != 404) || (!STRPREFIX(path, "/xend/domain/")))) { @@ -437,7 +434,7 @@ xend_post(virConnectPtr xend, const char swrites(s, ops); ret = xend_req(s, &err_buf); - close(s); + VIR_FORCE_CLOSE(s); if ((ret < 0) || (ret >= 300)) { virXendError(VIR_ERR_POST_FAILED, @@ -833,7 +830,7 @@ xenDaemonOpen_tcp(virConnectPtr conn, co if (connect (sock, r->ai_addr, r->ai_addrlen) == -1) { saved_errno = errno; - close (sock); + VIR_FORCE_CLOSE(sock); continue; } @@ -843,7 +840,7 @@ xenDaemonOpen_tcp(virConnectPtr conn, co memcpy(&priv->addr, r->ai_addr, r->ai_addrlen); - close(sock); + VIR_FORCE_CLOSE(sock); break; } @@ -5153,7 +5150,7 @@ xenDaemonDomainBlockPeek (virDomainPtr d ret = 0; cleanup: - if (fd >= 0) close (fd); + VIR_FORCE_CLOSE(fd); sexpr_free(root); virDomainDefFree(def); return ret; Index: libvirt-acl/daemon/libvirtd.c =================================================================== --- libvirt-acl.orig/daemon/libvirtd.c +++ libvirt-acl/daemon/libvirtd.c @@ -50,6 +50,7 @@ #include "libvirt_internal.h" #include "virterror_internal.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -425,7 +426,7 @@ static int daemonForkIntoBackground(void int stdoutfd = -1; int nextpid; - close(statuspipe[0]); + VIR_FORCE_CLOSE(statuspipe[0]); if ((stdinfd = open("/dev/null", O_RDONLY)) < 0) goto cleanup; @@ -437,12 +438,10 @@ static int daemonForkIntoBackground(void goto cleanup; if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO) goto cleanup; - if (close(stdinfd) < 0) + if (VIR_CLOSE(stdinfd) < 0) goto cleanup; - stdinfd = -1; - if (close(stdoutfd) < 0) + if (VIR_CLOSE(stdoutfd) < 0) goto cleanup; - stdoutfd = -1; if (setsid() < 0) goto cleanup; @@ -458,10 +457,8 @@ static int daemonForkIntoBackground(void } cleanup: - if (stdoutfd != -1) - close(stdoutfd); - if (stdinfd != -1) - close(stdinfd); + VIR_FORCE_CLOSE(stdoutfd); + VIR_FORCE_CLOSE(stdinfd); return -1; } @@ -475,7 +472,7 @@ static int daemonForkIntoBackground(void int ret; char status; - close(statuspipe[1]); + VIR_FORCE_CLOSE(statuspipe[1]); /* We wait to make sure the first child forked successfully */ if ((got = waitpid(pid, &exitstatus, 0)) < 0 || @@ -518,7 +515,7 @@ static int qemudWritePidFile(const char if (!(fh = fdopen(fd, "w"))) { VIR_ERROR(_("Failed to fdopen pid file '%s' : %s"), pidFile, virStrerror(errno, ebuf, sizeof ebuf)); - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } @@ -610,8 +607,7 @@ static int qemudListenUnix(struct qemud_ return 0; cleanup: - if (sock->fd >= 0) - close(sock->fd); + VIR_FORCE_CLOSE(sock->fd); VIR_FREE(sock); return -1; } @@ -665,7 +661,7 @@ remoteMakeSockets (int *fds, int max_fds VIR_ERROR(_("bind: %s"), virStrerror (errno, ebuf, sizeof ebuf)); return -1; } - close (fds[*nfds_r]); + VIR_FORCE_CLOSE(fds[*nfds_r]); } else { ++*nfds_r; } @@ -734,7 +730,7 @@ remoteListenTCP (struct qemud_server *se cleanup: for (i = 0; i < nfds; ++i) - close(fds[i]); + VIR_FORCE_CLOSE(fds[i]); return -1; } @@ -1483,7 +1479,7 @@ error: VIR_FREE(client); } VIR_FREE(addrstr); - close (fd); + VIR_FORCE_CLOSE(fd); PROBE(CLIENT_DISCONNECT, "fd=%d", fd); return -1; } @@ -1529,8 +1525,7 @@ void qemudDispatchClientFailure(struct q } if (client->fd != -1) { PROBE(CLIENT_DISCONNECT, "fd=%d", client->fd); - close(client->fd); - client->fd = -1; + VIR_FORCE_CLOSE(client->fd); } VIR_FREE(client->addrstr); } @@ -2433,17 +2428,15 @@ static int qemudStartEventLoop(struct qe static void qemudCleanup(struct qemud_server *server) { struct qemud_socket *sock; - if (server->sigread != -1) - close(server->sigread); - if (server->sigwrite != -1) - close(server->sigwrite); + VIR_FORCE_CLOSE(server->sigread); + VIR_FORCE_CLOSE(server->sigwrite); sock = server->sockets; while (sock) { struct qemud_socket *next = sock->next; if (sock->watch) virEventRemoveHandleImpl(sock->watch); - close(sock->fd); + VIR_FORCE_CLOSE(sock->fd); /* Unlink unix domain sockets which are not in * the abstract namespace */ @@ -2999,8 +2992,8 @@ daemonSetupSignals(struct qemud_server * return 0; error: - close(sigpipe[0]); - close(sigpipe[1]); + VIR_FORCE_CLOSE(sigpipe[0]); + VIR_FORCE_CLOSE(sigpipe[1]); return -1; } @@ -3257,8 +3250,7 @@ int main(int argc, char **argv) { while (write(statuswrite, &status, 1) == -1 && errno == EINTR) ; - close(statuswrite); - statuswrite = -1; + VIR_FORCE_CLOSE(statuswrite); } /* Start the event loop in a background thread, since @@ -3315,7 +3307,7 @@ error: errno == EINTR) ; } - close(statuswrite); + VIR_FORCE_CLOSE(statuswrite); } if (server) qemudCleanup(server); Index: libvirt-acl/src/util/hooks.c =================================================================== --- libvirt-acl.orig/src/util/hooks.c +++ libvirt-acl/src/util/hooks.c @@ -36,6 +36,7 @@ #include "conf/domain_conf.h" #include "logging.h" #include "memory.h" +#include "files.h" #define VIR_FROM_THIS VIR_FROM_HOOK @@ -368,11 +369,10 @@ virHookCall(int driver, const char *id, } ret = virExec(argv, env, NULL, &pid, pipefd[0], &outfd, &errfd, VIR_EXEC_NONE | VIR_EXEC_NONBLOCK); - if (close(pipefd[1]) < 0) { + if (VIR_CLOSE(pipefd[1]) < 0) { virReportSystemError(errno, "%s", _("unable to close pipe for hook input")); } - pipefd[1] = -1; } else { ret = virExec(argv, env, NULL, &pid, -1, &outfd, &errfd, VIR_EXEC_NONE | VIR_EXEC_NONBLOCK); @@ -418,17 +418,15 @@ virHookCall(int driver, const char *id, } cleanup: - if (pipefd[0] >= 0) { - if (close(pipefd[0]) < 0) { - virReportSystemError(errno, "%s", - _("unable to close pipe for hook input")); - } + if (VIR_CLOSE(pipefd[0]) < 0) { + virReportSystemError(errno, "%s", + _("unable to close pipe for hook input")); + ret = 1; } - if (pipefd[1] >= 0) { - if (close(pipefd[1]) < 0) { - virReportSystemError(errno, "%s", - _("unable to close pipe for hook input")); - } + if (VIR_CLOSE(pipefd[1]) < 0) { + virReportSystemError(errno, "%s", + _("unable to close pipe for hook input")); + ret = 1; } if (argv) { for (i = 0 ; i < argc ; i++) Index: libvirt-acl/tests/testutils.c =================================================================== --- libvirt-acl.orig/tests/testutils.c +++ libvirt-acl/tests/testutils.c @@ -47,6 +47,8 @@ ((((int) ((T)->tv_sec - (U)->tv_sec)) * 1000000.0 + \ ((int) ((T)->tv_usec - (U)->tv_usec))) / 1000.0) +#include "files.h" + static unsigned int testDebug = -1; static unsigned int testVerbose = -1; @@ -222,8 +224,10 @@ void virtTestCaptureProgramExecChild(con open_max = sysconf (_SC_OPEN_MAX); for (i = 0; i < open_max; i++) { if (i != stdinfd && - i != pipefd) - close(i); + i != pipefd) { + int tmpfd = i; + VIR_FORCE_CLOSE(tmpfd); + } } if (dup2(stdinfd, STDIN_FILENO) != STDIN_FILENO) @@ -237,8 +241,7 @@ void virtTestCaptureProgramExecChild(con execve(argv[0], (char *const*)argv, (char *const*)env); cleanup: - if (stdinfd != -1) - close(stdinfd); + VIR_FORCE_CLOSE(stdinfd); } int virtTestCaptureProgramOutput(const char *const argv[], @@ -252,10 +255,10 @@ int virtTestCaptureProgramOutput(const c int pid = fork(); switch (pid) { case 0: - close(pipefd[0]); + VIR_FORCE_CLOSE(pipefd[0]); virtTestCaptureProgramExecChild(argv, pipefd[1]); - close(pipefd[1]); + VIR_FORCE_CLOSE(pipefd[1]); _exit(1); case -1: @@ -267,7 +270,7 @@ int virtTestCaptureProgramOutput(const c int ret = -1; int want = buflen-1; - close(pipefd[1]); + VIR_FORCE_CLOSE(pipefd[1]); while (want) { if ((ret = read(pipefd[0], (*buf)+got, want)) <= 0) @@ -275,7 +278,7 @@ int virtTestCaptureProgramOutput(const c got += ret; want -= ret; } - close(pipefd[0]); + VIR_FORCE_CLOSE(pipefd[0]); if (!ret) (*buf)[got] = '\0'; Index: libvirt-acl/proxy/libvirt_proxy.c =================================================================== --- libvirt-acl.orig/proxy/libvirt_proxy.c +++ libvirt-acl/proxy/libvirt_proxy.c @@ -31,6 +31,7 @@ # include "xend_internal.h" # include "xs_internal.h" # include "xen_driver.h" +# include "files.h" static int fdServer = -1; static int debug = 0; @@ -133,10 +134,9 @@ proxyCloseUnixSocket(void) { if (fdServer < 0) return(0); - ret = close(fdServer); if (debug > 0) fprintf(stderr, "closing unix socket %d: %d\n", fdServer, ret); - fdServer = -1; + ret = VIR_CLOSE(fdServer); pollInfos[0].fd = -1; return(ret); } @@ -172,7 +172,7 @@ proxyListenUnixSocket(const char *path) addr.sun_path[0] = '\0'; if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { fprintf(stderr, "Path %s too long to fit into destination\n", path); - close(fd); + VIR_FORCE_CLOSE(fd); return -1; } @@ -181,12 +181,12 @@ proxyListenUnixSocket(const char *path) */ if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { fprintf(stderr, "Failed to bind to socket %s\n", path); - close(fd); + VIR_FORCE_CLOSE(fd); return (-1); } if (listen(fd, 30 /* backlog */ ) < 0) { fprintf(stderr, "Failed to listen to socket %s\n", path); - close(fd); + VIR_FORCE_CLOSE(fd); return (-1); } @@ -230,7 +230,7 @@ retry: if (nbClients >= MAX_CLIENT) { fprintf(stderr, "Too many client registered\n"); - close(client); + VIR_FORCE_CLOSE(client); return(-1); } nbClients++; @@ -260,7 +260,7 @@ static int proxyCloseClientSocket(int nr) { int ret; - ret = close(pollInfos[nr].fd); + ret = VIR_CLOSE(pollInfos[nr].fd); if (ret != 0) fprintf(stderr, "Failed to close socket %d from client %d\n", pollInfos[nr].fd, nr); @@ -285,7 +285,7 @@ proxyCloseClientSockets(void) { int i, ret; for (i = 1;i <= nbClients;i++) { - ret = close(pollInfos[i].fd); + ret = VIR_CLOSE(pollInfos[i].fd); if (ret != 0) fprintf(stderr, "Failed to close socket %d from client %d\n", pollInfos[i].fd, i); Index: libvirt-acl/tools/console.c =================================================================== --- libvirt-acl.orig/tools/console.c +++ libvirt-acl/tools/console.c @@ -39,6 +39,7 @@ # include "internal.h" # include "logging.h" # include "util.h" +# include "files.h" /* ie Ctrl-] as per telnet */ # define CTRL_CLOSE_BRACKET '\35' @@ -192,7 +193,7 @@ int vshRunConsole(const char *tty) { tcsetattr(STDIN_FILENO, TCSAFLUSH, &ttyattr); closetty: - close(ttyfd); + VIR_FORCE_CLOSE(ttyfd); return ret; } Diff tree+V3 against tree+V1 --- ../bak/libvirt-acl/daemon/libvirtd.c 2010-10-31 14:17:36.993380869 -0400 +++ daemon/libvirtd.c 2010-10-31 14:19:16.068381346 -0400 @@ -661,7 +661,7 @@ VIR_ERROR(_("bind: %s"), virStrerror (errno, ebuf, sizeof ebuf)); return -1; } - close (fds[*nfds_r]); + VIR_FORCE_CLOSE(fds[*nfds_r]); } else { ++*nfds_r; } @@ -1479,7 +1479,7 @@ VIR_FREE(client); } VIR_FREE(addrstr); - close (fd); + VIR_FORCE_CLOSE(fd); PROBE(CLIENT_DISCONNECT, "fd=%d", fd); return -1; } --- ../bak/libvirt-acl/src/libvirt.c 2010-10-31 14:17:36.974131928 -0400 +++ src/libvirt.c 2010-10-31 14:19:16.040380869 -0400 @@ -10884,8 +10884,8 @@ * ... report an error .... * done: * virStreamFree(st); - * VIR_FORCE_CLOSE(fd); - * + * if (VIR_CLOSE(fd) < 0) + * virReportSystemError(errno, "%s", _("failed to close file")); * * Returns the number of bytes read, which may be less * than requested. --- ../bak/libvirt-acl/src/lxc/lxc_container.c 2010-10-31 14:17:36.975130893 -0400 +++ src/lxc/lxc_container.c 2010-10-31 14:19:16.041381133 -0400 @@ -128,7 +128,7 @@ static int lxcContainerSetStdio(int control, int ttyfd) { int rc = -1; - int open_max, i, tpmfd; + int open_max, i; if (setsid() < 0) { virReportSystemError(errno, "%s", @@ -147,8 +147,8 @@ open_max = sysconf (_SC_OPEN_MAX); for (i = 0; i < open_max; i++) if (i != ttyfd && i != control) { - tpmfd = i; - VIR_FORCE_CLOSE(tpmfd); + int tmpfd = i; + VIR_FORCE_CLOSE(tmpfd); } if (dup2(ttyfd, 0) < 0) { --- ../bak/libvirt-acl/src/lxc/lxc_driver.c 2010-10-31 14:17:36.975130893 -0400 +++ src/lxc/lxc_driver.c 2010-10-31 14:41:44.756151598 -0400 @@ -1539,6 +1539,10 @@ rc = 0; cleanup: + if (VIR_CLOSE(logfd) < 0) { + virReportSystemError(errno, "%s", _("could not close logfile")); + rc = -1; + } for (i = 0 ; i < nveths ; i++) { if (rc != 0) vethDelete(veths[i]); @@ -1547,7 +1551,6 @@ if (rc != 0) VIR_FORCE_CLOSE(priv->monitor); VIR_FORCE_CLOSE(parentTty); - VIR_FORCE_CLOSE(logfd); VIR_FREE(logfile); return rc; } --- ../bak/libvirt-acl/src/phyp/phyp_driver.c 2010-10-31 14:17:36.977381236 -0400 +++ src/phyp/phyp_driver.c 2010-10-31 14:19:16.044380905 -0400 @@ -458,7 +458,11 @@ } } - VIR_FORCE_CLOSE(fd); + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, _("Could not close %s\n"), + local_file); + goto err; + } return 0; err: @@ -765,7 +769,11 @@ } break; } - VIR_FORCE_CLOSE(fd); + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, _("Could not close %s\n"), + local_file); + goto err; + } goto exit; exit: --- ../bak/libvirt-acl/src/qemu/qemu_driver.c 2010-10-31 14:17:36.981380798 -0400 +++ src/qemu/qemu_driver.c 2010-10-31 14:19:16.049381184 -0400 @@ -10336,8 +10336,7 @@ } cleanup: - if (fd >= 0) - close (fd); + VIR_FORCE_CLOSE(fd); if (vm) virDomainObjUnlock(vm); return ret; @@ -10424,7 +10423,7 @@ cleanup: VIR_FREE(tmp); - if (fd >= 0) close (fd); + VIR_FORCE_CLOSE(fd); unlink (tmp); if (vm) virDomainObjUnlock(vm); --- ../bak/libvirt-acl/src/qemu/qemu_monitor.c 2010-10-31 14:17:36.982380935 -0400 +++ src/qemu/qemu_monitor.c 2010-10-31 14:19:16.049381184 -0400 @@ -695,7 +695,8 @@ if (!mon->closed) { if (mon->watch) virEventRemoveHandle(mon->watch); - VIR_FORCE_CLOSE(mon->fd); + 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 --- ../bak/libvirt-acl/src/remote/remote_driver.c 2010-10-31 14:17:36.983381023 -0400 +++ src/remote/remote_driver.c 2010-10-31 14:19:16.051381190 -0400 @@ -622,7 +622,7 @@ if (connect (priv->sock, r->ai_addr, r->ai_addrlen) == -1) { saved_errno = errno; - close (priv->sock); + VIR_FORCE_CLOSE(priv->sock); continue; } @@ -631,8 +631,7 @@ negotiate_gnutls_on_connection (conn, priv, no_verify); if (!priv->session) { - close (priv->sock); - priv->sock = -1; + VIR_FORCE_CLOSE(priv->sock); goto failed; } } @@ -713,7 +712,6 @@ flags & VIR_DRV_OPEN_REMOTE_AUTOSTART && trials < 20) { VIR_FORCE_CLOSE(priv->sock); - priv->sock = -1; if (trials > 0 || remoteForkDaemon() == 0) { trials++; @@ -807,8 +805,8 @@ goto failed; /* Parent continues here. */ - close (sv[1]); - close (errfd[1]); + VIR_FORCE_CLOSE(sv[1]); + VIR_FORCE_CLOSE(errfd[1]); priv->sock = sv[0]; priv->errfd = errfd[0]; priv->pid = pid; @@ -963,7 +961,7 @@ gnutls_bye (priv->session, GNUTLS_SHUT_RDWR); gnutls_deinit (priv->session); } - close (priv->sock); + VIR_FORCE_CLOSE(priv->sock); #ifndef WIN32 if (priv->pid > 0) { pid_t reap; @@ -1440,8 +1438,8 @@ if (priv->saslconn) sasl_dispose (&priv->saslconn); #endif - close (priv->sock); - close (priv->errfd); + VIR_FORCE_CLOSE(priv->sock); + VIR_FORCE_CLOSE(priv->errfd); #ifndef WIN32 if (priv->pid > 0) { --- ../bak/libvirt-acl/src/storage/storage_backend.c 2010-10-31 14:17:36.985380881 -0400 +++ src/storage/storage_backend.c 2010-10-31 14:19:16.057380931 -0400 @@ -1576,7 +1576,7 @@ if (fp) fclose (fp); else - close (fd); + VIR_FORCE_CLOSE(fd); while ((w_err = waitpid (child, &exitstatus, 0) == -1) && errno == EINTR) /* empty */ ; --- ../bak/libvirt-acl/src/uml/uml_driver.c 2010-10-31 14:17:36.987380981 -0400 +++ src/uml/uml_driver.c 2010-10-31 14:19:16.060381549 -0400 @@ -811,7 +811,7 @@ virDomainObjPtr vm) { const char **argv = NULL, **tmp; const char **progenv = NULL; - int i, ret, tmpfd; + int i, ret; pid_t pid; char *logfile; int logfd = -1; @@ -920,7 +920,7 @@ */ for (i = 0; i < FD_SETSIZE; i++) if (FD_ISSET(i, &keepfd)) { - tmpfd = i; + int tmpfd = i; VIR_FORCE_CLOSE(tmpfd); } @@ -2088,7 +2088,7 @@ } cleanup: - if (fd >= 0) close (fd); + VIR_FORCE_CLOSE(fd); if (vm) virDomainObjUnlock(vm); return ret; --- ../bak/libvirt-acl/src/util/bridge.c 2010-10-31 14:17:36.988380773 -0400 +++ src/util/bridge.c 2010-10-31 14:19:16.060381549 -0400 @@ -110,7 +110,6 @@ return; VIR_FORCE_CLOSE(ctl->fd); - ctl->fd = 0; VIR_FREE(ctl); } --- ../bak/libvirt-acl/src/util/hooks.c 2010-10-31 14:17:36.993380869 -0400 +++ src/util/hooks.c 2010-10-31 14:46:06.855140940 -0400 @@ -373,7 +373,6 @@ virReportSystemError(errno, "%s", _("unable to close pipe for hook input")); } - pipefd[1] = -1; } else { ret = virExec(argv, env, NULL, &pid, -1, &outfd, &errfd, VIR_EXEC_NONE | VIR_EXEC_NONBLOCK); @@ -422,10 +421,12 @@ if (VIR_CLOSE(pipefd[0]) < 0) { virReportSystemError(errno, "%s", _("unable to close pipe for hook input")); + ret = 1; } if (VIR_CLOSE(pipefd[1]) < 0) { virReportSystemError(errno, "%s", _("unable to close pipe for hook input")); + ret = 1; } if (argv) { for (i = 0 ; i < argc ; i++) --- ../bak/libvirt-acl/src/util/macvtap.c 2010-10-31 14:17:36.989380812 -0400 +++ src/util/macvtap.c 2010-10-31 14:19:16.062381346 -0400 @@ -93,12 +93,6 @@ } -static void nlClose(int fd) -{ - VIR_FORCE_CLOSE(fd); -} - - /** * nlComm: * @nlmsg: pointer to netlink message @@ -192,7 +186,7 @@ *respbuflen = 0; } - nlClose(fd); + VIR_FORCE_CLOSE(fd); return rc; } @@ -690,8 +684,7 @@ if (rc >= 0) { if (configMacvtapTap(rc, vnet_hdr) < 0) { - VIR_FORCE_CLOSE(rc); - rc = -1; + VIR_FORCE_CLOSE(rc); /* sets rc to -1 */ goto disassociate_exit; } *res_ifname = strdup(cr_ifname); --- ../bak/libvirt-acl/src/util/util.c 2010-10-31 14:17:36.990380788 -0400 +++ src/util/util.c 2010-10-31 14:19:16.063381237 -0400 @@ -1127,13 +1127,13 @@ if (safewrite(fd, str, strlen(str)) < 0) { int saved_errno = errno; - close (fd); + VIR_FORCE_CLOSE(fd); errno = saved_errno; return -1; } /* Use errno from failed close only if there was no write error. */ - if (close (fd) != 0) + if (VIR_CLOSE(fd) != 0) return -1; return 0; --- ../bak/libvirt-acl/src/util/virtaudit.c 2010-10-31 14:18:05.137140928 -0400 +++ src/util/virtaudit.c 2010-10-31 14:20:15.982151881 -0400 @@ -135,7 +135,7 @@ void virAuditClose(void) { #if HAVE_AUDIT - VIR_CLOSE(auditfd); + VIR_FORCE_CLOSE(auditfd); #endif } --- ../bak/libvirt-acl/src/xen/proxy_internal.c 2010-10-31 14:17:36.990380788 -0400 +++ src/xen/proxy_internal.c 2010-10-31 14:19:16.066381180 -0400 @@ -233,15 +233,17 @@ static int virProxyCloseSocket(xenUnifiedPrivatePtr priv) { int ret; + int tmpfd; if (priv->proxy < 0) return(-1); + tmpfd = priv->proxy; ret = VIR_CLOSE(priv->proxy); if (ret != 0) - VIR_WARN("Failed to close socket %d", priv->proxy); + VIR_WARN("Failed to close socket %d", tmpfd); else - VIR_DEBUG("Closed socket %d", priv->proxy); + VIR_DEBUG("Closed socket %d", tmpfd); return(ret); } --- ../bak/libvirt-acl/src/xen/xend_internal.c 2010-10-31 14:17:36.992381259 -0400 +++ src/xen/xend_internal.c 2010-10-31 14:19:16.068381346 -0400 @@ -137,8 +137,7 @@ if (connect(s, (struct sockaddr *)&priv->addr, priv->addrlen) == -1) { - VIR_FORCE_CLOSE(s); - s = -1; + VIR_FORCE_CLOSE(s); /* preserves errno */ /* * Connecting to XenD when privileged is mandatory, so log this @@ -831,7 +830,7 @@ if (connect (sock, r->ai_addr, r->ai_addrlen) == -1) { saved_errno = errno; - close (sock); + VIR_FORCE_CLOSE(sock); continue; } @@ -5151,7 +5150,7 @@ ret = 0; cleanup: - if (fd >= 0) close (fd); + VIR_FORCE_CLOSE(fd); sexpr_free(root); virDomainDefFree(def); return ret; --- ../bak/libvirt-acl/tests/testutils.c 2010-10-31 14:17:36.993380869 -0400 +++ tests/testutils.c 2010-10-31 14:19:16.069381264 -0400 @@ -207,7 +207,7 @@ static void virtTestCaptureProgramExecChild(const char *const argv[], int pipefd) { - int i, tmpfd; + int i; int open_max; int stdinfd = -1; const char *const env[] = { @@ -225,7 +225,7 @@ for (i = 0; i < open_max; i++) { if (i != stdinfd && i != pipefd) { - tmpfd = i; + int tmpfd = i; VIR_FORCE_CLOSE(tmpfd); } }

3 5

[libvirt] [PATCH] esx: Add support for memtune/min_guarantee
by Matthias Bolte 10 Nov '10

10 Nov '10

Also add a test case for the VMX handling of it. --- src/esx/esx_driver.c | 133 ++++++++++++++++++++++- src/esx/esx_vi.c | 31 ++++++ src/esx/esx_vi.h | 3 + src/esx/esx_vi_types.c | 5 + src/esx/esx_vi_types.h | 1 + src/esx/esx_vmx.c | 49 +++++++-- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.vmx | 2 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml | 3 + tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx | 26 +++++ tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.xml | 37 +++++++ tests/xml2vmxtest.c | 1 + 11 files changed, 280 insertions(+), 11 deletions(-) create mode 100644 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.xml diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index b3e1284..0591115 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4149,6 +4149,135 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) +static int +esxDomainSetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, + int nparams, unsigned int flags) +{ + int result = -1; + esxPrivate *priv = domain->conn->privateData; + esxVI_ObjectContent *virtualMachine = NULL; + esxVI_VirtualMachineConfigSpec *spec = NULL; + esxVI_ManagedObjectReference *task = NULL; + esxVI_TaskInfoState taskInfoState; + int i; + + virCheckFlags(0, -1); + + if (esxVI_EnsureSession(priv->primary) < 0) { + return -1; + } + + if (esxVI_LookupVirtualMachineByUuidAndPrepareForTask + (priv->primary, domain->uuid, NULL, &virtualMachine, + priv->autoAnswer) < 0 || + esxVI_VirtualMachineConfigSpec_Alloc(&spec) < 0 || + esxVI_ResourceAllocationInfo_Alloc(&spec->memoryAllocation) < 0) { + goto cleanup; + } + + for (i = 0; i < nparams; ++i) { + if (STREQ (params[i].field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE) && + params[i].type == VIR_DOMAIN_SCHED_FIELD_ULLONG) { + if (esxVI_Long_Alloc(&spec->memoryAllocation->reservation) < 0) { + goto cleanup; + } + + spec->memoryAllocation->reservation->value = + params[i].value.ul / 1024; /* Scale from kilobytes to megabytes */ + } else { + ESX_ERROR(VIR_ERR_INVALID_ARG, _("Unknown field '%s'"), + params[i].field); + goto cleanup; + } + } + + if (esxVI_ReconfigVM_Task(priv->primary, virtualMachine->obj, spec, + &task) < 0 || + esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, + esxVI_Occurrence_RequiredItem, + priv->autoAnswer, &taskInfoState) < 0) { + goto cleanup; + } + + if (taskInfoState != esxVI_TaskInfoState_Success) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not change memory parameters")); + goto cleanup; + } + + result = 0; + + cleanup: + esxVI_ObjectContent_Free(&virtualMachine); + esxVI_VirtualMachineConfigSpec_Free(&spec); + esxVI_ManagedObjectReference_Free(&task); + + return result; +} + + + +static int +esxDomainGetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, + int *nparams, unsigned int flags) +{ + int result = -1; + esxPrivate *priv = domain->conn->privateData; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *virtualMachine = NULL; + esxVI_Long *reservation = NULL; + + virCheckFlags(0, -1); + + if (*nparams == 0) { + *nparams = 1; /* min_guarantee */ + return 0; + } + + if (*nparams < 1) { + ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", + _("Parameter array must have space for 1 item")); + return -1; + } + + if (esxVI_EnsureSession(priv->primary) < 0) { + return -1; + } + + if (esxVI_String_AppendValueToList + (&propertyNameList, "config.memoryAllocation.reservation") < 0 || + esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid, + propertyNameList, &virtualMachine, + esxVI_Occurrence_RequiredItem) < 0 || + esxVI_GetLong(virtualMachine, "config.memoryAllocation.reservation", + &reservation, esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + if (virStrcpyStatic(params[0].field, + VIR_DOMAIN_MEMORY_MIN_GUARANTEE) == NULL) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Field %s too big for destination"), + VIR_DOMAIN_MEMORY_MIN_GUARANTEE); + goto cleanup; + } + + params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; + params[0].value.ul = reservation->value * 1024; /* Scale from megabytes to kilobytes */ + + *nparams = 1; + result = 0; + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&virtualMachine); + esxVI_Long_Free(&reservation); + + return result; +} + + + static virDriver esxDriver = { VIR_DRV_ESX, "ESX", @@ -4251,8 +4380,8 @@ static virDriver esxDriver = { esxDomainRevertToSnapshot, /* domainRevertToSnapshot */ esxDomainSnapshotDelete, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ - NULL, /* domainSetMemoryParameters */ - NULL, /* domainGetMemoryParameters */ + esxDomainSetMemoryParameters, /* domainSetMemoryParameters */ + esxDomainGetMemoryParameters, /* domainGetMemoryParameters */ }; diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index c450730..9c96650 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -1840,6 +1840,37 @@ esxVI_GetBoolean(esxVI_ObjectContent *objectContent, const char *propertyName, return 0; } +int +esxVI_GetLong(esxVI_ObjectContent *objectContent, const char *propertyName, + esxVI_Long **value, esxVI_Occurrence occurence) +{ + esxVI_DynamicProperty *dynamicProperty; + + if (value == NULL || *value != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + for (dynamicProperty = objectContent->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, propertyName)) { + if (esxVI_Long_CastFromAnyType(dynamicProperty->val, value) < 0) { + return -1; + } + + break; + } + } + + if (*value == NULL && occurence == esxVI_Occurrence_RequiredItem) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Missing '%s' property"), propertyName); + return -1; + } + + return 0; +} + int diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index aa47e87..26779ad 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -302,6 +302,9 @@ int esxVI_GetBoolean(esxVI_ObjectContent *objectContent, const char *propertyName, esxVI_Boolean *value, esxVI_Occurrence occurence); +int esxVI_GetLong(esxVI_ObjectContent *objectContent, const char *propertyName, + esxVI_Long **value, esxVI_Occurrence occurence); + int esxVI_GetStringValue(esxVI_ObjectContent *objectContent, const char *propertyName, char **value, esxVI_Occurrence occurence); diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index bb791af..ad45483 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -1235,6 +1235,11 @@ ESX_VI__TEMPLATE__VALIDATE(Long, /* esxVI_Long_AppendToList */ ESX_VI__TEMPLATE__LIST__APPEND(Long) +/* esxVI_Long_CastFromAnyType */ +ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE(Long, +{ +}) + /* esxVI_Long_Serialize */ ESX_VI__TEMPLATE__SERIALIZE(Long, { diff --git a/src/esx/esx_vi_types.h b/src/esx/esx_vi_types.h index f659361..901eda0 100644 --- a/src/esx/esx_vi_types.h +++ b/src/esx/esx_vi_types.h @@ -227,6 +227,7 @@ int esxVI_Long_Alloc(esxVI_Long **number); void esxVI_Long_Free(esxVI_Long **numberList); int esxVI_Long_Validate(esxVI_Long *number); int esxVI_Long_AppendToList(esxVI_Long **numberList, esxVI_Long *number); +int esxVI_Long_CastFromAnyType(esxVI_AnyType *anyType, esxVI_Long **number); int esxVI_Long_Serialize(esxVI_Long *number, const char *element, virBufferPtr output); int esxVI_Long_SerializeList(esxVI_Long *numberList, const char *element, diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index 36818df..981b856 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -50,6 +50,7 @@ def->uuid = <value> <=> uuid.bios = "<value>" def->name = <value> <=> displayName = "<value>" def->mem.max_balloon = <value kilobyte> <=> memsize = "<value megabyte>" # must be a multiple of 4, defaults to 32 def->mem.cur_balloon = <value kilobyte> <=> sched.mem.max = "<value megabyte>" # defaults to "unlimited" -> def->mem.cur_balloon = def->mem.max_balloon +def->mem.min_guarantee = <value kilobyte> <=> sched.mem.minsize = "<value megabyte>" # defaults to 0 def->maxvcpus = <value> <=> numvcpus = "<value>" # must be 1 or a multiple of 2, defaults to 1 def->cpumask = <uint list> <=> sched.cpu.affinity = "<uint list>" @@ -874,7 +875,8 @@ esxVMX_ParseConfig(esxVMX_Context *ctx, virCapsPtr caps, const char *vmx, long long config_version = 0; long long virtualHW_version = 0; long long memsize = 0; - long long memory = 0; + long long sched_mem_max = 0; + long long sched_mem_minsize = 0; long long numvcpus = 0; char *sched_cpu_affinity = NULL; char *guestOS = NULL; @@ -1047,22 +1049,38 @@ esxVMX_ParseConfig(esxVMX_Context *ctx, virCapsPtr caps, const char *vmx, def->mem.max_balloon = memsize * 1024; /* Scale from megabytes to kilobytes */ - /* vmx:sched.mem.max -> def:memory */ - if (esxUtil_GetConfigLong(conf, "sched.mem.max", &memory, memsize, + /* vmx:sched.mem.max -> def:mem.cur_balloon */ + if (esxUtil_GetConfigLong(conf, "sched.mem.max", &sched_mem_max, memsize, true) < 0) { goto cleanup; } - if (memory < 0) { - memory = memsize; + if (sched_mem_max < 0) { + sched_mem_max = memsize; } - def->mem.cur_balloon = memory * 1024; /* Scale from megabytes to kilobytes */ + def->mem.cur_balloon = sched_mem_max * 1024; /* Scale from megabytes to kilobytes */ if (def->mem.cur_balloon > def->mem.max_balloon) { def->mem.cur_balloon = def->mem.max_balloon; } + /* vmx:sched.mem.minsize -> def:mem.min_guarantee */ + if (esxUtil_GetConfigLong(conf, "sched.mem.minsize", &sched_mem_minsize, 0, + true) < 0) { + goto cleanup; + } + + if (sched_mem_minsize < 0) { + sched_mem_minsize = 0; + } + + def->mem.min_guarantee = sched_mem_minsize * 1024; /* Scale from megabytes to kilobytes */ + + if (def->mem.min_guarantee > def->mem.max_balloon) { + def->mem.min_guarantee = def->mem.max_balloon; + } + /* vmx:numvcpus -> def:vcpus */ if (esxUtil_GetConfigLong(conf, "numvcpus", &numvcpus, 1, true) < 0) { goto cleanup; @@ -2594,12 +2612,12 @@ esxVMX_FormatConfig(esxVMX_Context *ctx, virCapsPtr caps, virDomainDefPtr def, virBufferVSprintf(&buffer, "memsize = \"%d\"\n", (int)(def->mem.max_balloon / 1024)); - /* def:memory -> vmx:sched.mem.max */ + /* def:mem.cur_balloon -> vmx:sched.mem.max */ if (def->mem.cur_balloon < def->mem.max_balloon) { if (def->mem.cur_balloon <= 0 || def->mem.cur_balloon % 1024 != 0) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Expecting domain XML entry 'currentMemory' to be an " - "unsigned integer (multiple of 1024) but found %lld"), + "unsigned integer (multiple of 1024) but found %llu"), (unsigned long long)def->mem.cur_balloon); goto cleanup; } @@ -2609,6 +2627,21 @@ esxVMX_FormatConfig(esxVMX_Context *ctx, virCapsPtr caps, virDomainDefPtr def, (int)(def->mem.cur_balloon / 1024)); } + /* def:mem.min_guarantee -> vmx:sched.mem.minsize */ + if (def->mem.min_guarantee > 0) { + if (def->mem.min_guarantee % 1024 != 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Expecting domain XML entry 'memtune/min_guarantee' to " + "be an unsigned integer (multiple of 1024) but found %llu"), + (unsigned long long)def->mem.min_guarantee); + goto cleanup; + } + + /* Scale from kilobytes to megabytes */ + virBufferVSprintf(&buffer, "sched.mem.minsize = \"%d\"\n", + (int)(def->mem.min_guarantee / 1024)); + } + /* def:maxvcpus -> vmx:numvcpus */ if (def->vcpus != def->maxvcpus) { ESX_ERROR(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.vmx b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.vmx index 6b11cc2..650f8fd 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.vmx +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.vmx @@ -52,7 +52,7 @@ snapshot.action = "keep" sched.cpu.min = "0" sched.cpu.units = "mhz" sched.cpu.shares = "normal" -sched.mem.minsize = "0" +sched.mem.minsize = "256" sched.mem.shares = "normal" tools.upgrade.policy = "manual" cpuid.80000001.eax = "--------------------------------" diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml index c324673..d55bf6b 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml @@ -4,6 +4,9 @@ <description>Centos 5.5 64bit Server</description> <memory>2097152</memory> <currentMemory>2097152</currentMemory> + <memtune> + <min_guarantee>262144</min_guarantee> + </memtune> <vcpu>2</vcpu> <os> <type arch='x86_64'>hvm</type> diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx new file mode 100644 index 0000000..cc2485f --- /dev/null +++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.vmx @@ -0,0 +1,26 @@ +.encoding = "UTF-8" +config.version = "8" +virtualHW.version = "4" +guestOS = "other-64" +uuid.bios = "42 3e 94 a9 a1 c7 b3 1d-71 61 76 c7 58 6c 83 0e" +displayName = "vmtest.local" +annotation = "Centos 5.5 64bit Server" +memsize = "2048" +sched.mem.minsize = "256" +numvcpus = "2" +scsi0.present = "true" +scsi0.virtualDev = "lsilogic" +scsi0:0.present = "true" +scsi0:0.deviceType = "scsi-hardDisk" +scsi0:0.fileName = "/vmfs/volumes/datastore/directory/vmtest-000001.vmdk" +ide0:0.present = "true" +ide0:0.deviceType = "cdrom-image" +ide0:0.fileName = "/vmfs/volumes/4af0231d-1eff559a-6369-0024e84773b6/isos/CentOS-5.5-x86_64-bin-DVD-1of2.iso" +floppy0.present = "false" +floppy1.present = "false" +ethernet0.present = "true" +ethernet0.virtualDev = "e1000" +ethernet0.networkName = "VM-LAN" +ethernet0.connectionType = "bridged" +ethernet0.addressType = "vpx" +ethernet0.generatedAddress = "00:50:56:BE:00:15" diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.xml b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.xml new file mode 100644 index 0000000..d55bf6b --- /dev/null +++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.xml @@ -0,0 +1,37 @@ +<domain type='vmware'> + <name>vmtest.local</name> + <uuid>423e94a9-a1c7-b31d-7161-76c7586c830e</uuid> + <description>Centos 5.5 64bit Server</description> + <memory>2097152</memory> + <currentMemory>2097152</currentMemory> + <memtune> + <min_guarantee>262144</min_guarantee> + </memtune> + <vcpu>2</vcpu> + <os> + <type arch='x86_64'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <source file='[datastore] directory/vmtest-000001.vmdk'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <disk type='file' device='cdrom'> + <source file='[4af0231d-1eff559a-6369-0024e84773b6] isos/CentOS-5.5-x86_64-bin-DVD-1of2.iso'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='lsilogic'/> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:50:56:be:00:15'/> + <source bridge='VM-LAN'/> + <model type='e1000'/> + </interface> + </devices> +</domain> diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index 0881ae9..db7eb53 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -269,6 +269,7 @@ mymain(int argc, char **argv) DO_TEST("esx-in-the-wild-2", "esx-in-the-wild-2", esxVI_ProductVersion_ESX35); DO_TEST("esx-in-the-wild-3", "esx-in-the-wild-3", esxVI_ProductVersion_ESX35); DO_TEST("esx-in-the-wild-4", "esx-in-the-wild-4", esxVI_ProductVersion_ESX35); + DO_TEST("esx-in-the-wild-5", "esx-in-the-wild-5", esxVI_ProductVersion_ESX35); DO_TEST("gsx-in-the-wild-1", "gsx-in-the-wild-1", esxVI_ProductVersion_ESX35); DO_TEST("gsx-in-the-wild-2", "gsx-in-the-wild-2", esxVI_ProductVersion_ESX35); -- 1.7.0.4

2 2

[libvirt] [PATCH] Fix syntax-check error in domain.rng
by Matthias Bolte 10 Nov '10

10 Nov '10

Replace tab with spaces. --- Pushed under the trivial-build-breaker-fix rule :) docs/schemas/domain.rng | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 43c8c44..bbbc846 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1149,7 +1149,7 @@ <value>insecure</value> </choice> </attribute> - <empty/> + <empty/> </element> </zeroOrMore> </group> -- 1.7.0.4

1 0

[libvirt] [PATCH] 0/4 Add SMBIOS settings to domain definition
by Daniel Veillard 09 Nov '10

09 Nov '10

The SMBIOS data are a standardized set of data structures available in the BIOS area of PCs. Those blocks of data describe things like BIOS version informations, machine vendor, model and identifiers, as well as various parts of the machine capability. On a linux machine running dmidecode allows to dump those informations. Spec available at the DMTF: http://dmtf.org/standards/smbios From a virtualization POV, it's mostly the first block describing the BIOS named "type 0" and the second block describing the machine named "type 1" which are of interest. Those data are usually accessed either from the OS or from management application, and being able to override the default setings may be needed for such management. The suggested XML description follows the logical structure of the data, one top smbios description, with one or more blocks, each containing the entries, the example below gives an idea: <smbios> <table type="0"> <entry name="Vendor">QEmu/KVM</entry> <entry name="Version">0.13</entry> </table> <table type="1"> <entry name="Manufacturer">Fedora</entry> <entry name="Product">Virt-Manager</entry> <entry name="Version">0.8.2-3.fc14</entry> <entry name="UUID">c7a5fdbdedaf9455926ad65c16db1809</entry> </table> </smbios> The patch serie includes: - the change to the doamin schemas - the code to parse this in config structure - the QEmu driver part - a test example Currently missing is the conf code to save the data back, and documentation for the domain extension. I think it's possible set some of those settings in Xen too, but I don't know if I will take the time of a driver for it. For ESX it seems there is an option to inherit some of the SMBIOS data from the host on the guest but I'm not sure some of the fields can be manipulated directly. I don't know what VirtualBox can set there either, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

5 27

[libvirt] [PATCH] schema: Move timer element inside clock
by Jiri Denemark 09 Nov '10

09 Nov '10

Both conf/domain_conf.c and domain XML documentation have <timer> elements inside /domain/clock. Change domain.rng schema to be consistent with them. --- docs/schemas/domain.rng | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index aaf9667..5a8576c 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -348,9 +348,6 @@ </interleave> </define> <define name="clock"> - <zeroOrMore> - <ref name="timer"/> - </zeroOrMore> <optional> <element name="clock"> <choice> @@ -381,7 +378,9 @@ </optional> </group> </choice> - <empty/> + <zeroOrMore> + <ref name="timer"/> + </zeroOrMore> </element> </optional> </define> -- 1.7.3.2

3 4

[libvirt] [PATCH] virsh: fix incorrect help text mentioning non-existent network 'id'
by Justin Clift 09 Nov '10

09 Nov '10

--- Pushed this under the trivial rule. There's no such thing as a network id, so the help text is plainly wrong. tools/virsh.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e704799..2bbae9b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3650,7 +3650,7 @@ static const vshCmdInfo info_network_destroy[] = { }; static const vshCmdOptDef opts_network_destroy[] = { - {"network", VSH_OT_DATA, VSH_OFLAG_REQ, N_("network name, id or uuid")}, + {"network", VSH_OT_DATA, VSH_OFLAG_REQ, N_("network name or uuid")}, {NULL, 0, 0, NULL} }; @@ -3689,7 +3689,7 @@ static const vshCmdInfo info_network_dumpxml[] = { }; static const vshCmdOptDef opts_network_dumpxml[] = { - {"network", VSH_OT_DATA, VSH_OFLAG_REQ, N_("network name, id or uuid")}, + {"network", VSH_OT_DATA, VSH_OFLAG_REQ, N_("network name or uuid")}, {NULL, 0, 0, NULL} }; @@ -9329,7 +9329,7 @@ static const vshCmdInfo info_network_edit[] = { }; static const vshCmdOptDef opts_network_edit[] = { - {"network", VSH_OT_DATA, VSH_OFLAG_REQ, N_("network name, id or uuid")}, + {"network", VSH_OT_DATA, VSH_OFLAG_REQ, N_("network name or uuid")}, {NULL, 0, 0, NULL} }; -- 1.7.3

2 1

[libvirt] [PATCH 00/11] Allow access to text console with virStream APIs (v4)
by Daniel P. Berrange 09 Nov '10

09 Nov '10

An update of the patches in http://www.redhat.com/archives/libvir-list/2010-November/msg00132.html http://www.redhat.com/archives/libvir-list/2010-November/msg00022.html http://www.redhat.com/archives/libvir-list/2010-August/msg00379.html The end goal is to allow 'virsh console' to work unprivileged, and/or over remote connections. The primary change in this series is to fixup the threading problems from Win32.

3 14

[libvirt] [PATCH] virsh: Add .xml suffix to tmp files used in *edit commands
by Jiri Denemark 09 Nov '10

09 Nov '10

This helps editors with detecting the temporary files as XML since the temporary files do not contain <?xml ...?> declaration. Requested by https://bugzilla.redhat.com/show_bug.cgi?id=602277 --- bootstrap.conf | 1 + tools/virsh.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 8f25554..12f64c8 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -42,6 +42,7 @@ inet_pton ioctl maintainer-makefile mkstemp +mkstemps mktempd netdb perror diff --git a/tools/virsh.c b/tools/virsh.c index bc746f8..26d7f5a 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8964,10 +8964,10 @@ editWriteToTempFile (vshControl *ctl, const char *doc) tmpdir = getenv ("TMPDIR"); if (!tmpdir) tmpdir = "/tmp"; - snprintf (ret, PATH_MAX, "%s/virshXXXXXX", tmpdir); - fd = mkstemp (ret); + snprintf (ret, PATH_MAX, "%s/virshXXXXXX.xml", tmpdir); + fd = mkstemps(ret, 4); if (fd == -1) { - vshError(ctl, _("mkstemp: failed to create temporary file: %s"), + vshError(ctl, _("mkstemps: failed to create temporary file: %s"), strerror(errno)); VIR_FREE(ret); return NULL; -- 1.7.3.2

2 2

[libvirt] [PATCH v4] qemu: call drive_del in DetachPciDiskDevice
by Ryan Harper 09 Nov '10

09 Nov '10

Currently libvirt doesn't confirm whether the guest has responded to the disk removal request. In some cases this can leave the guest with continued access to the device while the mgmt layer believes that it has been removed. With a recent qemu monitor command[1] we can deterministically revoke a guests access to the disk (on the QEMU side) to ensure no futher access is permitted. This patch adds support for the drive_del() command and introduces it in the disk removal paths. If the guest is running in a QEMU without this command we currently explicitly check for unknown command/CommandNotFound and log the issue. If QEMU supports the command we issue the drive_del command after we attempt to remove the device. The guest may respond and remove the block device before we get to attempt to call drive_del. In that case, we explicitly check for 'Device not found' from the monitor indicating that the target drive was auto-deleted upon guest responds to the device removal notification. 1. http://thread.gmane.org/gmane.comp.emulators.qemu/84255 Signed-off-by: Ryan Harper <ryanh(a)us.ibm.com> --- Changes since v3: - Renamed DriveUnplug -> DriveDel, use drive_del monitor cmd. - Moved invocation to after DelDevice and guest notification. - Handle the case where drive is auto-deleted before we call DriveDel by catching and ignoring 'Device not found' error. - Simplified DriveDel invocation; no need to check return codes as the monitor implementations handle all failure case and logs or ignores as needed. Changes since v2: - use VIR_ERROR to report when unplug command not found Changes since v1: - return > 0 when command isn't present, < 0 on command failure - detect when drive_unplug command isn't present and log error instead of failing entire command src/qemu/qemu_driver.c | 29 ++++++++++++++++++++++ src/qemu/qemu_monitor.c | 19 ++++++++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 38 +++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ src/qemu/qemu_monitor_text.c | 54 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 ++ 7 files changed, 149 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e7b37e1..d01bb2c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9037,6 +9037,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char drivestr[PATH_MAX]; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -9077,6 +9078,20 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, goto cleanup; } } + + /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", + QEMU_DRIVE_HOST_PREFIX, + detach->info.alias)) + < 0 || ret >= sizeof(drivestr)) { + virReportOOMError(); + goto cleanup; + } + + /* disconnect guest from host device */ + qemuMonitorDriveDel(priv->mon, drivestr); + qemuDomainObjExitMonitorWithDriver(driver, vm); qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0); @@ -9116,6 +9131,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + char drivestr[PATH_MAX]; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -9147,6 +9163,19 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, qemuDomainObjExitMonitor(vm); goto cleanup; } + + /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if ((ret = snprintf(drivestr, sizeof(drivestr), "%s%s", + QEMU_DRIVE_HOST_PREFIX, + detach->info.alias)) + < 0 || ret >= sizeof(drivestr)) { + virReportOOMError(); + goto cleanup; + } + /* disconnect guest from host device */ + qemuMonitorDriveDel(priv->mon, drivestr); + qemuDomainObjExitMonitorWithDriver(driver, vm); qemuDomainDiskAudit(vm, detach, NULL, "detach", ret >= 0); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2366fdb..80adba4 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1781,6 +1781,25 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, return ret; } +int qemuMonitorDriveDel(qemuMonitorPtr mon, + const char *drivestr) +{ + DEBUG("mon=%p drivestr=%s", mon, drivestr); + int ret; + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONDriveDel(mon, drivestr); + else + ret = qemuMonitorTextDriveDel(mon, drivestr); + return ret; +} + int qemuMonitorDelDevice(qemuMonitorPtr mon, const char *devalias) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7d09145..8cda43b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -381,6 +381,9 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon, int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorDriveDel(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d2c6f0a..a380ab2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2244,6 +2244,44 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, } +int qemuMonitorJSONDriveDel(qemuMonitorPtr mon, + const char *drivestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + DEBUG("JSONDriveDel drivestr=%s", drivestr); + cmd = qemuMonitorJSONMakeCommand("drive_del", + "s:id", drivestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) { + /* See if drive_del isn't supported */ + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_ERROR0(_("deleting disk is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { + /* NB: device not found errors mean the drive was + * auto-deleted and we ignore the error */ + ret = 0; + } else { + ret = qemuMonitorJSONCheckError(cmd, reply); + } + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 94806c1..82671c7 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -188,6 +188,9 @@ int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorJSONDriveDel(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7f15008..483ceb0 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2285,6 +2285,7 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon, goto cleanup; } + DEBUG("TextDelDevice devalias=%s", devalias); if (qemuMonitorCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("cannot detach %s device"), devalias); @@ -2391,6 +2392,59 @@ cleanup: return ret; } +/* Attempts to remove a host drive. + * Returns 1 if unsupported, 0 if ok, and -1 on other failure */ +int qemuMonitorTextDriveDel(qemuMonitorPtr mon, + const char *drivestr) +{ + char *cmd = NULL; + char *reply = NULL; + char *safedev; + int ret = -1; + DEBUG("TextDriveDel drivestr=%s", drivestr); + + if (!(safedev = qemuMonitorEscapeArg(drivestr))) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&cmd, "drive_del %s", safedev) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot delete %s drive"), drivestr); + goto cleanup; + } + + if (strstr(reply, "unknown command:")) { + VIR_ERROR0(_("deleting drive is not supported. " + "This may leak data if disk is reassigned")); + ret = 1; + goto cleanup; + + /* (qemu) drive_del wark + * Device 'wark' not found */ + } else if (STRPREFIX(reply, "Device '") && (strstr(reply, "not found"))) { + /* NB: device not found errors mean the drive was auto-deleted and we + * ignore the error */ + ret = 0; + } else if (STRNEQ(reply, "")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("deleting %s drive failed: %s"), drivestr, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safedev); + return ret; +} int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index c017509..3a88f87 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -186,6 +186,9 @@ int qemuMonitorTextDelDevice(qemuMonitorPtr mon, int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorTextDriveDel(qemuMonitorPtr mon, + const char *drivestr); + int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase); -- 1.6.3.3 -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh(a)us.ibm.com

2 1