| Message ID | 20170524112152.23226-1-quintela@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 24, 2017 at 01:21:52PM +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 <quintela@redhat.com> I thought Laurent will prepare one patch that will directly remove existing register_savevm() callers, no? > > --- > > check that se->ops is not NULL (thanks peterx) > --- > 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..569add3 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 && se->ops->is_allocated) { > + g_free(se->ops); > + } > g_free(se); > } > } > -- > 2.9.3 >
Peter Xu <peterx@redhat.com> wrote: > On Wed, May 24, 2017 at 01:21:52PM +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 <quintela@redhat.com> > > I thought Laurent will prepare one patch that will directly remove > existing register_savevm() callers, no? s390, vmxnet3 and slirp are using it. I am not sure that we will get that ones "changed" so easily. Later, Juan.
* Juan Quintela (quintela@redhat.com) wrote: > Peter Xu <peterx@redhat.com> wrote: > > On Wed, May 24, 2017 at 01:21:52PM +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 <quintela@redhat.com> > > > > I thought Laurent will prepare one patch that will directly remove > > existing register_savevm() callers, no? > > s390, vmxnet3 and slirp are using it. I am not sure that we will get > that ones "changed" so easily. I think Halil is working on s390, slirp has a tricky loop I need to look at again. And vmxnet3 is...odd. Dave > Later, Juan. -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
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..569add3 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 && se->ops->is_allocated) { + g_free(se->ops); + } g_free(se); } }
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 <quintela@redhat.com> --- check that se->ops is not NULL (thanks peterx) --- include/migration/vmstate.h | 2 ++ migration/savevm.c | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-)