On Mon, Nov 27, 2023 at 05:25:51PM -0300, Fabiano Rosas wrote:
> Add the necessary code to parse the format changes for the 'fixed-ram'
> capability.
>
> One of the more notable changes in behavior is that in the 'fixed-ram'
> case ram pages are restored in one go rather than constantly looping
> through the migration stream.
>
> Signed-off-by: Nikolay Borisov <[email protected]>
> Signed-off-by: Fabiano Rosas <[email protected]>
> ---
> - added sanity check for pages_offset alignment
> - s/parsing/reading
> - used Error
> - fixed buffer size computation, now allowing an arbitrary limit
> - fixed dereference of pointer to packed struct member in endianness
> conversion
> ---
> migration/ram.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 119 insertions(+)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 4a0ab8105f..08604222f2 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -106,6 +106,12 @@
> */
> #define FIXED_RAM_FILE_OFFSET_ALIGNMENT 0x100000
>
> +/*
> + * When doing fixed-ram migration, this is the amount we read from the
> + * pages region in the migration file at a time.
> + */
> +#define FIXED_RAM_LOAD_BUF_SIZE 0x100000
> +
> XBZRLECacheStats xbzrle_counters;
>
> /* used by the search for pages to send */
> @@ -2996,6 +3002,35 @@ static void fixed_ram_insert_header(QEMUFile *file,
> RAMBlock *block)
> qemu_put_buffer(file, (uint8_t *) header, header_size);
> }
>
> +static bool fixed_ram_read_header(QEMUFile *file, FixedRamHeader *header,
> + Error **errp)
> +{
> + size_t ret, header_size = sizeof(FixedRamHeader);
> +
> + ret = qemu_get_buffer(file, (uint8_t *)header, header_size);
> + if (ret != header_size) {
> + error_setg(errp, "Could not read whole fixed-ram migration header "
> + "(expected %zd, got %zd bytes)", header_size, ret);
> + return false;
> + }
> +
> + /* migration stream is big-endian */
> + header->version = be32_to_cpu(header->version);
> +
> + if (header->version > FIXED_RAM_HDR_VERSION) {
> + error_setg(errp, "Migration fixed-ram capability version mismatch "
> + "(expected %d, got %d)", FIXED_RAM_HDR_VERSION,
> + header->version);
> + return false;
> + }
> +
> + header->page_size = be64_to_cpu(header->page_size);
> + header->bitmap_offset = be64_to_cpu(header->bitmap_offset);
> + header->pages_offset = be64_to_cpu(header->pages_offset);
> +
> + return true;
> +}
> +
> /*
> * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
> * long-running RCU critical section. When rcu-reclaims in the code
> @@ -3892,6 +3927,80 @@ void colo_flush_ram_cache(void)
> trace_colo_flush_ram_cache_end();
> }
>
> +static void read_ramblock_fixed_ram(QEMUFile *f, RAMBlock *block,
> + long num_pages, unsigned long *bitmap)
> +{
> + unsigned long set_bit_idx, clear_bit_idx;
> + ram_addr_t offset;
> + void *host;
> + size_t read, unread, size, buf_size = FIXED_RAM_LOAD_BUF_SIZE;
> +
> + for (set_bit_idx = find_first_bit(bitmap, num_pages);
> + set_bit_idx < num_pages;
> + set_bit_idx = find_next_bit(bitmap, num_pages, clear_bit_idx + 1)) {
> +
> + clear_bit_idx = find_next_zero_bit(bitmap, num_pages, set_bit_idx +
> 1);
> +
> + unread = TARGET_PAGE_SIZE * (clear_bit_idx - set_bit_idx);
> + offset = set_bit_idx << TARGET_PAGE_BITS;
> +
> + while (unread > 0) {
> + host = host_from_ram_block_offset(block, offset);
> + size = MIN(unread, buf_size);
Use the macro directly? buf_size can be dropped then.
> +
> + read = qemu_get_buffer_at(f, host, size,
> + block->pages_offset + offset);
Error detection missing? qemu_get_buffer_at() returns 0 if error, then it
dead loops.
> + offset += read;
> + unread -= read;
> + }
> + }
> +}
> +
> +static int parse_ramblock_fixed_ram(QEMUFile *f, RAMBlock *block,
> + ram_addr_t length, Error **errp)
For new code, start to keep using boolean as retval when Error** exists?
> +{
> + g_autofree unsigned long *bitmap = NULL;
> + FixedRamHeader header;
> + size_t bitmap_size;
> + long num_pages;
> +
> + if (!fixed_ram_read_header(f, &header, errp)) {
> + return -EINVAL;
> + }
> +
> + block->pages_offset = header.pages_offset;
> +
> + /*
> + * Check the alignment of the file region that contains pages. We
> + * don't enforce FIXED_RAM_FILE_OFFSET_ALIGNMENT to allow that
> + * value to change in the future. Do only a sanity check with page
> + * size alignment.
> + */
> + if (!QEMU_IS_ALIGNED(block->pages_offset, TARGET_PAGE_SIZE)) {
> + error_setg(errp,
> + "Error reading ramblock %s pages, region has bad
> alignment",
> + block->idstr);
> + return -EINVAL;
> + }
> +
> + num_pages = length / header.page_size;
> + bitmap_size = BITS_TO_LONGS(num_pages) * sizeof(unsigned long);
> +
> + bitmap = g_malloc0(bitmap_size);
> + if (qemu_get_buffer_at(f, (uint8_t *)bitmap, bitmap_size,
> + header.bitmap_offset) != bitmap_size) {
> + error_setg(errp, "Error reading dirty bitmap");
> + return -EINVAL;
> + }
> +
> + read_ramblock_fixed_ram(f, block, num_pages, bitmap);
Detect error and fail properly?
> +
> + /* Skip pages array */
> + qemu_set_offset(f, block->pages_offset + length, SEEK_SET);
> +
> + return 0;
> +}
> +
> static int parse_ramblock(QEMUFile *f, RAMBlock *block, ram_addr_t length)
> {
> int ret = 0;
> @@ -3900,6 +4009,16 @@ static int parse_ramblock(QEMUFile *f, RAMBlock
> *block, ram_addr_t length)
>
> assert(block);
>
> + if (migrate_fixed_ram()) {
> + Error *local_err = NULL;
> +
> + ret = parse_ramblock_fixed_ram(f, block, length, &local_err);
> + if (local_err) {
> + error_report_err(local_err);
> + }
> + return ret;
We can optionally add one pre-requisite patch to convert parse_ramblock()
to return boolean too. I remember it was done somewhere before, but maybe
not merged.
> + }
> +
> if (!qemu_ram_is_migratable(block)) {
> error_report("block %s should not be migrated !", block->idstr);
> return -EINVAL;
> --
> 2.35.3
>
--
Peter Xu
