| Message ID | 1325604879-15862-5-git-send-email-owasserm@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jan 3, 2012 at 3:34 PM, Orit Wasserman <owasserm@redhat.com> wrote: > +static inline void *host_from_stream_offset_versioned(int version_id, > + QEMUFile *f, ram_addr_t offset, int flags) > +{ > + void *host; > + if (version_id == 3) { > + host = qemu_get_ram_ptr(offset); > + } else { > + host = host_from_stream_offset(f, offset, flags); > + } > + if (!host) { > + fprintf(stderr, "Failed to convert RAM address to host" > + " for offset 0x%lX!\n", offset); > + abort(); > + } Please use RAM_ADDR_FMT instead of %lX. Aborting isn't ideal but I guess there is nothing else we can do at this point. Stefan
On 01/04/2012 06:00 AM, Stefan Hajnoczi wrote: > On Tue, Jan 3, 2012 at 3:34 PM, Orit Wasserman<owasserm@redhat.com> wrote: >> +static inline void *host_from_stream_offset_versioned(int version_id, >> + QEMUFile *f, ram_addr_t offset, int flags) >> +{ >> + void *host; >> + if (version_id == 3) { >> + host = qemu_get_ram_ptr(offset); >> + } else { >> + host = host_from_stream_offset(f, offset, flags); >> + } >> + if (!host) { >> + fprintf(stderr, "Failed to convert RAM address to host" >> + " for offset 0x%lX!\n", offset); >> + abort(); >> + } > > Please use RAM_ADDR_FMT instead of %lX. > > Aborting isn't ideal but I guess there is nothing else we can do at this point. > Currently we we return -EINVAL to qemu_loadvm_state() when !host, which prints potentially useful warnings and exits gracefully for migration, and for savevm we print the warnings and fire up the main_loop with vcpus paused... Not sure what the rationale is for the latter, but it may have some utility (debugging maybe?). In any case, since the error paths are covered in some fashion I don't think we should resort to aborting here, maybe just print the warnings and check for NULL return in the callers as before. > Stefan >
diff --git a/arch_init.c b/arch_init.c index e87dfbc..05b8053 100644 --- a/arch_init.c +++ b/arch_init.c @@ -613,6 +613,23 @@ static inline void *host_from_stream_offset(QEMUFile *f, return NULL; } +static inline void *host_from_stream_offset_versioned(int version_id, + QEMUFile *f, ram_addr_t offset, int flags) +{ + void *host; + if (version_id == 3) { + host = qemu_get_ram_ptr(offset); + } else { + host = host_from_stream_offset(f, offset, flags); + } + if (!host) { + fprintf(stderr, "Failed to convert RAM address to host" + " for offset 0x%lX!\n", offset); + abort(); + } + return host; +} + int ram_load(QEMUFile *f, void *opaque, int version_id) { ram_addr_t addr; @@ -669,17 +686,10 @@ int ram_load(QEMUFile *f, void *opaque, int version_id) } if (flags & RAM_SAVE_FLAG_COMPRESS) { - void *host; uint8_t ch; - if (version_id == 3) - host = qemu_get_ram_ptr(addr); - else - host = host_from_stream_offset(f, addr, flags); - if (!host) { - return -EINVAL; - } - + host = host_from_stream_offset_versioned(version_id, + f, addr, flags); ch = qemu_get_byte(f); memset(host, ch, TARGET_PAGE_SIZE); #ifndef _WIN32 @@ -691,11 +701,8 @@ int ram_load(QEMUFile *f, void *opaque, int version_id) } else if (flags & RAM_SAVE_FLAG_PAGE) { void *host; - if (version_id == 3) - host = qemu_get_ram_ptr(addr); - else - host = host_from_stream_offset(f, addr, flags); - + host = host_from_stream_offset_versioned(version_id, + f, addr, flags); qemu_get_buffer(f, host, TARGET_PAGE_SIZE); } error = qemu_file_get_error(f);
Signed-off-by: Orit Wasserman <owasserm@redhat.com> --- arch_init.c | 35 +++++++++++++++++++++-------------- 1 files changed, 21 insertions(+), 14 deletions(-)