Re: [PATCH v4 7/9] migration: Simplify alignment and alignment checks

[David Hildenbrand]

On 03.09.21 12:07, David Hildenbrand wrote:
> On 03.09.21 10:47, David Hildenbrand wrote:
> > On 03.09.21 00:32, Peter Xu wrote:
> > > On Thu, Sep 02, 2021 at 03:14:30PM +0200, David Hildenbrand wrote:
> > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > index bb909781b7..ae97c2c461 100644
> > > > --- a/migration/migration.c
> > > > +++ b/migration/migration.c
> > > > @@ -391,7 +391,7 @@ int 
> > > > migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
> > > >     int migrate_send_rp_req_pages(MigrationIncomingState *mis,
> > > >                                   RAMBlock *rb, ram_addr_t start, uint64_t 
> > > > haddr)
> > > >     {
> > > > -    void *aligned = (void *)(uintptr_t)(haddr & (-qemu_ram_pagesize(rb)));
> > > > +    void *aligned = (void *)QEMU_ALIGN_DOWN(haddr, qemu_ram_pagesize(rb));
> > >
> > > Is uintptr_t still needed?  I thought it would generate a warning otherwise but
> > > not sure.
> >
> > It doesn't in my setup, but maybe it will on 32bit archs ...
> >
> > I discussed this with Phil in
> >
> > https://lkml.kernel.org/r/2c8d80ad-f171-7d5f-3235-92f02fa174b3@redhat.com
> >
> > Maybe
> >
> > QEMU_ALIGN_PTR_DOWN((void *)haddr, qemu_ram_pagesize(rb)));
> >
> > Is really what we want.
>
> ... but it would suffer the same issue I think. I just ran it trough the
> gitlab pipeline, including "i386-fedora-cross-compile" ... and it seems
> to compile just fine, which is weird, because I'd also expect

[I know, talking to my self] Some 32bit tests actually did fail later, 
so the CI is able to catch this properly.

--
Thanks,

David / dhildenb