Re: [Qemu-devel] [PATCH 07/18] arch_init: export RAM_SAVE_xxx flags

["Michael R. Hines" <mrhines@linux.vnet.ibm.com>]

On 08/21/2013 06:49 AM, Paolo Bonzini wrote:
> Il 21/08/2013 09:18, Lei Li ha scritto:
>> Export RAM_SAVE_xxx flags for localhost migration.
>>
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>> ---
>>   arch_init.c                   |   12 ------------
>>   include/migration/migration.h |   14 ++++++++++++++
>>   2 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index 68a7ab7..1ea7c29 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -108,18 +108,6 @@ static bool mig_throttle_on;
>>   static int dirty_rate_high_cnt;
>>   static void check_guest_throttling(void);
>>   
>> -/***********************************************************/
>> -/* ram save/restore */
>> -
>> -#define RAM_SAVE_FLAG_FULL     0x01 /* Obsolete, not used anymore */
>> -#define RAM_SAVE_FLAG_COMPRESS 0x02
>> -#define RAM_SAVE_FLAG_MEM_SIZE 0x04
>> -#define RAM_SAVE_FLAG_PAGE     0x08
>> -#define RAM_SAVE_FLAG_EOS      0x10
>> -#define RAM_SAVE_FLAG_CONTINUE 0x20
>> -#define RAM_SAVE_FLAG_XBZRLE   0x40
>> -/* 0x80 is reserved in migration.h start with 0x100 next */
>> -
>>   
>>   static struct defconfig_file {
>>       const char *filename;
>> diff --git a/include/migration/migration.h b/include/migration/migration.h
>> index 6a24e65..5336117 100644
>> --- a/include/migration/migration.h
>> +++ b/include/migration/migration.h
>> @@ -158,12 +158,26 @@ void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
>>   void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
>>   void ram_control_load_hook(QEMUFile *f, uint64_t flags);
>>   
>> +
>> +/***********************************************************/
>> +/* ram save/restore */
>> +
>> +#define RAM_SAVE_FLAG_FULL     0x01 /* Obsolete, not used anymore */
>> +#define RAM_SAVE_FLAG_COMPRESS 0x02
>> +#define RAM_SAVE_FLAG_MEM_SIZE 0x04
>> +#define RAM_SAVE_FLAG_PAGE     0x08
>> +#define RAM_SAVE_FLAG_EOS      0x10
>> +#define RAM_SAVE_FLAG_CONTINUE 0x20
>> +#define RAM_SAVE_FLAG_XBZRLE   0x40
>> +
>>   /* Whenever this is found in the data stream, the flags
>>    * will be passed to ram_control_load_hook in the incoming-migration
>>    * side. This lets before_ram_iterate/after_ram_iterate add
>>    * transport-specific sections to the RAM migration data.
>>    */
>>   #define RAM_SAVE_FLAG_HOOK     0x80
>> +/* Start with 0x100 next */
>> +
>>   
>>   #define RAM_SAVE_CONTROL_NOT_SUPP -1000
>>   #define RAM_SAVE_CONTROL_DELAYED  -2000
>>
> This also looks like an encapsulation violation.
>
> Localhost migration is not very different in concept from RDMA (except
> that it runs with the VM stopped, but that's just because you do
> MADV_DONTNEED---it's not specific to the migration transport), and it
> manages to do everything without touching arch_init.c and migration.c.
>
> Paolo
>

I think maybe we need a private header file here. Multiple parties want 
to add
potentially add to this flag list without modifying arch_init.c.

How about something like include/migration/shared.h or common.h? or 
Something like that?

It could hold both the state machine flags from migration.c as well as 
the RAM_SAVE flags.....

- Michael