qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/1] vfio: Change default dirty pages tracking behavior durin


From: Alex Williamson
Subject: Re: [PATCH 1/1] vfio: Change default dirty pages tracking behavior during migration
Date: Thu, 19 Nov 2020 08:19:41 -0700

On Thu, 19 Nov 2020 09:48:01 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> By default dirty pages tracking is enabled during iterative phase
> (pre-copy phase).
> Added per device opt-out option 'pre-copy-dirty-page-tracking' to
> disable dirty pages tracking during iterative phase. If the option
> 'pre-copy-dirty-page-tracking=off' is set for any VFIO device, dirty
> pages tracking during iterative phase will be disabled.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> ---
>  hw/vfio/common.c              | 11 +++++++----
>  hw/vfio/pci.c                 |  3 +++
>  include/hw/vfio/vfio-common.h |  1 +
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index c1fdbf17f2e6..6ff1daa763f8 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -311,7 +311,7 @@ bool vfio_mig_active(void)
>      return true;
>  }
>  
> -static bool vfio_devices_all_stopped_and_saving(VFIOContainer *container)
> +static bool vfio_devices_all_saving(VFIOContainer *container)
>  {
>      VFIOGroup *group;
>      VFIODevice *vbasedev;
> @@ -329,8 +329,11 @@ static bool 
> vfio_devices_all_stopped_and_saving(VFIOContainer *container)
>                  return false;
>              }
>  
> -            if ((migration->device_state & VFIO_DEVICE_STATE_SAVING) &&
> -                !(migration->device_state & VFIO_DEVICE_STATE_RUNNING)) {
> +            if (migration->device_state & VFIO_DEVICE_STATE_SAVING) {
> +                if ((vbasedev->pre_copy_dirty_page_tracking == 
> ON_OFF_AUTO_OFF)
> +                    && (migration->device_state & 
> VFIO_DEVICE_STATE_RUNNING)) {
> +                        return false;
> +                }
>                  continue;
>              } else {
>                  return false;
> @@ -1125,7 +1128,7 @@ static void vfio_listerner_log_sync(MemoryListener 
> *listener,
>          return;
>      }
>  
> -    if (vfio_devices_all_stopped_and_saving(container)) {
> +    if (vfio_devices_all_saving(container)) {
>          vfio_sync_dirty_bitmap(container, section);
>      }
>  }
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 58c0ce8971e3..5bea4b3e71f5 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3182,6 +3182,9 @@ static void vfio_instance_init(Object *obj)
>  static Property vfio_pci_dev_properties[] = {
>      DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host),
>      DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev),
> +    DEFINE_PROP_ON_OFF_AUTO("pre-copy-dirty-page-tracking", VFIOPCIDevice,
> +                            vbasedev.pre_copy_dirty_page_tracking,
> +                            ON_OFF_AUTO_ON),


Shouldn't this be an experimental option, ie.
x-pre-copy-dirty-page-tracking?  We can always make it a supported
option at a later time, but creating it as a supported option and later
changing our mind would require a deprecation process.  As I see it,
this option limits the device's ability to fully participate in
migration, leading to potentially erroneous downtime estimates and
should therefore, at best, make it an experimental option.  We also
have no data yet as to scenarios and configurations when this option
might be recommended.  Thanks,

Alex

>      DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
>                              display, ON_OFF_AUTO_OFF),
>      DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index baeb4dcff102..267cf854bbba 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -129,6 +129,7 @@ typedef struct VFIODevice {
>      unsigned int flags;
>      VFIOMigration *migration;
>      Error *migration_blocker;
> +    OnOffAuto pre_copy_dirty_page_tracking;
>  } VFIODevice;
>  
>  struct VFIODeviceOps {




reply via email to

[Prev in Thread] Current Thread [Next in Thread]