On 2015/12/3 15:19, Hailiang Zhang wrote:
On 2015/12/2 2:19, Dr. David Alan Gilbert wrote:
* 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.
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
Keeping the name host_from_ram_block_offset() seems better, it corresponds with
the name
colo_cache_from_block_offset() in next patch. ;)
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 / dgilb...@redhat.com / Manchester, UK
.
