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

[Michael Pasternak]

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.

> 
>>>> 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 ...

> 
>>
>>>
>>> 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