| Message ID | 1340793261-11400-8-git-send-email-owasserm@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 06/27/2012 04:34 AM, Orit Wasserman 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) Unfortunately, '## __VA_ARGS__' is a gcc extension not portable to C99. But as HACKING recommends its use, you are not the first offender, nor should this patch clean it up (rather, it should be a global cleanup). Here's a portable way to do it (borrowed from libvirt): >> /* Helper macros to implement VIR_DEBUG using just C99. This >> * assumes you pass fewer than 15 arguments to VIR_DEBUG, but >> * can easily be expanded if needed. >> * >> * Note that gcc provides extensions of "define a(b...) b" or >> * "define a(b,...) b,##__VA_ARGS__" as a means of eliding a comma >> * when no var-args are present, but we don't want to require gcc. >> */ >> #define VIR_ARG15(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, _15, ...) _15 >> #define VIR_HAS_COMMA(...) VIR_ARG15(__VA_ARGS__, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0) >> >> /* Form the name VIR_DEBUG_[01], then call that macro, >> * according to how many arguments are present. Two-phase due to >> * macro expansion rules. */ >> #define VIR_DEBUG_EXPAND(a, b, ...) \ >> VIR_DEBUG_PASTE(a, b, __VA_ARGS__) >> #define VIR_DEBUG_PASTE(a, b, ...) \ >> a##b(__VA_ARGS__) >> >> /* Internal use only, when VIR_DEBUG has one argument. */ >> #define VIR_DEBUG_0(fmt) \ >> VIR_DOMAIN_DEBUG_1(fmt "%s", "") >> >> /* Internal use only, when VIR_DEBUG has >=two arguments. */ >> #define VIR_DEBUG_1(fmt, ...) \ >> do { fprintf(stdout, "prefix: " fmt, __VA_ARGS__); } while (0) >> >> #define VIR_DEBUG(...) \ >> VIR_DEBUG_EXPAND(VIR_DEBUG_, \ >> VIR_HAS_COMMA(__VA_ARGS__), \ >> __VA_ARGS__) >> Ugly, huh? Which is why I won't ask you to fix HACKING. Note that if you live with the restriction that you always provide an additional argument beyond the format string, then life is much simpler for complying with C99: #define DPRINTF(fmt, ...) \ do { fprintf(stdout, "prefix: " fmt, __VA_ARGS__); } while (0) but it means you can't use DPRINTF("string") (you would instead have to use DPRINTF("string", "dummy"), which might trigger a gcc warning about printf argument mismatch, so to silence that warning, you would have to use DPRINTF("string%s", "") - which is basically what my goop from libvirt above was attempting to do automatically).
On 06/27/2012 08:53 PM, Eric Blake wrote: > On 06/27/2012 04:34 AM, Orit Wasserman 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) > > Unfortunately, '## __VA_ARGS__' is a gcc extension not portable to C99. > > But as HACKING recommends its use, you are not the first offender, nor > should this patch clean it up (rather, it should be a global cleanup). > > Here's a portable way to do it (borrowed from libvirt): > >>> /* Helper macros to implement VIR_DEBUG using just C99. This >>> * assumes you pass fewer than 15 arguments to VIR_DEBUG, but >>> * can easily be expanded if needed. >>> * >>> * Note that gcc provides extensions of "define a(b...) b" or >>> * "define a(b,...) b,##__VA_ARGS__" as a means of eliding a comma >>> * when no var-args are present, but we don't want to require gcc. >>> */ >>> #define VIR_ARG15(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, _15, ...) _15 >>> #define VIR_HAS_COMMA(...) VIR_ARG15(__VA_ARGS__, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0) >>> >>> /* Form the name VIR_DEBUG_[01], then call that macro, >>> * according to how many arguments are present. Two-phase due to >>> * macro expansion rules. */ >>> #define VIR_DEBUG_EXPAND(a, b, ...) \ >>> VIR_DEBUG_PASTE(a, b, __VA_ARGS__) >>> #define VIR_DEBUG_PASTE(a, b, ...) \ >>> a##b(__VA_ARGS__) >>> >>> /* Internal use only, when VIR_DEBUG has one argument. */ >>> #define VIR_DEBUG_0(fmt) \ >>> VIR_DOMAIN_DEBUG_1(fmt "%s", "") >>> >>> /* Internal use only, when VIR_DEBUG has >=two arguments. */ >>> #define VIR_DEBUG_1(fmt, ...) \ >>> do { fprintf(stdout, "prefix: " fmt, __VA_ARGS__); } while (0) >>> >>> #define VIR_DEBUG(...) \ >>> VIR_DEBUG_EXPAND(VIR_DEBUG_, \ >>> VIR_HAS_COMMA(__VA_ARGS__), \ >>> __VA_ARGS__) >>> > > Ugly, huh? Which is why I won't ask you to fix HACKING. yes quite ugly :) > > Note that if you live with the restriction that you always provide an > additional argument beyond the format string, then life is much simpler > for complying with C99: > > #define DPRINTF(fmt, ...) \ > do { fprintf(stdout, "prefix: " fmt, __VA_ARGS__); } while (0) > > but it means you can't use DPRINTF("string") (you would instead have to > use DPRINTF("string", "dummy"), which might trigger a gcc warning about > printf argument mismatch, so to silence that warning, you would have to > use DPRINTF("string%s", "") - which is basically what my goop from > libvirt above was attempting to do automatically). >
Orit Wasserman <owasserm@redhat.com> wrote:
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Also integrated in the pull request.
Later, Juan.
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
Signed-off-by: Orit Wasserman <owasserm@redhat.com> --- arch_init.c | 33 +++++++++++++++++++++++++++------ 1 files changed, 27 insertions(+), 6 deletions(-)