[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 14:42:28 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 14/03/2017 14:31, Dr. David Alan Gilbert wrote:
> * Laurent Vivier (address@hidden) wrote:
>> On 14/03/2017 12:38, Dr. David Alan Gilbert wrote:
>>> * Laurent Vivier (address@hidden) wrote:
>>>> 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.
>>>
>>> OK, but that sizeof() should never make it onto the wire; all that
>>> teh VMSTATE_STRUCT should do is execute the vmstate_vmxnet3_ring on
>>> that data.
>>> (And again I'm curious what happened in the version before I committed
>>> that change).
>>>
>>>>>
>>>>>> 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?
>>>
>>> It could, I need to see exactly how it's failing; what was the full
>>> command line you used, and with what guest?
>>
>> Tested with pseries machine type on ppc64le host, no guest started.
>>
>> qemu-system-ppc64 -M pseries-2.8 -S -serial mon:stdio -vnc :0 -device
>> vmxnet3
>
> QingFeng Hao's <address@hidden> patch:
> 'vmstate: fix failed iotests case 68 and 91'
> seems to fix that for me.
For me too.
Thanks,
Laurent
- [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, (continued)
- [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Jason Wang, 2017/03/06
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Laurent Vivier, 2017/03/13
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Dr. David Alan Gilbert, 2017/03/14
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Laurent Vivier, 2017/03/14
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Dr. David Alan Gilbert, 2017/03/14
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Laurent Vivier, 2017/03/14
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Dr. David Alan Gilbert, 2017/03/14
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Laurent Vivier, 2017/03/14
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Dr. David Alan Gilbert, 2017/03/14
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's, Dr. David Alan Gilbert, 2017/03/14
- Re: [Qemu-devel] [PULL RESEND 12/19] vmxnet3: Convert ring values to uint32_t's,
Laurent Vivier <=
[Qemu-devel] [PULL RESEND 14/19] net/colo: fix memory double free error, Jason Wang, 2017/03/06
[Qemu-devel] [PULL RESEND 13/19] vmxnet3: VMStatify rx/tx q_descr and int_state, Jason Wang, 2017/03/06
[Qemu-devel] [PULL RESEND 15/19] filter-rewriter: skip net_checksum_calculate() while offset = 0, Jason Wang, 2017/03/06
[Qemu-devel] [PULL RESEND 16/19] COLO-compare: Rename compare function and remove duplicate codes, Jason Wang, 2017/03/06
[Qemu-devel] [PULL RESEND 17/19] COLO-compare: Optimize compare_common and compare_tcp, Jason Wang, 2017/03/06
[Qemu-devel] [PULL RESEND 19/19] net/filter-mirror: Follow CODING_STYLE, Jason Wang, 2017/03/06
[Qemu-devel] [PULL RESEND 18/19] COLO-compare: Fix icmp and udp compare different packet always dump bug, Jason Wang, 2017/03/06
Re: [Qemu-devel] [PULL RESEND 00/19] Net patches, Peter Maydell, 2017/03/07