Message ID | 1451372975-5048-32-git-send-email-zhang.zhanghailiang@huawei.com |
---|---|
State | New |
Headers | show |
* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: > For COLO's checkpoint process, we will do savevm/loadvm repeatedly. > So every time we call qemu_loadvm_section_start_full(), we will > add all sections information into loadvm_handlers list for one time. > There will be many instances in loadvm_handlers for one section, > and this will lead to memory leak. > > We need to check if we have the section info in loadvm_handlers list > before save it. For normal migration, it is harmless. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Dave > --- > v13: > - New patch > --- > migration/savevm.c | 56 ++++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 40 insertions(+), 16 deletions(-) > > diff --git a/migration/savevm.c b/migration/savevm.c > index dcfb473..7ceec2d 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -1710,6 +1710,37 @@ void loadvm_free_handlers(MigrationIncomingState *mis) > } > } > > +static LoadStateEntry *loadvm_save_section_entry(MigrationIncomingState *mis, > + SaveStateEntry *se, > + uint32_t section_id, > + uint32_t version_id) > +{ > + LoadStateEntry *le; > + > + /* Add entry */ > + le = g_malloc0(sizeof(*le)); > + > + le->se = se; > + le->section_id = section_id; > + le->version_id = version_id; > + QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry); > + return le; > +} > + > +static LoadStateEntry *loadvm_find_section_entry(MigrationIncomingState *mis, > + uint32_t section_id) > +{ > + LoadStateEntry *le; > + > + QLIST_FOREACH(le, &mis->loadvm_handlers, entry) { > + if (le->section_id == section_id) { > + break; > + } > + } > + > + return le; > +} > + > static int > qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis) > { > @@ -1745,16 +1776,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis) > version_id, idstr, se->version_id); > return -EINVAL; > } > - > - /* Add entry */ > - le = g_malloc0(sizeof(*le)); > - > - le->se = se; > - le->section_id = section_id; > - le->version_id = version_id; > - QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry); > - > - ret = vmstate_load(f, le->se, le->version_id); > + /* Check if we have saved this section info before, if not, save it */ > + le = loadvm_find_section_entry(mis, section_id); > + if (!le) { > + le = loadvm_save_section_entry(mis, se, section_id, version_id); > + } > + ret = vmstate_load(f, se, version_id); > if (ret < 0) { > error_report("error while loading state for instance 0x%x of" > " device '%s'", instance_id, idstr); > @@ -1777,12 +1804,9 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis) > section_id = qemu_get_be32(f); > > trace_qemu_loadvm_state_section_partend(section_id); > - QLIST_FOREACH(le, &mis->loadvm_handlers, entry) { > - if (le->section_id == section_id) { > - break; > - } > - } > - if (le == NULL) { > + > + le = loadvm_find_section_entry(mis, section_id); > + if (!le) { > error_report("Unknown savevm section %d", section_id); > return -EINVAL; > } > -- > 1.8.3.1 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/migration/savevm.c b/migration/savevm.c index dcfb473..7ceec2d 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1710,6 +1710,37 @@ void loadvm_free_handlers(MigrationIncomingState *mis) } } +static LoadStateEntry *loadvm_save_section_entry(MigrationIncomingState *mis, + SaveStateEntry *se, + uint32_t section_id, + uint32_t version_id) +{ + LoadStateEntry *le; + + /* Add entry */ + le = g_malloc0(sizeof(*le)); + + le->se = se; + le->section_id = section_id; + le->version_id = version_id; + QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry); + return le; +} + +static LoadStateEntry *loadvm_find_section_entry(MigrationIncomingState *mis, + uint32_t section_id) +{ + LoadStateEntry *le; + + QLIST_FOREACH(le, &mis->loadvm_handlers, entry) { + if (le->section_id == section_id) { + break; + } + } + + return le; +} + static int qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis) { @@ -1745,16 +1776,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis) version_id, idstr, se->version_id); return -EINVAL; } - - /* Add entry */ - le = g_malloc0(sizeof(*le)); - - le->se = se; - le->section_id = section_id; - le->version_id = version_id; - QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry); - - ret = vmstate_load(f, le->se, le->version_id); + /* Check if we have saved this section info before, if not, save it */ + le = loadvm_find_section_entry(mis, section_id); + if (!le) { + le = loadvm_save_section_entry(mis, se, section_id, version_id); + } + ret = vmstate_load(f, se, version_id); if (ret < 0) { error_report("error while loading state for instance 0x%x of" " device '%s'", instance_id, idstr); @@ -1777,12 +1804,9 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis) section_id = qemu_get_be32(f); trace_qemu_loadvm_state_section_partend(section_id); - QLIST_FOREACH(le, &mis->loadvm_handlers, entry) { - if (le->section_id == section_id) { - break; - } - } - if (le == NULL) { + + le = loadvm_find_section_entry(mis, section_id); + if (!le) { error_report("Unknown savevm section %d", section_id); return -EINVAL; }
For COLO's checkpoint process, we will do savevm/loadvm repeatedly. So every time we call qemu_loadvm_section_start_full(), we will add all sections information into loadvm_handlers list for one time. There will be many instances in loadvm_handlers for one section, and this will lead to memory leak. We need to check if we have the section info in loadvm_handlers list before save it. For normal migration, it is harmless. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- v13: - New patch --- migration/savevm.c | 56 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 16 deletions(-)