On Tue, Jan 31, 2017 at 09:09:25AM +0000, 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(a)intel.com>
> 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>
> 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>
> > > 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>
> > > 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>
> > > > > 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>
> > > > > 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] Thanks! Will be ok to use config parameter name
"file_backing_memory_path"? Or you suggest different name?
I would recommend "memory_backing_file_dir" and the reason is that
we usually start with the most generic description and continue with
more and more specific description and our configure options uses mostly
"dir" instead of "path".
About the location of this configure option I would put it after
"hugetlbfs_mount" because it also configure memory backing.
Pavel