[libvirt] [PATCH] qemu: Do not unlink managedsave image if restoring fails.

[Osier Yang]

于 2011年04月06日 16:50, Daniel Veillard 写道:
> On Tue, Apr 05, 2011 at 08:55:01AM -0600, Eric Blake wrote:
>> On 04/05/2011 07:20 AM, Jiri Denemark wrote:
>>> On Tue, Apr 05, 2011 at 14:47:22 +0800, Osier Yang wrote:
>>>> 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.
>>>>
>>>> However, I'm not sure if it's the very correct way to fix it,
>>>> if restoring fails, and we didn't remove the image, it will
>>>> trys to restore from the image again next time, if that's
>>>> not the user expected (e.g. the user made quite many changes
>>>> on the guest), then it's a new problem.
>>>
>>> I think this patch is risky. You should either remove the state on error
>>> (which is the current state) or fail domain start if managed state is present
>>> but resuming from it fails. If you do something in the middle (your patch) you
>>> will certainly end up corrupting domain's disks.
>>
>> What's more, I think we should consider removing the saved-state file on
>> success for 'virsh restore file' - once a state has been restored, the
>> guest is running and has likely modified its disks, which means that the
>> saved (memory) state is no longer consistent with the new disk state,
>> and a second restore of the saved file is asking for a different type of
>> data corruption.
>>
>> That is, I think:
>>
>> virsh save dom file
>> virsh restore file
>>
>> should leave file intact if and only if the restore failed, and:
>
>    The problem there is that you are changing the command behaviour.
> The user may snapshot the disk separately and use this to implement
> a simplified domain snapshot. Doing the remove may avoid troubles
> for those not knowing what they are doing, but also break something
> for those who know what they are doing.

      Somehow this is true, but as we have API and virsh commands
for snapshot specificly, is it fine to make a change on the virsh
manual to tell the user about we remove the state file if restoring
succeeded? I did that when doing v3 patch, :-)

>    In general I would tend to not change the established operation
> behaviour, especially on suddenly removing files owned by the user
> without asking. For managed save that's completely different due to the
> fact the file is owned by libvir, and removing is fine.
>
> Daniel
>