> -----Original Message----- > From: Safka, JaroslavX > Sent: Thursday, June 23, 2016 2:47 PM > To: Martin Kletzander <mkletzan@xxxxxxxxxx> > Cc: libvir-list@xxxxxxxxxx; Mooney, Sean K <sean.k.mooney@xxxxxxxxx>; > Ptacek, MichalX <michalx.ptacek@xxxxxxxxx> > Subject: RE: [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@xxxxxxxxxx] > Sent: Thursday, June 23, 2016 3:42 PM > To: Safka, JaroslavX <jaroslavx.safka@xxxxxxxxx> > Cc: libvir-list@xxxxxxxxxx; Mooney, Sean K <sean.k.mooney@xxxxxxxxx>; > Ptacek, MichalX <michalx.ptacek@xxxxxxxxx> > Subject: Re: [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@xxxxxxxxxx > >https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list