At 03/01/2012 03:11 PM, HATAYAMA Daisuke Wrote: > From: Wen Congyang <we...@cn.fujitsu.com> > Subject: Re: [RFC][PATCH 04/14 v7] Add API to get memory mapping > Date: Thu, 01 Mar 2012 14:17:53 +0800 > >> At 03/01/2012 02:01 PM, HATAYAMA Daisuke Wrote: >>> From: Wen Congyang <we...@cn.fujitsu.com> >>> Subject: [RFC][PATCH 04/14 v7] Add API to get memory mapping >>> Date: Thu, 01 Mar 2012 10:43:13 +0800 >>> >>>> +int qemu_get_guest_memory_mapping(MemoryMappingList *list) >>>> +{ >>>> + CPUState *env; >>>> + MemoryMapping *memory_mapping; >>>> + RAMBlock *block; >>>> + ram_addr_t offset, length; >>>> + int ret; >>>> + >>>> +#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING) >>>> + for (env = first_cpu; env != NULL; env = env->next_cpu) { >>>> + ret = cpu_get_memory_mapping(list, env); >>>> + if (ret < 0) { >>>> + return -1; >>>> + } >>>> + } >>>> +#else >>>> + return -2; >>>> +#endif >>>> + >>>> + /* some memory may be not mapped, add them into memory mapping's list >>>> */ >>> >>> The part from here is logic fully for 2nd kernel? If so, I think it >>> better to describe why this addtional mapping is needed; we should >>> assume most people doesn't know kdump mechanism. >> >> Not only for 2nd kernel. If the guest has 1 vcpu, and is in the 2nd kernel, >> we need this logic for 1st kernel. >> > > So you should describe two cases explicitly. I don't understand them > from ``some memory''. > > and please consider cleanup below too. Conditionals over two lines are > hard to read.
OK. I will fix it. Thanks Wen Congyang > >>> >>> I think it more readable if shortening memory_mapping->phys_addr and >>> memmory_maping->length at the berinning of the innermost foreach loop. >>> >>> m_phys_addr = memory_mapping->phys_addr; >>> m_length = memory_mapping->length; >>> >>> Then, each conditionals becomes compact. > > Thanks. > HATAYAMA, Daisuke > >