Re: [Qemu-devel] [PATCH 0/6] migration: differentiate between pages and bytes

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Juan Quintela (quintela@redhat.com) wrote:
> Hi
> 
> (Li special edition)
> 
> Current migration code returns number of bytes transferred and from
> there we decide if we.have sent something or not.  Problem, we need
> two results: number of pages written, and number of bytes written
> (depending on compression, zero pages, etc, it is not possible to
> derive one value from the other).
> 
> So, I changed all relevant function to return the number of written
> pages, and then pass as uint64_t *bytes_transferred to update the
> written bytes.
> 
> On current code, makes things a bit easier to understand, but is not
> strictely necesary.  But for the compression patches from Li, it makes
> a big difference, we can return that we have written a page, even if
> we have just started the write, but having writtten in reality zero
> bytes.
> 
> Once there, I add doxygen documentation to all function that I touched
> (yes, I was long due).
> 
> save_block_hdr really saved headers for pages, not blocks.  Rename it,
> and simplify the interface.
> 
> Li, does this make your life easier?  I hope so.  Should make really
> easy to remove the one_bytes_sent "hack", and allow my other suggestions.
> 
> Comments?

I like it; it generally seems to make sense to separate the concept
of whether we've actually sent any pages from the actual byte counting.

While you're there though; do we actually care about bean counting
the individual header bytes?  For example the &bytes_transferred += 1 in the
RAM_SAVE_FLAG_COMPRESS case where it puts the 0, or the EOS mark where
we add 8 bytes?   Do we need to keep track of anything other than
stuff that's potentially big enough to make an impact on the
bandwidth calculations?

Dave

> 
> Later, Juan.
> 
> PD.  Code is obviously correct, but I run it through virt-test
>      anyways.  And it failed :-(  In despair, I tried virt-test on HEAD,
>      and it also failed, so ....  I add another thing to my ToDo list.
> 
> PD2.  Modifying ram_save_page() has become complicated, very
>       complicated. I think that know a couple of simplications, but
>       require touching RDMA code, and anyways, fixing migration in
>       "master" looks like a higher priority.
> 
> 
> Juan Quintela (6):
>   ram: make all save_page functions take a uint64_t parameter
>   ram_find_and_save_block:  change calling convention
>   ram_save_page: change calling covention
>   save_xbzrle_page: change calling convention
>   save_block_hdr: we can recalculate the cont parameter here
>   rename save_block_hdr to save_page_header
> 
>  arch_init.c                   | 176 ++++++++++++++++++++++++++----------------
>  include/migration/migration.h |   2 +-
>  include/migration/qemu-file.h |   2 +-
>  migration/qemu-file.c         |   3 +-
>  migration/rdma.c              |   2 +-
>  5 files changed, 114 insertions(+), 71 deletions(-)
> 
> -- 
> 2.1.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK