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().
>
> The reload will not happen until some point, perhaps system resets. I
> wonder whether that is an issue in this case.
>
> +Igor +Mst for this.
PS: If this is needed by CPR-transfer only because mmap() later can fail
due to a bigger max_length, 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
--
Peter Xu
