7:39 a.m.
*** BLURB HERE ***
Michal Prívozník (2):
conf: Forbid hugepages and <source type='file'/>
conf: Forbid hugepages and <allocation mode="ondemand"/>
src/conf/domain_conf.c | 25 +++++++++++++++++--
.../qemuxml2argvdata/hugepages-memaccess2.xml | 1 -
2 files changed, 23 insertions(+), 3 deletions(-)
--
2.18.1
7:39 a.m.
New subject: [libvirt] [PATCH 1/2] conf: Forbid hugepages and <source type='file'/>
https://bugzilla.redhat.com/show_bug.cgi?id=1633562
Under <memoryBacking/> we allow users to configure various memory
backing related knobs. However, there are some combinations that
make no sense. For instance, requesting hugepages and file
allocation at the same time. Forbid this configuration then.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 16 ++++++++++++++--
tests/qemuxml2argvdata/hugepages-memaccess2.xml | 1 -
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9911d56130..ef1d5caa1c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4041,7 +4041,7 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
}
-static void
+static int
virDomainDefPostParseMemtune(virDomainDefPtr def)
{
size_t i;
@@ -4063,6 +4063,17 @@ virDomainDefPostParseMemtune(virDomainDefPtr def)
}
}
}
+
+ if (def->mem.nhugepages &&
+ def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_NONE &&
+ def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unsupported combination of hugepages and source type
%s"),
+ virDomainMemorySourceTypeToString(def->mem.source));
+ return -1;
+ }
+
+ return 0;
}
@@ -5131,7 +5142,8 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
if (virDomainDefPostParseMemory(def, data->parseFlags) < 0)
return -1;
- virDomainDefPostParseMemtune(def);
+ if (virDomainDefPostParseMemtune(def) < 0)
+ return -1;
if (virDomainDefRejectDuplicateControllers(def) < 0)
return -1;
diff --git a/tests/qemuxml2argvdata/hugepages-memaccess2.xml
b/tests/qemuxml2argvdata/hugepages-memaccess2.xml
index 205f9efd92..e7f60291be 100644
--- a/tests/qemuxml2argvdata/hugepages-memaccess2.xml
+++ b/tests/qemuxml2argvdata/hugepages-memaccess2.xml
@@ -8,7 +8,6 @@
<hugepages>
<page size='2048' unit='KiB' nodeset='1'/>
</hugepages>
- <source type='file'/>
<access mode='private'/>
</memoryBacking>
<vcpu placement='static'>4</vcpu>
--
2.18.1
8:17 a.m.
New subject: [libvirt] [PATCH 1/2] conf: Forbid hugepages and <source type='file'/>
On Mon, Oct 15, 2018 at 02:39:26PM +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1633562
Under <memoryBacking/> we allow users to configure various memory
backing related knobs. However, there are some combinations that
make no sense. For instance, requesting hugepages and file
allocation at the same time. Forbid this configuration then.
The huge pages we allocate come from a file in /dev/hugepages,
so this description doesn't really make sense.
IIUC, what's actually happening is that you're trying to avoid
a historical bug.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 16 ++++++++++++++--
tests/qemuxml2argvdata/hugepages-memaccess2.xml | 1 -
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9911d56130..ef1d5caa1c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4041,7 +4041,7 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
}
-static void
+static int
virDomainDefPostParseMemtune(virDomainDefPtr def)
{
size_t i;
@@ -4063,6 +4063,17 @@ virDomainDefPostParseMemtune(virDomainDefPtr def)
}
}
}
+
+ if (def->mem.nhugepages &&
+ def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_NONE &&
+ def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unsupported combination of hugepages and source type
%s"),
+ virDomainMemorySourceTypeToString(def->mem.source));
+ return -1;
+ }
+
+ return 0;
}
@@ -5131,7 +5142,8 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
if (virDomainDefPostParseMemory(def, data->parseFlags) < 0)
return -1;
- virDomainDefPostParseMemtune(def);
+ if (virDomainDefPostParseMemtune(def) < 0)
+ return -1;
if (virDomainDefRejectDuplicateControllers(def) < 0)
return -1;
diff --git a/tests/qemuxml2argvdata/hugepages-memaccess2.xml
b/tests/qemuxml2argvdata/hugepages-memaccess2.xml
index 205f9efd92..e7f60291be 100644
--- a/tests/qemuxml2argvdata/hugepages-memaccess2.xml
+++ b/tests/qemuxml2argvdata/hugepages-memaccess2.xml
@@ -8,7 +8,6 @@
<hugepages>
<page size='2048' unit='KiB' nodeset='1'/>
</hugepages>
- <source type='file'/>
<access mode='private'/>
</memoryBacking>
<vcpu placement='static'>4</vcpu>
--
2.18.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
8:47 a.m.
New subject: [libvirt] [PATCH 1/2] conf: Forbid hugepages and <source type='file'/>
On 10/15/2018 03:17 PM, Daniel P. Berrangé wrote:
On Mon, Oct 15, 2018 at 02:39:26PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1633562
>
> Under <memoryBacking/> we allow users to configure various memory
> backing related knobs. However, there are some combinations that
> make no sense. For instance, requesting hugepages and file
> allocation at the same time. Forbid this configuration then.
The huge pages we allocate come from a file in /dev/hugepages,
so this description doesn't really make sense.
Sure, in the end the mmap() that qemu calls is a file somewhere under
/dev/hugepages (btw terrible, really terrible path for hugetlbfs mount
point because it suggests hugepages are devices).
But <source type='file'/> is usually used to put domain RAM under
/var/lib/libvirt/ram/.. (and to enforce memory-backing-file).
IIUC, what's actually happening is that you're trying to avoid
a historical bug.
More or less. This basically a result of 992bf863fcc.
Anyway, might as well do nothing on this front :-)
Michal
7:39 a.m.
New subject: [libvirt] [PATCH 2/2] conf: Forbid hugepages and <allocation mode="ondemand"/>
https://bugzilla.redhat.com/show_bug.cgi?id=1633562
Under <memoryBacking/> we allow users to configure various memory
backing related knobs. However, there are some combinations that
make no sense. For instance, requesting hugepages and 'ondemand'
allocation at the same time. Forbid this configuration then.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ef1d5caa1c..53cb4209d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4064,13 +4064,22 @@ virDomainDefPostParseMemtune(virDomainDefPtr def)
}
}
- if (def->mem.nhugepages &&
- def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_NONE &&
- def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS) {
- virReportError(VIR_ERR_XML_ERROR,
- _("unsupported combination of hugepages and source type
%s"),
- virDomainMemorySourceTypeToString(def->mem.source));
- return -1;
+ if (def->mem.nhugepages) {
+ if (def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_NONE &&
+ def->mem.source != VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unsupported combination of hugepages and source type
%s"),
+ virDomainMemorySourceTypeToString(def->mem.source));
+ return -1;
+ }
+
+ if (def->mem.allocation != VIR_DOMAIN_MEMORY_ALLOCATION_NONE &&
+ def->mem.allocation != VIR_DOMAIN_MEMORY_ALLOCATION_IMMEDIATE) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unsupported combination of hugepages and allocation
%s"),
+
virDomainMemoryAllocationTypeToString(def->mem.allocation));
+ return -1;
+ }
}
return 0;
--
2.18.1
10:40 a.m.
On 10/15/18 8:39 AM, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (2):
conf: Forbid hugepages and <source type='file'/>
conf: Forbid hugepages and <allocation mode="ondemand"/>
src/conf/domain_conf.c | 25 +++++++++++++++++--
.../qemuxml2argvdata/hugepages-memaccess2.xml | 1 -
2 files changed, 23 insertions(+), 3 deletions(-)
It wasn't clear from the existing review comment whether you were going
to update; however, the one thing I'd point out is that by changing Post
Parse processing to add some XML check/condition that was previously
accepted could theoretically cause a guest to disappear after libvirtd
restart, right?
Thus shouldn't the changes go in the Validate processing?
John
2207
days inactive
2230
days old
5 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
John Ferlan -
Michal Privoznik