- Devel - libvirt List Archives

[libvirt] [PATCH v4] selinux: Only create the selabel_handle once.
by Richard W.M. Jones 24 Jan '13

24 Jan '13

From: "Richard W.M. Jones" <rjones(a)redhat.com> According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 129 ++++++++++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 46 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..2affe69 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -63,6 +63,9 @@ struct _virSecuritySELinuxData { char *content_context; virHashTablePtr mcs; bool skipAllLabel; +#if HAVE_SELINUX_LABEL_H + struct selabel_handle *label_handle; +#endif }; struct _virSecuritySELinuxCallbackData { @@ -367,12 +370,21 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr) data->skipAllLabel = true; +#if HAVE_SELINUX_LABEL_H + data->label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); + if (!data->label_handle) { + virReportSystemError(errno, "%s", + _("cannot open SELinux label_handle")); + return -1; + } +#endif + selinux_conf = virConfReadFile(selinux_lxc_contexts_path(), 0); if (!selinux_conf) { virReportSystemError(errno, _("cannot open SELinux lxc contexts file '%s'"), selinux_lxc_contexts_path()); - return -1; + goto error; } scon = virConfGetValue(selinux_conf, "process"); @@ -418,6 +430,9 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr) return 0; error: +#if HAVE_SELINUX_LABEL_H + selabel_close(data->label_handle); +#endif virConfFree(selinux_conf); VIR_FREE(data->domain_context); VIR_FREE(data->file_context); @@ -444,6 +459,15 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr) data->skipAllLabel = false; +#if HAVE_SELINUX_LABEL_H + data->label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); + if (!data->label_handle) { + virReportSystemError(errno, "%s", + _("cannot open SELinux label_handle")); + return -1; + } +#endif + if (virFileReadAll(selinux_virtual_domain_context_path(), MAX_CONTEXT, &(data->domain_context)) < 0) { virReportSystemError(errno, _("cannot read SELinux virtual domain context file '%s'"), @@ -499,6 +523,9 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr) return 0; error: +#if HAVE_SELINUX_LABEL_H + selabel_close(data->label_handle); +#endif VIR_FREE(data->domain_context); VIR_FREE(data->alt_domain_context); VIR_FREE(data->file_context); @@ -763,6 +790,10 @@ virSecuritySELinuxSecurityDriverClose(virSecurityManagerPtr mgr) if (!data) return 0; +#if HAVE_SELINUX_LABEL_H + selabel_close(data->label_handle); +#endif + virHashFree(data->mcs); VIR_FREE(data->domain_context); @@ -937,18 +968,13 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) /* Set fcon to the appropriate label for path and mode, or return -1. */ static int -getContext(const char *newpath, mode_t mode, security_context_t *fcon) +getContext(virSecurityManagerPtr mgr, + const char *newpath, mode_t mode, security_context_t *fcon) { #if HAVE_SELINUX_LABEL_H - struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); - int ret; - - if (handle == NULL) - return -1; + virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); - ret = selabel_lookup_raw(handle, fcon, newpath, mode); - selabel_close(handle); - return ret; + return selabel_lookup_raw(data->label_handle, fcon, newpath, mode); #else return matchpathcon(newpath, mode, fcon); #endif @@ -958,7 +984,8 @@ getContext(const char *newpath, mode_t mode, security_context_t *fcon) /* This method shouldn't raise errors, since they'll overwrite * errors that the caller(s) are already dealing with */ static int -virSecuritySELinuxRestoreSecurityFileLabel(const char *path) +virSecuritySELinuxRestoreSecurityFileLabel(virSecurityManagerPtr mgr, + const char *path) { struct stat buf; security_context_t fcon = NULL; @@ -980,7 +1007,7 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) goto err; } - if (getContext(newpath, buf.st_mode, &fcon) < 0) { + if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) { /* Any user created path likely does not have a default label, * which makes this an expected non error */ @@ -997,7 +1024,7 @@ err: } static int -virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, +virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainDiskDefPtr disk, int migrated) @@ -1044,7 +1071,7 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBU } } - return virSecuritySELinuxRestoreSecurityFileLabel(disk->src); + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, disk->src); } @@ -1301,22 +1328,27 @@ virSecuritySELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN static int virSecuritySELinuxRestoreSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, const char *file, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { - return virSecuritySELinuxRestoreSecurityFileLabel(file); + virSecurityManagerPtr mgr = opaque; + + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, file); } static int virSecuritySELinuxRestoreSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { - return virSecuritySELinuxRestoreSecurityFileLabel(file); + virSecurityManagerPtr mgr = opaque; + + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, file); } static int -virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virDomainHostdevDefPtr dev, +virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, + virDomainHostdevDefPtr dev, const char *vroot) { @@ -1335,7 +1367,7 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virDomainHostdevDefPtr dev, if (!usb) goto done; - ret = usbDeviceFileIterate(usb, virSecuritySELinuxRestoreSecurityUSBLabel, NULL); + ret = usbDeviceFileIterate(usb, virSecuritySELinuxRestoreSecurityUSBLabel, mgr); usbFreeDevice(usb); break; @@ -1350,7 +1382,7 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virDomainHostdevDefPtr dev, if (!pci) goto done; - ret = pciDeviceFileIterate(pci, virSecuritySELinuxRestoreSecurityPCILabel, NULL); + ret = pciDeviceFileIterate(pci, virSecuritySELinuxRestoreSecurityPCILabel, mgr); pciFreeDevice(pci); break; @@ -1367,7 +1399,8 @@ done: static int -virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virDomainHostdevDefPtr dev, +virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virSecurityManagerPtr mgr, + virDomainHostdevDefPtr dev, const char *vroot) { int ret = -1; @@ -1387,7 +1420,7 @@ virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virDomainHostdevDefPtr dev, return -1; } } - ret = virSecuritySELinuxRestoreSecurityFileLabel(path); + ret = virSecuritySELinuxRestoreSecurityFileLabel(mgr, path); VIR_FREE(path); break; } @@ -1405,7 +1438,7 @@ virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virDomainHostdevDefPtr dev, return -1; } } - ret = virSecuritySELinuxRestoreSecurityFileLabel(path); + ret = virSecuritySELinuxRestoreSecurityFileLabel(mgr, path); VIR_FREE(path); break; } @@ -1420,7 +1453,7 @@ virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virDomainHostdevDefPtr dev, static int -virSecuritySELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, +virSecuritySELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainHostdevDefPtr dev, const char *vroot) @@ -1437,10 +1470,10 @@ virSecuritySELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUT switch (dev->mode) { case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: - return virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(dev, vroot); + return virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(mgr, dev, vroot); case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: - return virSecuritySELinuxRestoreSecurityHostdevCapsLabel(dev, vroot); + return virSecuritySELinuxRestoreSecurityHostdevCapsLabel(mgr, dev, vroot); default: return 0; @@ -1522,7 +1555,8 @@ done: } static int -virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def, +virSecuritySELinuxRestoreSecurityChardevLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, virDomainChrDefPtr dev, virDomainChrSourceDefPtr dev_source) @@ -1545,14 +1579,14 @@ virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def, switch (dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - if (virSecuritySELinuxRestoreSecurityFileLabel(dev_source->data.file.path) < 0) + if (virSecuritySELinuxRestoreSecurityFileLabel(mgr, dev_source->data.file.path) < 0) goto done; ret = 0; break; case VIR_DOMAIN_CHR_TYPE_UNIX: if (!dev_source->data.nix.listen) { - if (virSecuritySELinuxRestoreSecurityFileLabel(dev_source->data.file.path) < 0) + if (virSecuritySELinuxRestoreSecurityFileLabel(mgr, dev_source->data.file.path) < 0) goto done; } ret = 0; @@ -1565,11 +1599,11 @@ virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def, goto done; } if (virFileExists(in) && virFileExists(out)) { - if ((virSecuritySELinuxRestoreSecurityFileLabel(out) < 0) || - (virSecuritySELinuxRestoreSecurityFileLabel(in) < 0)) { + if ((virSecuritySELinuxRestoreSecurityFileLabel(mgr, out) < 0) || + (virSecuritySELinuxRestoreSecurityFileLabel(mgr, in) < 0)) { goto done; } - } else if (virSecuritySELinuxRestoreSecurityFileLabel(dev_source->data.file.path) < 0) { + } else if (virSecuritySELinuxRestoreSecurityFileLabel(mgr, dev_source->data.file.path) < 0) { goto done; } ret = 0; @@ -1590,14 +1624,16 @@ done: static int virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def, virDomainChrDefPtr dev, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { + virSecurityManagerPtr mgr = opaque; + /* This is taken care of by processing of def->serials */ if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) return 0; - return virSecuritySELinuxRestoreSecurityChardevLabel(def, dev, + return virSecuritySELinuxRestoreSecurityChardevLabel(mgr, def, dev, &dev->source); } @@ -1605,8 +1641,9 @@ virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def, static int virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def, virDomainSmartcardDefPtr dev, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { + virSecurityManagerPtr mgr = opaque; const char *database; switch (dev->type) { @@ -1617,10 +1654,10 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def, database = dev->data.cert.database; if (!database) database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; - return virSecuritySELinuxRestoreSecurityFileLabel(database); + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, database); case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - return virSecuritySELinuxRestoreSecurityChardevLabel(def, NULL, &dev->data.passthru); + return virSecuritySELinuxRestoreSecurityChardevLabel(mgr, def, NULL, &dev->data.passthru); default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1670,21 +1707,21 @@ virSecuritySELinuxRestoreSecurityAllLabel(virSecurityManagerPtr mgr, if (virDomainChrDefForeach(def, false, virSecuritySELinuxRestoreSecurityChardevCallback, - NULL) < 0) + mgr) < 0) rc = -1; if (virDomainSmartcardDefForeach(def, false, virSecuritySELinuxRestoreSecuritySmartcardCallback, - NULL) < 0) + mgr) < 0) rc = -1; if (def->os.kernel && - virSecuritySELinuxRestoreSecurityFileLabel(def->os.kernel) < 0) + virSecuritySELinuxRestoreSecurityFileLabel(mgr, def->os.kernel) < 0) rc = -1; if (def->os.initrd && - virSecuritySELinuxRestoreSecurityFileLabel(def->os.initrd) < 0) + virSecuritySELinuxRestoreSecurityFileLabel(mgr, def->os.initrd) < 0) rc = -1; return rc; @@ -1737,7 +1774,7 @@ virSecuritySELinuxSetSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, static int -virSecuritySELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, +virSecuritySELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, const char *savefile) { @@ -1750,7 +1787,7 @@ virSecuritySELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNU if (secdef->norelabel) return 0; - return virSecuritySELinuxRestoreSecurityFileLabel(savefile); + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, savefile); } @@ -2080,7 +2117,7 @@ virSecuritySELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, } static int -virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, +virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, int fd) { @@ -2108,7 +2145,7 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, goto cleanup; } - if (getContext("/dev/tap.*", buf.st_mode, &fcon) < 0) { + if (getContext(mgr, "/dev/tap.*", buf.st_mode, &fcon) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot lookup default selinux label for tap fd %d"), fd); goto cleanup; -- 1.8.1

2 1

[libvirt] [PATCHv2] qemu: Re-add driver unlock to qemuDomainSendKey
by Viktor Mihajlovski 24 Jan '13

24 Jan '13

Should have been done in commit 56fd513 already, but was missed due to oversight: qemuDomainSendKey didn't release the driver lock in its cleanup section. This fixes an issue introduced by commit 8c5d2ba. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- V2 Changes: - Removed bogus hunk for qemuDomainManagedSave - Enhanced subject line and patch description src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 72907d2..6d4c1e9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2439,6 +2439,7 @@ endjob: cleanup: if (vm) virObjectUnlock(vm); + qemuDriverUnlock(driver); return ret; } -- 1.7.9.5

3 3

[libvirt] Non-blocking virStreamRecv always returns -2 (EAGAIN)?
by John Eckersberg 24 Jan '13

24 Jan '13

I'm running the following test program, and it works as written with a blocking stream. Inside the guest I'm running: [root@f17-minimal ~]# socat /usr/share/dict/words /dev/virtio-ports/org.libguestfs.channel.0 As expected on the client side, I get all the words dumped. However if I swap the virStreamNew lines and instead use the non-blocking stream, the virStreamRecv call always returns -2. >From http://libvirt.org/internals/rpc.html#apiclientdispatch, I see: When no thread is performing any RPC method call, or sending stream data there is still a need to monitor the socket for incoming I/O related to asynchronous events, or stream data receipt. For this task, a watch is registered with the event loop which triggers whenever the socket is readable. This watch is automatically disabled whenever any other thread grabs the buck, and re-enabled when the buck is released. If I understand that correctly, shouldn't the watch be responsible for reading the stream data in this case? Or am I just completely missing something? ----- #include <libvirt.h> int main() { virConnectPtr conn; virDomainPtr dom; virStreamPtr st; char buf[1024+1]; int got = 0; conn = virConnectOpen("qemu+ssh://root@localhost/system"); dom = virDomainLookupByName(conn, "f17-minimal"); /* st = virStreamNew(conn, VIR_STREAM_NONBLOCK); */ st = virStreamNew(conn, 0); virDomainOpenChannel(dom, "org.libguestfs.channel.0", st, 0); while (1) { got = virStreamRecv(st, buf, 1024); switch (got) { case 0: goto finish; case -1: goto free; case -2: puts("Retrying"); sleep(1); continue; } buf[got] = '\0'; puts(buf); } finish: virStreamFinish(st); free: virStreamFree(st); return 0; }

3 2

[libvirt] [PATCH] S390: Enhance memballoon handling for virtio-s390
by Viktor Mihajlovski 24 Jan '13

24 Jan '13

The way in that memory balloon suppression was handled for S390 is flawed for a number or reasons. 1. Just preventing the default balloon to be created in the case of VIR_ARCH_S390[X] is not sufficient. An explicit memballoon element in the guest definition will still be honored, resulting both in a -balloon option and the allocation of a PCI bus address, neither being supported. 2. Prohibiting balloon for S390 altogether at a domain_conf level is no good solution either as there's work in progress on the QEMU side to implement a virtio-balloon device, although in conjunction with a new machine type. Suppressing the balloon should therefore be done at the QEMU driver level depending on the present capabilities. Therefore we remove the conditional suppression of the default balloon in domain_conf.c. Further, we are claiming the memballoon device for virtio-s390 during device address assignment to prevent it from being considered as a PCI device. Finally, we suppress the generation of the balloon command line option if this is a virtio-s390 machine. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- src/conf/domain_conf.c | 4 +--- src/qemu/qemu_command.c | 9 ++++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6feded4..325e1ee 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10285,9 +10285,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->memballoon = memballoon; VIR_FREE(nodes); - } else if (def->os.arch != VIR_ARCH_S390 && - def->os.arch != VIR_ARCH_S390X) { - /* TODO: currently no balloon support on s390 -> no default balloon */ + } else { if (def->virtType == VIR_DOMAIN_VIRT_XEN || def->virtType == VIR_DOMAIN_VIRT_QEMU || def->virtType == VIR_DOMAIN_VIRT_KQEMU || diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 981c692..3496dbf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -795,7 +795,7 @@ qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, { /* declare address-less virtio devices to be of address type 'type' - only disks, networks, consoles and controllers for now + only disks, networks, consoles, controllers and memballoon for now */ int i; @@ -825,6 +825,10 @@ qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, def->controllers[i]->info.type = type; } + if (def->memballoon && + def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO && + def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->memballoon->info.type = type; } static void @@ -6956,6 +6960,9 @@ qemuBuildCommandLine(virConnectPtr conn, * NB: Earlier we declared that VirtIO balloon will always be in * slot 0x3 on bus 0x0 */ + if (qemuCapsGet(caps, QEMU_CAPS_VIRTIO_S390) && (def->memballoon)) + def->memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_NONE; + if ((def->memballoon) && (def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) { if (def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) { -- 1.7.9.5

2 3

[libvirt] [PATCH] qemu: Move one misplaced driver unlock to the right function
by Viktor Mihajlovski 23 Jan '13

23 Jan '13

This should have been commit 56fd513 already, but was missed by initially. The driver unlock call in the cleanup section of DomainManagedSave does actually belong to DomainSendKey. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c28c223..853e35d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2434,6 +2434,7 @@ endjob: cleanup: if (vm) virObjectUnlock(vm); + qemuDriverUnlock(driver); return ret; } @@ -3174,7 +3175,6 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) cleanup: if (vm) virObjectUnlock(vm); - qemuDriverUnlock(driver); VIR_FREE(name); return ret; -- 1.7.9.5

2 5

[libvirt] [PATCH v2] selinux: Only create the selabel_handle once.
by Richard W.M. Jones 23 Jan '13

23 Jan '13

From: "Richard W.M. Jones" <rjones(a)redhat.com> According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..d4f0595 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -935,20 +935,30 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) return 0; } +#if HAVE_SELINUX_LABEL_H + +static struct selabel_handle *seLabelHandle = NULL; + +static int +seLabelHandleOnceInit (void) +{ + seLabelHandle = selabel_open(SELABEL_CTX_FILE, NULL, 0); + return seLabelHandle ? 0 : -1; +} + +VIR_ONCE_GLOBAL_INIT(seLabelHandle) + +#endif + /* Set fcon to the appropriate label for path and mode, or return -1. */ static int getContext(const char *newpath, mode_t mode, security_context_t *fcon) { #if HAVE_SELINUX_LABEL_H - struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); - int ret; - - if (handle == NULL) + if (seLabelHandleInitialize() < 0) return -1; - ret = selabel_lookup_raw(handle, fcon, newpath, mode); - selabel_close(handle); - return ret; + return selabel_lookup_raw(seLabelHandle, fcon, newpath, mode); #else return matchpathcon(newpath, mode, fcon); #endif -- 1.8.1

2 3

[libvirt] [PATCH] selinux: Only create the selabel_handle once.
by Richard W.M. Jones 23 Jan '13

23 Jan '13

From: "Richard W.M. Jones" <rjones(a)redhat.com> According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..8b88785 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -935,20 +935,26 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) return 0; } +#if HAVE_SELINUX_LABEL_H +static struct selabel_handle *sehandle = NULL; +static virOnceControl sehandleonce = VIR_ONCE_CONTROL_INITIALIZER; + +static void +seHandleInit (void) +{ + sehandle = selabel_open(SELABEL_CTX_FILE, NULL, 0); +} +#endif + /* Set fcon to the appropriate label for path and mode, or return -1. */ static int getContext(const char *newpath, mode_t mode, security_context_t *fcon) { #if HAVE_SELINUX_LABEL_H - struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); - int ret; - - if (handle == NULL) + if (virOnce(&sehandleonce, seHandleInit) < 0 || sehandle == NULL) return -1; - ret = selabel_lookup_raw(handle, fcon, newpath, mode); - selabel_close(handle); - return ret; + return selabel_lookup_raw(sehandle, fcon, newpath, mode); #else return matchpathcon(newpath, mode, fcon); #endif -- 1.8.1

3 3

[libvirt] Network stops with CentOS 6.3 guest on Ubuntu 12.10
by Andy Howell 23 Jan '13

23 Jan '13

I'm still have not resolved this, any idea how to debug it? Thanks, Andy Hello, I'm trying to get CentOS 6.3 running under Ubuntu 12.10. When the interface comes up, I can ping another machine for 9 packets. After that it stops working. Bouncing the guest network interface gets it going again for another 10 packets. Monitoring the vnet0 interface on the host, I see ARP requests from the host to the guest are not getting replies. I cleared out iptables to eliminate that. No luck. I have Fedora 17 and Windows 7 running on the machine with no problems. Any ideas? Thanks, Andy -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

1 0

[libvirt] [PATCH] Ensure nodeinfo struct is initialized to zero
by Daniel P. Berrange 23 Jan '13

23 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> When linuxNodeInfoCPUPopulate() method triggered use of an uninitialize value, since it did not initialize the 'sockets' field in the virNodeInfoPtr struct: ==30020== Conditional jump or move depends on uninitialised value(s) ==30020== at 0x5125DBD: linuxNodeInfoCPUPopulate (nodeinfo.c:513) ==30020== by 0x51261A0: nodeGetInfo (nodeinfo.c:884) ==30020== by 0x149B9B10: qemuCapsInit (qemu_capabilities.c:846) ==30020== by 0x14A11B25: qemuCreateCapabilities (qemu_driver.c:424) ==30020== by 0x14A12426: qemuStartup (qemu_driver.c:874) ==30020== by 0x512A7AF: virStateInitialize (libvirt.c:822) ==30020== by 0x40DE04: daemonRunStateInit (libvirtd.c:877) ==30020== by 0x50ADCE5: virThreadHelper (virthreadpthread.c:161) ==30020== by 0x328CA07D14: start_thread (pthread_create.c:308) ==30020== by 0x328C6F246C: clone (clone.S:114) (happened twice) if (socks > nodeinfo->sockets) <--- here nodeinfo->sockets = socks; Rather than doing this for each field, just make the caller memset the entire struct to zero. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/nodeinfo.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 477104f..5b91a12 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -387,11 +387,6 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, char *sysfs_nodedir = NULL; char *sysfs_cpudir = NULL; - nodeinfo->cpus = 0; - nodeinfo->mhz = 0; - nodeinfo->cores = 0; - nodeinfo->nodes = 0; - /* Start with parsing CPU clock speed from /proc/cpuinfo */ while (fgets(line, sizeof(line), cpuinfo) != NULL) { # if defined(__x86_64__) || \ @@ -868,6 +863,8 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { virArch hostarch = virArchFromHost(); + memset(nodeinfo, 0, sizeof(nodeinfo)); + if (virStrcpyStatic(nodeinfo->model, virArchToString(hostarch)) == NULL) return -1; -- 1.8.1

4 9

[libvirt] [PATCH] Avoid use of free'd memory in auto destroy callback
by Daniel P. Berrange 23 Jan '13

23 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The autodestroy callback code has the following function called from a hash iterator qemuDriverCloseCallbackRun(void *payload, const void *name, void *opaque) { ... char *uuidstr = name ... dom = closeDef->cb(data->driver, dom, data->conn); if (dom) virObjectUnlock(dom); virHashRemoveEntry(data->driver->closeCallbacks, uuidstr); } The closeDef->cb function may well cause the current callback to be removed, if it shuts down 'dom'. As such the use of 'uuidstr' in virHashRemoveEntry is accessing free'd memory. We must make a copy of the uuid str before invoking the callback to be safe. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_conf.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index b21392e..ee48333 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -510,20 +510,25 @@ qemuDriverCloseCallbackRun(void *payload, { struct qemuDriverCloseCallbackData *data = opaque; qemuDriverCloseDefPtr closeDef = payload; - const char *uuidstr = name; unsigned char uuid[VIR_UUID_BUFLEN]; + char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObjPtr dom; VIR_DEBUG("conn=%p, thisconn=%p, uuid=%s, cb=%p", - closeDef->conn, data->conn, uuidstr, closeDef->cb); + closeDef->conn, data->conn, (const char *)name, closeDef->cb); if (data->conn != closeDef->conn || !closeDef->cb) return; - if (virUUIDParse(uuidstr, uuid) < 0) { - VIR_WARN("Failed to parse %s", uuidstr); + if (virUUIDParse(name, uuid) < 0) { + VIR_WARN("Failed to parse %s", (const char *)name); return; } + /* We need to reformat uuidstr, because closeDef->cb + * might cause the current hash entry to be removed, + * which means 'name' will have been free()d + */ + virUUIDFormat(uuid, uuidstr); if (!(dom = virDomainFindByUUID(&data->driver->domains, uuid))) { VIR_DEBUG("No domain object with UUID %s", uuidstr); -- 1.8.1

3 2