Re: [PATCH QEMU v25 07/17] vfio: Register SaveVMHandlers for VFIO device
* Kirti Wankhede (kwankh...@nvidia.com) wrote:
> Define flags to be used as delimeter in migration file stream.
> Added .save_setup and .save_cleanup functions. Mapped & unmapped migration
> region from these functions at source during saving or pre-copy phase.
> Set VFIO device state depending on VM's state. During live migration, VM is
> running when .save_setup is called, _SAVING | _RUNNING state is set for VFIO
> device. During save-restore, VM is paused, _SAVING state is set for VFIO
> device.
>
> Signed-off-by: Kirti Wankhede
> Reviewed-by: Neo Jia
> ---
> hw/vfio/migration.c | 92
>
> hw/vfio/trace-events | 2 ++
> 2 files changed, 94 insertions(+)
>
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index e30bd8768701..133bb5b1b3b2 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -8,12 +8,15 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/cutils.h"
> #include
>
> #include "sysemu/runstate.h"
> #include "hw/vfio/vfio-common.h"
> #include "cpu.h"
> #include "migration/migration.h"
> +#include "migration/vmstate.h"
> #include "migration/qemu-file.h"
> #include "migration/register.h"
> #include "migration/blocker.h"
> @@ -24,6 +27,17 @@
> #include "pci.h"
> #include "trace.h"
>
> +/*
> + * Flags used as delimiter:
> + * 0x => MSB 32-bit all 1s
> + * 0xef10 => emulated (virtual) function IO
> + * 0x => 16-bits reserved for flags
> + */
> +#define VFIO_MIG_FLAG_END_OF_STATE (0xef11ULL)
> +#define VFIO_MIG_FLAG_DEV_CONFIG_STATE (0xef12ULL)
> +#define VFIO_MIG_FLAG_DEV_SETUP_STATE (0xef13ULL)
> +#define VFIO_MIG_FLAG_DEV_DATA_STATE(0xef14ULL)
> +
> static void vfio_migration_region_exit(VFIODevice *vbasedev)
> {
> VFIOMigration *migration = vbasedev->migration;
> @@ -126,6 +140,65 @@ static int vfio_migration_set_state(VFIODevice
> *vbasedev, uint32_t mask,
> return 0;
> }
>
> +/* -- */
> +
> +static int vfio_save_setup(QEMUFile *f, void *opaque)
> +{
> +VFIODevice *vbasedev = opaque;
> +VFIOMigration *migration = vbasedev->migration;
> +int ret;
> +
> +trace_vfio_save_setup(vbasedev->name);
> +
> +qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
> +
> +if (migration->region.mmaps) {
> +qemu_mutex_lock_iothread();
> +ret = vfio_region_mmap(>region);
> +qemu_mutex_unlock_iothread();
> +if (ret) {
> +error_report("%s: Failed to mmap VFIO migration region %d: %s",
> + vbasedev->name, migration->region.nr,
> + strerror(-ret));
> +return ret;
> +}
> +}
> +
> +ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_MASK,
> + VFIO_DEVICE_STATE_SAVING);
> +if (ret) {
> +error_report("%s: Failed to set state SAVING", vbasedev->name);
> +return ret;
> +}
> +
> +qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
> +
> +ret = qemu_file_get_error(f);
> +if (ret) {
> +return ret;
> +}
> +
> +return 0;
> +}
> +
> +static void vfio_save_cleanup(void *opaque)
> +{
> +VFIODevice *vbasedev = opaque;
> +VFIOMigration *migration = vbasedev->migration;
> +
> +if (migration->region.mmaps) {
> +vfio_region_unmap(>region);
> +}
> +trace_vfio_save_cleanup(vbasedev->name);
> +}
> +
> +static SaveVMHandlers savevm_vfio_handlers = {
> +.save_setup = vfio_save_setup,
> +.save_cleanup = vfio_save_cleanup,
> +};
> +
> +/* -- */
> +
> static void vfio_vmstate_change(void *opaque, int running, RunState state)
> {
> VFIODevice *vbasedev = opaque;
> @@ -180,6 +253,7 @@ static int vfio_migration_init(VFIODevice *vbasedev,
> struct vfio_region_info *info)
> {
> int ret;
> +char id[256] = "";
>
> vbasedev->migration = g_new0(VFIOMigration, 1);
>
> @@ -192,6 +266,24 @@ static int vfio_migration_init(VFIODevice *vbasedev,
> return ret;
> }
>
> +if (vbasedev->ops->vfio_get_object) {
> +Object *obj = vbasedev->ops->vfio_get_object(vbasedev);
> +
> +if (obj) {
> +DeviceState *dev = DEVICE(obj);
> +char *oid = vmstate_if_get_id(VMSTATE_IF(dev));
> +
> +if (oid) {
> +pstrcpy(id, sizeof(id), oid);
> +pstrcat(id, sizeof(id), "/");
> +g_free(oid);
> +}
> +}
> +}
> +pstrcat(id, sizeof(id), "vfio");
> +
> +register_savevm_live(id, VMSTATE_INSTANCE_ID_ANY, 1,
> _vfio_handlers,
> + vbasedev);
Right, so this version has finally changed to using this 'id' string
with a copy of the code from
Re: [PATCH QEMU v25 07/17] vfio: Register SaveVMHandlers for VFIO device
* Alex Williamson (alex.william...@redhat.com) wrote:
> On Wed, 24 Jun 2020 00:51:06 +0530
> Kirti Wankhede wrote:
>
> > On 6/23/2020 4:20 AM, Alex Williamson wrote:
> > > On Sun, 21 Jun 2020 01:51:16 +0530
> > > Kirti Wankhede wrote:
> > >
> > >> Define flags to be used as delimeter in migration file stream.
> > >> Added .save_setup and .save_cleanup functions. Mapped & unmapped
> > >> migration
> > >> region from these functions at source during saving or pre-copy phase.
> > >> Set VFIO device state depending on VM's state. During live migration, VM
> > >> is
> > >> running when .save_setup is called, _SAVING | _RUNNING state is set for
> > >> VFIO
> > >> device. During save-restore, VM is paused, _SAVING state is set for VFIO
> > >> device.
> > >>
> > >> Signed-off-by: Kirti Wankhede
> > >> Reviewed-by: Neo Jia
> > >> ---
> > >> hw/vfio/migration.c | 92
> > >>
> > >> hw/vfio/trace-events | 2 ++
> > >> 2 files changed, 94 insertions(+)
> > >>
> > >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> > >> index e30bd8768701..133bb5b1b3b2 100644
> > >> --- a/hw/vfio/migration.c
> > >> +++ b/hw/vfio/migration.c
> > >> @@ -8,12 +8,15 @@
> > >>*/
> > >>
> > >> #include "qemu/osdep.h"
> > >> +#include "qemu/main-loop.h"
> > >> +#include "qemu/cutils.h"
> > >> #include
> > >>
> > >> #include "sysemu/runstate.h"
> > >> #include "hw/vfio/vfio-common.h"
> > >> #include "cpu.h"
> > >> #include "migration/migration.h"
> > >> +#include "migration/vmstate.h"
> > >> #include "migration/qemu-file.h"
> > >> #include "migration/register.h"
> > >> #include "migration/blocker.h"
> > >> @@ -24,6 +27,17 @@
> > >> #include "pci.h"
> > >> #include "trace.h"
> > >>
> > >> +/*
> > >> + * Flags used as delimiter:
> > >> + * 0x => MSB 32-bit all 1s
> > >> + * 0xef10 => emulated (virtual) function IO
> > >> + * 0x => 16-bits reserved for flags
> > >> + */
> > >> +#define VFIO_MIG_FLAG_END_OF_STATE (0xef11ULL)
> > >> +#define VFIO_MIG_FLAG_DEV_CONFIG_STATE (0xef12ULL)
> > >> +#define VFIO_MIG_FLAG_DEV_SETUP_STATE (0xef13ULL)
> > >> +#define VFIO_MIG_FLAG_DEV_DATA_STATE(0xef14ULL)
> > >> +
> > >> static void vfio_migration_region_exit(VFIODevice *vbasedev)
> > >> {
> > >> VFIOMigration *migration = vbasedev->migration;
> > >> @@ -126,6 +140,65 @@ static int vfio_migration_set_state(VFIODevice
> > >> *vbasedev, uint32_t mask,
> > >> return 0;
> > >> }
> > >>
> > >> +/*
> > >> -- */
> > >> +
> > >> +static int vfio_save_setup(QEMUFile *f, void *opaque)
> > >> +{
> > >> +VFIODevice *vbasedev = opaque;
> > >> +VFIOMigration *migration = vbasedev->migration;
> > >> +int ret;
> > >> +
> > >> +trace_vfio_save_setup(vbasedev->name);
> > >> +
> > >> +qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
> > >> +
> > >> +if (migration->region.mmaps) {
> > >> +qemu_mutex_lock_iothread();
> > >> +ret = vfio_region_mmap(>region);
> > >> +qemu_mutex_unlock_iothread();
> > >> +if (ret) {
> > >> +error_report("%s: Failed to mmap VFIO migration region %d:
> > >> %s",
> > >> + vbasedev->name, migration->region.nr,
> > >> + strerror(-ret));
> > >> +return ret;
> > >
> > > OTOH to my previous comments, this shouldn't be fatal, right? mmaps
> > > are optional anyway so it should be sufficient to push an error report
> > > to explain why this might be slower than normal, but we can still
> > > proceed.
> > >
> >
> > Right, defining region to be sparse mmap is optional.
> > migration->region.mmaps is set if vendor driver defines sparse mmapable
> > regions and VFIO_REGION_INFO_FLAG_MMAP flag is set. If this flag is set
> > then error on mmap() should be fatal.
> >
> > If there is not mmapable region, then migration will proceed.
>
> It's both optional for the vendor to define sparse mmap support (or any
> mmap support) and optional for the user to make use of it. The user
> can recover from an mmap failure by using read/write accesses. The
> vendor MUST support this. It doesn't make sense to worry about
> aborting the VM in replying to comments for 05/17, where it's not clear
> how we proceed, yet intentionally cause a fatal error here when there
> is a very clear path to proceed.
>
> > >> +}
> > >> +}
> > >> +
> > >> +ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_MASK,
> > >> + VFIO_DEVICE_STATE_SAVING);
> > >> +if (ret) {
> > >> +error_report("%s: Failed to set state SAVING", vbasedev->name);
> > >> +return ret;
> > >> +}
> > >
> > > We seem to be lacking support in the callers for detecting if the
> > > device is in an error state. I'm not sure
Re: [PATCH QEMU v25 07/17] vfio: Register SaveVMHandlers for VFIO device
On Wed, 24 Jun 2020 00:51:06 +0530
Kirti Wankhede wrote:
> On 6/23/2020 4:20 AM, Alex Williamson wrote:
> > On Sun, 21 Jun 2020 01:51:16 +0530
> > Kirti Wankhede wrote:
> >
> >> Define flags to be used as delimeter in migration file stream.
> >> Added .save_setup and .save_cleanup functions. Mapped & unmapped migration
> >> region from these functions at source during saving or pre-copy phase.
> >> Set VFIO device state depending on VM's state. During live migration, VM is
> >> running when .save_setup is called, _SAVING | _RUNNING state is set for
> >> VFIO
> >> device. During save-restore, VM is paused, _SAVING state is set for VFIO
> >> device.
> >>
> >> Signed-off-by: Kirti Wankhede
> >> Reviewed-by: Neo Jia
> >> ---
> >> hw/vfio/migration.c | 92
> >>
> >> hw/vfio/trace-events | 2 ++
> >> 2 files changed, 94 insertions(+)
> >>
> >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> >> index e30bd8768701..133bb5b1b3b2 100644
> >> --- a/hw/vfio/migration.c
> >> +++ b/hw/vfio/migration.c
> >> @@ -8,12 +8,15 @@
> >>*/
> >>
> >> #include "qemu/osdep.h"
> >> +#include "qemu/main-loop.h"
> >> +#include "qemu/cutils.h"
> >> #include
> >>
> >> #include "sysemu/runstate.h"
> >> #include "hw/vfio/vfio-common.h"
> >> #include "cpu.h"
> >> #include "migration/migration.h"
> >> +#include "migration/vmstate.h"
> >> #include "migration/qemu-file.h"
> >> #include "migration/register.h"
> >> #include "migration/blocker.h"
> >> @@ -24,6 +27,17 @@
> >> #include "pci.h"
> >> #include "trace.h"
> >>
> >> +/*
> >> + * Flags used as delimiter:
> >> + * 0x => MSB 32-bit all 1s
> >> + * 0xef10 => emulated (virtual) function IO
> >> + * 0x => 16-bits reserved for flags
> >> + */
> >> +#define VFIO_MIG_FLAG_END_OF_STATE (0xef11ULL)
> >> +#define VFIO_MIG_FLAG_DEV_CONFIG_STATE (0xef12ULL)
> >> +#define VFIO_MIG_FLAG_DEV_SETUP_STATE (0xef13ULL)
> >> +#define VFIO_MIG_FLAG_DEV_DATA_STATE(0xef14ULL)
> >> +
> >> static void vfio_migration_region_exit(VFIODevice *vbasedev)
> >> {
> >> VFIOMigration *migration = vbasedev->migration;
> >> @@ -126,6 +140,65 @@ static int vfio_migration_set_state(VFIODevice
> >> *vbasedev, uint32_t mask,
> >> return 0;
> >> }
> >>
> >> +/* --
> >> */
> >> +
> >> +static int vfio_save_setup(QEMUFile *f, void *opaque)
> >> +{
> >> +VFIODevice *vbasedev = opaque;
> >> +VFIOMigration *migration = vbasedev->migration;
> >> +int ret;
> >> +
> >> +trace_vfio_save_setup(vbasedev->name);
> >> +
> >> +qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
> >> +
> >> +if (migration->region.mmaps) {
> >> +qemu_mutex_lock_iothread();
> >> +ret = vfio_region_mmap(>region);
> >> +qemu_mutex_unlock_iothread();
> >> +if (ret) {
> >> +error_report("%s: Failed to mmap VFIO migration region %d:
> >> %s",
> >> + vbasedev->name, migration->region.nr,
> >> + strerror(-ret));
> >> +return ret;
> >
> > OTOH to my previous comments, this shouldn't be fatal, right? mmaps
> > are optional anyway so it should be sufficient to push an error report
> > to explain why this might be slower than normal, but we can still
> > proceed.
> >
>
> Right, defining region to be sparse mmap is optional.
> migration->region.mmaps is set if vendor driver defines sparse mmapable
> regions and VFIO_REGION_INFO_FLAG_MMAP flag is set. If this flag is set
> then error on mmap() should be fatal.
>
> If there is not mmapable region, then migration will proceed.
It's both optional for the vendor to define sparse mmap support (or any
mmap support) and optional for the user to make use of it. The user
can recover from an mmap failure by using read/write accesses. The
vendor MUST support this. It doesn't make sense to worry about
aborting the VM in replying to comments for 05/17, where it's not clear
how we proceed, yet intentionally cause a fatal error here when there
is a very clear path to proceed.
> >> +}
> >> +}
> >> +
> >> +ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_MASK,
> >> + VFIO_DEVICE_STATE_SAVING);
> >> +if (ret) {
> >> +error_report("%s: Failed to set state SAVING", vbasedev->name);
> >> +return ret;
> >> +}
> >
> > We seem to be lacking support in the callers for detecting if the
> > device is in an error state. I'm not sure what our options are
> > though, maybe only a hw_error().
> >
>
> Returning error here fails migration process. And if device is in error
> state, any application running inside VM using this device would fail.
> I think, there is no need to take any special action here by detecting
> device error state.
If
Re: [PATCH QEMU v25 07/17] vfio: Register SaveVMHandlers for VFIO device
On 6/23/2020 4:20 AM, Alex Williamson wrote:
On Sun, 21 Jun 2020 01:51:16 +0530
Kirti Wankhede wrote:
Define flags to be used as delimeter in migration file stream.
Added .save_setup and .save_cleanup functions. Mapped & unmapped migration
region from these functions at source during saving or pre-copy phase.
Set VFIO device state depending on VM's state. During live migration, VM is
running when .save_setup is called, _SAVING | _RUNNING state is set for VFIO
device. During save-restore, VM is paused, _SAVING state is set for VFIO device.
Signed-off-by: Kirti Wankhede
Reviewed-by: Neo Jia
---
hw/vfio/migration.c | 92
hw/vfio/trace-events | 2 ++
2 files changed, 94 insertions(+)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index e30bd8768701..133bb5b1b3b2 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -8,12 +8,15 @@
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "qemu/cutils.h"
#include
#include "sysemu/runstate.h"
#include "hw/vfio/vfio-common.h"
#include "cpu.h"
#include "migration/migration.h"
+#include "migration/vmstate.h"
#include "migration/qemu-file.h"
#include "migration/register.h"
#include "migration/blocker.h"
@@ -24,6 +27,17 @@
#include "pci.h"
#include "trace.h"
+/*
+ * Flags used as delimiter:
+ * 0x => MSB 32-bit all 1s
+ * 0xef10 => emulated (virtual) function IO
+ * 0x => 16-bits reserved for flags
+ */
+#define VFIO_MIG_FLAG_END_OF_STATE (0xef11ULL)
+#define VFIO_MIG_FLAG_DEV_CONFIG_STATE (0xef12ULL)
+#define VFIO_MIG_FLAG_DEV_SETUP_STATE (0xef13ULL)
+#define VFIO_MIG_FLAG_DEV_DATA_STATE(0xef14ULL)
+
static void vfio_migration_region_exit(VFIODevice *vbasedev)
{
VFIOMigration *migration = vbasedev->migration;
@@ -126,6 +140,65 @@ static int vfio_migration_set_state(VFIODevice *vbasedev,
uint32_t mask,
return 0;
}
+/* -- */
+
+static int vfio_save_setup(QEMUFile *f, void *opaque)
+{
+VFIODevice *vbasedev = opaque;
+VFIOMigration *migration = vbasedev->migration;
+int ret;
+
+trace_vfio_save_setup(vbasedev->name);
+
+qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
+
+if (migration->region.mmaps) {
+qemu_mutex_lock_iothread();
+ret = vfio_region_mmap(>region);
+qemu_mutex_unlock_iothread();
+if (ret) {
+error_report("%s: Failed to mmap VFIO migration region %d: %s",
+ vbasedev->name, migration->region.nr,
+ strerror(-ret));
+return ret;
OTOH to my previous comments, this shouldn't be fatal, right? mmaps
are optional anyway so it should be sufficient to push an error report
to explain why this might be slower than normal, but we can still
proceed.
Right, defining region to be sparse mmap is optional.
migration->region.mmaps is set if vendor driver defines sparse mmapable
regions and VFIO_REGION_INFO_FLAG_MMAP flag is set. If this flag is set
then error on mmap() should be fatal.
If there is not mmapable region, then migration will proceed.
+}
+}
+
+ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_MASK,
+ VFIO_DEVICE_STATE_SAVING);
+if (ret) {
+error_report("%s: Failed to set state SAVING", vbasedev->name);
+return ret;
+}
We seem to be lacking support in the callers for detecting if the
device is in an error state. I'm not sure what our options are
though, maybe only a hw_error().
Returning error here fails migration process. And if device is in error
state, any application running inside VM using this device would fail.
I think, there is no need to take any special action here by detecting
device error state.
+
+qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+
+ret = qemu_file_get_error(f);
+if (ret) {
+return ret;
+}
+
+return 0;
+}
+
+static void vfio_save_cleanup(void *opaque)
+{
+VFIODevice *vbasedev = opaque;
+VFIOMigration *migration = vbasedev->migration;
+
+if (migration->region.mmaps) {
+vfio_region_unmap(>region);
+}
+trace_vfio_save_cleanup(vbasedev->name);
+}
+
+static SaveVMHandlers savevm_vfio_handlers = {
+.save_setup = vfio_save_setup,
+.save_cleanup = vfio_save_cleanup,
+};
+
+/* -- */
+
static void vfio_vmstate_change(void *opaque, int running, RunState state)
{
VFIODevice *vbasedev = opaque;
@@ -180,6 +253,7 @@ static int vfio_migration_init(VFIODevice *vbasedev,
struct vfio_region_info *info)
{
int ret;
+char id[256] = "";
vbasedev->migration = g_new0(VFIOMigration, 1);
@@ -192,6 +266,24 @@ static int
Re: [PATCH QEMU v25 07/17] vfio: Register SaveVMHandlers for VFIO device
On Sun, 21 Jun 2020 01:51:16 +0530
Kirti Wankhede wrote:
> Define flags to be used as delimeter in migration file stream.
> Added .save_setup and .save_cleanup functions. Mapped & unmapped migration
> region from these functions at source during saving or pre-copy phase.
> Set VFIO device state depending on VM's state. During live migration, VM is
> running when .save_setup is called, _SAVING | _RUNNING state is set for VFIO
> device. During save-restore, VM is paused, _SAVING state is set for VFIO
> device.
>
> Signed-off-by: Kirti Wankhede
> Reviewed-by: Neo Jia
> ---
> hw/vfio/migration.c | 92
>
> hw/vfio/trace-events | 2 ++
> 2 files changed, 94 insertions(+)
>
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index e30bd8768701..133bb5b1b3b2 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -8,12 +8,15 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/cutils.h"
> #include
>
> #include "sysemu/runstate.h"
> #include "hw/vfio/vfio-common.h"
> #include "cpu.h"
> #include "migration/migration.h"
> +#include "migration/vmstate.h"
> #include "migration/qemu-file.h"
> #include "migration/register.h"
> #include "migration/blocker.h"
> @@ -24,6 +27,17 @@
> #include "pci.h"
> #include "trace.h"
>
> +/*
> + * Flags used as delimiter:
> + * 0x => MSB 32-bit all 1s
> + * 0xef10 => emulated (virtual) function IO
> + * 0x => 16-bits reserved for flags
> + */
> +#define VFIO_MIG_FLAG_END_OF_STATE (0xef11ULL)
> +#define VFIO_MIG_FLAG_DEV_CONFIG_STATE (0xef12ULL)
> +#define VFIO_MIG_FLAG_DEV_SETUP_STATE (0xef13ULL)
> +#define VFIO_MIG_FLAG_DEV_DATA_STATE(0xef14ULL)
> +
> static void vfio_migration_region_exit(VFIODevice *vbasedev)
> {
> VFIOMigration *migration = vbasedev->migration;
> @@ -126,6 +140,65 @@ static int vfio_migration_set_state(VFIODevice
> *vbasedev, uint32_t mask,
> return 0;
> }
>
> +/* -- */
> +
> +static int vfio_save_setup(QEMUFile *f, void *opaque)
> +{
> +VFIODevice *vbasedev = opaque;
> +VFIOMigration *migration = vbasedev->migration;
> +int ret;
> +
> +trace_vfio_save_setup(vbasedev->name);
> +
> +qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
> +
> +if (migration->region.mmaps) {
> +qemu_mutex_lock_iothread();
> +ret = vfio_region_mmap(>region);
> +qemu_mutex_unlock_iothread();
> +if (ret) {
> +error_report("%s: Failed to mmap VFIO migration region %d: %s",
> + vbasedev->name, migration->region.nr,
> + strerror(-ret));
> +return ret;
OTOH to my previous comments, this shouldn't be fatal, right? mmaps
are optional anyway so it should be sufficient to push an error report
to explain why this might be slower than normal, but we can still
proceed.
> +}
> +}
> +
> +ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_MASK,
> + VFIO_DEVICE_STATE_SAVING);
> +if (ret) {
> +error_report("%s: Failed to set state SAVING", vbasedev->name);
> +return ret;
> +}
We seem to be lacking support in the callers for detecting if the
device is in an error state. I'm not sure what our options are
though, maybe only a hw_error().
> +
> +qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
> +
> +ret = qemu_file_get_error(f);
> +if (ret) {
> +return ret;
> +}
> +
> +return 0;
> +}
> +
> +static void vfio_save_cleanup(void *opaque)
> +{
> +VFIODevice *vbasedev = opaque;
> +VFIOMigration *migration = vbasedev->migration;
> +
> +if (migration->region.mmaps) {
> +vfio_region_unmap(>region);
> +}
> +trace_vfio_save_cleanup(vbasedev->name);
> +}
> +
> +static SaveVMHandlers savevm_vfio_handlers = {
> +.save_setup = vfio_save_setup,
> +.save_cleanup = vfio_save_cleanup,
> +};
> +
> +/* -- */
> +
> static void vfio_vmstate_change(void *opaque, int running, RunState state)
> {
> VFIODevice *vbasedev = opaque;
> @@ -180,6 +253,7 @@ static int vfio_migration_init(VFIODevice *vbasedev,
> struct vfio_region_info *info)
> {
> int ret;
> +char id[256] = "";
>
> vbasedev->migration = g_new0(VFIOMigration, 1);
>
> @@ -192,6 +266,24 @@ static int vfio_migration_init(VFIODevice *vbasedev,
> return ret;
> }
>
> +if (vbasedev->ops->vfio_get_object) {
Nit, vfio_migration_region_init() would have failed already if this were
not available. Perhaps do the test once at the start of this function
instead? Thanks,
Alex
> +Object *obj = vbasedev->ops->vfio_get_object(vbasedev);
> +
> +if (obj) {
> +
