From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33621) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z52K6-0000up-N8 for qemu-devel@nongnu.org; Tue, 16 Jun 2015 21:39:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z52K2-0007lM-VW for qemu-devel@nongnu.org; Tue, 16 Jun 2015 21:39:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53826) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z52K2-0007ke-OS for qemu-devel@nongnu.org; Tue, 16 Jun 2015 21:39:22 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 0628533B345 for ; Wed, 17 Jun 2015 01:39:22 +0000 (UTC) From: Juan Quintela In-Reply-To: <20150518113905.GH2201@work-vm> (David Alan Gilbert's message of "Mon, 18 May 2015 12:39:06 +0100") References: <1431620920-19710-1-git-send-email-quintela@redhat.com> <1431620920-19710-10-git-send-email-quintela@redhat.com> <20150518113905.GH2201@work-vm> Date: Wed, 17 Jun 2015 03:39:20 +0200 Message-ID: <87381rglkn.fsf@neno.neno> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 9/9] migration: Add configuration section Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: qemu-devel@nongnu.org "Dr. David Alan Gilbert" wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> It needs to be the first one and it is not optional, that is the reason >> why it is opencoded. For new machine types, it is required than machine >> type name is the same in both sides. >> >> It is just done right now for pc's. >> >> Signed-off-by: Juan Quintela >> --- >> hw/i386/pc_piix.c | 1 + >> hw/i386/pc_q35.c | 1 + >> include/migration/migration.h | 2 ++ >> savevm.c | 72 ++++++++++++++++++++++++++++++++++++++++--- >> 4 files changed, 71 insertions(+), 5 deletions(-) >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index 5c04784..95806b3 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -314,6 +314,7 @@ static void pc_init_pci(MachineState *machine) >> static void pc_compat_2_3(MachineState *machine) >> { >> global_state_set_optional(); >> + savevm_skip_configuration(); >> } >> >> static void pc_compat_2_2(MachineState *machine) >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >> index cc5827a..e32c040 100644 >> --- a/hw/i386/pc_q35.c >> +++ b/hw/i386/pc_q35.c >> @@ -293,6 +293,7 @@ static void pc_q35_init(MachineState *machine) >> static void pc_compat_2_3(MachineState *machine) >> { >> global_state_set_optional(); >> + savevm_skip_configuration(); > > It's a shame that these two functions, that do basically the same thing > (to two different pieces of data) have such different names. Code is similar, but meaning is different. 1st one: it is sent only if needed 2nd one: it is completely skiped. Calling the second: configuration_optional() looks weeird, when the meaning is NO_configuration. We could rename the 1st one to global_state_skip_if_not_needed, but not sure that it would be better :-( I am open to name changes if you provide better names O:-) > >> } >> >> static void pc_compat_2_2(MachineState *machine) >> diff --git a/include/migration/migration.h b/include/migration/migration.h >> index f939d88..da89827 100644 >> --- a/include/migration/migration.h >> +++ b/include/migration/migration.h >> @@ -34,6 +34,7 @@ >> #define QEMU_VM_SECTION_FULL 0x04 >> #define QEMU_VM_SUBSECTION 0x05 >> #define QEMU_VM_VMDESCRIPTION 0x06 >> +#define QEMU_VM_CONFIGURATION 0x07 >> >> struct MigrationParams { >> bool blk; >> @@ -184,4 +185,5 @@ void register_global_state(void); >> void global_state_store(void); >> char *global_state_get_runstate(void); >> void global_state_set_optional(void); >> +void savevm_skip_configuration(void); >> #endif >> diff --git a/savevm.c b/savevm.c >> index 2b4e554..ea149e7 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -238,11 +238,55 @@ typedef struct SaveStateEntry { >> typedef struct SaveState { >> QTAILQ_HEAD(, SaveStateEntry) handlers; >> int global_section_id; >> + bool skip_configuration; >> + uint32_t len; >> + char *name; >> } SaveState; >> >> static SaveState savevm_state = { >> .handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers), >> .global_section_id = 0, >> + .skip_configuration = false, >> +}; >> + >> +void savevm_skip_configuration(void) >> +{ >> + savevm_state.skip_configuration = true; >> +} >> + >> + >> +static void configuration_pre_save(void *opaque) >> +{ >> + SaveState *state = opaque; >> + const char *current_name = MACHINE_GET_CLASS(current_machine)->name; >> + >> + state->len = strlen(current_name); >> + state->name = strdup(current_name); > > Will that ever get freed? If it never gets freed is it safe to > just make it > > state->len = current_name; No, changed. > > >> + >> +static int configuration_post_load(void *opaque, int version_id) >> +{ >> + SaveState *state = opaque; >> + const char *current_name = MACHINE_GET_CLASS(current_machine)->name; >> + >> + if (strncmp(state->name, current_name, state->len) != 0) { >> + error_report("Machine type received is '%s' and local is '%s'", >> + state->name, current_name); >> + return -EINVAL; >> + } >> + return 0; >> +} >> + >> +static const VMStateDescription vmstate_configuration = { >> + .name = "configuartion", > > Typo! configuration Thanks.