On 01/31/2017 10:40 AM, Safka, JaroslavX wrote:
-----Original Message----- From: Pavel Hrdina [mailto:phrdina@redhat.com] Sent: Monday, January 30, 2017 1:50 PM To: Safka, JaroslavX <jaroslavx.safka@intel.com> Cc: Daniel P. Berrange <berrange@redhat.com>; Michal Privoznik <mprivozn@redhat.com>; libvir-list@redhat.com; Mooney, Sean K <sean.k.mooney@intel.com>; Ptacek, MichalX <michalx.ptacek@intel.com> Subject: Re: [libvirt] [PATCHv4 0/3] Add support for file memorybacking
On Mon, Jan 30, 2017 at 12:16:38PM +0000, Safka, JaroslavX wrote:
-----Original Message----- From: Pavel Hrdina [mailto:phrdina@redhat.com] Sent: Monday, January 30, 2017 1:13 PM To: Safka, JaroslavX <jaroslavx.safka@intel.com> Cc: Daniel P. Berrange <berrange@redhat.com>; Michal Privoznik <mprivozn@redhat.com>; libvir-list@redhat.com; Mooney, Sean K <sean.k.mooney@intel.com>; Ptacek, MichalX <michalx.ptacek@intel.com> Subject: Re: [libvirt] [PATCHv4 0/3] Add support for file memorybacking
On Mon, Jan 30, 2017 at 12:06:19PM +0000, Safka, JaroslavX wrote:
-----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Monday, January 30, 2017 12:35 PM To: Michal Privoznik <mprivozn@redhat.com> Cc: Safka, JaroslavX <jaroslavx.safka@intel.com>; libvir-list@redhat.com; Mooney, Sean K <sean.k.mooney@intel.com>; Ptacek, MichalX <michalx.ptacek@intel.com> Subject: Re: [libvirt] [PATCHv4 0/3] Add support for file memorybacking
On Sat, Jan 28, 2017 at 03:03:02PM +0100, Michal Privoznik wrote: > On 13.12.2016 13:12, Jaroslav Safka wrote: >> Hi, >> we would like to introduce 3 new elements source,access and >> allocation in memoryBacking element. >> For now it was made for numa topology. >> >> <memoryBacking> >> <source type="file|anonymous"/> >> <access mode="shared|private"/> >> <allocation mode="immediate|ondemand"/> >> </memoryBacking> >> >> If allocation is immediate then -mem-prealloc should be >> added to the qemu commanline. >> If source is file then >> -object memory-backend-file,id=mem,size=1024M,mem-path=*lib >> dir >> path* -numa node,memdev=mem Will be added to the qemu >> commandline >> >> If access is shared then the "share=on" parameter will 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 >> >> The access mode can be overriden by specifying token >> memAccess in numa cell. >> >> The test cpu-numa-memshared was removed, because behaviour >> was changed and is not needed anymore > > I beg to disagree. What if you don't have any <memoryBacking/>? > > I like these patches, but I'm not certainly sure about: > a) domain XML (in the past we used to require an ACK on schema > change from one of the Dans)
What are you unsure about ? This XML is what I suggested in previous rounds of discussion.
> b) the location for qemu to create its mmaped files (patch 3/3). > cfg->libDir looks very suspicious.
Well we've got two possibilities - source=anonymous, should be using /dev/shm, similar to how we do huge pages. For source=file, we need a real filesystem. Something under /var/lib/libvirt
is reasonable.
Perhaps a dedicated dir is needed ? eg /var/lib/libvirt/ram in order to provide a point where the admin can mount a sufficiently large filesystem ? Or make it configurable in /etc/libvirt/qemu.conf
[Jarek] It will be ok when I do something like libDir + "/ram". Or It should be configurable on compilation time?
Oh, I see that Dan already suggested to make it configurable. I would say both. Make it configurable in qemu.conf and use /var/lib/libvirt/ram as a default path.
[Jarek] will be ok to make it configurable in another patch set? Or it should be all in one patch set? Because it will take me some time, when I find out how to add this parameter as configurable.
It's not that hard :) for example this commit 661887f5582 can be used as an example how to add a new configurable option into qemu.conf. We have whole month before next release so it would be nice to have it as part of the same patch series.
Pavel
[Jarek] Also I'm not sure where the config parameter put. Probably in the "process_entry" ?
I'd go with memory_backing_path and create new memory_entry. Michal