Re:Re: Re: Re: [PATCH v1 1/3] softmmu/physmem: fallback to opening guest RAM file as readonly in a MAP_PRIVATE mapping

[ThinerLogoer]

At 2023-08-11 22:31:36, "Peter Xu" <peterx@redhat.com> wrote:
>On Fri, Aug 11, 2023 at 01:49:52PM +0800, ThinerLogoer wrote:
>> At 2023-08-11 05:24:43, "Peter Xu" <peterx@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.
>> 
>> Actually (readonly+private) is more reasonable than (private+discard), so I
>> want at least one room for a default (readonly+private) behavior.
>
>Just for purely discussion purpose: I think maybe someday private+discard
>could work.  IIUC what we're missing is an syscall interface to install a
>zero page for a MAP_PRIVATE, atomically freeing what's underneath: it seems
>either punching a hole or DONTNEED won't suffice here.  It'll just be
>another problem when having zero page involved in file mappings at least.
>
>> 
>> Also in my case I kind of have to use "-mem-path" despite it being considered
>> to be close to deprecated. Only with this I can avoid knowledge of memory
>> backend before migration. Actually there seems to be no equivalent working 
>> after-migration
>> setup of "-object memory-backend-file,... -machine q35,mem=..." that can 
>> match
>> before-migration setup of "-machine q35" (specifying nothing). Therefore
>> I must make a plan and choose a migration method BEFORE I boot the
>> machine and prepare to migrate, reducing the operation freedom.
>> Considering that, I have to use "-mem-path" which keeps the freedom but
>> has no configurable argument and I have to rely on default config.
>> 
>> Are there any "-object memory-backend-file..." setup equivalent to "-machine 
>> q35"
>> that can migrate from and to each other? If there is, I want to try it out.
>> By the way "-object memory-backend-file,id=pc.ram" has just been killed by 
>> an earlier
>> commit.
>
>I'm actually not familiar enough on the interfaces here, but I just checked
>up the man page; would this work for you, together with option (2)?
>
>        memory-backend='id'
>                An alternative to legacy -mem-path and mem-prealloc options.  
> Allows to use a memory backend as main RAM.
>
>                For example:
>
>                -object 
> memory-backend-file,id=pc.ram,size=512M,mem-path=/hugetlbfs,prealloc=on,share=on
>                -machine memory-backend=pc.ram
>                -m 512M
>

Wait ... I thought it should not work but it did work today. How bad am I at 
reading
the correct part of documentation ...

'-machine q35 -m 512M' is equivalent to '-object
memory-backend-file,id=pc.ram,size=512M
-machine q35,memory-backend=pc.ram',
the latter works, and the two mentioned setup can be
migrated from one to another.

What I was consistently trying was '-object
memory-backend-file,id=pc.ram,size=512M -machine q35', and qemu raises an error
for this in a recent update:

```
qemu-system-x86_64: object name 'pc.ram' is reserved for the default RAM 
backend, it can't be used for any other purposes. Change the object's 'id' to 
something else
```

This error is misleading. Actually in this case, the error report message 
should be more
close to:
```
object name 'pc.ram' is reserved for the default RAM backend, it can't
be used for any other purposes. Change the object's 'id' to something
else, or append "memory-backend=pc.ram" to -machine arguments
```

(I suggest rewriting the error message like this string because of the 
confusion just now)


Even though the default memory backend name is pc.ram, the
'-machine q35,memory-backend=pc.ram' part explicitly marks that qemu
uses a memory backend named pc.ram, rather than rely on default.

It seems that if it "rely on default" and memory-backend-file has an id
of "pc.ram" (in x86_64 of course), it will fail.

Great. Now I will consider using a "-object
memory-backend-file,id=pc.ram,size=512M
-machine q35,memory-backend=pc.ram"

--

Regards,

logoerthiner