April 2013 - Devel - libvirt List Archives

[libvirt] virNodeDevice APIs and VFIO
by Laine Stump 24 Apr '13

24 Apr '13

Yesterday for the first time I consciously noticed the virNodeDeviceDettach and virNodeDeviceReAttach APIs, and found that they are hardcoded to bind to/unbind from the pci-stub driver for qemu, and the "pciback" driver for Xen. If we want these APIs to be useful for VFIO, they will need to bind to the vfio driver instead, but there is currently no method in those APIs to specify which driver to bind to. I guess we could do this with new virNodeDeviceDetachFlags() and virNodeDeviceReAttachFlags() APIs which have a flag to indicate vfio, but before going to that trouble I'd like to know if these APIs are actually used or if they are deprecated (they don't seem to be of any use if the hostdev devices you're assigning have "managed='yes'" - as far as I can see, setting managed='yes' just makes the bind/unbind from the stub driver an automatic part of assigning/un-assigning the device to a guest). (Beyond that, this would be exposing a lower level of detail in the APIs than the original functions, and I'm not sure if it's appropriate - the choice of stub driver was previously hidden in each hypervisor driver).

3 6

[libvirt] [PATCH] util: Error out if the numa nodeset is out of range
by Osier Yang 24 Apr '13

24 Apr '13

Instead of a silent warning, it's better to error out if the numa nodeset is out of range. Just like for numa node larger than NUMA_NUM_NODES. --- src/util/virnuma.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index bace06f..902ed43 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -89,7 +89,6 @@ virNumaSetupMemoryPolicy(virNumaTuneDef numatune, int ret = -1; int i = 0; int maxnode = 0; - bool warned = false; virBitmapPtr tmp_nodemask = NULL; if (numatune.memory.placement_mode == @@ -113,20 +112,17 @@ virNumaSetupMemoryPolicy(virNumaTuneDef numatune, } maxnode = numa_max_node() + 1; + /* Convert nodemask to NUMA bitmask. */ nodemask_zero(&mask); i = -1; while ((i = virBitmapNextSetBit(tmp_nodemask, i)) >= 0) { - if (i > NUMA_NUM_NODES) { + if (i > maxnode || i > NUMA_NUM_NODES) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Host cannot support NUMA node %d"), i); + _("Nodeset is out of range, host cannot support " + "NUMA node bigger than %d"), i); return -1; } - if (i > maxnode && !warned) { - VIR_WARN("nodeset is out of range, there is only %d NUMA " - "nodes on host", maxnode); - warned = true; - } nodemask_set(&mask, i); } -- 1.8.1.4

2 3

[libvirt] [PATCHv2] selinux: Don't mask errors of virSecuritySELinuxGenNewContext
by Peter Krempa 24 Apr '13

24 Apr '13

Since cbe67ff9b0a5a94911afd4d12388bf182bcca86c virSecuritySELinuxGenNewContext reports good error messages. Fix callers that mask the errors by generic error message. --- src/security/security_selinux.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index c620a2e..61ff1de 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -685,13 +685,10 @@ virSecuritySELinuxGenSecurityLabel(virSecurityManagerPtr mgr, } } - seclabel->label = - virSecuritySELinuxGenNewContext(baselabel, mcs, false); - if (!seclabel->label) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot generate selinux context for %s"), mcs); + seclabel->label = virSecuritySELinuxGenNewContext(baselabel, mcs, false); + if (!seclabel->label) goto cleanup; - } + break; case VIR_DOMAIN_SECLABEL_NONE: @@ -709,11 +706,8 @@ virSecuritySELinuxGenSecurityLabel(virSecurityManagerPtr mgr, seclabel->imagelabel = virSecuritySELinuxGenNewContext(data->file_context, mcs, true); - if (!seclabel->imagelabel) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot generate selinux context for %s"), mcs); + if (!seclabel->imagelabel) goto cleanup; - } } if (!seclabel->model && -- 1.8.2.1

2 2

[libvirt] Fuse file system mounted at /proc/meminfo needs correct label.
by dwalsh＠redhat.com 24 Apr '13

24 Apr '13

Second Pass... [PATCH 1/1] Fuse file system mounted at /proc/meminfo needs correct

1 1

[libvirt] [PATCH] selinux: Don't mask errors of virSecuritySELinuxGenNewContext
by Peter Krempa 24 Apr '13

24 Apr '13

virSecuritySELinuxGenNewContext() reports sensible errors but at virSecuritySELinuxGenSecurityLabel() the error is masked with a non-specific one. --- src/security/security_selinux.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index c620a2e..89d63ab 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -685,13 +685,10 @@ virSecuritySELinuxGenSecurityLabel(virSecurityManagerPtr mgr, } } - seclabel->label = - virSecuritySELinuxGenNewContext(baselabel, mcs, false); - if (!seclabel->label) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot generate selinux context for %s"), mcs); + seclabel->label = virSecuritySELinuxGenNewContext(baselabel, mcs, false); + if (!seclabel->label) goto cleanup; - } + break; case VIR_DOMAIN_SECLABEL_NONE: -- 1.8.2.1

2 1

[libvirt] [PATCH] qemuBuildCommandLine: Don't overwrite errors with NWFilter's one
by Michal Privoznik 24 Apr '13

24 Apr '13

Currently, if there has been an error in building command line process after virtual interfaces has been created, the flow jumps to 'error' label, where virDomainConfNWFilterTeardown() is called. This may report an error as well, but should not overwrite the original cause why we jumped to 'error' label. --- src/qemu/qemu_command.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 741fa82..92b15d9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5872,6 +5872,7 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainSnapshotObjPtr snapshot, enum virNetDevVPortProfileOp vmop) { + virErrorPtr originalError = NULL; int i, j; const char *emulator; char uuid[VIR_UUID_STRING_BUFLEN]; @@ -7802,13 +7803,16 @@ qemuBuildCommandLine(virConnectPtr conn, virObjectUnref(cfg); return cmd; - no_memory: +no_memory: virReportOOMError(); - error: +error: virObjectUnref(cfg); - /* free up any resources in the network driver */ + /* free up any resources in the network driver + * but don't overwrite the original error */ + originalError = virSaveLastError(); for (i = 0; i <= last_good_net; i++) virDomainConfNWFilterTeardown(def->nets[i]); + virSetError(originalError); virCommandFree(cmd); return NULL; } -- 1.8.1.5

2 1

[libvirt] [PATCH 0/6] Cleanup of internal API naming conventions
by Daniel P. Berrange 24 Apr '13

24 Apr '13

The naming conventions used for APIs in both src/driver.h and src/remote/remote_protocol.x should follow the public API naming, but there are a great many violations of this. This series fixes them all, and adds some tests which will help prevent regressions.

2 18

[libvirt] [PATCH] qemu: Generate agent socket path if missing
by Michal Privoznik 24 Apr '13

24 Apr '13

It's not desired to force users imagine path for a socket they are not even supposed to connect to. On the other hand, we already have a release where the qemu agent socket path is exposed to XML, so we cannot silently drop it from there. The new path is generated in form: $LOCALSTATEDIR/lib/libvirt/qemu/channel/target/$domain.$name --- libvirt.spec.in | 1 + src/Makefile.am | 1 + src/conf/domain_conf.c | 34 ++++++++++++++++------------------ src/qemu/qemu_domain.c | 16 ++++++++++++++++ 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 328b009..11fc12d 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1719,6 +1719,7 @@ fi %if %{with_qemu} %ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ +%dir %attr{0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/target/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ %endif %if %{with_lxc} diff --git a/src/Makefile.am b/src/Makefile.am index fc6b846..9576da8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2097,6 +2097,7 @@ if WITH_SANLOCK endif if WITH_QEMU $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu" + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/channel/target" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/libvirt/qemu" diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 03e5740..3ff548a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6464,7 +6464,9 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_UNIX: - if (path == NULL) { + /* path can be auto generated */ + if (!path && + chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source path attribute for char device")); goto error; @@ -6559,7 +6561,6 @@ virDomainChrDefParseXML(xmlXPathContextPtr ctxt, char *type = NULL; const char *nodeName; virDomainChrDefPtr def; - int remaining; bool seenTarget = false; if (!(def = virDomainChrDefNew())) @@ -6583,29 +6584,26 @@ virDomainChrDefParseXML(xmlXPathContextPtr ctxt, } cur = node->children; - remaining = virDomainChrSourceDefParseXML(&def->source, cur, flags, - def, ctxt, - vmSeclabels, nvmSeclabels); - if (remaining < 0) - goto error; - if (remaining) { - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - if (xmlStrEqual(cur->name, BAD_CAST "target")) { - seenTarget = true; - if (virDomainChrDefParseTargetXML(def, cur) < 0) { - goto error; - } + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "target")) { + seenTarget = true; + if (virDomainChrDefParseTargetXML(def, cur) < 0) { + goto error; } } - cur = cur->next; } + cur = cur->next; } if (!seenTarget && ((def->targetType = virDomainChrDefaultTargetType(def->deviceType)) < 0)) goto cleanup; + if (virDomainChrSourceDefParseXML(&def->source, node->children, flags, def, + ctxt, vmSeclabels, nvmSeclabels) < 0) + goto error; + if (def->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { if (def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -13897,8 +13895,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf, case VIR_DOMAIN_CHR_TYPE_UNIX: virBufferAsprintf(buf, " <source mode='%s'", def->data.nix.listen ? "bind" : "connect"); - virBufferEscapeString(buf, " path='%s'/>\n", - def->data.nix.path); + virBufferEscapeString(buf, " path='%s'", def->data.nix.path); + virBufferAddLit(buf, "/>\n"); break; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3d6eef4..967890f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -738,6 +738,22 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, (def->os.arch == VIR_ARCH_S390 || def->os.arch == VIR_ARCH_S390X)) dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; + /* auto generate unix socket path */ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + dev->data.chr->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && + !dev->data.chr->source.data.nix.path && + (cfg || (driver && (cfg = virQEMUDriverGetConfig(driver))))) { + + if (virAsprintf(&dev->data.chr->source.data.nix.path, + "%s/channel/target/%s.%s", + cfg->libDir, def->name, + dev->data.chr->target.name) < 0) + goto no_memory; + dev->data.chr->source.data.nix.listen = true; + } + ret = 0; cleanup: -- 1.8.1.5

3 3

[libvirt] Using leases with virtlockd
by Jim Fehlig 24 Apr '13

24 Apr '13

Nearly a question for the users list, but I might be encountering a bug too. I'm using virtlockd with 'auto_disk_leases = 0' and struggle with the correct configuration for a lease in the domXML. Using <lease> <lockspace>test-ls</lockspace> <key>test-lock</key> <target path='/var/lib/libvirt/lockd'/> </lease> results in the following error when starting the domain internal error Lockspace for path /var/lib/libvirt/lockd/test-ls It appears the path attribute + lockspace are used as the Lockspace for this lease. But I'm not sure what "Lockspace" means in this context. Is it a directory? A file? I assume the latter since it is just flock()ed when the domain is started, right? I'll be happy to provide patches to fix any bugs that might exist here (docs included), but need understand the correct configuration of leases with virtlockd first :). BTW, nice piece of work Daniel! Regards, Jim

2 1

[libvirt] [PATCH] Use camelCase for XML attribute numQueues
by Martin Kletzander 24 Apr '13

24 Apr '13

In commit d4bf0a9, we used num_queues for an attribute in the XML, but the consensus is that we use camelCase for that. Since there was no release yet (the above commit describes as v1.0.4-65-gd4bf0a9), we still have time to change it. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- This patch applies on top of Laine's RNG tightening patch [1] and it was proposed in that thread as well. [1] http://www.redhat.com/archives/libvir-list/2013-April/msg01320.html --- docs/formatdomain.html.in | 2 +- docs/schemas/domaincommon.rng | 2 +- src/conf/domain_conf.c | 6 +++--- src/qemu/qemu_command.c | 2 +- tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0cc56d9..a5be162 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2135,7 +2135,7 @@ controller. A "scsi" controller has an optional attribute <code>model</code>, which is one of "auto", "buslogic", "ibmvscsi", "lsilogic", "lsisas1068", "lsisas1078", "virtio-scsi" or - "vmpvscsi". The attribute <code>num_queues</code> + "vmpvscsi". The attribute <code>numQueues</code> (<span class="since">1.0.5 (QEMU and KVM only)</span>) specifies the number of queues for the controller. For best performance, it's recommended to specify a value matching the number of vCPUs. A "usb" diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..bcb1453 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1444,7 +1444,7 @@ </attribute> </optional> <optional> - <attribute name="num_queues"> + <attribute name="numQueues"> <ref name="unsignedInt"/> </attribute> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1643f30..0487053 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5195,10 +5195,10 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->model = -1; } - if ((num_queues = virXMLPropString(node, "num_queues"))) { + if ((num_queues = virXMLPropString(node, "numQueues"))) { if (virStrToLong_ui(num_queues, NULL, 10, &def->num_queues) < 0) { virReportError(VIR_ERR_XML_ERROR, - _("Malformed 'num_queues' value '%s'"), num_queues); + _("Malformed 'numQueues' value '%s'"), num_queues); goto error; } } @@ -13531,7 +13531,7 @@ virDomainControllerDefFormat(virBufferPtr buf, } if (def->num_queues) - virBufferAsprintf(buf, " num_queues='%u'", def->num_queues); + virBufferAsprintf(buf, " numQueues='%u'", def->num_queues); switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 009d42d..65675b7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3574,7 +3574,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, !(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI && def->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'num_queues' is only supported by virtio-scsi controller")); + _("'numQueues' is only supported by virtio-scsi controller")); return NULL; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml index dfa9cf1..063ccf7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml @@ -20,7 +20,7 @@ <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'/> - <controller type='scsi' index='0' model='virtio-scsi' num_queues='8'/> + <controller type='scsi' index='0' model='virtio-scsi' numQueues='8'/> <memballoon model='virtio'/> </devices> </domain> -- 1.8.1.5

5 12