April 2013 - Devel - libvirt List Archives

[libvirt] [PATCH] Change the tag name "num_queues" into "queues"
by Osier Yang 24 Apr '13

24 Apr '13

Instead of making a choice between the underscore and camelCase, this simply changes "num_queues" into "queues", which is also consistent with Michal's multiple queue support for interface. --- docs/formatdomain.html.in | 2 +- docs/schemas/domaincommon.rng | 2 +- src/conf/domain_conf.c | 14 +++++++------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 8 ++++---- .../qemuxml2argv-disk-virtio-scsi-num_queues.xml | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 888c005..3dbd58b 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>queues</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..b1c4c2f 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1444,7 +1444,7 @@ </attribute> </optional> <optional> - <attribute name="num_queues"> + <attribute name="queues"> <ref name="unsignedInt"/> </attribute> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dc0ecaa..253c9ef 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5159,7 +5159,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, char *type = NULL; char *idx = NULL; char *model = NULL; - char *num_queues = NULL; + char *queues = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -5195,10 +5195,10 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->model = -1; } - if ((num_queues = virXMLPropString(node, "num_queues"))) { - if (virStrToLong_ui(num_queues, NULL, 10, &def->num_queues) < 0) { + if ((queues = virXMLPropString(node, "queues"))) { + if (virStrToLong_ui(queues, NULL, 10, &def->queues) < 0) { virReportError(VIR_ERR_XML_ERROR, - _("Malformed 'num_queues' value '%s'"), num_queues); + _("Malformed 'queues' value '%s'"), queues); goto error; } } @@ -5280,7 +5280,7 @@ cleanup: VIR_FREE(type); VIR_FREE(idx); VIR_FREE(model); - VIR_FREE(num_queues); + VIR_FREE(queues); return def; @@ -13524,8 +13524,8 @@ virDomainControllerDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " model='%s'", model); } - if (def->num_queues) - virBufferAsprintf(buf, " num_queues='%u'", def->num_queues); + if (def->queues) + virBufferAsprintf(buf, " queues='%u'", def->queues); switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5b069b6..acf18f8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -742,7 +742,7 @@ struct _virDomainControllerDef { int type; int idx; int model; /* -1 == undef */ - unsigned int num_queues; + unsigned int queues; union { virDomainVirtioSerialOpts vioserial; } opts; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b1d3d1b..35cfac8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3524,11 +3524,11 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, virBuffer buf = VIR_BUFFER_INITIALIZER; int model; - if (def->num_queues && + if (def->queues && !(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")); + _("'queues' is only supported by virtio-scsi controller")); return NULL; } @@ -3615,8 +3615,8 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, goto error; } - if (def->num_queues) - virBufferAsprintf(&buf, ",num_queues=%u", def->num_queues); + if (def->queues) + virBufferAsprintf(&buf, ",num_queues=%u", def->queues); if (qemuBuildDeviceAddressStr(&buf, &def->info, qemuCaps) < 0) goto error; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-num_queues.xml index dfa9cf1..b3b1289 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' queues='8'/> <memballoon model='virtio'/> </devices> </domain> -- 1.8.1.4

2 3

[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