[v5,4/9] Add host_from_stream_offset_versioned function

Submitted by Orit Wasserman on Jan. 3, 2012, 3:34 p.m.

Details

Message ID 1325604879-15862-5-git-send-email-owasserm@redhat.com
State New
Headers show

Commit Message

Orit Wasserman Jan. 3, 2012, 3:34 p.m.
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 arch_init.c |   35 +++++++++++++++++++++--------------
 1 files changed, 21 insertions(+), 14 deletions(-)

Comments

Stefan Hajnoczi Jan. 4, 2012, noon
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
Michael Roth Jan. 4, 2012, 8:59 p.m.
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
>

Patch hide | download patch | download mbox

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);