On Wed, 2020-03-25 at 11:18 +0100, Michal Privoznik wrote:
[...]
> void
> -qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg,
> +qemuGetMemoryBackingBasePath(virQEMUDriverPtr driver,
> char **path)
> {
> - *path = g_strdup_printf("%s/libvirt/qemu", cfg->memoryBackingDir);
> + const char *root = virQEMUDriverGetEmbedRoot(driver);
> + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> +
> + if (root) {
> + g_autofree char * hash = virDomainDriverHashRoot(QEMU_DRIVER_NAME, root);
> + *path = g_strdup_printf("%s/libvirt/%s", cfg->memoryBackingDir, hash);
> + } else {
> + *path = g_strdup_printf("%s/libvirt/qemu", cfg->memoryBackingDir);
> + }
I'll reply here for convenience, but the same comments apply to the
previous and following patches as well.
As anticipated during our earlier discussion, this naive approach
solves the issue but it's a bit crude.
It work well enough when memoryBackingDir is configured to something
like /dev/shm, but if you leave that setting alone and use the
default instead, the generated path will go from
$root/lib/qemu/ram/libvirt/qemu/$domid-$domname/ram-node0
to
$root/lib/qemu/ram/libvirt/QEMU-embed-$roothash/$domid-$domname/ram-node0
which means that we're unnecessarily repeating the disambiguation
information twice. I think we need to be smarter than that, and only
use the hashed version when memoryBackingDir is outside of the
embedded root.
Two additional nits:
* QEMU_DRIVER_NAME is defined as the uppercase string "QEMU", so
the hash ends up not matching the one used elsewhere;
* the default non-embedded path looks like
/var/lib/libvirt/qemu/ram/libvirt/qemu/$domid-$domname/ram-node0
where both "libvirt" and "qemu" are repeated twice... While not
a functional issue per se, that looks like it should also be
addressed, don't you think?
--
Andrea Bolognani / Red Hat / Virtualization
