diff mbox series

[v3,3/6] migration: Factor out checks for advised and listening incomming postcopy

Message ID 20221222110215.130392-4-david@redhat.com (mailing list archive)
State New, archived
Headers show
Series virtio-mem: Handle preallocation with migration | expand

Commit Message

David Hildenbrand Dec. 22, 2022, 11:02 a.m. UTC 
Let's factor out both checks, to be used in virtio-mem context next.

While at it, fix a spelling error in a related comment.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 include/migration/misc.h |  6 +++++-
 migration/migration.c    | 14 ++++++++++++++
 migration/ram.c          | 16 ++--------------
 3 files changed, 21 insertions(+), 15 deletions(-)

Comments

Peter Xu Jan. 5, 2023, 5:18 p.m. UTC | #1 
On Thu, Dec 22, 2022 at 12:02:12PM +0100, David Hildenbrand wrote:
> +bool migration_incoming_postcopy_listening(void)
> +{
> +    PostcopyState ps = postcopy_state_get();
> +
> +    return ps >= POSTCOPY_INCOMING_LISTENING && ps < POSTCOPY_INCOMING_END;
> +}

This name is misleading, IMHO.

The code means "we passed listening phase" but the name implies "we're
listening".  We can add the "incoming" into that if we want, though.
David Hildenbrand Jan. 9, 2023, 2:39 p.m. UTC | #2 
On 05.01.23 18:18, Peter Xu wrote:
> On Thu, Dec 22, 2022 at 12:02:12PM +0100, David Hildenbrand wrote:
>> +bool migration_incoming_postcopy_listening(void)
>> +{
>> +    PostcopyState ps = postcopy_state_get();
>> +
>> +    return ps >= POSTCOPY_INCOMING_LISTENING && ps < POSTCOPY_INCOMING_END;
>> +}
> 
> This name is misleading, IMHO.
> 
> The code means "we passed listening phase" but the name implies "we're
> listening".  We can add the "incoming" into that if we want, though.
> 

Let me call that migration_incoming_postcopy_running(). Thanks!
David Hildenbrand Jan. 9, 2023, 2:42 p.m. UTC | #3 
On 09.01.23 15:39, David Hildenbrand wrote:
> On 05.01.23 18:18, Peter Xu wrote:
>> On Thu, Dec 22, 2022 at 12:02:12PM +0100, David Hildenbrand wrote:
>>> +bool migration_incoming_postcopy_listening(void)
>>> +{
>>> +    PostcopyState ps = postcopy_state_get();
>>> +
>>> +    return ps >= POSTCOPY_INCOMING_LISTENING && ps < POSTCOPY_INCOMING_END;
>>> +}
>>
>> This name is misleading, IMHO.
>>
>> The code means "we passed listening phase" but the name implies "we're
>> listening".  We can add the "incoming" into that if we want, though.
>>
> 
> Let me call that migration_incoming_postcopy_running(). Thanks!
> 

... which is also misleading. Let me just drop the sanity check and this 
function.
diff mbox series

Patch

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 465906710d..c7e67a6804 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -67,8 +67,12 @@  bool migration_has_failed(MigrationState *);
 /* ...and after the device transmission */
 bool migration_in_postcopy_after_devices(MigrationState *);
 void migration_global_dump(Monitor *mon);
-/* True if incomming migration entered POSTCOPY_INCOMING_DISCARD */
+/* True if incoming migration entered POSTCOPY_INCOMING_DISCARD */
 bool migration_in_incoming_postcopy(void);
+/* True if incoming migration entered POSTCOPY_INCOMING_ADVISE */
+bool migration_incoming_postcopy_advised(void);
+/* True if incoming migration entered POSTCOPY_INCOMING_LISTENING */
+bool migration_incoming_postcopy_listening(void);
 /* True if background snapshot is active */
 bool migration_in_bg_snapshot(void);
 
diff --git a/migration/migration.c b/migration/migration.c
index 78b6bb8765..7a69bb93b0 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2094,6 +2094,20 @@  bool migration_in_incoming_postcopy(void)
     return ps >= POSTCOPY_INCOMING_DISCARD && ps < POSTCOPY_INCOMING_END;
 }
 
+bool migration_incoming_postcopy_advised(void)
+{
+    PostcopyState ps = postcopy_state_get();
+
+    return ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_END;
+}
+
+bool migration_incoming_postcopy_listening(void)
+{
+    PostcopyState ps = postcopy_state_get();
+
+    return ps >= POSTCOPY_INCOMING_LISTENING && ps < POSTCOPY_INCOMING_END;
+}
+
 bool migration_in_bg_snapshot(void)
 {
     MigrationState *s = migrate_get_current();
diff --git a/migration/ram.c b/migration/ram.c
index 334309f1c6..44b063eccd 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4091,18 +4091,6 @@  int ram_load_postcopy(QEMUFile *f, int channel)
     return ret;
 }
 
-static bool postcopy_is_advised(void)
-{
-    PostcopyState ps = postcopy_state_get();
-    return ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_END;
-}
-
-static bool postcopy_is_running(void)
-{
-    PostcopyState ps = postcopy_state_get();
-    return ps >= POSTCOPY_INCOMING_LISTENING && ps < POSTCOPY_INCOMING_END;
-}
-
 /*
  * Flush content of RAM cache into SVM's memory.
  * Only flush the pages that be dirtied by PVM or SVM or both.
@@ -4167,7 +4155,7 @@  static int ram_load_precopy(QEMUFile *f)
     MigrationIncomingState *mis = migration_incoming_get_current();
     int flags = 0, ret = 0, invalid_flags = 0, len = 0, i = 0;
     /* ADVISE is earlier, it shows the source has the postcopy capability on */
-    bool postcopy_advised = postcopy_is_advised();
+    bool postcopy_advised = migration_incoming_postcopy_advised();
     if (!migrate_use_compression()) {
         invalid_flags |= RAM_SAVE_FLAG_COMPRESS_PAGE;
     }
@@ -4365,7 +4353,7 @@  static int ram_load(QEMUFile *f, void *opaque, int version_id)
      * If system is running in postcopy mode, page inserts to host memory must
      * be atomic
      */
-    bool postcopy_running = postcopy_is_running();
+    bool postcopy_running = migration_incoming_postcopy_listening();
 
     seq_iter++;