* Re: [Qemu-devel] KVM call agenda for Tuesday, June 19th
2012-06-19 13:54 ` Juan Quintela
@ 2012-06-19 14:01 ` Anthony Liguori
2012-06-19 14:34 ` Takuya Yoshikawa
2012-06-19 15:59 ` Michael Roth
1 sibling, 1 reply; 7+ messages in thread
From: Anthony Liguori @ 2012-06-19 14:01 UTC (permalink / raw)
To: quintela
Cc: Orit Wasserman, Michael Roth, qemu-devel, KVM devel mailing list,
Isaku Yamahata
On 06/19/2012 08:54 AM, Juan Quintela wrote:
> Juan Quintela<quintela@redhat.com> wrote:
>> Hi
>>
>> Please send in any agenda items you are interested in covering.
>>
>> Anthony suggested for last week:
>> - multithreading vhost (and general vhost improvements)
>>
>> I suggest:
>> - status of migration: post-copy, IDL, XBRLE, huge memory, ...
>> Will send an email with an status before tomorrow call.
>
> XBRLE: v12 is coming today or so.
>
>
> This three patches should be a no-brainer (just refactoring code).
> 1st one is shared with postcopy.
>
> [PATCH v11 1/9] Add MigrationParams structure
> [PATCH v11 5/9] Add uleb encoding/decoding functions
> [PATCH v11 6/9] Add save_block_hdr function
>
> This ones can be be the ones that we can discuss.
>
> [PATCH v11 2/9] Add migration capabilites
> [PATCH v11 3/9] Add XBZRLE documentation
> [PATCH v11 4/9] Add cache handling functions
> [PATCH v11 7/9] Add XBZRLE to ram_save_block and ram_save_live
> [PATCH v11 8/9] Add set_cachesize command
>
> Postcopy: This is just refactoring that can be integrated.
>
> [PATCH v2 01/41] arch_init: export sort_ram_list() and ram_save_block()
> [PATCH v2 02/41] arch_init: export RAM_SAVE_xxx flags for postcopy
> [PATCH v2 03/41] arch_init/ram_save: introduce constant for ram save version = 4
> [PATCH v2 04/41] arch_init: refactor host_from_stream_offset()
> [PATCH v2 05/41] arch_init/ram_save_live: factor out RAM_SAVE_FLAG_MEM_SIZE case
> [PATCH v2 06/41] arch_init: refactor ram_save_block()
> [PATCH v2 07/41] arch_init/ram_save_live: factor out ram_save_limit
> [PATCH v2 08/41] arch_init/ram_load: refactor ram_load
> [PATCH v2 09/41] arch_init: introduce helper function to find ram block with id string
> [PATCH v2 10/41] arch_init: simplify a bit by ram_find_block()
> [PATCH v2 11/41] arch_init: factor out counting transferred bytes
> [PATCH v2 12/41] arch_init: factor out setting last_block, last_offset
> [PATCH v2 13/41] exec.c: factor out qemu_get_ram_ptr()
> [PATCH v2 14/41] exec.c: export last_ram_offset()
> [PATCH v2 15/41] savevm: export qemu_peek_buffer, qemu_peek_byte, qemu_file_skip
> [PATCH v2 16/41] savevm: qemu_pending_size() to return pending buffered size
> [PATCH v2 17/41] savevm, buffered_file: introduce method to drain buffer of buffered file
> [PATCH v2 18/41] QEMUFile: add qemu_file_fd() for later use
> [PATCH v2 19/41] savevm/QEMUFile: drop qemu_stdio_fd
> [PATCH v2 20/41] savevm/QEMUFileSocket: drop duplicated member fd
> [PATCH v2 21/41] savevm: rename QEMUFileSocket to QEMUFileFD, socket_close to fd_close
> [PATCH v2 22/41] savevm/QEMUFile: introduce qemu_fopen_fd
> [PATCH v2 23/41] migration.c: remove redundant line in migrate_init()
> [PATCH v2 24/41] migration: export migrate_fd_completed() and migrate_fd_cleanup()
> [PATCH v2 25/41] migration: factor out parameters into MigrationParams
> [PATCH v2 26/41] buffered_file: factor out buffer management logic
> [PATCH v2 27/41] buffered_file: Introduce QEMUFileNonblock for nonblock write
> [PATCH v2 28/41] buffered_file: add qemu_file to read/write to buffer in memory
>
> This is postcopy properly. From this one, postcopy needs to be the
> things addressed on previous review, and from there probably (at least)
> another review. Thing to have in account is that the umem (or whatever
> you want to call it), should be able to work over RDMA. Anyone that
> knows anything about RDMA to comment on this?
>
> [PATCH v2 29/41] umem.h: import Linux umem.h
> [PATCH v2 30/41] update-linux-headers.sh: teach umem.h to update-linux-headers.sh
> [PATCH v2 31/41] configure: add CONFIG_POSTCOPY option
> [PATCH v2 32/41] savevm: add new section that is used by postcopy
> [PATCH v2 33/41] postcopy: introduce -postcopy and -postcopy-flags option
> [PATCH v2 34/41] postcopy outgoing: add -p and -n option to migrate command
> [PATCH v2 35/41] postcopy: introduce helper functions for postcopy
> [PATCH v2 36/41] postcopy: implement incoming part of postcopy live migration
> [PATCH v2 37/41] postcopy: implement outgoing part of postcopy live migration
> [PATCH v2 38/41] postcopy/outgoing: add forward, backward option to specify the size of prefault
> [PATCH v2 39/41] postcopy/outgoing: implement prefault
> [PATCH v2 40/41] migrate: add -m (movebg) option to migrate command
> [PATCH v2 41/41] migration/postcopy: add movebg mode
I'm not at all convinced that postcopy is a good idea. There needs a clear
expression of what the value proposition is that's backed by benchmarks. Those
benchmarks need to include latency measurements of downtime which so far, I've
not seen.
I don't want to take any postcopy patches until this discussion happens.
Regards,
Anthony Liguori
>
> Huge memory migration.
> This ones should be trivial, and integrated.
>
> [PATCH 1/7] Add spent time for migration
> [PATCH 2/7] Add tracepoints for savevm section start/end
> [PATCH 3/7] No need to iterate if we already are over the limit
> [PATCH 4/7] Only TCG needs TLB handling
> [PATCH 5/7] Only calculate expected_time for stage 2
>
> This one is also trivial, but Anthony on previous reviews wanted to have
> migration-thread before we integrated this one.
>
> [PATCH 6/7] Exit loop if we have been there too long
>
> This one, Anthony wanted a different approach improving bitmap
> handling. Not done yet.
>
> [PATCH 7/7] Maintaing number of dirty pages
>
> IDL patchset. I am not against generating the VMState information, but
> I am trying to understand how the patch works. Notice that I don't grok
> Python, this is is one of the reasos it is taking long.
>
> This was one of the 1st things that catched my eyes, that we have to
> hack fields for the 1st example looks bad, but I haven't really go
> through all the patchset.
>
> +def vmstate_field_hacks(node, field):
> + # yes, this is gonna get hairy. may want to move to a seperate file at
> + # some point
> + push_indent()
> + vms_field=""
> + if node.has_key('typedef') and node['typedef'] == 'RTCState':
> + if field['type'] == 'struct tm':
> + vms_field += mcgen('''
>
> Plan:
> - My plan would be to integrate the patches that I marked as ready on a
> tree, ask for a pull, and that the other migration people developed
> againts that tree, that way we can "integrate" the bits that are getting
> ready without stepping on the toes of everybody else.
>
> What do you think?
>
> Later, Juan.
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] KVM call agenda for Tuesday, June 19th
2012-06-19 13:54 ` Juan Quintela
2012-06-19 14:01 ` Anthony Liguori
@ 2012-06-19 15:59 ` Michael Roth
1 sibling, 0 replies; 7+ messages in thread
From: Michael Roth @ 2012-06-19 15:59 UTC (permalink / raw)
To: Juan Quintela
Cc: Orit Wasserman, Michael Roth, qemu-devel, KVM devel mailing list,
Isaku Yamahata
On Tue, Jun 19, 2012 at 03:54:23PM +0200, Juan Quintela wrote:
> Juan Quintela <quintela@redhat.com> wrote:
> > Hi
> >
> > Please send in any agenda items you are interested in covering.
> >
> > Anthony suggested for last week:
> > - multithreading vhost (and general vhost improvements)
> >
> > I suggest:
> > - status of migration: post-copy, IDL, XBRLE, huge memory, ...
> > Will send an email with an status before tomorrow call.
>
> XBRLE: v12 is coming today or so.
>
>
> This three patches should be a no-brainer (just refactoring code).
> 1st one is shared with postcopy.
>
> [PATCH v11 1/9] Add MigrationParams structure
> [PATCH v11 5/9] Add uleb encoding/decoding functions
> [PATCH v11 6/9] Add save_block_hdr function
>
> This ones can be be the ones that we can discuss.
>
> [PATCH v11 2/9] Add migration capabilites
> [PATCH v11 3/9] Add XBZRLE documentation
> [PATCH v11 4/9] Add cache handling functions
> [PATCH v11 7/9] Add XBZRLE to ram_save_block and ram_save_live
> [PATCH v11 8/9] Add set_cachesize command
>
> Postcopy: This is just refactoring that can be integrated.
>
> [PATCH v2 01/41] arch_init: export sort_ram_list() and ram_save_block()
> [PATCH v2 02/41] arch_init: export RAM_SAVE_xxx flags for postcopy
> [PATCH v2 03/41] arch_init/ram_save: introduce constant for ram save version = 4
> [PATCH v2 04/41] arch_init: refactor host_from_stream_offset()
> [PATCH v2 05/41] arch_init/ram_save_live: factor out RAM_SAVE_FLAG_MEM_SIZE case
> [PATCH v2 06/41] arch_init: refactor ram_save_block()
> [PATCH v2 07/41] arch_init/ram_save_live: factor out ram_save_limit
> [PATCH v2 08/41] arch_init/ram_load: refactor ram_load
> [PATCH v2 09/41] arch_init: introduce helper function to find ram block with id string
> [PATCH v2 10/41] arch_init: simplify a bit by ram_find_block()
> [PATCH v2 11/41] arch_init: factor out counting transferred bytes
> [PATCH v2 12/41] arch_init: factor out setting last_block, last_offset
> [PATCH v2 13/41] exec.c: factor out qemu_get_ram_ptr()
> [PATCH v2 14/41] exec.c: export last_ram_offset()
> [PATCH v2 15/41] savevm: export qemu_peek_buffer, qemu_peek_byte, qemu_file_skip
> [PATCH v2 16/41] savevm: qemu_pending_size() to return pending buffered size
> [PATCH v2 17/41] savevm, buffered_file: introduce method to drain buffer of buffered file
> [PATCH v2 18/41] QEMUFile: add qemu_file_fd() for later use
> [PATCH v2 19/41] savevm/QEMUFile: drop qemu_stdio_fd
> [PATCH v2 20/41] savevm/QEMUFileSocket: drop duplicated member fd
> [PATCH v2 21/41] savevm: rename QEMUFileSocket to QEMUFileFD, socket_close to fd_close
> [PATCH v2 22/41] savevm/QEMUFile: introduce qemu_fopen_fd
> [PATCH v2 23/41] migration.c: remove redundant line in migrate_init()
> [PATCH v2 24/41] migration: export migrate_fd_completed() and migrate_fd_cleanup()
> [PATCH v2 25/41] migration: factor out parameters into MigrationParams
> [PATCH v2 26/41] buffered_file: factor out buffer management logic
> [PATCH v2 27/41] buffered_file: Introduce QEMUFileNonblock for nonblock write
> [PATCH v2 28/41] buffered_file: add qemu_file to read/write to buffer in memory
>
> This is postcopy properly. From this one, postcopy needs to be the
> things addressed on previous review, and from there probably (at least)
> another review. Thing to have in account is that the umem (or whatever
> you want to call it), should be able to work over RDMA. Anyone that
> knows anything about RDMA to comment on this?
>
> [PATCH v2 29/41] umem.h: import Linux umem.h
> [PATCH v2 30/41] update-linux-headers.sh: teach umem.h to update-linux-headers.sh
> [PATCH v2 31/41] configure: add CONFIG_POSTCOPY option
> [PATCH v2 32/41] savevm: add new section that is used by postcopy
> [PATCH v2 33/41] postcopy: introduce -postcopy and -postcopy-flags option
> [PATCH v2 34/41] postcopy outgoing: add -p and -n option to migrate command
> [PATCH v2 35/41] postcopy: introduce helper functions for postcopy
> [PATCH v2 36/41] postcopy: implement incoming part of postcopy live migration
> [PATCH v2 37/41] postcopy: implement outgoing part of postcopy live migration
> [PATCH v2 38/41] postcopy/outgoing: add forward, backward option to specify the size of prefault
> [PATCH v2 39/41] postcopy/outgoing: implement prefault
> [PATCH v2 40/41] migrate: add -m (movebg) option to migrate command
> [PATCH v2 41/41] migration/postcopy: add movebg mode
>
> Huge memory migration.
> This ones should be trivial, and integrated.
>
> [PATCH 1/7] Add spent time for migration
> [PATCH 2/7] Add tracepoints for savevm section start/end
> [PATCH 3/7] No need to iterate if we already are over the limit
> [PATCH 4/7] Only TCG needs TLB handling
> [PATCH 5/7] Only calculate expected_time for stage 2
>
> This one is also trivial, but Anthony on previous reviews wanted to have
> migration-thread before we integrated this one.
>
> [PATCH 6/7] Exit loop if we have been there too long
>
> This one, Anthony wanted a different approach improving bitmap
> handling. Not done yet.
>
> [PATCH 7/7] Maintaing number of dirty pages
>
> IDL patchset. I am not against generating the VMState information, but
> I am trying to understand how the patch works. Notice that I don't grok
> Python, this is is one of the reasos it is taking long.
I've done a lot of clean up on the parsing/codegen stuff and
stream-lined the process for QIDL-fying devices, so hopefully the next series
reads a lot easier.
>
> This was one of the 1st things that catched my eyes, that we have to
> hack fields for the 1st example looks bad, but I haven't really go
> through all the patchset.
>
> +def vmstate_field_hacks(node, field):
> + # yes, this is gonna get hairy. may want to move to a seperate file at
> + # some point
> + push_indent()
> + vms_field=""
> + if node.has_key('typedef') and node['typedef'] == 'RTCState':
> + if field['type'] == 'struct tm':
> + vms_field += mcgen('''
But I think this is gonna happen fairly often. Part of the issue is that
VMStateDescriptions are very open-ended. In this particular case we can:
1) access the embedded structs via another VMSD (in which case we need
to add annotations that specify the name of the VMSD so we can generate
the right VMSTATE_STRUCT field)
2) "flatten" the struct and reference the individual members of the embedded
struct as "primitives" (as we do in this case, in which case we need
to hard code because we have way of inferring the fields for
structures created out of tree, a human needs to do it)
3) even just send it as a byte array (in which case we need to hard code
because we can only guess that the "reasonable" default was taken)
Don't think we generally do 3) outside of the legacy save/load functions,
thankfully, but we have that kind of flexibility with vmstate.
Theoretically the hardcoded hacks could be done away with by making QIDL as
expressive as VMState, but that would only result in a more complicated
vmstate.
There are some benefits to the code generation for vmstate though, in
the cases where the hacks aren't too extensive. The main one being that
we can build a make check around the infrastructure to detect when a
change to a QIDL-fied device's state has resulted in a change in the
VMSD. The common case of switching a field from 32 to 64 bits for
instance would get flagged by make check unless the change to the VMSD
was explicitly checked in.
The next version of the series will also make it easier to select when
to enable code generation for things like vmstate so we can avoid using it
in places where it doesn't amount to much more than a big block in the
vmstate_hacks() function (which wouldn't benefit from the make check
functionality)
But the core goal with the series/QIDL is device serialization through QOM
properties/generated visitors, the vmstate/properties stuff is a best effort
to add incremental value while we're touching all these devices in cases
where it makes sense.
>
> Plan:
> - My plan would be to integrate the patches that I marked as ready on a
> tree, ask for a pull, and that the other migration people developed
> againts that tree, that way we can "integrate" the bits that are getting
> ready without stepping on the toes of everybody else.
>
> What do you think?
>
> Later, Juan.
>
^ permalink raw reply [flat|nested] 7+ messages in thread