Re: [Qemu-devel] [PATCH v2 2/2] migration: drop unused VMStateField.start

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
> 
> 
> On 10/20/2016 03:25 PM, Halil Pasic wrote:
> > diff --git a/migration/vmstate.c b/migration/vmstate.c
> > index fc29acf..8767e40 100644
> > --- a/migration/vmstate.c
> > +++ b/migration/vmstate.c
> > @@ -66,10 +66,10 @@ static void *vmstate_base_addr(void *opaque, VMStateField *field, bool alloc)
> >                  }
> >              }
> >              if (size) {
> > -                *((void **)base_addr + field->start) = g_malloc(size);
> > +                *(void **)base_addr = g_malloc(size);
> >              }
> >          }
> > -        base_addr = *(void **)base_addr + field->start;
> > +        base_addr = *(void **)base_addr;
> >      }
> > 
> >      return base_addr;
> Hi!
> 
> It is been a while, and IMHO this is still broken, and the
> VMSTATE_VBUFFER* macros are still only used with the start argument
> being zero.
> 
> What changed is that with commit 94869d5c ("migration: migrate QTAILQ")
> from Jan 19 we have code actually using VMStateDecription.start -- but
> for something different (IMHO), as allocation is done by get_qtailq and
> not by vmstate_base_addr (as in case of VMSTATE_VBUFFER_ALLOC_UINT32).
> Thus I would need to update the commit message and keep the start field
> at least.
> 
> But before I do so, I would like to ask the maintainers if there is
> interest in a change like this?

I think it's certainly worth fixing VMSTATE_BUFFER_ALLOC*; if it can't
use the start field properly then it should have the start parameter
removed.

I'll admit I can't remember the details of why field->start as an offset
didn't work; are the other macros that take a start value correct?
If we can't remove the start variable then it's probably not removing
the start parameter everywhere.

That alloc case looks the most unusual in that vmstate_base_addr()
function; I *think* the non-alloc case makes sense
(i.e. follow a pointer and then use an offset from that pointer?)
even if no one currently uses it.

Dave

> Regards,
> Halil
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK