On Mon, 6 Feb 2023 14:31:33 +0200
Avihai Horon <avih...@nvidia.com> wrote:
> @@ -523,6 +745,41 @@ static int vfio_save_iterate(QEMUFile *f, void *opaque)
> return 0;
> }
>
> +static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
> +{
> + VFIODevice *vbasedev = opaque;
> + enum vfio_device_mig_state recover_state;
> + int ret;
> +
> + /* We reach here with device state STOP only */
> + recover_state = VFIO_DEVICE_STATE_STOP;
Why do we need to put this in a local variable?
> + ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP_COPY,
> + recover_state);
> + if (ret) {
> + return ret;
> + }
> +
> + do {
> + ret = vfio_save_block(f, vbasedev->migration);
> + if (ret < 0) {
> + return ret;
> + }
> + } while (!ret);
> +
> + qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
> + ret = qemu_file_get_error(f);
> + if (ret) {
> + return ret;
> + }
> +
> + recover_state = VFIO_DEVICE_STATE_ERROR;
IIRC, the ERROR state is not reachable as a user directed state. I
suppose passing it as the recovery state guarantees a device reset when
it fails, but if that's the intention it should be documented with a
comment to explain so (and vfio_migration_set_state() should not bother
trying to use it as a recovery state).
> + ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP,
> + recover_state);
> + trace_vfio_save_complete_precopy(vbasedev->name, ret);
> +
> + return ret;
> +}
> +
> static int vfio_v1_save_complete_precopy(QEMUFile *f, void *opaque)
> {
> VFIODevice *vbasedev = opaque;
...
> @@ -769,12 +1087,17 @@ static void vfio_migration_state_notifier(Notifier
> *notifier, void *data)
> case MIGRATION_STATUS_CANCELLED:
> case MIGRATION_STATUS_FAILED:
> bytes_transferred = 0;
> - ret = vfio_migration_v1_set_state(vbasedev,
> - ~(VFIO_DEVICE_STATE_V1_SAVING |
> - VFIO_DEVICE_STATE_V1_RESUMING),
> - VFIO_DEVICE_STATE_V1_RUNNING);
> - if (ret) {
> - error_report("%s: Failed to set state RUNNING", vbasedev->name);
> + if (migration->v2) {
> + vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_RUNNING,
> + VFIO_DEVICE_STATE_ERROR);
Same here. Thanks,
Alex
> + } else {
> + ret = vfio_migration_v1_set_state(vbasedev,
> + ~(VFIO_DEVICE_STATE_V1_SAVING |
> +
> VFIO_DEVICE_STATE_V1_RESUMING),
> + VFIO_DEVICE_STATE_V1_RUNNING);
> + if (ret) {
> + error_report("%s: Failed to set state RUNNING",
> vbasedev->name);
> + }
> }
> }
> }
