Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-29 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 8:

* #1377849::Update tracker: OK
* #64301::Update tracker: OK
* Set MODIFIED::bug 1377849#1377849::IGNORE, skipping for branch 'master'

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-29 Thread nsoffer 
Nir Soffer has submitted this change and it was merged.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Live Merge: Teardown volume on HSM after live merge

If a VM is running on HSM and live merge is performed, the LV isn't
deactivated because the deactivation is done when deleting the volume.
However, deleting the volume is done on SPM and this means that the LV
is not deactivated on the HSM. In this patch, a logic to teardown the
volume is added after live merge has completed.

Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Bug-Url: https://bugzilla.redhat.com/1377849
Signed-off-by: Ala Hino 
Reviewed-on: https://gerrit.ovirt.org/64301
Reviewed-by: Nir Soffer 
Continuous-Integration: Nir Soffer 
Continuous-Integration: Jenkins CI
Reviewed-by: Adam Litke 
---
M vdsm/storage/blockSD.py
M vdsm/storage/sd.py
M vdsm/virt/vm.py
3 files changed, 19 insertions(+), 0 deletions(-)

Approvals:
  Adam Litke: Looks good to me, approved
  Nir Soffer: Looks good to me, but someone else must approve; Passed CI tests
  Jenkins CI: Passed CI tests
  Ala Hino: Verified



-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-29 Thread alitke 
Adam Litke has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 7: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-28 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 7: Continuous-Integration+1

CI is broken, we will ignore it.

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-28 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 7: Code-Review+1

Waiting for more reviews.

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-28 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 7: Verified+1

Verified both on SPM and HSM that after live merge the volume is deactivated. 
Verified top volume merge and internal volume merge

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-28 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 7:

* update_tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1377849::OK, public bug
* Check Product::#1377849::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-28 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/64301/6/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4779:  "(job %s)", self.job['jobID'])
Line 4780: self.vm._syncVolumeChain(self.drive)
Line 4781: if self.doPivot:
Line 4782: self.vm.enableDriveMonitor()
Line 4783: self.teardown_top_volume()
> How is this needed only after pivot?
Done
Line 4784: self.success = True
Line 4785: self.vm.log.info("Synchronization completed (job %s)",
Line 4786:  self.job['jobID'])
Line 4787: # At this point, merge has succesfully completed and the top 
volume is


Line 4788: # not part of the chain.  Now, we want to teardown the top 
volume. Note
Line 4789: # that if volume deactivation fails, we don't want to fail 
the merge
Line 4790: # whole operation as the VM is running without issues.  It 
is worth to
Line 4791: # note that if volume deactivation fails, chances are high 
that the
Line 4792: # environment is severely damaged.
> This comment conflict with the code now.
Removed
Line 4793: 
Line 4794: def isSuccessful(self):
Line 4795: """
Line 4796: Returns True if this phase completed successfully.


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-27 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 6:

(2 comments)

Did you verify the current code with both top layer merge and internal layer 
merge, or you just copied the verified flag from a previous version?

https://gerrit.ovirt.org/#/c/64301/6/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4779:  "(job %s)", self.job['jobID'])
Line 4780: self.vm._syncVolumeChain(self.drive)
Line 4781: if self.doPivot:
Line 4782: self.vm.enableDriveMonitor()
Line 4783: self.teardown_top_volume()
How is this needed only after pivot?

We need to teardown the volume after any merge, not only top layer merge.

I think I suggested to add teardown in this line, but not inside the if block.
Line 4784: self.success = True
Line 4785: self.vm.log.info("Synchronization completed (job %s)",
Line 4786:  self.job['jobID'])
Line 4787: # At this point, merge has succesfully completed and the top 
volume is


Line 4788: # not part of the chain.  Now, we want to teardown the top 
volume. Note
Line 4789: # that if volume deactivation fails, we don't want to fail 
the merge
Line 4790: # whole operation as the VM is running without issues.  It 
is worth to
Line 4791: # note that if volume deactivation fails, chances are high 
that the
Line 4792: # environment is severely damaged.
This comment conflict with the code now.
Line 4793: 
Line 4794: def isSuccessful(self):
Line 4795: """
Line 4796: Returns True if this phase completed successfully.


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-27 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 6: Code-Review-1

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-25 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 6: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-25 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 6:

* #1377849::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1377849::OK, public bug
* Check Product::#1377849::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-25 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 5:

(5 comments)

https://gerrit.ovirt.org/#/c/64301/5/vdsm/storage/sd.py
File vdsm/storage/sd.py:

PS5, Line 531: teared
> torn
Done


PS5, Line 532: teared
> torn
Done


https://gerrit.ovirt.org/#/c/64301/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

PS5, Line 4786: nvolume
> volume
Done


PS5, Line 4788: megre
> merge
Done


Line 4787: # not part of the chain.  Now, we want to teardown the top 
volume. Note
Line 4788: # that if volume deactivation fails, we don't want to fail 
the megre
Line 4789: # whole operation as the VM is running without issues.  It 
is worth to
Line 4790: # note that if volume deactivation fails, chances are high 
that the
Line 4791: # environment is severely damaged.
> I do want to the operation to fail, we cannot continue to use this environm
Moved
Line 4792: self.teardown_top_volume()
Line 4793: 
Line 4794: def isSuccessful(self):
Line 4795: """


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-23 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/64301/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4764:self.drive.imageID, baseVolUUID,
Line 4765:topVolInfo['capacity'])
Line 4766: 
Line 4767: def teardown_top_volume(self):
Line 4768: # TODO move this method to storage public API
> This needs to happen before we can merge.  We can't have code like this in 
The plan is to backport this to 4.0, where it is ok to add these dependencies. 
Nobody is going to break vdsm to multiple processes in 4.0 :-)

In master I expect to see a patch adding a proper storage api soon.
Line 4769: sd_manifest = 
sdc.sdCache.produce_manifest(self.drive.domainID)
Line 4770: sd_manifest.teardownVolume(self.drive.imageID,
Line 4771:self.job['topVolume'])
Line 4772: 


Line 4787: # not part of the chain.  Now, we want to teardown the top 
volume. Note
Line 4788: # that if volume deactivation fails, we don't want to fail 
the megre
Line 4789: # whole operation as the VM is running without issues.  It 
is worth to
Line 4790: # note that if volume deactivation fails, chances are high 
that the
Line 4791: # environment is severely damaged.
I do want to the operation to fail, we cannot continue to use this environment.
Line 4792: self.teardown_top_volume()
Line 4793: 
Line 4794: def isSuccessful(self):
Line 4795: """


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-23 Thread alitke 
Adam Litke has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 5:

(5 comments)

https://gerrit.ovirt.org/#/c/64301/5/vdsm/storage/sd.py
File vdsm/storage/sd.py:

PS5, Line 531: teared
torn


PS5, Line 532: teared
torn


https://gerrit.ovirt.org/#/c/64301/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

PS5, Line 4768: TODO
This needs to happen before we can merge.  We can't have code like this in virt.


PS5, Line 4786: nvolume
volume


PS5, Line 4788: megre
merge


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-23 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 5:

* #1377849::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1377849::OK, public bug
* Check Product::#1377849::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-23 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 4: Code-Review-1

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-23 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/64301/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4782: self.vm.enableDriveMonitor()
Line 4783: self.success = True
Line 4784: self.vm.log.info("Synchronization completed (job %s)",
Line 4785:  self.job['jobID'])
Line 4786: self.teardown_top_volume()
> After another thought, I think it is better not to fail merge in this case,
I don't think this is a good idea, if we cannot deactivate the lv, or remove 
the symlinks in /run/vdsm, the system is fucked up and we should fail hard and 
loud.

If you do not agree you should add a big comment here explaining why the clean 
must succeed when we cannot teardown the volume.
Line 4787: 
Line 4788: def isSuccessful(self):
Line 4789: """
Line 4790: Returns True if this phase completed successfully.


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-23 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/64301/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4769
Line 4770
Line 4771
Line 4772
Line 4773
> Actually, _syncVolumeChain is defined on the VM and the drive isn't held as
Right, but this operation should have been defined in the cleanup thread. A 
method should use the same abstraction level (e.g operations on self, or 
operations on self.vm etc.)


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-23 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/64301/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4769
Line 4770
Line 4771
Line 4772
Line 4773
> Another place where we should not pass an argument.
Actually, _syncVolumeChain is defined on the VM and the drive isn't held as a 
variable of the VM


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-23 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 3:

(4 comments)

https://gerrit.ovirt.org/#/c/64301/3//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2016-09-22 15:47:20 +0300
Line 4: Commit: Ala Hino 
Line 5: CommitDate: 2016-09-23 00:01:49 +0300
Line 6: 
Line 7: Live Merge: teardown volume on HSM after live merge
> Teardown
Done
Line 8: 
Line 9: If a VM is running on HSM and live merge is performed, the LV isn't
Line 10: deactivated because, the deactivation is done when deleting the volume.
Line 11: However, deleting the volume is done on SPM and this means that the LV


PS3, Line 10: ,
> This comma is redundant.
Done


PS3, Line 13:  
> missing "is"
Done


https://gerrit.ovirt.org/#/c/64301/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4782: self.vm.enableDriveMonitor()
Line 4783: self.success = True
Line 4784: self.vm.log.info("Synchronization completed (job %s)",
Line 4785:  self.job['jobID'])
Line 4786: self.teardown_top_volume()
> This must be done *before* we set self.success to True, so it this fails, w
After another thought, I think it is better not to fail merge in this case, as 
the chances to fail here are really low and if failed, probably the env is 
severely damaged
Line 4787: 
Line 4788: def isSuccessful(self):
Line 4789: """
Line 4790: Returns True if this phase completed successfully.


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: Teardown volume on HSM after live merge

2016-09-23 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Live Merge: Teardown volume on HSM after live merge
..


Patch Set 4:

* #1377849::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1377849::OK, public bug
* Check Product::#1377849::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/64301/3//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2016-09-22 15:47:20 +0300
Line 4: Commit: Ala Hino 
Line 5: CommitDate: 2016-09-23 00:01:49 +0300
Line 6: 
Line 7: Live Merge: teardown volume on HSM after live merge
Teardown
Line 8: 
Line 9: If a VM is running on HSM and live merge is performed, the LV isn't
Line 10: deactivated because, the deactivation is done when deleting the volume.
Line 11: However, deleting the volume is done on SPM and this means that the LV


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/64301/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4771
Line 4772
Line 4773
Line 4774
Line 4775
This can be good place for tearing down the volume.


Line 4782: self.vm.enableDriveMonitor()
Line 4783: self.success = True
Line 4784: self.vm.log.info("Synchronization completed (job %s)",
Line 4785:  self.job['jobID'])
Line 4786: self.teardown_top_volume()
This must be done *before* we set self.success to True, so it this fails, we 
will try again.
Line 4787: 
Line 4788: def isSuccessful(self):
Line 4789: """
Line 4790: Returns True if this phase completed successfully.


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread amureini 
Allon Mureinik has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 3: Code-Review+1

(2 comments)

https://gerrit.ovirt.org/#/c/64301/3//COMMIT_MSG
Commit Message:

PS3, Line 10: ,
This comma is redundant.


PS3, Line 13:  
missing "is"


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 3:

* #1377849::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1377849::OK, public bug
* Check Product::#1377849::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 2:

(3 comments)

https://gerrit.ovirt.org/#/c/64301/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4769
Line 4770
Line 4771
Line 4772
Line 4773
> Another place where we should not pass an argument.
separate patch ...


Line 4763: self.vm._setVolumeSize(self.drive.domainID, 
self.drive.poolID,
Line 4764:self.drive.imageID, baseVolUUID,
Line 4765:topVolInfo['capacity'])
Line 4766: 
Line 4767: def teardown_top_volume(self, imgUUID, volUUID):
> Don't pass arguments to this method, it make the code  harder to read, and 
Done
Line 4768: # TODO move this method to storage public API
Line 4769: sd_manifest = 
sdc.sdCache.produce_manifest(self.drive.domainID)
Line 4770: sd_manifest.teardownVolume(imgUUID, volUUID)
Line 4771: 


Line 4781: self.vm.enableDriveMonitor()
Line 4782: self.success = True
Line 4783: self.vm.log.info("Synchronization completed (job %s)",
Line 4784:  self.job['jobID'])
Line 4785: self.teardown_top_volume(self.drive.imageID, 
self.job['topVolume'])
> This would be much nicer as:
Done
Line 4786: 
Line 4787: def isSuccessful(self):
Line 4788: """
Line 4789: Returns True if this phase completed successfully.


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 2:

(3 comments)

https://gerrit.ovirt.org/#/c/64301/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 4769
Line 4770
Line 4771
Line 4772
Line 4773
Another place where we should not pass an argument.


Line 4763: self.vm._setVolumeSize(self.drive.domainID, 
self.drive.poolID,
Line 4764:self.drive.imageID, baseVolUUID,
Line 4765:topVolInfo['capacity'])
Line 4766: 
Line 4767: def teardown_top_volume(self, imgUUID, volUUID):
Don't pass arguments to this method, it make the code  harder to read, and is 
not needed. We have al the context needed to get the arguments inside this 
helper.
Line 4768: # TODO move this method to storage public API
Line 4769: sd_manifest = 
sdc.sdCache.produce_manifest(self.drive.domainID)
Line 4770: sd_manifest.teardownVolume(imgUUID, volUUID)
Line 4771: 


Line 4781: self.vm.enableDriveMonitor()
Line 4782: self.success = True
Line 4783: self.vm.log.info("Synchronization completed (job %s)",
Line 4784:  self.job['jobID'])
Line 4785: self.teardown_top_volume(self.drive.imageID, 
self.job['topVolume'])
This would be much nicer as:

self.teardown_top_volume()
Line 4786: 
Line 4787: def isSuccessful(self):
Line 4788: """
Line 4789: Returns True if this phase completed successfully.


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 2:

* #1377849::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1377849::OK, public bug
* Check Product::#1377849::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 1:

(8 comments)

https://gerrit.ovirt.org/#/c/64301/1/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 810: offset = ((slot + blockVolume.RESERVED_LEASES) * 
self.logBlkSize *
Line 811:   sd.LEASE_BLOCKS)
Line 812: return clusterlock.Lease(volUUID, self.getLeasesFilePath(), 
offset)
Line 813: 
Line 814: def teardownVolume(self, volUUID):
> Add image uuid, standard api for volume methods.
Done
Line 815: lvm.deactivateLVs(self.sdUUID, [volUUID])
Line 816: 
Line 817: 
Line 818: class BlockStorageDomain(sd.StorageDomain):


https://gerrit.ovirt.org/#/c/64301/1/vdsm/storage/sd.py
File vdsm/storage/sd.py:

Line 524: 
Line 525: if preallocate is not None and preallocate not in sc.VOL_TYPE:
Line 526: raise se.IncorrectType(preallocate)
Line 527: 
Line 528: def teardownVolume(self, volUUID):
> Volume is defined by domain id, image id and volume id. Please add the imag
Done
Line 529: """
Line 530: By default, this method does nothing. It is overriden in 
blockSD
Line 531: to do the necessary cleaning.
Line 532: """


Line 527: 
Line 528: def teardownVolume(self, volUUID):
Line 529: """
Line 530: By default, this method does nothing. It is overriden in 
blockSD
Line 531: to do the necessary cleaning.
> Replace the text with something like:
Done
Line 532: """
Line 533: pass
Line 534: 
Line 535: 


Line 529: """
Line 530: By default, this method does nothing. It is overriden in 
blockSD
Line 531: to do the necessary cleaning.
Line 532: """
Line 533: pass
> pass is not needed, having a docstring is enough.
Done
Line 534: 
Line 535: 
Line 536: class StorageDomain(object):
Line 537: log = logging.getLogger("storage.StorageDomain")


https://gerrit.ovirt.org/#/c/64301/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 64: from vdsm.virt.vmpowerdown import VmShutdown, VmReboot
Line 65: from vdsm.virt.utils import isVdsmImage, cleanup_guest_socket
Line 66: from storage import outOfProcess as oop
Line 67: from storage import sd
Line 68: from storage.sdc import sdCache
> Please avoid this bad practice - this must be:
Done
Line 69: 
Line 70: # local imports
Line 71: # In future those should be imported via ..
Line 72: import caps


Line 4763: self.vm._setVolumeSize(self.drive.domainID, 
self.drive.poolID,
Line 4764:self.drive.imageID, baseVolUUID,
Line 4765:topVolInfo['capacity'])
Line 4766: 
Line 4767: def teardownVolume(self, volUUID):
> This should match other methods like update_base_size - no arguments, lower
Done
Line 4768: dom_manifest = sdCache.produce_manifest(self.drive.domainID)
Line 4769: dom_manifest.teardownVolume(volUUID)
Line 4770: 
Line 4771: @utils.traceback()


Line 4764:self.drive.imageID, baseVolUUID,
Line 4765:topVolInfo['capacity'])
Line 4766: 
Line 4767: def teardownVolume(self, volUUID):
Line 4768: dom_manifest = sdCache.produce_manifest(self.drive.domainID)
> We call this sd_manifest elsewhere.
Done
Line 4769: dom_manifest.teardownVolume(volUUID)
Line 4770: 
Line 4771: @utils.traceback()
Line 4772: def run(self):


Line 4780: self.vm.enableDriveMonitor()
Line 4781: self.success = True
Line 4782: self.vm.log.info("Synchronization completed (job %s)",
Line 4783:  self.job['jobID'])
Line 4784: # teardown the merged volume
> The comment is not needed, the function name should reveal the intent. If y
Done
Line 4785: self.teardownVolume(self.job['topVolume'])
Line 4786: 
Line 4787: def isSuccessful(self):
Line 4788: """


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 1:

(8 comments)

Nice!

https://gerrit.ovirt.org/#/c/64301/1/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 810: offset = ((slot + blockVolume.RESERVED_LEASES) * 
self.logBlkSize *
Line 811:   sd.LEASE_BLOCKS)
Line 812: return clusterlock.Lease(volUUID, self.getLeasesFilePath(), 
offset)
Line 813: 
Line 814: def teardownVolume(self, volUUID):
Add image uuid, standard api for volume methods.
Line 815: lvm.deactivateLVs(self.sdUUID, [volUUID])
Line 816: 
Line 817: 
Line 818: class BlockStorageDomain(sd.StorageDomain):


https://gerrit.ovirt.org/#/c/64301/1/vdsm/storage/sd.py
File vdsm/storage/sd.py:

Line 524: 
Line 525: if preallocate is not None and preallocate not in sc.VOL_TYPE:
Line 526: raise se.IncorrectType(preallocate)
Line 527: 
Line 528: def teardownVolume(self, volUUID):
Volume is defined by domain id, image id and volume id. Please add the image id 
to this function in this patch (you added it in the next patch).
Line 529: """
Line 530: By default, this method does nothing. It is overriden in 
blockSD
Line 531: to do the necessary cleaning.
Line 532: """


Line 527: 
Line 528: def teardownVolume(self, volUUID):
Line 529: """
Line 530: By default, this method does nothing. It is overriden in 
blockSD
Line 531: to do the necessary cleaning.
Replace the text with something like:

Called when a volume is detached from a prepared image during
live merge flow. In this case the volume will not be teardown
when the image is tear down.

This does nothing, subclass should override this if needed.
Line 532: """
Line 533: pass
Line 534: 
Line 535: 


Line 529: """
Line 530: By default, this method does nothing. It is overriden in 
blockSD
Line 531: to do the necessary cleaning.
Line 532: """
Line 533: pass
pass is not needed, having a docstring is enough.
Line 534: 
Line 535: 
Line 536: class StorageDomain(object):
Line 537: log = logging.getLogger("storage.StorageDomain")


https://gerrit.ovirt.org/#/c/64301/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 64: from vdsm.virt.vmpowerdown import VmShutdown, VmReboot
Line 65: from vdsm.virt.utils import isVdsmImage, cleanup_guest_socket
Line 66: from storage import outOfProcess as oop
Line 67: from storage import sd
Line 68: from storage.sdc import sdCache
Please avoid this bad practice - this must be:

from storage import sdc
Line 69: 
Line 70: # local imports
Line 71: # In future those should be imported via ..
Line 72: import caps


Line 4763: self.vm._setVolumeSize(self.drive.domainID, 
self.drive.poolID,
Line 4764:self.drive.imageID, baseVolUUID,
Line 4765:topVolInfo['capacity'])
Line 4766: 
Line 4767: def teardownVolume(self, volUUID):
This should match other methods like update_base_size - no arguments, 
lower_case:

def teardown_top_volume(self):

The flow in _run() should read like documentation.

Add TODO to move this to storage public api. This is a layering violation, virt 
code should not call storage code except via public storage apis (e.g. 
irs.getVolumeSize())
Line 4768: dom_manifest = sdCache.produce_manifest(self.drive.domainID)
Line 4769: dom_manifest.teardownVolume(volUUID)
Line 4770: 
Line 4771: @utils.traceback()


Line 4764:self.drive.imageID, baseVolUUID,
Line 4765:topVolInfo['capacity'])
Line 4766: 
Line 4767: def teardownVolume(self, volUUID):
Line 4768: dom_manifest = sdCache.produce_manifest(self.drive.domainID)
We call this sd_manifest elsewhere.

Get volume and image id here, from self.job.
Line 4769: dom_manifest.teardownVolume(volUUID)
Line 4770: 
Line 4771: @utils.traceback()
Line 4772: def run(self):


Line 4780: self.vm.enableDriveMonitor()
Line 4781: self.success = True
Line 4782: self.vm.log.info("Synchronization completed (job %s)",
Line 4783:  self.job['jobID'])
Line 4784: # teardown the merged volume
The comment is not needed, the function name should reveal the intent. If you 
need more documentation - why we must do this, add a docstring to the function.
Line 4785: self.teardownVolume(self.job['topVolume'])
Line 4786: 
Line 4787: def isSuccessful(self):
Line 4788: """


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 1: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 1:

* #1377849::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1377849::OK, public bug
* Check Product::#1377849::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread ahino 
Ala Hino has uploaded a new change for review.

Change subject: Live Merge: teardown volume on HSM after live merge
..

Live Merge: teardown volume on HSM after live merge

If a VM is running on HSM and live merge is performed, the LV isn't
deactivated because, the deactivation is done when deleting the volume.
However, deleting the volume is done on SPM and this means that the LV
is not deactivated on the HSM. In this patch, a logic to teardown the
volume added after live merge has completed.

Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Bug-Url: https://bugzilla.redhat.com/1377849
Signed-off-by: Ala Hino 
---
M vdsm/storage/blockSD.py
M vdsm/storage/sd.py
M vdsm/virt/vm.py
3 files changed, 17 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/64301/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 8ac7433..a538581 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -811,6 +811,9 @@
   sd.LEASE_BLOCKS)
 return clusterlock.Lease(volUUID, self.getLeasesFilePath(), offset)
 
+def teardownVolume(self, volUUID):
+lvm.deactivateLVs(self.sdUUID, [volUUID])
+
 
 class BlockStorageDomain(sd.StorageDomain):
 manifestClass = BlockStorageDomainManifest
diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py
index b5b902e..29dcf53 100644
--- a/vdsm/storage/sd.py
+++ b/vdsm/storage/sd.py
@@ -525,6 +525,13 @@
 if preallocate is not None and preallocate not in sc.VOL_TYPE:
 raise se.IncorrectType(preallocate)
 
+def teardownVolume(self, volUUID):
+"""
+By default, this method does nothing. It is overriden in blockSD
+to do the necessary cleaning.
+"""
+pass
+
 
 class StorageDomain(object):
 log = logging.getLogger("storage.StorageDomain")
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 1d9a3d5..7004615 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -65,6 +65,7 @@
 from vdsm.virt.utils import isVdsmImage, cleanup_guest_socket
 from storage import outOfProcess as oop
 from storage import sd
+from storage.sdc import sdCache
 
 # local imports
 # In future those should be imported via ..
@@ -4763,6 +4764,10 @@
self.drive.imageID, baseVolUUID,
topVolInfo['capacity'])
 
+def teardownVolume(self, volUUID):
+dom_manifest = sdCache.produce_manifest(self.drive.domainID)
+dom_manifest.teardownVolume(volUUID)
+
 @utils.traceback()
 def run(self):
 self.update_base_size()
@@ -4776,6 +4781,8 @@
 self.success = True
 self.vm.log.info("Synchronization completed (job %s)",
  self.job['jobID'])
+# teardown the merged volume
+self.teardownVolume(self.job['topVolume'])
 
 def isSuccessful(self):
 """


-- 
To view, visit https://gerrit.ovirt.org/64301
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iec3b6adb50293d8c98f5d8726d668eb272d16549
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 4: Verified-1

-- 
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 4: Code-Review-1

Did we agree that this is not the right approach?

-- 
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-22 Thread ahino 
Ala Hino has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 4: Verified+1

Need to verify wipe after delete logic when deactivating the LV before deleting 
the volume

-- 
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Live Merge: teardown volume on HSM after live merge

2016-09-21 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: Live Merge: teardown volume on HSM after live merge
..


Patch Set 4:

* #1377849::Update tracker: OK
* #1321018::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1377849::OK, public bug
* Check Public Bug::#1321018::OK, public bug
* Check Product::#1377849::OK, Correct classification oVirt
* Check Product::#1321018::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/59725
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88bf92e702ac6c324b87c9459b01adf165eaca4
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org