Message ID | 20210720181644.196315-1-lvivier@redhat.com |
---|---|
State | New |
Headers | show |
Series | failover: unregister ram on unplug | expand |
On Tue, 20 Jul 2021 20:16:44 +0200 Laurent Vivier <lvivier@redhat.com> wrote: > This simple change allows to test failover with a simulated device > like e1000e rather than a vfio device. > > This is interesting to developers that want to test failover on > a system with no vfio device. Moreover it simplifies host networking > configuration as we can use the same bridge for virtio-net and > the other failover networking device. > > Without this change the migration of a system configured with failover > fails with: > > Unknown ramblock "0000:00:01.1:00.0/e1000e.rom", cannot accept migration > error while loading state for instance 0x0 of device 'ram' > load of migration failed: Invalid argument > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/net/virtio-net.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 16d20cdee52a..8f7735bad4f2 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -3256,6 +3256,9 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s) > if (migration_in_setup(s) && !should_be_hidden) { > if (failover_unplug_primary(n, dev)) { > vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev); > + if (PCI_DEVICE(dev)->has_rom) { > + vmstate_unregister_ram(&PCI_DEVICE(dev)->rom , dev); > + } > qapi_event_send_unplug_primary(dev->id); > qatomic_set(&n->failover_primary_hidden, true); > } else {
On 21/07/2021 10:58, Igor Mammedov wrote: > On Tue, 20 Jul 2021 20:16:44 +0200 > Laurent Vivier <lvivier@redhat.com> wrote: > >> This simple change allows to test failover with a simulated device >> like e1000e rather than a vfio device. >> >> This is interesting to developers that want to test failover on >> a system with no vfio device. Moreover it simplifies host networking >> configuration as we can use the same bridge for virtio-net and >> the other failover networking device. >> >> Without this change the migration of a system configured with failover >> fails with: >> >> Unknown ramblock "0000:00:01.1:00.0/e1000e.rom", cannot accept migration >> error while loading state for instance 0x0 of device 'ram' >> load of migration failed: Invalid argument >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > I've sent a v2 of this patch to reset has_rom to false. I've updated the commit log message and the subject of the email to "failover: unregister ROM on unplug" (that is more correct). Thanks, Laurent
On Wed, Jul 21, 2021 at 10:58:17AM +0200, Igor Mammedov wrote: > On Tue, 20 Jul 2021 20:16:44 +0200 > Laurent Vivier <lvivier@redhat.com> wrote: > > > This simple change allows to test failover with a simulated device > > like e1000e rather than a vfio device. > > > > This is interesting to developers that want to test failover on > > a system with no vfio device. Moreover it simplifies host networking > > configuration as we can use the same bridge for virtio-net and > > the other failover networking device. > > > > Without this change the migration of a system configured with failover > > fails with: > > > > Unknown ramblock "0000:00:01.1:00.0/e1000e.rom", cannot accept migration > > error while loading state for instance 0x0 of device 'ram' > > load of migration failed: Invalid argument > > > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > hw/net/virtio-net.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > index 16d20cdee52a..8f7735bad4f2 100644 > > --- a/hw/net/virtio-net.c > > +++ b/hw/net/virtio-net.c > > @@ -3256,6 +3256,9 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s) > > if (migration_in_setup(s) && !should_be_hidden) { > > if (failover_unplug_primary(n, dev)) { > > vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev); > > + if (PCI_DEVICE(dev)->has_rom) { Hmm. Any way to hide this behind an interface so we don't need to poke at pci device internals? > > + vmstate_unregister_ram(&PCI_DEVICE(dev)->rom , dev); > > + } > > qapi_event_send_unplug_primary(dev->id); > > qatomic_set(&n->failover_primary_hidden, true); > > } else {
On 21/07/2021 12:41, Michael S. Tsirkin wrote: > On Wed, Jul 21, 2021 at 10:58:17AM +0200, Igor Mammedov wrote: >> On Tue, 20 Jul 2021 20:16:44 +0200 >> Laurent Vivier <lvivier@redhat.com> wrote: >> >>> This simple change allows to test failover with a simulated device >>> like e1000e rather than a vfio device. >>> >>> This is interesting to developers that want to test failover on >>> a system with no vfio device. Moreover it simplifies host networking >>> configuration as we can use the same bridge for virtio-net and >>> the other failover networking device. >>> >>> Without this change the migration of a system configured with failover >>> fails with: >>> >>> Unknown ramblock "0000:00:01.1:00.0/e1000e.rom", cannot accept migration >>> error while loading state for instance 0x0 of device 'ram' >>> load of migration failed: Invalid argument >>> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> >> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >> >>> --- >>> hw/net/virtio-net.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>> index 16d20cdee52a..8f7735bad4f2 100644 >>> --- a/hw/net/virtio-net.c >>> +++ b/hw/net/virtio-net.c >>> @@ -3256,6 +3256,9 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s) >>> if (migration_in_setup(s) && !should_be_hidden) { >>> if (failover_unplug_primary(n, dev)) { >>> vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev); >>> + if (PCI_DEVICE(dev)->has_rom) { > > > Hmm. Any way to hide this behind an interface so > we don't need to poke at pci device internals? There is the pci_del_option_rom() but it's not exported. Do you want I export and use it? Thanks, Laurent > >>> + vmstate_unregister_ram(&PCI_DEVICE(dev)->rom , dev); >>> + } >>> qapi_event_send_unplug_primary(dev->id); >>> qatomic_set(&n->failover_primary_hidden, true); >>> } else { >
On 7/21/21 12:49 PM, Laurent Vivier wrote: > On 21/07/2021 12:41, Michael S. Tsirkin wrote: >> On Wed, Jul 21, 2021 at 10:58:17AM +0200, Igor Mammedov wrote: >>> On Tue, 20 Jul 2021 20:16:44 +0200 >>> Laurent Vivier <lvivier@redhat.com> wrote: >>> >>>> This simple change allows to test failover with a simulated device >>>> like e1000e rather than a vfio device. >>>> >>>> This is interesting to developers that want to test failover on >>>> a system with no vfio device. Moreover it simplifies host networking >>>> configuration as we can use the same bridge for virtio-net and >>>> the other failover networking device. >>>> >>>> Without this change the migration of a system configured with failover >>>> fails with: >>>> >>>> Unknown ramblock "0000:00:01.1:00.0/e1000e.rom", cannot accept migration >>>> error while loading state for instance 0x0 of device 'ram' >>>> load of migration failed: Invalid argument >>>> >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >>> >>> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >>> >>>> --- >>>> hw/net/virtio-net.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>>> index 16d20cdee52a..8f7735bad4f2 100644 >>>> --- a/hw/net/virtio-net.c >>>> +++ b/hw/net/virtio-net.c >>>> @@ -3256,6 +3256,9 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s) >>>> if (migration_in_setup(s) && !should_be_hidden) { >>>> if (failover_unplug_primary(n, dev)) { >>>> vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev); >>>> + if (PCI_DEVICE(dev)->has_rom) { >> >> >> Hmm. Any way to hide this behind an interface so >> we don't need to poke at pci device internals? > > There is the pci_del_option_rom() but it's not exported. > > Do you want I export and use it? Looks cleaner indeed.
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 16d20cdee52a..8f7735bad4f2 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3256,6 +3256,9 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s) if (migration_in_setup(s) && !should_be_hidden) { if (failover_unplug_primary(n, dev)) { vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev); + if (PCI_DEVICE(dev)->has_rom) { + vmstate_unregister_ram(&PCI_DEVICE(dev)->rom , dev); + } qapi_event_send_unplug_primary(dev->id); qatomic_set(&n->failover_primary_hidden, true); } else {
This simple change allows to test failover with a simulated device like e1000e rather than a vfio device. This is interesting to developers that want to test failover on a system with no vfio device. Moreover it simplifies host networking configuration as we can use the same bridge for virtio-net and the other failover networking device. Without this change the migration of a system configured with failover fails with: Unknown ramblock "0000:00:01.1:00.0/e1000e.rom", cannot accept migration error while loading state for instance 0x0 of device 'ram' load of migration failed: Invalid argument Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- hw/net/virtio-net.c | 3 +++ 1 file changed, 3 insertions(+)