Re: [PATCH v4 13/34] migration/ram: Add outgoing 'fixed-ram' migration

2024-02-25 Thread Peter Xu
On Tue, Feb 20, 2024 at 07:41:17PM -0300, Fabiano Rosas wrote:
> Implement the outgoing migration side for the 'fixed-ram' capability.
> 
> A bitmap is introduced to track which pages have been written in the
> migration file. Pages are written at a fixed location for every
> ramblock. Zero pages are ignored as they'd be zero in the destination
> migration as well.
> 
> The migration stream is altered to put the dirty pages for a ramblock
> after its header instead of having a sequential stream of pages that
> follow the ramblock headers.
> 
> Without fixed-ram (current):With fixed-ram (new):
> 
>  -   
>  | ramblock 1 header |   | ramblock 1 header|
>  -   
>  | ramblock 2 header |   | ramblock 1 fixed-ram header  |
>  -   
>  | ...   |   | padding to next 1MB boundary |
>  -   | ...  |
>  | ramblock n header |   
>  -   | ramblock 1 pages |
>  | RAM_SAVE_FLAG_EOS |   | ...  |
>  -   
>  | stream of pages   |   | ramblock 2 header|
>  | (iter 1)  |   
>  | ...   |   | ramblock 2 fixed-ram header  |
>  -   
>  | RAM_SAVE_FLAG_EOS |   | padding to next 1MB boundary |
>  -   | ...  |
>  | stream of pages   |   
>  | (iter 2)  |   | ramblock 2 pages |
>  | ...   |   | ...  |
>  -   
>  | ...   |   | ...  |
>  -   
>  | RAM_SAVE_FLAG_EOS|
>  
>  | ...  |
>  
> 
> where:
>  - ramblock header: the generic information for a ramblock, such as
>idstr, used_len, etc.
> 
>  - ramblock fixed-ram header: the new information added by this
>feature: bitmap of pages written, bitmap size and offset of pages
>in the migration file.
> 
> Signed-off-by: Nikolay Borisov 
> Signed-off-by: Fabiano Rosas 

Reviewed-by: Peter Xu 

Still one comment below:

[...]

> @@ -3187,6 +3288,18 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
>  return ret;
>  }
>  
> +if (migrate_fixed_ram()) {
> +ram_save_file_bmap(f);
> +
> +if (qemu_file_get_error(f)) {
> +Error *local_err = NULL;
> +int err = qemu_file_get_error_obj(f, &local_err);
> +
> +error_reportf_err(local_err, "Failed to write bitmap to file: ");

We always do error report if we set s->error.

Ideally I think we should have Error** passed to the caller and set
s->error there, instead of report here.  But the whole error handling is
still a bit of a mess, so I guess we can do anything on top.

> +return -err;
> +}
> +}

-- 
Peter Xu




[PATCH v4 13/34] migration/ram: Add outgoing 'fixed-ram' migration

2024-02-20 Thread Fabiano Rosas
Implement the outgoing migration side for the 'fixed-ram' capability.

A bitmap is introduced to track which pages have been written in the
migration file. Pages are written at a fixed location for every
ramblock. Zero pages are ignored as they'd be zero in the destination
migration as well.

The migration stream is altered to put the dirty pages for a ramblock
after its header instead of having a sequential stream of pages that
follow the ramblock headers.

Without fixed-ram (current):With fixed-ram (new):

 -   
 | ramblock 1 header |   | ramblock 1 header|
 -   
 | ramblock 2 header |   | ramblock 1 fixed-ram header  |
 -   
 | ...   |   | padding to next 1MB boundary |
 -   | ...  |
 | ramblock n header |   
 -   | ramblock 1 pages |
 | RAM_SAVE_FLAG_EOS |   | ...  |
 -   
 | stream of pages   |   | ramblock 2 header|
 | (iter 1)  |   
 | ...   |   | ramblock 2 fixed-ram header  |
 -   
 | RAM_SAVE_FLAG_EOS |   | padding to next 1MB boundary |
 -   | ...  |
 | stream of pages   |   
 | (iter 2)  |   | ramblock 2 pages |
 | ...   |   | ...  |
 -   
 | ...   |   | ...  |
 -   
 | RAM_SAVE_FLAG_EOS|
 
 | ...  |
 

where:
 - ramblock header: the generic information for a ramblock, such as
   idstr, used_len, etc.

 - ramblock fixed-ram header: the new information added by this
   feature: bitmap of pages written, bitmap size and offset of pages
   in the migration file.

Signed-off-by: Nikolay Borisov 
Signed-off-by: Fabiano Rosas 
---
 include/exec/ramblock.h |  13 
 migration/ram.c | 131 +---
 2 files changed, 135 insertions(+), 9 deletions(-)

diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
index 3eb79723c6..6a66301219 100644
--- a/include/exec/ramblock.h
+++ b/include/exec/ramblock.h
@@ -44,6 +44,19 @@ struct RAMBlock {
 size_t page_size;
 /* dirty bitmap used during migration */
 unsigned long *bmap;
+
+/*
+ * Below fields are only used by fixed-ram migration
+ */
+/* bitmap of pages present in the migration file */
+unsigned long *file_bmap;
+/*
+ * offset in the file pages belonging to this ramblock are saved,
+ * used only during migration to a file.
+ */
+off_t bitmap_offset;
+uint64_t pages_offset;
+
 /* bitmap of already received pages in postcopy */
 unsigned long *receivedmap;
 
diff --git a/migration/ram.c b/migration/ram.c
index 4649a81204..84c531722c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -94,6 +94,18 @@
 #define RAM_SAVE_FLAG_MULTIFD_FLUSH0x200
 /* We can't use any flag that is bigger than 0x200 */
 
+/*
+ * fixed-ram migration supports O_DIRECT, so we need to make sure the
+ * userspace buffer, the IO operation size and the file offset are
+ * aligned according to the underlying device's block size. The first
+ * two are already aligned to page size, but we need to add padding to
+ * the file to align the offset.  We cannot read the block size
+ * dynamically because the migration file can be moved between
+ * different systems, so use 1M to cover most block sizes and to keep
+ * the file offset aligned at page size as well.
+ */
+#define FIXED_RAM_FILE_OFFSET_ALIGNMENT 0x10
+
 XBZRLECacheStats xbzrle_counters;
 
 /* used by the search for pages to send */
@@ -1127,12 +1139,18 @@ static int save_zero_page(RAMState *rs, 
PageSearchStatus *pss,
 return 0;
 }
 
+stat64_add(&mig_stats.zero_pages, 1);
+
+if (migrate_fixed_ram()) {
+/* zero pages are not transferred with fixed-ram */
+clear_bit(offset >> TARGET_PAGE_BITS, pss->block->file_bmap);
+return 1;
+}
+
 len += save_page_header(pss, file, pss->block, offset | 
RAM_SAVE_FLAG_ZERO);