|
From: | Hailiang Zhang |
Subject: | Re: [Qemu-devel] [PATCH COLO-Frame v11 14/39] ram: Split host_from_stream_offset() into two helper functions |
Date: | Thu, 3 Dec 2015 15:19:31 +0800 |
User-agent: | Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 |
On 2015/12/2 2:19, Dr. David Alan Gilbert wrote:
* zhanghailiang (address@hidden) 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 <address@hidden>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.
Hmm, i didn't notice that before.
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
That sounds good. I will add the ramblock_ptr_try() and keep the ramblock_ptr(). They have different behavior when the check failed. I'm not supposed to break it.
(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).
Yes, that's not so good.
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?
Er, i think we should use used_length instead of max_length. Thanks, zhanghailiang
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 / address@hidden / Manchester, UK .
[Prev in Thread] | Current Thread | [Next in Thread] |