Re: [libvirt] [PATCH] Support configuration of huge pages in guests
On Wed, 2009-08-26 at 14:03 +0100, Daniel P. Berrange wrote:
I hope you're planning on adding all that useful info from the 0/1 mail
to the git commit log?
* configure.in: Add check for mntent.h
* qemud/libvirtd_qemu.aug, qemud/test_libvirtd_qemu.aug, src/qemu.conf
Add 'hugetlbfs_mount' config parameter
* src/qemu_conf.c, src/qemu_conf.h: Check for -mem-path flag in QEMU,
and pass it when hugepages are requested.
Load hugetlbfs_mount config parameter, search for mount if not given.
* src/qemu_driver.c: Free hugetlbfs_mount/path parameter in driver shutdown.
Create directory for QEMU hugepage usage, chowning if required.
* docs/formatdomain.html.in: Document memoryBacking/hugepages elements
* docs/schemas/domain.rng: Add memoryBacking/hugepages elements to schema
* src/util.c, src/util.h, src/libvirt_private.syms: Add virFileFindMountPoint
helper API
* tests/qemuhelptest.c: Add -mem-path constants
* tests/qemuxml2argvtest.c, tests/qemuxml2xmltest.c: Add tests for hugepage
handling
* tests/qemuxml2argvdata/qemuxml2argv-hugepages.xml,
tests/qemuxml2argvdata/qemuxml2argv-hugepages.args: Data files for
hugepage tests
---
configure.in |2 +-
docs/formatdomain.html |8 +++-
docs/formatdomain.html.in |8 +++
docs/schemas/domain.rng|9
qemud/libvirtd_qemu.aug|1 +
qemud/test_libvirtd_qemu.aug |4 ++
src/domain_conf.c | 10 -
src/domain_conf.h |1 +
src/libvirt_private.syms |1 +
src/qemu.conf | 13 +
src/qemu_conf.c| 49
src/qemu_conf.h|3 +
src/qemu_driver.c | 34 ++
src/util.c | 37 ++-
src/util.h |4 ++
tests/qemuhelptest.c |6 ++-
tests/qemuxml2argvdata/qemuxml2argv-hugepages.args |1 +
tests/qemuxml2argvdata/qemuxml2argv-hugepages.xml | 25 ++
tests/qemuxml2argvtest.c |7 ++-
tests/qemuxml2xmltest.c|1 +
20 files changed, 217 insertions(+), 7 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages.xml
...
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index 22f5edd..4935e2f 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -35,6 +35,7 @@
#include sys/wait.h
#include arpa/inet.h
#include sys/utsname.h
+#include mntent.h
#include c-ctype.h
#include virterror_internal.h
@@ -87,6 +88,7 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
NULL, /* no arg needed for xen */
NULL /* don't support vbox */);
+#define PROC_MOUNT_BUF_LEN 255
int qemudLoadDriverConfig(struct qemud_driver *driver,
const char *filename) {
@@ -106,6 +108,19 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
return -1;
}
+/* For privilege driver, try and find hugepage mount automatically.
privileged
+ * Non-privileged driver requires admin to create a dir for the
+ * user, chown it, and then let user configure it manually */
+if (driver-privileged
+!(driver-hugetlbfs_mount = virFileFindMountPoint(hugetlbfs))) {
+if (errno != ENOENT) {
+virReportSystemError(NULL, errno, %s,
+ _(unable to find hugetlbfs mountpoint));
+return -1;
+}
+}
+
+
/* Just check the file is readable before opening it, otherwise
* libvirt emits an error.
*/
@@ -290,10 +305,22 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
}
}
+ p = virConfGetValue (conf, hugetlbfs_mount);
+ CHECK_TYPE (hugetlbfs_mount, VIR_CONF_STRING);
+ if (p p-str) {
+ VIR_FREE(driver-hugetlbfs_mount);
+ if (!(driver-hugetlbfs_mount = strdup(p-str))) {
+ virReportOOMError(NULL);
+ virConfFree(conf);
+ return -1;
+ }
+ }
+
How come you probe for a hugetlbfs mount even when the config file
contains it?
virConfFree (conf);
return 0;
}
+
Spurious
...
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index eb22940..4140a63 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -542,6 +542,38 @@ qemudStartup(int privileged) {
goto error;
}
+/* If hugetlbfs is present, then we need to create a sub-directory within
+ * it, since we
Re: [libvirt] [PATCH] Support configuration of huge pages in guests
On Thu, Sep 03, 2009 at 11:48:53AM +0100, Mark McLoughlin wrote:
@@ -290,10 +305,22 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
}
}
+ p = virConfGetValue (conf, hugetlbfs_mount);
+ CHECK_TYPE (hugetlbfs_mount, VIR_CONF_STRING);
+ if (p p-str) {
+ VIR_FREE(driver-hugetlbfs_mount);
+ if (!(driver-hugetlbfs_mount = strdup(p-str))) {
+ virReportOOMError(NULL);
+ virConfFree(conf);
+ return -1;
+ }
+ }
+
How come you probe for a hugetlbfs mount even when the config file
contains it?
That's just the most convenient way with the way the config file
loading is structured. The previous patch of John's had the
probing down in this part of the code in the else {} clause
here. The trouble with that is that if there is no config file
on disk at all, it'll never be run. Moving it to the top
ensures its always got a sensible default.
diff --git a/src/util.c b/src/util.c
index 0d4f3fa..35efee2 100644
--- a/src/util.c
+++ b/src/util.c
@@ -60,7 +60,9 @@
#if HAVE_CAPNG
#include cap-ng.h
#endif
-
+#ifdef HAVE_MNTENT_H
+#include mntent.h
+#endif
#include virterror_internal.h
#include logging.h
@@ -1983,3 +1985,36 @@ int virGetGroupID(virConnectPtr conn,
return 0;
}
#endif
+
+
+#ifdef HAVE_MNTENT_H
Hmm, if mntent.h isn't found, the qemu driver will fail to link
Is the idea here that anywhere mntent.h isn't available, the qemu driver
won't be built so we don't need to check HAVE_MNTENT_H in qemu_conf.c?
That's an oversight - should have conditionalized the caller, though
I wouldn't be surprisd if QEMU driver is already broken on non-UNIX
since I don't think anyone's ever really tested it. Not that I ever
really expect people to use Windows for the QEMU driver itself
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Support configuration of huge pages in guests
On Wed, Aug 26, 2009 at 02:03:07PM +0100, Daniel P. Berrange wrote:
[...]
+if (def-hugepage_backed) {
+if (!driver-hugetlbfs_mount) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ %s, _(hugetlbfs filesystem is not mounted));
+goto error;
+}
+if (!driver-hugepage_path) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ %s, _(hugepages are disabled by
administrator config));
+goto error;
+}
+if (!(qemuCmdFlags QEMUD_CMD_FLAG_MEM_PATH)) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(hugepage backing not supported by '%s'),
+ def-emulator);
+goto error;
+}
+ADD_ARG_LIT(-mem-path);
+ADD_ARG_LIT(driver-hugepage_path);
+}
Hum, I wonder why hugepage, which IMHO are just a performance improvement,
should break guest startup if asked and not available for some reason.
Maybe it's just fine to emit the error here but allow the guest to
continue. Another question I have is about migration, suppose you
migrate from a host where hugepage are allowed and the domain makes use
of them, what's going on if you try to migrate to a destination where
it's not available. Should that be added in some way to migration
prerequisite checkings (if KVM doesn't already take care of it).
ADD_ARG_LIT(-smp);
ADD_ARG_LIT(vcpus);
Looks fine, I'm also wondering what's happening if HAVE_MNTENT_H
is not found, it's C99 but ...
ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Support configuration of huge pages in guests
Daniel P. Berrange wrote: * configure.in: Add check for mntent.h * qemud/libvirtd_qemu.aug, qemud/test_libvirtd_qemu.aug, src/qemu.conf Add 'hugetlbfs_mount' config parameter * src/qemu_conf.c, src/qemu_conf.h: Check for -mem-path flag in QEMU, and pass it when hugepages are requested. Load hugetlbfs_mount config parameter, search for mount if not given. * src/qemu_driver.c: Free hugetlbfs_mount/path parameter in driver shutdown. Create directory for QEMU hugepage usage, chowning if required. * docs/formatdomain.html.in: Document memoryBacking/hugepages elements * docs/schemas/domain.rng: Add memoryBacking/hugepages elements to schema * src/util.c, src/util.h, src/libvirt_private.syms: Add virFileFindMountPoint helper API * tests/qemuhelptest.c: Add -mem-path constants * tests/qemuxml2argvtest.c, tests/qemuxml2xmltest.c: Add tests for hugepage handling * tests/qemuxml2argvdata/qemuxml2argv-hugepages.xml, tests/qemuxml2argvdata/qemuxml2argv-hugepages.args: Data files for hugepage tests --- configure.in |2 +- docs/formatdomain.html |8 +++- docs/formatdomain.html.in |8 +++ docs/schemas/domain.rng|9 qemud/libvirtd_qemu.aug|1 + qemud/test_libvirtd_qemu.aug |4 ++ src/domain_conf.c | 10 - src/domain_conf.h |1 + src/libvirt_private.syms |1 + src/qemu.conf | 13 + src/qemu_conf.c| 49 src/qemu_conf.h|3 + src/qemu_driver.c | 34 ++ src/util.c | 37 ++- src/util.h |4 ++ tests/qemuhelptest.c |6 ++- tests/qemuxml2argvdata/qemuxml2argv-hugepages.args |1 + tests/qemuxml2argvdata/qemuxml2argv-hugepages.xml | 25 ++ tests/qemuxml2argvtest.c |7 ++- tests/qemuxml2xmltest.c|1 + 20 files changed, 217 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages.xml I did a fetch on libvirt.git before reviewing and it appears there is some code motion relative to the version this patch was against. Although AFAICT nothing which appears to result in more than patch bounce. Looks good to me. ACK. -john -- john.coo...@redhat.com -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
