* Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
2019-07-05 1:07 [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration Marcel Apfelbaum
@ 2019-07-05 10:57 ` Sukrit Bhatnagar
2019-07-05 10:59 ` Dmitry Fleytman
2019-08-07 17:12 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 10+ messages in thread
From: Sukrit Bhatnagar @ 2019-07-05 10:57 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: dmitry.fleytman, qemu-devel, Yuval Shaia, Dr. David Alan Gilbert
On Fri, 5 Jul 2019 at 06:38, Marcel Apfelbaum
<marcel.apfelbaum@gmail.com> wrote:
>
> At some point vmxnet3 live migration stopped working and git-bisect
> didn't help finding a working version.
> The issue is the PCI configuration space is not being migrated
> successfully and MSIX remains masked at destination.
>
> Remove the migration differentiation between PCI and PCIe since
> the logic resides now inside VMSTATE_PCI_DEVICE.
> Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
> since at 'realize' time is decided if the device is PCI or PCIe,
> then the above macro is enough.
>
> Use the opportunity to move to the standard VMSTATE_MSIX
> instead of the deprecated SaveVMHandlers.
>
> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Tested-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
> ---
> hw/net/vmxnet3.c | 52 ++----------------------------------------------
> 1 file changed, 2 insertions(+), 50 deletions(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 10d01d0058..8b17548b02 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
> msi_uninit(d);
> }
>
> -static void
> -vmxnet3_msix_save(QEMUFile *f, void *opaque)
> -{
> - PCIDevice *d = PCI_DEVICE(opaque);
> - msix_save(d, f);
> -}
> -
> -static int
> -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
> -{
> - PCIDevice *d = PCI_DEVICE(opaque);
> - msix_load(d, f);
> - return 0;
> -}
> -
> static const MemoryRegionOps b0_ops = {
> .read = vmxnet3_io_bar0_read,
> .write = vmxnet3_io_bar0_write,
> @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
> },
> };
>
> -static SaveVMHandlers savevm_vmxnet3_msix = {
> - .save_state = vmxnet3_msix_save,
> - .load_state = vmxnet3_msix_load,
> -};
> -
> static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> {
> uint64_t dsn_payload;
> @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>
> static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> {
> - DeviceState *dev = DEVICE(pci_dev);
> VMXNET3State *s = VMXNET3(pci_dev);
> int ret;
>
> @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
> vmxnet3_device_serial_num(s));
> }
> -
> - register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
> }
>
> static void vmxnet3_instance_init(Object *obj)
> @@ -2440,29 +2417,6 @@ static const VMStateDescription vmstate_vmxnet3_int_state = {
> }
> };
>
> -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
> -{
> - VMXNET3State *s = VMXNET3(opaque);
> -
> - return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
> -}
> -
> -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
> -{
> - return !vmxnet3_vmstate_need_pcie_device(opaque);
> -}
> -
> -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> - .name = "vmxnet3/pcie",
> - .version_id = 1,
> - .minimum_version_id = 1,
> - .needed = vmxnet3_vmstate_need_pcie_device,
> - .fields = (VMStateField[]) {
> - VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> - VMSTATE_END_OF_LIST()
> - }
> -};
> -
> static const VMStateDescription vmstate_vmxnet3 = {
> .name = "vmxnet3",
> .version_id = 1,
> @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
> .pre_save = vmxnet3_pre_save,
> .post_load = vmxnet3_post_load,
> .fields = (VMStateField[]) {
> - VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
> - vmxnet3_vmstate_test_pci_device, 0,
> - vmstate_pci_device, PCIDevice),
> + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> + VMSTATE_MSIX(parent_obj, VMXNET3State),
> VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
> VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
> VMSTATE_BOOL(lro_supported, VMXNET3State),
> @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
> },
> .subsections = (const VMStateDescription*[]) {
> &vmxstate_vmxnet3_mcast_list,
> - &vmstate_vmxnet3_pcie_device,
> NULL
> }
> };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
2019-07-05 1:07 [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration Marcel Apfelbaum
2019-07-05 10:57 ` Sukrit Bhatnagar
@ 2019-07-05 10:59 ` Dmitry Fleytman
2019-07-05 11:14 ` Sukrit Bhatnagar
2019-08-07 17:12 ` Dr. David Alan Gilbert
2 siblings, 1 reply; 10+ messages in thread
From: Dmitry Fleytman @ 2019-07-05 10:59 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: skrtbhtngr, qemu-devel, yuval.shaia, dgilbert
> On 5 Jul 2019, at 4:07, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
>
> At some point vmxnet3 live migration stopped working and git-bisect
> didn't help finding a working version.
> The issue is the PCI configuration space is not being migrated
> successfully and MSIX remains masked at destination.
>
> Remove the migration differentiation between PCI and PCIe since
> the logic resides now inside VMSTATE_PCI_DEVICE.
> Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
> since at 'realize' time is decided if the device is PCI or PCIe,
> then the above macro is enough.
>
> Use the opportunity to move to the standard VMSTATE_MSIX
> instead of the deprecated SaveVMHandlers.
>
> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
> ---
> hw/net/vmxnet3.c | 52 ++----------------------------------------------
> 1 file changed, 2 insertions(+), 50 deletions(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 10d01d0058..8b17548b02 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
> msi_uninit(d);
> }
>
> -static void
> -vmxnet3_msix_save(QEMUFile *f, void *opaque)
> -{
> - PCIDevice *d = PCI_DEVICE(opaque);
> - msix_save(d, f);
> -}
> -
> -static int
> -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
> -{
> - PCIDevice *d = PCI_DEVICE(opaque);
> - msix_load(d, f);
> - return 0;
> -}
> -
> static const MemoryRegionOps b0_ops = {
> .read = vmxnet3_io_bar0_read,
> .write = vmxnet3_io_bar0_write,
> @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
> },
> };
>
> -static SaveVMHandlers savevm_vmxnet3_msix = {
> - .save_state = vmxnet3_msix_save,
> - .load_state = vmxnet3_msix_load,
> -};
> -
> static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> {
> uint64_t dsn_payload;
> @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>
> static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> {
> - DeviceState *dev = DEVICE(pci_dev);
> VMXNET3State *s = VMXNET3(pci_dev);
> int ret;
>
> @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
> vmxnet3_device_serial_num(s));
> }
> -
> - register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
> }
>
> static void vmxnet3_instance_init(Object *obj)
> @@ -2440,29 +2417,6 @@ static const VMStateDescription vmstate_vmxnet3_int_state = {
> }
> };
>
> -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
> -{
> - VMXNET3State *s = VMXNET3(opaque);
> -
> - return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
> -}
> -
> -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
> -{
> - return !vmxnet3_vmstate_need_pcie_device(opaque);
> -}
> -
> -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> - .name = "vmxnet3/pcie",
> - .version_id = 1,
> - .minimum_version_id = 1,
> - .needed = vmxnet3_vmstate_need_pcie_device,
> - .fields = (VMStateField[]) {
> - VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> - VMSTATE_END_OF_LIST()
> - }
> -};
> -
> static const VMStateDescription vmstate_vmxnet3 = {
> .name = "vmxnet3",
> .version_id = 1,
> @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
> .pre_save = vmxnet3_pre_save,
> .post_load = vmxnet3_post_load,
> .fields = (VMStateField[]) {
> - VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
> - vmxnet3_vmstate_test_pci_device, 0,
> - vmstate_pci_device, PCIDevice),
> + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> + VMSTATE_MSIX(parent_obj, VMXNET3State),
> VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
> VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
> VMSTATE_BOOL(lro_supported, VMXNET3State),
> @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
> },
> .subsections = (const VMStateDescription*[]) {
> &vmxstate_vmxnet3_mcast_list,
> - &vmstate_vmxnet3_pcie_device,
> NULL
> }
> };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
2019-07-05 10:59 ` Dmitry Fleytman
@ 2019-07-05 11:14 ` Sukrit Bhatnagar
2019-07-05 11:20 ` Marcel Apfelbaum
0 siblings, 1 reply; 10+ messages in thread
From: Sukrit Bhatnagar @ 2019-07-05 11:14 UTC (permalink / raw)
To: Dmitry Fleytman; +Cc: Yuval Shaia, qemu-devel, Dr. David Alan Gilbert
On Fri, 5 Jul 2019 at 16:29, Dmitry Fleytman <dmitry.fleytman@gmail.com> wrote:
>
>
> > On 5 Jul 2019, at 4:07, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
> >
> > At some point vmxnet3 live migration stopped working and git-bisect
> > didn't help finding a working version.
> > The issue is the PCI configuration space is not being migrated
> > successfully and MSIX remains masked at destination.
> >
> > Remove the migration differentiation between PCI and PCIe since
> > the logic resides now inside VMSTATE_PCI_DEVICE.
> > Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
> > since at 'realize' time is decided if the device is PCI or PCIe,
> > then the above macro is enough.
> >
> > Use the opportunity to move to the standard VMSTATE_MSIX
> > instead of the deprecated SaveVMHandlers.
> >
> > Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>
>
> Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
>
Tested-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
>
> > ---
> > hw/net/vmxnet3.c | 52 ++----------------------------------------------
> > 1 file changed, 2 insertions(+), 50 deletions(-)
> >
> > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > index 10d01d0058..8b17548b02 100644
> > --- a/hw/net/vmxnet3.c
> > +++ b/hw/net/vmxnet3.c
> > @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
> > msi_uninit(d);
> > }
> >
> > -static void
> > -vmxnet3_msix_save(QEMUFile *f, void *opaque)
> > -{
> > - PCIDevice *d = PCI_DEVICE(opaque);
> > - msix_save(d, f);
> > -}
> > -
> > -static int
> > -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
> > -{
> > - PCIDevice *d = PCI_DEVICE(opaque);
> > - msix_load(d, f);
> > - return 0;
> > -}
> > -
> > static const MemoryRegionOps b0_ops = {
> > .read = vmxnet3_io_bar0_read,
> > .write = vmxnet3_io_bar0_write,
> > @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
> > },
> > };
> >
> > -static SaveVMHandlers savevm_vmxnet3_msix = {
> > - .save_state = vmxnet3_msix_save,
> > - .load_state = vmxnet3_msix_load,
> > -};
> > -
> > static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> > {
> > uint64_t dsn_payload;
> > @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> >
> > static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> > {
> > - DeviceState *dev = DEVICE(pci_dev);
> > VMXNET3State *s = VMXNET3(pci_dev);
> > int ret;
> >
> > @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> > pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
> > vmxnet3_device_serial_num(s));
> > }
> > -
> > - register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
> > }
> >
> > static void vmxnet3_instance_init(Object *obj)
> > @@ -2440,29 +2417,6 @@ static const VMStateDescription vmstate_vmxnet3_int_state = {
> > }
> > };
> >
> > -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
> > -{
> > - VMXNET3State *s = VMXNET3(opaque);
> > -
> > - return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
> > -}
> > -
> > -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
> > -{
> > - return !vmxnet3_vmstate_need_pcie_device(opaque);
> > -}
> > -
> > -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> > - .name = "vmxnet3/pcie",
> > - .version_id = 1,
> > - .minimum_version_id = 1,
> > - .needed = vmxnet3_vmstate_need_pcie_device,
> > - .fields = (VMStateField[]) {
> > - VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > - VMSTATE_END_OF_LIST()
> > - }
> > -};
> > -
> > static const VMStateDescription vmstate_vmxnet3 = {
> > .name = "vmxnet3",
> > .version_id = 1,
> > @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
> > .pre_save = vmxnet3_pre_save,
> > .post_load = vmxnet3_post_load,
> > .fields = (VMStateField[]) {
> > - VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
> > - vmxnet3_vmstate_test_pci_device, 0,
> > - vmstate_pci_device, PCIDevice),
> > + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > + VMSTATE_MSIX(parent_obj, VMXNET3State),
> > VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
> > VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
> > VMSTATE_BOOL(lro_supported, VMXNET3State),
> > @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
> > },
> > .subsections = (const VMStateDescription*[]) {
> > &vmxstate_vmxnet3_mcast_list,
> > - &vmstate_vmxnet3_pcie_device,
> > NULL
> > }
> > };
> > --
> > 2.17.1
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
2019-07-05 11:14 ` Sukrit Bhatnagar
@ 2019-07-05 11:20 ` Marcel Apfelbaum
2019-07-08 16:03 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 10+ messages in thread
From: Marcel Apfelbaum @ 2019-07-05 11:20 UTC (permalink / raw)
To: Sukrit Bhatnagar, Dmitry Fleytman, Dr. David Alan Gilbert
Cc: qemu-devel, Yuval Shaia
On 7/5/19 2:14 PM, Sukrit Bhatnagar wrote:
> On Fri, 5 Jul 2019 at 16:29, Dmitry Fleytman <dmitry.fleytman@gmail.com> wrote:
>>
>>> On 5 Jul 2019, at 4:07, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
>>>
>>> At some point vmxnet3 live migration stopped working and git-bisect
>>> didn't help finding a working version.
>>> The issue is the PCI configuration space is not being migrated
>>> successfully and MSIX remains masked at destination.
>>>
>>> Remove the migration differentiation between PCI and PCIe since
>>> the logic resides now inside VMSTATE_PCI_DEVICE.
>>> Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
>>> since at 'realize' time is decided if the device is PCI or PCIe,
>>> then the above macro is enough.
>>>
>>> Use the opportunity to move to the standard VMSTATE_MSIX
>>> instead of the deprecated SaveVMHandlers.
>>>
>>> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>>
>> Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
>>
> Tested-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
Thanks for the fast testing and review!
David, since the live migration was broken long before this patch,
I don't need to add any compatibility code, right?
If so, can you merge it using your migration tree?
Thanks,
Marcel
>>> ---
>>> hw/net/vmxnet3.c | 52 ++----------------------------------------------
>>> 1 file changed, 2 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>>> index 10d01d0058..8b17548b02 100644
>>> --- a/hw/net/vmxnet3.c
>>> +++ b/hw/net/vmxnet3.c
>>> @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
>>> msi_uninit(d);
>>> }
>>>
>>> -static void
>>> -vmxnet3_msix_save(QEMUFile *f, void *opaque)
>>> -{
>>> - PCIDevice *d = PCI_DEVICE(opaque);
>>> - msix_save(d, f);
>>> -}
>>> -
>>> -static int
>>> -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
>>> -{
>>> - PCIDevice *d = PCI_DEVICE(opaque);
>>> - msix_load(d, f);
>>> - return 0;
>>> -}
>>> -
>>> static const MemoryRegionOps b0_ops = {
>>> .read = vmxnet3_io_bar0_read,
>>> .write = vmxnet3_io_bar0_write,
>>> @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
>>> },
>>> };
>>>
>>> -static SaveVMHandlers savevm_vmxnet3_msix = {
>>> - .save_state = vmxnet3_msix_save,
>>> - .load_state = vmxnet3_msix_load,
>>> -};
>>> -
>>> static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>>> {
>>> uint64_t dsn_payload;
>>> @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>>>
>>> static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
>>> {
>>> - DeviceState *dev = DEVICE(pci_dev);
>>> VMXNET3State *s = VMXNET3(pci_dev);
>>> int ret;
>>>
>>> @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
>>> pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
>>> vmxnet3_device_serial_num(s));
>>> }
>>> -
>>> - register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
>>> }
>>>
>>> static void vmxnet3_instance_init(Object *obj)
>>> @@ -2440,29 +2417,6 @@ static const VMStateDescription vmstate_vmxnet3_int_state = {
>>> }
>>> };
>>>
>>> -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
>>> -{
>>> - VMXNET3State *s = VMXNET3(opaque);
>>> -
>>> - return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
>>> -}
>>> -
>>> -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
>>> -{
>>> - return !vmxnet3_vmstate_need_pcie_device(opaque);
>>> -}
>>> -
>>> -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
>>> - .name = "vmxnet3/pcie",
>>> - .version_id = 1,
>>> - .minimum_version_id = 1,
>>> - .needed = vmxnet3_vmstate_need_pcie_device,
>>> - .fields = (VMStateField[]) {
>>> - VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
>>> - VMSTATE_END_OF_LIST()
>>> - }
>>> -};
>>> -
>>> static const VMStateDescription vmstate_vmxnet3 = {
>>> .name = "vmxnet3",
>>> .version_id = 1,
>>> @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
>>> .pre_save = vmxnet3_pre_save,
>>> .post_load = vmxnet3_post_load,
>>> .fields = (VMStateField[]) {
>>> - VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
>>> - vmxnet3_vmstate_test_pci_device, 0,
>>> - vmstate_pci_device, PCIDevice),
>>> + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
>>> + VMSTATE_MSIX(parent_obj, VMXNET3State),
>>> VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
>>> VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
>>> VMSTATE_BOOL(lro_supported, VMXNET3State),
>>> @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
>>> },
>>> .subsections = (const VMStateDescription*[]) {
>>> &vmxstate_vmxnet3_mcast_list,
>>> - &vmstate_vmxnet3_pcie_device,
>>> NULL
>>> }
>>> };
>>> --
>>> 2.17.1
>>>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
2019-07-05 11:20 ` Marcel Apfelbaum
@ 2019-07-08 16:03 ` Dr. David Alan Gilbert
2019-07-08 18:47 ` Marcel Apfelbaum
2019-07-08 19:00 ` Dmitry Fleytman
0 siblings, 2 replies; 10+ messages in thread
From: Dr. David Alan Gilbert @ 2019-07-08 16:03 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: Sukrit Bhatnagar, Dmitry Fleytman, qemu-devel, Yuval Shaia
* Marcel Apfelbaum (marcel.apfelbaum@gmail.com) wrote:
>
>
> On 7/5/19 2:14 PM, Sukrit Bhatnagar wrote:
> > On Fri, 5 Jul 2019 at 16:29, Dmitry Fleytman <dmitry.fleytman@gmail.com> wrote:
> > >
> > > > On 5 Jul 2019, at 4:07, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
> > > >
> > > > At some point vmxnet3 live migration stopped working and git-bisect
> > > > didn't help finding a working version.
> > > > The issue is the PCI configuration space is not being migrated
> > > > successfully and MSIX remains masked at destination.
> > > >
> > > > Remove the migration differentiation between PCI and PCIe since
> > > > the logic resides now inside VMSTATE_PCI_DEVICE.
> > > > Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
> > > > since at 'realize' time is decided if the device is PCI or PCIe,
> > > > then the above macro is enough.
> > > >
> > > > Use the opportunity to move to the standard VMSTATE_MSIX
> > > > instead of the deprecated SaveVMHandlers.
> > > >
> > > > Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > >
> > > Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
> > >
> > Tested-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
>
> Thanks for the fast testing and review!
>
> David, since the live migration was broken long before this patch,
> I don't need to add any compatibility code, right?
Right, although we should probably bump the version_id field, that way
you'll get a nice clean error if you try and migrate from the old<->new.
(It's nice to get rid of the old msix oddity).
> If so, can you merge it using your migration tree?
I could do; or since it's entirely in vmxnet3 Dmitry can take it.
Dave
> Thanks,
> Marcel
>
>
> > > > ---
> > > > hw/net/vmxnet3.c | 52 ++----------------------------------------------
> > > > 1 file changed, 2 insertions(+), 50 deletions(-)
> > > >
> > > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > > > index 10d01d0058..8b17548b02 100644
> > > > --- a/hw/net/vmxnet3.c
> > > > +++ b/hw/net/vmxnet3.c
> > > > @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
> > > > msi_uninit(d);
> > > > }
> > > >
> > > > -static void
> > > > -vmxnet3_msix_save(QEMUFile *f, void *opaque)
> > > > -{
> > > > - PCIDevice *d = PCI_DEVICE(opaque);
> > > > - msix_save(d, f);
> > > > -}
> > > > -
> > > > -static int
> > > > -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
> > > > -{
> > > > - PCIDevice *d = PCI_DEVICE(opaque);
> > > > - msix_load(d, f);
> > > > - return 0;
> > > > -}
> > > > -
> > > > static const MemoryRegionOps b0_ops = {
> > > > .read = vmxnet3_io_bar0_read,
> > > > .write = vmxnet3_io_bar0_write,
> > > > @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
> > > > },
> > > > };
> > > >
> > > > -static SaveVMHandlers savevm_vmxnet3_msix = {
> > > > - .save_state = vmxnet3_msix_save,
> > > > - .load_state = vmxnet3_msix_load,
> > > > -};
> > > > -
> > > > static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> > > > {
> > > > uint64_t dsn_payload;
> > > > @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> > > >
> > > > static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> > > > {
> > > > - DeviceState *dev = DEVICE(pci_dev);
> > > > VMXNET3State *s = VMXNET3(pci_dev);
> > > > int ret;
> > > >
> > > > @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> > > > pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
> > > > vmxnet3_device_serial_num(s));
> > > > }
> > > > -
> > > > - register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
> > > > }
> > > >
> > > > static void vmxnet3_instance_init(Object *obj)
> > > > @@ -2440,29 +2417,6 @@ static const VMStateDescription vmstate_vmxnet3_int_state = {
> > > > }
> > > > };
> > > >
> > > > -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
> > > > -{
> > > > - VMXNET3State *s = VMXNET3(opaque);
> > > > -
> > > > - return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
> > > > -}
> > > > -
> > > > -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
> > > > -{
> > > > - return !vmxnet3_vmstate_need_pcie_device(opaque);
> > > > -}
> > > > -
> > > > -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> > > > - .name = "vmxnet3/pcie",
> > > > - .version_id = 1,
> > > > - .minimum_version_id = 1,
> > > > - .needed = vmxnet3_vmstate_need_pcie_device,
> > > > - .fields = (VMStateField[]) {
> > > > - VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > > > - VMSTATE_END_OF_LIST()
> > > > - }
> > > > -};
> > > > -
> > > > static const VMStateDescription vmstate_vmxnet3 = {
> > > > .name = "vmxnet3",
> > > > .version_id = 1,
> > > > @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
> > > > .pre_save = vmxnet3_pre_save,
> > > > .post_load = vmxnet3_post_load,
> > > > .fields = (VMStateField[]) {
> > > > - VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
> > > > - vmxnet3_vmstate_test_pci_device, 0,
> > > > - vmstate_pci_device, PCIDevice),
> > > > + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > > > + VMSTATE_MSIX(parent_obj, VMXNET3State),
> > > > VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
> > > > VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
> > > > VMSTATE_BOOL(lro_supported, VMXNET3State),
> > > > @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
> > > > },
> > > > .subsections = (const VMStateDescription*[]) {
> > > > &vmxstate_vmxnet3_mcast_list,
> > > > - &vmstate_vmxnet3_pcie_device,
> > > > NULL
> > > > }
> > > > };
> > > > --
> > > > 2.17.1
> > > >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
2019-07-08 16:03 ` Dr. David Alan Gilbert
@ 2019-07-08 18:47 ` Marcel Apfelbaum
2019-07-08 19:00 ` Dmitry Fleytman
1 sibling, 0 replies; 10+ messages in thread
From: Marcel Apfelbaum @ 2019-07-08 18:47 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Sukrit Bhatnagar, Dmitry Fleytman, qemu-devel, Yuval Shaia
On 7/8/19 7:03 PM, Dr. David Alan Gilbert wrote:
> * Marcel Apfelbaum (marcel.apfelbaum@gmail.com) wrote:
>>
>> On 7/5/19 2:14 PM, Sukrit Bhatnagar wrote:
>>> On Fri, 5 Jul 2019 at 16:29, Dmitry Fleytman <dmitry.fleytman@gmail.com> wrote:
>>>>> On 5 Jul 2019, at 4:07, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
>>>>>
>>>>> At some point vmxnet3 live migration stopped working and git-bisect
>>>>> didn't help finding a working version.
>>>>> The issue is the PCI configuration space is not being migrated
>>>>> successfully and MSIX remains masked at destination.
>>>>>
>>>>> Remove the migration differentiation between PCI and PCIe since
>>>>> the logic resides now inside VMSTATE_PCI_DEVICE.
>>>>> Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
>>>>> since at 'realize' time is decided if the device is PCI or PCIe,
>>>>> then the above macro is enough.
>>>>>
>>>>> Use the opportunity to move to the standard VMSTATE_MSIX
>>>>> instead of the deprecated SaveVMHandlers.
>>>>>
>>>>> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>>>> Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
>>>>
>>> Tested-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
>> Thanks for the fast testing and review!
>>
>> David, since the live migration was broken long before this patch,
>> I don't need to add any compatibility code, right?
> Right, although we should probably bump the version_id field, that way
> you'll get a nice clean error if you try and migrate from the old<->new.
Will do, thanks.
> (It's nice to get rid of the old msix oddity).
>
>> If so, can you merge it using your migration tree?
> I could do; or since it's entirely in vmxnet3 Dmitry can take it.
I'll ask him, thanks!
Marcel
>
> Dave
>
>> Thanks,
>> Marcel
>>
>>
>>>>> ---
>>>>> hw/net/vmxnet3.c | 52 ++----------------------------------------------
>>>>> 1 file changed, 2 insertions(+), 50 deletions(-)
>>>>>
>>>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>>>>> index 10d01d0058..8b17548b02 100644
>>>>> --- a/hw/net/vmxnet3.c
>>>>> +++ b/hw/net/vmxnet3.c
>>>>> @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
>>>>> msi_uninit(d);
>>>>> }
>>>>>
>>>>> -static void
>>>>> -vmxnet3_msix_save(QEMUFile *f, void *opaque)
>>>>> -{
>>>>> - PCIDevice *d = PCI_DEVICE(opaque);
>>>>> - msix_save(d, f);
>>>>> -}
>>>>> -
>>>>> -static int
>>>>> -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
>>>>> -{
>>>>> - PCIDevice *d = PCI_DEVICE(opaque);
>>>>> - msix_load(d, f);
>>>>> - return 0;
>>>>> -}
>>>>> -
>>>>> static const MemoryRegionOps b0_ops = {
>>>>> .read = vmxnet3_io_bar0_read,
>>>>> .write = vmxnet3_io_bar0_write,
>>>>> @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
>>>>> },
>>>>> };
>>>>>
>>>>> -static SaveVMHandlers savevm_vmxnet3_msix = {
>>>>> - .save_state = vmxnet3_msix_save,
>>>>> - .load_state = vmxnet3_msix_load,
>>>>> -};
>>>>> -
>>>>> static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>>>>> {
>>>>> uint64_t dsn_payload;
>>>>> @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>>>>>
>>>>> static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
>>>>> {
>>>>> - DeviceState *dev = DEVICE(pci_dev);
>>>>> VMXNET3State *s = VMXNET3(pci_dev);
>>>>> int ret;
>>>>>
>>>>> @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
>>>>> pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
>>>>> vmxnet3_device_serial_num(s));
>>>>> }
>>>>> -
>>>>> - register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
>>>>> }
>>>>>
>>>>> static void vmxnet3_instance_init(Object *obj)
>>>>> @@ -2440,29 +2417,6 @@ static const VMStateDescription vmstate_vmxnet3_int_state = {
>>>>> }
>>>>> };
>>>>>
>>>>> -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
>>>>> -{
>>>>> - VMXNET3State *s = VMXNET3(opaque);
>>>>> -
>>>>> - return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
>>>>> -}
>>>>> -
>>>>> -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
>>>>> -{
>>>>> - return !vmxnet3_vmstate_need_pcie_device(opaque);
>>>>> -}
>>>>> -
>>>>> -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
>>>>> - .name = "vmxnet3/pcie",
>>>>> - .version_id = 1,
>>>>> - .minimum_version_id = 1,
>>>>> - .needed = vmxnet3_vmstate_need_pcie_device,
>>>>> - .fields = (VMStateField[]) {
>>>>> - VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
>>>>> - VMSTATE_END_OF_LIST()
>>>>> - }
>>>>> -};
>>>>> -
>>>>> static const VMStateDescription vmstate_vmxnet3 = {
>>>>> .name = "vmxnet3",
>>>>> .version_id = 1,
>>>>> @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
>>>>> .pre_save = vmxnet3_pre_save,
>>>>> .post_load = vmxnet3_post_load,
>>>>> .fields = (VMStateField[]) {
>>>>> - VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
>>>>> - vmxnet3_vmstate_test_pci_device, 0,
>>>>> - vmstate_pci_device, PCIDevice),
>>>>> + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
>>>>> + VMSTATE_MSIX(parent_obj, VMXNET3State),
>>>>> VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
>>>>> VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
>>>>> VMSTATE_BOOL(lro_supported, VMXNET3State),
>>>>> @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
>>>>> },
>>>>> .subsections = (const VMStateDescription*[]) {
>>>>> &vmxstate_vmxnet3_mcast_list,
>>>>> - &vmstate_vmxnet3_pcie_device,
>>>>> NULL
>>>>> }
>>>>> };
>>>>> --
>>>>> 2.17.1
>>>>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
2019-07-08 16:03 ` Dr. David Alan Gilbert
2019-07-08 18:47 ` Marcel Apfelbaum
@ 2019-07-08 19:00 ` Dmitry Fleytman
2019-07-08 19:03 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 10+ messages in thread
From: Dmitry Fleytman @ 2019-07-08 19:00 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Sukrit Bhatnagar, Yuval Shaia, qemu-devel
> On 8 Jul 2019, at 19:03, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
>
> * Marcel Apfelbaum (marcel.apfelbaum@gmail.com) wrote:
>>
>>
>>> On 7/5/19 2:14 PM, Sukrit Bhatnagar wrote:
>>>> On Fri, 5 Jul 2019 at 16:29, Dmitry Fleytman <dmitry.fleytman@gmail.com> wrote:
>>>>
>>>>> On 5 Jul 2019, at 4:07, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
>>>>>
>>>>> At some point vmxnet3 live migration stopped working and git-bisect
>>>>> didn't help finding a working version.
>>>>> The issue is the PCI configuration space is not being migrated
>>>>> successfully and MSIX remains masked at destination.
>>>>>
>>>>> Remove the migration differentiation between PCI and PCIe since
>>>>> the logic resides now inside VMSTATE_PCI_DEVICE.
>>>>> Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
>>>>> since at 'realize' time is decided if the device is PCI or PCIe,
>>>>> then the above macro is enough.
>>>>>
>>>>> Use the opportunity to move to the standard VMSTATE_MSIX
>>>>> instead of the deprecated SaveVMHandlers.
>>>>>
>>>>> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>>>>
>>>> Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
>>>>
>>> Tested-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
>>
>> Thanks for the fast testing and review!
>>
>> David, since the live migration was broken long before this patch,
>> I don't need to add any compatibility code, right?
>
> Right, although we should probably bump the version_id field, that way
> you'll get a nice clean error if you try and migrate from the old<->new.
>
> (It's nice to get rid of the old msix oddity).
>
>> If so, can you merge it using your migration tree?
>
> I could do; or since it's entirely in vmxnet3 Dmitry can take it.
Dave, please take it to your migration tree.
Thanks, Dmitry.
>
> Dave
>
>> Thanks,
>> Marcel
>>
>>
>>>>> ---
>>>>> hw/net/vmxnet3.c | 52 ++----------------------------------------------
>>>>> 1 file changed, 2 insertions(+), 50 deletions(-)
>>>>>
>>>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>>>>> index 10d01d0058..8b17548b02 100644
>>>>> --- a/hw/net/vmxnet3.c
>>>>> +++ b/hw/net/vmxnet3.c
>>>>> @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
>>>>> msi_uninit(d);
>>>>> }
>>>>>
>>>>> -static void
>>>>> -vmxnet3_msix_save(QEMUFile *f, void *opaque)
>>>>> -{
>>>>> - PCIDevice *d = PCI_DEVICE(opaque);
>>>>> - msix_save(d, f);
>>>>> -}
>>>>> -
>>>>> -static int
>>>>> -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
>>>>> -{
>>>>> - PCIDevice *d = PCI_DEVICE(opaque);
>>>>> - msix_load(d, f);
>>>>> - return 0;
>>>>> -}
>>>>> -
>>>>> static const MemoryRegionOps b0_ops = {
>>>>> .read = vmxnet3_io_bar0_read,
>>>>> .write = vmxnet3_io_bar0_write,
>>>>> @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
>>>>> },
>>>>> };
>>>>>
>>>>> -static SaveVMHandlers savevm_vmxnet3_msix = {
>>>>> - .save_state = vmxnet3_msix_save,
>>>>> - .load_state = vmxnet3_msix_load,
>>>>> -};
>>>>> -
>>>>> static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>>>>> {
>>>>> uint64_t dsn_payload;
>>>>> @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>>>>>
>>>>> static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
>>>>> {
>>>>> - DeviceState *dev = DEVICE(pci_dev);
>>>>> VMXNET3State *s = VMXNET3(pci_dev);
>>>>> int ret;
>>>>>
>>>>> @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
>>>>> pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
>>>>> vmxnet3_device_serial_num(s));
>>>>> }
>>>>> -
>>>>> - register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
>>>>> }
>>>>>
>>>>> static void vmxnet3_instance_init(Object *obj)
>>>>> @@ -2440,29 +2417,6 @@ static const VMStateDescription vmstate_vmxnet3_int_state = {
>>>>> }
>>>>> };
>>>>>
>>>>> -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
>>>>> -{
>>>>> - VMXNET3State *s = VMXNET3(opaque);
>>>>> -
>>>>> - return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
>>>>> -}
>>>>> -
>>>>> -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
>>>>> -{
>>>>> - return !vmxnet3_vmstate_need_pcie_device(opaque);
>>>>> -}
>>>>> -
>>>>> -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
>>>>> - .name = "vmxnet3/pcie",
>>>>> - .version_id = 1,
>>>>> - .minimum_version_id = 1,
>>>>> - .needed = vmxnet3_vmstate_need_pcie_device,
>>>>> - .fields = (VMStateField[]) {
>>>>> - VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
>>>>> - VMSTATE_END_OF_LIST()
>>>>> - }
>>>>> -};
>>>>> -
>>>>> static const VMStateDescription vmstate_vmxnet3 = {
>>>>> .name = "vmxnet3",
>>>>> .version_id = 1,
>>>>> @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
>>>>> .pre_save = vmxnet3_pre_save,
>>>>> .post_load = vmxnet3_post_load,
>>>>> .fields = (VMStateField[]) {
>>>>> - VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
>>>>> - vmxnet3_vmstate_test_pci_device, 0,
>>>>> - vmstate_pci_device, PCIDevice),
>>>>> + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
>>>>> + VMSTATE_MSIX(parent_obj, VMXNET3State),
>>>>> VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
>>>>> VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
>>>>> VMSTATE_BOOL(lro_supported, VMXNET3State),
>>>>> @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
>>>>> },
>>>>> .subsections = (const VMStateDescription*[]) {
>>>>> &vmxstate_vmxnet3_mcast_list,
>>>>> - &vmstate_vmxnet3_pcie_device,
>>>>> NULL
>>>>> }
>>>>> };
>>>>> --
>>>>> 2.17.1
>>>>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
2019-07-08 19:00 ` Dmitry Fleytman
@ 2019-07-08 19:03 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 10+ messages in thread
From: Dr. David Alan Gilbert @ 2019-07-08 19:03 UTC (permalink / raw)
To: Dmitry Fleytman; +Cc: Sukrit Bhatnagar, Yuval Shaia, qemu-devel
* Dmitry Fleytman (dmitry.fleytman@gmail.com) wrote:
>
>
> > On 8 Jul 2019, at 19:03, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> >
> > * Marcel Apfelbaum (marcel.apfelbaum@gmail.com) wrote:
> >>
> >>
> >>> On 7/5/19 2:14 PM, Sukrit Bhatnagar wrote:
> >>>> On Fri, 5 Jul 2019 at 16:29, Dmitry Fleytman <dmitry.fleytman@gmail.com> wrote:
> >>>>
> >>>>> On 5 Jul 2019, at 4:07, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
> >>>>>
> >>>>> At some point vmxnet3 live migration stopped working and git-bisect
> >>>>> didn't help finding a working version.
> >>>>> The issue is the PCI configuration space is not being migrated
> >>>>> successfully and MSIX remains masked at destination.
> >>>>>
> >>>>> Remove the migration differentiation between PCI and PCIe since
> >>>>> the logic resides now inside VMSTATE_PCI_DEVICE.
> >>>>> Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
> >>>>> since at 'realize' time is decided if the device is PCI or PCIe,
> >>>>> then the above macro is enough.
> >>>>>
> >>>>> Use the opportunity to move to the standard VMSTATE_MSIX
> >>>>> instead of the deprecated SaveVMHandlers.
> >>>>>
> >>>>> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> >>>>
> >>>> Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
> >>>>
> >>> Tested-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
> >>
> >> Thanks for the fast testing and review!
> >>
> >> David, since the live migration was broken long before this patch,
> >> I don't need to add any compatibility code, right?
> >
> > Right, although we should probably bump the version_id field, that way
> > you'll get a nice clean error if you try and migrate from the old<->new.
> >
> > (It's nice to get rid of the old msix oddity).
> >
> >> If so, can you merge it using your migration tree?
> >
> > I could do; or since it's entirely in vmxnet3 Dmitry can take it.
>
> Dave, please take it to your migration tree.
OK, will do.
Dave
> Thanks, Dmitry.
>
> >
> > Dave
> >
> >> Thanks,
> >> Marcel
> >>
> >>
> >>>>> ---
> >>>>> hw/net/vmxnet3.c | 52 ++----------------------------------------------
> >>>>> 1 file changed, 2 insertions(+), 50 deletions(-)
> >>>>>
> >>>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> >>>>> index 10d01d0058..8b17548b02 100644
> >>>>> --- a/hw/net/vmxnet3.c
> >>>>> +++ b/hw/net/vmxnet3.c
> >>>>> @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
> >>>>> msi_uninit(d);
> >>>>> }
> >>>>>
> >>>>> -static void
> >>>>> -vmxnet3_msix_save(QEMUFile *f, void *opaque)
> >>>>> -{
> >>>>> - PCIDevice *d = PCI_DEVICE(opaque);
> >>>>> - msix_save(d, f);
> >>>>> -}
> >>>>> -
> >>>>> -static int
> >>>>> -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
> >>>>> -{
> >>>>> - PCIDevice *d = PCI_DEVICE(opaque);
> >>>>> - msix_load(d, f);
> >>>>> - return 0;
> >>>>> -}
> >>>>> -
> >>>>> static const MemoryRegionOps b0_ops = {
> >>>>> .read = vmxnet3_io_bar0_read,
> >>>>> .write = vmxnet3_io_bar0_write,
> >>>>> @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
> >>>>> },
> >>>>> };
> >>>>>
> >>>>> -static SaveVMHandlers savevm_vmxnet3_msix = {
> >>>>> - .save_state = vmxnet3_msix_save,
> >>>>> - .load_state = vmxnet3_msix_load,
> >>>>> -};
> >>>>> -
> >>>>> static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> >>>>> {
> >>>>> uint64_t dsn_payload;
> >>>>> @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> >>>>>
> >>>>> static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> >>>>> {
> >>>>> - DeviceState *dev = DEVICE(pci_dev);
> >>>>> VMXNET3State *s = VMXNET3(pci_dev);
> >>>>> int ret;
> >>>>>
> >>>>> @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> >>>>> pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
> >>>>> vmxnet3_device_serial_num(s));
> >>>>> }
> >>>>> -
> >>>>> - register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
> >>>>> }
> >>>>>
> >>>>> static void vmxnet3_instance_init(Object *obj)
> >>>>> @@ -2440,29 +2417,6 @@ static const VMStateDescription vmstate_vmxnet3_int_state = {
> >>>>> }
> >>>>> };
> >>>>>
> >>>>> -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
> >>>>> -{
> >>>>> - VMXNET3State *s = VMXNET3(opaque);
> >>>>> -
> >>>>> - return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
> >>>>> -}
> >>>>> -
> >>>>> -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
> >>>>> -{
> >>>>> - return !vmxnet3_vmstate_need_pcie_device(opaque);
> >>>>> -}
> >>>>> -
> >>>>> -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> >>>>> - .name = "vmxnet3/pcie",
> >>>>> - .version_id = 1,
> >>>>> - .minimum_version_id = 1,
> >>>>> - .needed = vmxnet3_vmstate_need_pcie_device,
> >>>>> - .fields = (VMStateField[]) {
> >>>>> - VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> >>>>> - VMSTATE_END_OF_LIST()
> >>>>> - }
> >>>>> -};
> >>>>> -
> >>>>> static const VMStateDescription vmstate_vmxnet3 = {
> >>>>> .name = "vmxnet3",
> >>>>> .version_id = 1,
> >>>>> @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
> >>>>> .pre_save = vmxnet3_pre_save,
> >>>>> .post_load = vmxnet3_post_load,
> >>>>> .fields = (VMStateField[]) {
> >>>>> - VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
> >>>>> - vmxnet3_vmstate_test_pci_device, 0,
> >>>>> - vmstate_pci_device, PCIDevice),
> >>>>> + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> >>>>> + VMSTATE_MSIX(parent_obj, VMXNET3State),
> >>>>> VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
> >>>>> VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
> >>>>> VMSTATE_BOOL(lro_supported, VMXNET3State),
> >>>>> @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
> >>>>> },
> >>>>> .subsections = (const VMStateDescription*[]) {
> >>>>> &vmxstate_vmxnet3_mcast_list,
> >>>>> - &vmstate_vmxnet3_pcie_device,
> >>>>> NULL
> >>>>> }
> >>>>> };
> >>>>> --
> >>>>> 2.17.1
> >>>>>
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
2019-07-05 1:07 [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration Marcel Apfelbaum
2019-07-05 10:57 ` Sukrit Bhatnagar
2019-07-05 10:59 ` Dmitry Fleytman
@ 2019-08-07 17:12 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 10+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-07 17:12 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: skrtbhtngr, dmitry.fleytman, qemu-devel, yuval.shaia
* Marcel Apfelbaum (marcel.apfelbaum@gmail.com) wrote:
> At some point vmxnet3 live migration stopped working and git-bisect
> didn't help finding a working version.
> The issue is the PCI configuration space is not being migrated
> successfully and MSIX remains masked at destination.
>
> Remove the migration differentiation between PCI and PCIe since
> the logic resides now inside VMSTATE_PCI_DEVICE.
> Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
> since at 'realize' time is decided if the device is PCI or PCIe,
> then the above macro is enough.
>
> Use the opportunity to move to the standard VMSTATE_MSIX
> instead of the deprecated SaveVMHandlers.
>
> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Queued
> ---
> hw/net/vmxnet3.c | 52 ++----------------------------------------------
> 1 file changed, 2 insertions(+), 50 deletions(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 10d01d0058..8b17548b02 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
> msi_uninit(d);
> }
>
> -static void
> -vmxnet3_msix_save(QEMUFile *f, void *opaque)
> -{
> - PCIDevice *d = PCI_DEVICE(opaque);
> - msix_save(d, f);
> -}
> -
> -static int
> -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
> -{
> - PCIDevice *d = PCI_DEVICE(opaque);
> - msix_load(d, f);
> - return 0;
> -}
> -
> static const MemoryRegionOps b0_ops = {
> .read = vmxnet3_io_bar0_read,
> .write = vmxnet3_io_bar0_write,
> @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
> },
> };
>
> -static SaveVMHandlers savevm_vmxnet3_msix = {
> - .save_state = vmxnet3_msix_save,
> - .load_state = vmxnet3_msix_load,
> -};
> -
> static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> {
> uint64_t dsn_payload;
> @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>
> static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> {
> - DeviceState *dev = DEVICE(pci_dev);
> VMXNET3State *s = VMXNET3(pci_dev);
> int ret;
>
> @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
> vmxnet3_device_serial_num(s));
> }
> -
> - register_savevm_live(dev, "vmxnet3-msix", -1, 1, &savevm_vmxnet3_msix, s);
> }
>
> static void vmxnet3_instance_init(Object *obj)
> @@ -2440,29 +2417,6 @@ static const VMStateDescription vmstate_vmxnet3_int_state = {
> }
> };
>
> -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
> -{
> - VMXNET3State *s = VMXNET3(opaque);
> -
> - return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
> -}
> -
> -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
> -{
> - return !vmxnet3_vmstate_need_pcie_device(opaque);
> -}
> -
> -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> - .name = "vmxnet3/pcie",
> - .version_id = 1,
> - .minimum_version_id = 1,
> - .needed = vmxnet3_vmstate_need_pcie_device,
> - .fields = (VMStateField[]) {
> - VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> - VMSTATE_END_OF_LIST()
> - }
> -};
> -
> static const VMStateDescription vmstate_vmxnet3 = {
> .name = "vmxnet3",
> .version_id = 1,
> @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
> .pre_save = vmxnet3_pre_save,
> .post_load = vmxnet3_post_load,
> .fields = (VMStateField[]) {
> - VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
> - vmxnet3_vmstate_test_pci_device, 0,
> - vmstate_pci_device, PCIDevice),
> + VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> + VMSTATE_MSIX(parent_obj, VMXNET3State),
> VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
> VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
> VMSTATE_BOOL(lro_supported, VMXNET3State),
> @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
> },
> .subsections = (const VMStateDescription*[]) {
> &vmxstate_vmxnet3_mcast_list,
> - &vmstate_vmxnet3_pcie_device,
> NULL
> }
> };
> --
> 2.17.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 10+ messages in thread