[libvirt] [PATCH 2/2] Add support for preallocated memory - xml2argv
Mooney, Sean K
sean.k.mooney at intel.com
Thu Jun 30 15:02:41 UTC 2016
> -----Original Message-----
> From: Safka, JaroslavX
> Sent: Thursday, June 23, 2016 2:47 PM
> To: Martin Kletzander <mkletzan at redhat.com>
> Cc: libvir-list at redhat.com; Mooney, Sean K <sean.k.mooney at intel.com>;
> Ptacek, MichalX <michalx.ptacek at intel.com>
> Subject: RE: [libvirt] [PATCH 2/2] Add support for preallocated memory - xml2argv
>
> If source is file then -object memory-backend-file,id=mem,size=1024M,mem-
> path=/var/lib/libvirt/qemu -numa node,memdev=mem should be added to the
> qemu commandline
[Mooney, Sean K] in the above example the size should not be hardcoded but instead match the amount of
Memory requested for the vm. The patch should also be defined by libvirt either as a complie time constant or
As a user specified path not a hardcoded string.
What those parameters actually mean is as follows.
-object memory-backing-file,id=mem,size=xM,path=xyz
# create a memory-backing-file object called mem of size x megabytes with an open file descriptor to xyz.
# Note that this does not actually create a file just a file descriptor so we will not be write to disk.
-numa node,memdev=mem
# this instruct qemu to use the memory-backing-file descriptor as the memory backing for the guest.
-object memory-backend-file,id=mem,size=1024M,mem-
> path=/var/lib/libvirt/qemu -numa node,memdev=mem
>
> If allocation is immediate then -mem-prealloc should be added to the qemu
> commanline.
>
> If access is shared then the share=on parameter should be added to the
> memory-backend-file e.g.-object memory-backend-
> file,id=mem,size=1024M,mem-path=/var/lib/libvirt/qemu,share=on
>
> -----Original Message-----
> From: Martin Kletzander [mailto:mkletzan at redhat.com]
> Sent: Thursday, June 23, 2016 3:42 PM
> To: Safka, JaroslavX <jaroslavx.safka at intel.com>
> Cc: libvir-list at redhat.com; Mooney, Sean K <sean.k.mooney at intel.com>;
> Ptacek, MichalX <michalx.ptacek at intel.com>
> Subject: Re: [libvirt] [PATCH 2/2] Add support for preallocated memory -
> xml2argv
>
> On Thu, Jun 23, 2016 at 01:25:29PM +0100, Jaroslav Safka wrote:
> >Add conversion from xml to argv for subelements source,access and
> >allocation of <memoryBacking>
> >
> >This change introduces support for preallocated shared file descriptor
> >based memory backing.
> >It allows vhost-user to be used without hugepages.
> >
>
> How does this show up in the guest?
>
> >Configured by these elements:
> ><memoryBacking>
> > <source type="file|anonymous"/>
> > <access Mode="shared|private"/>
> > <allocation mode="immediate|ondemand"/> </memoryBacking>
> >---
> > src/qemu/qemu_command.c | 56 ++++++++++++++++++++++
> > src/qemu/qemu_command.h | 4 ++
> > .../qemuxml2argv-memorybacking-set.args | 6 ++-
> > tests/qemuxml2argvtest.c | 3 ++
> > 4 files changed, 68 insertions(+), 1 deletion(-)
> >
> >diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index
> >6944129..321e71f 100644
> >--- a/src/qemu/qemu_command.c
> >+++ b/src/qemu/qemu_command.c
> >@@ -9328,6 +9328,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
> > if (qemuBuildNVRAMCommandLine(cmd, def, qemuCaps) < 0)
> > goto error;
> >
> >+ if (qemuBuildMemoryBackendCommandLine(cmd, def, qemuCaps) < 0)
> >+ goto error;
> >+
>
> So this is not accounted for in any of the memory sizes, right?
>
> Shouldn't this be reflected in some of those virDomainDefGetMemory*()
> functions? It probably depends on the answer to my previous question.
>
> > if (snapshot)
> > virCommandAddArgList(cmd, "-loadvm", snapshot->def->name,
> > NULL);
> >
> >@@ -9592,3 +9595,56 @@ qemuBuildChrDeviceStr(char **deviceStr,
> >
> > return ret;
> > }
> >+
> >+static char *
> >+qemuBuildMemoryBackendFileStr(const virDomainDef *def) {
> >+ virBuffer buf = VIR_BUFFER_INITIALIZER;
> >+ const char template[] =
> >+"memory-backend-file,id=mem,size=1024M,mem-path=/var/lib/libvirt/qemu"
> >+;
> >+
>
> Wow, this seems highly configurable. How come none of these options needs to
> be changed? That doesn't seem right.
>
> >+ if (VIR_DOMAIN_MEMORY_ACCESS_SHARED == def->mem.access) {
>
> As you might've noticed in the code, we don't do yoda conditions.
>
> >+ // add ",share=on" to -object memory-backend-file
> >+ virBufferAsprintf(&buf, "%s,share=on", template);
> >+ } else {
> >+ virBufferAsprintf(&buf, "%s", template);
> >+ }
> >+
>
> The virAsprintf() function should shorten this function a bit.
>
> >+
> >+ if (virBufferCheckError(&buf) < 0)
> >+ goto error;
> >+
> >+ return virBufferContentAndReset(&buf);
> >+
> >+ error:
> >+ virBufferFreeAndReset(&buf);
> >+ return NULL;
> >+}
> >+
> >+
> >+int
> >+qemuBuildMemoryBackendCommandLine(virCommandPtr cmd,
> >+ const virDomainDef *def,
> >+ virQEMUCapsPtr qemuCaps
> >+__attribute__((unused))) {
> >+ char *optstr = NULL;
> >+
> >+ if (VIR_DOMAIN_MEMORY_ALLOCATION_IMMEDIATE == def-
> >mem.allocation) {
> >+ // add '-mem-prealloc'
> >+ virCommandAddArg(cmd, "-mem-prealloc");
> >+ }
> >+
> >+ if (VIR_DOMAIN_MEMORY_SOURCE_FILE == def->mem.source) {
> >+ optstr = qemuBuildMemoryBackendFileStr(def);
> >+ if (optstr) {
> >+ virCommandAddArg(cmd, "-object");
> >+ virCommandAddArg(cmd, optstr);
> >+ VIR_FREE(optstr);
> >+ }
> >+
> >+ // add '-object memory-backend-file,id=mem,size=1024M,mem-
> path=/var/lib/libvirt/qemu'
> >+ // add '-numa node,memdev=mem'
> >+ virCommandAddArgList(cmd, "-numa", "node,memdev=mem", NULL);
>
> This looks like it duplicates some of the code that is there already.
> Couldn't this be handled more cleanly? Could there be only part of that memory
> shared and not all of it?
>
> >+ }
> >+
> >+ return 0;
> >+}
> >diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index
> >9ff4edb..f95d0ea 100644
> >--- a/src/qemu/qemu_command.h
> >+++ b/src/qemu/qemu_command.h
> >@@ -175,5 +175,9 @@ bool qemuCheckCCWS390AddressSupport(const
> virDomainDef *def,
> > virDomainDeviceInfo info,
> > virQEMUCapsPtr qemuCaps,
> > const char *devicename);
> >+int
> >+qemuBuildMemoryBackendCommandLine(virCommandPtr cmd,
> >+ const virDomainDef *def,
> >+ virQEMUCapsPtr qemuCaps);
> >
>
> Not aligned.
>
> > #endif /* __QEMU_COMMAND_H__*/
> >diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.args
> >b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.args
> >index bb702dc..626fb21 100644
> >--- a/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.args
> >+++ b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.args
> >@@ -19,4 +19,8 @@ QEMU_AUDIO_DRV=none \ -usb \ -drive
> >file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> >-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> >--device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> >+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
> >+-mem-prealloc \ -object
> >+memory-backend-file,id=mem,size=1024M,mem-path=/var/lib/libvirt/qemu,\
>
> So the only difference to setting this up with hugepages is that the mem-path is
> not set to hugetlbfs mountpoint?
>
> >+share=on \
> >+-numa node,memdev=mem
>
> And it automatically implies numa node? If the numa node is there automatically
> (which could be), why not just using the:
>
> <cpu>
> <numa>
> <cell ... memAccess='shared'/>?
>
> More info on:
> https://libvirt.org/formatdomain.html#elementsCPU
>
> Hope that helps, have a nice day,
> Martin
>
> >diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index
> >a4b8bf4..401b455 100644
> >--- a/tests/qemuxml2argvtest.c
> >+++ b/tests/qemuxml2argvtest.c
> >@@ -2024,6 +2024,9 @@ mymain(void)
> >
> > DO_TEST("acpi-table", NONE);
> >
> >+ DO_TEST("memorybacking-unset", NONE);
> >+ DO_TEST("memorybacking-set", NONE);
> >+
> > qemuTestDriverFree(&driver);
> >
> > return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> >--
> >2.1.0
> >
> >--------------------------------------------------------------
> >Intel Research and Development Ireland Limited Registered in Ireland
> >Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> >Registered Number: 308263
> >
> >
> >This e-mail and any attachments may contain confidential material for
> >the sole use of the intended recipient(s). Any review or distribution
> >by others is strictly prohibited. If you are not the intended
> >recipient, please contact the sender and delete all copies.
> >
> >--
> >libvir-list mailing list
> >libvir-list at redhat.com
> >https://www.redhat.com/mailman/listinfo/libvir-list
More information about the libvir-list
mailing list