[libvirt] [PATCH 0/2] Add support for preallocated memory
This change introduces support for preallocated shared file descriptor based memory backing. It allows vhost-user to be used without hugepages. This is achieved by introducing 3 new sub elements to the memoryBacking element: source, access & allocation which will configure qemu commandline during VM startup accordingly Jaroslav Safka (2): Add new elements source,access and allocation Add xml to argv for memorybacking access,source,.. docs/schemas/domaincommon.rng | 45 +++++++++++++++++ src/conf/domain_conf.c | 59 +++++++++++++++++++++- src/conf/domain_conf.h | 31 ++++++++++++ src/qemu/qemu_command.c | 56 ++++++++++++++++++++ src/qemu/qemu_command.h | 4 ++ .../qemuxml2argv-memorybacking-set.args | 26 ++++++++++ .../qemuxml2argv-memorybacking-set.xml | 32 ++++++++++++ .../qemuxml2argv-memorybacking-unset.args | 22 ++++++++ .../qemuxml2argv-memorybacking-unset.xml | 32 ++++++++++++ tests/qemuxml2argvtest.c | 3 ++ .../qemuxml2xmlout-memorybacking-set.xml | 40 +++++++++++++++ .../qemuxml2xmlout-memorybacking-unset.xml | 40 +++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 13 files changed, 392 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-set.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-unset.xml -- 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.
Add three new elements in memoryBacking and enable their parsing. (without converting to argv yet) <memoryBacking> <source type="file|anonymous"/> <access Mode="shared|private"/> <allocation mode="immediate|ondemand"/> </memoryBacking> --- docs/schemas/domaincommon.rng | 45 +++++++++++++++++ src/conf/domain_conf.c | 59 +++++++++++++++++++++- src/conf/domain_conf.h | 31 ++++++++++++ .../qemuxml2argv-memorybacking-set.args | 22 ++++++++ .../qemuxml2argv-memorybacking-set.xml | 32 ++++++++++++ .../qemuxml2argv-memorybacking-unset.args | 22 ++++++++ .../qemuxml2argv-memorybacking-unset.xml | 32 ++++++++++++ .../qemuxml2xmlout-memorybacking-set.xml | 40 +++++++++++++++ .../qemuxml2xmlout-memorybacking-unset.xml | 40 +++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 10 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-set.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-unset.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 162c2e0..b1bdcc7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -546,6 +546,27 @@ <empty/> </element> </optional> + <optional> + <element name="source"> + <attribute name="type"> + <ref name="mbSourceType"/> + </attribute> + </element> + </optional> + <optional> + <element name="access"> + <attribute name="mode"> + <ref name="mbAccessMode"/> + </attribute> + </element> + </optional> + <optional> + <element name="allocation"> + <attribute name="mode"> + <ref name="mbAllocMode"/> + </attribute> + </element> + </optional> </interleave> </element> </optional> @@ -5597,4 +5618,28 @@ </choice> </attribute> </define> + <define name="mbSourceType"> + <attribute name="sourcetype"> + <choice> + <value>file</value> + <value>anonymous</value> + </choice> + </attribute> + </define> + <define name="mbAccessMode"> + <attribute name="accessmode"> + <choice> + <value>shared</value> + <value>private</value> + </choice> + </attribute> + </define> + <define name="mbAllocMode"> + <attribute name="allocmode"> + <choice> + <value>immediate</value> + <value>ondemand</value> + </choice> + </attribute> + </define> </grammar> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9443281..c4fc886 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -819,6 +819,21 @@ VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST, "abort", "pivot") +VIR_ENUM_IMPL(virDomainMemorySource, VIR_DOMAIN_MEMORY_SOURCE_LAST, + "none", + "file", + "anonymous") + +VIR_ENUM_IMPL(virDomainMemoryAccess, VIR_DOMAIN_MEMORY_ACCESS_LAST, + "none", + "shared", + "private") + +VIR_ENUM_IMPL(virDomainMemoryAllocation, VIR_DOMAIN_MEMORY_ALLOCATION_LAST, + "none", + "immediate", + "ondemand") + VIR_ENUM_IMPL(virDomainLoader, VIR_DOMAIN_LOADER_TYPE_LAST, "rom", @@ -15943,6 +15958,36 @@ virDomainDefParseXML(xmlDocPtr xml, if (virXPathBoolean("boolean(./memoryBacking/locked)", ctxt)) def->mem.locked = true; + tmp = virXPathString("string(./memoryBacking/source/@type)", ctxt); + if (tmp) { + if ((def->mem.source = virDomainMemorySourceTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown memoryBacking/source/type '%s'"), tmp); + goto error; + } + VIR_FREE(tmp); + } + + tmp = virXPathString("string(./memoryBacking/access/@mode)", ctxt); + if (tmp) { + if ((def->mem.access = virDomainMemoryAccessTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown memoryBacking/access/mode '%s'"), tmp); + goto error; + } + VIR_FREE(tmp); + } + + tmp = virXPathString("string(./memoryBacking/allocation/@mode)", ctxt); + if (tmp) { + if ((def->mem.allocation = virDomainMemoryAllocationTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown memoryBacking/allocation/mode '%s'"), tmp); + goto error; + } + VIR_FREE(tmp); + } + /* Extract blkio cgroup tunables */ if (virXPathUInt("string(./blkiotune/weight)", ctxt, &def->blkio.weight) < 0) @@ -22970,7 +23015,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "</memtune>\n"); } - if (def->mem.nhugepages || def->mem.nosharepages || def->mem.locked) { + if (def->mem.nhugepages || def->mem.nosharepages || def->mem.locked + || def->mem.source || def->mem.access || def->mem.allocation) + { virBufferAddLit(buf, "<memoryBacking>\n"); virBufferAdjustIndent(buf, 2); if (def->mem.nhugepages) @@ -22979,6 +23026,16 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "<nosharepages/>\n"); if (def->mem.locked) virBufferAddLit(buf, "<locked/>\n"); + if (def->mem.source) + virBufferAsprintf(buf, "<source type='%s'/>\n", + virDomainMemorySourceTypeToString(def->mem.source)); + if (def->mem.access) + virBufferAsprintf(buf, "<access mode='%s'/>\n", + virDomainMemoryAccessTypeToString(def->mem.access)); + if (def->mem.allocation) + virBufferAsprintf(buf, "<allocation mode='%s'/>\n", + virDomainMemoryAllocationTypeToString(def->mem.allocation)); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</memoryBacking>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6e81e52..8dd8a93 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -565,6 +565,30 @@ typedef enum { VIR_DOMAIN_DISK_MIRROR_STATE_LAST } virDomainDiskMirrorState; +typedef enum { + VIR_DOMAIN_MEMORY_SOURCE_NONE = 0, /* No memory source defined */ + VIR_DOMAIN_MEMORY_SOURCE_FILE, /* Memory source is set as file */ + VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS, /* Memory source is set as anonymous */ + + VIR_DOMAIN_MEMORY_SOURCE_LAST, +} virDomainMemorySource; + +typedef enum { + VIR_DOMAIN_MEMORY_ACCESS_NONE = 0, /* No memory access defined */ + VIR_DOMAIN_MEMORY_ACCESS_SHARED, /* Memory access is set as shared */ + VIR_DOMAIN_MEMORY_ACCESS_PRIVATE, /* Memory access is set as private */ + + VIR_DOMAIN_MEMORY_ACCESS_LAST, +} virDomainMemoryAccess; + +typedef enum { + VIR_DOMAIN_MEMORY_ALLOCATION_NONE = 0, /* No memory allocation defined */ + VIR_DOMAIN_MEMORY_ALLOCATION_IMMEDIATE, /* Memory allocation is set as immediate */ + VIR_DOMAIN_MEMORY_ALLOCATION_ONDEMAND, /* Memory allocation is set as ondemand */ + + VIR_DOMAIN_MEMORY_ALLOCATION_LAST, +} virDomainMemoryAllocation; + /* Stores the virtual disk configuration */ struct _virDomainDiskDef { @@ -2087,6 +2111,10 @@ struct _virDomainMemtune { unsigned long long soft_limit; /* in kibibytes, limit at off_t bytes */ unsigned long long min_guarantee; /* in kibibytes, limit at off_t bytes */ unsigned long long swap_hard_limit; /* in kibibytes, limit at off_t bytes */ + + int source; /* enum virDomainMemorySource */ + int access; /* enum virDomainMemoryAccess */ + int allocation; /* enum virDomainMemoryAllocation */ }; typedef struct _virDomainPowerManagement virDomainPowerManagement; @@ -3006,6 +3034,9 @@ VIR_ENUM_DECL(virDomainTPMModel) VIR_ENUM_DECL(virDomainTPMBackend) VIR_ENUM_DECL(virDomainMemoryModel) VIR_ENUM_DECL(virDomainMemoryBackingModel) +VIR_ENUM_DECL(virDomainMemorySource) +VIR_ENUM_DECL(virDomainMemoryAccess) +VIR_ENUM_DECL(virDomainMemoryAllocation) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.args b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.args new file mode 100644 index 0000000..bb702dc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name SomeDummyHugepagesGuest \ +-S \ +-M pc \ +-m 1024 \ +-smp 2 \ +-uuid ef1bdff4-27f3-4e85-a807-5fb4d58463cc \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-SomeDummyHugepagesGu/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-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 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.xml b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.xml new file mode 100644 index 0000000..655eb8d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>SomeDummyHugepagesGuest</name> + <uuid>ef1bdff4-27f3-4e85-a807-5fb4d58463cc</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <memoryBacking> + <source type='file'/> + <access mode='shared'/> + <allocation mode='immediate'/> + </memoryBacking> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.args b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.args new file mode 100644 index 0000000..bb702dc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name SomeDummyHugepagesGuest \ +-S \ +-M pc \ +-m 1024 \ +-smp 2 \ +-uuid ef1bdff4-27f3-4e85-a807-5fb4d58463cc \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-SomeDummyHugepagesGu/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-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 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.xml b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.xml new file mode 100644 index 0000000..74f8a12 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>SomeDummyHugepagesGuest</name> + <uuid>ef1bdff4-27f3-4e85-a807-5fb4d58463cc</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <memoryBacking> + <source type="anonymous"/> + <access mode="private"/> + <allocation mode="ondemand"/> + </memoryBacking> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-set.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-set.xml new file mode 100644 index 0000000..f63ae38 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-set.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>SomeDummyHugepagesGuest</name> + <uuid>ef1bdff4-27f3-4e85-a807-5fb4d58463cc</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <memoryBacking> + <source type='file'/> + <access mode='shared'/> + <allocation mode='immediate'/> + </memoryBacking> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-unset.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-unset.xml new file mode 100644 index 0000000..7de9ffb --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-unset.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>SomeDummyHugepagesGuest</name> + <uuid>ef1bdff4-27f3-4e85-a807-5fb4d58463cc</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <memoryBacking> + <source type='anonymous'/> + <access mode='private'/> + <allocation mode='ondemand'/> + </memoryBacking> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7db9cb7..5adf1a7 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -822,6 +822,9 @@ mymain(void) DO_TEST("virtio-input"); DO_TEST("virtio-input-passthrough"); + DO_TEST("memorybacking-set"); + DO_TEST("memorybacking-unset"); + virObjectUnref(cfg); DO_TEST("acpi-table"); -- 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.
On Thu, Jun 23, 2016 at 01:25:28PM +0100, Jaroslav Safka wrote:
Add three new elements in memoryBacking and enable their parsing. (without converting to argv yet)
<memoryBacking> <source type="file|anonymous"/> <access Mode="shared|private"/> <allocation mode="immediate|ondemand"/> </memoryBacking>
This is all fine since it matches what we discussed previously in the context of Nova. For benefit of others, here is the thinking behind this. QEMU has ability to map guest memory anonymously (the default) or via a file. Libvirt only exposes the ability to use a file when dealing with huge pages. Nova wants ability to use a file with non-hugepage memory, eg a plain /dev/shm file. IOW, huge pages implies source type=file, but source type=file does not imply huge pages - it could be /dev/hugepages or /dev/shm Requiring anonymous with hugepages is an error scenario. When configuring NUMA we have ability to mark the memory as shared, vs private. eg <numa> ... <cell id='1' cpus='4-7' memory='512000' unit='KiB' memAccess='shared'/> ... </numa> The <access mode=shared|private> gives us ability to control this for the VM as a whole, without defining NUMA. The per-cell setting should override this if present. Conversely, <cell> with no memAccess attribute should use the <access mode=> top level setting. Finally QEMU has ability to allocate guest pages on-demand (when the guest first touches a page) or immediate (guaranteed fully reserved upfront). Use of huge pages implies "immediate" mode since huge pages are not swappable or overcommittable. In the non-huge page case though, we don't have a way to request immedate allocate, so this is adding it. Requiring allocate=ondemand in combination with hugepages is an error scenario.
--- docs/schemas/domaincommon.rng | 45 +++++++++++++++++ src/conf/domain_conf.c | 59 +++++++++++++++++++++- src/conf/domain_conf.h | 31 ++++++++++++
Also need to update docs/formatdomain.html.in to document this.
.../qemuxml2argv-memorybacking-set.args | 22 ++++++++ .../qemuxml2argv-memorybacking-set.xml | 32 ++++++++++++ .../qemuxml2argv-memorybacking-unset.args | 22 ++++++++ .../qemuxml2argv-memorybacking-unset.xml | 32 ++++++++++++ .../qemuxml2xmlout-memorybacking-set.xml | 40 +++++++++++++++ .../qemuxml2xmlout-memorybacking-unset.xml | 40 +++++++++++++++ tests/qemuxml2xmltest.c | 3 ++
The QEMU test suite additions should be in the patch that adds the code to the QEMU driver. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
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. 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; + 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"; + + if (VIR_DOMAIN_MEMORY_ACCESS_SHARED == def->mem.access) { + // add ",share=on" to -object memory-backend-file + virBufferAsprintf(&buf, "%s,share=on", template); + } else { + virBufferAsprintf(&buf, "%s", template); + } + + + 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); + } + + 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); #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,\ +share=on \ +-numa node,memdev=mem 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.
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 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@redhat.com] Sent: Thursday, June 23, 2016 3:42 PM To: Safka, JaroslavX <jaroslavx.safka@intel.com> Cc: libvir-list@redhat.com; Mooney, Sean K <sean.k.mooney@intel.com>; Ptacek, MichalX <michalx.ptacek@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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.
-----Original Message----- From: Safka, JaroslavX Sent: Thursday, June 23, 2016 2:47 PM To: Martin Kletzander <mkletzan@redhat.com> Cc: libvir-list@redhat.com; Mooney, Sean K <sean.k.mooney@intel.com>; Ptacek, MichalX <michalx.ptacek@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@redhat.com] Sent: Thursday, June 23, 2016 3:42 PM To: Safka, JaroslavX <jaroslavx.safka@intel.com> Cc: libvir-list@redhat.com; Mooney, Sean K <sean.k.mooney@intel.com>; Ptacek, MichalX <michalx.ptacek@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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.
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; + 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";
As Michael has pointed out - this is just insanity - you can't hardcode memory size like this.
+ + if (VIR_DOMAIN_MEMORY_ACCESS_SHARED == def->mem.access) { + // add ",share=on" to -object memory-backend-file + virBufferAsprintf(&buf, "%s,share=on", template); + } else { + virBufferAsprintf(&buf, "%s", template); + } + + + 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"); + } +
You need todo
+ if (VIR_DOMAIN_MEMORY_SOURCE_FILE == def->mem.source) { + optstr = qemuBuildMemoryBackendFileStr(def); + if (optstr) { + virCommandAddArg(cmd, "-object"); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + }
You're ignoring the error here - you must propagate the NULL error.
+ + // 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); + }
We already have code that runs when the guest has NUMA topology configured. This existing code must be adapted to honour the top level VIR_DOMAIN_MEMORY_ACCESS_* setting, vs the per-cell setting. This new code you're adding should only be used when a non-NUMA guest config is requested, and it should be using the -mem-path command line argument to set the file - not = creating a NUMA topology in a non-NUMA requested guest. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (5)
-
Daniel P. Berrange -
Jaroslav Safka -
Martin Kletzander -
Mooney, Sean K -
Safka, JaroslavX