[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Xen-devel] [RFC Patch v3 23/22] Introduce "xen-load-de
From: |
Wen Congyang |
Subject: |
Re: [Qemu-devel] [Xen-devel] [RFC Patch v3 23/22] Introduce "xen-load-devices-state" |
Date: |
Thu, 11 Sep 2014 13:03:12 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 |
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
>>>> address@hidden
>>>> http://lists.xen.org/xen-devel
>>>>
>>> .
>>>
>>
> .
>