[libvirt] [PATCH v5] qemu: Remove the managed state file only if restoring succeeded

[Osier Yang]

于 2011年04月07日 15:21, Daniel Veillard 写道:
> On Thu, Apr 07, 2011 at 10:31:52AM +0800, Osier Yang wrote:
>> 1) Both "qemuDomainStartWithFlags" and "qemuAutostartDomain" try to
>> restore the domain from managedsave'ed image if it exists (by
>> invoking "qemuDomainObjRestore"), but it unlinks the image even
>> if restoring fails, which causes data loss. (This problem exists
>> for "virsh managedsave dom; virsh start dom").
>>
>> The fix for is to unlink the managed state file only if restoring
>> succeeded.
>>
>> 2) For "virsh save dom; virsh restore dom;", it can cause data
>> corruption if one reuse the saved state file for restoring. Add
>> doc to tell user about it.
>>
>> 3) In "qemuDomainObjStart", if "managed_save" is NULL, we shouldn't
>> fallback to start the domain, skipping it to cleanup as a incidental
>> fix. Discovered by Eric.
>>
>> ---
>>   src/qemu/qemu_driver.c |   12 +++++++-----
>>   tools/virsh.pod        |    6 +++++-
>>   2 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 48fe266..a84780b 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -3423,18 +3423,20 @@ static int qemudDomainObjStart(virConnectPtr conn,
>>
>>       /*
>>        * If there is a managed saved state restore it instead of starting
>> -     * from scratch. In any case the old state is removed.
>> +     * from scratch. The old state is removed once the restoring succeeded.
>>        */
>>       managed_save = qemuDomainManagedSavePath(driver, vm);
>> +
>> +    if (!managed_save)
>> +        goto cleanup;
>> +
>>       if ((managed_save)&&  (virFileExists(managed_save))) {
>>           ret = qemuDomainObjRestore(conn, driver, vm, managed_save);
>>
>> -        if (unlink(managed_save)<  0) {
>> +        if ((ret == 0)&&  (unlink(managed_save)<  0))
>>               VIR_WARN("Failed to remove the managed state %s", managed_save);
>> -        }
>>
>> -        if (ret == 0)
>> -            goto cleanup;
>> +        goto cleanup;
>>       }
>>
>>       ret = qemuProcessStart(conn, driver, vm, NULL, start_paused, -1, NULL,
>> diff --git a/tools/virsh.pod b/tools/virsh.pod
>> index f4bd294..6319373 100644
>> --- a/tools/virsh.pod
>> +++ b/tools/virsh.pod
>> @@ -546,7 +546,11 @@ I<on_reboot>  parameter in the domain's XML definition.
>>
>>   =item B<restore>  I<state-file>
>>
>> -Restores a domain from an B<virsh save>  state file.  See I<save>  for more info.
>> +Restores a domain from an B<virsh save>  state file. See I<save>  for more info.
>> +
>> +B<Note>: To avoid corrupting file system contents within the domain, you
>> +should not reuse the saved state file to B<restore>  unless you are convinced
>> +with reverting the domain to the previous state.
>>
>>   =item B<save>  I<domain-id>  I<state-file>
>
>    ACK, thanks !
>
> Daniel
>

Thanks, applied.

Osier