Eduardo has uploaded a new change for review.
Change subject: Remove unnecesary preparePaths. ......................................................................
Remove unnecesary preparePaths.
Recovering running VM's therefore paths are already prepared. Prepare paths is not locking the volumes anymore (Federico).
Change-Id: I35890d36227633ca147387d670c152b9be357e50 --- M vdsm/clientIF.py 1 file changed, 0 insertions(+), 16 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/86/786/1 -- To view, visit http://gerrit.ovirt.org/786 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I35890d36227633ca147387d670c152b9be357e50 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com
Dan Kenigsberg has posted comments on this change.
Change subject: Remove unnecesary preparePaths. ......................................................................
Patch Set 1: I would prefer that you didn't submit this
this must be rebased on top of Fedorico patches in order to get in. (I believe that we SHOULD re-take the volume locks on Vdsm restart).
-- To view, visit http://gerrit.ovirt.org/786 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I35890d36227633ca147387d670c152b9be357e50 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com
Igor Lvovsky has posted comments on this change.
Change subject: Remove unnecesary preparePaths. ......................................................................
Patch Set 1: Verified
-- To view, visit http://gerrit.ovirt.org/786 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I35890d36227633ca147387d670c152b9be357e50 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Federico Simoncelli has posted comments on this change.
Change subject: Remove unnecesary preparePaths. ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/clientIF.py Line 985 Removing this is wrong (at least at the moment). To speed up the recovery the current flow is:
1. Recover all the VMs objects (WaitForLaunch) 2. Clean up the repository 3. Reconnect to the pool (re-creates the missing links) 4. Wait for the re-connection to complete 5. Prepare the paths (here), at the moment just to switch _volumesPrepared to True (but we can't guarantee that in the future we'll need more things so I'd like to keep the preparePaths call) 6. Now that everything is ready we can resume the vm stats
-- To view, visit http://gerrit.ovirt.org/786 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I35890d36227633ca147387d670c152b9be357e50 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Eduardo has posted comments on this change.
Change subject: Remove unnecesary preparePaths. ......................................................................
Patch Set 1: (1 inline comment)
.................................................... File vdsm/clientIF.py Line 985 Since the VM's are up and running, there is no need for any real storage operation. All is already done. We only need to update the VM register due to the unfortunate fact that we are using the conf instead live info from the host. This prepare call should not be here since all is already prepared. Additional future things should have proper calls, but I feel that for all cases this will not related to storage. The vmstats issue is another proof that stats is not related to VM life nor storage management.
-- To view, visit http://gerrit.ovirt.org/786 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I35890d36227633ca147387d670c152b9be357e50 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Ayal Baron has posted comments on this change.
Change subject: Remove unnecesary preparePaths. ......................................................................
Patch Set 1: Do not submit
I agree with Federico, this code is needed unless you find a different way to make sure that no other operation on these images can deactivate the LVs
-- To view, visit http://gerrit.ovirt.org/786 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I35890d36227633ca147387d670c152b9be357e50 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
Yaniv Bronhaim has posted comments on this change.
Change subject: Remove unnecesary preparePaths. ......................................................................
Patch Set 1: (1 inline comment)
.................................................... File vdsm/clientIF.py Line 981 Line 982 Line 983 Line 984 Line 985 I don't agree here: 1. We do that 2. We do that 3. I don't understand why we need to reconnect? We are already connected if vms are still running.. 4. Same as 3 5. Same as 3 - why to keep something that maybe in the future we will need... Until the future we want to do it faster and righter.. no? 6. We could resume the vm stats after 2...
Federico, Please explain why you think we need real storage operations here because I don't understand..
-- To view, visit http://gerrit.ovirt.org/786 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I35890d36227633ca147387d670c152b9be357e50 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Itamar Heim has posted comments on this change.
Change subject: Remove unnecesary preparePaths. ......................................................................
Patch Set 1:
ping
Itamar Heim has abandoned this change.
Change subject: Remove unnecesary preparePaths. ......................................................................
Abandoned
abandoning - old. no reply. not touched for a while. please restore if relevant
vdsm-patches@lists.fedorahosted.org