On Thu, 20 Oct 2016 12:47:34 -0200
Eduardo Habkost <address@hidden> wrote:
On Thu, Oct 20, 2016 at 04:15:21PM +0200, Igor Mammedov wrote:
> On Thu, 20 Oct 2016 11:56:10 -0200
> Eduardo Habkost <address@hidden> wrote:
>
> > On Thu, Oct 20, 2016 at 03:42:15PM +0200, Igor Mammedov wrote:
> > > On Thu, 20 Oct 2016 21:11:38 +0800
> > > Haozhong Zhang <address@hidden> wrote:
> > >
> > > > On 10/20/16 14:34 +0200, Igor Mammedov wrote:
> > > > >On Thu, 20 Oct 2016 14:13:01 +0800
> > > > >Haozhong Zhang <address@hidden> wrote:
> > > > >
> > > > >> If a file is used as the backend of memory-backend-file and its size
is
> > > > >> not identical to the property 'size', the file will be truncated.
For a
> > > > >> file used as the backend of vNVDIMM, its data is expected to be
> > > > >> persistent and the truncation may corrupt the existing data.
> > > > >I wonder if it's possible just skip 'size' property in your case
instead
> > > > >'notrunc' property. That way if size is not present one'd get actual
size
> > > > >using get_file_size() and set 'size' to it?
> > > > >And if 'size' is provided and 'size' != file_size then error out.
> > > > >
> > > >
> > > > I don't know how this can be implemented in QEMU. Specially, how does
> > > > the memory-backend-file know it's used for vNVDIMM, so that it can
> > > > skip the 'size' property?
> > > Does memory-backend-file needs to know that it's used by NVDIMM?
> > > Looking at nvdimm_realize it doesn't as it's assumes
> > > hostemem_size == pmem_size + label_size
> > >
> > > I'd make hostmem_file.size optional and take size from file
> > > and if 'size' is specified explictly require it to mach file size.
> > > It's generic and has nothing to do with nvdimm.
> >
> > We can take size from file, or take size from the
> > host_memory_backend_get_memory() callers.
> >
> > Enumerating all sizes that QEMU can use as input:
> >
> > A) Backend file size
> > B) memory backend "size" option
> > C) frontend-provided size (-numa size, -m, or pc-dimm "size"
> > property)
> -numa size affect only anon memory not backend backed one, for
> backend baked memory we use memdev where size comes from backend
>
> pc-dimm.size is readonly and isn't supposed to influence backend.size
>
> I'd drop C option
If C is not present, it should be, as it affects the guest ABI
(and the ABI must never depend on the host you are running or
backend configuration, only on the frontend configuration).
I've meant that C should not affect behavior of backend.
If we are dropping -numa size in favor of the
memory-backend-provided size, that's a bug.
-numa size is not applicable here as it's not using backends,
when backends are used it's -numa memdev instead in which case
numa_info[nodenr].node_mem = object_property_get_int(o, "size", NULL);
>
> >
> > My suggestion is:
> > * B should be optional.
> > * If B is omitted, we should never truncate the file to a smaller
> > size.
> i.e. derive backend.size from filesize if possible (i.e. not hugepages)
>
> > * If B is omitted, we can use C as the size when mapping the
> > file.
> frontend size is the size that's mapped into guest address space.
> it should not influence backend's size in backward direction.
> You may notice pc-dimm.size is not user settable (readonly) property.
Frontend size will not influence backend size, it will just
affect the size of the memory region the frontend code will ask
the backend to provide.
In other words: I believe host_memory_backend_get_memory() needs
a 'size' argument, and that memory allocation could be optionally
delayed to the host_memory_backend_get_memory() call. This way,
we don't need a backend size at all, unless we want the backend
to truncate files or preallocate memory for us.
To not complicate things I'd keep current behavior of
host_memory_backend_get_memory()
i.e return MR for all the baked memory and then frontend
can split and map it into guest address space as it sees fit
using aliases for non trivial cases taking in account frontend's
own size/label_size/whatnot properties.
That's what NVDIMM does now, it gets MR for whole file and
the splits it on data and label areas and maps into GA only
data part using memory_region_init_alias().