From: Wen Congyang <we...@cn.fujitsu.com> Subject: Re: [PATCH 11/11 v10] introduce a new monitor command 'dump-guest-memory' to dump guest's memory Date: Mon, 26 Mar 2012 09:39:24 +0800
> At 03/23/2012 05:40 PM, HATAYAMA Daisuke Wrote: >> From: Wen Congyang <we...@cn.fujitsu.com> >> Subject: [PATCH 11/11 v10] introduce a new monitor command >> 'dump-guest-memory' to dump guest's memory >> Date: Tue, 20 Mar 2012 11:57:43 +0800 >> >>> +/* get the memory's offset in the vmcore */ >>> +static target_phys_addr_t get_offset(target_phys_addr_t phys_addr, >>> + DumpState *s) >>> +{ >>> + RAMBlock *block; >>> + target_phys_addr_t offset = s->memory_offset; >>> + int64_t size_in_block, start; >>> + >>> + if (s->has_filter) { >>> + if (phys_addr < s->begin || phys_addr >= s->begin + s->length) { >>> + return -1; >>> + } >>> + } >>> + >>> + QLIST_FOREACH(block, &ram_list.blocks, next) { >>> + if (s->has_filter) { >>> + if (block->offset >= s->begin + s->length || >>> + block->offset + block->length <= s->begin) { >>> + /* This block is out of the range */ >>> + continue; >>> + } >>> + >>> + if (s->begin <= block->offset) { >>> + start = block->offset; >>> + } else { >>> + start = s->begin; >>> + } >>> + >>> + size_in_block = block->length - (start - block->offset); >>> + if (s->begin + s->length < block->offset + block->length) { >>> + size_in_block -= block->offset + block->length - >>> + (s->begin + s->length); >>> + } >>> + } else { >>> + start = block->offset; >>> + size_in_block = block->length; >>> + } >>> + >>> + if (phys_addr >= start && phys_addr < start + size_in_block) { >>> + return phys_addr - start + offset; >>> + } >>> + >>> + offset += size_in_block; >>> + } >>> + >>> + return -1; >>> +} >> >> OK. -1 is assigned to offset when the corresponding memory is filtered >> and so not contained in dumpfile. But please give -1 a name. I want >> the name to tell me the data is filterred. >> >> Also, I couldn't imagine get_offset() does filtering processing. This >> is important for the purspective of dump because there's data not >> saved in dumpfile. Could you claify this in some way? By moving >> filtering processing to the outside, or splitting it into anotehr >> funciton. > > offset should not be -1. I remove the PT_LOAD in the function > memory_mapping_filter(). > In the previous patchset, Luiz said he wants to squash the filtering > feature to this patch. > I didn't noticed that the patch was moved... _I_ think it should be presented in a single patch. Looking at memory_mapping_filter(), PT_LOAD is configured so that memory that is filtered doesn't exist. I think it better to keep such mapping with p_filesz == 0 to indicate to users the mapping is filtered. Also, get_offset() loops on ram_list.blocks each time it is called. Because dump_completed() loops memory_mapping list, offset can be calculated incrementally. Thanks. HATAYAMA, Daisuke