qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uin


From: Laurent Vivier
Subject: Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's
Date: Tue, 14 Mar 2017 12:29:45 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

On 14/03/2017 12:16, Dr. David Alan Gilbert wrote:
> * Laurent Vivier (address@hidden) wrote:
>> On 06/03/2017 06:25, Jason Wang wrote:
>>> From: "Dr. David Alan Gilbert" <address@hidden>
>>>
>>> The index's in the Vmxnet3Ring were migrated as 32bit ints
>>> yet are declared as size_t's.  They appear to be derived
>>> from 32bit values loaded from guest memory, so actually
>>> store them as that.
>>>
>>> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
>>> Acked-by: Dmitry Fleytman <address@hidden>
>>> Signed-off-by: Jason Wang <address@hidden>
>>> ---
>>>  hw/net/vmxnet3.c | 12 ++++++------
>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>>> index e13a798..224c109 100644
>>> --- a/hw/net/vmxnet3.c
>>> +++ b/hw/net/vmxnet3.c
>>> @@ -141,17 +141,17 @@ typedef struct VMXNET3Class {
>>>  /* Cyclic ring abstraction */
>>>  typedef struct {
>>>      hwaddr pa;
>>> -    size_t size;
>>> -    size_t cell_size;
>>> -    size_t next;
>>> +    uint32_t size;
>>> +    uint32_t cell_size;
>>> +    uint32_t next;
>>>      uint8_t gen;
>>>  } Vmxnet3Ring;
>>>  
>>>  static inline void vmxnet3_ring_init(PCIDevice *d,
>>>                                  Vmxnet3Ring *ring,
>>>                                       hwaddr pa,
>>> -                                     size_t size,
>>> -                                     size_t cell_size,
>>> +                                     uint32_t size,
>>> +                                     uint32_t cell_size,
>>>                                       bool zero_region)
>>>  {
>>>      ring->pa = pa;
>>> @@ -166,7 +166,7 @@ static inline void vmxnet3_ring_init(PCIDevice *d,
>>>  }
>>>  
>>>  #define VMXNET3_RING_DUMP(macro, ring_name, ridx, r)                       
>>>   \
>>> -    macro("%s#%d: base %" PRIx64 " size %zu cell_size %zu gen %d next 
>>> %zu",  \
>>> +    macro("%s#%d: base %" PRIx64 " size %u cell_size %u gen %d next %u",  \
>>>            (ring_name), (ridx),                                             
>>>   \
>>>            (r)->pa, (r)->size, (r)->cell_size, (r)->gen, (r)->next)
>>>  
>>>
>>
>>
>> David, with '-dump-vmstate' with 2.8 machine type and with v2.8 and
>> master I can see the size of "txq_descr" and "rxq_descr" changes because
>> of "sizeof(Vmxnet3TxqDescr)" and "sizeof(Vmxnet3RxqDescr)". This changes
>> because the size of Vmxnet3Ring has changed with the s/size_t/uint32_t/":
>>
>>         {
>>           "field": "txq_descr",
>>           "version_id": 0,
>>           "field_exists": false,
>>           "size": 176
>>         },
>>         {
>>           "field": "rxq_descr",
>>           "version_id": 0,
>>           "field_exists": false,
>>           "size": 216
>>         },
>>
>> becomes:
>>
>>         {
>>           "field": "txq_descr",
>>           "version_id": 0,
>>           "field_exists": false,
>>           "size": 144
>>         },
>>         {
>>           "field": "rxq_descr",
>>           "version_id": 0,
>>           "field_exists": false,
>>           "size": 168
>>         },
> 
> But the old migration code used to write these fields using qemu_put_be32
> (from a11f5cb005f9854f0d78da97fc23adf5bc8c83f3)
> 
> -static void vmxnet3_put_ring_to_file(QEMUFile *f, Vmxnet3Ring *r)
> -{
> -    qemu_put_be64(f, r->pa);
> -    qemu_put_be32(f, r->size);
> -    qemu_put_be32(f, r->cell_size);
> -    qemu_put_be32(f, r->next);
> -    qemu_put_byte(f, r->gen);
> -}
> 
> +static const VMStateDescription vmstate_vmxnet3_ring = {
> +    .name = "vmxnet3-ring",
> +    .version_id = 0,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(pa, Vmxnet3Ring),
> +        VMSTATE_UINT32(size, Vmxnet3Ring),
> +        VMSTATE_UINT32(cell_size, Vmxnet3Ring),
> +        VMSTATE_UINT32(next, Vmxnet3Ring),
> +        VMSTATE_UINT8(gen, Vmxnet3Ring),
> +        VMSTATE_END_OF_LIST()
> +    }
>  };
> 
> so they used to be size_t's written with be32 and now they're uint32_t's
> written as 32bit - so that should be the same.
> I'm not sure i understand where the old txq_descr sizes come from - what
> managed to print the 176 number before it was converted to VMState?

I think it comes from the sizeof() in VMSTATE_STRUCT(Vmxnet3TxqDescr):

static const VMStateDescription vmstate_vmxnet3_txq_descr = {
    .name = "vmxnet3-txq-descr",
    .version_id = 0,
    .fields = (VMStateField[]) {
        VMSTATE_STRUCT(tx_ring, Vmxnet3TxqDescr, 0, vmstate_vmxnet3_ring,
                       Vmxnet3Ring),
        VMSTATE_STRUCT(comp_ring, Vmxnet3TxqDescr, 0, vmstate_vmxnet3_ring,
                       Vmxnet3Ring),
        VMSTATE_UINT8(intr_idx, Vmxnet3TxqDescr),
        VMSTATE_UINT64(tx_stats_pa, Vmxnet3TxqDescr),
        VMSTATE_STRUCT(txq_stats, Vmxnet3TxqDescr, 0,
vmstate_vmxnet3_tx_stats,
                       struct UPT1_TxStats),
        VMSTATE_END_OF_LIST()
    }
};

I will check the '-dump-vmstate' code to see if it's relevant.

> 
>> And if I try a migration, I have:
>>
>> qemu/migration/vmstate.c:112: vmstate_load_state: Assertion `first_elem
>> || !n_elems' failed.
> 
> Interesting; that's Halil's new assert; I need to look.

Can't it be related to the size problem?

Thanks,
Laurent




reply via email to

[Prev in Thread] Current Thread [Next in Thread]