Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
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]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
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]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
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]: implementing StorageDomain.movePV

2016-09-21 Thread laravot 
Liron Aravot has posted comments on this change.

Change subject: implementing StorageDomain.movePV
..


Patch Set 2:

(19 comments)

https://gerrit.ovirt.org/#/c/63270/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 559: 
Line 560: def movePV(self, guid):
Line 561: self._validateNotMetadataPV(guid)
Line 562: with self._extendlock:
Line 563: lvm.movePV(self.sdUUID, guid)
> updateMapping should be needed only for domain version 0 - do we want to su
I don't see a reason to not support old domain unless its too complicated, Done.
Line 564: self.updateMapping()
Line 565: 
Line 566: def reduceDevice(self, guid):
Line 567: self._validateNotMetadataPV(guid)


Line 562: with self._extendlock:
Line 563: lvm.movePV(self.sdUUID, guid)
Line 564: self.updateMapping()
Line 565: 
Line 566: def reduceDevice(self, guid):
> I think should call this reduce(), similar to extend().
Done and moved to the correct patch (it was accidentally added to this one)
Line 567: self._validateNotMetadataPV(guid)
Line 568: with self._extendlock:
Line 569: lvm.reduceVG(self.sdUUID, guid)
Line 570: self.updateMapping()


Line 565: 
Line 566: def reduceDevice(self, guid):
Line 567: self._validateNotMetadataPV(guid)
Line 568: with self._extendlock:
Line 569: lvm.reduceVG(self.sdUUID, guid)
> No error handling here is fine, assuming that reduceVG remove the pv only i
Well, on that case the engine will fail the flow, if the user will try the 
operation again we should fail with an error indicating that the pv isn't part 
of the VG - when getting that error the engine should update the db accordingly.
That should be similar to deleting an image that has been already deleted.
Line 570: self.updateMapping()
Line 571: 
Line 572: def _validateNotMetadataPV(self, guid):
Line 573: lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)


Line 569: lvm.reduceVG(self.sdUUID, guid)
Line 570: self.updateMapping()
Line 571: 
Line 572: def _validateNotMetadataPV(self, guid):
Line 573: lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
> Why the lvm_ prefix? do we have other kind of metadata_pv?
Done
Line 574: if lvm.isSamePV(lvm_metadata_pv, guid):
Line 575: raise 
se.CouldNotPerformOperationOnStorageDomainMetadataPV(guid)
Line 576: 
Line 577: def getVolumeClass(self):


Line 570: self.updateMapping()
Line 571: 
Line 572: def _validateNotMetadataPV(self, guid):
Line 573: lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
Line 574: if lvm.isSamePV(lvm_metadata_pv, guid):
> We don't need this helper. 
Done
Line 575: raise 
se.CouldNotPerformOperationOnStorageDomainMetadataPV(guid)
Line 576: 
Line 577: def getVolumeClass(self):
Line 578: """


Line 571: 
Line 572: def _validateNotMetadataPV(self, guid):
Line 573: lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
Line 574: if lvm.isSamePV(lvm_metadata_pv, guid):
Line 575: raise 
se.CouldNotPerformOperationOnStorageDomainMetadataPV(guid)
> Error name is too long - how about CannotChangeMetadataPV?
I've shortened it by changing to CouldNotPerformOperationOnMetadataPV
Line 576: 
Line 577: def getVolumeClass(self):
Line 578: """
Line 579: Return a type specific volume generator object


https://gerrit.ovirt.org/#/c/63270/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 780: :type sdUUID: UUID
Line 781: :param guid: A block device GUID
Line 782: :type guid: str
Line 783: """
Line 784: job = sdm.api.move_sd_device_data.Job(jobUUID, 1, sdUUID, 
guid)
> Need to document why host_id=1 is correct in this case. If we have more cod
Done
Line 785: self.sdm_schedule(job)
Line 786: 
Line 787: def _deatchStorageDomainFromOldPools(self, sdUUID):
Line 788: # We are called with blank pool uuid, to avoid changing 
exiting


https://gerrit.ovirt.org/#/c/63270/2/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 692: return pv
Line 693: 
Line 694: 
Line 695: def isSamePV(pv_a, pv_b):
Line 696: return _fqpvname(pv_a) == _fqpvname(pv_b)
> I don't like this normalization orgy. The module should accept only guid or
Done
Line 697: 
Line 698: 
Line 699: def _createpv(devices, metadataSize, options=tuple()):
Line 700: """


Line 889: 
Line 890: def _hasDataToMove(pvName):
Line 891: pv = getPV(pvName)
Line 892: if pv.pe_alloc_count == "0":
Line 893: return False
> This helper is not very useful in this module, so better inline it in the f
well, it shortens the caller function and removes from it this not very 
interesting code.
Line 894: 
Line 895: return True
Line 896: 
Li

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/63270/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 562: with self._extendlock:
Line 563: lvm.movePV(self.sdUUID, guid)
Line 564: self.updateMapping()
Line 565: 
Line 566: def reduceDevice(self, guid):
I think should call this reduce(), similar to extend().
Line 567: self._validateNotMetadataPV(guid)
Line 568: with self._extendlock:
Line 569: lvm.reduceVG(self.sdUUID, guid)
Line 570: self.updateMapping()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 2:

(15 comments)

https://gerrit.ovirt.org/#/c/63270/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 570: self.updateMapping()
Line 571: 
Line 572: def _validateNotMetadataPV(self, guid):
Line 573: lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
Line 574: if lvm.isSamePV(lvm_metadata_pv, guid):
We don't need this helper. 

guid is always a guid, it should not be a device path. We must be super strict 
about what we accept in the api. If a user send  a path instead of guid the 
request should fail when creating the job.

Since we know guid is not a path, we don't need to normalize it.

lvm.getFirstExt returns single format. If it returns a path, use 
os.path.basename() to the the guid. If it returns a guid, we are done.
Line 575: raise 
se.CouldNotPerformOperationOnStorageDomainMetadataPV(guid)
Line 576: 
Line 577: def getVolumeClass(self):
Line 578: """


Line 571: 
Line 572: def _validateNotMetadataPV(self, guid):
Line 573: lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
Line 574: if lvm.isSamePV(lvm_metadata_pv, guid):
Line 575: raise 
se.CouldNotPerformOperationOnStorageDomainMetadataPV(guid)
Error name is too long - how about CannotChangeMetadataPV?
Line 576: 
Line 577: def getVolumeClass(self):
Line 578: """
Line 579: Return a type specific volume generator object


https://gerrit.ovirt.org/#/c/63270/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 780: :type sdUUID: UUID
Line 781: :param guid: A block device GUID
Line 782: :type guid: str
Line 783: """
Line 784: job = sdm.api.move_sd_device_data.Job(jobUUID, 1, sdUUID, 
guid)
Need to document why host_id=1 is correct in this case. If we have more code, 
maybe we need a constant like DETACHED_HOST_ID.
Line 785: self.sdm_schedule(job)
Line 786: 
Line 787: def _deatchStorageDomainFromOldPools(self, sdUUID):
Line 788: # We are called with blank pool uuid, to avoid changing 
exiting


https://gerrit.ovirt.org/#/c/63270/2/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 692: return pv
Line 693: 
Line 694: 
Line 695: def isSamePV(pv_a, pv_b):
Line 696: return _fqpvname(pv_a) == _fqpvname(pv_b)
I don't like this normalization orgy. The module should accept only guid or 
only path device, but not both. These kind of helpers hide bugs in caller code.
Line 697: 
Line 698: 
Line 699: def _createpv(devices, metadataSize, options=tuple()):
Line 700: """


Line 889: 
Line 890: def _hasDataToMove(pvName):
Line 891: pv = getPV(pvName)
Line 892: if pv.pe_alloc_count == "0":
Line 893: return False
This helper is not very useful in this module, so better inline it in the 
function.
Line 894: 
Line 895: return True
Line 896: 
Line 897: def movePV(vgName, guid):


Line 901: Raises se.CouldNotMovePhysicalVolumeData if pvmove fails
Line 902: """
Line 903: pvName = _fqpvname(guid)
Line 904: if not _hasDataToMove(pvName):
Line 905: return
Inlining _hasDataToMove is more clear:

pv = getPV(pvName)
if pv.pe_alloc_count == "0":
return
Line 906: 
Line 907: cmd = ["pvmove", pvName]
Line 908: rc, out, err = _lvminfo.cmd(cmd)
Line 909: if rc != 0:


Line 904: if not _hasDataToMove(pvName):
Line 905: return
Line 906: 
Line 907: cmd = ["pvmove", pvName]
Line 908: rc, out, err = _lvminfo.cmd(cmd)
We must use filter including the devices belonging to this vg. Otherwise lvm 
will go and read all the devices in the system, which is very slow.

This should work:

... = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName,)))
Line 909: if rc != 0:
Line 910: raise se.CouldNotMovePhysicalVolumeData(pvName, err)
Line 911: 
Line 912: _lvminfo._invalidatepvs(pvName)


Line 906: 
Line 907: cmd = ["pvmove", pvName]
Line 908: rc, out, err = _lvminfo.cmd(cmd)
Line 909: if rc != 0:
Line 910: raise se.CouldNotMovePhysicalVolumeData(pvName, err)
Exception name is too long, can we shorten it?
Line 911: 
Line 912: _lvminfo._invalidatepvs(pvName)
Line 913: _lvminfo._invalidatevgs(vgName)
Line 914: 


Line 909: if rc != 0:
Line 910: raise se.CouldNotMovePhysicalVolumeData(pvName, err)
Line 911: 
Line 912: _lvminfo._invalidatepvs(pvName)
Line 913: _lvminfo._invalidatevgs(vgName)
We need to invalidate all lvs in to this vg, since some of them has moved to 
another pv.

This should work:

_lvminfo._invalidatelvs(vgName)
Line 914: 
Line 915: 
Line 916: def getVG(vgName):
Line 917: vg = _lvminfo.getVg(vgName)  # returns single VG namedtuple


https://gerrit.ovirt.org/#/c/63270/2/vdsm/storage/sd.py
File vdsm/storage/sd.py:

Line 367: def resize

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 2:

(5 comments)

https://gerrit.ovirt.org/#/c/63270/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 545
Line 546
Line 547
Line 548
Line 549
Same...


Line 552
Line 553
Line 554
Line 555
Line 556
Same issue with successful lvm command and failure to update mapping in old 
code...


Line 559: 
Line 560: def movePV(self, guid):
Line 561: self._validateNotMetadataPV(guid)
Line 562: with self._extendlock:
Line 563: lvm.movePV(self.sdUUID, guid)
updateMapping should be needed only for domain version 0 - do we want to 
support removing devices on such domains? I guess not.

If we want to support old domains, we must run update mapping even if movePV 
failed, since some extents may have moved, changing the pv mapping.

Note that if both movePV and updateMapping failed, we should raise the original 
error from movePV, with the original traceback.

We can do this:

try:
lvm.movePV(self.sdUUID, guid)
except Exception:
exc = sys.exc_info()
else:
exc = None
 
try:
self.updateMapping()
except Exception:
if exc is None:
raise
log.exception("descrption...")

if exc:
try:
six.reraise(*exc)
finally:
del exc

If updateMapping failed, we are left with incorrect mapping, not sure the 
current code can handle this case.
Line 564: self.updateMapping()
Line 565: 
Line 566: def reduceDevice(self, guid):
Line 567: self._validateNotMetadataPV(guid)


Line 565: 
Line 566: def reduceDevice(self, guid):
Line 567: self._validateNotMetadataPV(guid)
Line 568: with self._extendlock:
Line 569: lvm.reduceVG(self.sdUUID, guid)
No error handling here is fine, assuming that reduceVG remove the pv only if it 
was successful.

However, what engine is going to do if reduceVG succeed, but updateMapping 
failed? the call will fail, but the pv was removed.
Line 570: self.updateMapping()
Line 571: 
Line 572: def _validateNotMetadataPV(self, guid):
Line 573: lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)


Line 569: lvm.reduceVG(self.sdUUID, guid)
Line 570: self.updateMapping()
Line 571: 
Line 572: def _validateNotMetadataPV(self, guid):
Line 573: lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
Why the lvm_ prefix? do we have other kind of metadata_pv?
Line 574: if lvm.isSamePV(lvm_metadata_pv, guid):
Line 575: raise 
se.CouldNotPerformOperationOnStorageDomainMetadataPV(guid)
Line 576: 
Line 577: def getVolumeClass(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

2016-09-04 Thread laravot 
Liron Aravot has posted comments on this change.

Change subject: implementing StorageDomain.movePV
..


Patch Set 1:

(1 comment)

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

Line 1119: # Now add blockSD specific data
Line 1120: vg = lvm.getVG(self.sdUUID)  # vg.name = self.sdUUID
Line 1121: info['vguuid'] = vg.uuid
Line 1122: info['state'] = vg.partial
Line 1123: info['metadatadevice'], _ = lvm.getFirstExt(self.sdUUID, 
sd.METADATA)
> Are you sure this is part of this patch? Seems to belong to the top patch i
Done
Line 1124: return info
Line 1125: 
Line 1126: def getStats(self):
Line 1127: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 1: Code-Review-1

(1 comment)

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

Line 1119: # Now add blockSD specific data
Line 1120: vg = lvm.getVG(self.sdUUID)  # vg.name = self.sdUUID
Line 1121: info['vguuid'] = vg.uuid
Line 1122: info['state'] = vg.partial
Line 1123: info['metadatadevice'], _ = lvm.getFirstExt(self.sdUUID, 
sd.METADATA)
Are you sure this is part of this patch? Seems to belong to the top patch in 
this series.
Line 1124: return info
Line 1125: 
Line 1126: def getStats(self):
Line 1127: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

2016-09-04 Thread Nir Soffer 
Hello Liron Aravot,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/63270

to review the following change.

Change subject: implementing StorageDomain.movePV
..

implementing StorageDomain.movePV

Change-Id: I74183d13061d114a59da23874c86186457046e94
Signed-off-by: Liron Aravot 
---
M client/vdsClient.py
M lib/vdsm/storage/exception.py
M vdsm.spec.in
M vdsm/storage/blockSD.py
M vdsm/storage/hsm.py
M vdsm/storage/lvm.py
M vdsm/storage/sd.py
M vdsm/storage/sdm/api/Makefile.am
A vdsm/storage/sdm/api/move_sd_device_data.py
9 files changed, 134 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/63270/1

diff --git a/client/vdsClient.py b/client/vdsClient.py
index dde8b87..90a1846 100755
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -831,6 +831,18 @@
 printDict(res, self.pretty)
 return 0, ''
 
+def movePV(self, args):
+validateArgTypes(args, [str, str, str], requiredArgsNumber=3)
+jobUUID = args[0]
+sdUUID = args[1]
+guid = args[2]
+res = self.s.movePV(jobUUID, sdUUID, guid)
+if res['status']['code']:
+return res['status']['code'], res['status']['message']
+del res['status']
+printDict(res, self.pretty)
+return 0, ''
+
 def discoverST(self, args):
 portal = args[0].split(":")
 ip = portal[0]
@@ -2335,6 +2347,11 @@
   'been resized in storage server, this method will cause '
   'the PV to use the entire size of the block device'
   )),
+'movePV': (serv.movePV,
+ ('  ',
+  'Moves data stored in a PV of the given Storage Domain 
to other '
+  'PVs of the Storage Domain'
+  )),
 'discoverST': (serv.discoverST,
('ip[:port] [[username password] [auth=]]',
 'Discover the available iSCSI targetnames on a '
diff --git a/lib/vdsm/storage/exception.py b/lib/vdsm/storage/exception.py
index 23e2628..33f28a5 100644
--- a/lib/vdsm/storage/exception.py
+++ b/lib/vdsm/storage/exception.py
@@ -1556,6 +1556,18 @@
 code = 615
 message = "Could not resize PV"
 
+class CouldNotPerformOperationOnStorageDomainMetadataPV(StorageException):
+def __init__(self, pvname):
+self.value = "pvname=%s" % pvname
+code = 616
+message = "Could not perform the operation on the Storage Domain  " \
+  "metadata PV"
+
+class CouldNotMovePhysicalVolumeData(StorageException):
+def __init__(self, pvname, err):
+self.value = "pvname=%s err=%s" % (pvname, err)
+code = 617
+message = "Could not move PV data"
 
 #
 #  SPM/HSM Exceptions
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 9f85c20..5c46d80 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1011,6 +1011,7 @@
 %{_datadir}/%{vdsm_name}/storage/sdm/api/__init__.py*
 %{_datadir}/%{vdsm_name}/storage/sdm/api/base.py*
 %{_datadir}/%{vdsm_name}/storage/sdm/api/copy_data.py*
+%{_datadir}/%{vdsm_name}/storage/sdm/api/move_sd_device_data.py*
 %{_datadir}/%{vdsm_name}/storage/sdm/api/create_volume.py*
 %{_libexecdir}/%{vdsm_name}/spmprotect.sh
 %{_libexecdir}/%{vdsm_name}/spmstop.sh
diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 11191cd..20ae454 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -557,6 +557,23 @@
 newsize = self.metaSize(self.sdUUID)
 lvm.extendLV(self.sdUUID, sd.METADATA, newsize)
 
+def movePV(self, guid):
+self._validateNotMetadataPV(guid)
+with self._extendlock:
+lvm.movePV(self.sdUUID, guid)
+self.updateMapping()
+
+def reduceDevice(self, guid):
+self._validateNotMetadataPV(guid)
+with self._extendlock:
+lvm.reduceVG(self.sdUUID, guid)
+self.updateMapping()
+
+def _validateNotMetadataPV(self, guid):
+lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
+if lvm.isSamePV(lvm_metadata_pv, guid):
+raise se.CouldNotPerformOperationOnStorageDomainMetadataPV(guid)
+
 def getVolumeClass(self):
 """
 Return a type specific volume generator object
@@ -1103,6 +1120,7 @@
 vg = lvm.getVG(self.sdUUID)  # vg.name = self.sdUUID
 info['vguuid'] = vg.uuid
 info['state'] = vg.partial
+info['metadatadevice'], _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
 return info
 
 def getStats(self):
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index fedaddd..d9b6758 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -81,6 +81,7 @@
 
 import sdm.api.create_volume
 import sdm.api.copy_data
+import sdm.api.move_sd_device_data
 
 GUID = "guid"
 NAME = "name"
@@ -780,7 +781,8 @

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

2016-08-30 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: implementing StorageDomain.movePV
..


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

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

Change subject: implementing StorageDomain.movePV
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: implementing StorageDomain.movePV

2016-08-24 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: implementing StorageDomain.movePV
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74183d13061d114a59da23874c86186457046e94
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org