Re: [Qemu-devel] [PATCH] migration/rdma: clang compilation fix

[Marcel Apfelbaum]

Hi David,

On 3/4/19 8:18 PM, Dr. David Alan Gilbert wrote:
> * Eric Blake (address@hidden) wrote:
> > On 3/4/19 2:42 AM, Marcel Apfelbaum wrote:
> > > Configuring QEMU with:
> > >           ../configure --cc=clang --enable-rdma
> > >
> > > Leads to compilation error:
> > >
> > >     CC      migration/rdma.o
> > >     CC      migration/block.o
> > >     qemu/migration/rdma.c:3615:58: error: taking address of packed member 
> > > 'rkey' of class or structure
> > >         'RDMARegisterResult' may result in an unaligned pointer value 
> > > [-Werror,-Waddress-of-packed-member]
> > >                               (uintptr_t)host_addr, NULL, &reg_result->rkey,
> > >                                                            ^~~~~~~~~~~~~~~~
> > >
> > > This is a false warning; even if RDMARegisterResult is "packed", rkey
> > > is the first field so is guaranteed to be aligned.
> > Not so. If you packed struct, gcc is free to abut that struct next to some
> > other unaligned field in a larger struct:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg06743.html
> >
> > > Fix it by disabling the warning only for this instance.
> > Ignoring the bug is not the same as fixing the bug.  You need to rework
> > this, as the compiler warning is not a false negative.
> In this case it is, however, I think it's easy enough to work around;
> all we need is something like:
>
>    uint23_t tmp_rkey;
>                    if (qemu_rdma_register_and_get_keys(rdma, block,
>                              (uintptr_t)host_addr, NULL, &tmp_rkey,
>                              chunk, chunk_start, chunk_end)) {
>                      error_report("cannot get rkey");
>                      ret = -EINVAL;
>                      goto out;
>                    }
>                    reg_result->rkey = tmp_rkey

Thanks for the suggestion, looks OK for me, I will re-spin.

Thanks,
Marcel

> Note that I think the structure pointed to by reg_result is
> visible over the wire, so we can't change it's (unfortunate) layout.
>
> Dave
>
> > > Signed-off-by: Marcel Apfelbaum <address@hidden>
> > > ---
> > >    migration/rdma.c | 3 +++
> > >    1 file changed, 3 insertions(+)
> > --
> > Eric Blake, Principal Software Engineer
> > Red Hat, Inc.           +1-919-301-3226
> > Virtualization:  qemu.org | libvirt.org
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK