From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33893) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W0ePD-0001oS-PD for qemu-devel@nongnu.org; Tue, 07 Jan 2014 16:41:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W0eP7-0000PI-Ia for qemu-devel@nongnu.org; Tue, 07 Jan 2014 16:41:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57311) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W0eP7-0000P4-AG for qemu-devel@nongnu.org; Tue, 07 Jan 2014 16:41:41 -0500 Message-ID: <52CC748A.1070003@redhat.com> Date: Tue, 07 Jan 2014 22:41:30 +0100 From: Laszlo Ersek MIME-Version: 1.0 References: <1388906864-1083-1-git-send-email-qiaonuohan@cn.fujitsu.com> <1388906864-1083-8-git-send-email-qiaonuohan@cn.fujitsu.com> <52CC1412.4050101@redhat.com> In-Reply-To: <52CC1412.4050101@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 07/11] dump: Add API to write dump_bitmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Qiao Nuohan Cc: stefanha@gmail.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, anderson@redhat.com, kumagai-atsushi@mxc.nes.nec.co.jp, akong@redhat.com, afaerber@suse.de On 01/07/14 15:49, Laszlo Ersek wrote: > > On 01/05/14 08:27, Qiao Nuohan wrote: >> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h >> index 9e47b4c..b5eaf8d 100644 >> --- a/include/sysemu/dump.h >> +++ b/include/sysemu/dump.h >> @@ -27,11 +27,18 @@ >> #define DUMP_DH_COMPRESSED_LZO (0x2) >> #define DUMP_DH_COMPRESSED_SNAPPY (0x4) >> >> +#define PAGE_SIZE (4096) >> #define KDUMP_SIGNATURE "KDUMP " >> #define SIG_LEN (sizeof(KDUMP_SIGNATURE) - 1) >> #define PHYS_BASE (0) >> #define DUMP_LEVEL (1) >> #define DISKDUMP_HEADER_BLOCKS (1) >> +#define BUFSIZE_BITMAP (PAGE_SIZE) >> +#define PFN_BUFBITMAP (CHAR_BIT * BUFSIZE_BITMAP) >> +#define ARCH_PFN_OFFSET (0) >> + >> +#define paddr_to_pfn(X, page_shift) \ >> + (((unsigned long long)(X) >> (page_shift)) - ARCH_PFN_OFFSET) >> >> typedef struct ArchDumpInfo { >> int d_machine; /* Architecture */ >> > > I think these magic constants are somewhat tied to x86, and therefore > should be in an arch-specific file rather than a common file, but > whoever wants to extend this to another architecture can do that. Stressing the argument a bit more for PAGE_SIZE specifically: - we already have TARGET_PAGE_SIZE, maybe that would be a better choice, - PAGE_SIZE is defined *as* TARGET_PAGE_SIZE in kvm-all.c. There's no actual conflict, but the mental conflict is bad enough. Anyway my R-b stands. Laszlo