Re: [PATCH v6 18/19] migration/multifd: Stop changing the packet on recv side

[Peter Xu]

On Tue, Aug 27, 2024 at 02:46:05PM -0300, Fabiano Rosas wrote:
> @@ -254,12 +250,10 @@ int multifd_ram_unfill_packet(MultiFDRecvParams *p, 
> Error **errp)
>          return 0;
>      }
>  
> -    /* make sure that ramblock is 0 terminated */
> -    packet->ramblock[255] = 0;
> -    p->block = qemu_ram_block_by_name(packet->ramblock);
> +    ramblock_name = g_strndup(packet->ramblock, 255);

I understand we want to move to a const*, however this introduces a 256B
allocation per multifd packet, which we definitely want to avoid.. I wonder
whether that's worthwhile just to make it const. :-(

I don't worry too much on the const* and vars pointed being abused /
updated when without it - the packet struct is pretty much limited only to
be referenced in this unfill function, and then we will do the load based
on MultiFDRecvParams* later anyway.  So personally I'd rather lose the
const* v.s. one allocation.

Or we could also sanity check byte 255 to be '\0' (which, AFAIU, should
always be the case..), then we can get both benefits.

> +    p->block = qemu_ram_block_by_name(ramblock_name);
>      if (!p->block) {
> -        error_setg(errp, "multifd: unknown ram block %s",
> -                   packet->ramblock);
> +        error_setg(errp, "multifd: unknown ram block %s", ramblock_name);
>          return -1;
>      }

-- 
Peter Xu