At 2023-08-12 03:00:54, "David Hildenbrand" <da...@redhat.com> wrote:
>On 11.08.23 07:49, ThinerLogoer wrote:
>> At 2023-08-11 05:24:43, "Peter Xu" <pet...@redhat.com> wrote:
>>> On Fri, Aug 11, 2023 at 01:06:12AM +0800, ThinerLogoer wrote:
>>>>> I think we have the following options (there might be more)
>>>>>
>>>>> 1) This patch.
>>>>>
>>>>> 2) New flag for memory-backend-file. We already have "readonly" and
>>>>> "share=". I'm having a hard time coming up with a good name that really
>>>>> describes the subtle difference.
>>>>>
>>>>> 3) Glue behavior to the QEMU machine
>>>>>
>>>>
>>>> 4) '-deny-private-discard' argv, or environment variable, or both
>>>
>>> I'd personally vote for (2). How about "fdperm"? To describe when we want
>>> to use different rw permissions on the file (besides the access permission
>>> of the memory we already provided with "readonly"=XXX). IIUC the only sane
>>> value will be ro/rw/default, where "default" should just use the same rw
>>> permission as the memory ("readonly"=XXX).
>>>
>>> Would that be relatively clean and also work in this use case?
>>>
>>> (the other thing I'd wish we don't have that fallback is, as long as we
>>> have any of that "fallback" we'll need to be compatible with it since
>>> then, and for ever...)
>>
>> If it must be (2), I would vote (2) + (4), with (4) adjust the default
>> behavior of said `fdperm`.
>> Mainly because (private+discard) is itself not a good practice and (4) serves
>> as a good tool to help catch existing (private+discard) problems.
>
>Instead of fdperm, maybe we could find a better name.
>
>The man page of "open" says: The argument flags must include one of the
>following access modes: O_RDONLY, O_WRONLY, or O_RDWR. These request
>opening the file read-only, write-only, or read/write, respectively.
>
>So maybe something a bit more mouthful like "file-access-mode" would be
>better.
Maybe "fd-mode"or "open-mode" to make it shorter and also non ambiguous.
"open-access-mode" can also be considered.
Btw my chatgpt agent suggested 10 names to me, in case you feel hard to
decide a name:
access-mode
file-mode
open-mode
file-permission
file-access-mode (aha!)
file-access-permission
file-io-access-mode
file-open-permission
file-operation-mode
file-read-write-mode
>
>We could have the options
>*readwrite
>*readonly
>*auto
>
>Whereby "auto" is the default.
I like the "auto" here.
>
>Specifying:
>
>* "readonly=true,file-access-mode=readwrite" would fail.
>* "shared=true,readonly=false,file-access-mode=readonly" would fail.
>* "shared=false,readonly=false,file-access-mode=readonly" would work.
Yeah, sanitizing the arguments here is wise.
>
>In theory, we could adjust the mode of "auto" with compat machines. But
>maybe it would be sufficient to do something like the following
>
>1) shared=true,readonly=false
> -> readwrite
>2) shared=true,readonly=true
> > readonly
>2) shared=false,readonly=true
> -> readonly
>3) shared=false,readonly=true
> hugetlb? -> readwrite
> otherwise -> readonly
Looks like there is some typo here. I assume it was:
1) shared=true,readonly=false
-> readwrite
2) shared=true,readonly=true
-> readonly
3) shared=false,readonly=true
-> readonly
4) shared=false,readonly=false
hugetlb? -> readwrite
otherwise -> readonly
>Reason being the traditional usage of hugetlb with MAP_PRIVATE where I
>can see possible users with postcopy. Further, VM templating with
>hugetlb might not be that common ... users could still modify the
>behavior if they really have to.
>
>That would make your use case work automatically with file-backed memory.
This seems a good idea. I am good with the solution you proposed
here as well.
--
Regards,
logoerthiner
