Il 22/08/2013 22:14, Michael R. Hines ha scritto: > 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 <li...@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. > > 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.
There is a mechanism for private flags, which is the page-load hook we introduced for RDMA. The existing RAM_SAVE_FLAG_* should not be needed outside arch_init.c. MIG_STATE_* might be needed elsewhere, but the right way to export it is this one: change MigrationInfo's status field from a string to a QAPI enum, and remove MIG_STATE_* altogether in favor of the QAPI enum. Paolo