[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 14/45] Move loadvm_handlers into MigrationInc
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v5 14/45] Move loadvm_handlers into MigrationIncomingState |
Date: |
Tue, 10 Mar 2015 10:12:14 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* David Gibson (address@hidden) wrote:
> On Wed, Feb 25, 2015 at 04:51:37PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > In postcopy we need the loadvm_handlers to be used in a couple
> > of different instances of the loadvm loop/routine, and thus
> > it can't be local any more.
> >
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> > include/migration/migration.h | 5 +++++
> > include/migration/vmstate.h | 2 ++
> > include/qemu/typedefs.h | 1 +
> > migration/migration.c | 2 ++
> > savevm.c | 28 ++++++++++++++++------------
> > 5 files changed, 26 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 3776e86..751caa0 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -50,10 +50,15 @@ enum mig_rpcomm_cmd {
> >
> > typedef struct MigrationState MigrationState;
> >
> > +typedef QLIST_HEAD(, LoadStateEntry) LoadStateEntry_Head;
> > +
> > /* State for the incoming migration */
> > struct MigrationIncomingState {
> > QEMUFile *file;
> >
> > + /* See savevm.c */
> > + LoadStateEntry_Head loadvm_handlers;
> > +
> > QEMUFile *return_path;
> > QemuMutex rp_mutex; /* We send replies from multiple threads */
> > };
> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> > index c20f2d1..18da207 100644
> > --- a/include/migration/vmstate.h
> > +++ b/include/migration/vmstate.h
> > @@ -797,6 +797,8 @@ extern const VMStateInfo vmstate_info_bitmap;
> >
> > #define SELF_ANNOUNCE_ROUNDS 5
> >
> > +void loadvm_free_handlers(MigrationIncomingState *mis);
> > +
> > int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
> > void *opaque, int version_id);
> > void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> > index 74dfad3..6fdcbcd 100644
> > --- a/include/qemu/typedefs.h
> > +++ b/include/qemu/typedefs.h
> > @@ -31,6 +31,7 @@ typedef struct I2CBus I2CBus;
> > typedef struct I2SCodec I2SCodec;
> > typedef struct ISABus ISABus;
> > typedef struct ISADevice ISADevice;
> > +typedef struct LoadStateEntry LoadStateEntry;
> > typedef struct MACAddr MACAddr;
> > typedef struct MachineClass MachineClass;
> > typedef struct MachineState MachineState;
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 34cd4fe..4592060 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -78,6 +78,7 @@ MigrationIncomingState
> > *migration_incoming_state_new(QEMUFile* f)
> > {
> > mis_current = g_malloc0(sizeof(MigrationIncomingState));
> > mis_current->file = f;
> > + QLIST_INIT(&mis_current->loadvm_handlers);
> > qemu_mutex_init(&mis_current->rp_mutex);
> >
> > return mis_current;
> > @@ -85,6 +86,7 @@ MigrationIncomingState
> > *migration_incoming_state_new(QEMUFile* f)
> >
> > void migration_incoming_state_destroy(void)
> > {
> > + loadvm_free_handlers(mis_current);
>
> AFAICT this is the only caler of loadvm_free_handlers(), so why not
> just open-code it here?
I was keeping this handler list as owned by savevm.c; all it's allocation
and it's use are done in savevm.c, so it would seem odd to open code
freeing the list in a separate file.
> > g_free(mis_current);
> > mis_current = NULL;
> > }
> > diff --git a/savevm.c b/savevm.c
> > index 7084d07..f42713d 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -1019,18 +1019,26 @@ static int loadvm_process_command(QEMUFile *f)
> > return 0;
> > }
> >
> > -typedef struct LoadStateEntry {
> > +struct LoadStateEntry {
>
> Why remove the typedef?
Because it is now typedef'd in typedefs.h, and older gcc (RHEL6)
object to two typedefs even if they're of the same thing.
Dave
> > QLIST_ENTRY(LoadStateEntry) entry;
> > SaveStateEntry *se;
> > int section_id;
> > int version_id;
> > -} LoadStateEntry;
> > +};
> >
> > -int qemu_loadvm_state(QEMUFile *f)
> > +void loadvm_free_handlers(MigrationIncomingState *mis)
> > {
> > - QLIST_HEAD(, LoadStateEntry) loadvm_handlers =
> > - QLIST_HEAD_INITIALIZER(loadvm_handlers);
> > LoadStateEntry *le, *new_le;
> > +
> > + QLIST_FOREACH_SAFE(le, &mis->loadvm_handlers, entry, new_le) {
> > + QLIST_REMOVE(le, entry);
> > + g_free(le);
> > + }
> > +}
> > +
> > +int qemu_loadvm_state(QEMUFile *f)
> > +{
> > + MigrationIncomingState *mis = migration_incoming_get_current();
> > Error *local_err = NULL;
> > uint8_t section_type;
> > unsigned int v;
> > @@ -1061,6 +1069,7 @@ int qemu_loadvm_state(QEMUFile *f)
> > while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
> > uint32_t instance_id, version_id, section_id;
> > SaveStateEntry *se;
> > + LoadStateEntry *le;
> > char idstr[256];
> >
> > trace_qemu_loadvm_state_section(section_type);
> > @@ -1102,7 +1111,7 @@ int qemu_loadvm_state(QEMUFile *f)
> > le->se = se;
> > le->section_id = section_id;
> > le->version_id = version_id;
> > - QLIST_INSERT_HEAD(&loadvm_handlers, le, entry);
> > + QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
> >
> > ret = vmstate_load(f, le->se, le->version_id);
> > if (ret < 0) {
> > @@ -1116,7 +1125,7 @@ int qemu_loadvm_state(QEMUFile *f)
> > section_id = qemu_get_be32(f);
> >
> > trace_qemu_loadvm_state_section_partend(section_id);
> > - QLIST_FOREACH(le, &loadvm_handlers, entry) {
> > + QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
> > if (le->section_id == section_id) {
> > break;
> > }
> > @@ -1152,11 +1161,6 @@ int qemu_loadvm_state(QEMUFile *f)
> > ret = 0;
> >
> > out:
> > - QLIST_FOREACH_SAFE(le, &loadvm_handlers, entry, new_le) {
> > - QLIST_REMOVE(le, entry);
> > - g_free(le);
> > - }
> > -
> > if (ret == 0) {
> > ret = qemu_file_get_error(f);
> > }
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_
> _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK