Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-13 Thread Jianjun Duan 


On 10/13/2016 09:32 AM, Halil Pasic wrote:
> 
> 
> On 10/13/2016 06:23 PM, Jianjun Duan wrote:
>>
>>
>> On 10/13/2016 03:48 AM, Halil Pasic wrote:
>>>
>>>
>>> On 10/13/2016 10:22 AM, Paolo Bonzini wrote:
>> No, I disagree.  We shouldn't be worried about making intrusive changes
 to all invocations or declarations, if that leads to a simpler API.
>>
>> If VMStateInfo was meant for complete customization, then it was missing
>> some parts. I think the API is indeed simpler. Just like
>> definition for VMStateField. Not all of its fields are used all time.
 Code moves.  We can decide how much customization to allow of VMStateInfo.

 You said it: "Not all of its fields are used all time".  Likewise, not
 all arguments are used all time for get/put, but it's not an issue if they
 are still there!  So this patch is good, but at the same time VMS_LINKED is
 pointless.

 Paolo

>>>
>>> I'm fine with this. I just think, it would be nice if the contract between
>>> the vmstate-core and the client code implementing VMStateInfo callbacks
>>> could be documented, including when are certain parameters valid, what
>>> they stand for, and how are they supposed to be used for the next version of
>>> the patch. Just to improve readability. Would this be OK with everybody?
>>>
>>> By the way the flag VMS_SINGLE is documented as ignored. Should we drop
>>> it?
>>>
>> I will prepare the VMStateInfo and QTAIL stuff as a separated series. If
>> indeed VMS_SINGLE is ignored, I can drop it here. It is similar to
>> VMS_LINKED situation.
>>
>> Thanks,
>> Jianjun
> 
> I think it's completely unrelated, so I would not lump it together with
> the QTAILQ stuff.
> 
> How do you feel about the apidoc part?
> 
I will add some comments inside vmstate_save/load_state about it.

Thanks,
Jianjun

> Cheers,
> Halil
>

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-13 Thread Halil Pasic 


On 10/13/2016 06:23 PM, Jianjun Duan wrote:
> 
> 
> On 10/13/2016 03:48 AM, Halil Pasic wrote:
>>
>>
>> On 10/13/2016 10:22 AM, Paolo Bonzini wrote:
> No, I disagree.  We shouldn't be worried about making intrusive changes
>>> to all invocations or declarations, if that leads to a simpler API.
>
> If VMStateInfo was meant for complete customization, then it was missing
> some parts. I think the API is indeed simpler. Just like
> definition for VMStateField. Not all of its fields are used all time.
>>> Code moves.  We can decide how much customization to allow of VMStateInfo.
>>>
>>> You said it: "Not all of its fields are used all time".  Likewise, not
>>> all arguments are used all time for get/put, but it's not an issue if they
>>> are still there!  So this patch is good, but at the same time VMS_LINKED is
>>> pointless.
>>>
>>> Paolo
>>>
>>
>> I'm fine with this. I just think, it would be nice if the contract between
>> the vmstate-core and the client code implementing VMStateInfo callbacks
>> could be documented, including when are certain parameters valid, what
>> they stand for, and how are they supposed to be used for the next version of
>> the patch. Just to improve readability. Would this be OK with everybody?
>>
>> By the way the flag VMS_SINGLE is documented as ignored. Should we drop
>> it?
>>
> I will prepare the VMStateInfo and QTAIL stuff as a separated series. If
> indeed VMS_SINGLE is ignored, I can drop it here. It is similar to
> VMS_LINKED situation.
> 
> Thanks,
> Jianjun

I think it's completely unrelated, so I would not lump it together with
the QTAILQ stuff.

How do you feel about the apidoc part?

Cheers,
Halil

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-13 Thread Jianjun Duan 


On 10/13/2016 03:48 AM, Halil Pasic wrote:
> 
> 
> On 10/13/2016 10:22 AM, Paolo Bonzini wrote:
 No, I disagree.  We shouldn't be worried about making intrusive changes
>> to all invocations or declarations, if that leads to a simpler API.

 If VMStateInfo was meant for complete customization, then it was missing
 some parts. I think the API is indeed simpler. Just like
 definition for VMStateField. Not all of its fields are used all time.
>> Code moves.  We can decide how much customization to allow of VMStateInfo.
>>
>> You said it: "Not all of its fields are used all time".  Likewise, not
>> all arguments are used all time for get/put, but it's not an issue if they
>> are still there!  So this patch is good, but at the same time VMS_LINKED is
>> pointless.
>>
>> Paolo
>>
> 
> I'm fine with this. I just think, it would be nice if the contract between
> the vmstate-core and the client code implementing VMStateInfo callbacks
> could be documented, including when are certain parameters valid, what
> they stand for, and how are they supposed to be used for the next version of
> the patch. Just to improve readability. Would this be OK with everybody?
> 
> By the way the flag VMS_SINGLE is documented as ignored. Should we drop
> it?
> 
I will prepare the VMStateInfo and QTAIL stuff as a separated series. If
indeed VMS_SINGLE is ignored, I can drop it here. It is similar to
VMS_LINKED situation.

Thanks,
Jianjun
>

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-13 Thread Paolo Bonzini 


On 13/10/2016 12:48, Halil Pasic wrote:
>> > 
> I'm fine with this. I just think, it would be nice if the contract between
> the vmstate-core and the client code implementing VMStateInfo callbacks
> could be documented, including when are certain parameters valid, what
> they stand for, and how are they supposed to be used for the next version of
> the patch. Just to improve readability. Would this be OK with everybody?
> 
> By the way the flag VMS_SINGLE is documented as ignored. Should we drop
> it?

Yes, I think so.

Paolo

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-13 Thread Halil Pasic 


On 10/13/2016 10:22 AM, Paolo Bonzini wrote:
>>> No, I disagree.  We shouldn't be worried about making intrusive changes
>>> > > to all invocations or declarations, if that leads to a simpler API.
>> > 
>> > If VMStateInfo was meant for complete customization, then it was missing
>> > some parts. I think the API is indeed simpler. Just like
>> > definition for VMStateField. Not all of its fields are used all time.
> Code moves.  We can decide how much customization to allow of VMStateInfo.
> 
> You said it: "Not all of its fields are used all time".  Likewise, not
> all arguments are used all time for get/put, but it's not an issue if they
> are still there!  So this patch is good, but at the same time VMS_LINKED is
> pointless.
> 
> Paolo
> 

I'm fine with this. I just think, it would be nice if the contract between
the vmstate-core and the client code implementing VMStateInfo callbacks
could be documented, including when are certain parameters valid, what
they stand for, and how are they supposed to be used for the next version of
the patch. Just to improve readability. Would this be OK with everybody?

By the way the flag VMS_SINGLE is documented as ignored. Should we drop
it?




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-13 Thread Paolo Bonzini 

> > No, I disagree.  We shouldn't be worried about making intrusive changes
> > to all invocations or declarations, if that leads to a simpler API.
> 
> If VMStateInfo was meant for complete customization, then it was missing
> some parts. I think the API is indeed simpler. Just like
> definition for VMStateField. Not all of its fields are used all time.

Code moves.  We can decide how much customization to allow of VMStateInfo.

You said it: "Not all of its fields are used all time".  Likewise, not
all arguments are used all time for get/put, but it's not an issue if they
are still there!  So this patch is good, but at the same time VMS_LINKED is
pointless.

Paolo

> > I agree that overloading .start can be a bit ugly but you can choose to
> > overload .num_offset instead, which is already better.
> >
> I did considered num_offset. But it is associated with an actual field.
> On the other hand, start means the start position of the q in the
> structure. So it is not that far stretched.
> 
> >> I would also suggest unit tests in test/test-vmstate.c. Maybe with
> >> that the vmstate migration of QTAILQ could be factored out as a separate
> >> patch series.
> > 
> > Yes, definitely.
> > 
> This sounds a good idea. Will do it.
> 
> > Paolo
> > 
> Thanks,
> Jianjun
> 
>

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-12 Thread Jianjun Duan 


On 10/12/2016 05:07 AM, Paolo Bonzini wrote:
> 
> 
> On 12/10/2016 13:59, Halil Pasic wrote:
>> IMHO this would:
>> * allow us to keep the good old MVStateInfo objects unmodified and
>>   the semantic of VMStateInfo unchanged
>> * make clear that VMStateLinked does not care about the calculated size
>>   and provide a new anchor for documentation
>> * if overloading the semantic of VMStateField.start is not desired we
>>   could put it into  VMStateLinked, if needed we could also put more
>>   stuff in there
>> * have clearer separation between special handling for (linked/certain)
>>   data structures and the usual scenario with the DAG.
> 
> No, I disagree.  We shouldn't be worried about making intrusive changes
> to all invocations or declarations, if that leads to a simpler API.
> 
If VMStateInfo was meant for complete customization, then it was missing
some parts. I think the API is indeed simpler. Just like
definition for VMStateField. Not all of its fields are used all time.

> I agree that overloading .start can be a bit ugly but you can choose to
> overload .num_offset instead, which is already better.
>
I did considered num_offset. But it is associated with an actual field.
On the other hand, start means the start position of the q in the
structure. So it is not that far stretched.

>> I would also suggest unit tests in test/test-vmstate.c. Maybe with
>> that the vmstate migration of QTAILQ could be factored out as a separate
>> patch series.
> 
> Yes, definitely.
> 
This sounds a good idea. Will do it.

> Paolo
> 
Thanks,
Jianjun

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-10 Thread David Gibson 
On Fri, Oct 07, 2016 at 07:42:12PM +0100, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (du...@linux.vnet.ibm.com) wrote:
> > 
> > 
> > On 10/07/2016 05:08 AM, Dr. David Alan Gilbert wrote:
> > > * Jianjun Duan (du...@linux.vnet.ibm.com) wrote:
> > >> Current migration code cannot handle some data structures such as
> > >> QTAILQ in qemu/queue.h. Here we extend the signatures of put/get
> > >> in VMStateInfo so that customized handling is supported.
> > >>
> > >> Signed-off-by: Jianjun Duan 
> > >> ---
> > >>  hw/net/vmxnet3.c| 18 ++---
> > >>  hw/nvram/eeprom93xx.c   |  6 ++-
> > >>  hw/nvram/fw_cfg.c   |  6 ++-
> > >>  hw/pci/msix.c   |  6 ++-
> > >>  hw/pci/pci.c| 12 --
> > >>  hw/pci/shpc.c   |  5 ++-
> > >>  hw/scsi/scsi-bus.c  |  6 ++-
> > >>  hw/timer/twl92230.c |  6 ++-
> > >>  hw/usb/redirect.c   | 18 ++---
> > >>  hw/virtio/virtio-pci.c  |  6 ++-
> > >>  hw/virtio/virtio.c  |  6 ++-
> > >>  include/migration/vmstate.h | 10 +++--
> > >>  migration/savevm.c  |  5 ++-
> > >>  migration/vmstate.c | 95 
> > >> -
> > >>  target-alpha/machine.c  |  5 ++-
> > >>  target-arm/machine.c| 12 --
> > >>  target-i386/machine.c   | 21 ++
> > >>  target-mips/machine.c   | 10 +++--
> > >>  target-ppc/machine.c| 10 +++--
> > >>  target-sparc/machine.c  |  5 ++-
> > >>  20 files changed, 171 insertions(+), 97 deletions(-)
> > >>
> > > 
> > > 
> > > 
> > >> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> > >> index 444672a..2ca4b46 100644
> > >> --- a/hw/usb/redirect.c
> > >> +++ b/hw/usb/redirect.c
> > >> @@ -2154,7 +2154,8 @@ static int usbredir_post_load(void *priv, int 
> > >> version_id)
> > >>  }
> > >>  
> > >>  /* For usbredirparser migration */
> > >> -static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused)
> > >> +static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused,
> > >> +void *opaque, QJSON *vmdesc)
> > >>  {
> > >>  USBRedirDevice *dev = priv;
> > >>  uint8_t *data;
> > >> @@ -2174,7 +2175,8 @@ static void usbredir_put_parser(QEMUFile *f, void 
> > >> *priv, size_t unused)
> > >>  free(data);
> > >>  }
> > >>  
> > >> -static int usbredir_get_parser(QEMUFile *f, void *priv, size_t unused)
> > >> +static int usbredir_get_parser(QEMUFile *f, void *priv, size_t unused,
> > >> +   void *opaque)
> > > 
> > > Neither of these built for me; I had to change those to VMStateField 
> > > rather than void *;
> > > 
> > > also is this series tested ontop of Halil's patches - because without them
> > > I'm finding I also had to fix up most of the other virtio devices.
> > > 
> > > Dave
> > 
> > I built it on top of ppc-for-2.8 without problems. Is Hail's patch in
> > ppc-for-2.8 yet?

If it's not in master, and not ppc specific, it won't be in ppc-for-2.8.

> I don't know about ppc-for-2.8;  patches for inclusion should work on the 
> current
> head unless stated otherwise.

Right.  These started as ppc specific patches, for which being on top
of ppc-for-2.8 would be the right choice (ppc-for-2.8 regularly
rebases on master).  However, they now include a bunch of generic
migration changes, it should be done directly against master.  This
may require splitting the series into generic and ppc specific
portions.

> Also, make sure you have the usbredir libraries installed and as much else as 
> possible
> to make sure you cover all the .get/.put functions - they're all over!
> 
> Dave
> 
> > Thanks,
> > Jianjun
> > 
> > > 
> > > 
> > >>  USBRedirDevice *dev = priv;
> > >>  uint8_t *data;
> > >> @@ -2217,7 +2219,8 @@ static const VMStateInfo 
> > >> usbredir_parser_vmstate_info = {
> > >>  
> > >>  
> > >>  /* For buffered packets (iso/irq) queue migration */
> > >> -static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused)
> > >> +static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused,
> > >> +   VMStateField *field, QJSON *vmdesc)
> > >>  {
> > >>  struct endp_data *endp = priv;
> > >>  USBRedirDevice *dev = endp->dev;
> > >> @@ -2237,7 +2240,8 @@ static void usbredir_put_bufpq(QEMUFile *f, void 
> > >> *priv, size_t unused)
> > >>  assert(i == endp->bufpq_size);
> > >>  }
> > >>  
> > >> -static int usbredir_get_bufpq(QEMUFile *f, void *priv, size_t unused)
> > >> +static int usbredir_get_bufpq(QEMUFile *f, void *priv, size_t unused,
> > >> +  VMStateField *field)
> > >>  {
> > >>  struct endp_data *endp = priv;
> > >>  USBRedirDevice *dev = endp->dev;
> > >> @@ -2340,7 +2344,8 @@ static const VMStateDescription 
> > >> usbredir_ep_vmstate = {
> > >>  
> > >>  
> > >>  /* For PacketIdQueue migration */
> > >> -static void usbredir_put_packet_id_q(QEMUFile

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-07 Thread Dr. David Alan Gilbert 
* Jianjun Duan (du...@linux.vnet.ibm.com) wrote:
> 
> 
> On 10/07/2016 05:08 AM, Dr. David Alan Gilbert wrote:
> > * Jianjun Duan (du...@linux.vnet.ibm.com) wrote:
> >> Current migration code cannot handle some data structures such as
> >> QTAILQ in qemu/queue.h. Here we extend the signatures of put/get
> >> in VMStateInfo so that customized handling is supported.
> >>
> >> Signed-off-by: Jianjun Duan 
> >> ---
> >>  hw/net/vmxnet3.c| 18 ++---
> >>  hw/nvram/eeprom93xx.c   |  6 ++-
> >>  hw/nvram/fw_cfg.c   |  6 ++-
> >>  hw/pci/msix.c   |  6 ++-
> >>  hw/pci/pci.c| 12 --
> >>  hw/pci/shpc.c   |  5 ++-
> >>  hw/scsi/scsi-bus.c  |  6 ++-
> >>  hw/timer/twl92230.c |  6 ++-
> >>  hw/usb/redirect.c   | 18 ++---
> >>  hw/virtio/virtio-pci.c  |  6 ++-
> >>  hw/virtio/virtio.c  |  6 ++-
> >>  include/migration/vmstate.h | 10 +++--
> >>  migration/savevm.c  |  5 ++-
> >>  migration/vmstate.c | 95 
> >> -
> >>  target-alpha/machine.c  |  5 ++-
> >>  target-arm/machine.c| 12 --
> >>  target-i386/machine.c   | 21 ++
> >>  target-mips/machine.c   | 10 +++--
> >>  target-ppc/machine.c| 10 +++--
> >>  target-sparc/machine.c  |  5 ++-
> >>  20 files changed, 171 insertions(+), 97 deletions(-)
> >>
> > 
> > 
> > 
> >> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> >> index 444672a..2ca4b46 100644
> >> --- a/hw/usb/redirect.c
> >> +++ b/hw/usb/redirect.c
> >> @@ -2154,7 +2154,8 @@ static int usbredir_post_load(void *priv, int 
> >> version_id)
> >>  }
> >>  
> >>  /* For usbredirparser migration */
> >> -static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused)
> >> +static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused,
> >> +void *opaque, QJSON *vmdesc)
> >>  {
> >>  USBRedirDevice *dev = priv;
> >>  uint8_t *data;
> >> @@ -2174,7 +2175,8 @@ static void usbredir_put_parser(QEMUFile *f, void 
> >> *priv, size_t unused)
> >>  free(data);
> >>  }
> >>  
> >> -static int usbredir_get_parser(QEMUFile *f, void *priv, size_t unused)
> >> +static int usbredir_get_parser(QEMUFile *f, void *priv, size_t unused,
> >> +   void *opaque)
> > 
> > Neither of these built for me; I had to change those to VMStateField rather 
> > than void *;
> > 
> > also is this series tested ontop of Halil's patches - because without them
> > I'm finding I also had to fix up most of the other virtio devices.
> > 
> > Dave
> 
> I built it on top of ppc-for-2.8 without problems. Is Hail's patch in
> ppc-for-2.8 yet?

I don't know about ppc-for-2.8;  patches for inclusion should work on the 
current
head unless stated otherwise.
Also, make sure you have the usbredir libraries installed and as much else as 
possible
to make sure you cover all the .get/.put functions - they're all over!

Dave

> Thanks,
> Jianjun
> 
> > 
> > 
> >>  USBRedirDevice *dev = priv;
> >>  uint8_t *data;
> >> @@ -2217,7 +2219,8 @@ static const VMStateInfo 
> >> usbredir_parser_vmstate_info = {
> >>  
> >>  
> >>  /* For buffered packets (iso/irq) queue migration */
> >> -static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused)
> >> +static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused,
> >> +   VMStateField *field, QJSON *vmdesc)
> >>  {
> >>  struct endp_data *endp = priv;
> >>  USBRedirDevice *dev = endp->dev;
> >> @@ -2237,7 +2240,8 @@ static void usbredir_put_bufpq(QEMUFile *f, void 
> >> *priv, size_t unused)
> >>  assert(i == endp->bufpq_size);
> >>  }
> >>  
> >> -static int usbredir_get_bufpq(QEMUFile *f, void *priv, size_t unused)
> >> +static int usbredir_get_bufpq(QEMUFile *f, void *priv, size_t unused,
> >> +  VMStateField *field)
> >>  {
> >>  struct endp_data *endp = priv;
> >>  USBRedirDevice *dev = endp->dev;
> >> @@ -2340,7 +2344,8 @@ static const VMStateDescription usbredir_ep_vmstate 
> >> = {
> >>  
> >>  
> >>  /* For PacketIdQueue migration */
> >> -static void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t 
> >> unused)
> >> +static void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t 
> >> unused,
> >> + VMStateField *field, QJSON *vmdesc)
> >>  {
> >>  struct PacketIdQueue *q = priv;
> >>  USBRedirDevice *dev = q->dev;
> >> @@ -2356,7 +2361,8 @@ static void usbredir_put_packet_id_q(QEMUFile *f, 
> >> void *priv, size_t unused)
> >>  assert(remain == 0);
> >>  }
> >>  
> >> -static int usbredir_get_packet_id_q(QEMUFile *f, void *priv, size_t 
> >> unused)
> >> +static int usbredir_get_packet_id_q(QEMUFile *f, void *priv, size_t 
> >> unused,
> >> +VMStateField *field)
> >>  {
> >>  struct

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 3/6] migration: extend VMStateInfo

2016-10-07 Thread Jianjun Duan 


On 10/07/2016 05:08 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (du...@linux.vnet.ibm.com) wrote:
>> Current migration code cannot handle some data structures such as
>> QTAILQ in qemu/queue.h. Here we extend the signatures of put/get
>> in VMStateInfo so that customized handling is supported.
>>
>> Signed-off-by: Jianjun Duan 
>> ---
>>  hw/net/vmxnet3.c| 18 ++---
>>  hw/nvram/eeprom93xx.c   |  6 ++-
>>  hw/nvram/fw_cfg.c   |  6 ++-
>>  hw/pci/msix.c   |  6 ++-
>>  hw/pci/pci.c| 12 --
>>  hw/pci/shpc.c   |  5 ++-
>>  hw/scsi/scsi-bus.c  |  6 ++-
>>  hw/timer/twl92230.c |  6 ++-
>>  hw/usb/redirect.c   | 18 ++---
>>  hw/virtio/virtio-pci.c  |  6 ++-
>>  hw/virtio/virtio.c  |  6 ++-
>>  include/migration/vmstate.h | 10 +++--
>>  migration/savevm.c  |  5 ++-
>>  migration/vmstate.c | 95 
>> -
>>  target-alpha/machine.c  |  5 ++-
>>  target-arm/machine.c| 12 --
>>  target-i386/machine.c   | 21 ++
>>  target-mips/machine.c   | 10 +++--
>>  target-ppc/machine.c| 10 +++--
>>  target-sparc/machine.c  |  5 ++-
>>  20 files changed, 171 insertions(+), 97 deletions(-)
>>
> 
> 
> 
>> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
>> index 444672a..2ca4b46 100644
>> --- a/hw/usb/redirect.c
>> +++ b/hw/usb/redirect.c
>> @@ -2154,7 +2154,8 @@ static int usbredir_post_load(void *priv, int 
>> version_id)
>>  }
>>  
>>  /* For usbredirparser migration */
>> -static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused)
>> +static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused,
>> +void *opaque, QJSON *vmdesc)
>>  {
>>  USBRedirDevice *dev = priv;
>>  uint8_t *data;
>> @@ -2174,7 +2175,8 @@ static void usbredir_put_parser(QEMUFile *f, void 
>> *priv, size_t unused)
>>  free(data);
>>  }
>>  
>> -static int usbredir_get_parser(QEMUFile *f, void *priv, size_t unused)
>> +static int usbredir_get_parser(QEMUFile *f, void *priv, size_t unused,
>> +   void *opaque)
> 
> Neither of these built for me; I had to change those to VMStateField rather 
> than void *;
> 
> also is this series tested ontop of Halil's patches - because without them
> I'm finding I also had to fix up most of the other virtio devices.
> 
> Dave

I built it on top of ppc-for-2.8 without problems. Is Hail's patch in
ppc-for-2.8 yet?

Thanks,
Jianjun

> 
> 
>>  USBRedirDevice *dev = priv;
>>  uint8_t *data;
>> @@ -2217,7 +2219,8 @@ static const VMStateInfo usbredir_parser_vmstate_info 
>> = {
>>  
>>  
>>  /* For buffered packets (iso/irq) queue migration */
>> -static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused)
>> +static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused,
>> +   VMStateField *field, QJSON *vmdesc)
>>  {
>>  struct endp_data *endp = priv;
>>  USBRedirDevice *dev = endp->dev;
>> @@ -2237,7 +2240,8 @@ static void usbredir_put_bufpq(QEMUFile *f, void 
>> *priv, size_t unused)
>>  assert(i == endp->bufpq_size);
>>  }
>>  
>> -static int usbredir_get_bufpq(QEMUFile *f, void *priv, size_t unused)
>> +static int usbredir_get_bufpq(QEMUFile *f, void *priv, size_t unused,
>> +  VMStateField *field)
>>  {
>>  struct endp_data *endp = priv;
>>  USBRedirDevice *dev = endp->dev;
>> @@ -2340,7 +2344,8 @@ static const VMStateDescription usbredir_ep_vmstate = {
>>  
>>  
>>  /* For PacketIdQueue migration */
>> -static void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused)
>> +static void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused,
>> + VMStateField *field, QJSON *vmdesc)
>>  {
>>  struct PacketIdQueue *q = priv;
>>  USBRedirDevice *dev = q->dev;
>> @@ -2356,7 +2361,8 @@ static void usbredir_put_packet_id_q(QEMUFile *f, void 
>> *priv, size_t unused)
>>  assert(remain == 0);
>>  }
>>  
>> -static int usbredir_get_packet_id_q(QEMUFile *f, void *priv, size_t unused)
>> +static int usbredir_get_packet_id_q(QEMUFile *f, void *priv, size_t unused,
>> +VMStateField *field)
>>  {
>>  struct PacketIdQueue *q = priv;
>>  USBRedirDevice *dev = q->dev;
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index 2d60a00..38a7abd 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -108,7 +108,8 @@ static bool virtio_pci_has_extra_state(DeviceState *d)
>>  return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA;
>>  }
>>  
>> -static int get_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size)
>> +static int get_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size,
>> +   VMStateField *field)
>>  {