> On 30 Sep 2016, at 15:08 PM, Markus Armbruster <arm...@redhat.com> wrote:
>
> Cao jin <caoj.f...@cn.fujitsu.com> writes:
>
>> On 09/29/2016 10:42 PM, Markus Armbruster wrote:
>>> Cao jin <caoj.f...@cn.fujitsu.com> writes:
>>>
>>
>>>> static int vmxnet3_post_load(void *opaque, int version_id)
>>>> {
>>>> VMXNET3State *s = opaque;
>>>> - PCIDevice *d = PCI_DEVICE(s);
>>>>
>>>> net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s),
>>>> s->max_tx_frags, s->peer_has_vhdr);
>>>> net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr);
>>>>
>>>> - if (s->msix_used) {
>>>> - if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) {
>>>> - VMW_WRPRN("Failed to re-use MSI-X vectors");
>>>> - msix_uninit(d, &s->msix_bar, &s->msix_bar);
>>>> - s->msix_used = false;
>>>> - return -1;
>>>> - }
>>>> - }
>>>> -
>>>> vmxnet3_validate_queues(s);
>>>> vmxnet3_validate_interrupts(s);
>>>
>>> This hunk isn't obvious. Can you explain the change?
>>>
>>
>> flag msix_used is used in VMStateDescription.post_Load().
>>
>> 1st, I think msix's code here is not necessary, because in
>> destination, device has been realized before incoming migration, So I
>> don't know why re-use MSI-X vectors here. Dmitry, could help to
>> explain?
>>
>> 2nd, this patch is going to remove this flag, so I removed the hunk.
>
> We need to find out whether the call of vmxnet3_use_msix_vectors() is
> necessary. I suspect it's not only not necessary, but actively wrong.
>
> If that's true, removing becomes a bug fix that should be a separate
> patch.
>
> If it's only unnecessary, the removal may stay in this patch, but it
> needs to be explained. Separate patch might be easier to explain. Your
> choice.
>
> If it correct and necessary, then this patch needs to be changed not to
> drop it. Instead, replace s->msix_used by msix_enabled(d) like you do
> elsewhere.
>
> Dmitry, can you help us find out?
Hello,
Yes, from what I see, this call is wrong and leads to
reference leaks on device unload at migration target.
It should be removed.
Best regards,
Dmitry
