[libvirt] [PATCH 3/6] conf: add virDomainGetMemsForGuestCpu()
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c | 39 +++
src/conf/domain_conf.h | 4
src/libvirt_private.syms | 1 +
3 files changed, 44 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 352ba92..fef956c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19879,3 +19879,42 @@ virDomainObjSetMetadata(virDomainObjPtr vm,
return 0;
}
+
+int
+virDomainGetMemsForGuestCpu(virDomainDefPtr def,
+size_t cpuid,
+char **nodemask)
+{
+ssize_t i = 0;
+
+*nodemask = NULL;
+
+/* nothing to do with no per-node settings */
+if (!def-numatune.nmem_nodes)
+return 0;
+
+/* !!(def-numatune.nmem_nodes) - !!(def-cpu) */
+for (i = 0; i def-cpu-ncells; i++) {
+bool result = false;
+
+if (virBitmapGetBit(def-cpu-cells[i].cpumask, cpuid, result) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Problem accessing domain's
+ NUMA cell information));
+return -1;
+}
+if (!result)
+continue;
+
+if (def-numatune.mem_nodes[i].mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT)
+return 0;
+
+*nodemask = virBitmapFormat(def-numatune.mem_nodes[i].nodemask);
+if (!*nodemask)
+return -1;
+
+return 0;
+}
+
+return 0;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2de807d..b6e0765 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2711,4 +2711,8 @@ int virDomainObjSetMetadata(virDomainObjPtr vm,
const char *configDir,
unsigned int flags);
+int virDomainGetMemsForGuestCpu(virDomainDefPtr def,
+size_t cpuid,
+char **nodemask);
+
#endif /* __DOMAIN_CONF_H */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d73a9f5..88eae2f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -244,6 +244,7 @@ virDomainFSTypeToString;
virDomainFSWrpolicyTypeFromString;
virDomainFSWrpolicyTypeToString;
virDomainGetFilesystemForTarget;
+virDomainGetMemsForGuestCpu;
virDomainGraphicsAuthConnectedTypeFromString;
virDomainGraphicsAuthConnectedTypeToString;
virDomainGraphicsDefFree;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] conf, schema: add support for numatune memnode element
This element specifies similar settings as the memory element,
although memnode can be used per guest NUMA node.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 15 +++
docs/schemas/domaincommon.rng | 17
src/conf/domain_conf.c| 220 +++---
src/qemu/qemu_domain.c| 23 -
src/qemu/qemu_driver.c| 11 +++
src/util/virnuma.h| 14 ++-
6 files changed, 260 insertions(+), 40 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 041f70d..2d855ea 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -709,6 +709,8 @@
...
lt;numatunegt;
lt;memory mode=strict nodeset=1-4,^3/gt;
+lt;memnode cellid=0 mode=strict nodeset=1/gt;
+lt;memnode cellid=2 mode=preferred nodeset=2/gt;
lt;/numatunegt;
...
lt;/domaingt;
@@ -745,6 +747,19 @@
span class='since'Since 0.9.3/span
/dd
+ dtcodememnode/code/dt
+ dd
+Optional codememnode/code elements can specify memory allocation
+policies per each guest NUMA node. For those nodes having no
+corresponding codememnode/code element, the default from
+element codememory/code will be used. Attribute
codecellid/code
+addresses guest NUMA node for which the settings are applied.
+Attributes codemode/code and codenodeset/code have the same
+meaning and syntax as in codememory/code element.
+
+This setting is not compatible with automatic placement.
+span class='since'QEMU Since 1.2.6/span
+ /dd
/dl
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 0787b5a..a8e3ba0 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -789,6 +789,23 @@
/choice
/element
/optional
+ zeroOrMore
+element name=memnode
+ attribute name=cellid
+ref name=unsignedInt/
+ /attribute
+ attribute name=mode
+choice
+ valuestrict/value
+ valuepreferred/value
+ valueinterleave/value
+/choice
+ /attribute
+ attribute name='nodeset'
+ref name='cpuset'/
+ /attribute
+/element
+ /zeroOrMore
/element
/define
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fe06921..352ba92 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2085,6 +2085,9 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainVcpuPinDefFree(def-cputune.emulatorpin);
virBitmapFree(def-numatune.memory.nodemask);
+for (i = 0; i def-numatune.nmem_nodes; i++)
+virBitmapFree(def-numatune.mem_nodes[i].nodemask);
+VIR_FREE(def-numatune.mem_nodes);
virSysinfoDefFree(def-sysinfo);
@@ -11232,6 +11235,8 @@ virDomainDefParseXML(xmlDocPtr xml,
bool usb_other = false;
bool usb_master = false;
bool primaryVideo = false;
+bool mem_nodes = false;
+
if (VIR_ALLOC(def) 0)
return NULL;
@@ -11666,6 +11671,33 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(nodes);
+
+/* analysis of cpu handling */
+if ((node = virXPathNode(./cpu[1], ctxt)) != NULL) {
+xmlNodePtr oldnode = ctxt-node;
+ctxt-node = node;
+def-cpu = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_GUEST);
+ctxt-node = oldnode;
+
+if (def-cpu == NULL)
+goto error;
+
+if (def-cpu-sockets
+def-maxvcpus
+def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
+virReportError(VIR_ERR_XML_DETAIL, %s,
+ _(Maximum CPUs greater than topology limit));
+goto error;
+}
+
+if (def-cpu-cells_cpus def-maxvcpus) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Number of CPUs in numa exceeds the
+ vcpu count));
+goto error;
+}
+}
+
/* Extract numatune if exists. */
if ((n = virXPathNodeSet(./numatune, ctxt, nodes)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -11682,6 +11714,12 @@ virDomainDefParseXML(xmlDocPtr xml,
if (n) {
cur = nodes[0]-children;
+if (def-cpu) {
+if (VIR_ALLOC_N(def-numatune.mem_nodes, def-cpu-ncells) 0)
+goto error;
+def-numatune.nmem_nodes = def-cpu-ncells;
+}
+
while (cur != NULL) {
if (cur-type == XML_ELEMENT_NODE) {
if (xmlStrEqual(cur-name, BAD_CAST memory)) {
@@ -11764,6 +11802,80 @@ virDomainDefParseXML(xmlDocPtr xml,
def-placement_mode =
VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;
def-numatune.memory.placement_mode = placement_mode;
+
+} else if (xmlStrEqual(cur-name, BAD_CAST memnode
[libvirt] [PATCH 0/6] Support for per-guest-node binding
Currently we are only able to bind the whole domain to some host nodes using the /domain/numatune/memory element. Numerous requests were made to support host-guest numa node bindings, so this series tries to pinch an idea on how to do that using /domain/numatune/memnode elements. That is incompatible with automatic numa placement (numad) since that makes no sense. Also this disables any live changes to numa parameters (the /domain/numatune/memory settings) since we cannot change the settings given to qemu. Martin Kletzander (6): conf, schema: add 'id' field for cells conf, schema: add support for numatune memnode element conf: add virDomainGetMemsForGuestCpu() qemu: purely a code movement qemu: memory-ram capability probing qemu: pass numa node binding preferences to qemu docs/formatdomain.html.in | 26 ++- docs/schemas/domaincommon.rng | 22 ++ src/conf/cpu_conf.c| 39 +++- src/conf/domain_conf.c | 259 ++--- src/conf/domain_conf.h | 4 + src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_cgroup.c | 18 +- src/qemu/qemu_command.c| 160 +++-- src/qemu/qemu_command.h| 3 +- src/qemu/qemu_domain.c | 23 +- src/qemu/qemu_driver.c | 23 +- src/qemu/qemu_process.c| 3 +- src/util/virnuma.h | 14 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 ++ .../qemuxml2argv-numatune-auto-prefer.xml | 29 +++ .../qemuxml2argv-numatune-auto.args| 6 + .../qemuxml2argv-numatune-auto.xml | 26 +++ .../qemuxml2argv-numatune-memnode-nocpu.xml| 25 ++ .../qemuxml2argv-numatune-memnodes-problematic.xml | 31 +++ .../qemuxml2argv-numatune-memnodes.args| 8 + .../qemuxml2argv-numatune-memnodes.xml | 31 +++ .../qemuxml2argv-numatune-prefer.args | 6 + .../qemuxml2argv-numatune-prefer.xml | 29 +++ tests/qemuxml2argvtest.c | 51 ++-- .../qemuxml2xmlout-cpu-numa1.xml | 28 +++ .../qemuxml2xmlout-cpu-numa2.xml | 28 +++ tests/qemuxml2xmltest.c| 4 + tests/qemuxmlnstest.c | 2 +- 32 files changed, 845 insertions(+), 94 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-prefer.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-prefer.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] qemu: pass numa node binding preferences to qemu
Currently, we only bind the whole QEMU domain to memory nodes
specified in nodemask altogether. That, however, doesn't make much
sense when one wants to control from where the memory for particular
guest nodes should be allocated. QEMU allows us to do that by
specifying 'host-nodes' parameter for the 'memory-ram' object, so
let's use that.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_cgroup.c | 18 ++-
src/qemu/qemu_command.c| 141 +++--
src/qemu/qemu_command.h| 3 +-
src/qemu/qemu_driver.c | 12 +-
src/qemu/qemu_process.c| 3 +-
.../qemuxml2argv-numatune-auto-prefer.xml | 29 +
.../qemuxml2argv-numatune-auto.args| 6 +
.../qemuxml2argv-numatune-auto.xml | 26
.../qemuxml2argv-numatune-memnode-nocpu.xml| 25
.../qemuxml2argv-numatune-memnodes-problematic.xml | 31 +
.../qemuxml2argv-numatune-memnodes.args| 8 ++
.../qemuxml2argv-numatune-memnodes.xml | 31 +
.../qemuxml2argv-numatune-prefer.args | 6 +
.../qemuxml2argv-numatune-prefer.xml | 29 +
tests/qemuxml2argvtest.c | 50 +---
tests/qemuxml2xmltest.c| 1 +
tests/qemuxmlnstest.c | 2 +-
17 files changed, 389 insertions(+), 32 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-prefer.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-prefer.xml
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index b1bfb5a..6415a6d 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -23,6 +23,8 @@
#include config.h
+#include domain_audit.h
+#include domain_conf.h
#include qemu_cgroup.h
#include qemu_domain.h
#include qemu_process.h
@@ -30,7 +32,6 @@
#include virlog.h
#include viralloc.h
#include virerror.h
-#include domain_audit.h
#include virscsi.h
#include virstring.h
#include virfile.h
@@ -895,6 +896,7 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
size_t i, j;
unsigned long long period = vm-def-cputune.period;
long long quota = vm-def-cputune.quota;
+char *mem_mask = NULL;
if ((period || quota)
!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
@@ -930,8 +932,15 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
goto cleanup;
}
+if (virDomainGetMemsForGuestCpu(def, i, mem_mask) 0)
+goto cleanup;
+
/* Set vcpupin in cgroup if vcpupin xml is provided */
if (virCgroupHasController(priv-cgroup,
VIR_CGROUP_CONTROLLER_CPUSET)) {
+
+if (mem_mask virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) 0)
+goto cleanup;
+
/* find the right CPU to pin, otherwise
* qemuSetupCgroupVcpuPin will fail. */
for (j = 0; j def-cputune.nvcpupin; j++) {
@@ -946,14 +955,21 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
break;
}
+} else if (mem_mask) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(cgroup cpuset is required for
+ per-node memory binding));
+goto cleanup;
}
+VIR_FREE(mem_mask);
virCgroupFree(cgroup_vcpu);
}
return 0;
cleanup:
+VIR_FREE(mem_mask);
if (cgroup_vcpu) {
virCgroupRemove(cgroup_vcpu);
virCgroupFree(cgroup_vcpu);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cc6023d..8699eea 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6531,21 +6531,99 @@ qemuBuildSmpArgStr(const virDomainDef *def,
return virBufferContentAndReset(buf);
}
+static const char *
+qemuNumaPolicyToString(virDomainNumatuneMemMode mode)
+{
+const char *policy = NULL;
+
+switch (mode) {
+case VIR_DOMAIN_NUMATUNE_MEM_STRICT:
+policy = bind;
+break;
+
+case VIR_DOMAIN_NUMATUNE_MEM_PREFERRED:
+policy = preferred;
+break;
+
+case VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE:
+policy = interleave;
+break;
+
+case VIR_DOMAIN_NUMATUNE_MEM_LAST:
+virReportError(VIR_ERR_INTERNAL_ERROR
[libvirt] [PATCH 5/6] qemu: memory-ram capability probing
The numa patch series in qemu adds memory-ram object type by which
we can tell whether we can use such objects.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 08c3d04..a479035 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
usb-kbd, /* 165 */
host-pci-multidomain,
msg-timestamp,
+ memory-ram,
);
@@ -1440,6 +1441,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ ich9-intel-hda, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA },
{ pvpanic, QEMU_CAPS_DEVICE_PANIC },
{ usb-kbd, QEMU_CAPS_DEVICE_USB_KBD },
+{ memory-ram, QEMU_CAPS_OBJECT_MEMORY_RAM },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d755caa..350c43e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -206,6 +206,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */
QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain 0 in host pci
address */
QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */
+QEMU_CAPS_OBJECT_MEMORY_RAM = 168, /* -object memory-ram */
QEMU_CAPS_LAST, /* this must always be the last item */
};
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] conf, schema: add support for numatune memnode element
On Wed, Jun 04, 2014 at 04:07:06PM +0100, Daniel P. Berrange wrote:
On Wed, Jun 04, 2014 at 04:56:28PM +0200, Martin Kletzander wrote:
@@ -43,9 +43,17 @@ typedef virNumaTuneDef *virNumaTuneDefPtr;
struct _virNumaTuneDef {
struct {
virBitmapPtr nodemask;
-int mode;
+int mode; /* enum virDomainNumatuneMemMode */
int placement_mode; /* enum virNumaTuneMemPlacementMode */
-} memory;
+} memory; /* pinning for all the memory */
+
+struct mem_node {
Declaring structs inline without typedefs isn't our usual style. There
should be a typedef for virNumaTuneMemNodeDef virNumaTuneMemNodeDefPtr
Yeah, it should. Also the numatunedef should be in some conf/ file
with encapsulated insides. Parsing should be in that file too, of
course.
I took the liberty of proposing it this way and re-factoring
afterwards since all those details were drafting me away of the thing
I wanted to make out of that. However, I forgot to mention that the
same way I forgot to mention that virGetNumaNodeParameters() API will
follow even though is not part of this patch.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] maint: prohibit empty first lines
On Wed, Jun 04, 2014 at 10:14:24AM -0600, Eric Blake wrote:
On 06/04/2014 07:01 AM, Martin Kletzander wrote:
Based on discussion with Eric:
https://www.redhat.com/archives/libvir-list/2014-March/msg01001.html
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
v2:
- rebase on current master and s/FILENAME:1:/FILENAME :1:/
cfg.mk | 7 +++
1 file changed, 7 insertions(+)
The existing sc_prohibit_empty_lines_at_EOF in maint.mk does a similar
check but using perl instead of awk. But that shouldn't matter.
Yeah, but that is taken from gnulib, isn't it? Do you suggest
proposing this fix there?
diff --git a/cfg.mk b/cfg.mk
index 675af21..80440b5 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -938,6 +938,13 @@ sc_require_locale_h:
fi;
\
done;
+sc_prohibit_empty_first_line:
+ @awk 'BEGIN { fail=0; }\
+ FNR == 1 { if ($$0 == ) { print FILENAME :1:; fail=1; } } \
why is this to stdout...
I wanted to simulate the behaviour of other syntax-check rules that
use grep without output redirection. No problem of making this go to
stderr, though.
+ END { if (fail == 1) { \
+ print $(ME): Prohibited empty first line /dev/stderr; \
...but this to stderr?
+ } exit fail; }' $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$');
I'd use ALL of VC_LIST_EXCEPT (not just the .c and .h files). But in
general it looks like it does the right thing.
Michal had a good point of those other files. I'll remove the grep,
add necessary files into except rule and fix all the other non-.[ch]
ones.
Thanks for the reviews
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] translations: Don't leave default template fields in .po files
On Tue, Jun 03, 2014 at 09:46:31AM -0600, Eric Blake wrote: On 06/03/2014 09:40 AM, Martin Kletzander wrote: New gettext-0.19 doesn't like it and we can't build without it. Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: I'm *not* pushing it as a build-breaker as it's not that urgent. It should be updated in transifex as well. @DV: Can you update this in transifex or do I need to go through some special procedure? ACK, but I agree on waiting up to 24 hours for DV to advise on whether to push now or whether transifex can be fixed quickly. Since there was no response, I pushed it. If DV says there's anything else needed to make this stick or update in trasifex, etc., I'd be happy to do that. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/2] Empty first lines series
v3 -- use the whole VC_LIST_EXCEPT, create exception for necessities and clear the rest v2 -- s/FILENAME:1:/FILENAME :1:/ Martin Kletzander (2): Remove unnecessary empty first lines maint: prohibit empty first lines cfg.mk| 10 ++ docs/generic.css | 1 - docs/libvirt.css | 2 -- src/locking/lockd.conf| 1 - src/locking/sanlock.conf | 1 - tests/nodeinfodata/linux-test3/node/node0/meminfo | 1 - tests/nodeinfodata/linux-test3/node/node1/meminfo | 1 - tests/nodeinfodata/linux-test3/node/node2/meminfo | 1 - tests/nodeinfodata/linux-test3/node/node3/meminfo | 1 - tests/nodeinfodata/linux-test3/node/node4/meminfo | 1 - tests/nodeinfodata/linux-test3/node/node5/meminfo | 1 - tests/nodeinfodata/linux-test3/node/node6/meminfo | 1 - tests/nodeinfodata/linux-test3/node/node7/meminfo | 1 - tests/nodeinfodata/linux-test4/node/node0/meminfo | 1 - tests/nodeinfodata/linux-test4/node/node1/meminfo | 1 - tests/nodeinfodata/linux-test6/node/node0/meminfo | 1 - tests/nodeinfodata/linux-test8/cpu/offline| 1 - tests/nodeinfodata/linux-test8/node/node0/meminfo | 1 - tests/nodeinfodata/linux-test8/node/node1/meminfo | 1 - tests/nodeinfodata/linux-test8/node/node2/meminfo | 1 - tests/nodeinfodata/linux-test8/node/node3/meminfo | 1 - tests/nodeinfodata/linux-test8/node/node4/meminfo | 1 - tests/nodeinfodata/linux-test8/node/node5/meminfo | 1 - tests/nodeinfodata/linux-test8/node/node6/meminfo | 1 - tests/nodeinfodata/linux-test8/node/node7/meminfo | 1 - 25 files changed, 10 insertions(+), 25 deletions(-) -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/2] maint: prohibit empty first lines
Based on discussion with Eric:
https://www.redhat.com/archives/libvir-list/2014-March/msg01001.html
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
I've kept the first output (list of files) to stdout to sope with the
rest of grep checks, but I have no problem changing it to stderr
instead.
cfg.mk | 10 ++
1 file changed, 10 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index b5f1fa2..10ad744 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -936,6 +936,13 @@ sc_require_locale_h:
halt='setlocale() requires locale.h' \
$(_sc_search_regexp)
+sc_prohibit_empty_first_line:
+ @awk 'BEGIN { fail=0; } \
+ FNR == 1 { if ($$0 == ) { print FILENAME :1:; fail=1; } } \
+ END { if (fail == 1) { \
+ print $(ME): Prohibited empty first line /dev/stderr; \
+ } exit fail; }' $$($(VC_LIST_EXCEPT));
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
@@ -1115,3 +1122,6 @@
exclude_file_name_regexp--sc_avoid_attribute_unused_in_header = \
exclude_file_name_regexp--sc_prohibit_mixed_case_abbreviations = \
^src/(vbox/vbox_CAPI.*.h|esx/esx_vi.(c|h)|esx/esx_storage_backend_iscsi.c)$$
+
+exclude_file_name_regexp--sc_prohibit_empty_first_line = \
+
^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt)$$
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/2] Remove unnecessary empty first lines
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/generic.css | 1 -
docs/libvirt.css | 2 --
src/locking/lockd.conf| 1 -
src/locking/sanlock.conf | 1 -
tests/nodeinfodata/linux-test3/node/node0/meminfo | 1 -
tests/nodeinfodata/linux-test3/node/node1/meminfo | 1 -
tests/nodeinfodata/linux-test3/node/node2/meminfo | 1 -
tests/nodeinfodata/linux-test3/node/node3/meminfo | 1 -
tests/nodeinfodata/linux-test3/node/node4/meminfo | 1 -
tests/nodeinfodata/linux-test3/node/node5/meminfo | 1 -
tests/nodeinfodata/linux-test3/node/node6/meminfo | 1 -
tests/nodeinfodata/linux-test3/node/node7/meminfo | 1 -
tests/nodeinfodata/linux-test4/node/node0/meminfo | 1 -
tests/nodeinfodata/linux-test4/node/node1/meminfo | 1 -
tests/nodeinfodata/linux-test6/node/node0/meminfo | 1 -
tests/nodeinfodata/linux-test8/cpu/offline| 1 -
tests/nodeinfodata/linux-test8/node/node0/meminfo | 1 -
tests/nodeinfodata/linux-test8/node/node1/meminfo | 1 -
tests/nodeinfodata/linux-test8/node/node2/meminfo | 1 -
tests/nodeinfodata/linux-test8/node/node3/meminfo | 1 -
tests/nodeinfodata/linux-test8/node/node4/meminfo | 1 -
tests/nodeinfodata/linux-test8/node/node5/meminfo | 1 -
tests/nodeinfodata/linux-test8/node/node6/meminfo | 1 -
tests/nodeinfodata/linux-test8/node/node7/meminfo | 1 -
24 files changed, 25 deletions(-)
diff --git a/docs/generic.css b/docs/generic.css
index 4be5fa2..ce022cd 100644
--- a/docs/generic.css
+++ b/docs/generic.css
@@ -1,4 +1,3 @@
-
body {
margin: 0em;
padding: 0px;
diff --git a/docs/libvirt.css b/docs/libvirt.css
index 1d27873..b42c9de 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -1,5 +1,3 @@
-
-
h1 {
font-weight: normal;
color: #3c857c;
diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf
index 85edb91..fa43760 100644
--- a/src/locking/lockd.conf
+++ b/src/locking/lockd.conf
@@ -1,4 +1,3 @@
-
#
# The default lockd behaviour is to acquire locks directly
# against each configured disk file / block device. If the
diff --git a/src/locking/sanlock.conf b/src/locking/sanlock.conf
index 40ece65..e5566ef 100644
--- a/src/locking/sanlock.conf
+++ b/src/locking/sanlock.conf
@@ -1,4 +1,3 @@
-
#
# The default sanlock configuration requires the management
# application to manually define lease elements in the
diff --git a/tests/nodeinfodata/linux-test3/node/node0/meminfo
b/tests/nodeinfodata/linux-test3/node/node0/meminfo
index ec2bf50..710e59c 100644
--- a/tests/nodeinfodata/linux-test3/node/node0/meminfo
+++ b/tests/nodeinfodata/linux-test3/node/node0/meminfo
@@ -1,4 +1,3 @@
-
Node 0 MemTotal: 67098848 kB
Node 0 MemFree:64989968 kB
Node 0 MemUsed: 2108880 kB
diff --git a/tests/nodeinfodata/linux-test3/node/node1/meminfo
b/tests/nodeinfodata/linux-test3/node/node1/meminfo
index f84bd4c..c7b2209 100644
--- a/tests/nodeinfodata/linux-test3/node/node1/meminfo
+++ b/tests/nodeinfodata/linux-test3/node/node1/meminfo
@@ -1,4 +1,3 @@
-
Node 1 MemTotal: 67108864 kB
Node 1 MemFree:65361692 kB
Node 1 MemUsed: 1747172 kB
diff --git a/tests/nodeinfodata/linux-test3/node/node2/meminfo
b/tests/nodeinfodata/linux-test3/node/node2/meminfo
index c755e48..7a41b79 100644
--- a/tests/nodeinfodata/linux-test3/node/node2/meminfo
+++ b/tests/nodeinfodata/linux-test3/node/node2/meminfo
@@ -1,4 +1,3 @@
-
Node 2 MemTotal: 67108864 kB
Node 2 MemFree:64350240 kB
Node 2 MemUsed: 2758624 kB
diff --git a/tests/nodeinfodata/linux-test3/node/node3/meminfo
b/tests/nodeinfodata/linux-test3/node/node3/meminfo
index b580944..836e488 100644
--- a/tests/nodeinfodata/linux-test3/node/node3/meminfo
+++ b/tests/nodeinfodata/linux-test3/node/node3/meminfo
@@ -1,4 +1,3 @@
-
Node 3 MemTotal: 67108864 kB
Node 3 MemFree:65372380 kB
Node 3 MemUsed: 1736484 kB
diff --git a/tests/nodeinfodata/linux-test3/node/node4/meminfo
b/tests/nodeinfodata/linux-test3/node/node4/meminfo
index 8ad9b86..7897b2b 100644
--- a/tests/nodeinfodata/linux-test3/node/node4/meminfo
+++ b/tests/nodeinfodata/linux-test3/node/node4/meminfo
@@ -1,4 +1,3 @@
-
Node 4 MemTotal: 67108864 kB
Node 4 MemFree:64730416 kB
Node 4 MemUsed: 2378448 kB
diff --git a/tests/nodeinfodata/linux-test3/node/node5/meminfo
b/tests/nodeinfodata/linux-test3/node/node5/meminfo
index fb93073..59d2792 100644
--- a/tests/nodeinfodata/linux-test3/node/node5/meminfo
+++ b/tests/nodeinfodata/linux-test3/node/node5/meminfo
@@ -1,4 +1,3 @@
-
Node 5 MemTotal: 67108864 kB
Node 5 MemFree:65315792 kB
Node 5 MemUsed: 1793072 kB
diff --git a/tests/nodeinfodata/linux-test3/node/node6/meminfo
b/tests/nodeinfodata/linux-test3/node/node6/meminfo
index d4fa1d0..3c6c10b 100644
--- a/tests/nodeinfodata/linux-test3/node/node6/meminfo
+++ b/tests/nodeinfodata/linux-test3/node/node6/meminfo
@@ -1,4
Re: [libvirt] [PATCH] Remove ssp buffer size setting
On Fri, Jun 06, 2014 at 11:40:24AM +0200, Ján Tomko wrote: This option only makes sense with -fstack-protector. With -fstack-protector-all, even functions with buffers smaller than this are protected. https://bugzilla.redhat.com/show_bug.cgi?id=1105456 --- m4/virt-compile-warnings.m4 | 8 1 file changed, 8 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 574fbc4..ebc931d 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -171,14 +171,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl be great overhead in adding -fstack-protector-all instead dnl wantwarn=$wantwarn -fstack-protector wantwarn=$wantwarn -fstack-protector-all - wantwarn=$wantwarn --param=ssp-buffer-size=4 It would be nice to keep that line in here with the explanation that -fstack-protector-all does not make use of that param. - dnl Even though it supports it, clang complains about - dnl use of --param=ssp-buffer-size=4 unless used with - dnl the -c arg. It doesn't like it when used with args - dnl that just link together .o files. Unfortunately - dnl we can't avoid that with automake, so we must turn - dnl off the following clang specific warning - wantwarn=$wantwarn -Wno-unused-command-line-argument Why do you also remove this line? ;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle Also, out of the context of this patch, doesn't that param need to be added to the freebsd version since it uses -fstack-protector only? Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] Empty first lines series
On Fri, Jun 06, 2014 at 06:44:07AM -0600, Eric Blake wrote: On 06/06/2014 03:20 AM, Martin Kletzander wrote: v3 -- use the whole VC_LIST_EXCEPT, create exception for necessities and clear the rest v2 -- s/FILENAME:1:/FILENAME :1:/ Martin Kletzander (2): Remove unnecessary empty first lines maint: prohibit empty first lines cfg.mk| 10 ++ docs/generic.css | 1 - docs/libvirt.css | 2 -- src/locking/lockd.conf| 1 - src/locking/sanlock.conf | 1 - tests/nodeinfodata/linux-test3/node/node0/meminfo | 1 - Was this (and similar) file copied from an actual sysfs output? If so, and if sysfs includes the leading blank lines, then we should exempt these files rather than touching them up. There are more files like this, about half of them had the first line empty and half of them hadn't. I tested that there is no difference between those two and also checked, on systems I came to contact with, that there is no empty line in that file. Otherwise, ACK series. Thanks, pushed already after review from Dan. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] parallels: Avoid possible leak of cpu from parallelsBuildCapabilities
On Mon, Jun 09, 2014 at 09:39:44AM +0200, Peter Krempa wrote: 4d06af97d38c3648937eb8f732704379b3cd9e59 introduced a possible memory leak of the memory allocated into the cpu pointer in parallelsBuildCapabilities in the case nodeGetInfo() would fail right after the allocation. Rearrange the code to avoid the possibility of the leak. Found by Coverity. --- src/parallels/parallels_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK, Martin diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 153961b..411527c 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -151,10 +151,10 @@ parallelsBuildCapabilities(void) parallels, NULL, NULL, 0, NULL) == NULL) goto error; -if (VIR_ALLOC(cpu) 0) +if (nodeGetInfo(nodeinfo)) goto error; -if (nodeGetInfo(nodeinfo)) +if (VIR_ALLOC(cpu) 0) goto error; cpu-arch = caps-host.arch; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] virsh: Add details about specified migration host
On Tue, Jun 10, 2014 at 08:52:19AM -0600, Eric Blake wrote: From: Chen Fan chen.fan.f...@cn.fujitsu.com the 'migration_host' description may be a bit difficult to understand for some users, so enhance the manual Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com Signed-off-by: Eric Blake ebl...@redhat.com --- tools/virsh.pod | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) This version looks better, ACK. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/2] maint: prohibit empty first lines
On Tue, Jun 10, 2014 at 01:21:49PM -0600, Eric Blake wrote:
On 06/10/2014 12:18 PM, Roman Bogorodskiy wrote:
+sc_prohibit_empty_first_line:
+ @awk 'BEGIN { fail=0; } \
+ FNR == 1 { if ($$0 == ) { print FILENAME :1:; fail=1; } } \
+ END { if (fail == 1) { \
+ print $(ME): Prohibited empty first line /dev/stderr;\
+ } exit fail; }' $$($(VC_LIST_EXCEPT));
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
@@ -1115,3 +1122,6 @@
exclude_file_name_regexp--sc_avoid_attribute_unused_in_header = \
exclude_file_name_regexp--sc_prohibit_mixed_case_abbreviations = \
^src/(vbox/vbox_CAPI.*.h|esx/esx_vi.(c|h)|esx/esx_storage_backend_iscsi.c)$$
+
+exclude_file_name_regexp--sc_prohibit_empty_first_line = \
+
^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt)$$
It started to fail for me with this change:
prohibit_empty_first_line
tools/libvirt_win_icon_16x16.ico:1:
Hmm, it might be simpler just to globally exclude all image formats,
rather than repeating lists of image files in affected rules.
Definitely. It's weird, though, that I'm not seeing this problem. Is
it because I'm building on Linux? Why didn't some test box we have
fail as well?
I was able to fix it with:
exclude_file_name_regexp--sc_prohibit_empty_first_line = \
-
^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt)$$
+
^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt|tools/.*\.ico)$$
Not quite consistent with other places where we ignore image files; for
example:
exclude_file_name_regexp--sc_trailing_blank = \
(/qemuhelpdata/|\.(fig|gif|ico|png)$$)
but my thoughts are that we should really ditch those one-off exception
lists for graphic binary files, and instead modify this list:
VC_LIST_ALWAYS_EXCLUDE_REGEX = \
(^(HACKING|docs/(news\.html\.in|.*\.patch))|\.po)$$
And this looks like the right way to go. It would deal with possible
future problems as well. After figuring out the parentheses, I can
ACK this to go in if you want. I'm hoping to get to cleaning up the
excludes one day.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: exempt graphic binaries from syntax check
On Tue, Jun 10, 2014 at 01:39:03PM -0600, Eric Blake wrote: Roman Bogorodskiy reported a syntax-check failure when using FreeBSD; complaining that: prohibit_empty_first_line tools/libvirt_win_icon_16x16.ico:1: tools/libvirt_win_icon_32x32.ico:1: tools/libvirt_win_icon_48x48.ico:1: tools/libvirt_win_icon_64x64.ico:1: maint.mk: Prohibited empty first line In reality, the first 'line' of that file is NOT empty; but since it is a binary file, awk is not required to handle it gracefully. The simplest solution is to exempt all image files from syntax checks in the first place - after all, we only store them in git because they are inconvenient to regenerate, but they are not our preferred format for making modifications, and syntax check should only cover files that we are likely to modify. * cfg.mk (VC_LIST_ALWAYS_EXCLUDE_REGEX): Exempt images. (exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF): Simplify. (exclude_file_name_regexp--sc_trailing_blank): Likewise. ACK Signed-off-by: Eric Blake ebl...@redhat.com --- cfg.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index 10ad744..1f07639 100644 --- a/cfg.mk +++ b/cfg.mk @@ -90,7 +90,7 @@ endif # Files that should never cause syntax check failures. VC_LIST_ALWAYS_EXCLUDE_REGEX = \ - (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.po)$$ + (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ # Functions like free() that are no-ops on NULL arguments. useless_free_options = \ @@ -1047,7 +1047,7 @@ exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c|tests/vir(cgroup|pci)mock\.c)$$) exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ - (^tests/(qemuhelp|nodeinfo|virpcitest)data/|\.(gif|ico|png|diff)$$) + (^tests/(qemuhelp|nodeinfo|virpcitest)data/|\.diff$$) _src2=src/(util/vircommand|libvirt|lxc/lxc_controller|locking/lock_daemon) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ @@ -1093,7 +1093,7 @@ exclude_file_name_regexp--sc_require_config_h_first = \ ^(examples/|tools/virsh-edit\.c$$) exclude_file_name_regexp--sc_trailing_blank = \ - (/qemuhelpdata/|/sysinfodata/.*\.data|\.(fig|gif|ico|png)$$) + /qemuhelpdata/|/sysinfodata/.*\.data$$ exclude_file_name_regexp--sc_unmarked_diagnostics = \ ^(docs/apibuild.py|tests/virt-aa-helper-test)$$ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] qemu: Properly label FDs when restoring domain with static label
On Wed, Jun 11, 2014 at 09:48:34AM -0400, Shivaprasad G Bhat wrote: When saving domain with relabel=no, the file that gets created must have the context set anyway. That way restore can be successful without the need of relabeling the file. Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5d40239..cf5c27c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2966,6 +2966,9 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, if (fd 0) goto cleanup; +if (virSecurityManagerSetImageFDLabel(driver-securityManager, vm-def, fd) 0) +goto cleanup; + if (!(wrapperFd = virFileWrapperFdNew(fd, path, wrapperFlags))) goto cleanup; ACK Pushed. Thanks, Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] virpci: Introduce virPCIDeviceIsPCIExpress and friends
On Fri, Jun 06, 2014 at 12:54:17PM +0200, Michal Privoznik wrote:
These functions will handle PCIe devices and their link capabilities
to query some info about it.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/libvirt_private.syms | 3 ++
src/util/virpci.c| 81 +++-
src/util/virpci.h| 8 +
3 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d73a9f5..f72a3ad 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1690,6 +1690,8 @@ virPCIDeviceFree;
virPCIDeviceGetDriverPathAndName;
virPCIDeviceGetIOMMUGroupDev;
virPCIDeviceGetIOMMUGroupList;
+virPCIDeviceGetLinkCap;
+virPCIDeviceGetLinkSta;
virPCIDeviceGetManaged;
virPCIDeviceGetName;
virPCIDeviceGetRemoveSlot;
@@ -1698,6 +1700,7 @@ virPCIDeviceGetStubDriver;
virPCIDeviceGetUnbindFromStub;
virPCIDeviceGetUsedBy;
virPCIDeviceIsAssignable;
+virPCIDeviceIsPCIExpress;
virPCIDeviceListAdd;
virPCIDeviceListAddCopy;
virPCIDeviceListCount;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index e0f2344..8ad28b8 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -130,7 +130,13 @@ struct _virPCIDeviceList {
/* PCIe20 7.8.3 Device Capabilities Register (Offset 04h) */
#define PCI_EXP_DEVCAP 0x4 /* Device capabilities */
-#define PCI_EXP_DEVCAP_FLR (128) /* Function Level Reset */
+#define PCI_EXP_LNKCAP 0xc /* Link Capabilities */
+#define PCI_EXP_LNKSTA 0x12/* Link Status */
+#define PCI_EXP_DEVCAP_FLR (128) /* Function Level Reset */
+#define PCI_EXP_LNKCAP_SPEED0xf /* Maximum Link Speed */
+#define PCI_EXP_LNKCAP_WIDTH0x003f0 /* Maximum Link Width */
+#define PCI_EXP_LNKSTA_SPEED0x000f /* Negotiated Link Speed */
+#define PCI_EXP_LNKSTA_WIDTH0x03f0 /* Negotiated Link Width */
These two sets are essentially the same, just keep it as e.g.
PCI_EXP_LINK_(SPEED|WIDTH) with 0x(f|3f) respectively. And keep the
order of the names alphabetical.
/* Header type 1 BR12 3.2 PCI-to-PCI Bridge Configuration Space Header Format */
#define PCI_PRIMARY_BUS 0x18/* BR12 3.2.5.2 Primary bus number */
@@ -2750,3 +2756,76 @@ virPCIGetVirtualFunctionInfo(const char
*vf_sysfs_device_path ATTRIBUTE_UNUSED,
return -1;
}
#endif /* __linux__ */
+
+int
+virPCIDeviceIsPCIExpress(virPCIDevicePtr dev)
+{
+int fd;
+int ret = -1;
+
+if ((fd = virPCIDeviceConfigOpen(dev, true)) 0)
+return ret;
+
+if (virPCIDeviceInit(dev, fd) 0)
+goto cleanup;
+
+ret = dev-pcie_cap_pos != 0;
+
+ cleanup:
+virPCIDeviceConfigClose(dev, fd);
+return ret;
+}
+
+int
+virPCIDeviceGetLinkCap(virPCIDevicePtr dev,
+ int *port,
+ unsigned int *speed,
+ unsigned int *width)
+{
+uint32_t t;
+int fd;
+int ret = -1;
+
+if ((fd = virPCIDeviceConfigOpen(dev, true) 0))
+return ret;
+
+if (virPCIDeviceInit(dev, fd) 0)
+goto cleanup;
+
+t = virPCIDeviceRead32(dev, fd, dev-pcie_cap_pos + PCI_EXP_LNKCAP);
+
+*port = t 0x18;
Took me second to figure out you just want the highest byte.
s/0x18/24/ would make that much more clear, at least for me.
+*speed = t PCI_EXP_LNKCAP_SPEED;
+*width = (t PCI_EXP_LNKCAP_WIDTH) 4;
And if I wanted to be *really* digging deep into this and nitpicking a
lot, I'd say you can do:
port = Read8
speed = Read16 SPEED
width = Read8
+ret = 0;
+
+ cleanup:
+virPCIDeviceConfigClose(dev, fd);
+return ret;
+}
+
+int
+virPCIDeviceGetLinkSta(virPCIDevicePtr dev,
+ unsigned int *speed,
+ unsigned int *width)
+{
+uint32_t t;
+int fd;
+int ret = -1;
+
+if ((fd = virPCIDeviceConfigOpen(dev, true) 0))
+return ret;
+
+if (virPCIDeviceInit(dev, fd) 0)
+goto cleanup;
+
+t = virPCIDeviceRead32(dev, fd, dev-pcie_cap_pos + PCI_EXP_LNKSTA);
+
+*speed = t PCI_EXP_LNKSTA_SPEED;
+*width = (t PCI_EXP_LNKSTA_WIDTH) 4;
+ret = 0;
+
+ cleanup:
+virPCIDeviceConfigClose(dev, fd);
+return ret;
+}
Both these functions do the same thing with 2 micro differences,
either merge it to one, so you avoid double open, init, etc. Or
create one that at least takes a parameter saying whether the caller
wants CAPability or STAtus data.
Other than that it looks good.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] nodedev: Introduce pci-express/ to PCI devices
On Fri, Jun 06, 2014 at 12:54:18PM +0200, Michal Privoznik wrote:
This new element is there to represent PCI-Express capabilities
of a PCI devices, like link speed, number of lanes, etc.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
docs/formatnode.html.in| 19
docs/schemas/nodedev.rng | 26 +
src/conf/node_device_conf.c| 123 -
src/conf/node_device_conf.h| 31 +-
src/node_device/node_device_udev.c | 31 ++
.../pci_8086_4238_pcie_wireless.xml| 26 +
tests/nodedevxml2xmltest.c | 1 +
7 files changed, 254 insertions(+), 3 deletions(-)
create mode 100644 tests/nodedevschemadata/pci_8086_4238_pcie_wireless.xml
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index b424c96..46ec2bc 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -110,6 +110,21 @@
have a list of codeaddress/code subelements, one
for each VF on this PF.
/dd
+ dtcodepci-express/code/dt
+ dd
+ This optional element contains information on PCI Express part of
+ the device. For example, it can contain a child element
+ codelink/code which addresses the PCI Express device's link.
+ While a device has it's own capabilities
+ (codevalidity='cap'/code), the actual run time capabilities
+ are negotiated on the device initialization
+ (codevalidity='sta'/code). The codelink/code element then
+ contains three attributes: codeport/code which says in which
+ port is the device plugged in, codespeed/code (in
+ GigaTransfers per second) and codewidth/code for the number
+ of lanes used. Since the port can't be negotiated, it's not
+ exposed in code./pci-express/link/[@validity='sta']/code.
+ /dd
/dl
/dd
dtcodeusb_device/code/dt
@@ -291,6 +306,10 @@
lt;address domain='0x' bus='0x02' slot='0x00' function='0x0'/gt;
lt;address domain='0x' bus='0x02' slot='0x00' function='0x1'/gt;
lt;/iommuGroupgt;
+lt;pci-expressgt;
+ lt;link validity='cap' port='1' speed='2.5' width='1'/gt;
+ lt;link validity='sta' speed='2.5' width='1'/gt;
+lt;/pci-expressgt;
lt;/capabilitygt;
lt;/devicegt;
/pre
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 81ab4d4..79e8fd2 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -158,6 +158,32 @@
/element
/optional
+optional
+element name='pci-express'
+zeroOrMore
+element name='link'
+attribute name='validity'
+choice
+valuecap/value
+valuesta/value
+/choice
+/attribute
+optional
+attribute name='port'
+ref name='unsignedInt'/
+/attribute
+/optional
+optional
+attribute name='speed'
If you don't want to name the values here, there should be at least
text/ or data type=string/. Maybe the best would be to do
(not tested):
data type=string
param name=pattern[0-9]+(.[0-9]+)?/param
/data
+/attribute
+/optional
+attribute name='width'
+ref name='unsignedInt'/
+/attribute
+/element
+/zeroOrMore
+/element
+/optional
/define
define name='capusbdev'
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index e65b5e4..70634cc 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -58,6 +58,9 @@ VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST,
80203,
80211)
+VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST,
+ , 2.5, 5, 8)
+
static int
virNodeDevCapsDefParseString(const char *xpath,
xmlXPathContextPtr ctxt,
@@ -218,6 +221,35 @@ void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
}
}
+static void
+virPCIELinkFormat(virBufferPtr buf,
+ virPCIELinkPtr lnk,
+ const char *attrib)
+{
+virBufferAsprintf(buf, link validity='%s', attrib);
+if (lnk-port = 0)
+virBufferAsprintf(buf, port='%d', lnk-port);
+if (lnk-speed)
+virBufferAsprintf(buf, speed='%s',
+ virPCIELinkSpeedTypeToString(lnk-speed));
+virBufferAsprintf(buf, width='%d', lnk-width);
+virBufferAddLit(buf, /\n);
+}
+
+static void
+virPCIEDeviceInfoFormat(virBufferPtr
Re: [libvirt] [PATCH] virNodeDevCapPCIDevParseXML: Initialize numa_node variable
On Thu, Jun 12, 2014 at 11:27:31AM +0200, Michal Privoznik wrote: With one of my recent patches (1c70277) libvirt's capable of reporting NUMA node locality for PCI devices. The node ID is stored in pci_dev.numa_node variable. However, since zero is valid NUMA node ID, the default is -1 as it is in kernel too. So, if the PCI device is not tied to any specific NUMA node, the default is then NOT printed into XML. Therefore, when parsing node device XML, the node/ element is optional. But currently, if it's not there, we must set sane default, otherwise after parsing the in memory representation doesn't match the XML. We s/the in/in the/ are already doing this in other place: udevProcessPCI(). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/node_device_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 6153aa1..12e40e6 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1145,6 +1145,8 @@ virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt, } } +/* The default value is -1 since zero is valid NUMA node number */ +data-pci_dev.numa_node = -1; if (virNodeDevCapsDefParseIntOptional(number(./numa[1]/@node), ctxt, data-pci_dev.numa_node, def, _(invalid NUMA node ID supplied for '%s')) 0) ACK, Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] virpci: Introduce virPCIDeviceIsPCIExpress and friends
On Thu, Jun 12, 2014 at 12:08:08PM +0200, Michal Privoznik wrote:
On 12.06.2014 10:56, Martin Kletzander wrote:
On Fri, Jun 06, 2014 at 12:54:17PM +0200, Michal Privoznik wrote:
These functions will handle PCIe devices and their link capabilities
to query some info about it.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/libvirt_private.syms | 3 ++
src/util/virpci.c| 81
+++-
src/util/virpci.h| 8 +
3 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d73a9f5..f72a3ad 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1690,6 +1690,8 @@ virPCIDeviceFree;
virPCIDeviceGetDriverPathAndName;
virPCIDeviceGetIOMMUGroupDev;
virPCIDeviceGetIOMMUGroupList;
+virPCIDeviceGetLinkCap;
+virPCIDeviceGetLinkSta;
virPCIDeviceGetManaged;
virPCIDeviceGetName;
virPCIDeviceGetRemoveSlot;
@@ -1698,6 +1700,7 @@ virPCIDeviceGetStubDriver;
virPCIDeviceGetUnbindFromStub;
virPCIDeviceGetUsedBy;
virPCIDeviceIsAssignable;
+virPCIDeviceIsPCIExpress;
virPCIDeviceListAdd;
virPCIDeviceListAddCopy;
virPCIDeviceListCount;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index e0f2344..8ad28b8 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -130,7 +130,13 @@ struct _virPCIDeviceList {
/* PCIe20 7.8.3 Device Capabilities Register (Offset 04h) */
#define PCI_EXP_DEVCAP 0x4 /* Device capabilities */
-#define PCI_EXP_DEVCAP_FLR (128) /* Function Level Reset */
+#define PCI_EXP_LNKCAP 0xc /* Link Capabilities */
+#define PCI_EXP_LNKSTA 0x12/* Link Status */
+#define PCI_EXP_DEVCAP_FLR (128) /* Function Level Reset */
+#define PCI_EXP_LNKCAP_SPEED0xf /* Maximum Link Speed */
+#define PCI_EXP_LNKCAP_WIDTH0x003f0 /* Maximum Link Width */
+#define PCI_EXP_LNKSTA_SPEED0x000f /* Negotiated Link Speed */
+#define PCI_EXP_LNKSTA_WIDTH0x03f0 /* Negotiated Link Width */
These two sets are essentially the same, just keep it as e.g.
PCI_EXP_LINK_(SPEED|WIDTH) with 0x(f|3f) respectively. And keep the
order of the names alphabetical.
The values are copied from /usr/include/pci/header.h. One day we could
drop all of these copies and include the file directly. For that, I'd
rather keep it just as is in the foreign file. Until the time we do
that, I'm sorting these alphabetically, okay.
Well, the only ones I was concerned with were simple masks for 8 bits
and 13 bits, there's nothing special about those. We even use:
(1 x) - 1 for mask of x bits somewhere, I think. Including them
fromt he system is probably the best option (for applicable systems,
of course), so that would be preferred. Be sure we depend on
pciutils-devel or what's the package if we don't already.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] virCaps: expose huge page info
On Thu, Jun 12, 2014 at 02:30:50PM +0100, Daniel P. Berrange wrote:
On Tue, Jun 10, 2014 at 07:21:12PM +0200, Michal Privoznik wrote:
There are two places where you'll find info on huge pages. The first
one is under cpu/ element, where all supported huge page sizes are
listed. Then the second one is under each cell/ element which refers
to concrete NUMA node. At this place, the size of huge page's pool is
reported. So the capabilities XML looks something like this:
capabilities
host
uuid01281cda-f352-cb11-a9db-e905fe22010c/uuid
cpu
archx86_64/arch
modelWestmere/model
vendorIntel/vendor
topology sockets='1' cores='1' threads='1'/
...
pages unit='KiB' size='1048576'/
pages unit='KiB' size='2048'/
Should have normal sized pages (ie 4k on x86) too, to avoid
apps having to special case small pages.
Since we have to special-case small pages and kernel (at least to my
knowledge) doesn't expose that information by classic means, I think
reporting only hugepages is actually what we want here. For normal
memory there are existing APIs already.
Hugepages are different mainly because of one thing. The fact that
there are some hugepages allocated is known by the user of the machine
(be it mgmt app or an admin) and these hugepages were allocated for
some purpose. It is fairly OK to presume that the number of hugepages
(free or total) will change only when and if the user wants to
(e.g. running a machine with specified size and hugepages). That
cannot be said about small pages, though, and I think it is fair
reason to special-case normal pages like this.
Martin
/cpu
...
topology
cells num='4'
cell id='0'
memory unit='KiB'4054408/memory
pages unit='KiB' size='1048576'1/pages
pages unit='KiB' size='2048'3/pages
Should have normal sized pages here too.
We should also declare whether 'memory' refers to total memory
across all page sizes, or just total memory of smallest page
sizes. I'd say it should refer to all memory, since conceptually
this is really about telling you how much RAM DIMMS are in each
node.
distances/
cpus num='1'
cpu id='0' socket_id='0' core_id='0' siblings='0'/
/cpus
/cell
cell id='1'
memory unit='KiB'4071072/memory
pages unit='KiB' size='1048576'2/pages
pages unit='KiB' size='2048'1024/pages
distances/
cpus num='1'
cpu id='1' socket_id='0' core_id='0' siblings='1'/
/cpus
/cell
...
/cells
/topology
...
/host
guest/
/capabilities
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
docs/schemas/capability.rng | 21 +
src/conf/capabilities.c | 25 ++---
src/conf/capabilities.h | 15 ++-
src/libxl/libxl_conf.c | 1 +
src/nodeinfo.c | 41 -
src/qemu/qemu_capabilities.c | 29 -
src/test/test_driver.c | 2 +-
src/xen/xend_internal.c | 1 +
tests/vircaps2xmltest.c | 3 ++-
tests/vircapstest.c | 1 +
10 files changed, 131 insertions(+), 8 deletions(-)
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 53a83c9..384e256 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -102,6 +102,13 @@ struct _virCapsHostNUMACellSiblingInfo {
unsigned int distance; /* distance to the node */
};
+typedef struct _virCapsHostNUMACellHugePageInfo
virCapsHostNUMACellHugePageInfo;
+typedef virCapsHostNUMACellHugePageInfo *virCapsHostNUMACellHugePageInfoPtr;
+struct _virCapsHostNUMACellHugePageInfo {
+unsigned int size; /* huge page size in kibibytes */
+size_t avail; /* the size of pool */
+};
s/Huge//. since this should be used to report on all page sizes
+
typedef struct _virCapsHostNUMACell virCapsHostNUMACell;
typedef virCapsHostNUMACell *virCapsHostNUMACellPtr;
struct _virCapsHostNUMACell {
@@ -111,6 +118,8 @@ struct _virCapsHostNUMACell {
virCapsHostNUMACellCPUPtr cpus;
int nsiblings;
virCapsHostNUMACellSiblingInfoPtr siblings;
+int nhugepages;
+virCapsHostNUMACellHugePageInfoPtr hugepages;
Better named as 'pageinfo' rather than 'hugepages'
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virNetDevGetLinkInfo: Don't report link speed if NIC's down
On Fri, Jun 13, 2014 at 08:46:59AM +0200, Michal Privoznik wrote:
The kernel's more broken than one would think. Various drivers report
various (usually spurious) values if the interface is down. While on
some we experience -EINVAL when read()-ing the speed sysfs file, with
other drivers we might get anything from 0 to UINT_MAX. If that's the
case it's better to not report link speed. Well, the interface is down
anyway.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virnetdev.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 6f3a202..80ef572 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1843,7 +1843,7 @@ virNetDevGetLinkInfo(const char *ifname,
char *buf = NULL;
char *tmp;
int tmp_state;
-unsigned int tmp_speed;
+unsigned int tmp_speed; /* virInterfaceState */
You probably wanted to put this comment next to the line with
tmp_state and not tmp_speed.
if (virNetDevSysfsFile(path, ifname, operstate) 0)
goto cleanup;
@@ -1875,6 +1875,16 @@ virNetDevGetLinkInfo(const char *ifname,
lnk-state = tmp_state;
+/* Shortcut to avoid some kernel issues. If link is down (and possibly in
+ * other states too) several drivers report several values. While igb
+ * reports 65535, realtek goes with 10. To avoid muddying XML with insane
+ * values, don't report link speed */
+if (lnk-state == VIR_INTERFACE_STATE_DOWN) {
+lnk-speed = 0;
+ret = 0;
+goto cleanup;
+}
+
Pity that's not standardized, but what you did makes more sense than
anything else. If the link is down, then no need to report it.
VIR_FREE(path);
VIR_FREE(buf);
@@ -1884,6 +1894,7 @@ virNetDevGetLinkInfo(const char *ifname,
if (virFileReadAll(path, 1024, buf) 0) {
/* Some devices doesn't report speed, in which case we get EINVAL */
if (errno == EINVAL) {
+lnk-speed = 0;
ret = 0;
goto cleanup;
}
--
1.8.5.5
ACK,
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virNetDevGetLinkInfo: Don't report link speed if NIC's down
On Fri, Jun 13, 2014 at 10:31:53AM +0300, Laine Stump wrote:
On 06/13/2014 10:10 AM, Martin Kletzander wrote:
On Fri, Jun 13, 2014 at 08:46:59AM +0200, Michal Privoznik wrote:
The kernel's more broken than one would think. Various drivers report
various (usually spurious) values if the interface is down. While on
some we experience -EINVAL when read()-ing the speed sysfs file, with
other drivers we might get anything from 0 to UINT_MAX. If that's the
case it's better to not report link speed. Well, the interface is down
anyway.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virnetdev.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 6f3a202..80ef572 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1843,7 +1843,7 @@ virNetDevGetLinkInfo(const char *ifname,
char *buf = NULL;
char *tmp;
int tmp_state;
-unsigned int tmp_speed;
+unsigned int tmp_speed; /* virInterfaceState */
You probably wanted to put this comment next to the line with
tmp_state and not tmp_speed.
if (virNetDevSysfsFile(path, ifname, operstate) 0)
goto cleanup;
@@ -1875,6 +1875,16 @@ virNetDevGetLinkInfo(const char *ifname,
lnk-state = tmp_state;
+/* Shortcut to avoid some kernel issues. If link is down (and
possibly in
+ * other states too) several drivers report several values.
While igb
+ * reports 65535, realtek goes with 10. To avoid muddying XML
with insane
+ * values, don't report link speed */
+if (lnk-state == VIR_INTERFACE_STATE_DOWN) {
Also for VIR_INTERFACE_LOWER_LAYER_DOWN (verified by looking at the
speed reported by a macvtap device when its physdev is down). And I'm
not sure how to get an interface into NOT_PRESENT or DORMANT state,
but I would imagine that the speed should be 0 in those cases too.
I've seen many other states I have no idea how to achieve. Wouldn't
it make more sense to report the speed only if the state is UP?
ACK with LOWER_LAYER_DOWN added (I won't insist on the others
until/unless I see experimental evidence that they need it).
BTW, thinking more about bridge devices - maybe they should be given
state up if the device has been ifup'ed. In other words, in their case
you could call the functional equivalent of if_is_active() in netcf
(which does an SIOCGIFFLAGS ioctl and checks for the IFF_UP flag). (in
any case, bridges should probably just always report a speed of 0).
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] virCaps: expose huge page info
On Fri, Jun 13, 2014 at 04:30:41PM +0200, Michal Privoznik wrote: On 13.06.2014 10:28, Daniel P. Berrange wrote: On Thu, Jun 12, 2014 at 07:21:47PM +0200, Martin Kletzander wrote: On Thu, Jun 12, 2014 at 02:30:50PM +0100, Daniel P. Berrange wrote: On Tue, Jun 10, 2014 at 07:21:12PM +0200, Michal Privoznik wrote: There are two places where you'll find info on huge pages. The first one is under cpu/ element, where all supported huge page sizes are listed. Then the second one is under each cell/ element which refers to concrete NUMA node. At this place, the size of huge page's pool is reported. So the capabilities XML looks something like this: capabilities host uuid01281cda-f352-cb11-a9db-e905fe22010c/uuid cpu archx86_64/arch modelWestmere/model vendorIntel/vendor topology sockets='1' cores='1' threads='1'/ ... pages unit='KiB' size='1048576'/ pages unit='KiB' size='2048'/ Should have normal sized pages (ie 4k on x86) too, to avoid apps having to special case small pages. Since we have to special-case small pages and kernel (at least to my knowledge) doesn't expose that information by classic means, I think reporting only hugepages is actually what we want here. For normal memory there are existing APIs already. Hugepages are different mainly because of one thing. The fact that there are some hugepages allocated is known by the user of the machine (be it mgmt app or an admin) and these hugepages were allocated for some purpose. It is fairly OK to presume that the number of hugepages (free or total) will change only when and if the user wants to (e.g. running a machine with specified size and hugepages). That cannot be said about small pages, though, and I think it is fair reason to special-case normal pages like this. That difference is something that's only relevant to the person who is provisioning the machine though. For applications consuming the libvirt APIs it is not relevant. For OpenStack we really want to have normal size pages dealt with the in the same way as huge pages since it will simplify our schedular/placement logic. So I really want these APIs to do this in libvirt so that OpenStack doesn't have to reverse engineer this itself. But if we go this way, there are black holes hidden. For instance, the sizeof(ordinary pages pool). This is not accessible anywhere and the only algorithm I can think of is to take [(MemTotal on NODE #i) - sum(mem taken by all huge pages)] / PAGE_SIZE. So for instance on my machine where I have 1GB huge page per NUMA node, and 3 2MB per NUMA node: # grep MemTotal /sys/devices/system/node/node0/meminfo Node 0 MemTotal:4054408 kB # cat /sys/devices/system/node/node0/hugepages/hugepages-1048576kB/nr_hugepages 1 # cat /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages 3 # getconf PAGESIZE 4096 (4054408 - (1*1048576 + 3*2048)) / 4 = 2999688 / 4 = 749922 ordinary pages. But it's not that simple as not all pages are available. Some are reserved for DMA transfers, some for kernel itself, etc. Without overcommit it's impossible to allocate that nearly 3GB. Is this something we really want to do? I've found one other way to get the number of free normal pages. It looks like nr_free_pages in /proc/zoneinfo is what Daniel wants to report probably. But given the fact that this is something that might not be true even in the time of parsing the file, I'm still not convinced it's something you want to report. Bit more accurate would be having the amount of memory that might be available to the machine. Although with overcommit settings and file caches this might not be feasible. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] virpci: Introduce virPCIDeviceIsPCIExpress and friends
On Thu, Jun 12, 2014 at 05:27:34PM +0200, Michal Privoznik wrote:
These functions will handle PCIe devices and their link capabilities
to query some info about it.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/libvirt_private.syms | 3 ++
src/util/virpci.c| 96 +++-
src/util/virpci.h| 8
3 files changed, 106 insertions(+), 1 deletion(-)
[...]
diff --git a/src/util/virpci.c b/src/util/virpci.c
index e0f2344..6f4f6af 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -120,6 +120,7 @@ struct _virPCIDeviceList {
/* PCI30 6.7 Capabilities List */
#define PCI_CAPABILITY_LIST 0x34/* Offset of first capability list
entry */
+#define PCI_CAP_FLAGS 2 /* Capability defined flags (16 bits)
*/
/* PM12 3.2.1 Capability Identifier */
#define PCI_CAP_ID_PM 0x01/* Power Management */
@@ -130,7 +131,13 @@ struct _virPCIDeviceList {
/* PCIe20 7.8.3 Device Capabilities Register (Offset 04h) */
#define PCI_EXP_DEVCAP 0x4 /* Device capabilities */
-#define PCI_EXP_DEVCAP_FLR (128) /* Function Level Reset */
+#define PCI_EXP_DEVCAP_FLR (128) /* Function Level Reset */
+#define PCI_EXP_LNKCAP 0xc /* Link Capabilities */
+#define PCI_EXP_LNKCAP_SPEED0xf /* Maximum Link Speed */
+#define PCI_EXP_LNKCAP_WIDTH0x003f0 /* Maximum Link Width */
+#define PCI_EXP_LNKSTA 0x12/* Link Status */
+#define PCI_EXP_LNKSTA_SPEED0x000f /* Negotiated Link Speed */
+#define PCI_EXP_LNKSTA_WIDTH0x03f0 /* Negotiated Link Width */
I'm still not convinced that this isn't just obfuscating the code, but
as far as the usage goes, this work (and will work) as expected, I
just blabbed about the looks of it.
ACK,
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] nodedev: Introduce pci-express/ to PCI devices
On Thu, Jun 12, 2014 at 05:27:35PM +0200, Michal Privoznik wrote:
This new element is there to represent PCI-Express capabilities
of a PCI devices, like link speed, number of lanes, etc.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
[...]
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 505f913..0a8b7fa 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -168,6 +168,32 @@
/element
/optional
+optional
+ element name='pci-express'
+zeroOrMore
+ element name='link'
+attribute name='validity'
+ choice
+valuecap/value
+valuesta/value
+ /choice
+/attribute
+optional
+ attribute name='port'
+ref name='unsignedInt'/
+ /attribute
+/optional
+optional
+ attribute name='speed'
List the values here or use the string with pattern from my suggestion
in v1.
+ /attribute
+/optional
+attribute name='width'
+ ref name='unsignedInt'/
+/attribute
+ /element
+/zeroOrMore
+ /element
+/optional
/define
define name='capusbdev'
[...]
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index d1311dd..b95ccf1 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -76,10 +76,38 @@ typedef enum {
} virNodeDevSCSIHostCapFlags;
typedef enum {
-VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION= (1 0),
-VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 1),
+VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 0),
+VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 1),
+VIR_NODE_DEV_CAP_FLAG_PCIE = (1 2),
} virNodeDevPCICapFlags;
+typedef enum {
+VIR_PCIE_LINK_SPEED_NA = 0,
+VIR_PCIE_LINK_SPEED_25,
+VIR_PCIE_LINK_SPEED_5,
+VIR_PCIE_LINK_SPEED_8,
+VIR_PCIE_LINK_SPEED_LAST
+} virPCIELinkSpeed;
+
+VIR_ENUM_DECL(virPCIELinkSpeed)
+
+typedef struct _virPCIELink virPCIELink;
+typedef virPCIELink *virPCIELinkPtr;
+struct _virPCIELink {
+int port;
+virPCIELinkSpeed speed;
+unsigned int width;
+};
+
+typedef struct _virPCIEDeviceInfo virPCIEDeviceInfo;
+typedef virPCIEDeviceInfo *virPCIEDeviceInfoPtr;
+struct _virPCIEDeviceInfo {
+/* Not all PCI Express devices has link. For example this 'Root Complex
+ * Integrated Endpoint' and 'Root Complex Event Collector' doesn't have. */
Actually, exactly only these two do not have it.
[...]
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index a8e74e4..bb6a0b9 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -426,6 +426,8 @@ static int udevProcessPCI(struct udev_device *device,
const char *syspath = NULL;
union _virNodeDevCapData *data = def-caps-data;
virPCIDeviceAddress addr;
+virPCIEDeviceInfoPtr pci_express = NULL;
+virPCIDevicePtr pciDev = NULL;
int tmpGroup, ret = -1;
char *p;
int rc;
@@ -535,9 +537,41 @@ static int udevProcessPCI(struct udev_device *device,
data-pci_dev.iommuGroupNumber = tmpGroup;
}
+if (!(pciDev = virPCIDeviceNew(data-pci_dev.domain,
+ data-pci_dev.bus,
+ data-pci_dev.slot,
+ data-pci_dev.function)))
+goto out;
+
+if (virPCIDeviceIsPCIExpress(pciDev) 0) {
+if (VIR_ALLOC(pci_express) 0)
+goto out;
+
+if (virPCIDeviceHasPCIExpressLink(pciDev) 0) {
+if (VIR_ALLOC(pci_express-link_cap) 0 ||
+VIR_ALLOC(pci_express-link_sta) 0)
+goto out;
+
+if (virPCIDeviceGetLinkCapSta(pciDev,
+ pci_express-link_cap-port,
+ pci_express-link_cap-speed,
+ pci_express-link_cap-width,
+ pci_express-link_sta-speed,
+ pci_express-link_sta-width) 0)
You wouldn't have to pass all the params if you defined the structs in
virpci, but that's just a cosmetic thing.
ACK with the RelaxNG schema fixed,
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: link virstoragetest with libxml
Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: To be honest, I have no idea why this fails for me in one situation, but it prevents the following error during compilation: /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.0/../../../../x86_64-pc-linux-gnu/bin/ld: ../src/.libs/libvirt_driver_storage_impl.a(libvirt_driver_storage_impl_la-storage_backend.o): undefined reference to symbol 'xmlFreeDoc@@LIBXML2_2.4.30' /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.0/../../../../lib64/libxml2.so: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status Makefile:4228: recipe for target 'virstoragetest' failed Therefore I'm not pushing it as a build-breaker since this might not be the root cause or the best solution. The other fix (and probably more appropriate one) would be to add LIBXML_LIBS into libvirt_conf_la_LIBADD since the xmlFreeDoc() is called in storage_conf.c. Any other preferred way is accepted as well, feel free to comment. tests/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index 025b847..457eb99 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -899,6 +899,7 @@ virstoragetest_LDADD = $(LDADDS) \ ../src/libvirt_util.la \ ../src/libvirt_driver_storage_impl.la \ ../gnulib/lib/libgnu.la \ + $(LIBXML_LIBS) \ $(NULL) viridentitytest_SOURCES = \ -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] LXC: check whether we get MemSwap[Total|Usage]
On Wed, Jun 25, 2014 at 09:57:32AM +0800, Chen Hanxiao wrote: Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_cgroup.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 8dfdc60..39e30ad 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -288,8 +288,11 @@ int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo) if (virLXCCgroupGetMemUsage(cgroup, meminfo) 0) goto cleanup; -virLXCCgroupGetMemSwapTotal(cgroup, meminfo); -virLXCCgroupGetMemSwapUsage(cgroup, meminfo); +if (virLXCCgroupGetMemSwapTotal(cgroup, meminfo) 0) +goto cleanup; + +if (virLXCCgroupGetMemSwapUsage(cgroup, meminfo) 0) +goto cleanup; ret = 0; cleanup: -- 1.9.0 ACK Pushed. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] Support for per-guest-node binding
On Thu, Jun 26, 2014 at 01:50:22AM +, chen.fan.f...@cn.fujitsu.com wrote: On Wed, 2014-06-04 at 16:56 +0200, Martin Kletzander wrote: Currently we are only able to bind the whole domain to some host nodes using the /domain/numatune/memory element. Numerous requests were made to support host-guest numa node bindings, so this series tries to pinch an idea on how to do that using /domain/numatune/memnode elements. That is incompatible with automatic numa placement (numad) since that makes no sense. Also this disables any live changes to numa parameters (the /domain/numatune/memory settings) since we cannot change the settings given to qemu. Hi Martin, Sorry for that I have not observed this patch. I made a duplicated work about this recently. and I found this patch has not been updated for several days, but since the QEMU have extra supported memory-file and some flags/properties, this patches should be refactored. Do you plan to send a new version ? If not, Can I take over them? I'm completely re-factoring the numatune parsing code and reworking few other things for this patch. For memory-file, that will be automatically supported as well, but with Michal's patches. We already have an option that says use hugepages and we would like to re-use that instead of creating new device(s). But we will greatly value your input on these patches (both mine and Michal's) when these hit the list. So if there's something else you find missing or wrong, that should be added or fixed, let me know. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: link virstoragetest with libxml
On Mon, Jun 23, 2014 at 04:08:42PM +0200, Martin Kletzander wrote: Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: To be honest, I have no idea why this fails for me in one situation, but it prevents the following error during compilation: /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.0/../../../../x86_64-pc-linux-gnu/bin/ld: ../src/.libs/libvirt_driver_storage_impl.a(libvirt_driver_storage_impl_la-storage_backend.o): undefined reference to symbol 'xmlFreeDoc@@LIBXML2_2.4.30' /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.0/../../../../lib64/libxml2.so: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status Makefile:4228: recipe for target 'virstoragetest' failed Therefore I'm not pushing it as a build-breaker since this might not be the root cause or the best solution. The other fix (and probably more appropriate one) would be to add LIBXML_LIBS into libvirt_conf_la_LIBADD since the xmlFreeDoc() is called in storage_conf.c. Any other preferred way is accepted as well, feel free to comment. tests/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index 025b847..457eb99 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -899,6 +899,7 @@ virstoragetest_LDADD = $(LDADDS) \ ../src/libvirt_util.la \ ../src/libvirt_driver_storage_impl.la \ ../gnulib/lib/libgnu.la \ + $(LIBXML_LIBS) \ $(NULL) viridentitytest_SOURCES = \ -- 2.0.0 Ping? signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.2.6
On Thu, Jun 26, 2014 at 11:05:55AM +0800, Daniel Veillard wrote: As stated yesterday, we just entered freeze for libvirt-1.2.6, I have pushed a first release candidate tarball and signed rpms at the usual place: ftp://libvirt.org/libvirt/ I gave it a try and with my limited testing it looks okay, but please have a look and test. The goal is to make the release on Tues 1st if all goes well. I was wondering what's the status of the translations (wrt the problem with gettext-0.19) [1]? Apart from that (because the fix is pushed) two build problems that I have right now are: Building virstoragetest with Gentoo's package manager fails, but looking at the code, it probably should, so I've sent a patch [2], although it may just be my setup causing this. Python bindings can't be built, but that's just because virNetwork*DHCPLease* are not covered. [1] https://www.redhat.com/archives/libvir-list/2014-June/msg00160.html [2] https://www.redhat.com/archives/libvir-list/2014-June/msg01060.html Also of note I will likely move the libvirt.org server Monday morning (chinese time i.e. sunday for most), it should be quick, just that if you hit the old IP untim DNS propagates you may have the old server (I will block ssh access after the move on the old to avoid split pushes) Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Initial implementation of new job control api
On Wed, Jun 18, 2014 at 05:59:47PM -0400, Tucker DiNapoli wrote:
This is my initial definition of a new internal job control api. I am
working on this as a part of the google summer of code. These patches
contain the core job control api and deal only with managing individual
jobs. I am currently working on writing code using this api to manage
jobs in domains, in such a way that I will be able to replace the
current job control code in qemu and libxl. Ultimately I will use this
to implement job control in the storage driver which is my ultimate
goal for the summer of code.
---
src/Makefile.am | 1 +
src/util/virjobcontrol.c | 574 +++
src/util/virjobcontrol.h | 342
3 files changed, 917 insertions(+)
create mode 100644 src/util/virjobcontrol.c
create mode 100644 src/util/virjobcontrol.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 2b9ac61..77de0e7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -118,6 +118,7 @@ UTIL_SOURCES =
\
util/virinitctl.c util/virinitctl.h \
util/viriptables.c util/viriptables.h \
util/viriscsi.c util/viriscsi.h \
+ util/virjobcontrol.h util/virjobcontrol.c \
util/virjson.c util/virjson.h \
util/virkeycode.c util/virkeycode.h \
util/virkeyfile.c util/virkeyfile.h \
diff --git a/src/util/virjobcontrol.c b/src/util/virjobcontrol.c
new file mode 100644
index 000..04a5246
--- /dev/null
+++ b/src/util/virjobcontrol.c
@@ -0,0 +1,574 @@
+/*
+ * virjobcontrol.c Core implementation of job control
+ *
+ * Copyright (C) 2014 Tucker DiNapoli
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Tucker DiNapoli
+ */
+
+#include config.h
+
+#include virjobcontrol.h
+#include viralloc.h
+#include virtime.h
+#include virlog.h
+VIR_LOG_INIT(virjobcontrol);
+
+VIR_ENUM_IMPL(virJob, 4,
You should use VIR_JOB_LAST for easy extension and proper size
checking.
I've just now discovered that running make syntax-check would tell you
the same.
+ none,
+ query,
+ modify,
+ destroy,
+);
These are probably general enough, yet.
+/*
+ No files other then this and virjobcontrol.c should need to
+ have access to the core implmentation of jobs. The code in these
+ files is intended to serve as a base for job control independent of
+ drivers.
+*/
+
+#define LOCK_JOB(job) \
+virMutexLock(job-lock)
+#define UNLOCK_JOB(job) \
+virMutexUnlock(job-lock)
+#define LOCK_JOB_INFO(job) \
+virMutexLock(job-info-lock)
+#define UNLOCK_JOB_INFO(job)\
+virMutexUnlock(job-info-lock)
We prefer having these in a separate functions. Not only can you
trace them better when debugging (for such purposes I use
CFLAGS=-ggdb -O0), but it also shouldn't add too much of an overhead
with proper compiler optimizations.
+#define GET_CURRENT_TIME(time) \
+if (virTimeMillisNow(time) 0) { \
+return -1; \
+}
+
This creates code with two flaws. One is readability, because it can
return from function (skipping possible clean-ups, e.g. in future
code) even though it's not obvious from the name. Second one is that
there is no need to have a semicolon after the macro (which confuses
some editors). The usual workaround is:
#define ASDF() \
do {\
asdf(); \
while (0)
+
+#define CHECK_FLAG_ATOMIC(job, flag) (virAtomicIntGet(job-flags)
VIR_JOB_FLAG_##flag)
+#define CHECK_FLAG(job, flag) (job-flags VIR_JOB_FLAG_##flag)
+#define SET_FLAG_ATOMIC(job, flag) (virAtomicIntOr(job-flags,
VIR_JOB_FLAG_##flag))
+#define SET_FLAG(job, flag) (job-flags |= VIR_JOB_FLAG_##flag)
+#define UNSET_FLAG_ATOMIC(job, flag) (virAtomicIntAnd(job-flags,
(~VIR_JOB_FLAG_##flag)))
+#define UNSET_FLAG(job, flag) (job-flags = (~VIR_JOB_FLAG_##flag))
+#define CLEAR_FLAGS_ATOMIC(job) (virAtomicIntSet(job-flags,
VIR_JOB_FLAG_NONE))
+#define CLEAR_FLAGS(job) (job-flags = VIR_JOB_FLAG_NONE)
+
While the resulting code looks
[libvirt] [PATCH] qemu: don't label anything before locking the domain
If locking the domain failed, files were already labelled and thus we
restored the previous label on them. Having disks on NFS means the
domain having the lock already gets permission denial.
This code moves the labelling part into the command hook since it's
still privileged, and also moves the clearing of
VIR_QEMU_PROCESS_STOP_NO_RELABEL from stop_flags right after the
handshare after hook.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1113327
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_process.c | 69 -
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5b598be..bc751b9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2700,6 +2700,8 @@ struct qemuProcessHookData {
virQEMUDriverPtr driver;
virBitmapPtr nodemask;
virQEMUDriverConfigPtr cfg;
+const char *stdin_path;
+int stdin_fd;
};
static int qemuProcessHook(void *data)
@@ -2739,6 +2741,34 @@ static int qemuProcessHook(void *data)
if (virNumaSetupMemoryPolicy(h-vm-def-numatune, h-nodemask) 0)
goto cleanup;
+/*
+ * Only after we managed to get a domain lock we can label
+ * domain-related objects.
+ */
+VIR_DEBUG(Setting domain security labels);
+if (virSecurityManagerSetAllLabel(h-driver-securityManager,
+ h-vm-def, h-stdin_path) 0)
+goto cleanup;
+
+if (h-stdin_fd != -1) {
+/* if there's an fd to migrate from, and it's a pipe, put the
+ * proper security label on it
+ */
+struct stat stdin_sb;
+
+VIR_DEBUG(setting security label on pipe used for migration);
+
+if (fstat(h-stdin_fd, stdin_sb) 0) {
+virReportSystemError(errno,
+ _(cannot stat fd %d), h-stdin_fd);
+goto cleanup;
+}
+if (S_ISFIFO(stdin_sb.st_mode)
+virSecurityManagerSetImageFDLabel(h-driver-securityManager,
+ h-vm-def, h-stdin_fd) 0)
+goto cleanup;
+}
+
ret = 0;
cleanup:
@@ -3702,6 +3732,8 @@ int qemuProcessStart(virConnectPtr conn,
hookData.driver = driver;
/* We don't increase cfg's reference counter here. */
hookData.cfg = cfg;
+hookData.stdin_path = stdin_path;
+hookData.stdin_fd = stdin_fd;
VIR_DEBUG(Beginning VM startup process);
@@ -4082,6 +4114,12 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
}
+/* Security manager labeled all devices, therefore
+ * if any operation from now on fails and we goto cleanup,
+ * where virSecurityManagerRestoreAllLabel() is called
+ * (hidden under qemuProcessStop) we need to restore labels. */
+stop_flags = ~VIR_QEMU_PROCESS_STOP_NO_RELABEL;
+
VIR_DEBUG(Setting up domain cgroup (if required));
if (qemuSetupCgroup(driver, vm, nodemask) 0)
goto cleanup;
@@ -4092,36 +4130,7 @@ int qemuProcessStart(virConnectPtr conn,
qemuProcessInitCpuAffinity(driver, vm, nodemask) 0)
goto cleanup;
-VIR_DEBUG(Setting domain security labels);
-if (virSecurityManagerSetAllLabel(driver-securityManager,
- vm-def, stdin_path) 0)
-goto cleanup;
-
-/* Security manager labeled all devices, therefore
- * if any operation from now on fails and we goto cleanup,
- * where virSecurityManagerRestoreAllLabel() is called
- * (hidden under qemuProcessStop) we need to restore labels. */
-stop_flags = ~VIR_QEMU_PROCESS_STOP_NO_RELABEL;
-
-if (stdin_fd != -1) {
-/* if there's an fd to migrate from, and it's a pipe, put the
- * proper security label on it
- */
-struct stat stdin_sb;
-
-VIR_DEBUG(setting security label on pipe used for migration);
-
-if (fstat(stdin_fd, stdin_sb) 0) {
-virReportSystemError(errno,
- _(cannot stat fd %d), stdin_fd);
-goto cleanup;
-}
-if (S_ISFIFO(stdin_sb.st_mode)
-virSecurityManagerSetImageFDLabel(driver-securityManager,
vm-def, stdin_fd) 0)
-goto cleanup;
-}
-
-VIR_DEBUG(Labelling done, completing handshake to child);
+VIR_DEBUG(Affinity/cgroups set, completing handshake to child);
if (virCommandHandshakeNotify(cmd) 0) {
goto cleanup;
}
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't label anything before locking the domain
On Thu, Jun 26, 2014 at 12:42:52PM +0100, Daniel P. Berrange wrote: On Thu, Jun 26, 2014 at 01:20:02PM +0200, Martin Kletzander wrote: If locking the domain failed, files were already labelled and thus we restored the previous label on them. Having disks on NFS means the domain having the lock already gets permission denial. This code moves the labelling part into the command hook since it's still privileged, and also moves the clearing of VIR_QEMU_PROCESS_STOP_NO_RELABEL from stop_flags right after the handshare after hook. This problem description / fix doesn't make much sense to me. IIUC the control flow is - Parent runs fork() - Parent waits for handshake notify - Child runs hook - Hook *only* registers with lock daemon - Child sends handshake notify to parent - Child waits for handshake response - Parent received handshake notify - Parent does labelling - Parent sends handshake response - Child execs QEMU - QEMU launches but CPUs are paused - Parent acquires disk locks - Parent tells QEMU to start CPUs Note that the hook does not acquire any locks - it merely connects to the lock daemon. Locks are not acquired until the CPUs are ready to be started. So I don't see how moving labelling into the hook solves anything. Oh, my fault, I haven't realized, we're just registering there. Note that the goal of the locking code as it is today, was only to prevent the content of the disk image being corrupted by 2 QEMUs running concurrently. The design as it is succeeds in this. Stopping changes to the labelling was not attempted. Yes, this will result in a running QEMU loosing access to a disk if another QEMU attempts to start and use those disks, but the content is protected in this way. It isn't actually possible to protect against concurrent changes to both the content and the labelling with a single lock because there are differing lock ordering protection rules requires for these. To do that, we actually need to incorporate use of the lock manager into the security drivers using a separate lock space and use locking rules that apply explicitly to the needs of the labelling. It occurred to me too that this might be either fixed or the fix eased after Michal's patches are applied (not my area, though): http://www.redhat.com/archives/libvir-list/2014-March/msg00826.html What I think is that it would (almost) solve it automatically, since it would restore the original label, even though there would be a small window when the first QEMU process doesn't have access to the disk. But definitely better result than now. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix guestfwd chardev option back how it was
Since commit d86c876a66e320b55220d00113027c9ad6199cff we are using
guestfwd=tcp:IP:PORT,chardev=ID for guestfwd specification, however,
that has not changed in qemu, so guestfwd does not work since.
Apart from that, guestfwd is not working with older qemu that doesn't
have QEMU_CAPS_DEVICE.
Both regressions exist since late 2009 and nobody found that (until
now), so I'm only fixing the first one.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1112066
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 93d303e..5074aa1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9186,7 +9186,7 @@ qemuBuildChannelChrDeviceStr(char **deviceStr,
port = virSocketAddrGetPort(chr-target.addr);
if (virAsprintf(deviceStr,
-user,guestfwd=tcp:%s:%i,chardev=char%s,id=user-%s,
+user,guestfwd=tcp:%s:%i-chardev:char%s,id=user-%s,
addr, port, chr-info.alias, chr-info.alias) 0) {
virReportOOMError();
goto cleanup;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args
b/tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args
index 7a15369..eb13430 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args
@@ -4,5 +4,5 @@ pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev
socket,\
id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,\
id=monitor,mode=readline -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1
-chardev \
pipe,id=charchannel0,path=/tmp/guestfwd -netdev user,\
-guestfwd=tcp:10.0.2.1:4600,chardev=charchannel0,id=user-channel0 -device \
+guestfwd=tcp:10.0.2.1:4600-chardev:charchannel0,id=user-channel0 -device \
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix guestfwd chardev option back how it was
On Thu, Jun 26, 2014 at 04:48:23PM +0200, Ján Tomko wrote:
On 06/26/2014 04:36 PM, Martin Kletzander wrote:
Since commit d86c876a66e320b55220d00113027c9ad6199cff we are using
guestfwd=tcp:IP:PORT,chardev=ID for guestfwd specification, however,
that has not changed in qemu, so guestfwd does not work since.
Apart from that, guestfwd is not working with older qemu that doesn't
have QEMU_CAPS_DEVICE.
Both regressions exist since late 2009 and nobody found that (until
now), so I'm only fixing the first one.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1112066
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
ACK
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 93d303e..5074aa1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9186,7 +9186,7 @@ qemuBuildChannelChrDeviceStr(char **deviceStr,
port = virSocketAddrGetPort(chr-target.addr);
if (virAsprintf(deviceStr,
-user,guestfwd=tcp:%s:%i,chardev=char%s,id=user-%s,
+user,guestfwd=tcp:%s:%i-chardev:char%s,id=user-%s,
addr, port, chr-info.alias, chr-info.alias) 0) {
virReportOOMError();
The OOM error is redundant here and right above it in
qemuBuildParallelChrDeviceStr.
And few other places all over the code as well. Since this is
pre-existing and not related to this code I won't change it in this
patch, but cleanup for more of these would be nice.
'git grep virReportOOMError src/ tests/' reports 273 matching lines
and I'm _pretty_ certain we don't have that many allocation functions.
Thanks, pushed.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] memdev device: add share argument for type=file
On Fri, Jun 27, 2014 at 10:53:10PM +0200, Michele Paolino wrote: This patch enables the possibility to run a qemu virtual machine with the share option for the memory-backend-file. The xml description looks like: memdev type='file' share='on' nameram0/name source mem-path='/hugetlbfs'/ capacity unit='MiB'1024/capacity /memdev This work is based on the previous work of Chen Fan[1]. We are aware of the existing conflict with the previous numa patches[2]. We are sharing this because it is a dependency for some use cases of the qemu vhost-user support(e.g. snabbswitch). There is one important thing left out. What qemu uses as a memdev should not be exposed as such (a device) in the XML, that way the definition makes sense only for people aware of how qemu works, but we are trying to keep the XML as hypervisor-independent as possible. Of course only where applicable, many places must have hypervisor-dependent structures, but I don't think memory device is one of these exceptions. All the options that qemu aspose as memdev options should be abstracted in the XML. For example3, we already have an element in the XML that says use hugepages as a backing for memory and that element should be used for that. I just skimmed the patches a bit, but it looks like none of the series works with that information or changes the behaviour based on that. Martin [1] http://www.redhat.com/archives/libvir-list/2014-June/msg01195.html [2] http://www.redhat.com/archives/libvir-list/2014-June/msg00201.html Michele Paolino (2): Add share argument to memdev devices(type=file) Documentation and test for the share argument in memdev device docs/formatdomain.html.in | 7 +++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 16 src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c| 3 +++ tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa4.xml | 2 +- 7 files changed, 32 insertions(+), 2 deletions(-) -- 1.9.3 signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Track an error
On Sat, Jun 28, 2014 at 08:52:32AM +0300, David kiarie wrote: Hi there, Just a silly question, do we have a simpler way of checking where exactly a test is failing.The tests are failing after hacking stuff around. I have this error ../build-aux/test-driver: line 95: 20128 Segmentation fault $@ $log_file 21 Yes am having a segfault! I don't know how to track it, well, I could review the whole file but is there some other method I am missing.Could you help me interpret the error, what does 20128 stand for? I believe that's the pid, best way is changing into the tests directory and running ../run gdb test, where test is the test that's segfaulting. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: raise an error when trying to use readonly ide disks
On Mon, Jun 30, 2014 at 12:05:06PM +0200, Giuseppe Scrivano wrote:
The IDE bus doesn't support readonly disks, so inform the user with an
error message instead of let qemu fail with a more obscure Device
'ide-hd' could not be initialized error message.
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1112939
Signed-off-by: Giuseppe Scrivano gscri...@redhat.com
---
src/qemu/qemu_command.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 63f322a..4829176 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3385,8 +3385,15 @@ qemuBuildDriveStr(virConnectPtr conn,
disk-bus != VIR_DOMAIN_DISK_BUS_IDE)
virBufferAddLit(opt, ,boot=on);
if (disk-readonly
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY))
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) {
+if (disk-bus == VIR_DOMAIN_DISK_BUS_IDE
+disk-device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(readonly ide disks are not supported));
+goto error;
+}
virBufferAddLit(opt, ,readonly=on);
+}
I'm not very sure we should do that. Second opinion would be great.
My point is that if qemu does not support the readonly flag, we just
skip setting it, but if it supports it, we will error out? OTOH
skipping the readonly=on when user requests it is, ehm, not nice
either. I think Eric was saying that *some* readonly flags do not
make much of a sense, but that was probably WRT backing chains or
something like that. Eric?
Martin
if (disk-transient) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(transient disks not supported yet));
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [for 1.2.6 PATCH] qemu: snapshot: Save persistent domain config when taking external snapshot
On Mon, Jun 30, 2014 at 01:54:09PM +0200, Peter Krempa wrote:
Commit 55bbb011b965c7962933604c70f61cef45e8ec04 introduced a regression
where we forgot to save the persistent domain configuration after a
s/after a/after an/
external snapshot. This would make libvirt forget the snapshots and
effectively revert to the previous state in the following scenario:
1) Start VM
2) Take snapshot
3) Destroy VM
4) Restart libvirtd
Also fix spurious blank line added by patch mentioned above.
---
ACK,
Martin
src/qemu/qemu_driver.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d34da6f..775f6ab 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13126,8 +13126,10 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr
driver,
int indx = virDomainDiskIndexByName(vm-newDef,
vm-def-disks[i]-dst,
false);
-if (indx = 0)
+if (indx = 0) {
persistDisk = vm-newDef-disks[indx];
+persist = true;
+}
}
ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm,
@@ -13173,7 +13175,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr
driver,
persistDisk = vm-newDef-disks[indx];
persist = true;
}
-
}
qemuDomainSnapshotUndoSingleDiskActive(driver, vm,
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [python PATCH] maint: document development against uninstalled libvirt
On Wed, Jun 25, 2014 at 11:12:52AM -0600, Eric Blake wrote: On 06/25/2014 10:59 AM, Peter Krempa wrote: On 06/25/14 18:53, Eric Blake wrote: Thanks to Dan's recent work in libvirt.git, it is much easier to develop against uninstalled libvirt. Mention how. * README: More details. Signed-off-by: Eric Blake ebl...@redhat.com --- README | 15 +++ 1 file changed, 15 insertions(+) ACK, Thanks; pushed. FYI: I'm also using this (but considered it hackish enough to not publish as a formal patch): $ echo /GNUmakefile .git/info/exclude $ cat GNUmakefile # My hidden wrapper to preset things that I like... include Makefile export LD_LIBRARY_PATH=/home/eblake/libvirt/src/.libs/ export PKG_CONFIG_PATH=/home/eblake/libvirt/src/ $ For people like me, who use 'git clean -fxd'; it removes such excludes. :( How about either adding similar GNUmakefile, of modifying the Makefile you added into the repo liike this The path used for the exports would be something like: $(pwd | sed -e 's_/libvirt\(\.git\)\?/_/libvirt-python\1/') but only if such path exists. There would be nothing changed for those who do not have such paths. It could be also hidden under some other make command (e.g. make devel). Does that make sense or is it not worth trying to do that at all? Martin Now all I have to do is run 'python setup.py build' to build against installed libvirt, vs. 'make' to build against development libvirt, at least for my machine. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [python PATCH] maint: document development against uninstalled libvirt
On Mon, Jun 30, 2014 at 02:05:39PM +0100, Daniel P. Berrange wrote: On Mon, Jun 30, 2014 at 02:53:04PM +0200, Martin Kletzander wrote: On Wed, Jun 25, 2014 at 11:12:52AM -0600, Eric Blake wrote: On 06/25/2014 10:59 AM, Peter Krempa wrote: On 06/25/14 18:53, Eric Blake wrote: Thanks to Dan's recent work in libvirt.git, it is much easier to develop against uninstalled libvirt. Mention how. * README: More details. Signed-off-by: Eric Blake ebl...@redhat.com --- README | 15 +++ 1 file changed, 15 insertions(+) ACK, Thanks; pushed. FYI: I'm also using this (but considered it hackish enough to not publish as a formal patch): $ echo /GNUmakefile .git/info/exclude $ cat GNUmakefile # My hidden wrapper to preset things that I like... include Makefile export LD_LIBRARY_PATH=/home/eblake/libvirt/src/.libs/ export PKG_CONFIG_PATH=/home/eblake/libvirt/src/ $ For people like me, who use 'git clean -fxd'; it removes such excludes. :( How about either adding similar GNUmakefile, of modifying the Makefile you added into the repo liike this The path used for the exports would be something like: $(pwd | sed -e 's_/libvirt\(\.git\)\?/_/libvirt-python\1/') but only if such path exists. There would be nothing changed for those who do not have such paths. It could be also hidden under some other make command (e.g. make devel). Does that make sense or is it not worth trying to do that at all? IMHO this GNUmakefile Eric suggests is just duplicating what the 'run' script already does. eg if you have libvirt libvirt-python checked out at the same place, you can just do '../libvirt/run make' or '../libvirt run python setup.py build' Yep, you can, but if you don't (and have some subdirs for example), than might help, someone. I'm doing ~lv/run make, so that was not for me anyway ;) Martin Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: raise an error when trying to use readonly ide disks
On Mon, Jun 30, 2014 at 02:28:54PM +0100, Daniel P. Berrange wrote:
On Mon, Jun 30, 2014 at 03:23:40PM +0200, Giuseppe Scrivano wrote:
Martin Kletzander mklet...@redhat.com writes:
Q On Mon, Jun 30, 2014 at 12:05:06PM +0200, Giuseppe Scrivano wrote:
The IDE bus doesn't support readonly disks, so inform the user with an
error message instead of let qemu fail with a more obscure Device
'ide-hd' could not be initialized error message.
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1112939
Signed-off-by: Giuseppe Scrivano gscri...@redhat.com
---
src/qemu/qemu_command.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 63f322a..4829176 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3385,8 +3385,15 @@ qemuBuildDriveStr(virConnectPtr conn,
disk-bus != VIR_DOMAIN_DISK_BUS_IDE)
virBufferAddLit(opt, ,boot=on);
if (disk-readonly
-virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY))
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) {
+if (disk-bus == VIR_DOMAIN_DISK_BUS_IDE
+disk-device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(readonly ide disks are not supported));
+goto error;
+}
virBufferAddLit(opt, ,readonly=on);
+}
I'm not very sure we should do that. Second opinion would be great.
My point is that if qemu does not support the readonly flag, we just
skip setting it, but if it supports it, we will error out? OTOH
skipping the readonly=on when user requests it is, ehm, not nice
either. I think Eric was saying that *some* readonly flags do not
make much of a sense, but that was probably WRT backing chains or
something like that. Eric?
I don't think that this error is related to the presence of the
DRIVE_READONLY Qemu caps. My understanding of this comment on the Qemu
bug conterpart (https://bugzilla.redhat.com/show_bug.cgi?id=915162#c1) is
that this is not going to change as readonly disks are not supported by
the IDE bus, and this can be considered just an early detection of this
situation.
Yep, that matches my understanding - IDE hardware simply doesn't have a
concept of a readonly hard disk.
The libvirt readonly/ flag does two things
- Causes SELinux to make the file readonly
- Passes readonly=on to QEMU
If we silently skipped adding readonly=on, then libvirt would still tell
SELinux to make the file readonly, and then when the guest tried to issue
a write request to the disk, it would get an I/O error. Some might argue
that they want this behaviour, but I think erroring out by default is
probably better. If people really want a read-write disk in the guest
with readonly SELinux labelling, the mgmt app can always override the
seclabel for that disk to set a readonly selinux label.
IOW, ACK
If the user updates from QEMU without DRIVE_READONLY to newer one,
that supports that flag, than XML with readonly IDE HDD will stop
working even though it worked before the update *as requested*. That
readonly flag does not reflect how the disk is exposed in the guest;
as you said IDE does not have any readonly concept, that is only how
the device reacts to writes.
Changing the behaviour to:
if (disk-readonly
virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)
!(disk-bus == VIR_DOMAIN_DISK_BUS_IDE
disk-device == VIR_DOMAIN_DISK_DEVICE_DISK))
virBufferAddLit(opt, ,readonly=on);
would keep the backward compatibility. This behaviour makes more
sense to me.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: raise an error when trying to use readonly ide disks
On Tue, Jul 01, 2014 at 09:40:00AM +0200, Giuseppe Scrivano wrote: Martin Kletzander mklet...@redhat.com writes: If the user updates from QEMU without DRIVE_READONLY to newer one, that supports that flag, than XML with readonly IDE HDD will stop working even though it worked before the update *as requested*. That readonly flag does not reflect how the disk is exposed in the guest; as you said IDE does not have any readonly concept, that is only how the device reacts to writes. Changing the behaviour to: if (disk-readonly virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY) !(disk-bus == VIR_DOMAIN_DISK_BUS_IDE disk-device == VIR_DOMAIN_DISK_DEVICE_DISK)) virBufferAddLit(opt, ,readonly=on); would keep the backward compatibility. This behaviour makes more sense to me. this behaves in a quite different way that my proposed patch but if readonly/ affects also the SELinux label and we allow the process to run anyway by skipping readonly=on for IDE disks, wouldn't qemu fail in weird ways when trying to write to the file? My case was that it doesn't fail in a new way if you update from older qemu that didn't support the DRIVE_READONLY capability. However it would probably fail differently when there is readonly=on because it would not report an error in the guest. Anyway, thinking about it, qemu doesn't start for some versions of libvirt already with these setting, this just changes the error to a more readable one. So given the circumstances such backward compatibility is not such huge issue (since we already broke it). I, therefore, agree with Dan's ACK. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libnuma build failure [was: securityselinuxlabeltest test fails on v1.2.5]
On Mon, Jun 30, 2014 at 03:26:07PM -0600, Eric Blake wrote: On 06/30/2014 01:46 PM, Scott Sullivan wrote: I've tested the v1.2.6-rc2 git tag, im getting this build error: CC util/libvirt_util_la-virnuma.lo util/virnuma.c: In function 'virNumaNodeIsAvailable': util/virnuma.c:428: error: 'numa_nodes_ptr' undeclared (first use in this function) util/virnuma.c:428: error: (Each undeclared identifier is reported only once util/virnuma.c:428: error: for each function it appears in.) What version of numactl-devel do you have installed? make[3]: *** [util/libvirt_util_la-virnuma.lo] Error 1 make[3]: Leaving directory `/home/rpmbuild/packages/libvirt/src' make[2]: *** [all] Error 2 make[2]: Leaving directory `/home/rpmbuild/packages/libvirt/src' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/rpmbuild/packages/libvirt' make: *** [all] Error 2 error: Bad exit status from /var/tmp/rpm-tmp.3Gu7nd (%build) Build works fine on tag v1.2.5-maint. Sounds like we need to make code added in the meantime be conditional to compile to older numa libraries. Guess we have to check for numa_nodes_ptr in the conditional for HAVE_NUMA_BITMASK_ISBITSET, but that's definitely some odd version of numactl. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 1.2.6] vboxsnapshotxmltest: Don't write to a file in abs_srcdir
On Tue, Jul 01, 2014 at 03:28:49PM +0200, Michal Privoznik wrote:
In the test, the snapshot XML is written into a file that's located
under:
abs_srcdir/vboxsnapshotxmldata/testResult.vbox
However, the abs_srcdir doesn't have to be necessarily writable. It
should have been abs_builddir instead. Moreover, the label in the func
creating the file is called 'fail' while it fulfils the duty of
'cleanup' label.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/vboxsnapshotxmltest.c | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/tests/vboxsnapshotxmltest.c b/tests/vboxsnapshotxmltest.c
index 7795688..85ef96a 100644
--- a/tests/vboxsnapshotxmltest.c
+++ b/tests/vboxsnapshotxmltest.c
@@ -59,42 +59,47 @@ testCompareXMLtoXMLFiles(const char *xml)
char *pathResult = NULL;
int ret = -1;
virVBoxSnapshotConfMachinePtr machine = NULL;
-if (virAsprintf(pathResult, %s/vboxsnapshotxmldata/testResult.vbox,
-abs_srcdir) 0)
+
+if (VIR_STRDUP(pathResult,
+ abs_builddir /vboxsnapshotxmldata/testResult.vbox) 0)
return -1;
+if (virFileMakePath(abs_builddir /vboxsnapshotxmldata) 0)
+goto cleanup;
+
if (virtTestLoadFile(xml, xmlData) 0)
-goto fail;
+goto cleanup;
if (!(machine = virVBoxSnapshotConfLoadVboxFile(xml, (char*
-goto fail;
+goto cleanup;
if (virVBoxSnapshotConfSaveVboxFile(machine, pathResult) 0)
-goto fail;
+goto cleanup;
if (virtTestLoadFile(pathResult, actual) 0)
-goto fail;
+goto cleanup;
if (unlink(pathResult) 0)
-goto fail;
+goto cleanup;
This unlink should be done in the clean-up as well, otherwise rmdir()
won't remove the directory in case the test fails for example in
virtTestLoadFile().
ACK with that changed.
Martin
if (!(actual = testFilterXML(actual)))
-goto fail;
+goto cleanup;
if (!(xmlData = testFilterXML(xmlData)))
-goto fail;
+goto cleanup;
if (STRNEQ(actual, xmlData)) {
virtTestDifference(stderr, xmlData, actual);
-goto fail;
+goto cleanup;
}
ret = 0;
- fail:
+ cleanup:
VIR_FREE(xmlData);
VIR_FREE(actual);
virVBoxSnapshotConfMachineFree(machine);
VIR_FREE(pathResult);
+rmdir(abs_builddir /vboxsnapshotxmldata);
return ret;
}
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 for 1.2.6] build: link libvirt_conf with libxml
Since there is code using functions from the libxml library, libvirt_conf should have that in LIBADD so it can be linked against even without libvirt_util (which usually deals with the error itself, since libvirt_util has libxml in LIBADD). The same applies to storage_backend.c. Signed-off-by: Martin Kletzander mklet...@redhat.com --- This is a v2 of: https://www.redhat.com/archives/libvir-list/2014-June/msg01322.html If this gets ACKed for th 1.2.6 release, please push this as well since I won't be available before DV makes the release, thank you. --- src/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 35720be..047d4c6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -991,6 +991,7 @@ libvirt_la_BUILT_LIBADD += libvirt_conf.la libvirt_conf_la_SOURCES = $(CONF_SOURCES) libvirt_conf_la_CFLAGS = $(AM_CFLAGS) libvirt_conf_la_LDFLAGS = $(AM_LDFLAGS) +libvirt_conf_la_LIBADD = $(LIBXML_LIBS) noinst_LTLIBRARIES += libvirt_cpu.la libvirt_la_BUILT_LIBADD += libvirt_cpu.la @@ -1452,7 +1453,7 @@ libvirt_driver_storage_impl_la_CFLAGS = \ $(AM_CFLAGS) libvirt_driver_storage_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_storage_impl_la_LIBADD = -libvirt_driver_storage_impl_la_LIBADD += $(SECDRIVER_LIBS) +libvirt_driver_storage_impl_la_LIBADD += $(SECDRIVER_LIBS) $(LIBXML_LIBS) if WITH_BLKID libvirt_driver_storage_impl_la_CFLAGS += $(BLKID_CFLAGS) libvirt_driver_storage_impl_la_LIBADD += $(BLKID_LIBS) -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: unify extra asterisk in viralloc.h
Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: Pushed as trivial. src/util/viralloc.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 55372e5..7125e67 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -1,7 +1,7 @@ /* * viralloc.h: safer memory allocation * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -279,7 +279,7 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); # define VIR_SHRINK_N(ptr, count, remove) \ virShrinkN((ptr), sizeof(*(ptr)), (count), remove) -/* +/** * VIR_TYPEMATCH: * * The following macro seems a bit cryptic, so it needs a thorough @@ -481,7 +481,7 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); # define VIR_DELETE_ELEMENT_INPLACE(ptr, at, count) \ virDeleteElementsN((ptr), sizeof(*(ptr)), at, (count), 1, true) -/* +/** * VIR_ALLOC_VAR_OVERSIZED: * @M: size of base structure * @N: number of array elements in trailing array -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] wireshark: Include more of libvirt internals
On Fri, Jul 04, 2014 at 10:52:19AM +0200, Michal Privoznik wrote:
The rationale is to not duplicate code which is done in
packet-libvirt.h for instance. Moreover, this way we can drop
__attribute_((unused)) used int packet-libvirt.c in favor of
ATTRIBUTE_UNUSED.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tools/wireshark/src/Makefile.am | 2 +-
tools/wireshark/src/packet-libvirt.c | 3 ++-
tools/wireshark/src/packet-libvirt.h | 18 ++
3 files changed, 5 insertions(+), 18 deletions(-)
diff --git a/tools/wireshark/src/Makefile.am b/tools/wireshark/src/Makefile.am
index 44f22be..40fe368 100644
--- a/tools/wireshark/src/Makefile.am
+++ b/tools/wireshark/src/Makefile.am
@@ -18,7 +18,7 @@
#
# Author: Yuto KAWAMURA(kawamuray)
-INCLUDES = -I$(top_srcdir)
+INCLUDES = -I$(top_srcdir) -I$(top_srcdir)/src -I$(top_srcdir)/gnulib/lib
you need to add includes/ as well if you want to do ...
ws_plugin_LTLIBRARIES = libvirt.la
libvirt_la_SOURCES= packet-libvirt.h packet-libvirt.c plugin.c
diff --git a/tools/wireshark/src/packet-libvirt.c
b/tools/wireshark/src/packet-libvirt.c
index 07098bf..5138453 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -34,6 +34,7 @@
#endif
#include rpc/xdr.h
#include packet-libvirt.h
+#include internal.h
static int proto_libvirt = -1;
static int hf_libvirt_length = -1;
@@ -413,7 +414,7 @@ dissect_libvirt_message(tvbuff_t *tvb, packet_info *pinfo,
proto_tree *tree)
}
static guint32
-get_message_len(packet_info *pinfo __attribute__((unused)), tvbuff_t *tvb, int
offset)
+get_message_len(packet_info *pinfo ATTRIBUTE_UNUSED, tvbuff_t *tvb, int offset)
{
return tvb_get_ntohl(tvb, offset);
}
diff --git a/tools/wireshark/src/packet-libvirt.h
b/tools/wireshark/src/packet-libvirt.h
index 0cab637..af54407 100644
--- a/tools/wireshark/src/packet-libvirt.h
+++ b/tools/wireshark/src/packet-libvirt.h
@@ -21,6 +21,8 @@
#ifndef _PACKET_LIBVIRT_H_
# define _PACKET_LIBVIRT_H_
+# include libvirt/libvirt.h
+
... this :)
# ifndef LIBVIRT_PORT
# define LIBVIRT_PORT 16509
# endif
@@ -84,22 +86,6 @@ static const value_string status_strings[] = {
{ -1, NULL }
};
-/* TODO: These symbols will automatically included in generated headers in the
feature */
-# define VIR_SECURITY_MODEL_BUFLEN (256 + 1)
-# define VIR_SECURITY_LABEL_BUFLEN (4096 + 1)
-# define VIR_SECURITY_DOI_BUFLEN (256 + 1)
-# define VIR_UUID_BUFLEN (16)
-enum {
-VIR_TYPED_PARAM_INT = 1, /* integer case */
-VIR_TYPED_PARAM_UINT= 2, /* unsigned integer case */
-VIR_TYPED_PARAM_LLONG = 3, /* long long case */
-VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case */
-VIR_TYPED_PARAM_DOUBLE = 5, /* double case */
-VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case */
-VIR_TYPED_PARAM_STRING = 7, /* string case */
-};
-/* / */
-
# define VIR_ERROR_MESSAGE_DISSECTOR dissect_xdr_remote_error
static gboolean dissect_xdr_int(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int
hf);
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Adapt to wireshark-1.12
On Fri, Jul 04, 2014 at 10:52:18AM +0200, Michal Privoznik wrote: With the new wireshark installed I've noticed couple of build breaks. This fixes the problems I've seen. Michal Privoznik (2): wireshark: Include more of libvirt internals wireshark: Honor API change coming with 1.12 release tools/wireshark/src/Makefile.am | 2 +- tools/wireshark/src/packet-libvirt.c| 35 +++-- tools/wireshark/src/packet-libvirt.h| 18 ++--- tools/wireshark/util/make-dissector-reg | 7 +++ 4 files changed, 43 insertions(+), 19 deletions(-) -- 1.8.5.5 We could probably create a new file where we would deal with the differences just to keep the code clean. We also proabbly don't build with any other wireshark versions (I only ever built with 1.10 and 1.12), so that is something someone might try inspecting. However this is enough and working, so it's better than nothing right now, so ACK series if you fix the include in 1/2. Let's hope this won't end up looking like the vbox driver ;) Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] docs: Fix broken link in the HACKING page
On Fri, Jul 04, 2014 at 04:12:47PM +0200, Michele Paolino wrote: The link to the page how to get your code into an open source project has been fixed. Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com --- docs/hacking.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 9456520..6a92f46 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in We also automatically generate the HACKING file from this, so these should be updated together, but since this is a minor thing, I'll update the commit when pushing. ACK, Martin @@ -260,7 +260,7 @@ p There is more on this subject, including lots of links to background reading on the subject, on - a href=http://et.redhat.com/~rjones/how-to-supply-code-to-open-source-projects/; + a href=http://people.redhat.com/rjones/how-to-supply-code-to-open-source-projects/; Richard Jones' guide to working with open source projects/a. /p -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] docs: hacking and formatdomain fixes
On Fri, Jul 04, 2014 at 04:12:46PM +0200, Michele Paolino wrote: replaced link in haking.html.in and fixed XML tags in formatdomain.html.in Michele Paolino (2): docs: Fix broken link in the HACKING page docs: formatdomain.html fixes docs/formatdomain.html.in | 8 docs/hacking.html.in | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) ACK pushed with the change in 1/2. Martin -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Free the return value of virFileFindResource
On Mon, Jul 07, 2014 at 08:33:37AM +0200, Ján Tomko wrote: Commits e18a80a and 57e5c3c switched from a getenv wrapper which does not allocate a string to virFileFindResource which does not, without freeing it. https://bugzilla.redhat.com/show_bug.cgi?id=1116427 --- src/locking/lock_driver_lockd.c | 2 ++ src/remote/remote_driver.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index c67bda6..1ca7772 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -273,11 +273,13 @@ static virNetClientPtr virLockManagerLockDaemonConnectionNew(bool privileged, if (virNetClientAddProgram(client, *prog) 0) goto error; +VIR_FREE(daemonPath); VIR_FREE(lockdpath); return client; error: +VIR_FREE(daemonPath); VIR_FREE(lockdpath); virNetClientClose(client); virObjectUnref(client); diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 88fc977..9d8120f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1068,6 +1068,9 @@ doRemoteOpen(virConnectPtr conn, VIR_FREE(pkipath); VIR_FREE(knownHostsVerify); VIR_FREE(knownHosts); +#ifndef WIN32 +VIR_FREE(daemonPath); +#endif Would this compile on windows without warning if this free was ran unconditionally and the conditional around its declaration was removed as well? ACK either way, Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] LXC: remove duplicate controller check code
On Mon, Jul 07, 2014 at 02:26:57PM +0800, Chen Hanxiao wrote:
We invoked virCgroupHasController twice for checking
VIR_CGROUP_CONTROLLER_DEVICES
in lxcDomainAttachDeviceDiskLive.
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
src/lxc/lxc_driver.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index fce16f2..9c006e9 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4052,12 +4052,6 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver,
goto cleanup;
}
-if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(devices cgroup isn't mounted));
-goto cleanup;
-}
-
perms = (def-readonly ?
VIR_CGROUP_DEVICE_READ :
VIR_CGROUP_DEVICE_RW) |
--
1.9.0
ACK Pushed.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: dump: Report better error when dumping VM with passthrough devices
On Mon, Jul 07, 2014 at 10:05:22AM +0200, Peter Krempa wrote: For the regular dump operation we migrate the VM to a file. This won't work when the VM has passthrough devices assigned. Rather than reporting a cryptic error from qemu run our check whether it can be migrated. This does not influence the memory-only dump that is allowed with passthrough devices. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=874418 --- src/qemu/qemu_driver.c | 4 1 file changed, 4 insertions(+) ACK, Martin diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2d1aa9e..fe76d55 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3481,6 +3481,10 @@ doCoreDump(virQEMUDriverPtr driver, memory-only dump)); goto cleanup; } + +if (!qemuMigrationIsAllowed(driver, vm, vm-def, false, false)) +goto cleanup; + ret = qemuMigrationToFile(driver, vm, fd, 0, path, qemuCompressProgramName(compress), false, QEMU_ASYNC_JOB_DUMP); -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] properly set video ram size for qemu VGA video device
On Mon, Jul 07, 2014 at 01:44:06PM +0200, Pavel Hrdina wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1076098
We support vram option for video devices in domain xml, but so far
only for QXL it had some effect. VGA video device in QEMU can also
accept the size of video ram and we should pass it.
Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
src/qemu/qemu_command.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fb64cda..09e29c9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4800,6 +4800,9 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def,
/* QEMU accepts bytes for vram_size. */
virBufferAsprintf(buf, ,vram_size=%u, video-vram * 1024);
+} else if (video-type == VIR_DOMAIN_VIDEO_TYPE_VGA) {
The patch [1] that adds this option also adds it for VMWare VGA, can
we use that too?
+/* QEMU accepts megabytes for vgamem_mb. */
+virBufferAsprintf(buf, ,vgamem_mb=%u, video-vram / 1024);
How will this behave with pre-1.1.0 qemu (without the commit mentioned
[1])? Will it (a) just skip the parameter or will it (b) error out?
In case of (b) this is a regression.
Martin
P.S.: Some tests in qemuxml2argv would go nicely with such change.
[1] 4a1e244eb65c646bdd938d9d137ace42d76c95a7
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 13/16] qemu: enable disjoint numa cpu ranges
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c| 26 ++--
.../qemuxml2argv-cpu-numa-disjoint.args| 6 +
.../qemuxml2argv-cpu-numa-disjoint.xml | 28 ++
tests/qemuxml2argvtest.c | 2 ++
tests/qemuxml2xmltest.c| 1 +
5 files changed, 51 insertions(+), 12 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 249ce8d..284664b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6366,11 +6366,13 @@ qemuBuildSmpArgStr(const virDomainDef *def,
}
static int
-qemuBuildNumaArgStr(const virDomainDef *def, virCommandPtr cmd)
+qemuBuildNumaArgStr(const virDomainDef *def,
+virCommandPtr cmd,
+virQEMUCapsPtr qemuCaps)
{
size_t i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
-char *cpumask = NULL;
+char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
int ret = -1;
for (i = 0; i def-cpu-ncells; i++) {
@@ -6382,7 +6384,8 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virCommandPtr cmd)
if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
goto cleanup;
-if (strchr(cpumask, ',')) {
+if (strchr(cpumask, ',')
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(disjoint NUMA cpu ranges are not supported
with this QEMU));
@@ -6391,15 +6394,14 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virCommandPtr cmd)
virCommandAddArg(cmd, -numa);
virBufferAsprintf(buf, node,nodeid=%zu, i);
-virBufferAddLit(buf, ,cpus=);
-/* Up through qemu 1.4, -numa does not accept a cpus
- * argument any more complex than start-stop.
- *
- * XXX For qemu 1.5, the syntax has not yet been decided;
- * but when it is, we need a capability bit and
- * translation of our cpumask into the qemu syntax. */
-virBufferAdd(buf, cpumask, -1);
+for (tmpmask = cpumask; tmpmask; tmpmask = next) {
+if ((next = strchr(tmpmask, ',')))
+*(next++) = '\0';
+virBufferAddLit(buf, ,cpus=);
+virBufferAdd(buf, tmpmask, -1);
+}
+
virBufferAsprintf(buf, ,mem=%d, cellmem);
virCommandAddArgBuffer(cmd, buf);
@@ -7230,7 +7232,7 @@ qemuBuildCommandLine(virConnectPtr conn,
VIR_FREE(smp);
if (def-cpu def-cpu-ncells)
-if (qemuBuildNumaArgStr(def, cmd) 0)
+if (qemuBuildNumaArgStr(def, cmd, qemuCaps) 0)
goto error;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_UUID))
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args
new file mode 100644
index 000..073e84b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args
@@ -0,0 +1,6 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc \
+-m 214 -smp 16 -numa node,nodeid=0,cpus=0-3,cpus=8-11,mem=107 \
+-numa node,nodeid=1,cpus=4-7,cpus=12-15,mem=107 -nographic -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -usb -net none -serial
none \
+-parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
new file mode 100644
index 000..474a238
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml
@@ -0,0 +1,28 @@
+domain type='qemu'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory unit='KiB'219100/memory
+ currentMemory unit='KiB'219100/currentMemory
+ vcpu placement='static'16/vcpu
+ os
+type arch='x86_64' machine='pc'hvm/type
+boot dev='network'/
+ /os
+ cpu
+topology sockets='2' cores='4' threads='2'/
+numa
+ cell id='0' cpus='0-3,8-11' memory='109550'/
+ cell id='1' cpus='4-7,12-15' memory='109550'/
+/numa
+ /cpu
+ clock offset='utc'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashdestroy/on_crash
+ devices
+emulator/usr/bin/qemu/emulator
+controller type='usb' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='virtio'/
+ /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index bb7f580..a2d27cb 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1178,6 +1178,8 @@ mymain(void)
DO_TEST(cpu-numa1, NONE);
DO_TEST(cpu-numa2, QEMU_CAPS_SMP_TOPOLOGY);
DO_TEST_PARSE_ERROR(cpu-numa3, NONE);
+DO_TEST_FAILURE(cpu-numa-disjoint
[libvirt] [PATCH v2 01/16] qemu: purely a code movement
to ease the review of commits to follow.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fb64cda..aed7411 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6374,12 +6374,24 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virCommandPtr cmd)
int ret = -1;
for (i = 0; i def-cpu-ncells; i++) {
+int cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024);
+def-cpu-cells[i].mem = cellmem * 1024;
+
VIR_FREE(cpumask);
+
+if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
+goto cleanup;
+
+if (strchr(cpumask, ',')) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(disjoint NUMA cpu ranges are not supported
+ with this QEMU));
+goto cleanup;
+}
+
virCommandAddArg(cmd, -numa);
virBufferAsprintf(buf, node,nodeid=%d, def-cpu-cells[i].cellid);
virBufferAddLit(buf, ,cpus=);
-if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
-goto cleanup;
/* Up through qemu 1.4, -numa does not accept a cpus
* argument any more complex than start-stop.
@@ -6387,16 +6399,8 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virCommandPtr cmd)
* XXX For qemu 1.5, the syntax has not yet been decided;
* but when it is, we need a capability bit and
* translation of our cpumask into the qemu syntax. */
-if (strchr(cpumask, ',')) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(disjoint NUMA cpu ranges are not supported
- with this QEMU));
-goto cleanup;
-}
virBufferAdd(buf, cpumask, -1);
-def-cpu-cells[i].mem = VIR_DIV_UP(def-cpu-cells[i].mem,
-1024) * 1024;
-virBufferAsprintf(buf, ,mem=%d, def-cpu-cells[i].mem / 1024);
+virBufferAsprintf(buf, ,mem=%d, cellmem);
if (virBufferCheckError(buf) 0)
goto cleanup;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 10/16] qemu: allow qmp probing for cmdline options without params
That can be lately achieved with by having .param == NULL in the
virQEMUCapsCommandLineProps struct.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 10 --
src/qemu/qemu_monitor.c | 6 --
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 8 +++-
src/qemu/qemu_monitor_json.h | 3 ++-
tests/qemumonitorjsontest.c | 20 +---
6 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index cbfc728..9c4ea87 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2428,6 +2428,7 @@ static int
virQEMUCapsProbeQMPCommandLine(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon)
{
+bool found = false;
int nvalues;
char **values;
size_t i, j;
@@ -2435,10 +2436,15 @@ virQEMUCapsProbeQMPCommandLine(virQEMUCapsPtr qemuCaps,
for (i = 0; i ARRAY_CARDINALITY(virQEMUCapsCommandLine); i++) {
if ((nvalues = qemuMonitorGetCommandLineOptionParameters(mon,
virQEMUCapsCommandLine[i].option,
- values)) 0)
+ values,
+ found)) 0)
return -1;
+
+if (found !virQEMUCapsCommandLine[i].param)
+virQEMUCapsSet(qemuCaps, virQEMUCapsCommandLine[i].flag);
+
for (j = 0; j nvalues; j++) {
-if (STREQ(virQEMUCapsCommandLine[i].param, values[j])) {
+if (STREQ_NULLABLE(virQEMUCapsCommandLine[i].param, values[j])) {
virQEMUCapsSet(qemuCaps, virQEMUCapsCommandLine[i].flag);
break;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index db3dd73..3d9f87b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3659,7 +3659,8 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon,
int
qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr mon,
const char *option,
- char ***params)
+ char ***params,
+ bool *found)
{
VIR_DEBUG(mon=%p option=%s params=%p, mon, option, params);
@@ -3675,7 +3676,8 @@ qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr
mon,
return -1;
}
-return qemuMonitorJSONGetCommandLineOptionParameters(mon, option, params);
+return qemuMonitorJSONGetCommandLineOptionParameters(mon, option,
+ params, found);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8a23267..c3695f2 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -748,7 +748,8 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon,
char ***events);
int qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr mon,
const char *option,
- char ***params);
+ char ***params,
+ bool *found);
int qemuMonitorGetKVMState(qemuMonitorPtr mon,
bool *enabled,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bef6a14..a62c02f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4504,7 +4504,8 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon,
int
qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
const char *option,
- char ***params)
+ char ***params,
+ bool *found)
{
int ret;
virJSONValuePtr cmd = NULL;
@@ -4516,6 +4517,8 @@
qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
size_t i;
*params = NULL;
+if (found)
+*found = false;
/* query-command-line-options has fixed output for a given qemu
* binary; but since callers want to query parameters for one
@@ -4576,6 +4579,9 @@
qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
goto cleanup;
}
+if (found)
+*found = true;
+
if ((n = virJSONValueArraySize(data)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(query-command-line-options parameter data was not
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 385211c..5f6c846 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -332,7 +332,8 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr
[libvirt] [PATCH v2 11/16] qemu: memory-backend-ram capability probing
The numa patch series in qemu adds memory-backend-ram object type by
which we can tell whether we can use such objects.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9c4ea87..0dbcd68 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -260,6 +260,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
msg-timestamp,
active-commit,
change-backing-file,
+
+ memory-backend-ram, /* 170 */
);
@@ -1476,6 +1478,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ ich9-intel-hda, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA },
{ pvpanic, QEMU_CAPS_DEVICE_PANIC },
{ usb-kbd, QEMU_CAPS_DEVICE_USB_KBD },
+{ memory-backend-ram, QEMU_CAPS_OBJECT_MEMORY_RAM },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 99cf9ed..c661d5a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -209,6 +209,7 @@ typedef enum {
QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */
QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */
QEMU_CAPS_CHANGE_BACKING_FILE = 169, /* change name of backing file in
metadata */
+QEMU_CAPS_OBJECT_MEMORY_RAM = 170, /* -object memory-backend-ram */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 14/16] qemu: pass numa node binding preferences to qemu
Currently, we only bind the whole QEMU domain to memory nodes
specified in nodemask altogether. That, however, doesn't make much
sense when one wants to control from where the memory for particular
guest nodes should be allocated. QEMU allows us to do that by
specifying 'host-nodes' parameter for the 'memory-backend-ram' object,
so let's use that.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c| 59 +-
.../qemuxml2argv-numatune-memnode-no-memory.args | 8 +++
.../qemuxml2argv-numatune-memnode.args | 11
.../qemuxml2argv-numatune-memnode.xml | 14 ++---
tests/qemuxml2argvtest.c | 7 +++
5 files changed, 92 insertions(+), 7 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 284664b..fc3406e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -150,6 +150,11 @@ VIR_ENUM_IMPL(qemuDomainFSDriver,
VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
NULL,
NULL);
+VIR_ENUM_DECL(qemuNumaPolicy)
+VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST,
+ bind,
+ preferred,
+ interleave);
/**
* qemuPhysIfaceConnect:
@@ -6373,13 +6378,23 @@ qemuBuildNumaArgStr(const virDomainDef *def,
size_t i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
+char *nodemask = NULL;
int ret = -1;
+if (virDomainNumatuneHasPerNodeBinding(def-numatune)
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node memory binding is not supported
+ with this QEMU));
+goto cleanup;
+}
+
for (i = 0; i def-cpu-ncells; i++) {
int cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024);
def-cpu-cells[i].mem = cellmem * 1024;
VIR_FREE(cpumask);
+VIR_FREE(nodemask);
if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
goto cleanup;
@@ -6392,6 +6407,43 @@ qemuBuildNumaArgStr(const virDomainDef *def,
goto cleanup;
}
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virDomainNumatuneMemMode mode;
+const char *policy = NULL;
+
+mode = virDomainNumatuneGetMode(def-numatune, i);
+policy = qemuNumaPolicyTypeToString(mode);
+
+virBufferAsprintf(buf,
memory-backend-ram,size=%dM,id=ram-node%zu,
+ cellmem, i);
+
+if (virDomainNumatuneMaybeFormatNodeset(def-numatune, NULL,
+nodemask, i) 0)
+goto cleanup;
+
+if (nodemask) {
+if (strchr(nodemask, ',')
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(disjoint NUMA node ranges are not
supported
+ with this QEMU));
+goto cleanup;
+}
+
+for (tmpmask = nodemask; tmpmask; tmpmask = next) {
+if ((next = strchr(tmpmask, ',')))
+*(next++) = '\0';
+virBufferAddLit(buf, ,host-nodes=);
+virBufferAdd(buf, tmpmask, -1);
+}
+
+virBufferAsprintf(buf, ,policy=%s, policy);
+}
+
+virCommandAddArg(cmd, -object);
+virCommandAddArgBuffer(cmd, buf);
+}
+
virCommandAddArg(cmd, -numa);
virBufferAsprintf(buf, node,nodeid=%zu, i);
@@ -6402,7 +6454,11 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virBufferAdd(buf, tmpmask, -1);
}
-virBufferAsprintf(buf, ,mem=%d, cellmem);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virBufferAsprintf(buf, ,memdev=ram-node%zu, i);
+} else {
+virBufferAsprintf(buf, ,mem=%d, cellmem);
+}
virCommandAddArgBuffer(cmd, buf);
}
@@ -6410,6 +6466,7 @@ qemuBuildNumaArgStr(const virDomainDef *def,
cleanup:
VIR_FREE(cpumask);
+VIR_FREE(nodemask);
virBufferFreeAndReset(buf);
return ret;
}
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
new file mode 100644
index 000..b0e274c
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
@@ -0,0 +1,8 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/kvm -S
[libvirt] [PATCH v2 08/16] conf, schema: add support for memnode elements
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 15 ++
docs/schemas/domaincommon.rng | 17 ++
src/conf/numatune_conf.c | 183 +++--
.../qemuxml2argv-numatune-memnode-no-memory.xml| 30
.../qemuxml2argv-numatune-memnode-nocpu.xml| 25 +++
.../qemuxml2argv-numatune-memnode.xml | 31
.../qemuxml2argv-numatune-memnodes-problematic.xml | 31
tests/qemuxml2argvtest.c | 2 +
.../qemuxml2xmlout-numatune-memnode.xml| 33
tests/qemuxml2xmltest.c| 2 +
10 files changed, 356 insertions(+), 13 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ad87b7c..d845c1b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -709,6 +709,8 @@
...
lt;numatunegt;
lt;memory mode=strict nodeset=1-4,^3/gt;
+lt;memnode cellid=0 mode=strict nodeset=1/gt;
+lt;memnode cellid=2 mode=preferred nodeset=2/gt;
lt;/numatunegt;
...
lt;/domaingt;
@@ -745,6 +747,19 @@
span class='since'Since 0.9.3/span
/dd
+ dtcodememnode/code/dt
+ dd
+Optional codememnode/code elements can specify memory allocation
+policies per each guest NUMA node. For those nodes having no
+corresponding codememnode/code element, the default from
+element codememory/code will be used. Attribute
codecellid/code
+addresses guest NUMA node for which the settings are applied.
+Attributes codemode/code and codenodeset/code have the same
+meaning and syntax as in codememory/code element.
+
+This setting is not compatible with automatic placement.
+span class='since'QEMU Since 1.2.6/span
+ /dd
/dl
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 155a33e..0b31261 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -789,6 +789,23 @@
/choice
/element
/optional
+ zeroOrMore
+element name=memnode
+ attribute name=cellid
+ref name=unsignedInt/
+ /attribute
+ attribute name=mode
+choice
+ valuestrict/value
+ valuepreferred/value
+ valueinterleave/value
+/choice
+ /attribute
+ attribute name='nodeset'
+ref name='cpuset'/
+ /attribute
+/element
+ /zeroOrMore
/element
/define
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 8b66558..67fc799 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -42,17 +42,137 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
static,
auto);
+typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
+typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
+
struct _virDomainNumatune {
struct {
+bool specified;
virBitmapPtr nodeset;
virDomainNumatuneMemMode mode;
virDomainNumatunePlacement placement;
} memory; /* pinning for all the memory */
+struct _virDomainNumatuneNode {
+virBitmapPtr nodeset;
+virDomainNumatuneMemMode mode;
+} *mem_nodes; /* fine tuning per guest node */
+size_t nmem_nodes;
+
/* Future NUMA tuning related stuff should go here. */
};
+static int
+virDomainNumatuneNodeParseXML(virDomainDefPtr def,
+ xmlXPathContextPtr ctxt)
+{
+int n = 0;;
+int ret = -1;
+size_t i = 0;
+xmlNodePtr *nodes = NULL;
+
+if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes)) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot extract memnode nodes));
+goto cleanup;
+}
+
+if (!n)
+return 0;
+
+if (def-numatune def-numatune-memory.specified
+def-numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node binding is not compatible with
+ automatic NUMA placement.));
+goto cleanup;
+}
+
+if (!def-cpu || !def-cpu-ncells) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Element 'memnode' is invalid without
+ any guest NUMA cells));
+goto cleanup
[libvirt] [PATCH v2 00/16] Support for per-guest-node binding
Currently we are only able to bind the whole domain to some host nodes using the /domain/numatune/memory element. Numerous requests were made to support host-guest numa node bindings, so this series tries to implement that using /domain/numatune/memnode elements. That is incompatible with automatic numa placement (numad) since that makes no sense. Some of these patches were ACK'd in the previous round, but this version completely rewrites the parsing and formatting of the numatune XML element and places it into a separate file. Hence the repost of all the patches even with those ACK'd ones. Patches 1-3 move some code around, patch 4 adds cell id specification into the XML (which is used later on). Then patches 5-7 rework the numatune handling, which clears out some odd things, but mostly cleans the parsing code. Patch 8 adds the support for memnode elements in the XML conf code and together with patch 9 enables the use of it outside numatune_conf.c. After that, I needed to probe qemu for 2 capabilities; for one of them patch 10 adds the possibility to probe for it, then two following patches 11 and 12 add the probing data. One of the capabilities tells us that we can use disjoint ranges (not necessarily for the cpus= param) with qemu, so patch 13 makes a use of it. Finally patch 14 uses the memnode data to tell qemu which host nodes should be used for the allocations of memory blocks. Patch 15 does almost nothing, but the next patch looks better with it. And the last patch, number 16, fixes a bug with KVM and cgroups. One last note, APIs for handling the memnode settings will be added later. I'm sending this to prepare the ground for Michal's work with hugepages. Martin Kletzander (16): qemu: purely a code movement qemu: remove useless error check conf: purely a code movement conf, schema: add 'id' field for cells numatune: create new module for numatune numatune: unify numatune struct and enum names numatune: Encapsulate numatune configuration in order to unify results conf, schema: add support for memnode elements numatune: add support for per-node memory bindings in private APIs qemu: allow qmp probing for cmdline options without params qemu: memory-backend-ram capability probing qemu: newer -numa parameter capability probing qemu: enable disjoint numa cpu ranges qemu: pass numa node binding preferences to qemu qemu: split out cpuset.mems setting qemu: leave restricting cpuset.mems after initialization docs/formatdomain.html.in | 26 +- docs/schemas/domaincommon.rng | 22 + po/POTFILES.in | 1 + src/Makefile.am| 3 +- src/conf/cpu_conf.c| 39 +- src/conf/cpu_conf.h| 3 +- src/conf/domain_conf.c | 203 ++- src/conf/domain_conf.h | 10 +- src/conf/numatune_conf.c | 589 + src/conf/numatune_conf.h | 108 src/libvirt_private.syms | 24 +- src/lxc/lxc_cgroup.c | 20 +- src/lxc/lxc_controller.c | 5 +- src/lxc/lxc_native.c | 15 +- src/parallels/parallels_driver.c | 7 +- src/qemu/qemu_capabilities.c | 16 +- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_cgroup.c | 52 +- src/qemu/qemu_cgroup.h | 4 +- src/qemu/qemu_command.c| 98 +++- src/qemu/qemu_driver.c | 84 ++- src/qemu/qemu_monitor.c| 6 +- src/qemu/qemu_monitor.h| 3 +- src/qemu/qemu_monitor_json.c | 8 +- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_process.c| 12 +- src/util/virnuma.c | 61 +-- src/util/virnuma.h | 28 +- tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 5 + tests/qemumonitorjsontest.c| 20 +- .../qemuxml2argv-cpu-numa-disjoint.args| 6 + .../qemuxml2argv-cpu-numa-disjoint.xml | 28 + tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 + .../qemuxml2argv-numatune-auto-prefer.xml | 29 + .../qemuxml2argv-numatune-memnode-no-memory.args | 8 + .../qemuxml2argv-numatune-memnode-no-memory.xml| 30 ++ .../qemuxml2argv-numatune-memnode-nocpu.xml| 25 + .../qemuxml2argv-numatune-memnode.args
[libvirt] [PATCH v2 09/16] numatune: add support for per-node memory bindings in private APIs
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/numatune_conf.c | 111 ---
src/conf/numatune_conf.h | 14 --
src/libvirt_private.syms | 1 +
src/lxc/lxc_cgroup.c | 3 +-
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_driver.c | 10 ++---
src/util/virnuma.c | 6 +--
7 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 67fc799..60b6867 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -63,6 +63,18 @@ struct _virDomainNumatune {
};
+static inline bool
+numa_cell_specified(virDomainNumatunePtr numatune,
+int cellid)
+{
+if (numatune
+cellid = 0
+cellid numatune-nmem_nodes)
+return numatune-mem_nodes[cellid].nodeset;
+
+return false;
+}
+
static int
virDomainNumatuneNodeParseXML(virDomainDefPtr def,
xmlXPathContextPtr ctxt)
@@ -312,6 +324,7 @@ void
virDomainNumatuneFree(virDomainNumatunePtr numatune)
{
size_t i = 0;
+
if (!numatune)
return;
@@ -324,26 +337,37 @@ virDomainNumatuneFree(virDomainNumatunePtr numatune)
}
virDomainNumatuneMemMode
-virDomainNumatuneGetMode(virDomainNumatunePtr numatune)
+virDomainNumatuneGetMode(virDomainNumatunePtr numatune,
+ int cellid)
{
-return (numatune numatune-memory.specified) ? numatune-memory.mode :
0;
+if (!numatune)
+return 0;
+
+if (numa_cell_specified(numatune, cellid))
+return numatune-mem_nodes[cellid].mode;
+
+if (numatune-memory.specified)
+return numatune-memory.mode;
+
+return 0;
}
virBitmapPtr
virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune,
-virBitmapPtr auto_nodeset)
+virBitmapPtr auto_nodeset,
+int cellid)
{
if (!numatune)
return NULL;
-if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
+if (numatune-memory.specified
+numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
return auto_nodeset;
-/*
- * This weird logic has the same meaning as switch with
- * auto/static/default, but can be more readably changed later.
- */
-if (numatune-memory.placement != VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC)
+if (numa_cell_specified(numatune, cellid))
+return numatune-mem_nodes[cellid].nodeset;
+
+if (!numatune-memory.specified)
return NULL;
return numatune-memory.nodeset;
@@ -351,23 +375,30 @@ virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune,
char *
virDomainNumatuneFormatNodeset(virDomainNumatunePtr numatune,
- virBitmapPtr auto_nodeset)
+ virBitmapPtr auto_nodeset,
+ int cellid)
{
return virBitmapFormat(virDomainNumatuneGetNodeset(numatune,
- auto_nodeset));
+ auto_nodeset,
+ cellid));
}
int
virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune,
virBitmapPtr auto_nodeset,
-char **mask)
+char **mask,
+int cellid)
{
*mask = NULL;
if (!numatune)
return 0;
-if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO
+if (!numa_cell_specified(numatune, cellid) !numatune-memory.specified)
+return 0;
+
+if (numatune-memory.specified
+numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO
!auto_nodeset) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Advice from numad is needed in case of
@@ -375,7 +406,7 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr
numatune,
return -1;
}
-*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset);
+*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset, cellid);
if (!*mask)
return -1;
@@ -469,6 +500,35 @@ virDomainNumatuneSet(virDomainDefPtr def,
return ret;
}
+static bool
+virDomainNumatuneNodesEqual(virDomainNumatunePtr n1,
+virDomainNumatunePtr n2)
+{
+size_t i = 0;
+
+if (n1-nmem_nodes != n2-nmem_nodes)
+return false;
+
+for (i = 0; i n1-nmem_nodes; i++) {
+virDomainNumatuneNodePtr nd1 = n1-mem_nodes[i];
+virDomainNumatuneNodePtr nd2 = n2-mem_nodes[i];
+
+if (!nd1-nodeset !nd2-nodeset)
+continue;
+
+if (!nd1-nodeset || !nd2-nodeset)
+return false;
+
+if (nd1-mode != nd2-mode)
+return false;
+
+if (!virBitmapEqual(nd1-nodeset, nd2
[libvirt] [PATCH v2 16/16] qemu: leave restricting cpuset.mems after initialization
When domain is started with numatune memory mode strict and the
nodeset does not include host NUMA node with DMA and DMA32 zones, KVM
initialization fails. This is because cgroup restrict even kernel
allocations. We are already doing numa_set_membind() which does the
same thing, only it does not restrict kernel allocations.
This patch leaves the userspace numa_set_membind() in place and moves
the cpuset.mems setting after the point where monitor comes up, but
before vcpu and emulator sub-groups are created.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
Another approach would be not using cgroups for this at all; it should
still work as expected.
src/qemu/qemu_cgroup.c | 10 +++---
src/qemu/qemu_cgroup.h | 4 +++-
src/qemu/qemu_process.c | 4
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index eebe9e9..ffef1fb 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -614,9 +614,6 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm,
if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
return 0;
-if (qemuSetupCpusetMems(vm, nodemask) 0)
-goto cleanup;
-
if (vm-def-cpumask ||
(vm-def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)) {
@@ -813,6 +810,13 @@ qemuSetupCgroup(virQEMUDriverPtr driver,
}
int
+qemuSetupCgroupPostInit(virDomainObjPtr vm,
+virBitmapPtr nodemask)
+{
+return qemuSetupCpusetMems(vm, nodemask);
+}
+
+int
qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
unsigned long long period,
long long quota)
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 14404d1..40a031e 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -1,7 +1,7 @@
/*
* qemu_cgroup.h: QEMU cgroup management
*
- * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -44,6 +44,8 @@ int qemuConnectCgroup(virQEMUDriverPtr driver,
int qemuSetupCgroup(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virBitmapPtr nodemask);
+int qemuSetupCgroupPostInit(virDomainObjPtr vm,
+virBitmapPtr nodemask);
int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
unsigned long long period,
long long quota);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4a27eab..8e70258 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4166,6 +4166,10 @@ int qemuProcessStart(virConnectPtr conn,
if (!qemuProcessVerifyGuestCPU(driver, vm))
goto cleanup;
+VIR_DEBUG(Setting up post-init cgroup restrictions);
+if (qemuSetupCgroupPostInit(vm, nodemask) 0)
+goto cleanup;
+
VIR_DEBUG(Detecting VCPU PIDs);
if (qemuProcessDetectVcpuPIDs(driver, vm) 0)
goto cleanup;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 06/16] numatune: unify numatune struct and enum names
Since there was already public virDomainNumatune*, I changed the
private virNumaTune to match the same, so all the uses are unified and
public API is kept:
s/vir\(Domain\)\?Numa[tT]une/virDomainNumatune/g
then shrunk long lines, and mainly functions, that were created after
that:
sed -i 's/virDomainNumatuneMemPlacementMode/virDomainNumatunePlacement/g'
And to cope with the enum name, I haad to change the constants as
well:
s/VIR_NUMA_TUNE_MEM_PLACEMENT_MODE/VIR_DOMAIN_NUMATUNE_PLACEMENT/g
Last thing I did was at least a little shortening of already long
name:
s/virDomainNumatuneDef/virDomainNumatune/g
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c | 20 ++--
src/conf/domain_conf.h | 2 +-
src/conf/numatune_conf.c | 4 ++--
src/conf/numatune_conf.h | 20 ++--
src/libvirt_private.syms | 4 ++--
src/lxc/lxc_cgroup.c | 4 ++--
src/lxc/lxc_controller.c | 2 +-
src/lxc/lxc_native.c | 2 +-
src/qemu/qemu_cgroup.c | 4 ++--
src/qemu/qemu_driver.c | 10 +-
src/qemu/qemu_process.c | 2 +-
src/util/virnuma.c | 8
src/util/virnuma.h | 2 +-
13 files changed, 42 insertions(+), 42 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3aa3476..31f4bc0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11748,7 +11748,7 @@ virDomainDefParseXML(xmlDocPtr xml,
int placement_mode = 0;
if (placement) {
if ((placement_mode =
-
virNumaTuneMemPlacementModeTypeFromString(placement)) 0) {
+
virDomainNumatunePlacementTypeFromString(placement)) 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Unsupported memory placement
mode '%s'), placement);
@@ -11758,18 +11758,18 @@ virDomainDefParseXML(xmlDocPtr xml,
VIR_FREE(placement);
} else if (def-numatune.memory.nodemask) {
/* Defaults to static if nodeset is specified. */
-placement_mode =
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC;
+placement_mode = VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC;
} else {
/* Defaults to placement of vcpu if nodeset is
* not specified.
*/
if (def-placement_mode ==
VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC)
-placement_mode =
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC;
+placement_mode =
VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC;
else
-placement_mode =
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO;
+placement_mode =
VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO;
}
-if (placement_mode ==
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC
+if (placement_mode == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC
!def-numatune.memory.nodemask) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(nodeset for NUMA memory tuning must
be set
@@ -11778,7 +11778,7 @@ virDomainDefParseXML(xmlDocPtr xml,
}
/* Ignore 'nodeset' if 'placement' is 'auto' finally */
-if (placement_mode ==
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO) {
+if (placement_mode == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) {
virBitmapFree(def-numatune.memory.nodemask);
def-numatune.memory.nodemask = NULL;
}
@@ -11786,7 +11786,7 @@ virDomainDefParseXML(xmlDocPtr xml,
/* Copy 'placement' of numatune to vcpu if its
'placement'
* is not specified and 'placement' of numatune is
specified.
*/
-if (placement_mode ==
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO
+if (placement_mode == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO
!def-cpumask)
def-placement_mode =
VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;
@@ -11805,7 +11805,7 @@ virDomainDefParseXML(xmlDocPtr xml,
* and 'placement' of vcpu is 'auto'.
*/
if (def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
-def-numatune.memory.placement_mode =
VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO;
+def-numatune.memory.placement_mode =
VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO;
def-numatune.memory.mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
}
}
@@ -17403,13 +17403,13 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf
[libvirt] [PATCH v2 12/16] qemu: newer -numa parameter capability probing
When qemu switched to using OptsVisitor for -numa parameter, it did
two things in the same patch. One of them is that the numa parameter
is now visible in query-command-line-options, the second one is that
it enabled using disjoint cpu ranges for -numa specification. This
will be used in later patch.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps| 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 5 +
4 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0dbcd68..e553ce5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -262,6 +262,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
change-backing-file,
memory-backend-ram, /* 170 */
+ numa,
);
@@ -2425,6 +2426,7 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ boot-opts, reboot-timeout, QEMU_CAPS_REBOOT_TIMEOUT },
{ spice, disable-agent-file-xfer, QEMU_CAPS_SPICE_FILE_XFER_DISABLE },
{ msg, timestamp, QEMU_CAPS_MSG_TIMESTAMP },
+{ numa, NULL, QEMU_CAPS_NUMA },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c661d5a..4332633 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -210,6 +210,7 @@ typedef enum {
QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */
QEMU_CAPS_CHANGE_BACKING_FILE = 169, /* change name of backing file in
metadata */
QEMU_CAPS_OBJECT_MEMORY_RAM = 170, /* -object memory-backend-ram */
+QEMU_CAPS_NUMA = 171, /* newer -numa handling with disjoint
cpu ranges */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index 32bccdb..4b9f693 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -142,4 +142,5 @@
flag name='usb-kbd'/
flag name='host-pci-multidomain'/
flag name='msg-timestamp'/
+flag name='numa'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
index a60542a..ec7451f 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
@@ -2389,6 +2389,11 @@
{
parameters: [
],
+option: numa
+},
+{
+parameters: [
+],
option: device
},
{
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 07/16] numatune: Encapsulate numatune configuration in order to unify results
There were numerous places where numatune configuration (and thus
domain config as well) was changed in different ways. On some
places this even resulted in persistent domain definition not to be
stable (it would change with daemon's restart).
In order to uniformly change how numatune config is dealt with, all
the internals are now accessible directly only in numatune_conf.c and
outside this file accessors must be used.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
po/POTFILES.in | 1 +
src/conf/domain_conf.c | 159 ++-
src/conf/domain_conf.h | 8 +-
src/conf/numatune_conf.c | 316 +
src/conf/numatune_conf.h | 72 -
src/libvirt_private.syms | 11 +
src/lxc/lxc_cgroup.c | 19 +-
src/lxc/lxc_controller.c | 5 +-
src/lxc/lxc_native.c | 15 +-
src/parallels/parallels_driver.c | 7 +-
src/qemu/qemu_cgroup.c | 23 +-
src/qemu/qemu_driver.c | 84 +++---
src/qemu/qemu_process.c| 8 +-
src/util/virnuma.c | 48 ++--
src/util/virnuma.h | 2 +-
.../qemuxml2argv-numatune-auto-prefer.xml | 29 ++
.../qemuxml2xmlout-numatune-auto-prefer.xml| 29 ++
tests/qemuxml2xmltest.c| 2 +
18 files changed, 553 insertions(+), 285 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 64a987e..b5cfae4 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -25,6 +25,7 @@ src/conf/netdev_vlan_conf.c
src/conf/netdev_vport_profile_conf.c
src/conf/network_conf.c
src/conf/node_device_conf.c
+src/conf/numatune_conf.c
src/conf/nwfilter_conf.c
src/conf/nwfilter_params.c
src/conf/object_event.c
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 31f4bc0..f21a556 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2097,7 +2097,7 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainVcpuPinDefFree(def-cputune.emulatorpin);
-virBitmapFree(def-numatune.memory.nodemask);
+virDomainNumatuneFree(def-numatune);
virSysinfoDefFree(def-sysinfo);
@@ -11231,7 +11231,6 @@ virDomainDefParseXML(xmlDocPtr xml,
unsigned long count;
bool uuid_generated = false;
virHashTablePtr bootHash = NULL;
-xmlNodePtr cur;
bool usb_none = false;
bool usb_other = false;
bool usb_master = false;
@@ -11693,123 +11692,8 @@ virDomainDefParseXML(xmlDocPtr xml,
}
}
-/* Extract numatune if exists. */
-if ((n = virXPathNodeSet(./numatune, ctxt, nodes)) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- %s, _(cannot extract numatune nodes));
+if (virDomainNumatuneParseXML(def, ctxt) 0)
goto error;
-}
-
-if (n 1) {
-virReportError(VIR_ERR_XML_ERROR, %s,
- _(only one numatune is supported));
-VIR_FREE(nodes);
-goto error;
-}
-
-if (n) {
-cur = nodes[0]-children;
-while (cur != NULL) {
-if (cur-type == XML_ELEMENT_NODE) {
-if (xmlStrEqual(cur-name, BAD_CAST memory)) {
-char *mode = NULL;
-char *placement = NULL;
-char *nodeset = NULL;
-
-mode = virXMLPropString(cur, mode);
-if (mode) {
-if ((def-numatune.memory.mode =
- virDomainNumatuneMemModeTypeFromString(mode))
0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(Unsupported NUMA memory
- tuning mode '%s'),
- mode);
-VIR_FREE(mode);
-goto error;
-}
-VIR_FREE(mode);
-} else {
-def-numatune.memory.mode =
VIR_DOMAIN_NUMATUNE_MEM_STRICT;
-}
-
-nodeset = virXMLPropString(cur, nodeset);
-if (nodeset) {
-if (virBitmapParse(nodeset,
- 0,
- def-numatune.memory.nodemask,
- VIR_DOMAIN_CPUMASK_LEN) 0) {
-VIR_FREE(nodeset);
-goto error
[libvirt] [PATCH v2 15/16] qemu: split out cpuset.mems setting
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_cgroup.c | 29 -
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ff268fb..eebe9e9 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -576,13 +576,11 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver,
static int
-qemuSetupCpusetCgroup(virDomainObjPtr vm,
- virBitmapPtr nodemask,
- virCapsPtr caps)
+qemuSetupCpusetMems(virDomainObjPtr vm,
+virBitmapPtr nodemask)
{
qemuDomainObjPrivatePtr priv = vm-privateData;
char *mem_mask = NULL;
-char *cpu_mask = NULL;
int ret = -1;
if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
@@ -597,6 +595,28 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm,
virCgroupSetCpusetMems(priv-cgroup, mem_mask) 0)
goto cleanup;
+ret = 0;
+ cleanup:
+VIR_FREE(mem_mask);
+return ret;
+}
+
+
+static int
+qemuSetupCpusetCgroup(virDomainObjPtr vm,
+ virBitmapPtr nodemask,
+ virCapsPtr caps)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+char *cpu_mask = NULL;
+int ret = -1;
+
+if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+return 0;
+
+if (qemuSetupCpusetMems(vm, nodemask) 0)
+goto cleanup;
+
if (vm-def-cpumask ||
(vm-def-placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)) {
@@ -619,7 +639,6 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm,
ret = 0;
cleanup:
-VIR_FREE(mem_mask);
VIR_FREE(cpu_mask);
return ret;
}
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 03/16] conf: purely a code movement
to ease the review of commits to follow.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c | 52 +-
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 54925ba..3aa3476 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11667,6 +11667,32 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(nodes);
+/* analysis of cpu handling */
+if ((node = virXPathNode(./cpu[1], ctxt)) != NULL) {
+xmlNodePtr oldnode = ctxt-node;
+ctxt-node = node;
+def-cpu = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_GUEST);
+ctxt-node = oldnode;
+
+if (def-cpu == NULL)
+goto error;
+
+if (def-cpu-sockets
+def-maxvcpus
+def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
+virReportError(VIR_ERR_XML_DETAIL, %s,
+ _(Maximum CPUs greater than topology limit));
+goto error;
+}
+
+if (def-cpu-cells_cpus def-maxvcpus) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Number of CPUs in numa exceeds the
+ vcpu count));
+goto error;
+}
+}
+
/* Extract numatune if exists. */
if ((n = virXPathNodeSet(./numatune, ctxt, nodes)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -12864,32 +12890,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
-/* analysis of cpu handling */
-if ((node = virXPathNode(./cpu[1], ctxt)) != NULL) {
-xmlNodePtr oldnode = ctxt-node;
-ctxt-node = node;
-def-cpu = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_GUEST);
-ctxt-node = oldnode;
-
-if (def-cpu == NULL)
-goto error;
-
-if (def-cpu-sockets
-def-maxvcpus
-def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
-virReportError(VIR_ERR_XML_DETAIL, %s,
- _(Maximum CPUs greater than topology limit));
-goto error;
-}
-
-if (def-cpu-cells_cpus def-maxvcpus) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(Number of CPUs in numa exceeds the
- vcpu count));
-goto error;
-}
-}
-
if ((node = virXPathNode(./sysinfo[1], ctxt)) != NULL) {
xmlNodePtr oldnode = ctxt-node;
ctxt-node = node;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 05/16] numatune: create new module for numatune
There are many places with numatune-related code that should be put
into special numatune_conf and this patch creates a basis for that.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/Makefile.am | 3 ++-
src/conf/domain_conf.h | 2 +-
src/conf/numatune_conf.c | 37 ++
src/conf/numatune_conf.h | 54
src/libvirt_private.syms | 12 ++
src/qemu/qemu_capabilities.c | 1 +
src/util/virnuma.c | 13 +--
src/util/virnuma.h | 26 ++---
8 files changed, 106 insertions(+), 42 deletions(-)
create mode 100644 src/conf/numatune_conf.c
create mode 100644 src/conf/numatune_conf.h
diff --git a/src/Makefile.am b/src/Makefile.am
index e2f76a7..48c8e33 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -252,7 +252,8 @@ DOMAIN_CONF_SOURCES =
\
conf/domain_conf.c conf/domain_conf.h \
conf/domain_audit.c conf/domain_audit.h \
conf/domain_nwfilter.c conf/domain_nwfilter.h \
- conf/snapshot_conf.c conf/snapshot_conf.h
+ conf/snapshot_conf.c conf/snapshot_conf.h \
+ conf/numatune_conf.c conf/numatune_conf.h
OBJECT_EVENT_SOURCES = \
conf/object_event.c conf/object_event.h \
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a00e30a..018b516 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -36,6 +36,7 @@
# include virhash.h
# include virsocketaddr.h
# include nwfilter_params.h
+# include numatune_conf.h
# include virnetdevmacvlan.h
# include virsysinfo.h
# include virnetdevvportprofile.h
@@ -46,7 +47,6 @@
# include device_conf.h
# include virbitmap.h
# include virstoragefile.h
-# include virnuma.h
# include virseclabel.h
/* forward declarations of all device types, required by
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
new file mode 100644
index 000..e9be040
--- /dev/null
+++ b/src/conf/numatune_conf.c
@@ -0,0 +1,37 @@
+/*
+ * numatune_conf.c
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Martin Kletzander mklet...@redhat.com
+ */
+
+#include config.h
+
+#include numatune_conf.h
+
+VIR_ENUM_IMPL(virDomainNumatuneMemMode,
+ VIR_DOMAIN_NUMATUNE_MEM_LAST,
+ strict,
+ preferred,
+ interleave);
+
+VIR_ENUM_IMPL(virNumaTuneMemPlacementMode,
+ VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_LAST,
+ default,
+ static,
+ auto);
diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
new file mode 100644
index 000..6bdfdc0
--- /dev/null
+++ b/src/conf/numatune_conf.h
@@ -0,0 +1,54 @@
+/*
+ * numatune_conf.h
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Martin Kletzander mklet...@redhat.com
+ */
+
+#ifndef __NUMATUNE_CONF_H__
+# define __NUMATUNE_CONF_H__
+
+# include internal.h
+# include virutil.h
+# include virbitmap.h
+
+typedef enum {
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_DEFAULT = 0,
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC,
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO,
+
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_LAST
+} virDomainNumaTuneMemPlacementMode;
+
+VIR_ENUM_DECL(virNumaTuneMemPlacementMode)
+
+VIR_ENUM_DECL(virDomainNumatuneMemMode)
+
+typedef struct _virNumaTuneDef virNumaTuneDef;
+typedef virNumaTuneDef *virNumaTuneDefPtr;
+struct _virNumaTuneDef {
+struct {
+virBitmapPtr nodemask
[libvirt] [PATCH v2 02/16] qemu: remove useless error check
Excerpt from the virCommandAddArgBuffer() description: Correctly transfers memory errors or contents from buf to cmd. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_command.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aed7411..0ead1d0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6402,9 +6402,6 @@ qemuBuildNumaArgStr(const virDomainDef *def, virCommandPtr cmd) virBufferAdd(buf, cpumask, -1); virBufferAsprintf(buf, ,mem=%d, cellmem); -if (virBufferCheckError(buf) 0) -goto cleanup; - virCommandAddArgBuffer(cmd, buf); } ret = 0; -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 04/16] conf, schema: add 'id' field for cells
In XML format, by definition, order of fields should not matter, so
order of parsing the elements doesn't affect the end result. When
specifying guest NUMA cells, we depend only on the order of the 'cell'
elements. With this patch all older domain XMLs are parsed as before,
but with the 'id' attribute they are parsed and formatted according to
that field. This will be useful when we have tuning settings for
particular guest NUMA node.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 11 +++---
docs/schemas/domaincommon.rng | 5 +++
src/conf/cpu_conf.c| 39 +++---
src/conf/cpu_conf.h| 3 +-
src/qemu/qemu_command.c| 2 +-
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 ++
tests/qemuxml2argvtest.c | 1 +
.../qemuxml2xmlout-cpu-numa1.xml | 28
.../qemuxml2xmlout-cpu-numa2.xml | 28
tests/qemuxml2xmltest.c| 3 ++
12 files changed, 139 insertions(+), 18 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b69da4c..ad87b7c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1030,8 +1030,8 @@
lt;cpugt;
...
lt;numagt;
- lt;cell cpus='0-3' memory='512000'/gt;
- lt;cell cpus='4-7' memory='512000'/gt;
+ lt;cell id='0' cpus='0-3' memory='512000'/gt;
+ lt;cell id='1' cpus='4-7' memory='512000'/gt;
lt;/numagt;
...
lt;/cpugt;
@@ -1041,8 +1041,11 @@
Each codecell/code element specifies a NUMA cell or a NUMA node.
codecpus/code specifies the CPU or range of CPUs that are part of
the node. codememory/code specifies the node memory in kibibytes
- (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
- or nodeid in the increasing order starting from 0.
+ (i.e. blocks of 1024 bytes). All cells should have codeid/code
+ attribute in case referring to some cell is necessary in the code,
+ otherwise the cells are assigned ids in the increasing order starting
+ from 0. Mixing cells with and without the codeid/code attribute
+ is not recommended as it may result in unwanted behaviour.
/p
p
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7be028d..155a33e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3902,6 +3902,11 @@
define name=numaCell
element name=cell
+ optional
+attribute name=id
+ ref name=unsignedInt/
+/attribute
+ /optional
attribute name=cpus
ref name=cpuset/
/attribute
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 811893d..9e0af08 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -152,7 +152,6 @@ virCPUDefCopy(const virCPUDef *cpu)
copy-ncells_max = copy-ncells = cpu-ncells;
for (i = 0; i cpu-ncells; i++) {
-copy-cells[i].cellid = cpu-cells[i].cellid;
copy-cells[i].mem = cpu-cells[i].mem;
copy-cells[i].cpumask = virBitmapNewCopy(cpu-cells[i].cpumask);
@@ -438,17 +437,46 @@ virCPUDefParseXML(xmlNodePtr node,
for (i = 0; i n; i++) {
char *cpus, *memory;
int ret, ncpus = 0;
+unsigned int cur_cell;
+char *tmp = NULL;
+
+tmp = virXMLPropString(nodes[i], id);
+if (!tmp) {
+cur_cell = i;
+} else {
+ret = virStrToLong_ui(tmp, NULL, 10, cur_cell);
+VIR_FREE(tmp);
+if (ret == -1) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Invalid 'id' attribute in NUMA cell));
+goto error;
+}
+}
+
+if (cur_cell = n) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Exactly one 'cell' element per guest
+ NUMA cell allowed, non-contiguous ranges or
+ ranges not starting from 0 are not
allowed));
+goto error;
+}
+
+if (def-cells[cur_cell].cpustr) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Duplicate NUMA cell info for cell id '%u'),
+ cur_cell);
+goto error
[libvirt] [PATCH] properly indent virSecurityLabelDefsParseXML() parameters
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
Pushed as trivial
src/conf/domain_conf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 70f1103..b80e7cf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4664,9 +4664,9 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
static int
virSecurityLabelDefsParseXML(virDomainDefPtr def,
-xmlXPathContextPtr ctxt,
-virCapsPtr caps,
-unsigned int flags)
+ xmlXPathContextPtr ctxt,
+ virCapsPtr caps,
+ unsigned int flags)
{
size_t i = 0;
int n;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: cgroup: Fix build on non-cgroup platforms
On Tue, Jul 08, 2014 at 05:40:40PM +0200, Peter Krempa wrote: Commit 48f44510098cead629ede9a49ea4e840a28ccca introduced a helper You probably meant a48f44510098cead629ede9a49ea4e840a28ccca. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: don't error out when cgroups don't exist
When creating cgroups for vcpu and emulator threads whilst starting a domain, we explicitly skip creating those cgroups in case priv-cgroup is NULL (cgroups not supported) because SetAffinity() serves the same purpose. If the host supports only some cgroups (the ones we need are either unmounted or disabled in qemu.conf), we error out with weird message even though we could continue starting the domain. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1097028 Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_cgroup.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3394c68..0af6ac5 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -949,7 +949,11 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) virCgroupFree(cgroup_vcpu); } -return -1; +if (period || quota) +return -1; + +virResetLastError(); +return 0; } int @@ -1016,7 +1020,11 @@ qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, virCgroupFree(cgroup_emulator); } -return -1; +if (period || quota) +return -1; + +virResetLastError(); +return 0; } int -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: document the possibility of accepting integers for numatune mode
According to the code, 'virsh numatune' supports integers for
specifying --mode as well as the string definitions strict,
interleave, and preferred. However, this possibility was not
documented anywhere, so this patch adds it to both the man page and
command help.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1085706
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
tools/virsh-domain.c | 3 ++-
tools/virsh.pod | 8 +---
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b5b9f91..5a17aff 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7823,7 +7823,8 @@ static const vshCmdOptDef opts_numatune[] = {
},
{.name = mode,
.type = VSH_OT_DATA,
- .help = N_(NUMA mode, one of strict, preferred and interleave)
+ .help = N_(NUMA mode, one of strict, preferred and interleave \n
+or a number from the virDomainNumatuneMemMode enum)
},
{.name = nodeset,
.type = VSH_OT_DATA,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 99d0b74..a5e8406 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1402,9 +1402,11 @@ Set or get a domain's numa parameters, corresponding to
the numatune
element of domain XML. Without flags, the current settings are
displayed.
-Imode can be one of `strict', `interleave' and `preferred'. For a
-running domain, the mode can't be changed, and the nodeset can be
-changed only if the domain was started with a mode of `strict'.
+Imode can be one of `strict', `interleave' and `preferred' or any
+valid number from the virDomainNumatuneMemMode enum in case the daemon
+supports it. For a running domain, the mode can't be changed, and the
+nodeset can be changed only if the domain was started with a mode of
+`strict'.
Inodeset is a list of numa nodes used by the host for running the domain.
Its syntax is a comma separated list, with '-' for ranges and '^' for
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: don't error out when cgroups don't exist
On Wed, Jul 09, 2014 at 01:07:47PM +0200, Ján Tomko wrote: On 07/09/2014 10:15 AM, Martin Kletzander wrote: When creating cgroups for vcpu and emulator threads whilst starting a domain, we explicitly skip creating those cgroups in case priv-cgroup is NULL (cgroups not supported) because SetAffinity() serves the same purpose. If the host supports only some cgroups (the ones we need are either unmounted or disabled in qemu.conf), we error out with weird message even though we could continue starting the domain. Yet this patch does not change the error message. Yes, because there should be no error. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1097028 Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_cgroup.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3394c68..0af6ac5 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -949,7 +949,11 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) virCgroupFree(cgroup_vcpu); } -return -1; +if (period || quota) +return -1; + +virResetLastError(); +return 0; } This also resets OOM errors and errors that happen when these controllers are mounted. Can't we just check upfront if the needed controllers are available? There might be more problems than the controllers not being mounted, but OK, the others are corner cases anyway. Would the following diff be more suitable? diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3394c68..d4c969b 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -893,6 +893,15 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) return -1; } +/* + * If CPU cgroup controller is not initialized here, than we need + * neither period nor quota settings. And if CPUSET controller is + * not initialized either, than there's nothing to do anyway. + */ +if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPU) +!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) +return 0; + /* We are trying to setup cgroups for CPU pinning, which can also be done * with virProcessSetAffinity, thus the lack of cgroups is not fatal here. */ @@ -972,6 +981,15 @@ qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, return -1; } +/* + * If CPU cgroup controller is not initialized here, than we need + * neither period nor quota settings. And if CPUSET controller is + * not initialized either, than there's nothing to do anyway. + */ +if (!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPU) +!virCgroupHasController(priv-cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) +return 0; + if (priv-cgroup == NULL) return 0; /* Not supported, so claim success */ -- Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/16] conf, schema: add 'id' field for cells
On Fri, Jul 11, 2014 at 05:11:05PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
In XML format, by definition, order of fields should not matter, so
order of parsing the elements doesn't affect the end result. When
specifying guest NUMA cells, we depend only on the order of the 'cell'
elements. With this patch all older domain XMLs are parsed as before,
but with the 'id' attribute they are parsed and formatted according to
that field. This will be useful when we have tuning settings for
particular guest NUMA node.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 11 +++---
docs/schemas/domaincommon.rng | 5 +++
src/conf/cpu_conf.c| 39 +++---
src/conf/cpu_conf.h| 3 +-
src/qemu/qemu_command.c| 2 +-
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 ++
tests/qemuxml2argvtest.c | 1 +
.../qemuxml2xmlout-cpu-numa1.xml | 28
.../qemuxml2xmlout-cpu-numa2.xml | 28
tests/qemuxml2xmltest.c| 3 ++
12 files changed, 139 insertions(+), 18 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b69da4c..ad87b7c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
[...]
@@ -1041,8 +1041,11 @@
Each codecell/code element specifies a NUMA cell or a NUMA node.
codecpus/code specifies the CPU or range of CPUs that are part of
the node. codememory/code specifies the node memory in kibibytes
- (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
- or nodeid in the increasing order starting from 0.
+ (i.e. blocks of 1024 bytes). All cells should have codeid/code
+ attribute in case referring to some cell is necessary in the code,
+ otherwise the cells are assigned ids in the increasing order starting
+ from 0. Mixing cells with and without the codeid/code attribute
+ is not recommended as it may result in unwanted behaviour.
I'd note here, that the @id attribute is since 1.2.7
I wasn't sure this is needed for attributes, but it cannot hurt,
right? The new hunk would now look like this:
@@ -1039,10 +1039,15 @@
p
Each codecell/code element specifies a NUMA cell or a NUMA node.
- codecpus/code specifies the CPU or range of CPUs that are part of
- the node. codememory/code specifies the node memory in kibibytes
- (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
- or nodeid in the increasing order starting from 0.
+ codecpus/code specifies the CPU or range of CPUs that are
+ part of the node. codememory/code specifies the node memory
+ in kibibytes (i.e. blocks of 1024 bytes).
+ span class=sinceSince 1.2.7/span all cells should
+ have codeid/code attribute in case referring to some cell is
+ necessary in the code, otherwise the cells are
+ assigned codeid/codes in the increasing order starting from
+ 0. Mixing cells with and without the codeid/code attribute
+ is not recommended as it may result in unwanted behaviour.
/p
p
--
Is that OK?
[...]
@@ -438,17 +437,46 @@ virCPUDefParseXML(xmlNodePtr node,
for (i = 0; i n; i++) {
char *cpus, *memory;
int ret, ncpus = 0;
+unsigned int cur_cell;
+char *tmp = NULL;
+
+tmp = virXMLPropString(nodes[i], id);
+if (!tmp) {
+cur_cell = i;
+} else {
+ret = virStrToLong_ui(tmp, NULL, 10, cur_cell);
+VIR_FREE(tmp);
+if (ret == -1) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Invalid 'id' attribute in NUMA cell));
+goto error;
+}
+}
If there's a typo in the @id, I think this can make users lives easier:
You mean like a typo in an integer, right? :-) If the user cannot find
a typo that causes our code not to parse it as an integer; then I
guess such user's biggest issue isn't the typo :-) But I squashed in
your suggestion.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 07/16] numatune: Encapsulate numatune configuration in order to unify results
On Fri, Jul 11, 2014 at 05:11:00PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
There were numerous places where numatune configuration (and thus
domain config as well) was changed in different ways. On some
places this even resulted in persistent domain definition not to be
stable (it would change with daemon's restart).
In order to uniformly change how numatune config is dealt with, all
the internals are now accessible directly only in numatune_conf.c and
outside this file accessors must be used.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
po/POTFILES.in | 1 +
src/conf/domain_conf.c | 159 ++-
src/conf/domain_conf.h | 8 +-
src/conf/numatune_conf.c | 316 +
src/conf/numatune_conf.h | 72 -
src/libvirt_private.syms | 11 +
src/lxc/lxc_cgroup.c | 19 +-
src/lxc/lxc_controller.c | 5 +-
src/lxc/lxc_native.c | 15 +-
src/parallels/parallels_driver.c | 7 +-
src/qemu/qemu_cgroup.c | 23 +-
src/qemu/qemu_driver.c | 84 +++---
src/qemu/qemu_process.c| 8 +-
src/util/virnuma.c | 48 ++--
src/util/virnuma.h | 2 +-
.../qemuxml2argv-numatune-auto-prefer.xml | 29 ++
.../qemuxml2xmlout-numatune-auto-prefer.xml| 29 ++
tests/qemuxml2xmltest.c| 2 +
18 files changed, 553 insertions(+), 285 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml
Nice.
Thanks :)
[...]
+tmp = virXMLPropString(node, nodeset);
+if (tmp virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN) 0)
+goto cleanup;
+VIR_FREE(tmp);
+
+if (virDomainNumatuneSet(def, placement, mode, nodeset) 0)
The virDomainNumatuneSet() takes a copy of @nodeset, so you need to call
virBitmaskFree(nodeset); at the cleanup label.
Yep, that happens when you change the behaviour of a function that
used to steal a pointer, in a rebase. Thanks!
+goto cleanup;
+
+if (!n) {
+ret = 0;
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(tmp);
+return ret;
+}
+
+int
+virDomainNumatuneFormatXML(virBufferPtr buf,
+ virDomainNumatunePtr numatune)
+{
+const char *tmp = NULL;
s /const// ..
+
+if (!numatune)
+return 0;
+
+virBufferAddLit(buf, numatune\n);
+virBufferAdjustIndent(buf, 2);
+
+tmp = virDomainNumatuneMemModeTypeToString(numatune-memory.mode);
+virBufferAsprintf(buf, memory mode='%s' , tmp);
+
+if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
+if (!(tmp = virBitmapFormat(numatune-memory.nodeset)))
+return -1;
+virBufferAsprintf(buf, nodeset='%s'/\n, tmp);
+VIR_FREE(tmp);
.. because free()-ing a const char * is not nice. If you, however, do
this I bet you'll get error in TypeToString(). So just leave tmp as
const char * and introduce char *nodeset;
I take the 'const' as a sign of the fact that I won't be modifying
any part of the string. Just adding 'const' to a pointer should be
perfectly OK, but I have not objections to your idea, so I squashed
this in:
diff --git i/src/conf/numatune_conf.c w/src/conf/numatune_conf.c
index 8b66558..375428c 100644
--- i/src/conf/numatune_conf.c
+++ w/src/conf/numatune_conf.c
@@ -141,6 +142,7 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
virDomainNumatunePtr numatune)
{
const char *tmp = NULL;
+char *nodeset = NULL;
if (!numatune)
return 0;
@@ -152,10 +154,10 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
virBufferAsprintf(buf, memory mode='%s' , tmp);
if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
-if (!(tmp = virBitmapFormat(numatune-memory.nodeset)))
+if (!(nodeset = virBitmapFormat(numatune-memory.nodeset)))
return -1;
virBufferAsprintf(buf, nodeset='%s'/\n, tmp);
-VIR_FREE(tmp);
+VIR_FREE(nodeset);
} else if (numatune-memory.placement) {
tmp =
virDomainNumatunePlacementTypeToString(numatune-memory.placement);
virBufferAsprintf(buf, placement='%s'/\n, tmp);
--
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew
On Tue, Jul 15, 2014 at 04:32:03PM +0800, Jincheng Miao wrote:
In virObjectLockableNew, when virMutexInit fails,
virReportSystemError should use errno to get the right
error number, instead of VIR_ERR_INTERNAL_ERROR.
Signed-off-by: Jincheng Miao jm...@redhat.com
---
src/util/virobject.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 6cb84b4..b5d2c02 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -220,7 +220,7 @@ void *virObjectLockableNew(virClassPtr klass)
return NULL;
if (virMutexInit(obj-lock) 0) {
-virReportSystemError(VIR_ERR_INTERNAL_ERROR, %s,
+virReportSystemError(errno, %s,
_(Unable to initialize mutex));
I'm not sure errno is set when using our virMutexInit(). Most of the
code uses virReportError instead, I suggest using that. This should
be changed everywhere in the code. Rough idea of the places could be
gotten by the following command:
git grep -nA5 virMutexInit | grep SystemError
but as I said, only rough idea :)
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] lxc: print ENOTSUP when usernamespace is not supported
On Tue, Jul 15, 2014 at 04:32:01PM +0800, Jincheng Miao wrote:
In lxcContainerStart, when user namespace is not supported,
the virReportSystemError is called. But the first argument
should be ENOTSUPP, instead of VIR_ERR_CONFIG_UNSUPPORTED.
Signed-off-by: Jincheng Miao jm...@redhat.com
---
src/lxc/lxc_container.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 4d89677..343df47 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2020,7 +2020,7 @@ int lxcContainerStart(virDomainDefPtr def,
VIR_DEBUG(Enable user namespace);
cflags |= CLONE_NEWUSER;
} else {
-virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+virReportSystemError(ENOTSUP, %s,
_(Kernel doesn't support user namespace));
VIR_FREE(stack);
return -1;
--
1.8.3.1
Using virReportSystemError() with anything else than 'errno' is a
misuse by my standards. Just use virReportError() instead if there's
VIR_ERR_CONFIG_UNSUPPORTED used.
Martin
P.S.: The same applies for following patches.
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 08/16] conf, schema: add support for memnode elements
On Fri, Jul 11, 2014 at 05:10:53PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 15 ++
docs/schemas/domaincommon.rng | 17 ++
src/conf/numatune_conf.c | 183 +++--
.../qemuxml2argv-numatune-memnode-no-memory.xml| 30
.../qemuxml2argv-numatune-memnode-nocpu.xml| 25 +++
.../qemuxml2argv-numatune-memnode.xml | 31
.../qemuxml2argv-numatune-memnodes-problematic.xml | 31
tests/qemuxml2argvtest.c | 2 +
.../qemuxml2xmlout-numatune-memnode.xml| 33
tests/qemuxml2xmltest.c| 2 +
10 files changed, 356 insertions(+), 13 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ad87b7c..d845c1b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -709,6 +709,8 @@
...
lt;numatunegt;
lt;memory mode=strict nodeset=1-4,^3/gt;
+lt;memnode cellid=0 mode=strict nodeset=1/gt;
+lt;memnode cellid=2 mode=preferred nodeset=2/gt;
lt;/numatunegt;
...
lt;/domaingt;
@@ -745,6 +747,19 @@
span class='since'Since 0.9.3/span
/dd
+ dtcodememnode/code/dt
+ dd
+Optional codememnode/code elements can specify memory allocation
+policies per each guest NUMA node. For those nodes having no
+corresponding codememnode/code element, the default from
+element codememory/code will be used. Attribute
codecellid/code
+addresses guest NUMA node for which the settings are applied.
+Attributes codemode/code and codenodeset/code have the same
+meaning and syntax as in codememory/code element.
+
+This setting is not compatible with automatic placement.
+span class='since'QEMU Since 1.2.6/span
1.2.8 actually
Are you suggesting I should wait yet another release or did you mean
1.2.7 by any chance? :)
+ /dd
/dl
+static int
+virDomainNumatuneNodeParseXML(virDomainDefPtr def,
+ xmlXPathContextPtr ctxt)
+{
+int n = 0;;
+int ret = -1;
+size_t i = 0;
+xmlNodePtr *nodes = NULL;
+
+if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes)) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot extract memnode nodes));
+goto cleanup;
+}
+
+if (!n)
+return 0;
+
+if (def-numatune def-numatune-memory.specified
+def-numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node binding is not compatible with
+ automatic NUMA placement.));
+goto cleanup;
+}
+
+if (!def-cpu || !def-cpu-ncells) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Element 'memnode' is invalid without
+ any guest NUMA cells));
+goto cleanup;
+}
+
+if (!def-numatune VIR_ALLOC(def-numatune) 0)
+goto cleanup;
Here you allow def-numatune to be allocated already.
+
+if (VIR_ALLOC_N(def-numatune-mem_nodes, def-cpu-ncells) 0)
+goto cleanup;
Which means, this can exists too. VIR_REALLOC_N() is safer IMO.
But that won't zero the memory. VIR_FREE(); VIR_ALLOC_N(); should
cover all cases.
+
+def-numatune-nmem_nodes = def-cpu-ncells;
+
+for (i = 0; i n; i++) {
+const char *tmp = NULL;
No. s/const//.
+int mode = 0;
+unsigned int cellid = 0;
+virDomainNumatuneNodePtr mem_node = NULL;
+xmlNodePtr cur_node = nodes[i];
+
+tmp = virXMLPropString(cur_node, cellid);
+if (!tmp) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Missing required cellid attribute
+ in memnode element));
+goto cleanup;
+}
+if (virStrToLong_uip(tmp, NULL, 10, cellid) 0) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Invalid cellid attribute in memnode element));
Moreover, @tmp is leaked here. And it would be nice to tell users in the error
message @tmp somehow.
You mean if the user has a typo in an integer? I guess that such user
has more issues that that typo then and needs more than that to make
his life easier
Re: [libvirt] [PATCH v2 09/16] numatune: add support for per-node memory bindings in private APIs
On Fri, Jul 11, 2014 at 05:11:02PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/numatune_conf.c | 111 ---
src/conf/numatune_conf.h | 14 --
src/libvirt_private.syms | 1 +
src/lxc/lxc_cgroup.c | 3 +-
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_driver.c | 10 ++---
src/util/virnuma.c | 6 +--
7 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 67fc799..60b6867 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -63,6 +63,18 @@ struct _virDomainNumatune {
};
+static inline bool
+numa_cell_specified(virDomainNumatunePtr numatune,
Whoa, when I met this function call I thought to myself that this is
some libnuma function. Please name it differently to match our
virSomethingShiny pattern.
I wanted this function to stand out as it is a macro (or static
inline) and I've seen it somewhere else in the code. I changed it to
virDomainNumatuneNodeSpecified(ewww, gross) and all the calls too
(with proper wrapping as well).
+int cellid)
+{
+if (numatune
+cellid = 0
+cellid numatune-nmem_nodes)
+return numatune-mem_nodes[cellid].nodeset;
+
+return false;
+}
+
static int
virDomainNumatuneNodeParseXML(virDomainDefPtr def,
xmlXPathContextPtr ctxt)
@@ -312,6 +324,7 @@ void
virDomainNumatuneFree(virDomainNumatunePtr numatune)
{
size_t i = 0;
+
This change is spurious. Either move it to 8/16 or drop this one.
Done.
[...]
@@ -469,6 +500,35 @@ virDomainNumatuneSet(virDomainDefPtr def,
return ret;
}
+static bool
+virDomainNumatuneNodesEqual(virDomainNumatunePtr n1,
+virDomainNumatunePtr n2)
+{
+size_t i = 0;
+
+if (n1-nmem_nodes != n2-nmem_nodes)
+return false;
+
+for (i = 0; i n1-nmem_nodes; i++) {
+virDomainNumatuneNodePtr nd1 = n1-mem_nodes[i];
+virDomainNumatuneNodePtr nd2 = n2-mem_nodes[i];
+
+if (!nd1-nodeset !nd2-nodeset)
+continue;
So if both are missing nodeset, they are considered equal? What if they
differ in mode?
Yes, because !nd-nodeset means there was no memnode/ with that
cellid. Therefore mode doesn't make sense (and is not set anyway).
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] conf, schema: add support for memnode elements
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 15 ++
docs/schemas/domaincommon.rng | 17 ++
src/conf/numatune_conf.c | 187 +++--
.../qemuxml2argv-numatune-memnode-no-memory.xml| 30
.../qemuxml2argv-numatune-memnode-nocpu.xml| 25 +++
.../qemuxml2argv-numatune-memnode.xml | 33
.../qemuxml2argv-numatune-memnodes-problematic.xml | 31
tests/qemuxml2argvtest.c | 2 +
.../qemuxml2xmlout-numatune-memnode.xml| 33
tests/qemuxml2xmltest.c| 2 +
10 files changed, 362 insertions(+), 13 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9f1082b..1301639 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -709,6 +709,8 @@
...
lt;numatunegt;
lt;memory mode=strict nodeset=1-4,^3/gt;
+lt;memnode cellid=0 mode=strict nodeset=1/gt;
+lt;memnode cellid=2 mode=preferred nodeset=2/gt;
lt;/numatunegt;
...
lt;/domaingt;
@@ -745,6 +747,19 @@
span class='since'Since 0.9.3/span
/dd
+ dtcodememnode/code/dt
+ dd
+Optional codememnode/code elements can specify memory allocation
+policies per each guest NUMA node. For those nodes having no
+corresponding codememnode/code element, the default from
+element codememory/code will be used. Attribute
codecellid/code
+addresses guest NUMA node for which the settings are applied.
+Attributes codemode/code and codenodeset/code have the same
+meaning and syntax as in codememory/code element.
+
+This setting is not compatible with automatic placement.
+span class='since'QEMU Since 1.2.7/span
+ /dd
/dl
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 155a33e..0b31261 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -789,6 +789,23 @@
/choice
/element
/optional
+ zeroOrMore
+element name=memnode
+ attribute name=cellid
+ref name=unsignedInt/
+ /attribute
+ attribute name=mode
+choice
+ valuestrict/value
+ valuepreferred/value
+ valueinterleave/value
+/choice
+ /attribute
+ attribute name='nodeset'
+ref name='cpuset'/
+ /attribute
+/element
+ /zeroOrMore
/element
/define
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 6ce1e2d..a39c028 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -42,17 +42,140 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
static,
auto);
+typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
+typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
+
struct _virDomainNumatune {
struct {
+bool specified;
virBitmapPtr nodeset;
virDomainNumatuneMemMode mode;
virDomainNumatunePlacement placement;
} memory; /* pinning for all the memory */
+struct _virDomainNumatuneNode {
+virBitmapPtr nodeset;
+virDomainNumatuneMemMode mode;
+} *mem_nodes; /* fine tuning per guest node */
+size_t nmem_nodes;
+
/* Future NUMA tuning related stuff should go here. */
};
+static int
+virDomainNumatuneNodeParseXML(virDomainDefPtr def,
+ xmlXPathContextPtr ctxt)
+{
+char *tmp = NULL;
+int n = 0;;
+int ret = -1;
+size_t i = 0;
+xmlNodePtr *nodes = NULL;
+
+if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes)) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot extract memnode nodes));
+goto cleanup;
+}
+
+if (!n)
+return 0;
+
+if (def-numatune def-numatune-memory.specified
+def-numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node binding is not compatible with
+ automatic NUMA placement.));
+goto cleanup;
+}
+
+if (!def-cpu || !def-cpu-ncells) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Element 'memnode' is invalid without
+ any guest NUMA cells));
+goto
[libvirt] [PATCH v2] qemu: pass numa node binding preferences to qemu
Currently, we only bind the whole QEMU domain to memory nodes
specified in nodemask altogether. That, however, doesn't make much
sense when one wants to control from where the memory for particular
guest nodes should be allocated. QEMU allows us to do that by
specifying 'host-nodes' parameter for the 'memory-backend-ram' object,
so let's use that.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c| 59 +-
.../qemuxml2argv-numatune-memnode-no-memory.args | 8 +++
.../qemuxml2argv-numatune-memnode.args | 11
tests/qemuxml2argvtest.c | 7 +++
4 files changed, 84 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f1dbb34..6235a74 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -150,6 +150,11 @@ VIR_ENUM_IMPL(qemuDomainFSDriver,
VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
NULL,
NULL);
+VIR_ENUM_DECL(qemuNumaPolicy)
+VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST,
+ bind,
+ preferred,
+ interleave);
/**
* qemuPhysIfaceConnect:
@@ -6383,13 +6388,23 @@ qemuBuildNumaArgStr(const virDomainDef *def,
size_t i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
+char *nodemask = NULL;
int ret = -1;
+if (virDomainNumatuneHasPerNodeBinding(def-numatune)
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node memory binding is not supported
+ with this QEMU));
+goto cleanup;
+}
+
for (i = 0; i def-cpu-ncells; i++) {
int cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024);
def-cpu-cells[i].mem = cellmem * 1024;
VIR_FREE(cpumask);
+VIR_FREE(nodemask);
if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
goto cleanup;
@@ -6402,6 +6417,43 @@ qemuBuildNumaArgStr(const virDomainDef *def,
goto cleanup;
}
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virDomainNumatuneMemMode mode;
+const char *policy = NULL;
+
+mode = virDomainNumatuneGetMode(def-numatune, i);
+policy = qemuNumaPolicyTypeToString(mode);
+
+virBufferAsprintf(buf,
memory-backend-ram,size=%dM,id=ram-node%zu,
+ cellmem, i);
+
+if (virDomainNumatuneMaybeFormatNodeset(def-numatune, NULL,
+nodemask, i) 0)
+goto cleanup;
+
+if (nodemask) {
+if (strchr(nodemask, ',')
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(disjoint NUMA node ranges are not
supported
+ with this QEMU));
+goto cleanup;
+}
+
+for (tmpmask = nodemask; tmpmask; tmpmask = next) {
+if ((next = strchr(tmpmask, ',')))
+*(next++) = '\0';
+virBufferAddLit(buf, ,host-nodes=);
+virBufferAdd(buf, tmpmask, -1);
+}
+
+virBufferAsprintf(buf, ,policy=%s, policy);
+}
+
+virCommandAddArg(cmd, -object);
+virCommandAddArgBuffer(cmd, buf);
+}
+
virCommandAddArg(cmd, -numa);
virBufferAsprintf(buf, node,nodeid=%zu, i);
@@ -6412,7 +6464,11 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virBufferAdd(buf, tmpmask, -1);
}
-virBufferAsprintf(buf, ,mem=%d, cellmem);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virBufferAsprintf(buf, ,memdev=ram-node%zu, i);
+} else {
+virBufferAsprintf(buf, ,mem=%d, cellmem);
+}
virCommandAddArgBuffer(cmd, buf);
}
@@ -6420,6 +6476,7 @@ qemuBuildNumaArgStr(const virDomainDef *def,
cleanup:
VIR_FREE(cpumask);
+VIR_FREE(nodemask);
virBufferFreeAndReset(buf);
return ret;
}
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
new file mode 100644
index 000..b0e274c
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
@@ -0,0 +1,8 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/kvm -S -M pc -m 64 -smp 2 \
+-object memory-backend-ram,size=32M,id=ram
Re: [libvirt] [PATCH 4/4] virFree: Check const correctness
On Tue, Jul 15, 2014 at 02:38:36PM +0200, Michal Privoznik wrote: Up to now it's possible to do something like this: const char *ptr; ptr = strdup(my example string); VIR_FREE(ptr); The problem is, const char * pointers should not be modified (and freeing them is kind of modification). We should avoid this. A little trick is used: assigning a const pointer into 'void *' triggers compiler warning about discarding 'const' qualifier from pointer. So the virFree() function gains new dummy argument, that is not touched anyhow, just fulfills the const correctness check duty. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/viralloc.c | 6 -- src/util/viralloc.h | 20 src/xenapi/xenapi_utils.c | 2 +- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/util/viralloc.c b/src/util/viralloc.c index be9f0fe..0134e67 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c [...] @@ -569,13 +569,15 @@ int virAllocVar(void *ptrptr, /** * virFree: + * @ptr: dummy pointer to check const correctness * @ptrptr: pointer to pointer for address of memory to be freed * * Release the chunk of memory in the pointer pointed to by * the 'ptrptr' variable. After release, 'ptrptr' will be * updated to point to NULL. */ -void virFree(void *ptrptr) +void virFree(void *ptr ATTRIBUTE_UNUSED, + void *ptrptr) What if you don't add another argument, but just change the void *ptrptr to void **ptrptr. Compiler shouldn't be mad about not knowing the size resulting of de-referencing ptrptr, you get the check you want and keep the macro without side-effects. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Fix condition value assignments in conditions
On Tue, Jul 15, 2014 at 02:59:50PM +0200, Ján Tomko wrote: Split into two patches, as we might want to backport the first one somewhere. Ján Tomko (2): Fix error on fs pool build failure Fix assignment of comparison against zero src/storage/storage_backend_fs.c | 2 +- tests/virnettlshelpers.c | 4 ++-- tools/virsh-network.c| 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) -- 1.8.5.5 ACK series. Maybe libvirt will now work after 4 years (the commit in 1/2) :) Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew
On Tue, Jul 15, 2014 at 10:47:04PM -0400, Jincheng Miao wrote:
- Original Message -
I'm not sure errno is set when using our virMutexInit(). Most of the
code uses virReportError instead, I suggest using that. This should
Actually, the implement of virMutexInit() already set errno when failure:
int virMutexInit(virMutexPtr m)
{
int ret;
pthread_mutexattr_t attr;
pthread_mutexattr_init(attr);
pthread_mutexattr_settype(attr, PTHREAD_MUTEX_NORMAL);
ret = pthread_mutex_init(m-lock, attr);
pthread_mutexattr_destroy(attr);
if (ret != 0) {
errno = ret;
return -1;
}
return 0;
}
Oh, sorry; /me facepalms...
You're right then, but if you want to change it everywhere in the code
(which I don't require), it would be better to create a
virMutexInitInternal that takes 2 parameters; the second being a bool
that controls error reporting and virMutexInit and virMutexInitQuiet
would be two macros. Similarly to virVasprintf for example.
be changed everywhere in the code. Rough idea of the places could be
I think it worthy of adding after all virMutexInit, I will include it in
my V2 patchset.
gotten by the following command:
git grep -nA5 virMutexInit | grep SystemError
but as I said, only rough idea :)
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 00/16] Support for per-guest-node binding
v3 of https://www.redhat.com/archives/libvir-list/2014-July/msg00372.html v3: - Michal's suggestions worked in - rebased on current master Martin Kletzander (16): qemu: purely a code movement qemu: remove useless error check conf: purely a code movement conf, schema: add 'id' field for cells numatune: create new module for numatune numatune: unify numatune struct and enum names numatune: Encapsulate numatune configuration in order to unify results conf, schema: add support for memnode elements numatune: add support for per-node memory bindings in private APIs qemu: allow qmp probing for cmdline options without params qemu: memory-backend-ram capability probing qemu: newer -numa parameter capability probing qemu: enable disjoint numa cpu ranges qemu: pass numa node binding preferences to qemu qemu: split out cpuset.mems setting qemu: leave restricting cpuset.mems after initialization docs/formatdomain.html.in | 32 +- docs/schemas/domaincommon.rng | 22 + po/POTFILES.in | 1 + src/Makefile.am| 3 +- src/conf/cpu_conf.c| 41 +- src/conf/cpu_conf.h| 3 +- src/conf/domain_conf.c | 203 ++- src/conf/domain_conf.h | 10 +- src/conf/numatune_conf.c | 595 + src/conf/numatune_conf.h | 108 src/libvirt_private.syms | 24 +- src/lxc/lxc_cgroup.c | 20 +- src/lxc/lxc_controller.c | 5 +- src/lxc/lxc_native.c | 15 +- src/parallels/parallels_driver.c | 7 +- src/qemu/qemu_capabilities.c | 16 +- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_cgroup.c | 52 +- src/qemu/qemu_cgroup.h | 4 +- src/qemu/qemu_command.c| 98 +++- src/qemu/qemu_driver.c | 84 ++- src/qemu/qemu_monitor.c| 6 +- src/qemu/qemu_monitor.h| 3 +- src/qemu/qemu_monitor_json.c | 8 +- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_process.c| 12 +- src/util/virnuma.c | 61 +-- src/util/virnuma.h | 28 +- tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 5 + tests/qemumonitorjsontest.c| 20 +- .../qemuxml2argv-cpu-numa-disjoint.args| 6 + .../qemuxml2argv-cpu-numa-disjoint.xml | 28 + tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 + .../qemuxml2argv-numatune-auto-prefer.xml | 29 + .../qemuxml2argv-numatune-memnode-no-memory.args | 8 + .../qemuxml2argv-numatune-memnode-no-memory.xml| 30 ++ .../qemuxml2argv-numatune-memnode-nocpu.xml| 25 + .../qemuxml2argv-numatune-memnode.args | 11 + .../qemuxml2argv-numatune-memnode.xml | 33 ++ .../qemuxml2argv-numatune-memnodes-problematic.xml | 31 ++ tests/qemuxml2argvtest.c | 12 + .../qemuxml2xmlout-cpu-numa1.xml | 28 + .../qemuxml2xmlout-cpu-numa2.xml | 28 + .../qemuxml2xmlout-numatune-auto-prefer.xml| 29 + .../qemuxml2xmlout-numatune-memnode.xml| 33 ++ tests/qemuxml2xmltest.c| 8 + 49 files changed, 1479 insertions(+), 389 deletions(-) create mode 100644 src/conf/numatune_conf.c create mode 100644 src/conf/numatune_conf.h create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-disjoint.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml create mode 100644 tests
[libvirt] [PATCH v3 04/16] conf, schema: add 'id' field for cells
In XML format, by definition, order of fields should not matter, so
order of parsing the elements doesn't affect the end result. When
specifying guest NUMA cells, we depend only on the order of the 'cell'
elements. With this patch all older domain XMLs are parsed as before,
but with the 'id' attribute they are parsed and formatted according to
that field. This will be useful when we have tuning settings for
particular guest NUMA node.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 17 +
docs/schemas/domaincommon.rng | 5 +++
src/conf/cpu_conf.c| 41 +++---
src/conf/cpu_conf.h| 3 +-
src/qemu/qemu_command.c| 2 +-
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 +
tests/qemuxml2argvtest.c | 1 +
.../qemuxml2xmlout-cpu-numa1.xml | 28 +++
.../qemuxml2xmlout-cpu-numa2.xml | 28 +++
tests/qemuxml2xmltest.c| 3 ++
12 files changed, 145 insertions(+), 20 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b69da4c..9f1082b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1030,8 +1030,8 @@
lt;cpugt;
...
lt;numagt;
- lt;cell cpus='0-3' memory='512000'/gt;
- lt;cell cpus='4-7' memory='512000'/gt;
+ lt;cell id='0' cpus='0-3' memory='512000'/gt;
+ lt;cell id='1' cpus='4-7' memory='512000'/gt;
lt;/numagt;
...
lt;/cpugt;
@@ -1039,10 +1039,15 @@
p
Each codecell/code element specifies a NUMA cell or a NUMA node.
- codecpus/code specifies the CPU or range of CPUs that are part of
- the node. codememory/code specifies the node memory in kibibytes
- (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
- or nodeid in the increasing order starting from 0.
+ codecpus/code specifies the CPU or range of CPUs that are
+ part of the node. codememory/code specifies the node memory
+ in kibibytes (i.e. blocks of 1024 bytes).
+ span class=sinceSince 1.2.7/span all cells should
+ have codeid/code attribute in case referring to some cell is
+ necessary in the code, otherwise the cells are
+ assigned codeid/codes in the increasing order starting from
+ 0. Mixing cells with and without the codeid/code attribute
+ is not recommended as it may result in unwanted behaviour.
/p
p
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7be028d..155a33e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3902,6 +3902,11 @@
define name=numaCell
element name=cell
+ optional
+attribute name=id
+ ref name=unsignedInt/
+/attribute
+ /optional
attribute name=cpus
ref name=cpuset/
/attribute
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 811893d..5003cf1 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -152,7 +152,6 @@ virCPUDefCopy(const virCPUDef *cpu)
copy-ncells_max = copy-ncells = cpu-ncells;
for (i = 0; i cpu-ncells; i++) {
-copy-cells[i].cellid = cpu-cells[i].cellid;
copy-cells[i].mem = cpu-cells[i].mem;
copy-cells[i].cpumask = virBitmapNewCopy(cpu-cells[i].cpumask);
@@ -438,17 +437,48 @@ virCPUDefParseXML(xmlNodePtr node,
for (i = 0; i n; i++) {
char *cpus, *memory;
int ret, ncpus = 0;
+unsigned int cur_cell;
+char *tmp = NULL;
+
+tmp = virXMLPropString(nodes[i], id);
+if (!tmp) {
+cur_cell = i;
+} else {
+ret = virStrToLong_ui(tmp, NULL, 10, cur_cell);
+if (ret == -1) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Invalid 'id' attribute in NUMA cell:
%s),
+ tmp);
+VIR_FREE(tmp);
+goto error;
+}
+VIR_FREE(tmp);
+}
+
+if (cur_cell = n) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Exactly one 'cell' element per guest
+ NUMA cell allowed, non-contiguous ranges or
+ ranges not starting from 0 are not
allowed));
+goto
[libvirt] [PATCH v3 03/16] conf: purely a code movement
to ease the review of commits to follow.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c | 52 +-
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1d83f13..315ea6a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11692,6 +11692,32 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(nodes);
+/* analysis of cpu handling */
+if ((node = virXPathNode(./cpu[1], ctxt)) != NULL) {
+xmlNodePtr oldnode = ctxt-node;
+ctxt-node = node;
+def-cpu = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_GUEST);
+ctxt-node = oldnode;
+
+if (def-cpu == NULL)
+goto error;
+
+if (def-cpu-sockets
+def-maxvcpus
+def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
+virReportError(VIR_ERR_XML_DETAIL, %s,
+ _(Maximum CPUs greater than topology limit));
+goto error;
+}
+
+if (def-cpu-cells_cpus def-maxvcpus) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Number of CPUs in numa exceeds the
+ vcpu count));
+goto error;
+}
+}
+
/* Extract numatune if exists. */
if ((n = virXPathNodeSet(./numatune, ctxt, nodes)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -12889,32 +12915,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
-/* analysis of cpu handling */
-if ((node = virXPathNode(./cpu[1], ctxt)) != NULL) {
-xmlNodePtr oldnode = ctxt-node;
-ctxt-node = node;
-def-cpu = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_GUEST);
-ctxt-node = oldnode;
-
-if (def-cpu == NULL)
-goto error;
-
-if (def-cpu-sockets
-def-maxvcpus
-def-cpu-sockets * def-cpu-cores * def-cpu-threads) {
-virReportError(VIR_ERR_XML_DETAIL, %s,
- _(Maximum CPUs greater than topology limit));
-goto error;
-}
-
-if (def-cpu-cells_cpus def-maxvcpus) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(Number of CPUs in numa exceeds the
- vcpu count));
-goto error;
-}
-}
-
if ((node = virXPathNode(./sysinfo[1], ctxt)) != NULL) {
xmlNodePtr oldnode = ctxt-node;
ctxt-node = node;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 10/16] qemu: allow qmp probing for cmdline options without params
That can be lately achieved with by having .param == NULL in the
virQEMUCapsCommandLineProps struct.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 10 --
src/qemu/qemu_monitor.c | 6 --
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 8 +++-
src/qemu/qemu_monitor_json.h | 3 ++-
tests/qemumonitorjsontest.c | 20 +---
6 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c5fef53..a8d4648 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2429,6 +2429,7 @@ static int
virQEMUCapsProbeQMPCommandLine(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon)
{
+bool found = false;
int nvalues;
char **values;
size_t i, j;
@@ -2436,10 +2437,15 @@ virQEMUCapsProbeQMPCommandLine(virQEMUCapsPtr qemuCaps,
for (i = 0; i ARRAY_CARDINALITY(virQEMUCapsCommandLine); i++) {
if ((nvalues = qemuMonitorGetCommandLineOptionParameters(mon,
virQEMUCapsCommandLine[i].option,
- values)) 0)
+ values,
+ found)) 0)
return -1;
+
+if (found !virQEMUCapsCommandLine[i].param)
+virQEMUCapsSet(qemuCaps, virQEMUCapsCommandLine[i].flag);
+
for (j = 0; j nvalues; j++) {
-if (STREQ(virQEMUCapsCommandLine[i].param, values[j])) {
+if (STREQ_NULLABLE(virQEMUCapsCommandLine[i].param, values[j])) {
virQEMUCapsSet(qemuCaps, virQEMUCapsCommandLine[i].flag);
break;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index db3dd73..3d9f87b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3659,7 +3659,8 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon,
int
qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr mon,
const char *option,
- char ***params)
+ char ***params,
+ bool *found)
{
VIR_DEBUG(mon=%p option=%s params=%p, mon, option, params);
@@ -3675,7 +3676,8 @@ qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr
mon,
return -1;
}
-return qemuMonitorJSONGetCommandLineOptionParameters(mon, option, params);
+return qemuMonitorJSONGetCommandLineOptionParameters(mon, option,
+ params, found);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8a23267..c3695f2 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -748,7 +748,8 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon,
char ***events);
int qemuMonitorGetCommandLineOptionParameters(qemuMonitorPtr mon,
const char *option,
- char ***params);
+ char ***params,
+ bool *found);
int qemuMonitorGetKVMState(qemuMonitorPtr mon,
bool *enabled,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bef6a14..a62c02f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4504,7 +4504,8 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon,
int
qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
const char *option,
- char ***params)
+ char ***params,
+ bool *found)
{
int ret;
virJSONValuePtr cmd = NULL;
@@ -4516,6 +4517,8 @@
qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
size_t i;
*params = NULL;
+if (found)
+*found = false;
/* query-command-line-options has fixed output for a given qemu
* binary; but since callers want to query parameters for one
@@ -4576,6 +4579,9 @@
qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
goto cleanup;
}
+if (found)
+*found = true;
+
if ((n = virJSONValueArraySize(data)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(query-command-line-options parameter data was not
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 385211c..5f6c846 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -332,7 +332,8 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr
[libvirt] [PATCH v3 02/16] qemu: remove useless error check
Excerpt from the virCommandAddArgBuffer() description: Correctly transfers memory errors or contents from buf to cmd. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_command.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4307f1f..9fc0778 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6412,9 +6412,6 @@ qemuBuildNumaArgStr(const virDomainDef *def, virCommandPtr cmd) virBufferAdd(buf, cpumask, -1); virBufferAsprintf(buf, ,mem=%d, cellmem); -if (virBufferCheckError(buf) 0) -goto cleanup; - virCommandAddArgBuffer(cmd, buf); } ret = 0; -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 11/16] qemu: memory-backend-ram capability probing
The numa patch series in qemu adds memory-backend-ram object type by
which we can tell whether we can use such objects.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a8d4648..c1a8947 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -260,6 +260,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
msg-timestamp,
active-commit,
change-backing-file,
+
+ memory-backend-ram, /* 170 */
);
@@ -1477,6 +1479,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ ich9-intel-hda, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA },
{ pvpanic, QEMU_CAPS_DEVICE_PANIC },
{ usb-kbd, QEMU_CAPS_DEVICE_USB_KBD },
+{ memory-backend-ram, QEMU_CAPS_OBJECT_MEMORY_RAM },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 99cf9ed..c661d5a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -209,6 +209,7 @@ typedef enum {
QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */
QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */
QEMU_CAPS_CHANGE_BACKING_FILE = 169, /* change name of backing file in
metadata */
+QEMU_CAPS_OBJECT_MEMORY_RAM = 170, /* -object memory-backend-ram */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 12/16] qemu: newer -numa parameter capability probing
When qemu switched to using OptsVisitor for -numa parameter, it did
two things in the same patch. One of them is that the numa parameter
is now visible in query-command-line-options, the second one is that
it enabled using disjoint cpu ranges for -numa specification. This
will be used in later patch.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps| 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 5 +
4 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c1a8947..07306e5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -262,6 +262,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
change-backing-file,
memory-backend-ram, /* 170 */
+ numa,
);
@@ -2426,6 +2427,7 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ boot-opts, reboot-timeout, QEMU_CAPS_REBOOT_TIMEOUT },
{ spice, disable-agent-file-xfer, QEMU_CAPS_SPICE_FILE_XFER_DISABLE },
{ msg, timestamp, QEMU_CAPS_MSG_TIMESTAMP },
+{ numa, NULL, QEMU_CAPS_NUMA },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c661d5a..4332633 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -210,6 +210,7 @@ typedef enum {
QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */
QEMU_CAPS_CHANGE_BACKING_FILE = 169, /* change name of backing file in
metadata */
QEMU_CAPS_OBJECT_MEMORY_RAM = 170, /* -object memory-backend-ram */
+QEMU_CAPS_NUMA = 171, /* newer -numa handling with disjoint
cpu ranges */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index 32bccdb..4b9f693 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -142,4 +142,5 @@
flag name='usb-kbd'/
flag name='host-pci-multidomain'/
flag name='msg-timestamp'/
+flag name='numa'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
index a60542a..ec7451f 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.replies
@@ -2389,6 +2389,11 @@
{
parameters: [
],
+option: numa
+},
+{
+parameters: [
+],
option: device
},
{
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 05/16] numatune: create new module for numatune
There are many places with numatune-related code that should be put
into special numatune_conf and this patch creates a basis for that.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/Makefile.am | 3 ++-
src/conf/domain_conf.h | 2 +-
src/conf/numatune_conf.c | 37 ++
src/conf/numatune_conf.h | 54
src/libvirt_private.syms | 12 ++
src/qemu/qemu_capabilities.c | 1 +
src/util/virnuma.c | 13 +--
src/util/virnuma.h | 26 ++---
8 files changed, 106 insertions(+), 42 deletions(-)
create mode 100644 src/conf/numatune_conf.c
create mode 100644 src/conf/numatune_conf.h
diff --git a/src/Makefile.am b/src/Makefile.am
index a40e63f..982f63d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -252,7 +252,8 @@ DOMAIN_CONF_SOURCES =
\
conf/domain_conf.c conf/domain_conf.h \
conf/domain_audit.c conf/domain_audit.h \
conf/domain_nwfilter.c conf/domain_nwfilter.h \
- conf/snapshot_conf.c conf/snapshot_conf.h
+ conf/snapshot_conf.c conf/snapshot_conf.h \
+ conf/numatune_conf.c conf/numatune_conf.h
OBJECT_EVENT_SOURCES = \
conf/object_event.c conf/object_event.h \
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 32674e0..fc7680c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -36,6 +36,7 @@
# include virhash.h
# include virsocketaddr.h
# include nwfilter_params.h
+# include numatune_conf.h
# include virnetdevmacvlan.h
# include virsysinfo.h
# include virnetdevvportprofile.h
@@ -46,7 +47,6 @@
# include device_conf.h
# include virbitmap.h
# include virstoragefile.h
-# include virnuma.h
# include virseclabel.h
/* forward declarations of all device types, required by
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
new file mode 100644
index 000..e9be040
--- /dev/null
+++ b/src/conf/numatune_conf.c
@@ -0,0 +1,37 @@
+/*
+ * numatune_conf.c
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Martin Kletzander mklet...@redhat.com
+ */
+
+#include config.h
+
+#include numatune_conf.h
+
+VIR_ENUM_IMPL(virDomainNumatuneMemMode,
+ VIR_DOMAIN_NUMATUNE_MEM_LAST,
+ strict,
+ preferred,
+ interleave);
+
+VIR_ENUM_IMPL(virNumaTuneMemPlacementMode,
+ VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_LAST,
+ default,
+ static,
+ auto);
diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
new file mode 100644
index 000..6bdfdc0
--- /dev/null
+++ b/src/conf/numatune_conf.h
@@ -0,0 +1,54 @@
+/*
+ * numatune_conf.h
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Author: Martin Kletzander mklet...@redhat.com
+ */
+
+#ifndef __NUMATUNE_CONF_H__
+# define __NUMATUNE_CONF_H__
+
+# include internal.h
+# include virutil.h
+# include virbitmap.h
+
+typedef enum {
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_DEFAULT = 0,
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC,
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO,
+
+VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_LAST
+} virDomainNumaTuneMemPlacementMode;
+
+VIR_ENUM_DECL(virNumaTuneMemPlacementMode)
+
+VIR_ENUM_DECL(virDomainNumatuneMemMode)
+
+typedef struct _virNumaTuneDef virNumaTuneDef;
+typedef virNumaTuneDef *virNumaTuneDefPtr;
+struct _virNumaTuneDef {
+struct {
+virBitmapPtr nodemask
[libvirt] [PATCH v3 09/16] numatune: add support for per-node memory bindings in private APIs
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/numatune_conf.c | 111 ---
src/conf/numatune_conf.h | 14 --
src/libvirt_private.syms | 1 +
src/lxc/lxc_cgroup.c | 3 +-
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_driver.c | 10 ++---
src/util/virnuma.c | 6 +--
7 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index a39c028..82418aa 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -63,6 +63,18 @@ struct _virDomainNumatune {
};
+static inline bool
+virDomainNumatuneNodeSpecified(virDomainNumatunePtr numatune,
+ int cellid)
+{
+if (numatune
+cellid = 0
+cellid numatune-nmem_nodes)
+return numatune-mem_nodes[cellid].nodeset;
+
+return false;
+}
+
static int
virDomainNumatuneNodeParseXML(virDomainDefPtr def,
xmlXPathContextPtr ctxt)
@@ -330,26 +342,37 @@ virDomainNumatuneFree(virDomainNumatunePtr numatune)
}
virDomainNumatuneMemMode
-virDomainNumatuneGetMode(virDomainNumatunePtr numatune)
+virDomainNumatuneGetMode(virDomainNumatunePtr numatune,
+ int cellid)
{
-return (numatune numatune-memory.specified) ? numatune-memory.mode :
0;
+if (!numatune)
+return 0;
+
+if (virDomainNumatuneNodeSpecified(numatune, cellid))
+return numatune-mem_nodes[cellid].mode;
+
+if (numatune-memory.specified)
+return numatune-memory.mode;
+
+return 0;
}
virBitmapPtr
virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune,
-virBitmapPtr auto_nodeset)
+virBitmapPtr auto_nodeset,
+int cellid)
{
if (!numatune)
return NULL;
-if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
+if (numatune-memory.specified
+numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
return auto_nodeset;
-/*
- * This weird logic has the same meaning as switch with
- * auto/static/default, but can be more readably changed later.
- */
-if (numatune-memory.placement != VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC)
+if (virDomainNumatuneNodeSpecified(numatune, cellid))
+return numatune-mem_nodes[cellid].nodeset;
+
+if (!numatune-memory.specified)
return NULL;
return numatune-memory.nodeset;
@@ -357,23 +380,31 @@ virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune,
char *
virDomainNumatuneFormatNodeset(virDomainNumatunePtr numatune,
- virBitmapPtr auto_nodeset)
+ virBitmapPtr auto_nodeset,
+ int cellid)
{
return virBitmapFormat(virDomainNumatuneGetNodeset(numatune,
- auto_nodeset));
+ auto_nodeset,
+ cellid));
}
int
virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune,
virBitmapPtr auto_nodeset,
-char **mask)
+char **mask,
+int cellid)
{
*mask = NULL;
if (!numatune)
return 0;
-if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO
+if (!virDomainNumatuneNodeSpecified(numatune, cellid)
+!numatune-memory.specified)
+return 0;
+
+if (numatune-memory.specified
+numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO
!auto_nodeset) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Advice from numad is needed in case of
@@ -381,7 +412,7 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr
numatune,
return -1;
}
-*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset);
+*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset, cellid);
if (!*mask)
return -1;
@@ -475,6 +506,35 @@ virDomainNumatuneSet(virDomainDefPtr def,
return ret;
}
+static bool
+virDomainNumatuneNodesEqual(virDomainNumatunePtr n1,
+virDomainNumatunePtr n2)
+{
+size_t i = 0;
+
+if (n1-nmem_nodes != n2-nmem_nodes)
+return false;
+
+for (i = 0; i n1-nmem_nodes; i++) {
+virDomainNumatuneNodePtr nd1 = n1-mem_nodes[i];
+virDomainNumatuneNodePtr nd2 = n2-mem_nodes[i];
+
+if (!nd1-nodeset !nd2-nodeset)
+continue;
+
+if (!nd1-nodeset || !nd2-nodeset)
+return false;
+
+if (nd1-mode != nd2-mode)
+return false;
+
+if (!virBitmapEqual(nd1-nodeset, nd2-nodeset))
+return false;
+}
+
+return true;
+}
+
bool
