* [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration
@ 2019-07-05 1:07 Marcel Apfelbaum
2019-07-05 10:57 ` Sukrit Bhatnagar
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Marcel Apfelbaum @ 2019-07-05 1:07 UTC (permalink / raw)
To: qemu-devel, dmitry.fleytman; +Cc: skrtbhtngr, dgilbert, yuval.shaia
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>
---
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 related [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: 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
end of thread, other threads:[~2019-08-07 17:13 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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
2019-07-08 19:03 ` Dr. David Alan Gilbert
2019-08-07 17:12 ` Dr. David Alan Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.