diff mbox series

[v2,3/5] migration: Add .save_prepare() handler to struct SaveVMHandlers

Message ID 20230831125702.11263-4-avihaih@nvidia.com (mailing list archive)
State New, archived
Headers show
Series vfio/migration: Block VFIO migration with postcopy and background snapshot | expand

Commit Message

Avihai Horon Aug. 31, 2023, 12:57 p.m. UTC
Add a new .save_prepare() handler to struct SaveVMHandlers. This handler
is called early, even before migration starts, and can be used by
devices to perform early checks.

Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 include/migration/register.h |  5 +++++
 migration/savevm.h           |  1 +
 migration/migration.c        |  4 ++++
 migration/savevm.c           | 29 +++++++++++++++++++++++++++++
 4 files changed, 39 insertions(+)

Comments

Peter Xu Sept. 1, 2023, 3:49 p.m. UTC | #1
On Thu, Aug 31, 2023 at 03:57:00PM +0300, Avihai Horon wrote:
> Add a new .save_prepare() handler to struct SaveVMHandlers. This handler
> is called early, even before migration starts, and can be used by
> devices to perform early checks.
> 
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>

Shouldn't be hard to unify the two call sites for qmp migrate and save
snapshot, but we can leave that for later:

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,
Cédric Le Goater Sept. 5, 2023, 4:13 p.m. UTC | #2
On 9/1/23 17:49, Peter Xu wrote:
> On Thu, Aug 31, 2023 at 03:57:00PM +0300, Avihai Horon wrote:
>> Add a new .save_prepare() handler to struct SaveVMHandlers. This handler
>> is called early, even before migration starts, and can be used by
>> devices to perform early checks.
>>
>> Suggested-by: Peter Xu <peterx@redhat.com>
>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> 
> Shouldn't be hard to unify the two call sites for qmp migrate and save
> snapshot, but we can leave that for later:

yes. It could be called from migrate_init() with minor changes.

We could probably move :

     memset(&mig_stats, 0, sizeof(mig_stats));
     memset(&compression_counters, 0, sizeof(compression_counters));
     migration_reset_vfio_bytes_transferred();

under migrate_init() also. Anyhow,

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.
Avihai Horon Sept. 6, 2023, 2:11 p.m. UTC | #3
On 05/09/2023 19:13, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> On 9/1/23 17:49, Peter Xu wrote:
>> On Thu, Aug 31, 2023 at 03:57:00PM +0300, Avihai Horon wrote:
>>> Add a new .save_prepare() handler to struct SaveVMHandlers. This 
>>> handler
>>> is called early, even before migration starts, and can be used by
>>> devices to perform early checks.
>>>
>>> Suggested-by: Peter Xu <peterx@redhat.com>
>>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>>
>> Shouldn't be hard to unify the two call sites for qmp migrate and save
>> snapshot, but we can leave that for later:
>
> yes. It could be called from migrate_init() with minor changes.
>
> We could probably move :
>
>     memset(&mig_stats, 0, sizeof(mig_stats));
>     memset(&compression_counters, 0, sizeof(compression_counters));
>     migration_reset_vfio_bytes_transferred();
>
> under migrate_init() also.

I will send v3 with the above changes shortly.

Thanks.

> Anyhow,
>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>
> Thanks,
>
> C.
>
Cédric Le Goater Sept. 6, 2023, 2:21 p.m. UTC | #4
On 9/6/23 16:11, Avihai Horon wrote:
> 
> On 05/09/2023 19:13, Cédric Le Goater wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 9/1/23 17:49, Peter Xu wrote:
>>> On Thu, Aug 31, 2023 at 03:57:00PM +0300, Avihai Horon wrote:
>>>> Add a new .save_prepare() handler to struct SaveVMHandlers. This handler
>>>> is called early, even before migration starts, and can be used by
>>>> devices to perform early checks.
>>>>
>>>> Suggested-by: Peter Xu <peterx@redhat.com>
>>>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>>>
>>> Shouldn't be hard to unify the two call sites for qmp migrate and save
>>> snapshot, but we can leave that for later:
>>
>> yes. It could be called from migrate_init() with minor changes.
>>
>> We could probably move :
>>
>>     memset(&mig_stats, 0, sizeof(mig_stats));
>>     memset(&compression_counters, 0, sizeof(compression_counters));
>>     migration_reset_vfio_bytes_transferred();
>>
>> under migrate_init() also.
> 
> I will send v3 with the above changes shortly.

Good timing. I was going to start working on downstream.

Thanks !

C.
diff mbox series

Patch

diff --git a/include/migration/register.h b/include/migration/register.h
index 90914f32f5..2b12c6adec 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -20,6 +20,11 @@  typedef struct SaveVMHandlers {
     /* This runs inside the iothread lock.  */
     SaveStateHandler *save_state;
 
+    /*
+     * save_prepare is called early, even before migration starts, and can be
+     * used to perform early checks.
+     */
+    int (*save_prepare)(void *opaque, Error **errp);
     void (*save_cleanup)(void *opaque);
     int (*save_live_complete_postcopy)(QEMUFile *f, void *opaque);
     int (*save_live_complete_precopy)(QEMUFile *f, void *opaque);
diff --git a/migration/savevm.h b/migration/savevm.h
index e894bbc143..74669733dd 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -31,6 +31,7 @@ 
 
 bool qemu_savevm_state_blocked(Error **errp);
 void qemu_savevm_non_migratable_list(strList **reasons);
+int qemu_savevm_state_prepare(Error **errp);
 void qemu_savevm_state_setup(QEMUFile *f);
 bool qemu_savevm_state_guest_unplug_pending(void);
 int qemu_savevm_state_resume_prepare(MigrationState *s);
diff --git a/migration/migration.c b/migration/migration.c
index 92866a8f49..914783f07b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1634,6 +1634,10 @@  static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
         migrate_set_block_incremental(true);
     }
 
+    if (qemu_savevm_state_prepare(errp)) {
+        return false;
+    }
+
     migrate_init(s);
     /*
      * set mig_stats compression_counters memory to zero for a
diff --git a/migration/savevm.c b/migration/savevm.c
index 5bf8b59a7d..818510ec57 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1233,6 +1233,30 @@  bool qemu_savevm_state_guest_unplug_pending(void)
     return false;
 }
 
+int qemu_savevm_state_prepare(Error **errp)
+{
+    SaveStateEntry *se;
+    int ret;
+
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+        if (!se->ops || !se->ops->save_prepare) {
+            continue;
+        }
+        if (se->ops->is_active) {
+            if (!se->ops->is_active(se->opaque)) {
+                continue;
+            }
+        }
+
+        ret = se->ops->save_prepare(se->opaque, errp);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    return 0;
+}
+
 void qemu_savevm_state_setup(QEMUFile *f)
 {
     MigrationState *ms = migrate_get_current();
@@ -1619,6 +1643,11 @@  static int qemu_savevm_state(QEMUFile *f, Error **errp)
         return -EINVAL;
     }
 
+    ret = qemu_savevm_state_prepare(errp);
+    if (ret) {
+        return ret;
+    }
+
     migrate_init(ms);
     memset(&mig_stats, 0, sizeof(mig_stats));
     memset(&compression_counters, 0, sizeof(compression_counters));