On 10/7/2024 12:28 PM, Peter Xu wrote:
On Mon, Oct 07, 2024 at 11:49:25AM -0400, Peter Xu wrote:
On Mon, Sep 30, 2024 at 12:40:36PM -0700, Steve Sistare wrote:
Save the memfd for anonymous ramblocks in CPR state, along with a name
that uniquely identifies it.  The block's idstr is not yet set, so it
cannot be used for this purpose.  Find the saved memfd in new QEMU when
creating a block.  QEMU hard-codes the length of some internally-created
blocks, so to guard against that length changing, use lseek to get the
actual length of an incoming memfd.

Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
---
  system/physmem.c | 25 ++++++++++++++++++++++++-
  1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/system/physmem.c b/system/physmem.c
index 174f7e0..ddbeec9 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -72,6 +72,7 @@
#include "qapi/qapi-types-migration.h"
  #include "migration/options.h"
+#include "migration/cpr.h"
  #include "migration/vmstate.h"
#include "qemu/range.h"
@@ -1663,6 +1664,19 @@ void qemu_ram_unset_idstr(RAMBlock *block)
      }
  }
+static char *cpr_name(RAMBlock *block)
+{
+    MemoryRegion *mr = block->mr;
+    const char *mr_name = memory_region_name(mr);
+    g_autofree char *id = mr->dev ? qdev_get_dev_path(mr->dev) : NULL;
+
+    if (id) {
+        return g_strdup_printf("%s/%s", id, mr_name);
+    } else {
+        return g_strdup(mr_name);
+    }
+}
+
  size_t qemu_ram_pagesize(RAMBlock *rb)
  {
      return rb->page_size;
@@ -1858,14 +1872,18 @@ static void ram_block_add(RAMBlock *new_block, Error 
**errp)
                                          TYPE_MEMORY_BACKEND)) {
              size_t max_length = new_block->max_length;
              MemoryRegion *mr = new_block->mr;
-            const char *name = memory_region_name(mr);
+            g_autofree char *name = cpr_name(new_block);
new_block->mr->align = QEMU_VMALLOC_ALIGN;
              new_block->flags |= RAM_SHARED;
+            new_block->fd = cpr_find_fd(name, 0);
if (new_block->fd == -1) {
                  new_block->fd = qemu_memfd_create(name, max_length + 
mr->align,
                                                    0, 0, 0, errp);
+                cpr_save_fd(name, 0, new_block->fd);
+            } else {
+                new_block->max_length = lseek(new_block->fd, 0, SEEK_END);

So this can overwrite the max_length that the caller specified..

I remember we used to have some tricks on specifying different max_length
for ROMs on dest QEMU (on which, qemu firmwares also upgraded on the dest
host so the size can be bigger than src qemu's old ramblocks), so that the
MR is always large enough to reload even the new firmwares, while migration
only migrates the smaller size (used_length) so it's fine as we keep the
extra sizes empty. I think that can relevant to the qemu_ram_resize() call
of parse_ramblock().

Yes, resizable ram block for firmware blob is the only case I know of where
the length changed in the past.  If a length changes in the future, we will
need to detect and accommodate that change here, and I believe the fix will
be to simply use the actual length, as per the code above.  But if you prefer,
for now I can check for length change and return an error. New qemu will fail
to start, and old qemu will recover.

The reload will not happen until some point, perhaps system resets.  I
wonder whether that is an issue in this case.

Firmware is only generated once, via this path on x86:
  qmp_x_exit_preconfig
    qemu_machine_creation_done
      qdev_machine_creation_done
        pc_machine_done
          acpi_setup
            acpi_add_rom_blob
              rom_add_blob
                rom_set_mr

After a system reset, the ramblock contents from memory are used as-is.

PS: If this is needed by CPR-transfer only because mmap() later can fail
due to a bigger max_length,

That is the reason.  IMO adjusting max_length is more robust than fiddling
with truncate and pretending that max_length is larger, when qemu will never
be able to use the phantom space up to max_length.

- Steve

I wonder whether it can be fixed by passing
truncate=true in the upcoming file_ram_alloc(), rather than overwritting
the max_length value itself.


              }
if (new_block->fd >= 0) {
@@ -1875,6 +1893,7 @@ static void ram_block_add(RAMBlock *new_block, Error 
**errp)
                                                   false, 0, errp);
              }
              if (!new_block->host) {
+                cpr_delete_fd(name, 0);
                  qemu_mutex_unlock_ramlist();
                  return;
              }
@@ -2182,6 +2201,8 @@ static void reclaim_ramblock(RAMBlock *block)
void qemu_ram_free(RAMBlock *block)
  {
+    g_autofree char *name = NULL;
+
      if (!block) {
          return;
      }
@@ -2192,6 +2213,8 @@ void qemu_ram_free(RAMBlock *block)
      }
qemu_mutex_lock_ramlist();
+    name = cpr_name(block);
+    cpr_delete_fd(name, 0);
      QLIST_REMOVE_RCU(block, next);
      ram_list.mru_block = NULL;
      /* Write list before version */
--
1.8.3.1


--
Peter Xu



Reply via email to