| Message ID | alpine.DEB.2.02.1409102005450.8137@kaball.uk.xensource.com |
|---|---|
| State | New |
| Headers | show |
On 09/11/2014 03:15 AM, Stefano Stabellini wrote: > On Tue, 9 Sep 2014, Wen Congyang wrote: >> At 09/06/2014 05:57 AM, Stefano Stabellini Write: >>> On Fri, 5 Sep 2014, Wen Congyang wrote: >>>> introduce a "xen-load-devices-state" QAPI command that can be used to load >>>> the state of all devices, but not the RAM or the block devices of the >>>> VM. >>> >>> Hello Wen, >>> please CC qemu-devel too for QEMU patches. >>> >>> Could you please explain why do you need this command? >>> >>> Is the main issue that there are no QMP commands today to load the state >>> of a VM? It looks like that for vm restore we are using the -incoming >>> command line option, but there is no alternative over QMP. >> >> We only have hmp commands savevm/loadvm, and qmp commands xen-save-devices-state. >> >> We use this new command for COLO: >> 1. suspend both primay vm and secondary vm >> 2. sync the state >> 3. resume both primary vm and secondary vm >> >> In such case, we need to update all devices's state in any time. > > OK. Please state this in the commit message. > > >>> [...] >>> >>> >>>> diff --git a/savevm.c b/savevm.c >>>> index 22123be..c6aa502 100644 >>>> --- a/savevm.c >>>> +++ b/savevm.c >>>> @@ -863,6 +863,105 @@ out: >>>> return ret; >>>> } >>>> >>>> +static int qemu_load_devices_state(QEMUFile *f) >>>> +{ >>>> + uint8_t section_type; >>>> + unsigned int v; >>>> + int ret; >>>> + >>>> + if (qemu_savevm_state_blocked(NULL)) { >>>> + return -EINVAL; >>>> + } >>>> + >>>> + v = qemu_get_be32(f); >>>> + if (v != QEMU_VM_FILE_MAGIC) { >>>> + return -EINVAL; >>>> + } >>>> + >>>> + v = qemu_get_be32(f); >>>> + if (v == QEMU_VM_FILE_VERSION_COMPAT) { >>>> + fprintf(stderr, "SaveVM v2 format is obsolete and don't work anymore\n"); >>>> + return -ENOTSUP; >>>> + } >>>> + if (v != QEMU_VM_FILE_VERSION) { >>>> + return -ENOTSUP; >>>> + } >>>> + >>>> + while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) { >>>> + uint32_t instance_id, version_id, section_id; >>>> + SaveStateEntry *se; >>>> + char idstr[257]; >>>> + int len; >>>> + >>>> + switch (section_type) { >>>> + case QEMU_VM_SECTION_FULL: >>>> + /* Read section start */ >>>> + section_id = qemu_get_be32(f); >>>> + len = qemu_get_byte(f); >>>> + qemu_get_buffer(f, (uint8_t *)idstr, len); >>>> + idstr[len] = 0; >>>> + instance_id = qemu_get_be32(f); >>>> + version_id = qemu_get_be32(f); >>>> + >>>> + /* Find savevm section */ >>>> + se = find_se(idstr, instance_id); >>>> + if (se == NULL) { >>>> + fprintf(stderr, "Unknown savevm section or instance '%s' %d\n", >>>> + idstr, instance_id); >>>> + ret = -EINVAL; >>>> + goto out; >>>> + } >>>> + >>>> + /* Validate version */ >>>> + if (version_id > se->version_id) { >>>> + fprintf(stderr, "loadvm: unsupported version %d for '%s' v%d\n", >>>> + version_id, idstr, se->version_id); >>>> + ret = -EINVAL; >>>> + goto out; >>>> + } >>>> + >>>> + /* Validate if it is a device's state */ >>>> + if (se->is_ram) { >>>> + fprintf(stderr, "loadvm: %s is not devices state\n", idstr); >>>> + ret = -EINVAL; >>>> + goto out; >>>> + } >>>> + >>>> + ret = vmstate_load(f, se, version_id); >>>> + if (ret < 0) { >>>> + fprintf(stderr, "qemu: warning: error while loading state for instance 0x%x of device '%s'\n", >>>> + instance_id, idstr); >>>> + goto out; >>>> + } >>>> + break; >>>> + case QEMU_VM_SECTION_START: >>>> + case QEMU_VM_SECTION_PART: >>>> + case QEMU_VM_SECTION_END: >>>> + /* >>>> + * The file is saved by the command xen-save-devices-state, >>>> + * So it should not contain section start/part/end. >>>> + */ >>>> + default: >>>> + fprintf(stderr, "Unknown savevm section type %d\n", section_type); >>>> + ret = -EINVAL; >>>> + goto out; >>>> + } >>>> + } >>>> + >>>> + cpu_synchronize_all_post_init(); >>>> + >>>> + ret = 0; >>>> + >>>> +out: >>>> + if (ret == 0) { >>>> + if (qemu_file_get_error(f)) { >>>> + ret = -EIO; >>>> + } >>>> + } >>>> + >>>> + return ret; >>>> +} >>> >>> Assuming that the state file only contains device states, why don't you >>> just call qemu_loadvm_state to implement the command? >> >> Do you mean there is no need to check the file? If the file contains >> some memory, it will cause unexpected problem. > > I would prefer to avoid duplicating qemu_loadvm_state just to add an > if (se->is_ram) check. > I would rather introduce a generic loadvm QMP command and rely on the > fact that we are saving only device states via xen-save-devices-state. > > Given that loading memory in QEMU on Xen always leads to errors, maybe > we could still add a check in qemu_loadvm_state anyway. Something like: > > diff --git a/savevm.c b/savevm.c > index e19ae0a..759eefa 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -938,6 +938,13 @@ int qemu_loadvm_state(QEMUFile *f) > goto out; > } > > + /* Validate if it is a device's state */ > + if (xen_enabled() && se->is_ram) { > + fprintf(stderr, "loadvm: %s RAM loading not allowed on Xen\n", idstr); > + ret = -EINVAL; > + goto out; > + } > + > /* Add entry */ > le = g_malloc0(sizeof(*le)); Thanks, I think it works for me. Wen Congyang > > > > >> Thanks >> Wen Congyang >> >>> >>> >>> >>>> static BlockDriverState *find_vmstate_bs(void) >>>> { >>>> BlockDriverState *bs = NULL; >>>> @@ -1027,6 +1126,33 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp) >>>> } >>>> } >>>> >>>> +void qmp_xen_load_devices_state(const char *filename, Error **errp) >>>> +{ >>>> + QEMUFile *f; >>>> + int saved_vm_running; >>>> + int ret; >>>> + >>>> + saved_vm_running = runstate_is_running(); >>>> + vm_stop(RUN_STATE_RESTORE_VM); >>>> + >>>> + f = qemu_fopen(filename, "rb"); >>>> + if (!f) { >>>> + error_setg_file_open(errp, errno, filename); >>>> + goto out; >>>> + } >>>> + >>>> + ret = qemu_load_devices_state(f); >>>> + qemu_fclose(f); >>>> + if (ret < 0) { >>>> + error_set(errp, QERR_IO_ERROR); >>>> + } >>>> + >>>> +out: >>>> + if (saved_vm_running) { >>>> + vm_start(); >>>> + } >>>> +} >>>> + >>>> int load_vmstate(const char *name) >>>> { >>>> BlockDriverState *bs, *bs_vm_state; >>>> -- >>>> 1.9.0 >>>> >>>> >>>> _______________________________________________ >>>> Xen-devel mailing list >>>> Xen-devel@lists.xen.org >>>> http://lists.xen.org/xen-devel >>>> >>> . >>> >> > . >
diff --git a/savevm.c b/savevm.c index e19ae0a..759eefa 100644 --- a/savevm.c +++ b/savevm.c @@ -938,6 +938,13 @@ int qemu_loadvm_state(QEMUFile *f) goto out; } + /* Validate if it is a device's state */ + if (xen_enabled() && se->is_ram) { + fprintf(stderr, "loadvm: %s RAM loading not allowed on Xen\n", idstr); + ret = -EINVAL; + goto out; + } + /* Add entry */ le = g_malloc0(sizeof(*le));