Peter Xu <pet...@redhat.com> wrote:
> On Wed, May 24, 2017 at 09:37:19AM +0200, Juan Quintela wrote:
>> Migration non save_live handlers have an ops member that is
>> dinamically allocated by migration code. Save_live handlers have it
>> passed as argument and are responsability of the caller. Add a new
>> member is_allocated that remembers if ops has to be freed. This
>> allows unregister_savevm() to work with save_live handlers.
>>
>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>> ---
>> include/migration/vmstate.h | 2 ++
>> migration/savevm.c | 5 ++++-
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> 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) {
>
> Would it be good to check against (se->ops && se->ops->is_allocated)?
> Since I see that devices registered via
> vmstate_register_with_alias_id() won't have this se->ops. I just don't
> know whether that case will be allowed to be unregistered with current
> function.
good point. I thought that not having ->ops was wrong, but I had
completely forgot about the alias case. Fixing it.
Thanks, Juan.
