* [PATCH] failover: unregister ram on unplug
@ 2021-07-20 18:16 Laurent Vivier
2021-07-21 8:58 ` Igor Mammedov
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2021-07-20 18:16 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, Jens Freimann, Michael S. Tsirkin,
Dr . David Alan Gilbert, Juan Quintela
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(+)
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 {
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] failover: unregister ram on unplug
2021-07-20 18:16 [PATCH] failover: unregister ram on unplug Laurent Vivier
@ 2021-07-21 8:58 ` Igor Mammedov
2021-07-21 9:31 ` Laurent Vivier
2021-07-21 10:41 ` Michael S. Tsirkin
0 siblings, 2 replies; 6+ messages in thread
From: Igor Mammedov @ 2021-07-21 8:58 UTC (permalink / raw)
To: Laurent Vivier
Cc: Juan Quintela, Jason Wang, Michael S. Tsirkin, qemu-devel,
Dr . David Alan Gilbert, Jens Freimann
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 {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] failover: unregister ram on unplug
2021-07-21 8:58 ` Igor Mammedov
@ 2021-07-21 9:31 ` Laurent Vivier
2021-07-21 10:41 ` Michael S. Tsirkin
1 sibling, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2021-07-21 9:31 UTC (permalink / raw)
To: qemu-devel
Cc: Juan Quintela, Jason Wang, Michael S. Tsirkin,
Dr . David Alan Gilbert, Igor Mammedov, Jens Freimann
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] failover: unregister ram on unplug
2021-07-21 8:58 ` Igor Mammedov
2021-07-21 9:31 ` Laurent Vivier
@ 2021-07-21 10:41 ` Michael S. Tsirkin
2021-07-21 10:49 ` Laurent Vivier
1 sibling, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2021-07-21 10:41 UTC (permalink / raw)
To: Igor Mammedov
Cc: Laurent Vivier, Juan Quintela, Jason Wang, qemu-devel,
Dr . David Alan Gilbert, Jens Freimann
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 {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] failover: unregister ram on unplug
2021-07-21 10:41 ` Michael S. Tsirkin
@ 2021-07-21 10:49 ` Laurent Vivier
2021-07-21 15:36 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2021-07-21 10:49 UTC (permalink / raw)
To: Michael S. Tsirkin, Igor Mammedov
Cc: Juan Quintela, Jason Wang, Jens Freimann, qemu-devel,
Dr . David Alan Gilbert
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 {
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] failover: unregister ram on unplug
2021-07-21 10:49 ` Laurent Vivier
@ 2021-07-21 15:36 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-21 15:36 UTC (permalink / raw)
To: Laurent Vivier, Michael S. Tsirkin, Igor Mammedov
Cc: Jason Wang, Jens Freimann, qemu-devel, Dr . David Alan Gilbert,
Juan Quintela
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-21 15:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-20 18:16 [PATCH] failover: unregister ram on unplug Laurent Vivier
2021-07-21 8:58 ` Igor Mammedov
2021-07-21 9:31 ` Laurent Vivier
2021-07-21 10:41 ` Michael S. Tsirkin
2021-07-21 10:49 ` Laurent Vivier
2021-07-21 15:36 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).