[v1] add failover feature for assigned network devices

[Qemu-devel] [PATCH 1/4] migration: allow unplug during migration for failover devices

Posted by Jens Freimann 6 years, 8 months ago

In "b06424de62 migration: Disable hotplug/unplug during migration" we
added a check to disable unplug for all devices until we have figured
out what works. For failover primary devices qdev_unplug() is called
from the migration handler, i.e. during migration.

This patch adds a flag to DeviceState which is set to false for all
devices and makes an exception for vfio-pci devices that are also
primary devices in a failover pair.

Signed-off-by: Jens Freimann <jfreimann@redhat.com>
---
 hw/core/qdev.c         | 1 +
 include/hw/qdev-core.h | 1 +
 qdev-monitor.c         | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f9b6efe509..98cdaa6bf7 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -954,6 +954,7 @@ static void device_initfn(Object *obj)
 
     dev->instance_id_alias = -1;
     dev->realized = false;
+    dev->allow_unplug_during_migration = false;
 
     object_property_add_bool(obj, "realized",
                              device_get_realized, device_set_realized, NULL);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 33ed3b8dde..5437395779 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -146,6 +146,7 @@ struct DeviceState {
     bool pending_deleted_event;
     QemuOpts *opts;
     int hotplugged;
+    bool allow_unplug_during_migration;
     BusState *parent_bus;
     QLIST_HEAD(, NamedGPIOList) gpios;
     QLIST_HEAD(, BusState) child_bus;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 373b9ad445..9cce8b93c2 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -867,7 +867,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
         return;
     }
 
-    if (!migration_is_idle()) {
+    if (!migration_is_idle() && !dev->allow_unplug_during_migration) {
         error_setg(errp, "device_del not allowed while migrating");
         return;
     }
-- 
2.21.0

Re: [Qemu-devel] [PATCH 1/4] migration: allow unplug during migration for failover devices

Posted by Dr. David Alan Gilbert 6 years, 8 months ago

* Jens Freimann (jfreimann@redhat.com) wrote:
> In "b06424de62 migration: Disable hotplug/unplug during migration" we
> added a check to disable unplug for all devices until we have figured
> out what works. For failover primary devices qdev_unplug() is called
> from the migration handler, i.e. during migration.
> 
> This patch adds a flag to DeviceState which is set to false for all
> devices and makes an exception for vfio-pci devices that are also
> primary devices in a failover pair.
> 
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>

So I think this is safe in your case, because you trigger the unplug
right at the start of migration during setup and plug after failure;
however it's not generally safe - I can't unplug a device while the
migration is actually in progress.

Dave

> ---
>  hw/core/qdev.c         | 1 +
>  include/hw/qdev-core.h | 1 +
>  qdev-monitor.c         | 2 +-
>  3 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index f9b6efe509..98cdaa6bf7 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -954,6 +954,7 @@ static void device_initfn(Object *obj)
>  
>      dev->instance_id_alias = -1;
>      dev->realized = false;
> +    dev->allow_unplug_during_migration = false;
>  
>      object_property_add_bool(obj, "realized",
>                               device_get_realized, device_set_realized, NULL);
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 33ed3b8dde..5437395779 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -146,6 +146,7 @@ struct DeviceState {
>      bool pending_deleted_event;
>      QemuOpts *opts;
>      int hotplugged;
> +    bool allow_unplug_during_migration;
>      BusState *parent_bus;
>      QLIST_HEAD(, NamedGPIOList) gpios;
>      QLIST_HEAD(, BusState) child_bus;
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 373b9ad445..9cce8b93c2 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -867,7 +867,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> -    if (!migration_is_idle()) {
> +    if (!migration_is_idle() && !dev->allow_unplug_during_migration) {
>          error_setg(errp, "device_del not allowed while migrating");
>          return;
>      }
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/4] migration: allow unplug during migration for failover devices

Posted by Daniel P. Berrangé 6 years, 8 months ago

On Tue, May 21, 2019 at 10:33:36AM +0100, Dr. David Alan Gilbert wrote:
> * Jens Freimann (jfreimann@redhat.com) wrote:
> > In "b06424de62 migration: Disable hotplug/unplug during migration" we
> > added a check to disable unplug for all devices until we have figured
> > out what works. For failover primary devices qdev_unplug() is called
> > from the migration handler, i.e. during migration.
> > 
> > This patch adds a flag to DeviceState which is set to false for all
> > devices and makes an exception for vfio-pci devices that are also
> > primary devices in a failover pair.
> > 
> > Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> 
> So I think this is safe in your case, because you trigger the unplug
> right at the start of migration during setup and plug after failure;
> however it's not generally safe - I can't unplug a device while the
> migration is actually in progress.

Libvirt will also block any attempt to hotplug/unplug device during
migration.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/4] migration: allow unplug during migration for failover devices

Posted by Jens Freimann 6 years, 8 months ago

On Tue, May 21, 2019 at 10:33:36AM +0100, Dr. David Alan Gilbert wrote:
>* Jens Freimann (jfreimann@redhat.com) wrote:
>> In "b06424de62 migration: Disable hotplug/unplug during migration" we
>> added a check to disable unplug for all devices until we have figured
>> out what works. For failover primary devices qdev_unplug() is called
>> from the migration handler, i.e. during migration.
>>
>> This patch adds a flag to DeviceState which is set to false for all
>> devices and makes an exception for vfio-pci devices that are also
>> primary devices in a failover pair.
>>
>> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
>
>So I think this is safe in your case, because you trigger the unplug
>right at the start of migration during setup and plug after failure;
>however it's not generally safe - I can't unplug a device while the
>migration is actually in progress.

I tried to limit it to only allow it in failover case. You're saying
it's missing something and not strict enough? I could allow it only
during migration setup. I guess we'll need a similar exception for
failover in libvirt. 

regards,
Jens

Re: [Qemu-devel] [PATCH 1/4] migration: allow unplug during migration for failover devices

Posted by Dr. David Alan Gilbert 6 years, 8 months ago

* Jens Freimann (jfreimann@redhat.com) wrote:
> On Tue, May 21, 2019 at 10:33:36AM +0100, Dr. David Alan Gilbert wrote:
> > * Jens Freimann (jfreimann@redhat.com) wrote:
> > > In "b06424de62 migration: Disable hotplug/unplug during migration" we
> > > added a check to disable unplug for all devices until we have figured
> > > out what works. For failover primary devices qdev_unplug() is called
> > > from the migration handler, i.e. during migration.
> > > 
> > > This patch adds a flag to DeviceState which is set to false for all
> > > devices and makes an exception for vfio-pci devices that are also
> > > primary devices in a failover pair.
> > > 
> > > Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> > 
> > So I think this is safe in your case, because you trigger the unplug
> > right at the start of migration during setup and plug after failure;
> > however it's not generally safe - I can't unplug a device while the
> > migration is actually in progress.
> 
> I tried to limit it to only allow it in failover case. You're saying
> it's missing something and not strict enough? I could allow it only
> during migration setup. I guess we'll need a similar exception for
> failover in libvirt.

I might be wrong, but I think with your patch I could hot unplug your
device part way through migration; where as I think you only care about
it doing it at a very specific point during setup.

(I still would prefer the hotplug to be done outside qemu, but still
that's separate).

Dave
> regards,
> Jens
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK