[Engine-devel] Adding atomic restore snapshot command at backend
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, i suggested refactoring 2 to include 1 as single atomic operation at backend. -- Michael Pasternak RedHat, ENG-Virtualization R&D
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, 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. 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. 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. Thanks, Livnat
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, 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.
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.
Thanks, Livnat
-- Michael Pasternak RedHat, ENG-Virtualization R&D
----- Original Message -----
From: "Michael Pasternak" <mpastern@redhat.com> To: "Livnat Peer" <lpeer@redhat.com> Cc: "engine-devel" <engine-devel@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?
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.
Thanks, Livnat
--
Michael Pasternak RedHat, ENG-Virtualization R&D _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
On 05/30/2012 05:39 PM, Omer Frenkel wrote:
----- Original Message -----
From: "Michael Pasternak" <mpastern@redhat.com> To: "Livnat Peer" <lpeer@redhat.com> Cc: "engine-devel" <engine-devel@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@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
-- Michael Pasternak RedHat, ENG-Virtualization R&D
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@redhat.com> To: "Livnat Peer" <lpeer@redhat.com> Cc: "engine-devel" <engine-devel@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?
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@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
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@redhat.com> To: "Livnat Peer" <lpeer@redhat.com> Cc: "engine-devel" <engine-devel@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@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
-- Michael Pasternak RedHat, ENG-Virtualization R&D
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@redhat.com> To: "Livnat Peer" <lpeer@redhat.com> Cc: "engine-devel" <engine-devel@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?
Moti and I fixed this and it's in pre-integration, it might work. Regardless, the Backend imposes unnecessarily complex API for a simple operation that a client might want to do: 'Go back to Snapshot xxx for VM yyy' Any client who wants to support this must activate two actions, blocking between the first and the second. Why replicate this complexity in all potential clients? Why does a client care that Backend chooses to implement this command as two steps? That is an implmentation detail. It really doesn't matter that the first client we had was the GUI, and that in GUI previewing the VM and then approving is convenient.
> 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
--
Michael Pasternak RedHat, ENG-Virtualization R&D
participants (4)
-
Livnat Peer -
Michael Pasternak -
Omer Frenkel -
Ori Liel