Re: [Qemu-devel] [PATCH v2 06/30] trace: Fix parameter types in migration

[Dr. David Alan Gilbert]

* Eric Blake (address@hidden) wrote:
> On 03/13/2017 03:07 PM, Dr. David Alan Gilbert wrote:
> > * Eric Blake (address@hidden) wrote:
> >> An upcoming patch will let the compiler warn us when we are silently
> >> losing precision in traces; update the trace definitions to pass
> >> through the full value at the callsite.
> >>
> >> Signed-off-by: Eric Blake <address@hidden>
> >> ---
> >>  migration/trace-events | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> 
> >> -postcopy_ram_fault_thread_request(uint64_t hostaddr, const char 
> >> *ramblock, size_t offset) "Request for HVA=%" PRIx64 " rb=%s offset=%zx"
> >> +postcopy_ram_fault_thread_request(unsigned long long hostaddr, const char 
> >> *ramblock, size_t offset) "Request for HVA=%llx rb=%s offset=%zx"
> > 
> > Hmm - why?
> > That's called as:
> >         trace_postcopy_ram_fault_thread_request(msg.arg.pagefault.address,
> >                                                 qemu_ram_get_idstr(rb),
> >                                                 rb_offset);
> >     struct uffd_msg msg;
> > struct uffd_msg {
> > ..
> >         union {
> >                 struct {
> >                         __u64   flags;
> >                         __u64   address;
> >                 } pagefault;
> > ..
> >         } arg;
> > }
> > 
> > So why is a PRIx64 not the right way to print a __u64 ?
> 
> Because __u64 is not the same type as uint64_t.  On 64-bit Linux, __u64
> is 'unsigned long long', while uint64_t is 'unsigned long'.
> 
> > (I prefer %llx to the horrid PRIx64 syntax, but it still seems weird in 
> > this case)
> 
> As it is, I'm not sure if __u64 is always 'unsigned long long' in ALL
> Linux clients; an even-more-conservative patch would be to switch all
> callers to use explicit casts to something (like uint64_t or unsigned
> long long) that we have full control over, rather than passing __u64
> where we have no control over what type it ultimately resolves to.

That would be my preference - casting to (uint64_t) at the caller and
keep this as PRIx64.  We know it's a 64bit value so we should never
use unsigned long long anywhere for it.

Dave

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 



--
Dr. David Alan Gilbert / address@hidden / Manchester, UK