Re: [Crash-utility] [PATCH] add support to "virsh dump-guest-memory"(qemu memory dump)

----- Original Message ----- > At 2012-8-20 16:32, qiaonuohan wrote: > > > > I have modified the patches, and they are based on crash 6.0.9. > > I find some mistakes in my former patches, please ignore them and refer > to the attachments of this mail. > > -- > -- > Regards > Qiao Nuohan The patch is looking better, but a few issues still remain to be cleaned up. I'm wondering about the correctness of this addition to read_netdump() for 32-bit dumpfiles: int read_netdump(int fd, void *bufptr, int cnt, ulong addr, physaddr_t paddr) { off_t offset; struct pt_load_segment *pls; int i; + if (nd->flags & QEMU_MEM_DUMP_KDUMP_BACKUP && + paddr >= nd->backup_src_start && + paddr < nd->backup_src_start + nd->backup_src_size) { + ulong orig_paddr; + + orig_paddr = paddr; + paddr += nd->backup_offset - nd->backup_src_start; + + if (CRASHDEBUG(1)) + error(INFO, + "qemu_mem_dump: kdump backup region: %#lx => %#lx\n", + orig_paddr, paddr); + } The incoming "paddr" parameter is type physaddr_t, which is declared as: typedef uint64_t physaddr_t; I see that you've pretty much copied the "if" statement from read_sadump(), but I'm not sure whether SADUMP supports 32-bit dumpfiles?

Since nd->backup_src_start is a physical address, maybe it should be declared as a physaddr_t as well? Or if the incoming paddr is 4GB or greater, then perhaps it shouldn't be checked against nd->backup_src_start. In other words, I'm not sure what happens when you do this: paddr >= nd->backup_src_start When running on a 32-bit machine, does paddr get truncated to a 32-bit value, or does nd->backup_src_start get promoted to a 64-bit value?