[PATCH v3 0/1] fallocate missing fd_offset

[PATCH v3 0/1] fallocate missing fd_offset

[“William Roche]

From: William Roche <william.roche@oracle.com>

Working on the poisoned memory recovery mechanisms with David
Hildenbrand, it appeared that the file hole punching done with
the memory discard functions are missing the file offset value
fd_offset to correctly modify the right file location.

Note that guest_memfd would not currently take into account
fd_offset, so I'm adding a comment next the the fallocate use
in ram_block_discard_guest_memfd_range().

The version is also checkpatch.pl clean
make check runs fine on both ARM and x86

v1->v2
  . replacing the ram_block_discard_guest_memfd_range()
    modifications with a comment.
  . use a local variable for the global file offset

v2->v3
  . change the error reporting messages separating start and fd_offset
  . the local variable is no longer needed


William Roche (1):
  system/physmem: take into account fd_offset for file fallocate

 system/physmem.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.43.5

[PATCH v3 1/1] system/physmem: take into account fd_offset for file fallocate

[“William Roche]

From: William Roche <william.roche@oracle.com>

Punching a hole in a file with fallocate needs to take into account the
fd_offset value for a correct file location.
But guest_memfd internal use doesn't currently consider fd_offset.

Fixes: 4b870dc4d0c0 ("hostmem-file: add offset option")

Signed-off-by: William Roche <william.roche@oracle.com>
---
 system/physmem.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index c76503aea8..9a449ad4f9 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3689,18 +3689,19 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
             }
 
             ret = fallocate(rb->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
-                            start, length);
+                            start + rb->fd_offset, length);
             if (ret) {
                 ret = -errno;
-                error_report("%s: Failed to fallocate %s:%" PRIx64 " +%zx (%d)",
-                             __func__, rb->idstr, start, length, ret);
+                error_report("%s: Failed to fallocate %s:%" PRIx64 "+%" PRIx64
+                             " +%zx (%d)", __func__, rb->idstr, start,
+                             rb->fd_offset, length, ret);
                 goto err;
             }
 #else
             ret = -ENOSYS;
             error_report("%s: fallocate not available/file"
-                         "%s:%" PRIx64 " +%zx (%d)",
-                         __func__, rb->idstr, start, length, ret);
+                         "%s:%" PRIx64 "+%" PRIx64 " +%zx (%d)", __func__,
+                         rb->idstr, start, rb->fd_offset, length, ret);
             goto err;
 #endif
         }
@@ -3747,6 +3748,7 @@ int ram_block_discard_guest_memfd_range(RAMBlock *rb, uint64_t start,
     int ret = -1;
 
 #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
+    /* ignore fd_offset with guest_memfd */
     ret = fallocate(rb->guest_memfd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
                     start, length);
 
-- 
2.43.5

Re: [PATCH v3 1/1] system/physmem: take into account fd_offset for file fallocate

[Peter Xu]

On Wed, Jan 22, 2025 at 07:40:53PM +0000, “William Roche wrote:
> From: William Roche <william.roche@oracle.com>
> 
> Punching a hole in a file with fallocate needs to take into account the
> fd_offset value for a correct file location.
> But guest_memfd internal use doesn't currently consider fd_offset.
> 
> Fixes: 4b870dc4d0c0 ("hostmem-file: add offset option")
> 
> Signed-off-by: William Roche <william.roche@oracle.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH v3 1/1] system/physmem: take into account fd_offset for file fallocate

[David Hildenbrand]

On 22.01.25 20:40, “William Roche wrote:
> From: William Roche <william.roche@oracle.com>
> 
> Punching a hole in a file with fallocate needs to take into account the
> fd_offset value for a correct file location.
> But guest_memfd internal use doesn't currently consider fd_offset.
> 
> Fixes: 4b870dc4d0c0 ("hostmem-file: add offset option")
> 
> Signed-off-by: William Roche <william.roche@oracle.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb