Re: [libvirt] [PATCHv4 0/3] Add support for file memorybacking

-----Original Message----- From: Pavel Hrdina [mailto:phrdina@redhat.com] Sent: Monday, January 30, 2017 1:50 PM To: Safka, JaroslavX <jaroslavx.safka(a)intel.com&gt; Cc: Daniel P. Berrange <berrange(a)redhat.com>; Michal Privoznik <mprivozn(a)redhat.com>; libvir-list(a)redhat.com; Mooney, Sean K <sean.k.mooney(a)intel.com>; Ptacek, MichalX <michalx.ptacek(a)intel.com&gt; 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(a)intel.com&gt; > > Cc: Daniel P. Berrange <berrange(a)redhat.com>; Michal Privoznik > > <mprivozn(a)redhat.com>; libvir-list(a)redhat.com; Mooney, Sean K > > <sean.k.mooney(a)intel.com>; Ptacek, MichalX > > <michalx.ptacek(a)intel.com&gt; > > 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(a)redhat.com&gt; > > > > Cc: Safka, JaroslavX <jaroslavx.safka(a)intel.com>; > > > > libvir-list(a)redhat.com; Mooney, Sean K > > > > <sean.k.mooney(a)intel.com>; Ptacek, MichalX > > > > <michalx.ptacek(a)intel.com&gt; > > > > 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