Re: Ping: Re: [PATCH v2] softmmu/physmem: try opening file readonly before failure in file_ram_open

[David Hildenbrand <david@redhat.com>]

On 03.08.23 17:43, ThinerLogoer wrote:
> At 2023-07-28 18:45:20, "David Hildenbrand" <david@redhat.com> wrote:
>>
>>
>> Whatever you prefer! If I resend the patch, I would keep you the author
>> and only add my Co-authored-by: Signed-off-by:.
>>
>> Just let me know.
>>
> 
> Hello,
> 
> I wonder whether you have planned to resubmit the current patch anytime soon, or is it already
> inside the patch queue?

I'm currently testing the following change on top, not compile-tested under
Windows, though.


 From b1abec2fe024ea90860ecf600c381e4a25e22ed8 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Thu, 3 Aug 2023 19:14:34 +0200
Subject: [PATCH] softmmu/physmem: Always detect and handle directories in
  file_ram_open()

open() does not fail on directories when opening readonly -- O_RDONLY.

To identify directories and handle them accordingly in file_ram_open()
also when readonly=true was specified, detect if we just opened a directory
using fstat() instead.

Before this change:

$ ./qemu-system-x86_64 \
     -object memory-backend-file,id=ram0,mem-path=tmp,readonly=true,size=1g
qemu-system-x86_64: unable to map backing store for guest RAM: No such device

With this change, it works as expected: we create a temporary hidden
file in that directory, just like when specifying readonly=false.

Reported-by: Thiner Logoer <logoerthiner1@163.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
  softmmu/physmem.c | 24 ++++++++++++++++++++++--
  1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index d1ae694b20..32b51fd54d 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1300,9 +1300,29 @@ static int file_ram_open(const char *path,
      for (;;) {
          fd = open(path, readonly ? O_RDONLY : O_RDWR);
          if (fd >= 0) {
-            /* @path names an existing file, use it */
-            break;
+            /*
+             * open() won't fail when passing O_RDONLY on directories. So
+             * check manually if we're given a directory, and convert to
+             * EISDIR.
+             */
+            if (readonly) {
+                struct stat file_stat;
+
+                if (fstat(fd, &file_stat)) {
+                    close(fd);
+                    return -errno;
+                } else if (S_ISDIR(file_stat.st_mode)) {
+                    close(fd);
+                    fd = -1;
+                    errno = EISDIR;
+                }
+            }
+            if (fd >= 0) {
+                /* @path names an existing file, use it */
+                break;
+            }
          }
+
          if (errno == ENOENT) {
              /* @path names a file that doesn't exist, create it */
              fd = open(path, O_RDWR | O_CREAT | O_EXCL, 0644);
-- 
2.41.0


-- 
Cheers,

David / dhildenb