[Engine-devel] Adding atomic restore snapshot command at backend

[Michael Pasternak]

On 05/31/2012 09:04 AM, Livnat Peer wrote:
> On 30/05/12 17:55, Michael Pasternak wrote:
>> On 05/30/2012 05:39 PM, Omer Frenkel wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Michael Pasternak" <mpastern at redhat.com>
>>>> To: "Livnat Peer" <lpeer at redhat.com>
>>>> Cc: "engine-devel" <engine-devel at ovirt.org>
>>>> Sent: Monday, May 28, 2012 2:37:10 PM
>>>> Subject: Re: [Engine-devel] Adding atomic restore snapshot command at backend
>>>>
>>>>
>>>> Hi Livnat,
>>>>
>>>> On 05/28/2012 02:07 PM, Livnat Peer wrote:
>>>>> On 28/05/12 12:59, Michael Pasternak wrote:
>>>>>>
>>>>>> Currently 'restore snapshot' done in two steps on a client side:
>>>>>>
>>>>>> 1. TryBackToAllSnapshotsOfVm
>>>>>> 2. RestoreAllSnapshots
>>>>>>
>>>>>> This implementation creates race condition on 1 and therefore
>>>>>> unstable & bug prone,
>>>
>>> IIRC, there is an easy way to fix this,
>>> by polling on engine jobs and not vdsm tasks, no?
>>
>> indeed, and we already doing that, but it still fails on race at
>> former command.
>>
> 
> Can you give more details on that?
> The Job indicates it is finished but actually it is not?

Ori?

> 
> 
>>>
>>>>>> i suggested refactoring 2 to include 1 as single atomic operation
>>>>>> at backend.
>>>>>>
>>>>>>
>>>>>
>>>>> Hi Michael,
>>>>>
>>>>> The two commands above are used for functionality that is needed
>>>>> regardless of the functionality you are looking for.
>>>>> We want to present the user the option to preview a snapshot
>>>>> without
>>>>> committing to it and without loosing the current snapshot.
>>>>> I think we need to model the the two options above in the REST, it
>>>>> is
>>>>> functionality that is used in the UI.
>>>
>>> +1 for that.
>>>
>>>>
>>>> this is out of scope of this RFE.
>>>>
>>>>>
>>>>> What you are looking for is a functionality 'restore to snapshot',
>>>>> this
>>>>> functionality does not exist in the engine in a single step, and I
>>>>> think
>>>>> that because we assumed that the user can get it in two steps It
>>>>> wasn't
>>>>> prioritize so far.
>>>>
>>>> the cons. of this approach is an async nature of the former command.
>>>>
>>>>>
>>>>> Implementing the missing functionality with the two functions above
>>>>> is
>>>>> a compromise that was done in the API.
>>>>>
>>>>> To summarize I think the requirement you present is a missing
>>>>> functionality in the engine and solving it is not about refactoring
>>>>> the
>>>>> two existing verbs into one.
>>>>
>>>> what i meant is not deprecating/refactoring old commands, but
>>>> introducing
>>>> new one that reusing them and expose as single (atomic) command
>>>> called
>>>> 'RestoreAllSnapshots',
>>>>
>>>> sorry if i wasn't clear.
>>>
>>> another suggestion is to have a way to send to the engine list of commands that will be performed.
>>
>> afaik both UI and API perform restore in a same manner,
>>
>> 1. TryBackToAllSnapshotsOfVm
>> 2. polling
>> 3. RestoreAllSnapshots
>>
>> so why not combining 1 & 3 in to single command, if from clients perspective
>> it is such ...
>>
> 
> The UI is not polling and does not run the two operations above
> automatically.
> It is doing the TryBack and then the user can start the VM and decides
> if he wants to commit to this snapshot or return to the original one -
> which is the functionality the API is missing.
> 
> 
>>>
>>>>
>>>>>
>>>>> Thanks, Livnat
>>>>>
>>>>
>>>>
>>>> --
>>>>
>>>> Michael Pasternak
>>>> RedHat, ENG-Virtualization R&D
>>>> _______________________________________________
>>>> Engine-devel mailing list
>>>> Engine-devel at ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>>
>>
>>
> 


-- 

Michael Pasternak
RedHat, ENG-Virtualization R&D