On Tue, Dec 24, 2024 at 08:16:47AM -0800, Steve Sistare wrote:
> Extend qemu_ram_alloc_from_fd to support resizable ram, and define
> qemu_ram_resize_cb to clean up the API.
>
> Add a grow parameter to extend the file if necessary. However, if
> grow is false, a zero-sized file is always extended.
>
> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
> ---
> include/exec/ram_addr.h | 13 +++++++++----
> system/memory.c | 4 ++--
> system/physmem.c | 35 ++++++++++++++++++++---------------
> 3 files changed, 31 insertions(+), 21 deletions(-)
>
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index ff157c1..94bb3cc 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -111,23 +111,30 @@ long qemu_maxrampagesize(void);
> *
> * Parameters:
> * @size: the size in bytes of the ram block
> + * @max_size: the maximum size of the block after resizing
> * @mr: the memory region where the ram block is
> + * @resized: callback after calls to qemu_ram_resize
> * @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
> * RAM_NORESERVE, RAM_PROTECTED, RAM_NAMED_FILE, RAM_READONLY,
> * RAM_READONLY_FD, RAM_GUEST_MEMFD
> * @mem_path or @fd: specify the backing file or device
> * @offset: Offset into target file
> + * @grow: extend file if necessary (but an empty file is always extended).
> * @errp: pointer to Error*, to store an error if it happens
> *
> * Return:
> * On success, return a pointer to the ram block.
> * On failure, return NULL.
> */
> +typedef void (*qemu_ram_resize_cb)(const char *, uint64_t length, void
> *host);
> +
> RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> uint32_t ram_flags, const char *mem_path,
> off_t offset, Error **errp);
> -RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
> +RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, ram_addr_t max_size,
> + qemu_ram_resize_cb resized, MemoryRegion
> *mr,
> uint32_t ram_flags, int fd, off_t offset,
> + bool grow,
> Error **errp);
>
> RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
> @@ -135,9 +142,7 @@ RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void
> *host,
> RAMBlock *qemu_ram_alloc(ram_addr_t size, uint32_t ram_flags, MemoryRegion
> *mr,
> Error **errp);
> RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t max_size,
> - void (*resized)(const char*,
> - uint64_t length,
> - void *host),
> + qemu_ram_resize_cb resized,
> MemoryRegion *mr, Error **errp);
> void qemu_ram_free(RAMBlock *block);
>
> diff --git a/system/memory.c b/system/memory.c
> index 78e17e0..290c522 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -1680,8 +1680,8 @@ bool memory_region_init_ram_from_fd(MemoryRegion *mr,
> mr->readonly = !!(ram_flags & RAM_READONLY);
> mr->terminates = true;
> mr->destructor = memory_region_destructor_ram;
> - mr->ram_block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, offset,
> - &err);
> + mr->ram_block = qemu_ram_alloc_from_fd(size, size, NULL, mr, ram_flags,
> fd,
> + offset, false, &err);
> if (err) {
> mr->size = int128_zero();
> object_unparent(OBJECT(mr));
> diff --git a/system/physmem.c b/system/physmem.c
> index c76503a..48c544f 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -1942,8 +1942,10 @@ out_free:
> }
>
> #ifdef CONFIG_POSIX
> -RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
> +RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, ram_addr_t max_size,
> + qemu_ram_resize_cb resized, MemoryRegion
> *mr,
> uint32_t ram_flags, int fd, off_t offset,
> + bool grow,
> Error **errp)
> {
> RAMBlock *new_block;
> @@ -1953,7 +1955,9 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size,
> MemoryRegion *mr,
> /* Just support these ram flags by now. */
> assert((ram_flags & ~(RAM_SHARED | RAM_PMEM | RAM_NORESERVE |
> RAM_PROTECTED | RAM_NAMED_FILE | RAM_READONLY |
> - RAM_READONLY_FD | RAM_GUEST_MEMFD)) == 0);
> + RAM_READONLY_FD | RAM_GUEST_MEMFD |
> + RAM_RESIZEABLE)) == 0);
> + assert(max_size >= size);
>
> if (xen_enabled()) {
> error_setg(errp, "-mem-path not supported with Xen");
> @@ -1968,12 +1972,14 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size,
> MemoryRegion *mr,
>
> size = TARGET_PAGE_ALIGN(size);
> size = REAL_HOST_PAGE_ALIGN(size);
> + max_size = TARGET_PAGE_ALIGN(max_size);
> + max_size = REAL_HOST_PAGE_ALIGN(max_size);
>
> file_size = get_file_size(fd);
> - if (file_size > offset && file_size < (offset + size)) {
> + if (file_size && file_size < offset + max_size && !grow) {
Is this a bugfix for the case offset < fsize? If so, better make it a
small patch and copy stable..
$ touch ramfile
$ truncate -s 64M ramfile
$ ./qemu-system-x86_64 -object
memory-backend-file,mem-path=./ramfile,offset=128M,size=128M,id=mem1,prealloc=on
qemu-system-x86_64: qemu_prealloc_mem: preallocating memory failed: Bad address
So yes, it's a bug..
> error_setg(errp, "backing store size 0x%" PRIx64
> " does not match 'size' option 0x" RAM_ADDR_FMT,
> - file_size, size);
> + file_size, max_size);
> return NULL;
> }
>
> @@ -1988,11 +1994,13 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size,
> MemoryRegion *mr,
> new_block = g_malloc0(sizeof(*new_block));
> new_block->mr = mr;
> new_block->used_length = size;
> - new_block->max_length = size;
> + new_block->max_length = max_size;
> + new_block->resized = resized;
> new_block->flags = ram_flags;
> new_block->guest_memfd = -1;
> - new_block->host = file_ram_alloc(new_block, size, fd, !file_size, offset,
> - errp);
> + new_block->host = file_ram_alloc(new_block, max_size, fd,
> + file_size < offset + max_size,
Same here, looks like relevant to above.
In general, we want to either fail properly detecting wrong-sized files, or
making sure qemu can boot correctly with the fd. I confess pretty corner
cases.. but still.
So the whole patch looks correct, but maybe need a split.
> + offset, errp);
> if (!new_block->host) {
> g_free(new_block);
> return NULL;
> @@ -2044,7 +2052,8 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size,
> MemoryRegion *mr,
> return NULL;
> }
>
> - block = qemu_ram_alloc_from_fd(size, mr, ram_flags, fd, offset, errp);
> + block = qemu_ram_alloc_from_fd(size, size, NULL, mr, ram_flags, fd,
> offset,
> + false, errp);
> if (!block) {
> if (created) {
> unlink(mem_path);
> @@ -2059,9 +2068,7 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size,
> MemoryRegion *mr,
>
> static
> RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
> - void (*resized)(const char*,
> - uint64_t length,
> - void *host),
> + qemu_ram_resize_cb resized,
> void *host, uint32_t ram_flags,
> MemoryRegion *mr, Error **errp)
> {
> @@ -2113,10 +2120,8 @@ RAMBlock *qemu_ram_alloc(ram_addr_t size, uint32_t
> ram_flags,
> }
>
> RAMBlock *qemu_ram_alloc_resizeable(ram_addr_t size, ram_addr_t maxsz,
> - void (*resized)(const char*,
> - uint64_t length,
> - void *host),
> - MemoryRegion *mr, Error **errp)
> + qemu_ram_resize_cb resized,
> + MemoryRegion *mr, Error **errp)
> {
> return qemu_ram_alloc_internal(size, maxsz, resized, NULL,
> RAM_RESIZEABLE, mr, errp);
> --
> 1.8.3.1
>
--
Peter Xu
