At 2023-07-27 21:18:44, "David Hildenbrand" <da...@redhat.com> wrote:
>On 26.07.23 16:59, Thiner Logoer wrote:
>> Users may give "-mem-path" a read only file and expect the file
>> to be mapped read-write privately. Allow this but give a warning
>> since other users may surprise when the ram file is readonly and
>> qemu suddenly aborts elsewhere.
>>
>> Suggested-by: David Hildenbrand <da...@redhat.com>
>> Signed-off-by: Thiner Logoer <logoerthin...@163.com>
>> ---
>>
>> See the previous version at:
>> https://lore.kernel.org/qemu-devel/96a462ec-6f9d-fd83-f697-73e132432...@redhat.com/T/
>>
>> verified, this patch works for my setup, both functionality and the warning
>> are expected behavior.
>>
>> Also another problem when I look at the file_ram_open
>>
>> When readonly is true and the path is a directory, the open will succeed but
>> any later operations will fail since it is a directory fd. This may require
>> additional commits which is out of my scope. Merely record the question here.
Maybe you can notice this edge case? I am not sure whether this
case is on your todo list?
>>
>> softmmu/physmem.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
>> index 3df73542e1..e8279d69d4 100644
>> --- a/softmmu/physmem.c
>> +++ b/softmmu/physmem.c
>> @@ -1296,6 +1296,7 @@ static int file_ram_open(const char *path,
>> char *sanitized_name;
>> char *c;
>> int fd = -1;
>> + bool first_trial = true;
>>
>> *created = false;
>> for (;;) {
>> @@ -1332,6 +1333,18 @@ static int file_ram_open(const char *path,
>> break;
>> }
>> g_free(filename);
>> + } else if (first_trial && !readonly && errno == EACCES) {
>
>I guess it's better to only retry on private mappings, for shared
>mappings that cannot possibly work.
I feel that the retry can be applied in general - for shared mappings,
it will merely fail on the mmap step and should be ok?
Though, to retry only on private mapping seems straightforwards -
this function is called only once, and whether the mapping is private
can be passed here with a boolean flag as argument. Nonetheless
it may make the logic of the function more complex and less intuitive.
---
Regards,
logoerthiner
