Re: [Qemu-devel] [PATCH v3 1/3] migration: Introduce unregister_savevm_live()

[Juan Quintela <quintela@redhat.com>]

Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> Introduce a new function unregister_savevm_live() to unregister the vmstate
> handlers registered via register_savevm_live().
>
> register_savevm() allocates SaveVMHandlers while register_savevm_live()
> gets passed with SaveVMHandlers. During unregistration, we  want to
> free SaveVMHandlers in the former case but not free in the latter case.
> Hence this new API is needed to differentiate this.
>
> This new API will be needed by PowerPC to unregister the HTAB savevm
> handlers.
>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>

Hi

How about this one?
I just test compiled it.

Advantage from my point of view is that we always do the right thing.
And as migration code already knows if it has to be freed or not, I
think it is a better API.

What do you think?

Later, Juan.

commit 6e71fac7a9c29cef9625135c58ce59ccfbb3b86f
Author: Juan Quintela <quintela@redhat.com>
Date:   Wed May 24 08:25:06 2017 +0200

    migration: Allow to free save_live handlers
    
    Migration save_live handlers have an ops member that is not dynamic.
    Add a new member is_allocated that remembers if ops has to be freed.
    
    Signed-off-by: Juan Quintela <quintela@redhat.com>

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index f97411d..1d20e30 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -57,6 +57,8 @@ typedef struct SaveVMHandlers {
                               uint64_t *non_postcopiable_pending,
                               uint64_t *postcopiable_pending);
     LoadStateHandler *load_state;
+    /* Has been allocated by migratation code */
+    bool is_allocated;
 } SaveVMHandlers;
 
 int register_savevm(DeviceState *dev,
diff --git a/migration/savevm.c b/migration/savevm.c
index d971e5e..187f386 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -628,6 +628,7 @@ int register_savevm(DeviceState *dev,
     SaveVMHandlers *ops = g_new0(SaveVMHandlers, 1);
     ops->save_state = save_state;
     ops->load_state = load_state;
+    ops->is_allocated = true;
     return register_savevm_live(dev, idstr, instance_id, version_id,
                                 ops, opaque);
 }
@@ -651,7 +652,9 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
         if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
             QTAILQ_REMOVE(&savevm_state.handlers, se, entry);
             g_free(se->compat);
-            g_free(se->ops);
+            if (se->ops->is_allocated) {
+                g_free(se->ops);
+            }
             g_free(se);
         }
     }