Patchwork [v5,4/9] Add host_from_stream_offset_versioned function

login
register
mail settings
Submitter Orit Wasserman
Date Jan. 3, 2012, 3:34 p.m.
Message ID <1325604879-15862-5-git-send-email-owasserm@redhat.com>
Download mbox | patch
Permalink /patch/134020/
State New
Headers show

Comments

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

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