@Stefan, see below on a R/O NVDIMM question.
We're discussing how to get MAPR_PRIVATE R/W mapping of a
memory-backend-file running when using R/O files.
This seems a good idea. I am good with the solution you proposed
here as well.
I was just going to get started working on that, when I realized
something important:
"@readonly: if true, the backing file is opened read-only; if false,
it is opened read-write. (default: false)"
"@share: if false, the memory is private to QEMU; if true, it is
shared (default: false)"
So readonly is *all* about the file access mode already ... the mmap()
parameters are just a side-effect of that. Adding a new
"file-access-mode" or similar would be wrong.
Here are the combinations we have right now:
-object memory-backend-file,share=on,readonly=on
-> Existing behavior: Open readonly, mmap readonly shared
-> Makes sense, mmap'ing readwrite would fail
-object memory-backend-file,share=on,readonly=off
-> Existing behavior: Open readwrite, mmap readwrite shared
-> Mostly makes sense: why open a shared file R/W and not mmap it
R/W?
-object memory-backend-file,share=off,readonly=off
-> Existing behavior: Open readwrite, mmap readwrite private
-> Mostly makes sense: why open a file R/W and not map it R/W (even if
private)?
-object memory-backend-file,share=off,readonly=on
-> Existing behavior: Open readonly, mmap readonly private
-> That's the problematic one
So for your use case (VM templating using a readonly file), you
would actually want to use:
-object memory-backend-file,share=off,readonly=on
BUT, have the mmap be writable (instead of currently readonly).
Assuming we would change the current behavior, what if someone would
specify:
-object memory-backend-file,readonly=on
(because the default is share=off ...) and using it for a R/O NVDIMM,
where we expect any write access to fail.
But let's look at the commit that added the "readonly" parameter:
commit 86635aa4e9d627d5142b81c57a33dd1f36627d07
Author: Stefan Hajnoczi <stefa...@redhat.com>
Date: Mon Jan 4 17:13:19 2021 +0000
hostmem-file: add readonly=on|off option
Let -object memory-backend-file work on read-only files when the
readonly=on option is given. This can be used to share the contents of a
file between multiple guests while preventing them from consuming
Copy-on-Write memory if guests dirty the pages, for example.
That was part of
https://lore.kernel.org/all/20210104171320.575838-3-stefa...@redhat.com/T/#m712f995e6dcfdde433958bae5095b145dd0ee640
From what I understand, for NVDIMMs we always use
"-object memory-backend-file,share=on", even when we want a
readonly NVDIMM.
So we have two options:
1) Change the current behavior of -object
memory-backend-file,share=off,readonly=on:
-> Open the file r/o but mmap it writable
2) Add a new property to configure the mmap accessibility. Not a big fan of
that.
@Stefan, do you have any concern when we would do 1) ?
As far as I can tell, we have to set the nvdimm to "unarmed=on" either way:
+ "unarmed" controls the ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM
+ State Flags" Bit 3 indicating that the device is "unarmed" and cannot accept
+ persistent writes. Linux guest drivers set the device to read-only when this
+ bit is present. Set unarmed to on when the memdev has readonly=on.
So changing the behavior would not really break the nvdimm use case.
Further, we could warn in nvdimm code when we stumble over this configuration
with
unarmed=on.
--
Cheers,
David / dhildenb
