Patchwork [v14,07/13] Add debugging infrastructure

login
register
mail settings
Submitter Orit Wasserman
Date July 3, 2012, 1:52 p.m.
Message ID <1341323574-23206-8-git-send-email-owasserm@redhat.com>
Download mbox | patch
Permalink /patch/168814/
State New
Headers show

Comments

Orit Wasserman - July 3, 2012, 1:52 p.m.
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 arch_init.c |   33 +++++++++++++++++++++++++++------
 1 files changed, 27 insertions(+), 6 deletions(-)
Blue Swirl - July 3, 2012, 7:25 p.m.
On Tue, Jul 3, 2012 at 1:52 PM, Orit Wasserman <owasserm@redhat.com> wrote:
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> ---
>  arch_init.c |   33 +++++++++++++++++++++++++++------
>  1 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 9dafb6e..ee20c33 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -44,6 +44,14 @@
>  #include "exec-memory.h"
>  #include "hw/pcspk.h"
>
> +#ifdef DEBUG_ARCH_INIT
> +#define DPRINTF(fmt, ...) \
> +    do { fprintf(stdout, "arch_init: " fmt, ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) \
> +    do { } while (0)
> +#endif

I think you missed my comments to the version that Juan sent, about
using trace points. Also in Juan's version the %lds were changed to
PRId64s which now are reverted, why?

> +
>  #ifdef TARGET_SPARC
>  int graphic_width = 1024;
>  int graphic_height = 768;
> @@ -380,6 +388,9 @@ int ram_save_live(QEMUFile *f, int stage, void *opaque)
>
>      expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
>
> +    DPRINTF("ram_save_live: expected(%ld) <= max(%ld)?\n", expected_time,
> +        migrate_max_downtime());
> +
>      return (stage == 2) && (expected_time <= migrate_max_downtime());
>  }
>
> @@ -416,8 +427,11 @@ static inline void *host_from_stream_offset(QEMUFile *f,
>  int ram_load(QEMUFile *f, void *opaque, int version_id)
>  {
>      ram_addr_t addr;
> -    int flags;
> +    int flags, ret = 0;
>      int error;
> +    static uint64_t seq_iter;
> +
> +    seq_iter++;
>
>      if (version_id < 4 || version_id > 4) {
>          return -EINVAL;
> @@ -447,8 +461,10 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>
>                      QLIST_FOREACH(block, &ram_list.blocks, next) {
>                          if (!strncmp(id, block->idstr, sizeof(id))) {
> -                            if (block->length != length)
> -                                return -EINVAL;
> +                            if (block->length != length) {
> +                                ret =  -EINVAL;
> +                                goto done;
> +                            }
>                              break;
>                          }
>                      }
> @@ -456,7 +472,8 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>                      if (!block) {
>                          fprintf(stderr, "Unknown ramblock \"%s\", cannot "
>                                  "accept migration\n", id);
> -                        return -EINVAL;
> +                        ret = -EINVAL;
> +                        goto done;
>                      }
>
>                      total_ram_bytes -= length;
> @@ -490,11 +507,15 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>          }
>          error = qemu_file_get_error(f);
>          if (error) {
> -            return error;
> +            ret = error;
> +            goto done;
>          }
>      } while (!(flags & RAM_SAVE_FLAG_EOS));
>
> -    return 0;
> +done:
> +    DPRINTF("Completed load of VM with exit code %d seq iteration %ld\n",
> +            ret, seq_iter);
> +    return ret;
>  }
>
>  #ifdef HAS_AUDIO
> --
> 1.7.7.6
>
Orit Wasserman - July 4, 2012, 7:19 a.m.
On 07/03/2012 10:25 PM, Blue Swirl wrote:
> On Tue, Jul 3, 2012 at 1:52 PM, Orit Wasserman <owasserm@redhat.com> wrote:
>> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
>> ---
>>  arch_init.c |   33 +++++++++++++++++++++++++++------
>>  1 files changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index 9dafb6e..ee20c33 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -44,6 +44,14 @@
>>  #include "exec-memory.h"
>>  #include "hw/pcspk.h"
>>
>> +#ifdef DEBUG_ARCH_INIT
>> +#define DPRINTF(fmt, ...) \
>> +    do { fprintf(stdout, "arch_init: " fmt, ## __VA_ARGS__); } while (0)
>> +#else
>> +#define DPRINTF(fmt, ...) \
>> +    do { } while (0)
>> +#endif
> 
> I think you missed my comments to the version that Juan sent, about
> using trace points. Also in Juan's version the %lds were changed to
> PRId64s which now are reverted, why?
I will base the next version on top of Juan's. This patch would be removed

Orit
> 
>> +
>>  #ifdef TARGET_SPARC
>>  int graphic_width = 1024;
>>  int graphic_height = 768;
>> @@ -380,6 +388,9 @@ int ram_save_live(QEMUFile *f, int stage, void *opaque)
>>
>>      expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
>>
>> +    DPRINTF("ram_save_live: expected(%ld) <= max(%ld)?\n", expected_time,
>> +        migrate_max_downtime());
>> +
>>      return (stage == 2) && (expected_time <= migrate_max_downtime());
>>  }
>>
>> @@ -416,8 +427,11 @@ static inline void *host_from_stream_offset(QEMUFile *f,
>>  int ram_load(QEMUFile *f, void *opaque, int version_id)
>>  {
>>      ram_addr_t addr;
>> -    int flags;
>> +    int flags, ret = 0;
>>      int error;
>> +    static uint64_t seq_iter;
>> +
>> +    seq_iter++;
>>
>>      if (version_id < 4 || version_id > 4) {
>>          return -EINVAL;
>> @@ -447,8 +461,10 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>>
>>                      QLIST_FOREACH(block, &ram_list.blocks, next) {
>>                          if (!strncmp(id, block->idstr, sizeof(id))) {
>> -                            if (block->length != length)
>> -                                return -EINVAL;
>> +                            if (block->length != length) {
>> +                                ret =  -EINVAL;
>> +                                goto done;
>> +                            }
>>                              break;
>>                          }
>>                      }
>> @@ -456,7 +472,8 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>>                      if (!block) {
>>                          fprintf(stderr, "Unknown ramblock \"%s\", cannot "
>>                                  "accept migration\n", id);
>> -                        return -EINVAL;
>> +                        ret = -EINVAL;
>> +                        goto done;
>>                      }
>>
>>                      total_ram_bytes -= length;
>> @@ -490,11 +507,15 @@ int ram_load(QEMUFile *f, void *opaque, int version_id)
>>          }
>>          error = qemu_file_get_error(f);
>>          if (error) {
>> -            return error;
>> +            ret = error;
>> +            goto done;
>>          }
>>      } while (!(flags & RAM_SAVE_FLAG_EOS));
>>
>> -    return 0;
>> +done:
>> +    DPRINTF("Completed load of VM with exit code %d seq iteration %ld\n",
>> +            ret, seq_iter);
>> +    return ret;
>>  }
>>
>>  #ifdef HAS_AUDIO
>> --
>> 1.7.7.6
>>

Patch

diff --git a/arch_init.c b/arch_init.c
index 9dafb6e..ee20c33 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -44,6 +44,14 @@ 
 #include "exec-memory.h"
 #include "hw/pcspk.h"
 
+#ifdef DEBUG_ARCH_INIT
+#define DPRINTF(fmt, ...) \
+    do { fprintf(stdout, "arch_init: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+    do { } while (0)
+#endif
+
 #ifdef TARGET_SPARC
 int graphic_width = 1024;
 int graphic_height = 768;
@@ -380,6 +388,9 @@  int ram_save_live(QEMUFile *f, int stage, void *opaque)
 
     expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
 
+    DPRINTF("ram_save_live: expected(%ld) <= max(%ld)?\n", expected_time,
+        migrate_max_downtime());
+
     return (stage == 2) && (expected_time <= migrate_max_downtime());
 }
 
@@ -416,8 +427,11 @@  static inline void *host_from_stream_offset(QEMUFile *f,
 int ram_load(QEMUFile *f, void *opaque, int version_id)
 {
     ram_addr_t addr;
-    int flags;
+    int flags, ret = 0;
     int error;
+    static uint64_t seq_iter;
+
+    seq_iter++;
 
     if (version_id < 4 || version_id > 4) {
         return -EINVAL;
@@ -447,8 +461,10 @@  int ram_load(QEMUFile *f, void *opaque, int version_id)
 
                     QLIST_FOREACH(block, &ram_list.blocks, next) {
                         if (!strncmp(id, block->idstr, sizeof(id))) {
-                            if (block->length != length)
-                                return -EINVAL;
+                            if (block->length != length) {
+                                ret =  -EINVAL;
+                                goto done;
+                            }
                             break;
                         }
                     }
@@ -456,7 +472,8 @@  int ram_load(QEMUFile *f, void *opaque, int version_id)
                     if (!block) {
                         fprintf(stderr, "Unknown ramblock \"%s\", cannot "
                                 "accept migration\n", id);
-                        return -EINVAL;
+                        ret = -EINVAL;
+                        goto done;
                     }
 
                     total_ram_bytes -= length;
@@ -490,11 +507,15 @@  int ram_load(QEMUFile *f, void *opaque, int version_id)
         }
         error = qemu_file_get_error(f);
         if (error) {
-            return error;
+            ret = error;
+            goto done;
         }
     } while (!(flags & RAM_SAVE_FLAG_EOS));
 
-    return 0;
+done:
+    DPRINTF("Completed load of VM with exit code %d seq iteration %ld\n",
+            ret, seq_iter);
+    return ret;
 }
 
 #ifdef HAS_AUDIO