Re: [PATCH v2 1/4] qemu.conf changes to support multiple memory backend

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 06, 2024 at 03:45:54PM -0500, Michael Galaxy wrote:
Thank you so much for the detailed comments. I'll work on those.

I believe you had a couple questions below on this one......inlined .....

On 8/6/24 07:35, Martin Kletzander wrote:
On Wed, Jun 05, 2024 at 04:37:35PM -0400, mgalaxy@xxxxxxxxxx wrote:
From: Michael Galaxy <mgalaxy@xxxxxxxxxx>

+                               _("Domain requesting configuration
for %1$lu NUMA nodes, but memory backing directory only has (%2$lu)
directory paths available. Either reduce this to one directory or
provide more paths to use."),
+                               numa_node_count,
+                               cfg->nb_memoryBackingDirs);
+                return -1;
+            }
+
+            path_index = virBitmapNextSetBit(numaBitmap, -1);

What if the single pinning is to the third host node, but there are only
two memoryBackingDirs supplied in the config?

That case is totally supported (and expected). For example a *non* vNUMA
guests (a single virtual NUMA node)
running on a standard 2-socket (2-NUMA-node) host in a typical pizza box.

If I understood that correctly, that is supported without a problem. The
error above is describing the case that
a virtual machine has *more* virtual NUMA nodes configured than the host
has available to offer. In such a case,
either the user needs to provide more host nodes or turn this feature
off or use a single NUMA node (in which
case all the guest nodes would be placed in a single host node).


If the machine is only pinned to the third node and two
memoryBackingDirs are supplied then path_index will be 2 and later
accessing memoryBackingDirs[path_index] will crash.  Or maybe I have
missed something.  That's why I asked since I'm unsure myself, I have
not tested it.

{
+    virQEMUDriver *driver = priv->driver;
    g_autofree char *path = NULL;
+    g_autoptr(virQEMUDriverConfig) cfg =
virQEMUDriverGetConfig(driver);
+    size_t i;

-    if (qemuGetMemoryBackingPath(driver, vm->def, mem->info.alias,
&path) < 0)
-        return -1;
+    for (i = 0; i < cfg->nb_memoryBackingDirs; i++) {
+        virDomainXMLPrivateDataCallbacks *privateData =
(virDomainXMLPrivateDataCallbacks *) priv;
+        if (qemuGetMemoryBackingPath(def, privateData, i,
mem->info.alias, &path) < 0)
+            return -1;


I wonder why are the paths not saved in the status XML of the running
domain.  Since they can be changed during the VM's lifetime (while
restarting libvirt) it could cause issues during clean-up.  Sure,
unlink() will set errno to ENOENT, but the proper paths will not be
cleaned up.  It'd also make this part of the code easier and safer to
use from the callers.  But that's pre-existing.

I'm not sure I'm following the question on this one. I do not think we
want the paths to be changing
during the VM's lifetime. Are you asking if we *want*  to support that?
The current
libvirt XML schema does not support this. The actual path is kept hidden
from the guest VM
and is only known internally to libvirt.

Can you clarify?


Exactly, we do not want the paths to change.  But if you start the VM,
then stop virtqemud/libvirtd (totally supported), change the config file
to point to a different directory (or even different number of
directories, I haven't even thought of that), then start the daemon back
again, we'll have a problem when the VM is being cleaned up.

For these kinds of situations we are keeping more pieces of information
about running domains in a so called status XML.  This is saved on disk,
but unavailable to the user so saving something there does not
explicitly expose it to the user.  Having the paths saved there would
make it nicer to clean up and it should've probably been done even with
that one path that is supported nowadays.  I can have a look at fixing
that at first and then your patches could be applied on top of that fix
if that's fine with you or you can have a look at tackling that
yourself, it should not be difficult, tests for that should be nice to
write as well, it just needs to be done in a back-compat way, of course.

See qemuDomainObjPrivateXMLFormat() and qemuDomainObjPrivateXMLParse()
for some starting points.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux