- Devel - libvirt List Archives

[libvirt] [PATCH] security_dac: Honor norelabel attribute
by Michal Privoznik 26 Mar '14

26 Mar '14

The inspiration for this patch comes from a question on the list asking if there's a way to not label some disks. Well, in DAC driver there's not. Even if user have requested norelabel: <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/some/dummy/path/test.bin'> <seclabel model='dac' relabel='no'/> </source> <target dev='vdb' bus='virtio'/> <readonly/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </disk> the DAC driver ignores this completely. When adjusting the code, I realized it's a ragbag with plenty of things that we try to avoid. >From the variety just a few things: callback data were passed as: void params[2]; params[0] = mgr; params[1] = def; Or my favorite - checking for passed pointer being non NULL on each level of the stack, in each callee. As a pattern of readable code the selinux driver was used. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/security/security_dac.c | 244 ++++++++++++++++++++++++-------------------- 1 file changed, 131 insertions(+), 113 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9f45063..3b8fe04 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -53,6 +53,15 @@ struct _virSecurityDACData { char *baselabel; }; +typedef struct _virSecurityDACCallbackData virSecurityDACCallbackData; +typedef virSecurityDACCallbackData *virSecurityDACCallbackDataPtr; + +struct _virSecurityDACCallbackData { + virSecurityManagerPtr manager; + virSecurityLabelDefPtr secdef; +}; + + /* returns -1 on error, 0 on success */ int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr, @@ -81,65 +90,42 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, /* returns 1 if label isn't found, 0 on success, -1 on error */ static int -virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) +virSecurityDACParseIds(virSecurityLabelDefPtr seclabel, + uid_t *uidPtr, gid_t *gidPtr) { - uid_t uid; - gid_t gid; - virSecurityLabelDefPtr seclabel; - - if (def == NULL) - return 1; - - seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - if (seclabel == NULL || seclabel->label == NULL) { - VIR_DEBUG("DAC seclabel for domain '%s' wasn't found", def->name); + if (!seclabel || !seclabel->label) return 1; - } - if (virParseOwnershipIds(seclabel->label, &uid, &gid) < 0) + if (virParseOwnershipIds(seclabel->label, uidPtr, gidPtr) < 0) return -1; - if (uidPtr) - *uidPtr = uid; - if (gidPtr) - *gidPtr = gid; - return 0; } static int -virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, +virSecurityDACGetIds(virSecurityLabelDefPtr seclabel, + virSecurityDACDataPtr priv, uid_t *uidPtr, gid_t *gidPtr, gid_t **groups, int *ngroups) { int ret; - if (!def && !priv) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to determine default DAC seclabel " - "for an unknown object")); - return -1; - } - if (groups) *groups = priv ? priv->groups : NULL; if (ngroups) *ngroups = priv ? priv->ngroups : 0; - if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) + if ((ret = virSecurityDACParseIds(seclabel, uidPtr, gidPtr)) <= 0) return ret; if (!priv) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("DAC seclabel couldn't be determined " - "for domain '%s'"), def->name); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("DAC seclabel couldn't be determined")); return -1; } - if (uidPtr) - *uidPtr = priv->user; - if (gidPtr) - *gidPtr = priv->group; + *uidPtr = priv->user; + *gidPtr = priv->group; return 0; } @@ -147,60 +133,36 @@ virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, /* returns 1 if label isn't found, 0 on success, -1 on error */ static int -virSecurityDACParseImageIds(virDomainDefPtr def, +virSecurityDACParseImageIds(virSecurityLabelDefPtr seclabel, uid_t *uidPtr, gid_t *gidPtr) { - uid_t uid; - gid_t gid; - virSecurityLabelDefPtr seclabel; - - if (def == NULL) - return 1; - - seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - if (seclabel == NULL || seclabel->imagelabel == NULL) { - VIR_DEBUG("DAC imagelabel for domain '%s' wasn't found", def->name); + if (!seclabel || !seclabel->imagelabel) return 1; - } - if (virParseOwnershipIds(seclabel->imagelabel, &uid, &gid) < 0) + if (virParseOwnershipIds(seclabel->imagelabel, uidPtr, gidPtr) < 0) return -1; - if (uidPtr) - *uidPtr = uid; - if (gidPtr) - *gidPtr = gid; - return 0; } static int -virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv, +virSecurityDACGetImageIds(virSecurityLabelDefPtr seclabel, + virSecurityDACDataPtr priv, uid_t *uidPtr, gid_t *gidPtr) { int ret; - if (!def && !priv) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to determine default DAC imagelabel " - "for an unknown object")); - return -1; - } - - if ((ret = virSecurityDACParseImageIds(def, uidPtr, gidPtr)) <= 0) + if ((ret = virSecurityDACParseImageIds(seclabel, uidPtr, gidPtr)) <= 0) return ret; if (!priv) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("DAC imagelabel couldn't be determined " - "for domain '%s'"), def->name); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("DAC imagelabel couldn't be determined")); return -1; } - if (uidPtr) - *uidPtr = priv->user; - if (gidPtr) - *gidPtr = priv->group; + *uidPtr = priv->user; + *gidPtr = priv->group; return 0; } @@ -324,20 +286,32 @@ err: static int -virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, +virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk, const char *path, size_t depth ATTRIBUTE_UNUSED, void *opaque) { - void **params = opaque; - virSecurityManagerPtr mgr = params[0]; - virDomainDefPtr def = params[1]; + virSecurityDACCallbackDataPtr cbdata = opaque; + virSecurityManagerPtr mgr = cbdata->manager; + virSecurityLabelDefPtr secdef = cbdata->secdef; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityDeviceLabelDefPtr disk_seclabel; uid_t user; gid_t group; - if (virSecurityDACGetImageIds(def, priv, &user, &group)) - return -1; + disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk, + SECURITY_DAC_NAME); + + if (disk_seclabel && disk_seclabel->norelabel) + return 0; + + if (disk_seclabel && disk_seclabel->label) { + if (virParseOwnershipIds(disk_seclabel->label, &user, &group) < 0) + return -1; + } else { + if (virSecurityDACGetImageIds(secdef, priv, &user, &group)) + return -1; + } return virSecurityDACSetOwnership(path, user, group); } @@ -349,8 +323,9 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainDiskDefPtr disk) { - void *params[2]; + virSecurityDACCallbackData cbdata; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; if (!priv->dynamicOwnership) return 0; @@ -358,12 +333,16 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) return 0; - params[0] = mgr; - params[1] = def; + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + if (secdef && secdef->norelabel) + return 0; + + cbdata.manager = mgr; + cbdata.secdef = secdef; return virDomainDiskDefForeachPath(disk, false, virSecurityDACSetSecurityFileLabel, - params); + &cbdata); } @@ -428,14 +407,14 @@ static int virSecurityDACSetSecurityHostdevLabelHelper(const char *file, void *opaque) { - void **params = opaque; - virSecurityManagerPtr mgr = params[0]; - virDomainDefPtr def = params[1]; + virSecurityDACCallbackDataPtr cbdata = opaque; + virSecurityManagerPtr mgr = cbdata->manager; + virSecurityLabelDefPtr secdef = cbdata->secdef; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); uid_t user; gid_t group; - if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) + if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL)) return -1; return virSecurityDACSetOwnership(file, user, group); @@ -475,8 +454,8 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainHostdevDefPtr dev, const char *vroot) { - void *params[] = {mgr, def}; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityDACCallbackData cbdata; int ret = -1; if (!priv->dynamicOwnership) @@ -485,7 +464,13 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) return 0; - switch (dev->source.subsys.type) { + cbdata.manager = mgr; + cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + + if (cbdata.secdef && cbdata.secdef->norelabel) + return 0; + + switch ((enum virDomainHostdevSubsysType) dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { virUSBDevicePtr usb; @@ -498,8 +483,9 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (!usb) goto done; - ret = virUSBDeviceFileIterate(usb, virSecurityDACSetSecurityUSBLabel, - params); + ret = virUSBDeviceFileIterate(usb, + virSecurityDACSetSecurityUSBLabel, + &cbdata); virUSBDeviceFree(usb); break; } @@ -522,11 +508,12 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, virPCIDeviceFree(pci); goto done; } - ret = virSecurityDACSetSecurityPCILabel(pci, vfioGroupDev, params); + ret = virSecurityDACSetSecurityPCILabel(pci, vfioGroupDev, &cbdata); VIR_FREE(vfioGroupDev); } else { - ret = virPCIDeviceFileIterate(pci, virSecurityDACSetSecurityPCILabel, - params); + ret = virPCIDeviceFileIterate(pci, + virSecurityDACSetSecurityPCILabel, + &cbdata); } virPCIDeviceFree(pci); @@ -546,15 +533,15 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (!scsi) goto done; - ret = virSCSIDeviceFileIterate(scsi, virSecurityDACSetSecuritySCSILabel, - params); + ret = virSCSIDeviceFileIterate(scsi, + virSecurityDACSetSecuritySCSILabel, + &cbdata); virSCSIDeviceFree(scsi); break; } - default: - ret = 0; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: break; } @@ -606,7 +593,7 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) return 0; - switch (dev->source.subsys.type) { + switch ((enum virDomainHostdevSubsysType) dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { virUSBDevicePtr usb; @@ -684,34 +671,52 @@ done: static int virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, - virDomainChrSourceDefPtr dev) + virDomainChrDefPtr dev, + virDomainChrSourceDefPtr dev_source) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr seclabel; + virSecurityDeviceLabelDefPtr chr_seclabel = NULL; char *in = NULL, *out = NULL; int ret = -1; uid_t user; gid_t group; - if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) - return -1; + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - switch (dev->type) { + if (dev) + chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev, + SECURITY_DAC_NAME); + + if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel)) + return 0; + + if (chr_seclabel && chr_seclabel->label) { + if (virParseOwnershipIds(chr_seclabel->label, &user, &group) < 0) + return -1; + } else { + if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL)) + return -1; + } + + switch ((enum virDomainChrType) dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - ret = virSecurityDACSetOwnership(dev->data.file.path, user, group); + ret = virSecurityDACSetOwnership(dev_source->data.file.path, + user, group); break; case VIR_DOMAIN_CHR_TYPE_PIPE: - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) + if ((virAsprintf(&in, "%s.in", dev_source->data.file.path) < 0) || + (virAsprintf(&out, "%s.out", dev_source->data.file.path) < 0)) goto done; if (virFileExists(in) && virFileExists(out)) { if ((virSecurityDACSetOwnership(in, user, group) < 0) || (virSecurityDACSetOwnership(out, user, group) < 0)) { goto done; } - } else if (virSecurityDACSetOwnership(dev->data.file.path, + } else if (virSecurityDACSetOwnership(dev_source->data.file.path, user, group) < 0) { goto done; } @@ -736,7 +741,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, char *in = NULL, *out = NULL; int ret = -1; - switch (dev->type) { + switch ((enum virDomainChrType) dev->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: ret = virSecurityDACRestoreSecurityFileLabel(dev->data.file.path); @@ -789,7 +794,7 @@ virSecurityDACSetSecurityTPMFileLabel(virSecurityManagerPtr mgr, switch (tpm->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: - ret = virSecurityDACSetChardevLabel(mgr, def, + ret = virSecurityDACSetChardevLabel(mgr, def, NULL, &tpm->data.passthrough.source); break; case VIR_DOMAIN_TPM_TYPE_LAST: @@ -885,7 +890,7 @@ virSecurityDACSetChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, { virSecurityManagerPtr mgr = opaque; - return virSecurityDACSetChardevLabel(mgr, def, &dev->source); + return virSecurityDACSetChardevLabel(mgr, def, dev, &dev->source); } @@ -895,13 +900,17 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, const char *stdin_path ATTRIBUTE_UNUSED) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; size_t i; uid_t user; gid_t group; - if (!priv->dynamicOwnership) + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + + if (!priv->dynamicOwnership || (secdef && secdef->norelabel)) return 0; + for (i = 0; i < def->ndisks; i++) { /* XXX fixme - we need to recursively label the entire tree :-( */ if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR) @@ -932,7 +941,7 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, return -1; } - if (virSecurityDACGetImageIds(def, priv, &user, &group)) + if (virSecurityDACGetImageIds(secdef, priv, &user, &group)) return -1; if (def->os.kernel && @@ -956,11 +965,14 @@ virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, const char *savefile) { + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; uid_t user; gid_t group; - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - if (virSecurityDACGetImageIds(def, priv, &user, &group)) + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + + if (virSecurityDACGetImageIds(secdef, priv, &user, &group)) return -1; return virSecurityDACSetOwnership(savefile, user, group); @@ -985,13 +997,16 @@ static int virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def ATTRIBUTE_UNUSED) { + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; uid_t user; gid_t group; gid_t *groups; int ngroups; - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - if (virSecurityDACGetIds(def, priv, &user, &group, &groups, &ngroups)) + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + + if (virSecurityDACGetIds(secdef, priv, &user, &group, &groups, &ngroups)) return -1; VIR_DEBUG("Dropping privileges of DEF to %u:%u, %d supplemental groups", @@ -1009,11 +1024,14 @@ virSecurityDACSetChildProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def ATTRIBUTE_UNUSED, virCommandPtr cmd) { + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityLabelDefPtr secdef; uid_t user; gid_t group; - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) + secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); + + if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL)) return -1; VIR_DEBUG("Setting child to drop privileges of DEF to %u:%u", -- 1.9.0

2 1

[libvirt] [PATCH v2] dnsmasq: allowing RFC 2782 compliant SRV records
by Steven Malin 26 Mar '14

26 Mar '14

This patch allows RFC 2782 compliant SRV records in the network config and corrects the documentation. --- docs/formatnetwork.html.in | 6 +++--- src/conf/network_conf.c | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index fc56b42..9d3e525 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -721,7 +721,7 @@ <txt name="example" value="example value" /> <forwarder addr="8.8.8.8"/> <forwarder addr="8.8.4.4"/> - <srv service='name' protocol='tcp' domain='test-domain-name' target='.' port='1024' priority='10' weight='10'/> + <srv service='_name' protocol='_tcp' domain='test-domain-name' target='.' port='1024' priority='10' weight='10'/> <host ip='192.168.122.2'> <hostname>myhost</hostname> <hostname>myhostalias</hostname> @@ -805,8 +805,8 @@ <dd>The <code>dns</code> element can have also 0 or more <code>srv</code> record elements. Each <code>srv</code> record element defines a DNS SRV record and has 2 mandatory and 5 optional attributes. The mandatory attributes - are service name and protocol (tcp, udp) and the optional attributes are - target, port, priority, weight and domain as defined in DNS server SRV + are service name (_name) and protocol (_tcp, _udp) and the optional attributes + are target, port, priority, weight and domain as defined in DNS server SRV RFC (RFC 2782). <span class="since">Since 0.9.9</span> </dd> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index bac0465..c7d55c8 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -961,9 +961,11 @@ virNetworkDNSSrvDefParseXML(const char *networkName, goto error; } - /* Check whether protocol value is the supported one */ + /* Check whether protocol value is supported (allows RFC 2782 compliance) */ if (def->protocol && STRNEQ(def->protocol, "tcp") && - (STRNEQ(def->protocol, "udp"))) { + STRNEQ(def->protocol, "udp") && + STRNEQ(def->protocol, "_tcp") && + STRNEQ(def->protocol, "_udp")) { virReportError(VIR_ERR_XML_DETAIL, _("Invalid protocol attribute value '%s' " "in DNS SRV record of network %s"), -- 1.7.1

2 1

[libvirt] [PATCH 0/5] virCommandRunRegex cleanups
by Ján Tomko 26 Mar '14

26 Mar '14

Simplify the code. This should have no functional change (except for the impossible leak fix in 3/5). Most of this functionality is tested in viriscsitest. Ján Tomko (5): Remove useless 'maxReg' variable Simplify the loop in virCommandRunRegex Free groups in case of a partial match Use VIR_STRNDUP instead of modifying the matched string Shift the for loop over matched vars by one src/util/vircommand.c | 46 +++++++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 27 deletions(-) -- 1.8.3.2

2 7

[libvirt] [RFC] VM which uses macvtap will not respond ping request when being migrated
by Wangrui (K) 26 Mar '14

26 Mar '14

A vm which uses macvtap will not respond ping request, when the vm is being migrated. I found that on the destination side the macvtap will send a IPv6 packet at the begin of migration to update the route table in switch, however VM is still on the src. In this case , what can I do to avoid VM's network disconnection Regards, Wangrui

3 3

[libvirt] [PATCH 0/3] v2: qemu: export disk snapshot capability
by Francesco Romani 26 Mar '14

26 Mar '14

This patch series extend the QEMU capabilities XML to report if the underlying QEMU binary supports, or not, the live disk snapshotting. Without this patch series, the only way to know if QEMU has this support is to actually request a disk snapshot and to see what happens. The change is split in three patches: * patch #1 actually adds the new element in the QEMU capabilities XML. formatcaps.html.in wasn't very detailed about the actual XML format, so I've not updated it. Anyone feel free to point out what should be added, and I'll comply. The new element has the form <disksnapshot default='value' toggle='off'> because I'd like to convey two informations: - disk snapshot is supposed to be here, and it is (default='on') - disk snapshot is supposed to be here, and is NOT (default='off') Put in a different way, I tried to help the client as much as possible. This patch was alread reviewed in the first version of this changeset and it is unchanged. * patch #2 Extracts the actual QMEU guest capability inizialitation from the binary probe/capabilities code, in order to isolate the actual initialization from the probing part. I added a new function for the real initialization and left the interface unchanged. Existing code will still call the old code which do the probing and after that calls the new initialization function. The main purpose of this patch is to * allow to write an unit test for this change * make the unit test more robust (with respect the first version of this change) and make it independent from the filesystem layout/qemu availability * patch #3 add a new unit test, aiming to test not only this new feature but also the whole output XML capabilities. Francesco Romani (3): qemu: export disk snapshot support in capabilities qemu: extract guest capabilities initialization qemu: add unit tests for the capabilities xml docs/schemas/capability.rng | 6 + src/qemu/qemu_capabilities.c | 50 +++-- src/qemu/qemu_capabilities.h | 7 + tests/Makefile.am | 10 +- tests/qemucaps2xmldata/all_1.6.0-1.caps | 142 ++++++++++++++ tests/qemucaps2xmldata/all_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 ++++++++++++++ tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml | 31 ++++ tests/qemucaps2xmltest.c | 206 +++++++++++++++++++++ 9 files changed, 609 insertions(+), 15 deletions(-) create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.xml create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml create mode 100644 tests/qemucaps2xmltest.c -- 1.8.5.3

2 7

[libvirt] [PATCH v2 0/2] bhyve: add xml2args unittest
by Roman Bogorodskiy 26 Mar '14

26 Mar '14

Changes from v1: - Chase MAC address support by adding virMacAddrGenerate() mock, so we can get a constant MAC address - Add a test for the case when MAC address is specified in the domain xml Roman Bogorodskiy (2): Move virBhyveTapGetRealDeviceName to virnetdevtap bhyve: add xml2args unittest src/bhyve/bhyve_command.c | 70 +--------- src/libvirt_private.syms | 1 + src/util/virnetdevtap.c | 78 +++++++++++ src/util/virnetdevtap.h | 3 + tests/Makefile.am | 25 ++++ .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 3 + tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml | 24 ++++ tests/bhyvexml2argvdata/bhyvexml2argv-base.args | 3 + tests/bhyvexml2argvdata/bhyvexml2argv-base.xml | 20 +++ .../bhyvexml2argv-disk-virtio.args | 3 + .../bhyvexml2argv-disk-virtio.xml | 20 +++ tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args | 3 + tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml | 21 +++ tests/bhyvexml2argvmock.c | 49 +++++++ tests/bhyvexml2argvtest.c | 155 +++++++++++++++++++++ 15 files changed, 409 insertions(+), 69 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml create mode 100644 tests/bhyvexml2argvmock.c create mode 100644 tests/bhyvexml2argvtest.c -- 1.8.4.2

2 5

[libvirt] [PATCH] qemuDomainAttachDeviceFlags: Parse device xml as inactive
by Michal Privoznik 26 Mar '14

26 Mar '14

In all other drivers we are doing so. Moreover, we don't want to parse runtime information in attach (even if the attach is meant as live) because we are generating the runtime info ourselves. We can't trust users they supply sane values anyway. ==1140== 9 bytes in 1 blocks are definitely lost in loss record 72 of 1,151 ==1140== at 0x4A06C2B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==1140== by 0x623C758: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1) ==1140== by 0x50FD763: virXMLPropString (virxml.c:483) ==1140== by 0x510F8B7: virDomainDeviceInfoParseXML (domain_conf.c:3685) ==1140== by 0x511ACFD: virDomainChrDefParseXML (domain_conf.c:7535) ==1140== by 0x5121D13: virDomainDeviceDefParse (domain_conf.c:9918) ==1140== by 0x13AE6313: qemuDomainAttachDeviceFlags (qemu_driver.c:6926) ==1140== by 0x13AE65FA: qemuDomainAttachDevice (qemu_driver.c:7005) ==1140== by 0x51C77DA: virDomainAttachDevice (libvirt.c:10231) ==1140== by 0x127FDD: remoteDispatchDomainAttachDevice (remote_dispatch.h:2404) ==1140== by 0x127EC5: remoteDispatchDomainAttachDeviceHelper (remote_dispatch.h:2382) ==1140== by 0x5241F81: virNetServerProgramDispatchCall (virnetserverprogram.c:437) When doing live attach, we are passing the inactive definition anyway since we are passing the result of virDomainDeviceDefCopy() which does inactive copy by default. Moreover, we are doing the same mistake in qemuhotplugtest. Just a side note - it makes perfect sense to parse the runtime info like alias in qemuDomainDetachDevice and qemuDomainUpdateDeviceFlags() as in some cases the only difference to distinguish two devices can be just their alias. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_driver.c | 6 +----- tests/qemuhotplugtest.c | 7 ++++++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2707bec..8678d24 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6871,7 +6871,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virDomainDefPtr vmdef = NULL; virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; int ret = -1; - unsigned int affect, parse_flags = 0; + unsigned int affect, parse_flags = VIR_DOMAIN_XML_INACTIVE; virQEMUCapsPtr qemuCaps = NULL; qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; @@ -6919,10 +6919,6 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, goto endjob; } - if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && - !(flags & VIR_DOMAIN_AFFECT_LIVE)) - parse_flags |= VIR_DOMAIN_XML_INACTIVE; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 4ef81e0..a7a4065 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -209,6 +209,7 @@ testQemuHotplug(const void *data) const char *const *tmp; bool fail = test->fail; bool keep = test->keep; + unsigned int device_parse_flags = 0; virDomainObjPtr vm = NULL; virDomainDeviceDefPtr dev = NULL; virCapsPtr caps = NULL; @@ -244,8 +245,12 @@ testQemuHotplug(const void *data) goto cleanup; } + if (test->action == ATTACH) + device_parse_flags = VIR_DOMAIN_XML_INACTIVE; + if (!(dev = virDomainDeviceDefParse(device_xml, vm->def, - caps, driver.xmlopt, 0))) + caps, driver.xmlopt, + device_parse_flags))) goto cleanup; /* Now is the best time to feed the spoofed monitor with predefined -- 1.9.0

2 1

[libvirt] [PATCH v5 0/2] fix query-command-line-options
by Amos Kong 26 Mar '14

26 Mar '14

This patchset fixed some issues of query-command-line-options: * some new options that haven't argument can't be queried. (eg: -enable-fips) * some legacy options that have argument can't be queried. (eg: -vnc display) More discussion: http://marc.info/?l=qemu-devel&m=139081830416684&w=2 V2: remove duplicate option tables, update schema (eric) V3: fix typo in commitlog and export qemu_options talbe (eric) V4: avoid the duplicate static table (eric) V5: rename new field, other fix (markus) Thanks for your review! Amos Kong (2): qmp: rename query_option_descs() to get_param_infolist() query-command-line-options: query all the options in qemu-options.hx qapi-schema.json | 9 +++++++-- qemu-options.h | 12 ++++++++++++ util/qemu-config.c | 49 +++++++++++++++++++++++++++++++++++++++---------- vl.c | 19 ++----------------- 4 files changed, 60 insertions(+), 29 deletions(-) -- 1.8.5.3

2 4

[libvirt] [libvirt-java] [PATCH 0/2] Ensure JNA callbacks cannot be GCed
by Chris Ellis 26 Mar '14

26 Mar '14

Hi. This is a small patch set to fix a few issues I've discovered while testing Claudio's patch set. The first patch is trivial, it adds the ConnectionCloseListener interface and corresponding enum, which were missing. The second patch ensures that the JNA callbacks cannot be garbage collected whilst still registered with the libvirt C library. Durring testing, I was finding that events would stop working. Wireshark showed the remote daemon sending the event and some time spent tracing through the libvirt calls led me to discover that the JNA callback objects were being GCed. The JNA documentation states that a reference to the Callback object must be held whilst it is in use by the C layer. This patch updates the map of event listeners to also hold a reference to the callback object. Regards, Chris Chris Ellis (2): Add close listener types Ensure JNA callbacks cannot get GCed src/main/java/org/libvirt/Connect.java | 37 ++++++++++++++-------- .../org/libvirt/event/ConnectionCloseListener.java | 9 ++++++ .../org/libvirt/event/ConnectionCloseReason.java | 8 +++++ 3 files changed, 41 insertions(+), 13 deletions(-) create mode 100644 src/main/java/org/libvirt/event/ConnectionCloseListener.java create mode 100644 src/main/java/org/libvirt/event/ConnectionCloseReason.java -- 1.8.4.5

2 5

[libvirt] question for <interface> in ethernet type and multiple queues in KVM (start VM failed)
by Wangrui (K) 26 Mar '14

26 Mar '14

Hi, I use ethernet vif for VM (libvirt 1.1.0 qemu 1.5.1). xml as such <interface type='ethernet'> <script path='' /> <target dev='tap_mq'/> <mac address='52:54:00:19:9e:4e'/> <model type='virtio' /> <driver name='vhost' queues='2'/> </interface> tap_mq is a tap device with multi_queue property which was created on host by ip command, such as follows #ip tuntap add tap_mq mode tap multi_queue I want to use multiple queue feature for this vif so I set queues='2' in xml. When start VM , an error occurs "error: unsupported configuration: Multiqueue network is not supported for: ethernet". This error is reported from function qemuBuildInterfaceCommandLine /* Currently nothing besides TAP devices supports multiqueue. */ if (net->driver.virtio.queues > 0 && !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Multiqueue network is not supported for: %s"), virDomainNetTypeToString(actualType)); return -1; } The comment specifies that only TAP device supports multiqueue. But VIR_DOMAIN_NET_TYPE_ETHERNET is also TAP device I think. IMHO, libvirt can build qemu commandline as "-device virtio-net,netdev=net1,mq=on,vectors=5,mac=52:54:00:19:9e:4e" "-netdev type=tap,ifname=tap0,id=net1,vhost=on,vhostforce=on,queues=2". Qemu can open tapfds and vhostfds by itself other than libvirt opens and transfers them to qemu in this case. Or there's other way to use multiqueue feature in ethernet tpye ? Regards, Rui

4 4

[libvirt] [PATCH] maint: use $(SED) instead of sed for syntax-check
by Roman Bogorodskiy 26 Mar '14

26 Mar '14

Some syntax-check rules use GNU sed specific regexps, so allow to override which sed would be used to fix 'syntax-check' for non GNU-userland systems. --- cfg.mk | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cfg.mk b/cfg.mk index 207dfeb..bd984bd 100644 --- a/cfg.mk +++ b/cfg.mk @@ -614,7 +614,7 @@ sc_libvirt_unmarked_diagnostics: $(_sc_search_regexp) @{ grep -nE '\<$(func_re) *$.*;$$' $$($(VC_LIST_EXCEPT)); \ grep -A1 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \ - | sed 's/_("\([^\"]\|\\.$\+"//;s/[ ]"%s"//' \ + | $(SED) 's/_("$[^\"]\|\\.$\+"//;s/[ ]"%s"//' \ | grep '[ ]"' && \ { echo '$(ME): found unmarked diagnostic(s)' 1>&2; \ exit 1; } || : @@ -639,7 +639,7 @@ sc_prohibit_newline_at_end_of_diagnostic: sc_prohibit_diagnostic_without_format: @{ grep -nE '\<$(func_re) *$.*;$$' $$($(VC_LIST_EXCEPT)); \ grep -A2 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \ - | sed -rn -e ':l; /[,"]$$/ {N;b l;}' \ + | $(SED) -rn -e ':l; /[,"]$$/ {N;b l;}' \ -e '/(xenapiSessionErrorHandler|vah_(error|warning))/d' \ -e '/\<$(func_re) *\([^"]*"([^%"]|"\n[^"]*")*"[,)]/p' \ | grep -vE 'VIR_ERROR' && \ @@ -661,7 +661,7 @@ sc_prohibit_useless_translation: # or \n on one side of the split. sc_require_whitespace_in_translation: @grep -n -A1 '"$$' $$($(VC_LIST_EXCEPT)) \ - | sed -ne ':l; /"$$/ {N;b l;}; s/"\n[^"]*"/""/g; s/\\n/ /g' \ + | $(SED) -ne ':l; /"$$/ {N;b l;}; s/"\n[^"]*"/""/g; s/\\n/ /g' \ -e '/_(.*[^\ ]""[^\ ]/p' | grep . && \ { echo '$(ME): missing whitespace at line split' 1>&2; \ exit 1; } || : @@ -681,11 +681,11 @@ sc_preprocessor_indentation: sc_spec_indentation: @if cppi --version >/dev/null 2>&1; then \ for f in $$($(VC_LIST_EXCEPT) | grep '\.spec\.in$$'); do \ - sed -e 's|#|// #|; s|%ifn*\(arch$* |#if a // |' \ + $(SED) -e 's|#|// #|; s|%ifn*$arch$* |#if a // |' \ -e 's/%$else\|endif\|define$/#\1/' \ -e 's/^$ *$\1\1\1#/#\1/' \ -e 's|^$ *[^#/ ]$|// \1|; s|^$ */[^/]$|// \1|' $$f \ - | cppi -a -c 2>&1 | sed "s|standard input|$$f|"; \ + | cppi -a -c 2>&1 | $(SED) "s|standard input|$$f|"; \ done | { if grep . >&2; then false; else :; fi; } \ || { echo '$(ME): incorrect preprocessor indentation' 1>&2; \ exit 1; }; \ @@ -777,7 +777,7 @@ sc_prohibit_cross_inclusion: # elements added to the enum by using a _LAST marker. sc_require_enum_last_marker: @grep -A1 -nE '^[^#]*VIR_ENUM_IMPL *\(' $$($(VC_LIST_EXCEPT)) \ - | sed -ne '/VIR_ENUM_IMPL[^,]*,$$/N' \ + | $(SED) -ne '/VIR_ENUM_IMPL[^,]*,$$/N' \ -e '/VIR_ENUM_IMPL[^,]*,[^,]*[^_,][^L,][^A,][^S,][^T,],/p' \ -e '/VIR_ENUM_IMPL[^,]*,[^,]\{0,4\},/p' \ | grep . && \ @@ -878,7 +878,7 @@ ifeq (0,$(MAKELEVEL)) # b653eda3ac4864de205419d9f41eec267cb89eeb # # Keep this logic in sync with autogen.sh. - _submodule_hash = sed 's/^[ +-]//;s/ .*//' + _submodule_hash = $(SED) 's/^[ +-]//;s/ .*//' _update_required := $(shell \ cd '$(srcdir)'; \ test -d .git || { echo 0; exit; }; \ -- 1.8.4.3

3 4

[libvirt] [PATCH] qemu: remove redundant virQEMUDriverGetConfig
by Tomoki Sekiyama 26 Mar '14

26 Mar '14

qemuDomainSetSchedulerParametersFlags() calls virQEMUDriverGetConfig() twice and makes the reference counter leak. This removes redundant call. --- src/qemu/qemu_driver.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 76f789e..ca971f3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9050,7 +9050,6 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, if (virDomainSetSchedulerParametersFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; - cfg = virQEMUDriverGetConfig(driver); if (!cfg->privileged) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("CPU tuning is not available in session mode"));

4 3

[libvirt] [PATCH 0/3] Resolve recent/new Coverity warnings
by John Ferlan 26 Mar '14

26 Mar '14

For virlog.c and commandtest.c - it seems recent changes within the modules have allowed Coverity to dig deeper and find different issues. Not sure why Coverity has that "ability"... Since I was making changes, figured I'd clean up the lxc_controller fd leak as well - these should clean the Jenkins build environment. John Ferlan (3): Coverity: Resolve a CHECKED_RETURN message Coverity: Resolve a FORWARD_NULL Coverity: Resolve a RESOURCE_LEAK src/lxc/lxc_controller.c | 14 ++++++++++---- src/util/virlog.c | 2 +- tests/commandtest.c | 1 + 3 files changed, 12 insertions(+), 5 deletions(-) -- 1.8.5.3

3 8

[libvirt] [libvirt-java] [PATCH] Set Java source level in pom
by Chris Ellis 26 Mar '14

26 Mar '14

Hi Simple patch to set the Java source level within the pom, this makes it easier to import the project into Eclipse. I've also added an ignore for a local pom file. I've added plugins to publish the sources and javadocs into Maven. This allows Maven (and IDE integration) to automatically use the javadocs and source. This should make life easier for users of libvirt-java. Regards, Chris Chris Ellis (1): Update pom to include Java source level .gitignore | 1 + pom.xml.in | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) -- 1.8.4.5

3 6

[libvirt] [PATCHv2 0/2] Fix zero cpu shares handling
by Ján Tomko 26 Mar '14

26 Mar '14

Treat 0 as a valid value for cputune shares. v1: https://www.redhat.com/archives/libvir-list/2014-March/msg00215.html v2: squashed the fixup that was sent as a separate patch to the first commit and fixed two more occurences (one of them pointed out by Martin) Ján Tomko (2): Treat zero cpu shares as a valid value Show the real cpu shares value in live xml src/conf/domain_conf.c | 12 ++++---- src/conf/domain_conf.h | 1 + src/lxc/lxc_cgroup.c | 13 ++++++-- src/lxc/lxc_driver.c | 8 ++++- src/lxc/lxc_native.c | 8 +++-- src/parallels/parallels_driver.c | 1 + src/qemu/qemu_cgroup.c | 14 ++++++--- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 13 ++++++-- src/vmx/vmx.c | 3 +- .../qemuxml2argv-cputune-zero-shares.args | 5 ++++ .../qemuxml2argv-cputune-zero-shares.xml | 35 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 14 files changed, 97 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-zero-shares.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-zero-shares.xml -- 1.8.3.2

2 5

[libvirt] [libivrt][RFC]Send "netdev_del" command after receive the DEVICE_DELETED event
by x00221466 26 Mar '14

26 Mar '14

Hi all: There maybe a bug about detaching virtual net device（such as virtio nic、 RTL8138、E1000）。 When live detaching the net device, there are some problems: (1)If the Guest OS don't support the hot plugging pci device, detach the virtual network device by Libvirt, the "net device" in Qemu will still exist, but "hostnet"(tap) in Qemu will be removed. so the net device in Guest OS will be of no effect. (2)If reject the nic in Guest OS, Qemu will remove the "net device", then Qemu send DEVICE_DELETED to Libvirt, Libvirt receive the event in event-loop thread and release info of the net device in qemuDomainRemoveNetDevice func. but "hostnet" in Qemu still exist. So next live attaching virtual net device will be failed because of "Duplicate ID". #virsh attach-device win2008_st_r2_64 net.xml --live error: Failed to attach device from net.xml error: internal error: unable to execute QEMU command 'netdev_add': Duplicate ID 'hostnet0' for netdev (3)In addition, in qemuDomainDetachNetDevice, detach net device func, "netdev_del" command will be sent after sending "device_del" command at once. So it is violent to remove the tap device before the net device is completely removed. So I think it's more logical that doing the work of sending Qemu command "netdev_del" after receive the DEVICE_DELETED event. It can avoid the conflict of device info between Libvirt side and Qemu side. I create a thread in qemuDomainRemoveDevice,the handle of DEVICE_DELETED event, to execute QEMU command "netdev_del". But I find there is another problem when sending monitor msg in multiply threads. in "qemuMonitorSend", monitor lock maybe unlocked and locked, so another thread has the opportunity to get the monitor lock, and use the same monitor. It will occur error. I wonder if it is workable that sending "netdev_del" command after receive the DEVICE_DELETED event. Thanks. -xie

1 0

[libvirt] [PATCH python 0/4] setup.py: Don't unconditionally run pkg-config
by Cole Robinson 25 Mar '14

25 Mar '14

This series reorganizes setup.py to not unconditionally call pkg-config. This will allow 'setup.py clean' and 'setup.py --help' to work correctly even if pkg-config or libvirt-devel aren't installed. https://bugzilla.redhat.com/show_bug.cgi?id=1074170 Cole Robinson (4): setup.py: Remove unused import setup.py: Move module list building to its own function setup.py: Make have_libvirt_lxc a function setup.py: Allow running --help or clean without pkg-config setup.py | 144 +++++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 85 insertions(+), 59 deletions(-) -- 1.8.5.3

2 6

[libvirt] [PATCH] bhyve: don't fail on busy tap devices
by Roman Bogorodskiy 25 Mar '14

25 Mar '14

We use virBhyveTapGetRealDeviceName() to map network interface name to a real device path, trying to open possible devices and getting names by ioctl. Make it skip devices that fail to open with EBUSY because they're most likely already used by other VMs. --- src/bhyve/bhyve_command.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 6d64c27..e569aab 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -69,6 +69,8 @@ virBhyveTapGetRealDeviceName(char *name) goto cleanup; } if ((fd = open(devpath, O_RDWR)) < 0) { + if (errno == EBUSY) + continue; virReportSystemError(errno, _("Unable to open '%s'"), devpath); goto cleanup; } -- 1.8.4.2

2 2

[libvirt] [PATCH] libxl: Create log directory earlier
by Stefan Bader 25 Mar '14

25 Mar '14

Commit d9f19c30d054c86b15a304f4118baa4fa75af9d2 moved a lot of the configuration setup into libxlDriverConfigNew(). However that tries to create the libxl/libxl-driver.log before the libxl directory gets created in libxlStateInitialize(). This causes the daemon to fail on systems that have not had the directory created before. Move the code to create the libxl directory into libxlDriverConfigNew(). Signed-off-by: Stefan Bader <stefan.bader(a)canonical.com> --- src/libxl/libxl_conf.c | 8 ++++++++ src/libxl/libxl_driver.c | 7 ------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index a6bf1cf..56bb84a 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1108,6 +1108,14 @@ libxlDriverConfigNew(void) if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) < 0) goto error; + if (virFileMakePath(cfg->logDir) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to create log dir '%s': %s"), + cfg->logDir, + virStrerror(errno, ebuf, sizeof(ebuf))); + goto error; + } + if ((cfg->logger_file = fopen(log_file, "a")) == NULL) { VIR_ERROR(_("Failed to create log file '%s': %s"), log_file, virStrerror(errno, ebuf, sizeof(ebuf))); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 0c6672b..e6316a7 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -306,13 +306,6 @@ libxlStateInitialize(bool privileged, goto error; libxl_driver->config = cfg; - if (virFileMakePath(cfg->logDir) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to create log dir '%s': %s"), - cfg->logDir, - virStrerror(errno, ebuf, sizeof(ebuf))); - goto error; - } if (virFileMakePath(cfg->stateDir) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to create state dir '%s': %s"), -- 1.7.9.5

2 1

[libvirt] [PATCHv2] network: fix problems with SRV records
by Laine Stump 25 Mar '14

25 Mar '14

A patch submitted by Steven Malin last week pointed out a problem with libvirt's DNS SRV record configuration: https://www.redhat.com/archives/libvir-list/2014-March/msg00536.html When searching for that message later, I found another series that had been posted by Guannan Ren back in 2012 that somehow slipped between the cracks: https://www.redhat.com/archives/libvir-list/2012-July/msg00236.html That patch was very much out of date, but also pointed out some real problems. This patch fixes all the noted problems by refactoring virNetworkDNSSrvDefParseXML() and networkDnsmasqConfContents(), then verifies those fixes by added several new records to the test case. Problems fixed: * both service and protocol now have an underscore ("_") prepended on the commandline, as required by RFC2782. <srv service='sip' protocol='udp' domain='example.com' target='tests.example.com' port='5060' priority='10' weight='150'/> before: srv-host=sip.udp.example.com,tests.example.com,5060,10,150 after: srv-host=_sip._udp.example.com,tests.example.com,5060,10,150 * if "domain" wasn't specified in the <srv> element, the extra trailing "." will no longer be added to the dnsmasq commandline. <srv service='sip' protocol='udp' target='tests.example.com' port='5060' priority='10' weight='150'/> before: srv-host=sip.udp.,tests.example.com,5060,10,150 after: srv-host=_sip._udp,tests.example.com,5060,10,150 * when optional attributes aren't specified, the separating comma is also now not placed on the dnsmasq commandline. If optional attributes in the middle of the line are not specified, they are replaced with a default value in the commandline (1 for port, 0 for priority and weight). <srv service='sip' protocol='udp' target='tests.example.com' port='5060'/> before: srv-host=sip.udp.,tests.example.com,5060,, after: srv-host=_sip._udp,tests.example.com,5060 (actually the would have generated an error, because "optional" attributes weren't really optional.) * The allowed characters for both service and protocol are now limited to alphanumerics, plus a few special characters that are found in existing names in /etc/services and /etc/protocols. (One exception is that both of these files contain names with an embedded ".", but "." can't be used in these fields of an SRV record because it is used as a field separator and there is no method to escape a "." into a field.) (Previously only the strings "tcp" and "udp" were allowed for protocol, but this restriction has been removed, since RFC2782 specifically says that it isn't limited to those, and that anyway it is case insensitive.) * the "domain" attribute is no longer required in order to recognize the port, priority, and weight attributes during parsing. Only "target" is required for this. * if "target" isn't specified, port, priority, and weight are not allowed (since they are meaningless - an empty target means "this service is *not available* for this domain"). * port, priority, and weight are now truly optional, as the comments originally suggested, but which was not actually true. --- Changes from V1: https://www.redhat.com/archives/libvir-list/2014-March/msg01172.html src/conf/network_conf.c | 129 ++++++++++++++------- src/network/bridge_driver.c | 80 ++++++++----- .../nat-network-dns-srv-record-minimal.conf | 2 +- .../nat-network-dns-srv-record.conf | 8 +- .../nat-network-dns-srv-record.xml | 8 +- 5 files changed, 149 insertions(+), 78 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 9be06d3..f1e6243 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -924,6 +924,21 @@ error: return -1; } +/* This includes all characters used in the names of current + * /etc/services and /etc/protocols files (on Fedora 20), except ".", + * which we can't allow because it would conflict with the use of "." + * as a field separator in the SRV record, there appears to be no way + * to escape it in, and the protocols/services that use "." in the + * name are obscure and unlikely to be used anyway. + */ +#define PROTOCOL_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" \ + "-+/" + +#define SERVICE_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" \ + "_-+/*" + static int virNetworkDNSSrvDefParseXML(const char *networkName, xmlNodePtr node, @@ -931,80 +946,108 @@ virNetworkDNSSrvDefParseXML(const char *networkName, virNetworkDNSSrvDefPtr def, bool partialOkay) { + int ret; + xmlNodePtr save_ctxt = ctxt->node; + + ctxt->node = node; + if (!(def->service = virXMLPropString(node, "service")) && !partialOkay) { virReportError(VIR_ERR_XML_DETAIL, _("Missing required service attribute in DNS SRV record " "of network %s"), networkName); goto error; } - if (def->service && strlen(def->service) > DNS_RECORD_LENGTH_SRV) { - virReportError(VIR_ERR_XML_DETAIL, - _("Service name '%s' in network %s is too long, limit is %d bytes"), - def->service, networkName, DNS_RECORD_LENGTH_SRV); - goto error; + if (def->service) { + if (strlen(def->service) > DNS_RECORD_LENGTH_SRV) { + virReportError(VIR_ERR_XML_DETAIL, + _("service attribute '%s' in network %s is too long, " + "limit is %d bytes"), + def->service, networkName, DNS_RECORD_LENGTH_SRV); + goto error; + } + if (strspn(def->service, SERVICE_CHARS) < strlen(def->service)) { + virReportError(VIR_ERR_XML_DETAIL, + _("invalid character in service attribute '%s' " + "in network %s DNS SRV record"), + def->service, networkName); + goto error; + } } if (!(def->protocol = virXMLPropString(node, "protocol")) && !partialOkay) { virReportError(VIR_ERR_XML_DETAIL, _("Missing required protocol attribute " - "in dns srv record '%s' of network %s"), + "in DNS SRV record '%s' of network %s"), def->service, networkName); goto error; } - - /* Check whether protocol value is the supported one */ - if (def->protocol && STRNEQ(def->protocol, "tcp") && - (STRNEQ(def->protocol, "udp"))) { + if (def->protocol && + strspn(def->protocol, PROTOCOL_CHARS) < strlen(def->protocol)) { virReportError(VIR_ERR_XML_DETAIL, - _("Invalid protocol attribute value '%s' " - "in DNS SRV record of network %s"), + _("invalid character in protocol attribute '%s' " + "in network %s DNS SRV record"), def->protocol, networkName); goto error; } /* Following attributes are optional */ - if ((def->target = virXMLPropString(node, "target")) && - (def->domain = virXMLPropString(node, "domain"))) { - xmlNodePtr save_ctxt = ctxt->node; - - ctxt->node = node; - if (virXPathUInt("string(./@port)", ctxt, &def->port) < 0 || - def->port > 65535) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing or invalid port attribute " - "in network %s"), networkName); - goto error; - } - - if (virXPathUInt("string(./@priority)", ctxt, &def->priority) < 0 || - def->priority > 65535) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing or invalid priority attribute " - "in network %s"), networkName); - goto error; - } + def->domain = virXMLPropString(node, "domain"); + def->target = virXMLPropString(node, "target"); - if (virXPathUInt("string(./@weight)", ctxt, &def->weight) < 0 || - def->weight > 65535) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing or invalid weight attribute " - "in network %s"), networkName); - goto error; - } + ret = virXPathUInt("string(./@port)", ctxt, &def->port); + if (ret >= 0 && !def->target) { + virReportError(VIR_ERR_XML_DETAIL, + _("DNS SRV port attribute not permitted without " + "target for service %s in network %s"), + def->service, networkName); + goto error; + } + if (ret == -2 || (ret >= 0 && (def->port < 1 || def->port > 65535))) { + virReportError(VIR_ERR_XML_DETAIL, + _("Invalid DNS SRV port attribute " + "for service %s in network %s"), + def->service, networkName); + goto error; + } - ctxt->node = save_ctxt; + ret = virXPathUInt("string(./@priority)", ctxt, &def->priority); + if (ret >= 0 && !def->target) { + virReportError(VIR_ERR_XML_DETAIL, + _("DNS SRV priority attribute not permitted without " + "target for service %s in network %s"), + def->service, networkName); + goto error; + } + if (ret == -2 || (ret >= 0 && def->priority > 65535)) { + virReportError(VIR_ERR_XML_DETAIL, + _("Invalid DNS SRV priority attribute " + "for service %s in network %s"), + def->service, networkName); + goto error; } - if (!(def->service || def->protocol)) { + ret = virXPathUInt("string(./@weight)", ctxt, &def->weight); + if (ret >= 0 && !def->target) { virReportError(VIR_ERR_XML_DETAIL, - _("Missing required service attribute or protocol " - "in DNS SRV record of network %s"), networkName); + _("DNS SRV weight attribute not permitted without " + "target for service %s in network %s"), + def->service, networkName); + goto error; + } + if (ret == -2 || (ret >= 0 && def->weight > 65535)) { + virReportError(VIR_ERR_XML_DETAIL, + _("Invalid DNS SRV weight attribute " + "for service %s in network %s"), + def->service, networkName); goto error; } + + ctxt->node = save_ctxt; return 0; error: virNetworkDNSSrvDefClear(def); + ctxt->node = save_ctxt; return -1; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 59b6c09..e256dba 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -736,10 +736,6 @@ networkDnsmasqConfContents(virNetworkObjPtr network, int r, ret = -1; int nbleases = 0; size_t i; - char *record = NULL; - char *recordPort = NULL; - char *recordWeight = NULL; - char *recordPriority = NULL; virNetworkDNSDefPtr dns = &network->def->dns; virNetworkIpDefPtr tmpipdef, ipdef, ipv4def, ipv6def; bool ipv6SLAAC; @@ -880,33 +876,57 @@ networkDnsmasqConfContents(virNetworkObjPtr network, } for (i = 0; i < dns->nsrvs; i++) { - if (dns->srvs[i].service && dns->srvs[i].protocol) { - if (dns->srvs[i].port && - virAsprintf(&recordPort, "%d", dns->srvs[i].port) < 0) - goto cleanup; - if (dns->srvs[i].priority && - virAsprintf(&recordPriority, "%d", dns->srvs[i].priority) < 0) - goto cleanup; - if (dns->srvs[i].weight && - virAsprintf(&recordWeight, "%d", dns->srvs[i].weight) < 0) - goto cleanup; + /* service/protocol are required, and should have been validated + * by the parser. + */ + if (!dns->srvs[i].service) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing required 'service' " + "attribute in SRV record of network '%s'"), + network->def->name); + goto cleanup; + } + if (!dns->srvs[i].protocol) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing required 'service' " + "attribute in SRV record of network '%s'"), + network->def->name); + goto cleanup; + } + /* RFC2782 requires that service and protocol be preceded by + * an underscore. + */ + virBufferAsprintf(&configbuf, "srv-host=_%s._%s", + dns->srvs[i].service, dns->srvs[i].protocol); - if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s", - dns->srvs[i].service, - dns->srvs[i].protocol, - dns->srvs[i].domain ? dns->srvs[i].domain : "", - dns->srvs[i].target ? dns->srvs[i].target : "", - recordPort ? recordPort : "", - recordPriority ? recordPriority : "", - recordWeight ? recordWeight : "") < 0) - goto cleanup; + /* domain is optional - it defaults to the domain of this network */ + if (dns->srvs[i].domain) + virBufferAsprintf(&configbuf, ".%s", dns->srvs[i].domain); - virBufferAsprintf(&configbuf, "srv-host=%s\n", record); - VIR_FREE(record); - VIR_FREE(recordPort); - VIR_FREE(recordWeight); - VIR_FREE(recordPriority); + /* If target is empty or ".", that means "the service is + * decidedly not available at this domain" (RFC2782). In that + * case, any port, priority, or weight is irrelevant. + */ + if (dns->srvs[i].target && STRNEQ(dns->srvs[i].target, ".")) { + + virBufferAsprintf(&configbuf, ",%s", dns->srvs[i].target); + /* port, priority, and weight are optional, but are + * identified by their position in the line. If an item is + * unspecified, but something later in the line *is* + * specified, we need to give the default value for the + * unspecified item. (According to the dnsmasq manpage, + * the default for port is 1). + */ + if (dns->srvs[i].port || + dns->srvs[i].priority || dns->srvs[i].weight) + virBufferAsprintf(&configbuf, ",%d", + dns->srvs[i].port ? dns->srvs[i].port : 1); + if (dns->srvs[i].priority || dns->srvs[i].weight) + virBufferAsprintf(&configbuf, ",%d", dns->srvs[i].priority); + if (dns->srvs[i].weight) + virBufferAsprintf(&configbuf, ",%d", dns->srvs[i].weight); } + virBufferAddLit(&configbuf, "\n"); } /* Find the first dhcp for both IPv4 and IPv6 */ @@ -1082,10 +1102,6 @@ networkDnsmasqConfContents(virNetworkObjPtr network, cleanup: virBufferFreeAndReset(&configbuf); - VIR_FREE(record); - VIR_FREE(recordPort); - VIR_FREE(recordWeight); - VIR_FREE(recordPriority); return ret; } diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf index ce4dd6f..e60411b 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf @@ -12,7 +12,7 @@ listen-address=192.168.123.1 listen-address=fc00:db8:ac10:fe01::1 listen-address=fc00:db8:ac10:fd01::1 listen-address=10.24.10.1 -srv-host=name.tcp.,,,, +srv-host=_name._tcp dhcp-range=192.168.122.2,192.168.122.254 dhcp-no-override dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.conf b/tests/networkxml2confdata/nat-network-dns-srv-record.conf index b47cbe7..16e7dca 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.conf @@ -8,7 +8,13 @@ strict-order except-interface=lo bind-dynamic interface=virbr0 -srv-host=name.tcp.test-domain-name,.,1024,10,10 +srv-host=_name._tcp.test-domain-name.com,test.example.com,1111,11,111 +srv-host=_name2._udp,test2.example.com,2222,22,222 +srv-host=_name3._tcp.test3.com,test3.example.com,3333,33 +srv-host=_name4._tcp.test4.com,test4.example.com,4444 +srv-host=_name5._udp,test5.example.com,1,55,555 +srv-host=_name6._tcp.test6.com,test6.example.com,6666,0,666 +srv-host=_name7._tcp.test7.com,test7.example.com,1,0,777 dhcp-range=192.168.122.2,192.168.122.254 dhcp-no-override dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.xml b/tests/networkxml2confdata/nat-network-dns-srv-record.xml index 3dd19e6..d01b331 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record.xml +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.xml @@ -6,7 +6,13 @@ </forward> <bridge name='virbr0' stp='on' delay='0'/> <dns> - <srv service='name' protocol='tcp' domain='test-domain-name' target='.' port='1024' priority='10' weight='10'/> + <srv service='name' protocol='tcp' domain='test-domain-name.com' target='test.example.com' port='1111' priority='11' weight='111'/> + <srv service='name2' protocol='udp' target='test2.example.com' port='2222' priority='22' weight='222'/> + <srv service='name3' protocol='tcp' domain='test3.com' target='test3.example.com' port='3333' priority='33'/> + <srv service='name4' protocol='tcp' domain='test4.com' target='test4.example.com' port='4444'/> + <srv service='name5' protocol='udp' target='test5.example.com' priority='55' weight='555'/> + <srv service='name6' protocol='tcp' domain='test6.com' target='test6.example.com' port='6666' weight='666'/> + <srv service='name7' protocol='tcp' domain='test7.com' target='test7.example.com' weight='777'/> </dns> <ip address='192.168.122.1' netmask='255.255.255.0'> <dhcp> -- 1.8.5.3

2 1

[libvirt] [PATCH 00/24] Indent top-level labels by one space
by Ján Tomko 25 Mar '14

25 Mar '14

3 28

[libvirt] [python PATCHv2 0/2] arbitrary qemu events
by Eric Blake 25 Mar '14

25 Mar '14

Changes since v1: rebase to latest code base, address review comments regarding a leak of dom, add prereq patch to fix bugs that I had been copying and pasting Eric Blake (2): event: fix domain reference bugs qemu: support arbitrary monitor events generator.py | 20 +-- libvirt-override.c | 319 +++++++++++++++++++++++++++++------------------ libvirt-qemu-override.c | 223 ++++++++++++++++++++++++++++++++- libvirt-qemu-override.py | 35 ++++++ 4 files changed, 469 insertions(+), 128 deletions(-) create mode 100644 libvirt-qemu-override.py -- 1.8.5.3

2 4

[libvirt] [libvirt-python PATCH] generator: Add virConnectDomainQemuMonitorEventCallback to skipped_types
by Martin Kletzander 25 Mar '14

25 Mar '14

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Notes: pushed as a build-breaker generator.py | 1 + 1 file changed, 1 insertion(+) diff --git a/generator.py b/generator.py index 0e9600f..e409921 100755 --- a/generator.py +++ b/generator.py @@ -273,6 +273,7 @@ skipped_types = { 'virConnectDomainEventWatchdogCallback': "No function types in python", 'virConnectDomainEventIOErrorCallback': "No function types in python", 'virConnectDomainEventGraphicsCallback': "No function types in python", + 'virConnectDomainQemuMonitorEventCallback': "No function types in python", 'virStreamEventCallback': "No function types in python", 'virEventHandleCallback': "No function types in python", 'virEventTimeoutCallback': "No function types in python", -- 1.9.1

1 0

[libvirt] [PATCH 0/2] sanlock: Forbid unsupported lock failure actions
by Jiri Denemark 25 Mar '14

25 Mar '14

Some lock failure actions do not make any sense in combination with sanlock driver. Let's just report an error if someone tries to use them instead of causing unexpected and possibly quite bad thing to happen. Jiri Denemark (2): sanlock: Forbid VIR_DOMAIN_LOCK_FAILURE_IGNORE sanlock: Forbid VIR_DOMAIN_LOCK_FAILURE_RESTART src/locking/lock_driver_sanlock.c | 12 +++++++++++- src/locking/sanlock_helper.c | 28 +++------------------------- 2 files changed, 14 insertions(+), 26 deletions(-) -- 1.9.1

2 4

[libvirt] [PATCH 0/4] bhyve integration: pie, blk, MAC, host API
by Wojciech Macek 25 Mar '14

25 Mar '14

Hi, I'm working on enabling OpenStack/libvirt support for FreeBSD hosts. Please look into some patches I'd like to submit for rewiev. 1. PIE-flag: on some FreeBSD-10 the clang toolchain is broken and cannot build executables with PIE option, so I added the flag to let the user choose. 2. bhyve MAC: add MAC address configuration for bhyve virtio network adapter 3. bhyve blk: add support for up to 8 blk devices, atapi/sata/virtio bus with file/block backend 4. bhyve host API: initial support for "host" functions on FreeBSD Regards, Wojtek Wojciech Macek (4): pie: add Position-Independent-Executable flag bhyve: MAC address configuration bhyve: multiple virtio-blk devices support bhyve: host API support configure.ac | 14 +++++++- src/bhyve/bhyve_command.c | 92 ++++++++++++++++++++++++++++++----------------- src/bhyve/bhyve_command.h | 8 +++++ src/bhyve/bhyve_driver.c | 66 ++++++++++++++++++++++++++++++++++ src/nodeinfo.c | 19 ++++++++++ 5 files changed, 165 insertions(+), 34 deletions(-) -- 1.9.0

4 13