On Mon, Jan 29, 2024 at 04:43:53PM -0500, mgalaxy@xxxxxxxxxx wrote:
From: Michael Galaxy <mgalaxy@xxxxxxxxxx> We start by introducing a backwards-compatible, comma-separated specification that will not break existing installations, such as in the following example: $ cat qemu.conf | grep memory_backing_dir memory_backing_dir = "/path/to/pmem/0,/path/to/pmem/1"
This would be better as an array, but I understand you thought it would be easier to code for backward compatibility.
In our case, we almost always have two NUMA nodes, so in that example, we have two PMEM regions which are created on the Linux kernel command line that get mounted into those two locations for libvirt to use.
There are PMEM devices which you then expose as filesystems to use for libvirt as a backing for VM's PMEMs. Do I understand that correctly? If yes, how are these different? Can't they be passed through?
Later patches will recognize this configuration and activate it.
Signed-off-by: Michael Galaxy <mgalaxy@xxxxxxxxxx>
---
src/qemu/qemu_conf.c | 39 +++++++++++++++++++++++++++++++++------
src/qemu/qemu_conf.h | 3 ++-
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 4050a82341..aae9f316d8 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -43,6 +43,7 @@
#include "virfile.h"
#include "virstring.h"
#include "virutil.h"
+#include "virnuma.h"
#include "configmake.h"
#include "security/security_util.h"
@@ -137,6 +138,9 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
cfg->cgroupControllers = -1; /* -1 == auto-detect */
+ cfg->memoryBackingDirs = g_new0(char *, 1);
+ cfg->nb_memoryBackingDirs = 1;
+
if (root != NULL) {
cfg->logDir = g_strdup_printf("%s/log/qemu", root);
cfg->swtpmLogDir = g_strdup_printf("%s/log/swtpm", root);
@@ -153,7 +157,7 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
cfg->checkpointDir = g_strdup_printf("%s/checkpoint", cfg->libDir);
cfg->autoDumpPath = g_strdup_printf("%s/dump", cfg->libDir);
cfg->nvramDir = g_strdup_printf("%s/nvram", cfg->libDir);
- cfg->memoryBackingDir = g_strdup_printf("%s/ram", cfg->libDir);
+ cfg->memoryBackingDirs[0] = g_strdup_printf("%s/ram", cfg->libDir);
} else if (privileged) {
cfg->logDir = g_strdup_printf("%s/log/libvirt/qemu", LOCALSTATEDIR);
@@ -174,7 +178,8 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
cfg->checkpointDir = g_strdup_printf("%s/checkpoint", cfg->libDir);
cfg->autoDumpPath = g_strdup_printf("%s/dump", cfg->libDir);
cfg->nvramDir = g_strdup_printf("%s/nvram", cfg->libDir);
- cfg->memoryBackingDir = g_strdup_printf("%s/ram", cfg->libDir);
+
+ cfg->memoryBackingDirs[0] = g_strdup_printf("%s/ram", cfg->libDir);
cfg->swtpmStorageDir = g_strdup_printf("%s/lib/libvirt/swtpm",
LOCALSTATEDIR);
} else {
@@ -201,7 +206,7 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
cfg->configBaseDir);
cfg->autoDumpPath = g_strdup_printf("%s/qemu/dump", cfg->configBaseDir);
cfg->nvramDir = g_strdup_printf("%s/qemu/nvram", cfg->configBaseDir);
- cfg->memoryBackingDir = g_strdup_printf("%s/qemu/ram", cfg->configBaseDir);
+ cfg->memoryBackingDirs[0] = g_strdup_printf("%s/qemu/ram", cfg->configBaseDir);
cfg->swtpmStorageDir = g_strdup_printf("%s/qemu/swtpm",
cfg->configBaseDir);
}
@@ -369,7 +374,12 @@ static void virQEMUDriverConfigDispose(void *obj)
virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
- g_free(cfg->memoryBackingDir);
+ for (size_t i = 0; i < cfg->nb_memoryBackingDirs; i++) {
We don't allow variable declarations in loops, make sure you run our test suite, more tips are available here: https://libvirt.org/hacking.html
+ g_free(cfg->memoryBackingDirs[i]);
TAB in indentation
+ }
Extra braces around one-line body, make sure you follow our coding style, linked from the above article, available here: https://libvirt.org/coding-style.html
+
+ g_free(cfg->memoryBackingDirs);
+
g_free(cfg->swtpmStorageDir);
g_strfreev(cfg->capabilityfilters);
@@ -1019,14 +1029,31 @@ virQEMUDriverConfigLoadMemoryEntry(virQEMUDriverConfig *cfg,
virConf *conf)
{
g_autofree char *dir = NULL;
+ g_auto(GStrv) params = NULL;
+ GStrv next;
int rc;
if ((rc = virConfGetValueString(conf, "memory_backing_dir", &dir)) < 0)
return -1;
if (rc > 0) {
- VIR_FREE(cfg->memoryBackingDir);
- cfg->memoryBackingDir = g_strdup_printf("%s/libvirt/qemu", dir);
+ size_t i;
+ for (i = 0; i < cfg->nb_memoryBackingDirs; i++)
+ VIR_FREE(cfg->memoryBackingDirs[i]);
Please don't mix the old VIR_FREE() and new g_free() as described in our glib adoption page: https://www.libvirt.org/glib-adoption.html
+
+ if (!(params = g_strsplit(dir, ",", 0))) {
+ return -1;
+ }
+
+ cfg->nb_memoryBackingDirs = 0;
+ for (next = params; *next; next++) {
+ cfg->nb_memoryBackingDirs++;
+ }
g_strv_length()?
+
+ cfg->memoryBackingDirs = g_new0(char *, cfg->nb_memoryBackingDirs);
+ for (i = 0, next = params; *next; next++, i++) {
+ cfg->memoryBackingDirs[i] = g_strdup_printf("%s/libvirt/qemu", *next);
+ }
You can do this with one loop, the extra allocation at the start of the program won't ever hurt. Also if you stored the values as GStrv, you could then work with it using g_strv*() functions. See glib-adoption linked above.
}
return 0;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 36049b4bfa..2b8d540df0 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -221,7 +221,8 @@ struct _virQEMUDriverConfig {
unsigned int glusterDebugLevel;
bool virtiofsdDebug;
- char *memoryBackingDir;
+ char **memoryBackingDirs;
+ size_t nb_memoryBackingDirs;
You change this here, but the other uses of the old member is fixed in following patches. I like that you split the series, but the tree should be buildable (with passing tests) after each and every commit. On top of all this, I still don't understand what's the benefit of having multiple backing dirs, so please first enlighten me with that as I'm clearly missing some info.
uid_t swtpm_user;
gid_t swtpm_group;
--
2.25.1
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
