On 08/02/2023 1:42, Alex Williamson wrote:
External email: Use caution opening links or attachments
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?
No need. I will remove the 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).
Right, that's the intention.
I will add a comment and adjust vfio_migration_set_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.
Will change.
Thanks.