* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote: > Split host_from_stream_offset() into two parts: > One is to get ram block, which the block idstr may be get from migration > stream, the other is to get hva (host) address from block and the offset. > > Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
OK, I see why you're doing this from the next patch. > --- > v11: > - New patch > --- > migration/ram.c | 29 +++++++++++++++++++++-------- > 1 file changed, 21 insertions(+), 8 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index cfe78aa..a161620 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2136,9 +2136,9 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, > void *host) > * offset: Offset within the block > * flags: Page flags (mostly to see if it's a continuation of previous block) > */ > -static inline void *host_from_stream_offset(QEMUFile *f, > - ram_addr_t offset, > - int flags) > +static inline RAMBlock *ram_block_from_stream(QEMUFile *f, > + ram_addr_t offset, > + int flags) > { > static RAMBlock *block = NULL; > char id[256]; > @@ -2150,22 +2150,31 @@ static inline void *host_from_stream_offset(QEMUFile > *f, > return NULL; > } > > - return block->host + offset; > + return block; > } > - > len = qemu_get_byte(f); > qemu_get_buffer(f, (uint8_t *)id, len); > id[len] = 0; > > block = qemu_ram_block_by_name(id); > if (block && block->max_length > offset) { > - return block->host + offset; > + return block; > } > > error_report("Can't find block %s", id); > return NULL; > } > > +static inline void *host_from_ram_block_offset(RAMBlock *block, > + ram_addr_t offset) > +{ > + if (!block) { > + return NULL; > + } > + > + return block->host + offset; > +} That's almost the same as ramblock_ptr in include/exec/ram_addr.h, but it assert's rather than doing NULL on errors. I'm not sure about this, but can I suggest: ram_block_from_stream(QEMUFile *f, int flags) doesn't have the offset; just finds the block and handles the CONT. bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset); actually does the check; put this in exec.c, and declare it in include/exec/ram_addr.h void *ramblock_ptr_try(RAMBlock *block, ram_addr_t offset) which returns NULL if offset_in_ramblock fails, and otherwise returns the result of ramblock_ptr - again put that in include/exec/ram_addr.h (I'm not sure about this - I almost suggested changing ramblock_ptr to not do the checks, and just add a call to assert(offset_in_ramblock) before each use, but that sounded too painful). Hmm - we check here for block->max_length > offset - where as the check in ram_addr.h is used_length - I wonder if we should be using used_length? Dave > + > /* > * If a page (or a whole RDMA chunk) has been > * determined to be zero, then zap it. > @@ -2310,7 +2319,9 @@ static int ram_load_postcopy(QEMUFile *f) > trace_ram_load_postcopy_loop((uint64_t)addr, flags); > place_needed = false; > if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE)) { > - host = host_from_stream_offset(f, addr, flags); > + RAMBlock *block = ram_block_from_stream(f, addr, flags); > + > + host = host_from_ram_block_offset(block, addr); > if (!host) { > error_report("Illegal RAM offset " RAM_ADDR_FMT, addr); > ret = -EINVAL; > @@ -2441,7 +2452,9 @@ static int ram_load(QEMUFile *f, void *opaque, int > version_id) > > if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE | > RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) { > - host = host_from_stream_offset(f, addr, flags); > + RAMBlock *block = ram_block_from_stream(f, addr, flags); > + > + host = host_from_ram_block_offset(block, addr); > if (!host) { > error_report("Illegal RAM offset " RAM_ADDR_FMT, addr); > ret = -EINVAL; > -- > 1.8.3.1 > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK