From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50754) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VClw5-0005X7-3D for qemu-devel@nongnu.org; Fri, 23 Aug 2013 03:37:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VClvz-0008P5-4D for qemu-devel@nongnu.org; Fri, 23 Aug 2013 03:37:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45007) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VClvy-0008Os-Sc for qemu-devel@nongnu.org; Fri, 23 Aug 2013 03:37:27 -0400 Message-ID: <52171104.3020002@redhat.com> Date: Fri, 23 Aug 2013 09:36:36 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1377069536-12658-1-git-send-email-lilei@linux.vnet.ibm.com> <1377069536-12658-8-git-send-email-lilei@linux.vnet.ibm.com> <52149B56.4090407@redhat.com> <52167132.5070102@linux.vnet.ibm.com> In-Reply-To: <52167132.5070102@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 07/18] arch_init: export RAM_SAVE_xxx flags List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael R. Hines" Cc: aarcange@redhat.com, aliguori@us.ibm.com, Lei Li , quintela@redhat.com, qemu-devel@nongnu.org, lagarcia@br.ibm.com, rcj@linux.vnet.ibm.com 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 >>> --- >>> 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