From 1d44ff586f8a8e113379430750b5a0a2a3f64cf9 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Thu, 6 Jul 2023 09:56:06 +0200 Subject: [PATCH] softmmu/physmem: Warn with ram_block_discard_range() on MAP_PRIVATE file mapping ram_block_discard_range() cannot possibly do the right thing in MAP_PRIVATE file mappings in the general case. To achieve the documented semantics, we also have to punch a hole into the file, possibly messing with other MAP_PRIVATE/MAP_SHARED mappings of such a file. For example, using VM templating -- see commit b17fbbe55cba ("migration: allow private destination ram with x-ignore-shared") -- in combination with any mechanism that relies on discarding of RAM is problematic. This includes: * Postcopy live migration * virtio-balloon inflation/deflation or free-page-reporting * virtio-mem So at least warn that there is something possibly dangerous is going on when using ram_block_discard_range() in these cases. Message-ID: <20230706075612.67404-2-david@redhat.com> Acked-by: Peter Xu Tested-by: Mario Casquero Reviewed-by: Juan Quintela Reviewed-by: Michael S. Tsirkin Signed-off-by: David Hildenbrand --- softmmu/physmem.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/softmmu/physmem.c b/softmmu/physmem.c index bda475a719..3df73542e1 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -3456,6 +3456,24 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length) * so a userfault will trigger. */ #ifdef CONFIG_FALLOCATE_PUNCH_HOLE + /* + * We'll discard data from the actual file, even though we only + * have a MAP_PRIVATE mapping, possibly messing with other + * MAP_PRIVATE/MAP_SHARED mappings. There is no easy way to + * change that behavior whithout violating the promised + * semantics of ram_block_discard_range(). + * + * Only warn, because it works as long as nobody else uses that + * file. + */ + if (!qemu_ram_is_shared(rb)) { + warn_report_once("ram_block_discard_range: Discarding RAM" + " in private file mappings is possibly" + " dangerous, because it will modify the" + " underlying file and will affect other" + " users of the file"); + } + ret = fallocate(rb->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, start, length); if (ret) {