softmmu/physmem: file_ram_open() readonly improvements

[PATCH v1 0/3] softmmu/physmem: file_ram_open() readonly improvements

Posted by David Hildenbrand 9 months, 2 weeks ago

Patch #1 is the result of the discussion of:
    "[PATCH v2] softmmu/physmem: try opening file readonly before failure
     in file_ram_open" [1]

Instead of handling it inside file_ram_open(), handle it in the caller
and only fallback to readonly in a MAP_PRIVATE mapping.

Patch #2 refuses to create readonly files instead of creating a new file
and opening it writable. Patch #3 no longer returns
directories from file_ram_open(), resulting in a better error message when
trying to open a readonly file but specifying a directory.

[1] https://lkml.kernel.org/r/20230726145912.88545-1-logoerthiner1@163.com

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Thiner Logoer <logoerthiner1@163.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>

David Hildenbrand (2):
  softmmu/physmem: fail creation of new files in file_ram_open() with
    readonly=true
  softmmu/physmem: never return directories from file_ram_open()

Thiner Logoer (1):
  softmmu/physmem: fallback to opening guest RAM file as readonly in a
    MAP_PRIVATE mapping

 softmmu/physmem.c | 52 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 44 insertions(+), 8 deletions(-)

-- 
2.41.0

Re:[PATCH v1 0/3] softmmu/physmem: file_ram_open() readonly improvements

Posted by ThinerLogoer 9 months, 2 weeks ago

At 2023-08-08 03:07:31, "David Hildenbrand" <david@redhat.com> wrote:
>Patch #1 is the result of the discussion of:
>    "[PATCH v2] softmmu/physmem: try opening file readonly before failure
>     in file_ram_open" [1]
>
>Instead of handling it inside file_ram_open(), handle it in the caller
>and only fallback to readonly in a MAP_PRIVATE mapping.
>
>Patch #2 refuses to create readonly files instead of creating a new file
>and opening it writable. Patch #3 no longer returns
>directories from file_ram_open(), resulting in a better error message when
>trying to open a readonly file but specifying a directory.
>
>[1] https://lkml.kernel.org/r/20230726145912.88545-1-logoerthiner1@163.com
>
>Cc: Paolo Bonzini <pbonzini@redhat.com>
>Cc: Peter Xu <peterx@redhat.com>
>Cc: Igor Mammedov <imammedo@redhat.com>
>Cc: Thiner Logoer <logoerthiner1@163.com>
>Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>
>David Hildenbrand (2):
>  softmmu/physmem: fail creation of new files in file_ram_open() with
>    readonly=true
>  softmmu/physmem: never return directories from file_ram_open()
>
>Thiner Logoer (1):
>  softmmu/physmem: fallback to opening guest RAM file as readonly in a
>    MAP_PRIVATE mapping
>
> softmmu/physmem.c | 52 +++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 44 insertions(+), 8 deletions(-)
>
>-- 
>2.41.0

I have tested the patch on my compilation environment. These patches does not
have problem on my setup. Great job on handling more cases about file
opening here!

--

Regards,

logoerthiner

Re: [PATCH v1 0/3] softmmu/physmem: file_ram_open() readonly improvements

Posted by Philippe Mathieu-Daudé 9 months, 2 weeks ago

Hi,

On 8/8/23 19:26, ThinerLogoer wrote:
> 
> At 2023-08-08 03:07:31, "David Hildenbrand" <david@redhat.com> wrote:

>> Instead of handling it inside file_ram_open(), handle it in the caller
>> and only fallback to readonly in a MAP_PRIVATE mapping.

> I have tested the patch on my compilation environment. These patches does not
> have problem on my setup. Great job on handling more cases about file
> opening here!

Does that mean we can add your tag on this series?

Tested-by: Thiner Logoer <logoerthiner1@163.com>

Re:Re: [PATCH v1 0/3] softmmu/physmem: file_ram_open() readonly improvements

Posted by ThinerLogoer 9 months, 2 weeks ago

At 2023-08-10 19:11:03, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote:
>Hi,
>
>On 8/8/23 19:26, ThinerLogoer wrote:
>> 
>> At 2023-08-08 03:07:31, "David Hildenbrand" <david@redhat.com> wrote:
>
>>> Instead of handling it inside file_ram_open(), handle it in the caller
>>> and only fallback to readonly in a MAP_PRIVATE mapping.
>
>> I have tested the patch on my compilation environment. These patches does not
>> have problem on my setup. Great job on handling more cases about file
>> opening here!
>
>Does that mean we can add your tag on this series?
>
>Tested-by: Thiner Logoer <logoerthiner1@163.com>

This tag is OK, despite that I highly suspect whether my testing is sufficient.
My testing is very rough and only focus on the functionalities I care about.
It would be better to have a more professional tester.

--

Regards,

logoerthiner