Do not modify RAMBlocks in ram_block_add. The block should be fully formed before calling ram_block_add to add it to the block list. This will simplify error handling and be more modular.
Start by hoisting guest_memfd creation to the call sites. No functional change. Signed-off-by: Steve Sistare <steven.sist...@oracle.com> --- system/physmem.c | 85 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 37 deletions(-) diff --git a/system/physmem.c b/system/physmem.c index 6216b14..ffcf012 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -1803,13 +1803,34 @@ static void dirty_memory_extend(ram_addr_t old_ram_size, } } +static int ram_block_create_guest_memfd(RAMBlock *rb, Error **errp) +{ + assert(kvm_enabled()); + + if (ram_block_discard_require(true) < 0) { + error_setg_errno(errp, errno, + "cannot set up private guest memory: discard currently blocked"); + error_append_hint(errp, "Are you using assigned devices?\n"); + return -1; + } + + return kvm_create_guest_memfd(rb->max_length, 0, errp); +} + +static void ram_block_destroy_guest_memfd(RAMBlock *rb) +{ + if (rb->guest_memfd >= 0) { + close(rb->guest_memfd); + ram_block_discard_require(false); + } +} + static void ram_block_add(RAMBlock *new_block, Error **errp) { const bool noreserve = qemu_ram_is_noreserve(new_block); const bool shared = qemu_ram_is_shared(new_block); RAMBlock *block; RAMBlock *last_block = NULL; - bool free_on_error = false; ram_addr_t old_ram_size, new_ram_size; Error *err = NULL; @@ -1839,26 +1860,6 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) return; } memory_try_enable_merging(new_block->host, new_block->max_length); - free_on_error = true; - } - } - - if (new_block->flags & RAM_GUEST_MEMFD) { - assert(kvm_enabled()); - assert(new_block->guest_memfd < 0); - - if (ram_block_discard_require(true) < 0) { - error_setg_errno(errp, errno, - "cannot set up private guest memory: discard currently blocked"); - error_append_hint(errp, "Are you using assigned devices?\n"); - goto out_free; - } - - new_block->guest_memfd = kvm_create_guest_memfd(new_block->max_length, - 0, errp); - if (new_block->guest_memfd < 0) { - qemu_mutex_unlock_ramlist(); - goto out_free; } } @@ -1910,17 +1911,11 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) ram_block_notify_add(new_block->host, new_block->used_length, new_block->max_length); } - return; - -out_free: - if (free_on_error) { - qemu_anon_ram_free(new_block->host, new_block->max_length); - new_block->host = NULL; - } } static RAMBlock *ram_block_create(MemoryRegion *mr, ram_addr_t size, - ram_addr_t max_size, uint32_t ram_flags) + ram_addr_t max_size, uint32_t ram_flags, + Error **errp) { RAMBlock *rb = g_malloc0(sizeof(*rb)); @@ -1930,7 +1925,17 @@ static RAMBlock *ram_block_create(MemoryRegion *mr, ram_addr_t size, rb->flags = ram_flags; rb->page_size = qemu_real_host_page_size(); rb->mr = mr; - rb->guest_memfd = -1; + + if (ram_flags & RAM_GUEST_MEMFD) { + rb->guest_memfd = ram_block_create_guest_memfd(rb, errp); + if (rb->guest_memfd < 0) { + g_free(rb); + return NULL; + } + } else { + rb->guest_memfd = -1; + } + trace_ram_block_create(rb->idstr, rb->flags, rb->fd, rb->used_length, rb->max_length, mr->align); return rb; @@ -1981,9 +1986,14 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr, return NULL; } - new_block = ram_block_create(mr, size, size, ram_flags); + new_block = ram_block_create(mr, size, size, ram_flags, errp); + if (!new_block) { + return NULL; + } + host = file_ram_alloc(new_block, size, fd, !file_size, offset, errp); if (!host) { + ram_block_destroy_guest_memfd(new_block); g_free(new_block); return NULL; } @@ -2068,11 +2078,16 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size, size = ROUND_UP(size, align); max_size = ROUND_UP(max_size, align); assert(max_size >= size); - new_block = ram_block_create(mr, size, max_size, ram_flags); + new_block = ram_block_create(mr, size, max_size, ram_flags, errp); + if (!new_block) { + return NULL; + } new_block->resized = resized; + new_block->host = host; ram_block_add(new_block, &local_err); if (local_err) { + ram_block_destroy_guest_memfd(new_block); g_free(new_block); error_propagate(errp, local_err); return NULL; @@ -2119,11 +2134,7 @@ static void reclaim_ramblock(RAMBlock *block) qemu_anon_ram_free(block->host, block->max_length); } - if (block->guest_memfd >= 0) { - close(block->guest_memfd); - ram_block_discard_require(false); - } - + ram_block_destroy_guest_memfd(block); g_free(block); } -- 1.8.3.1