Change in vdsm[master]: StorageDomain.getInfo - report vg metadata device for block sd
Liron Aravot has posted comments on this change.
Change subject: StorageDomain.getInfo - report vg metadata device for block sd
..
Patch Set 7:
(1 comment)
https://gerrit.ovirt.org/#/c/64433/7/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:
Line 1341: pvs = _lvminfo.getVgPvs(vgName)
Line 1342: mdpvs = [pv for pv in pvs if not isinstance(pv, Stub) and
_isMetadataPv(pv)]
Line 1343: if len(mdpvs) != 1:
Line 1344: se.UnexpectedVolumeGroupMetadata("Expected one metadata pv
in vg: %s, "
Line 1345: "vg pvs: %s" % (vgName,
pvs))
> Errors looks fine, raising it would be even better :-)
Yes..you've got a point :)
Line 1346: return mdpvs[0]
Line 1347:
Line 1348: def _isMetadataPv(pv):
Line 1349: """
--
To view, visit https://gerrit.ovirt.org/64433
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: StorageDomain.getInfo - report vg metadata device for block sd
Liron Aravot has posted comments on this change.
Change subject: StorageDomain.getInfo - report vg metadata device for block sd
..
Patch Set 7:
(4 comments)
https://gerrit.ovirt.org/#/c/64433/7/lib/vdsm/storage/exception.py
File lib/vdsm/storage/exception.py:
Line 1558:
Line 1559:
Line 1560: class UnexpectedVolumeGroupMetadata(StorageException):
Line 1561: def __init__(self, err):
Line 1562: self.value = "err=%s" % err
> reason=?
Done
Line 1563: code = 616
Line 1564: message = "Volume Group metadata isn't as expected"
Line 1565:
Line 1566:
https://gerrit.ovirt.org/#/c/64433/7/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 451: """
Line 452: dev, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
Line 453: return os.path.basename(dev)
Line 454:
Line 455: def getVgMetadataDevice(self):
> Add docstring similar to getMetadataLVDevice (see my comment in the previou
Done
Line 456: return os.path.basename(lvm.getVgMetadataPv(self.sdUUID))
Line 457:
Line 458: @classmethod
Line 459: def getMetaDataMapping(cls, vgName, oldMapping={}):
https://gerrit.ovirt.org/#/c/64433/7/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:
Line 588: return vgs.values()
Line 589:
Line 590: def getVgPvs(self, vgName):
Line 591: """
Line 592: Returns the pvs of the given vg
> ... reloading pvs only once if some of the pvs are missing or stale.
1. Done
2. moved and renamed to getPvs
Line 593: """
Line 594: stalepvs = []
Line 595: vgpvs = []
Line 596: vg = self.getVg(vgName)
Line 594: stalepvs = []
Line 595: vgpvs = []
Line 596: vg = self.getVg(vgName)
Line 597: for pvName in vg.pv_name:
Line 598: pv = self._pvs.get(pvName)
> One issue here, you are accessing a dict which may be modified by other the
I'm not sure that's needed.
Currently the methods that take the lock are the one's the modify the dicts and
the objects in them - so that look is essentially a write lock only (except
GetVGDevs).
In this method we don't modify the dict, only _reloadpvs does and it takes the
lock when it does.
acquiring the lock here means that we'll here on the different operations - do
we really need it? I can see that you actually changed the lock to be a write
lock in change I86153c34c3ddc4055bb0679ce46aa48757d9cae1 :)
I'd start by adding the method as is, keeping the same semantics - if we'll see
its needed we can add it (though i believe it doesn't).
Line 599: if pv is None or isinstance(pv, Stub):
Line 600: stalepvs.append(pvName)
Line 601: else:
Line 602: vgpvs.append(pv)
--
To view, visit https://gerrit.ovirt.org/64433
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
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
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report the first pv of the metadata lv .. Patch Set 12: (4 comments) https://gerrit.ovirt.org/#/c/63027/12/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml: Line 5653: type: *UUID Line 5654: Line 5655: - defaultvalue: null Line 5656: description: The GUID of the first device containing the domain Line 5657: metadata lv for block storage domains (optional) > Same as next patch, this is available on block storage, and never available Done Line 5658: name: metadataDevice Line 5659: type: string Line 5660: type: object Line 5661: Line 5654: Line 5655: - defaultvalue: null Line 5656: description: The GUID of the first device containing the domain Line 5657: metadata lv for block storage domains (optional) Line 5658: name: metadataDevice > +1 Thanks Adam, I get your point, but i'm not really in favor of making this change for few reasons: 1. I do hate the fact that we have two returned values here, but that's a result of the current bug that we don't create the metadata lv in such way that guarantees that it'll be on the vg metadata device. After that will be fixed, we'll be able to return and save in the engine only one device rather than preparing ourselves for a list of devices here and on the engine side. 2. if we'll ever remove the limitation to perform operations on "one" of the metadata devices - it's very easy to reflect it on the engine (as it knows why each device is immutable) rather then check the immutable devices list against vdsm each time. To sum up, i see reasons for both approaches, I currently prefer to leave this one - what do you guys think? Line 5659: type: string Line 5660: type: object Line 5661: Line 5662: StorageDomainStatus: &StorageDomainStatus https://gerrit.ovirt.org/#/c/63027/12/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: PS12, Line 448: getMetadataLVDevice > This can be an internal function. this function is in the StorageDomainManifest, used by StorageDomain.getInfo() Line 446: lvm.extendLV(self.sdUUID, volumeUUID, size) # , isShuttingDown) Line 447: Line 448: def getMetadataLVDevice(self): Line 449: """ Line 450: Returns the first device of the domain metadata lv > Returns the devices the metadata lv was created on. 1. "Returns the devices the metadata lv was created on."- can you explain this part? we always return the device of the first extant. 2. Done. Line 451: """ Line 452: dev, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA) Line 453: return os.path.basename(dev) Line 454: -- To view, visit https://gerrit.ovirt.org/63027 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b Gerrit-PatchSet: 12 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 To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report lvm metadata device for block sd .. Patch Set 5: (8 comments) https://gerrit.ovirt.org/#/c/64433/5/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml: Line 5659: type: string Line 5660: Line 5661: - defaultvalue: null Line 5662: description: The GUID of the device containing the domain lvm metadata (optional) Line 5663: name: lvmmetadatadevice > vgMetadataDevice Done Line 5664: type: string Line 5665: type: object Line 5666: Line 5667: StorageDomainStatus: &StorageDomainStatus https://gerrit.ovirt.org/#/c/64433/5/lib/vdsm/storage/exception.py File lib/vdsm/storage/exception.py: Line 1558: Line 1559: Line 1560: class CannotGetVGMetadataPV(StorageException): Line 1561: def __init__(self, vgname): Line 1562: self.value = "vgname=%s" % (vgname) > Expected one pv with pv metadata, vgname=%s, pvs=%s As this error is intended to be "general" - the code that raises will pass the exact error. Line 1563: code = 616 Line 1564: message = "Cannot get Volume Group metadata PV" Line 1565: Line 1566: https://gerrit.ovirt.org/#/c/64433/5/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 1109 Line 1110 Line Line 1112 Line 1113 > Take this Done Line 1116: <<< HEAD Line 1117: info['metadatadevice'] = self._manifest.getMetadataLVDevice() Line 1118: === Line 1119: info['metadatadevice'] = self._manifest.getMetadataLVFirstDevice() Line 1120: info['lvmmetadatadevice'] = self._manifest.getVgMetadataDevice() > Use mixed case name, already used in this verb response. Done Line 1121: >>> 7da51c9... StorageDomain.getInfo - report lvm metadata device for block sd Line 1122: return info Line 1123: Line 1124: def getStats(self): https://gerrit.ovirt.org/#/c/64433/5/vdsm/storage/lvm.py File vdsm/storage/lvm.py: Line 585 Line 586 Line 587 Line 588 Line 589 > Let put the new method here, more specific. Done Line 502: def _invalidatevgs(self, vgNames): Line 503: vgNames = _normalizeargs(vgNames) Line 504: with self._lock: Line 505: for vgName in vgNames: Line 506: self._vgs[vgName] = aStub(vgName, True) > Not related. Done Line 507: Line 508: def _invalidateAllVgs(self): Line 509: with self._lock: Line 510: self._stalevg = True Line 555: reloaded = self._reloadpvs(stalepvs) Line 556: pvs.update(reloaded) Line 557: return pvs.values() Line 558: Line 559: def getVgPvs(self, vgName): > Add docsting explaining this new function compared with getPv. Done Line 560: stalepvs = [] Line 561: vgpvs = [] Line 562: vg = self.getVg(vgName) Line 563: for pvName in vg.pv_name: Line 1337: def getVgMetadataPv(vgName): Line 1338: pvs = _lvminfo.getVgPvs(vgName) Line 1339: mdpvs = [pv for pv in pvs if not isinstance(pv, Stub) and _isMetadataPv(pv)] Line 1340: if len(mdpvs) != 1: Line 1341: se.CannotGetVGMetadataPV(vgName) > "Expected one metadata pv in vg: %s, found: %s" % (vg.name, pvs) Done Line 1342: return mdpvs[0] Line 1343: Line 1344: def _isMetadataPv(pv): Line 1345: """ -- To view, visit https://gerrit.ovirt.org/64433 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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 To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report the first pv of the metadata lv .. Patch Set 9: (2 comments) https://gerrit.ovirt.org/#/c/63027/9/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml: Line 5653: type: *UUID Line 5654: Line 5655: - defaultvalue: null Line 5656: description: The GUID of the first device containing the domain Line 5657: metadata lv (optional) > Always available only on block storage. Done Line 5658: name: metadatadevice Line 5659: type: string Line 5660: type: object Line 5661: Line 5654: Line 5655: - defaultvalue: null Line 5656: description: The GUID of the first device containing the domain Line 5657: metadata lv (optional) Line 5658: name: metadatadevice > metadataDevice Done Line 5659: type: string Line 5660: type: object Line 5661: Line 5662: StorageDomainStatus: &StorageDomainStatus -- To view, visit https://gerrit.ovirt.org/63027 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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 To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report lvm metadata device for block sd .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/64433/4/vdsm/storage/lvm.py File vdsm/storage/lvm.py: Line 556: pvs.update(reloaded) Line 557: return pvs.values() Line 558: Line 559: def getVgPvNames(self, vgName): Line 560: return self.getVg(vgName).pv_name > We can work on refactoring this class later if you think such helper is nee Well, i use this method in the following patch - so i assume i'll return it (see line 913 in https://gerrit.ovirt.org/#/c/63270/8/vdsm/storage/lvm.py) Line 561: Line 562: def getVgPvs(self, vgName): Line 563: stalepvs = [] Line 564: vgpvs = [] -- To view, visit https://gerrit.ovirt.org/64433 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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 To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report lvm metadata device for block sd .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/64433/4/vdsm/storage/lvm.py File vdsm/storage/lvm.py: Line 1343: return pv Line 1344: except AttributeError: Line 1345: pass Line 1346: Line 1347: raise se.CannotGetVGMetadataPV(vgName) Do we want to have a general exception or to use that specific one? Line 1348: Line 1349: Line 1350: def _isMetadataPv(pv): Line 1351: return pv.mda_used_count == '2' -- To view, visit https://gerrit.ovirt.org/64433 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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 To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report lvm metadata device for block sd .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/64433/4/vdsm/storage/lvm.py File vdsm/storage/lvm.py: Line 1341: try: Line 1342: if _isMetadataPv(pv): Line 1343: return pv Line 1344: except AttributeError: Line 1345: pass > 1. Done, i see your point. elaboration to b: What i'm trying to say is that we do check the pvs here, but just to get "information" related to the vg (we are trying to understand what pv stored its metadata). Therefore if we manage to get that info, it seems better to not raise. Line 1346: Line 1347: raise se.CannotGetVGMetadataPV(vgName) Line 1348: Line 1349: -- To view, visit https://gerrit.ovirt.org/64433 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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 To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report lvm metadata device for block sd .. Patch Set 4: (6 comments) https://gerrit.ovirt.org/#/c/64433/4/vdsm/storage/lvm.py File vdsm/storage/lvm.py: Line 556: pvs.update(reloaded) Line 557: return pvs.values() Line 558: Line 559: def getVgPvNames(self, vgName): Line 560: return self.getVg(vgName).pv_name > I don't think we need this helper, getting a vg and accessing pv.name is pe I don't agree as this code is already repeated, but Done. Line 561: Line 562: def getVgPvs(self, vgName): Line 563: stalepvs = [] Line 564: vgpvs = [] Line 563: stalepvs = [] Line 564: vgpvs = [] Line 565: for pvName in self.getVgPvNames(vgName): Line 566: pv = self._pvs.get(pvName) Line 567: if pv is None or isinstance(pv, Stub): > Need to check what is the difference between None and Stub - do we need to That's the standard way we check the pvs in this file. Let's use it here as well - if we find out its unnecessary let's change all the occurrences at once. Line 568: stalepvs.append(pvName) Line 569: else: Line 570: vgpvs.append(pv) Line 571: Line 568: stalepvs.append(pvName) Line 569: else: Line 570: vgpvs.append(pv) Line 571: Line 572: vgpvs.extend(self._reloadpvs(pvName=stalepvs).values()) > This line is doing too much, this style is a recipe for bugs, lets write si Done Line 573: return vgpvs Line 574: Line 575: def getVg(self, vgName): Line 576: # Get specific VG Line 1340: for pv in pvs: Line 1341: try: Line 1342: if _isMetadataPv(pv): Line 1343: return pv Line 1344: except AttributeError: > This code is trying to ignore unreadable pv, assuming that unreadable pv do Checking for the specific case is obviously better, I've checked for AttributeError as that's the standard way we handle that scenario all across that file. Line 1345: pass Line 1346: Line 1347: raise se.CannotGetVGMetadataPV(vgName) Line 1348: Line 1341: try: Line 1342: if _isMetadataPv(pv): Line 1343: return pv Line 1344: except AttributeError: Line 1345: pass > This should work when the vg is ok, but if some other code or lvm itself wi 1. Done, i see your point. 2. I'm not sure about fail fast. few reasons: a. we extend the failure cases for this method opposed for today - I'm not sure it's desired (StorageDomain.getInfo() will fail when one pv is unreachable) and maintains BC. b. currently all the methods here doesn't fail fast (for example, when failing to load vg/pv/lv we don't raise but use the Unreadable object) - so it makes sense that this method will be written using the same approach. Line 1346: Line 1347: raise se.CannotGetVGMetadataPV(vgName) Line 1348: Line 1349: Line 1346: Line 1347: raise se.CannotGetVGMetadataPV(vgName) Line 1348: Line 1349: Line 1350: def _isMetadataPv(pv): > Please add docstring explaining this and referring to the place configuring 1. I'll add the docstring. 2. The approach used in this file is to try to access the pv object, if it is an instance of Unreadable/Stub it will raise AttributeError. I'd say that if we want to raise UnreachablePhysicalVolume we should modify the Unreadable class (because that's why it was added, for unified behavior) and not to add special treatment to each method. Line 1351: return pv.mda_used_count == '2' Line 1352: Line 1353: Line 1354: def listPVNames(vgName): -- To view, visit https://gerrit.ovirt.org/64433 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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 To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report the first pv of the metadata lv .. Patch Set 8: (1 comment) https://gerrit.ovirt.org/#/c/63027/8/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 444: with self._extendlock: Line 445: # FIXME: following line. Line 446: lvm.extendLV(self.sdUUID, volumeUUID, size) # , isShuttingDown) Line 447: Line 448: def getMetadataLVFirstDevice(self): > getMetadataLVDevice? getMetadataLVPV? Done Line 449: dev, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA) Line 450: return os.path.basename(dev) Line 451: Line 452: @classmethod -- To view, visit https://gerrit.ovirt.org/63027 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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 To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: implementing SDM.reduce_domain_device
Liron Aravot has uploaded a new change for review.
Change subject: implementing SDM.reduce_domain_device
..
implementing SDM.reduce_domain_device
Change-Id: Id5d3f93da11edcb626c2f050d4ce2856aa8faca0
Signed-off-by: Liron Aravot
---
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/reduce_device.py
8 files changed, 139 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/64521/1
diff --git a/lib/vdsm/storage/exception.py b/lib/vdsm/storage/exception.py
index 73c0e64..3993799 100644
--- a/lib/vdsm/storage/exception.py
+++ b/lib/vdsm/storage/exception.py
@@ -1332,6 +1332,12 @@
code = 518
message = "Device block size is not supported"
+class VolumeGroupReduceError(StorageException):
+def __init__(self, vgname, pvname, err):
+self.value = "vgname=%s pvname=%s err=%s" % (vgname, pvname, err)
+code = 519
+message = "Cannot reduce the Volume Group"
+
class CannotCreateLogicalVolume(StorageException):
code = 550
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 57e3a9b..883b159 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1012,6 +1012,7 @@
%{_datadir}/%{vdsm_name}/storage/sdm/api/base.py*
%{_datadir}/%{vdsm_name}/storage/sdm/api/copy_data.py*
%{_datadir}/%{vdsm_name}/storage/sdm/api/move_device.py*
+%{_datadir}/%{vdsm_name}/storage/sdm/api/reduce_device.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 424b8c8..fe3ca91 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -583,6 +583,36 @@
with self._extendlock:
lvm.movePV(self.sdUUID, src_device, dst_devices)
+def reduceVG(self, guid):
+lvm.validatePvsPartOfVG([guid], self.sdUUID)
+self._manifest.validateNotFirstMetadataLVDevice(guid)
+self._manifest.validateNotVgMetadataDevice(guid)
+with self._extendlock:
+try:
+lvm.reduceVG(self.sdUUID, guid)
+except Exception:
+exc = sys.exc_info()
+else:
+exc = None
+
+try:
+self.updateMapping()
+except Exception:
+if exc is None:
+raise
+log.exception("Failed to update the domain metadata mapping")
+
+if exc:
+try:
+six.reraise(*exc)
+finally:
+del exc
+
+try:
+lvm.removePV(guid)
+except Exception:
+log.exception("Failed to remove pv on device %s, ignoring" % guid)
+
def getVolumeClass(self):
"""
Return a type specific volume generator object
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index ce67f98..c528195 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -82,6 +82,7 @@
import sdm.api.create_volume
import sdm.api.copy_data
import sdm.api.move_device
+import sdm.api.reduce_device
GUID = "guid"
NAME = "name"
@@ -3565,4 +3566,13 @@
:param guid: A block device GUID
:type guid: str
"""
-raise NotImplementedError()
\ No newline at end of file
+# a host is being with a host id in a storage pool when it's being
+# connected to a pool.
+# This verb can be executed by hosts that aren't connected to a pool
+# and to be executed on domain that isn't part of the pool - therefore
+# we can't rely on having a usable and meaningful host id for the
operation.
+# using 1 as the host id is good enough as this verb should be
executed only on
+# storage domains that aren't being accessed by any other host -
therefore this id
+# shouldn't be in use by other host.
+job = sdm.api.reduce_device.Job(job_id, 1, sd_id, guid)
+self.sdm_schedule(job)
\ No newline at end of file
diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index a8288eb..68e8865 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -938,6 +938,18 @@
if rc != 0:
raise se.CouldNotMovePVData(pvName, err)
+def removePV(guid):
+"""
+Removes the lvm metadata from an unused pv
+
+Raises se.CouldNotRemovePhysicalVolume if pvremove fails
+"""
+pvName = _fqpvname(guid)
+cmd = ["pvremove", pvName]
+rc, out, err = _lvminfo.cmd(cmd)
+if rc != 0:
+log.warning("Failed to execute pvremove on pv %s, err %s ", guid, err)
+_lvminfo._invalidatepvs(pvName)
def getVG(vgName):
vg = _lvminfo.getVg(vgName) # returns single VG namedtuple
@@ -1057,6 +1069,15 @@
else:
raise se.VolumeGroupExtendError(vgName, pvs)
+def r
Change in vdsm[master]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has restored this change. Change subject: StorageDomain.getInfo - report lvm metadata device for block sd .. Restored -- To view, visit https://gerrit.ovirt.org/64433 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: restore Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI 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]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has abandoned this change. Change subject: StorageDomain.getInfo - report lvm metadata device for block sd .. Abandoned -- To view, visit https://gerrit.ovirt.org/64433 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI 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]: Define the SDM.move_domain_device API
Liron Aravot has uploaded a new change for review. Change subject: Define the SDM.move_domain_device API .. Define the SDM.move_domain_device API This patch defines the SDM.move_domain_device API that will be used to move data from a block domain device to other devices that are part of the domain. This API will be used currently prior to reducing a device from a storage domain. Change-Id: If6dce392d4cf157c42a47f70e972fa5d1d9dbdc2 Signed-off-by: Liron Aravot --- M lib/api/vdsm-api.yml M vdsm/API.py M vdsm/storage/hsm.py 3 files changed, 42 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/34/64434/1 diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml index 02e1ede..7a59112 100644 --- a/lib/api/vdsm-api.yml +++ b/lib/api/vdsm-api.yml @@ -7070,7 +7070,6 @@ values: - *CopyDataDIVEndpoint - Host.setupNetworks: added: '3.1' description: Reconfigure host networking by adding, removing, and editing @@ -9874,3 +9873,25 @@ - description: The destination endpoint name: destination type: *CopyDataEndpoint + +SDM.move_domain_device: +added: '4.1' +description: Moves the data stored on a Storage Domain block device PV to other PVs that are part of the domain. +params: +- description: A UUID to be used for tracking the job progress +name: job_id +type: *UUID + +- description: The UUID of the Storage Domain +name: sd_id +type: *UUID + +- description: The GUID of block device to move the data from +name: src_guid +type: string + +- defaultvalue: null +description: Optional list of block devices GUIDs to move the data to +name: dst_guids +type: +- string diff --git a/vdsm/API.py b/vdsm/API.py index 57a30df..e2a163a 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -1636,3 +1636,7 @@ def copy_data(self, job_id, source, destination): return self._irs.sdm_copy_data(job_id, source, destination) + +def move_domain_device(self, job_id, sd_id, src_guid, dst_guids): +return self._irs.sdm_move_domain_device(job_id, sd_id, src_guid, +dst_guids) diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index d622ffc..3083042 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -3523,3 +3523,19 @@ def sdm_copy_data(self, job_id, source, destination): job = sdm.api.copy_data.Job(job_id, None, source, destination) self.sdm_schedule(job) + +@public +def sdm_move_domain_device(self, job_id , sd_id, src_guid, dst_guids): +""" +Moves the data stored on a PV to other PVs that are part of the Storage Domain. + +:param job_id: The UUID of the job. +:type job_id: UUID +:param sd_id: The UUID of the storage domain that owns the PV. +:type sd_id: UUID +:param src_guid: The GUID of block device to move the data from +:type src_guid: str +:param dst_guids: Optional list of block devices GUIDs to move the data to +:type dst_guids: str +""" +raise NotImplementedError() -- To view, visit https://gerrit.ovirt.org/64434 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If6dce392d4cf157c42a47f70e972fa5d1d9dbdc2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Nir Soffer ___ 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]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has uploaded a new change for review.
Change subject: StorageDomain.getInfo - report lvm metadata device for block sd
..
StorageDomain.getInfo - report lvm metadata device for block sd
When a block storage domain is created, we disable the lvm metadata
on all the domain pvs except to the first one.
As we intend to add support for operations that modify the vg
stracture (like reducing devices) - we want to prevent those
operations from being made on the device containing the vg metadata.
This patch adds the reporting of that info, so that the engine can
leverage the info to block such operations.
Change-Id: I4a7763d2ab7d796be633ecd69f661cba96e29dde
Signed-off-by: Liron Aravot
---
M lib/api/vdsm-api.yml
M lib/vdsm/storage/exception.py
M vdsm/storage/blockSD.py
M vdsm/storage/lvm.py
4 files changed, 48 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/33/64433/1
diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml
index 8168fbb..02e1ede 100644
--- a/lib/api/vdsm-api.yml
+++ b/lib/api/vdsm-api.yml
@@ -5653,6 +5653,11 @@
type: *UUID
- defaultvalue: null
+description: The GUID of the device containing the domain lvm
metadata (optional)
+name: lvmmetadatadevice
+type: string
+
+- defaultvalue: null
description: The GUID of the first device containing the domain
metadata lv (optional)
name: metadatadevice
type: string
diff --git a/lib/vdsm/storage/exception.py b/lib/vdsm/storage/exception.py
index 23e2628..743c5d9 100644
--- a/lib/vdsm/storage/exception.py
+++ b/lib/vdsm/storage/exception.py
@@ -1556,6 +1556,11 @@
code = 615
message = "Could not resize PV"
+class CannotGetVGMetadataPV(StorageException):
+def __init__(self, vgname):
+self.value = "vgname=%s" % (vgname)
+code = 616
+message = "Cannot get Volume Group metadata PV"
#
# SPM/HSM Exceptions
diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index b164551..ec060c1 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -449,6 +449,9 @@
dev, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
return os.path.basename(dev)
+def getVgMetadataDevice(self):
+return os.path.basename(lvm.getVgMetadataPv(self.sdUUID))
+
@classmethod
def getMetaDataMapping(cls, vgName, oldMapping={}):
firstDev, firstExtent = lvm.getFirstExt(vgName, sd.METADATA)
@@ -1108,6 +,7 @@
info['vguuid'] = vg.uuid
info['state'] = vg.partial
info['metadatadevice'] = self._manifest.getMetadataLVFirstDevice()
+info['lvmmetadatadevice'] = self._manifest.getVgMetadataDevice()
return info
def getStats(self):
diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 3d3c31a..c0a5c1c 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -51,7 +51,7 @@
LVM_DEFAULT_TTL = 100
PV_FIELDS = ("uuid,name,size,vg_name,vg_uuid,pe_start,pe_count,"
- "pe_alloc_count,mda_count,dev_size")
+ "pe_alloc_count,mda_count,dev_size,mda_used_count")
VG_FIELDS = ("uuid,name,attr,size,free,extent_size,extent_count,free_count,"
"tags,vg_mda_size,vg_mda_free,lv_count,pv_count,pv_name")
LV_FIELDS = "uuid,name,vg_name,attr,size,seg_start_pe,devices,tags"
@@ -555,6 +555,25 @@
reloaded = self._reloadpvs(stalepvs)
pvs.update(reloaded)
return pvs.values()
+
+def _isMetadataPv(self, pv):
+return int(pv.pv_mda_used_count) > 0
+
+def getVgMetadataPv(self, vgName):
+stalepvs = []
+for pvName in self.listPVNames(vgName):
+pv = self._pvs.get(pvName)
+if not pv or isinstance(pv, Stub):
+stalepvs.append(pvName)
+if self._isMetadataPv(pv):
+return pv
+
+pvs = self._reloadpvs(pvName=stalepvs)
+for pv in pvs:
+if not isinstance(pv, Stub) and self._isMetadataPv(pv):
+return pv
+
+raise se.CouldNotResizePhysicalVolume(vgName)
def getVg(self, vgName):
# Get specific VG
@@ -1319,6 +1338,20 @@
return getLV(vg, lv).devices.strip(" )").split("(")
+def getVgMetadataPv(vgName):
+pvs = _lvminfo.getVgPvs(vgName)
+for pv in pvs:
+try:
+if _isMetadataPv(pv):
+return pv
+except AttributeError:
+pass
+
+raise se.CannotGetVGMetadataPV(vgName)
+
+def _isMetadataPv(pv):
+return pv.mda_used_count == '2'
+
def listPVNames(vgName):
try:
pvNames = _lvminfo._vgs[vgName].pv_name
--
To view, visit https://gerrit.ovirt.org/64433
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4a7763d2ab7d796be633ecd69f661c
Change in vdsm[master]: StorageDomain.getInfo - report the first pv of the metadata lv
Liron Aravot has uploaded a new change for review.
Change subject: StorageDomain.getInfo - report the first pv of the metadata lv
..
StorageDomain.getInfo - report the first pv of the metadata lv
This patch adds the report of the first pv of the domain metadata lv
(for block domains) to StorageDomain.getInfo().
Currently we assume that the metadata lv is created on the first extant
of the device and verify that when performing different operations on
the domain.
As we intend to add support for operations that modify the vg
stracture (like reducing devices) - we want to prevent those
operations from being made on the first device of the metadata lv
as they might cause the metadata lv first extant to be changed from
0.
This patch adds the reporting of that info, so that the engine can
leverage the info to block such operations.
Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Signed-off-by: Liron Aravot
---
M lib/api/vdsm-api.yml
M vdsm/storage/blockSD.py
2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/27/63027/7
diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml
index 514206d..8168fbb 100644
--- a/lib/api/vdsm-api.yml
+++ b/lib/api/vdsm-api.yml
@@ -5651,6 +5651,12 @@
- description: The Storage Domain UUID
name: uuid
type: *UUID
+
+- defaultvalue: null
+description: The GUID of the first device containing the domain
metadata lv (optional)
+name: metadatadevice
+type: string
+
type: object
StorageDomainStatus: &StorageDomainStatus
diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 11191cd..b164551 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -445,6 +445,10 @@
# FIXME: following line.
lvm.extendLV(self.sdUUID, volumeUUID, size) # , isShuttingDown)
+def getMetadataLVFirstDevice(self):
+dev, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA)
+return os.path.basename(dev)
+
@classmethod
def getMetaDataMapping(cls, vgName, oldMapping={}):
firstDev, firstExtent = lvm.getFirstExt(vgName, sd.METADATA)
@@ -1103,6 +1107,7 @@
vg = lvm.getVG(self.sdUUID) # vg.name = self.sdUUID
info['vguuid'] = vg.uuid
info['state'] = vg.partial
+info['metadatadevice'] = self._manifest.getMetadataLVFirstDevice()
return info
def getStats(self):
--
To view, visit https://gerrit.ovirt.org/63027
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
Gerrit-Reviewer: Liron Aravot
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]: implementing StorageDomain.reduce
Liron Aravot has posted comments on this change. Change subject: implementing StorageDomain.reduce .. Patch Set 3: (2 comments) https://gerrit.ovirt.org/#/c/63269/3/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 567: self._validateNotMetadataPV(guid) Line 568: with self._extendlock: Line 569: lvm.reduceVG(self.sdUUID, guid) Line 570: self.updateMapping() Line 571: lvm.removePV(guid) > Why separate patch to remove the pv? Right, fixed. Line 572: Line 573: def _validateNotMetadataPV(self, guid): Line 574: lvm_metadata_pv, _ = lvm.getFirstExt(self.sdUUID, sd.METADATA) Line 575: if lvm.isSamePV(lvm_metadata_pv, guid): https://gerrit.ovirt.org/#/c/63269/3/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 754: :type sdUUID: UUID Line 755: :param guid: A block device GUID Line 756: :type guid: str Line 757: """ Line 758: job = sdm.api.reduce_sd_device.Job(jobUUID, 1, sdUUID, guid) > Document why host_id=1 is correct, see also comment in movePV patch. Done Line 759: self.sdm_schedule(job) Line 760: Line 761: @public Line 762: def resizePV(self, sdUUID, spUUID, guid, options=None): -- To view, visit https://gerrit.ovirt.org/63269 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c68f64b6c90f0af4d3fa4da94d5747b4ad9cfd6 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Liron Aravot 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]: Define the StorageDomain.reduce API
Liron Aravot has posted comments on this change. Change subject: Define the StorageDomain.reduce API .. Patch Set 7: (2 comments) https://gerrit.ovirt.org/#/c/62853/7/vdsm/API.py File vdsm/API.py: Line 1078: def extend(self, storagepoolID, devlist, force=False): Line 1079: return self._irs.extendStorageDomain(self._UUID, storagepoolID, Line 1080: devlist, force) Line 1081: Line 1082: def reduce(self, jobID, guid): > Please use the same name in all layers (jobID or jobUUID or job_id). the names are different across the layers for all the verbs as it seems - i added this one to match the current pattern. Line 1083: return self._irs.reduceStorageDomain(jobID, self._UUID, guid) Line 1084: Line 1085: def resizePV(self, storagepoolID, guid): Line 1086: return self._irs.resizePV(self._UUID, storagepoolID, guid) https://gerrit.ovirt.org/#/c/62853/7/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 747: """ Line 748: Reduce a block-based Storage Domain by removing a device from it. Line 749: Line 750: :param UUID: The UUID of the job. Line 751: :type UUID: UUID > UUID -> jobUUID Done Line 752: :param sdUUID: The UUID of the storage domain that owns the PV. Line 753: :type sdUUID: UUID Line 754: :param guid: A block device GUID Line 755: :type guid: str -- To view, visit https://gerrit.ovirt.org/62853 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic5e41b9fa2df4ffef1f3cbb9fbfc57022ffedd9a Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Kaul 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]: implementing StorageDomain.movePV
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
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]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report lvm metadata device for block sd .. Patch Set 3: i'll rebase it on top of master when we'll have +2 on it. -- To view, visit https://gerrit.ovirt.org/63027 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Liron Aravot 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]: StorageDomain.getInfo - report lvm metadata device for block sd
Liron Aravot has posted comments on this change. Change subject: StorageDomain.getInfo - report lvm metadata device for block sd .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/63027/1/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml: Line 5654: Line 5655: - defaultvalue: null Line 5656: description: The device containing the domain metadata volume (optional) Line 5657: name: metadatadevice Line 5658: type: *UUID > GUID or path - should be same info returned in the pv list. Done Line 5659: Line 5660: type: object Line 5661: Line 5662: StorageDomainStatus: &StorageDomainStatus -- To view, visit https://gerrit.ovirt.org/63027 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32c847ae89b9f8f512c3dd8a0fff96fbc753ee5b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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]: hsm: add getVGInfo discard related fields
Liron Aravot has posted comments on this change. Change subject: hsm: add getVGInfo discard related fields .. Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/62800/2/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 2807: info["vgUUID"] = str(pv.vg_uuid) Line 2808: info["pvUUID"] = str(pv.uuid) Line 2809: info["GUID"] = str(pv.guid) Line 2810: info["discard_max_bytes"] = devInfo["discard_max_bytes"] Line 2811: info["discard_zeroes_data"] = devInfo["discard_zeroes_data"] you need to update vdsm-api.yml as well. Line 2812: return info Line 2813: Line 2814: @deprecated Line 2815: @public -- To view, visit https://gerrit.ovirt.org/62800 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cdb535923f6f5ffc961750271e8cde823ed835a Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan 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]: Define the SDM.copy_data API
Liron Aravot has posted comments on this change. Change subject: Define the SDM.copy_data API .. Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/60419 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id90a39820141365b865e861e1f89704947452c78 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke 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 https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[ovirt-4.0]: sp: try lock when executing user initated pool upgrade
Liron Aravot has posted comments on this change. Change subject: sp: try lock when executing user initated pool upgrade .. Patch Set 1: Verified+1 -- To view, visit https://gerrit.ovirt.org/60520 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I93bfdcb19437fdc8901c08ea17dc80fe9c1e5510 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-4.0 Gerrit-Owner: Liron Aravot 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 https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[ovirt-4.0]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has posted comments on this change. Change subject: sp: allow executing upgradePool even if there is a pending update .. Patch Set 1: Verified+1 Rerun-hooks: all -- To view, visit https://gerrit.ovirt.org/60519 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-4.0 Gerrit-Owner: Liron Aravot 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 https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[ovirt-4.0]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has uploaded a new change for review.
Change subject: sp: allow executing upgradePool even if there is a pending
update
..
sp: allow executing upgradePool even if there is a pending update
Currently when a pool upgrade is initated (either by startSpm() or by
executing upgradePool() explicitly) the _domainsToUpgrade member of the
storage pool is initiated with the active domains, when each domain
upgrade is completed (or if the upgrade is unneeded) its id is removed
from that list.
When there is an unavailable domain, it'll remain on that list until its
update is complete - while it's unavailable other upgrade can't be
executed.
This patch changes that, we should allow upgrading the pool even if an
older upgrade wasn't complete yet - the domains that the old update
wasn't completed yet for will be upgraded to the new version.
Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Bug-Url: https://bugzilla.redhat.com/1260428
Signed-off-by: Liron Aravot
---
M vdsm/storage/sp.py
1 file changed, 4 insertions(+), 29 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/19/60519/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 4347d3c..24b7d2b 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -175,8 +175,8 @@
exc_info=True)
return
-with rmanager.acquireResource(sc.STORAGE, "upgrade_" + sdUUID,
- rm.LockType.exclusive):
+with rmanager.acquireResource(sc.STORAGE, "upgrade_" + self.spUUID,
+ rm.LockType.shared):
with rmanager.acquireResource(sc.STORAGE, sdUUID,
rm.LockType.exclusive):
if sdUUID not in self._domainsToUpgrade:
@@ -352,31 +352,13 @@
self.log.debug("Shutting down upgrade process")
with rmanager.acquireResource(sc.STORAGE, "upgrade_" + self.spUUID,
rm.LockType.exclusive):
-domains = self._domainsToUpgrade[:]
try:
self.domainMonitor.onDomainStateChange.unregister(
self._upgradeCallback)
except KeyError:
pass
-requests = []
-def cancelUpgrade(sdUUID, req, res):
-try:
-self._domainsToUpgrade.remove(sdUUID)
-except ValueError:
-pass
-
-res.release()
-
-for sdUUID in domains:
-req = rmanager.registerResource(sc.STORAGE,
-"upgrade_" + sdUUID,
-rm.LockType.exclusive,
-partial(cancelUpgrade, sdUUID))
-requests.append(req)
-
-for req in requests:
-req.wait()
+self._domainsToUpgrade = []
@classmethod
def cleanupMasterMount(cls):
@@ -440,9 +422,6 @@
def _upgradePool(self, targetDomVersion):
with rmanager.acquireResource(sc.STORAGE, "upgrade_" + self.spUUID,
rm.LockType.exclusive):
-if len(self._domainsToUpgrade) > 0:
-raise se.PoolUpgradeInProgress(self.spUUID)
-
sd.validateDomainVersion(targetDomVersion)
self.log.info("Trying to upgrade master domain `%s`",
self.masterDomain.sdUUID)
@@ -461,11 +440,7 @@
self.domainMonitor.onDomainStateChange.register(
self._upgradeCallback)
self.log.debug("Running initial domain upgrade threads")
-# We need to copy the list as the domain monitor registered
-# callback and the initiated update threads may modify the list
-# while we iterate over it.
-# http://bugzilla.redhat.com/1319523
-for sdUUID in self._domainsToUpgrade[:]:
+for sdUUID in self._domainsToUpgrade:
concurrent.thread(self._upgradeCallback,
args=(sdUUID, True),
kwargs={"__securityOverride": True},
--
To view, visit https://gerrit.ovirt.org/60519
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Liron Aravot
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[ovirt-4.0]: sp: try lock when executing user initated pool upgrade
Liron Aravot has uploaded a new change for review.
Change subject: sp: try lock when executing user initated pool upgrade
..
sp: try lock when executing user initated pool upgrade
When attempting to execute upgradeStoragePool a concurrent
update on one of the pool domains may be during execution and hold
the pool upgrade lock.
In that case, the request to acquire the lock will wait for the lock
to be released which may cause the request to take longer then 3 minutes
and cause to engine failover.
When attempting to upgrade we should attempt to acquire the lock without
wait (try to lock) and if we fail return error immediately, that scenario
is rare and if it happens the user can simply try again.
Change-Id: I93bfdcb19437fdc8901c08ea17dc80fe9c1e5510
Bug-Url: https://bugzilla.redhat.com/1260428
Signed-off-by: Liron Aravot
---
M vdsm/storage/hsm.py
M vdsm/storage/sp.py
2 files changed, 32 insertions(+), 25 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/20/60520/1
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 359b2b3..e92edfd 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -3342,7 +3342,7 @@
# activateSD/deactivateSD) as the operation uses the pool metadata.
vars.task.getExclusiveLock(STORAGE, spUUID)
pool = self.getPool(spUUID)
-pool._upgradePool(targetDomVersion)
+pool._upgradePool(targetDomVersion, lockTimeout=0)
return {"upgradeStatus": "started"}
def _getDomsStats(self, domainMonitor, doms):
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 24b7d2b..bb55fd2 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -419,32 +419,39 @@
self.spmRole = SPM_FREE
-def _upgradePool(self, targetDomVersion):
-with rmanager.acquireResource(sc.STORAGE, "upgrade_" + self.spUUID,
- rm.LockType.exclusive):
-sd.validateDomainVersion(targetDomVersion)
-self.log.info("Trying to upgrade master domain `%s`",
- self.masterDomain.sdUUID)
-with rmanager.acquireResource(sc.STORAGE, self.masterDomain.sdUUID,
- rm.LockType.exclusive):
-self._convertDomain(self.masterDomain, str(targetDomVersion))
+def _upgradePool(self, targetDomVersion, lockTimeout=None):
+try:
+with rmanager.acquireResource(sc.STORAGE, "upgrade_" + self.spUUID,
+ rm.LockType.exclusive,
+ timeout=lockTimeout):
+sd.validateDomainVersion(targetDomVersion)
+self.log.info("Trying to upgrade master domain `%s`",
+ self.masterDomain.sdUUID)
+with rmanager.acquireResource(sc.STORAGE,
+ self.masterDomain.sdUUID,
+ rm.LockType.exclusive):
+self._convertDomain(self.masterDomain,
+str(targetDomVersion))
-self.log.debug("Marking all domains for upgrade")
-self._domainsToUpgrade = self.getDomains(activeOnly=True).keys()
-try:
-self._domainsToUpgrade.remove(self.masterDomain.sdUUID)
-except ValueError:
-pass
+self.log.debug("Marking all domains for upgrade")
+self._domainsToUpgrade = self.getDomains(activeOnly=True)\
+.keys()
+try:
+self._domainsToUpgrade.remove(self.masterDomain.sdUUID)
+except ValueError:
+pass
-self.log.debug("Registering with state change event")
-self.domainMonitor.onDomainStateChange.register(
-self._upgradeCallback)
-self.log.debug("Running initial domain upgrade threads")
-for sdUUID in self._domainsToUpgrade:
-concurrent.thread(self._upgradeCallback,
- args=(sdUUID, True),
- kwargs={"__securityOverride": True},
- logger=self.log.name).start()
+self.log.debug("Registering with state change event")
+self.domainMonitor.onDomainStateChange.register(
+self._upgradeCallback)
+self.log.debug("Running initial domain upgrade threads")
+for sdUUID in self._domainsToUpgrade:
+concurrent.thread(self._upgradeCallback,
+ args=(sdUUID, True),
+ kwargs={"__securityOverride": True},
+ logger=self.log.name).start()
+except rm.RequestTimedOutError:
+raise se
Change in vdsm[master]: sp: try lock when executing user initated pool upgrade
Liron Aravot has posted comments on this change. Change subject: sp: try lock when executing user initated pool upgrade .. Patch Set 6: Verified+1 -- To view, visit https://gerrit.ovirt.org/59514 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I93bfdcb19437fdc8901c08ea17dc80fe9c1e5510 Gerrit-PatchSet: 6 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 https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has posted comments on this change. Change subject: sp: allow executing upgradePool even if there is a pending update .. Patch Set 9: Verified+1 -- To view, visit https://gerrit.ovirt.org/57188 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan 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]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has posted comments on this change. Change subject: sp: allow executing upgradePool even if there is a pending update .. Patch Set 7: (1 comment) https://gerrit.ovirt.org/#/c/57188/7/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 464 Line 465 Line 466 Line 467 Line 468 > This copy and the comment are not needed now, we are protected now by the u Good catch, removing it. -- To view, visit https://gerrit.ovirt.org/57188 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan 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]: sp: try lock when executing user initated pool upgrade
Liron Aravot has posted comments on this change.
Change subject: sp: try lock when executing user initated pool upgrade
..
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/59514/4/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 3337: # This lock has to be mutual with the pool metadata
operations (like
Line 3338: # activateSD/deactivateSD) as the operation uses the pool
metadata.
Line 3339: vars.task.getExclusiveLock(STORAGE, spUUID)
Line 3340: pool = self.getPool(spUUID)
Line 3341: pool._upgradePool(targetDomVersion, lock_timeout=0)
> We cannot use new style arguments for when adding an argument to old api. l
Done
Line 3342: return {"upgradeStatus": "started"}
Line 3343:
Line 3344: def _getDomsStats(self, domainMonitor, doms):
Line 3345: domInfo = {}
--
To view, visit https://gerrit.ovirt.org/59514
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I93bfdcb19437fdc8901c08ea17dc80fe9c1e5510
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
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]: sp: use timeout when acquiring pool upgrade lock
Liron Aravot has uploaded a new change for review.
Change subject: sp: use timeout when acquiring pool upgrade lock
..
sp: use timeout when acquiring pool upgrade lock
When attempting to execute upgradeStoragePool a concurrent
update on one of the pool domains may be during execution and hold
the pool upgrade lock.
In that case, the request to acquire the lock will wait for the lock
to be released which may cause the request to take longer then 3 minutes
and cause to engine failover.
When attempting to upgrade we should attempt to acquire the lock without
wait (try to lock) and if we fail return error immediately, that scenario
is rare and if it happens the user can simply try again.
Change-Id: I93bfdcb19437fdc8901c08ea17dc80fe9c1e5510
Bug-Url: https://bugzilla.redhat.com/1260428
Bug-Url: https://bugzilla.redhat.com/1319523
Signed-off-by: Liron Aravot
---
M vdsm/storage/hsm.py
M vdsm/storage/sp.py
2 files changed, 32 insertions(+), 29 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/14/59514/1
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 2d683c2..17d3517 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -3338,7 +3338,7 @@
# activateSD/deactivateSD) as the operation uses the pool metadata.
vars.task.getExclusiveLock(STORAGE, spUUID)
pool = self.getPool(spUUID)
-pool._upgradePool(targetDomVersion)
+pool._upgradePool(targetDomVersion, lock_timeout=0)
return {"upgradeStatus": "started"}
def _getDomsStats(self, domainMonitor, doms):
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index a2b5e10..5ba7c1d 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -419,36 +419,39 @@
self.spmRole = SPM_FREE
-def _upgradePool(self, targetDomVersion):
-with rmanager.acquireResource(sc.STORAGE, "upgrade_" + self.spUUID,
- rm.LockType.exclusive):
-sd.validateDomainVersion(targetDomVersion)
-self.log.info("Trying to upgrade master domain `%s`",
- self.masterDomain.sdUUID)
-with rmanager.acquireResource(sc.STORAGE, self.masterDomain.sdUUID,
- rm.LockType.exclusive):
-self._convertDomain(self.masterDomain, str(targetDomVersion))
+def _upgradePool(self, targetDomVersion, lock_timeout=None):
+try:
+with rmanager.acquireResource(sc.STORAGE, "upgrade_" + self.spUUID,
+ rm.LockType.exclusive,
timeout=lock_timeout):
+sd.validateDomainVersion(targetDomVersion)
+self.log.info("Trying to upgrade master domain `%s`",
+ self.masterDomain.sdUUID)
+with rmanager.acquireResource(sc.STORAGE,
self.masterDomain.sdUUID,
+ rm.LockType.exclusive):
+self._convertDomain(self.masterDomain,
str(targetDomVersion))
-self.log.debug("Marking all domains for upgrade")
-self._domainsToUpgrade = self.getDomains(activeOnly=True).keys()
-try:
-self._domainsToUpgrade.remove(self.masterDomain.sdUUID)
-except ValueError:
-pass
+self.log.debug("Marking all domains for upgrade")
+self._domainsToUpgrade =
self.getDomains(activeOnly=True).keys()
+try:
+self._domainsToUpgrade.remove(self.masterDomain.sdUUID)
+except ValueError:
+pass
-self.log.debug("Registering with state change event")
-self.domainMonitor.onDomainStateChange.register(
-self._upgradeCallback)
-self.log.debug("Running initial domain upgrade threads")
-# We need to copy the list as the domain monitor registered
-# callback and the initiated update threads may modify the list
-# while we iterate over it.
-# http://bugzilla.redhat.com/1319523
-for sdUUID in self._domainsToUpgrade[:]:
-concurrent.thread(self._upgradeCallback,
- args=(sdUUID, True),
- kwargs={"__securityOverride": True},
- logger=self.log.name).start()
+self.log.debug("Registering with state change event")
+self.domainMonitor.onDomainStateChange.register(
+self._upgradeCallback)
+self.log.debug("Running initial domain upgrade threads")
+# We need to copy the list as the domain monitor registered
+# callback and the initiated update threads may modify the list
+# while we iterate over it.
+# http://bugzilla.redhat.c
Change in vdsm[master]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has posted comments on this change.
Change subject: sp: allow executing upgradePool even if there is a pending
update
..
Patch Set 5:
(3 comments)
https://gerrit.ovirt.org/#/c/57188/3/vdsm/storage/sp.py
File vdsm/storage/sp.py:
Line 439:
Line 440: self.log.debug("Registering with state change event")
Line 441: self.domainMonitor.onDomainStateChange.register(
Line 442: self._upgradeCallback)
Line 443: self.log.debug("Running initial domain upgrade
threads")
> It should, otherwise the list may be modified while you iterated over it, w
I've meant to move it out properly (by making a copy in the lock scope). The
reason to do is that when no upgrade thread will start till the loop ends and
the lock will be released. neither of the options seems perfect - what do you
think?
Line 444: # We need to copy the list as the domain monitor
registered
Line 445: # callback and the initiated update threads may
modify the list
Line 446: # while we iterate over it.
Line 447: # http://bugzilla.redhat.com/1319523
https://gerrit.ovirt.org/#/c/57188/5/vdsm/storage/sp.py
File vdsm/storage/sp.py:
Line 174: self.log.error("Error while preparing domain `%s`
upgrade", sdUUID,
Line 175:exc_info=True)
Line 176: return
Line 177:
Line 178: with rmanager.acquireResource(STORAGE, "upgrade_" +
self.spUUID,
> Are you sure about this change? sc.XXX is now our common way to refer to st
its rebase foobar (as we still didn't use sc.XXX on patch #1). will return.
Line 179: rm.LockType.shared):
Line 180: with rmanager.acquireResource(STORAGE, sdUUID,
Line 181: rm.LockType.exclusive):
Line 182: if sdUUID not in self._domainsToUpgrade:
Line 418: panic("Unrecoverable errors during SPM stop process.")
Line 419:
Line 420: self.spmRole = SPM_FREE
Line 421:
Line 422: def _upgradePool(self, targetDomVersion, lock_timeout=None):
> I think we agreed that this should be a separate patch?
Agreed, I'll separate this to a separate change.
As regards to the second comment, I think its preferred to leave it here for
several reasons:
1. We should have the same treatment for resource timeout when failing to
acquire lock - if tomorrow we'll add a call to this method from another place
(or start using timeout from the spmStart flow) we'll need to keep in mind to
catch and raise the PoolUpgradeInProgres exception
2. Theoretically speaking, we could use another lock here which will require a
different raised exception.
3. This method encapsulates the upgrade and the raised exceptions, if we stop
using the rmanager and do a different check one day (as was done before), we'll
need to update the callers.
Line 423: try:
Line 424: with rmanager.acquireResource(sc.STORAGE, "upgrade_" +
self.spUUID,
Line 425: rm.LockType.exclusive,
timeout=lock_timeout):
Line 426: sd.validateDomainVersion(targetDomVersion)
--
To view, visit https://gerrit.ovirt.org/57188
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Tal Nisan
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]: sp: _upgradePoolDomain - improved logging
Liron Aravot has uploaded a new change for review.
Change subject: sp: _upgradePoolDomain - improved logging
..
sp: _upgradePoolDomain - improved logging
This patch changes the logging in _upgradePoolDomain() to make the
domain upgrade process clearer and easier to debug.
Change-Id: I1b004a763e4322214015d558768b81e5bfc646ab
Signed-off-by: Liron Aravot
---
M vdsm/storage/sp.py
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/57551/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index efe486f..93e4826 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -155,18 +155,19 @@
self._refreshDomainLinks(domain)
def _upgradePoolDomain(self, sdUUID, isValid):
+self.log.debug("Attempting to upgrade domain %s", sdUUID)
+
# This method is called everytime the onDomainStateChange
# event is emitted, this event is emitted even when a domain goes
# INVALID if this happens there is nothing for us to do no matter what
# the domain is
if not isValid:
+self.log.debug("Domain %s isn't valid, skipping upgrade", sdUUID)
return
domain = sdCache.produce(sdUUID)
if sdUUID not in self._domainsToUpgrade:
return
-
-self.log.debug("Preparing to upgrade domain %s", sdUUID)
try:
# Assumed that the domain can be attached only to one pool
--
To view, visit https://gerrit.ovirt.org/57551
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1b004a763e4322214015d558768b81e5bfc646ab
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: _upgradePool - log before starting domain upgrade thread
Liron Aravot has uploaded a new change for review.
Change subject: sp: _upgradePool - log before starting domain upgrade thread
..
sp: _upgradePool - log before starting domain upgrade thread
Change-Id: I6f13ad128dbd9aa9d69891d5999d584a8cc26d0d
Signed-off-by: Liron Aravot
---
M vdsm/storage/sp.py
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/50/57550/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 9b7433f..efe486f 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -466,6 +466,7 @@
# while we iterate over it.
# http://bugzilla.redhat.com/1319523
for sdUUID in self._domainsToUpgrade[:]:
+self.log.debug("Running initial domain upgrade thread for
domain %s", sdUUID)
concurrent.thread(self._upgradeCallback,
args=(sdUUID, True),
kwargs={"__securityOverride": True},
--
To view, visit https://gerrit.ovirt.org/57550
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6f13ad128dbd9aa9d69891d5999d584a8cc26d0d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: _upgradePool - concurrent.thread instead threading....
Liron Aravot has abandoned this change. Change subject: storage: _upgradePool - concurrent.thread instead threading.Thread .. Abandoned irrelevant for master and we won't take it to 3.6 -- To view, visit https://gerrit.ovirt.org/57099 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I1bb080c25e3dbf3c93f84ce73417e7e64bcb4fc4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: _upgradePool - concurrent.thread instead threading....
Liron Aravot has posted comments on this change. Change subject: storage: _upgradePool - concurrent.thread instead threading.Thread .. Patch Set 2: -Verified -- To view, visit https://gerrit.ovirt.org/57099 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1bb080c25e3dbf3c93f84ce73417e7e64bcb4fc4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: _upgradePool - concurrent.thread instead threading....
Liron Aravot has abandoned this change. Change subject: storage: _upgradePool - concurrent.thread instead threading.Thread .. Abandoned irrelevant for master and we won't take it to 3.6 -- To view, visit https://gerrit.ovirt.org/57099 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I1bb080c25e3dbf3c93f84ce73417e7e64bcb4fc4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: _upgradePool - concurrent.thread instead threading....
Liron Aravot has restored this change. Change subject: storage: _upgradePool - concurrent.thread instead threading.Thread .. Restored -- To view, visit https://gerrit.ovirt.org/57099 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: restore Gerrit-Change-Id: I1bb080c25e3dbf3c93f84ce73417e7e64bcb4fc4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.6]: sp: race in domains upgrade prevents further pool upgrades
Liron Aravot has posted comments on this change. Change subject: sp: race in domains upgrade prevents further pool upgrades .. Patch Set 2: Rerun-Hooks: all -- To view, visit https://gerrit.ovirt.org/57450 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie384c63315214786dc62f2b4998002320e314d30 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.6 Gerrit-Owner: Liron Aravot 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.6]: sp: race in domains upgrade prevents further pool upgrades
Liron Aravot has posted comments on this change. Change subject: sp: race in domains upgrade prevents further pool upgrades .. Patch Set 1: Verified+1 -- To view, visit https://gerrit.ovirt.org/57450 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie384c63315214786dc62f2b4998002320e314d30 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.6 Gerrit-Owner: Liron Aravot 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.6]: sp: race in domains upgrade prevents further pool upgrades
Liron Aravot has uploaded a new change for review.
Change subject: sp: race in domains upgrade prevents further pool upgrades
..
sp: race in domains upgrade prevents further pool upgrades
When executing _upgradePool() a callback a domain upgrade callback
is registered to the domain monitor and an initial update thread is
started for each of the domain in self._domainsToUpgrade (which contains
the active pool domains).
Whenever upgrade for domain is ended, the upgrade thread deletes the domain
from self._domainsToUpgrade, which is racey because that might happen while
we still iterate over the list. If that does happen, the results are unexpected
and on some inspected cases we didn't start the upgrade threads for all the
domains which causes to a bug - an update will be considered running and
no further pool upgrades could be initiated.
This patch copies the list to a temporary list before attempting to register
the callback/start the upgrade threads and by that avoiding that race.
this solution is simple as its targeted to the 3.6 branch, a more complete
solution which will also allow concurrent updates is introduced in
I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Change-Id: Ie384c63315214786dc62f2b4998002320e314d30
Bug-Url: http://bugzilla.redhat.com/1319523
Signed-off-by: Liron Aravot
---
M vdsm/storage/sp.py
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/50/57450/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index f5e69e8..fc825e6 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -459,7 +459,11 @@
self.domainMonitor.onDomainStateChange.register(
self._upgradeCallback)
self.log.debug("Running initial domain upgrade threads")
-for sdUUID in self._domainsToUpgrade:
+# We need to copy the list as the domain monitor registered
+# callback and the initiated update threads may modify the list
+# while we iterate over it.
+# http://bugzilla.redhat.com/1319523
+for sdUUID in self._domainsToUpgrade[:]:
threading.Thread(target=self._upgradeCallback,
args=(sdUUID, True),
kwargs={"__securityOverride": True}).start()
--
To view, visit https://gerrit.ovirt.org/57450
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie384c63315214786dc62f2b4998002320e314d30
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Liron Aravot
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: race in domains upgrade prevents further pool upgrades
Liron Aravot has posted comments on this change. Change subject: sp: race in domains upgrade prevents further pool upgrades .. Patch Set 7: Verified+1 -- To view, visit https://gerrit.ovirt.org/57315 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie384c63315214786dc62f2b4998002320e314d30 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: race in domains upgrade prevents further pool upgrades
Liron Aravot has posted comments on this change. Change subject: sp: race in domains upgrade prevents further pool upgrades .. Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/57315/2/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 460 Line 461 Line 462 Line 463 Line 464 > list[:] is atomic, we cannot have any race. Generally i'm not in favor of depending on implementation details - who knows what will be in a further version? is something in the specification guarantees us its safe? if we write code for multi threaded env i'd prefer to write it correctly and not rely on impl details. if you both are strongly against - i'll change it, but i'd be happy if you reconsidered. -- To view, visit https://gerrit.ovirt.org/57315 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie384c63315214786dc62f2b4998002320e314d30 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: race in domains upgrade prevents further pool upgrades
Liron Aravot has posted comments on this change. Change subject: sp: race in domains upgrade prevents further pool upgrades .. Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/57315/2/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 460 Line 461 Line 462 Line 463 Line 464 > can you just put the [:] here without storing the local variable domainsToU having it here is problematic, because the domain monitor callback is registered in line 461 so an element could be removed during the copy of the list if its done here -- To view, visit https://gerrit.ovirt.org/57315 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie384c63315214786dc62f2b4998002320e314d30 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: race in domains upgrade prevents further pool upgrades
Liron Aravot has uploaded a new change for review.
Change subject: sp: race in domains upgrade prevents further pool upgrades
..
sp: race in domains upgrade prevents further pool upgrades
When executing _upgradePool() a callback a domain upgrade callback
is registered to the domain monitor and an initial update thread is
started for each of the domain in self._domainsToUpgrade (which contains
the active pool domains).
Whenever upgrade for domain is ended, the upgrade thread deletes the domain
from self._domainsToUpgrade, which is racey because that might happen while
we still iterate over the list. If that does happen, the results are unexpected
and on some inspected cases we didn't start the upgrade threads for all the
domains which causes to a bug - an update will be considered running and
no further pool upgrades could be initiated.
This patch copies the list to a temporary list before attempting to register
the callback/start the upgrade threads and by that avoiding that race.
this solution is simple as its targeted to the 3.6 branch, a more complete
solution which will also allow concurrent updates is introduced in
I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Change-Id: Ie384c63315214786dc62f2b4998002320e314d30
Signed-off-by: Liron Aravot
---
M vdsm/storage/sp.py
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/57315/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index af838de..e6ed8a4 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -457,11 +457,15 @@
except ValueError:
pass
+# This is needed as the domain monitor registered callback and
+# the initiated update threads may modify the list while we
+# iterate over it
+domainsToUpgrade = self._domainsToUpgrade[:]
self.log.debug("Registering with state change event")
self.domainMonitor.onDomainStateChange.register(
self._upgradeCallback)
self.log.debug("Running initial domain upgrade threads")
-for sdUUID in self._domainsToUpgrade:
+for sdUUID in domainsToUpgrade:
concurrent.thread(self._upgradeCallback,
args=(sdUUID, True),
kwargs={"__securityOverride": True},
--
To view, visit https://gerrit.ovirt.org/57315
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie384c63315214786dc62f2b4998002320e314d30
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has posted comments on this change.
Change subject: sp: allow executing upgradePool even if there is a pending
update
..
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/57188/3/vdsm/storage/sp.py
File vdsm/storage/sp.py:
Line 439: self.log.debug("Registering with state change event")
Line 440: self.domainMonitor.onDomainStateChange.register(
Line 441: self._upgradeCallback)
Line 442: self.log.debug("Running initial domain upgrade threads")
Line 443: for sdUUID in self._domainsToUpgrade:
> need to check about moving this out of the lock scope
right now it seems like that should remain here.
Line 444: concurrent.thread(target=self._upgradeCallback,
Line 445: args=(sdUUID, True),
Line 446: kwargs={"__securityOverride":
True}).start()
Line 447:
--
To view, visit https://gerrit.ovirt.org/57188
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Allon Mureinik
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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has posted comments on this change. Change subject: sp: allow executing upgradePool even if there is a pending update .. Patch Set 3: -Code-Review Verified-1 -- To view, visit https://gerrit.ovirt.org/57188 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has posted comments on this change.
Change subject: sp: allow executing upgradePool even if there is a pending
update
..
Patch Set 3: Code-Review-1
(1 comment)
needs more checking
https://gerrit.ovirt.org/#/c/57188/3/vdsm/storage/sp.py
File vdsm/storage/sp.py:
Line 439: self.log.debug("Registering with state change event")
Line 440: self.domainMonitor.onDomainStateChange.register(
Line 441: self._upgradeCallback)
Line 442: self.log.debug("Running initial domain upgrade threads")
Line 443: for sdUUID in self._domainsToUpgrade:
need to check about moving this out of the lock scope
Line 444: concurrent.thread(target=self._upgradeCallback,
Line 445: args=(sdUUID, True),
Line 446: kwargs={"__securityOverride":
True}).start()
Line 447:
--
To view, visit https://gerrit.ovirt.org/57188
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Allon Mureinik
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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: _upgradePool - concurrent.thread instead threading....
Liron Aravot has posted comments on this change. Change subject: storage: _upgradePool - concurrent.thread instead threading.Thread .. Patch Set 2: Verified-1 concurrent.thread() doesnt support kwargs yet -- To view, visit https://gerrit.ovirt.org/57099 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1bb080c25e3dbf3c93f84ce73417e7e64bcb4fc4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: _upgradePool - concurrent.thread instead threading....
Liron Aravot has abandoned this change. Change subject: storage: _upgradePool - concurrent.thread instead threading.Thread .. Abandoned -- To view, visit https://gerrit.ovirt.org/57099 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I1bb080c25e3dbf3c93f84ce73417e7e64bcb4fc4 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has posted comments on this change. Change subject: sp: allow executing upgradePool even if there is a pending update .. Patch Set 2: (3 comments) https://gerrit.ovirt.org/#/c/57188/2/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 174 Line 175 Line 176 Line 177 Line 178 > This removes the upgrade_domain_uuid lock, right? indeed Line 202 Line 203 Line 204 Line 205 Line 206 > This changes pool, se we need exclusive lock on the pool for this operation i agree that it should be inspected, but this is irrelevant for this change - today the lock is taken for the domain only, so the list isn't updated under pool lock. Line 203 Line 204 Line 205 Line 206 Line 207 > This also changes the pool state, needs an exclusive lock on the pool. i agree that it should be inspected, but this is irrelevant for this change - today the lock is taken for the domain only, so the this method doesn't run under pool lock. -- To view, visit https://gerrit.ovirt.org/57188 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3 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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: _upgradeStoragePool - log domains pending for update
Liron Aravot has uploaded a new change for review.
Change subject: sp: _upgradeStoragePool - log domains pending for update
..
sp: _upgradeStoragePool - log domains pending for update
This patch is for debugging purposes and is supposed to be minimal,
therefore the exception isn't modified to contain the pending domains
but only a log is added
Change-Id: I2f0b9c262f4a671ff55ec1da12db2e10d1c9ee24
Signed-off-by: Liron Aravot
---
M vdsm/storage/sp.py
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/57193/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index ef15135..b50430e 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -439,7 +439,11 @@
def _upgradePool(self, targetDomVersion):
with rmanager.acquireResource(STORAGE, "upgrade_" + self.spUUID,
rm.LockType.exclusive):
+self._domainsToUpgrade = self.getDomains(activeOnly=True).keys()
if len(self._domainsToUpgrade) > 0:
+self._domainsToUpgrade = []
+self.log.error("Domains '%s' are still pending for upgrade",
+ self._domainsToUpgrade)
raise se.PoolUpgradeInProgress(self.spUUID)
sd.validateDomainVersion(targetDomVersion)
--
To view, visit https://gerrit.ovirt.org/57193
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2f0b9c262f4a671ff55ec1da12db2e10d1c9ee24
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: allow executing upgradePool even if there is a pending u...
Liron Aravot has uploaded a new change for review.
Change subject: sp: allow executing upgradePool even if there is a pending
update
..
sp: allow executing upgradePool even if there is a pending update
Currently when a pool upgrade is initated (either by startSpm() or by
executing upgradePool() explicitly) the _domainsToUpgrade member of the
storage pool is initiated with the active domains, when each domain
upgrade is completed (or if the upgrade is unneeded) its id is removed
from that list.
When there is an unavailable domain, it'll remain on that list until its
update is complete - while it's unavailable other upgrade can't be
executed.
This patch changes that, we should allow upgrading the pool even if an
older upgrade wasn't complete yet - the domains that the old update
wasn't completed yet for will be upgraded to the new version.
Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1260428
Signed-off-by: Liron Aravot
---
M vdsm/storage/sp.py
1 file changed, 3 insertions(+), 23 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/57188/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index ef15135..e04112c 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -175,8 +175,8 @@
exc_info=True)
return
-with rmanager.acquireResource(STORAGE, "upgrade_" + sdUUID,
- rm.LockType.exclusive):
+with rmanager.acquireResource(STORAGE, "upgrade_" + self.spUUID,
+ rm.LockType.shared):
with rmanager.acquireResource(STORAGE, sdUUID,
rm.LockType.exclusive):
if sdUUID not in self._domainsToUpgrade:
@@ -352,30 +352,13 @@
self.log.debug("Shutting down upgrade process")
with rmanager.acquireResource(STORAGE, "upgrade_" + self.spUUID,
rm.LockType.exclusive):
-domains = self._domainsToUpgrade[:]
try:
self.domainMonitor.onDomainStateChange.unregister(
self._upgradeCallback)
except KeyError:
pass
-requests = []
-def cancelUpgrade(sdUUID, req, res):
-try:
-self._domainsToUpgrade.remove(sdUUID)
-except ValueError:
-pass
-
-res.release()
-
-for sdUUID in domains:
-req = rmanager.registerResource(STORAGE, "upgrade_" + sdUUID,
-rm.LockType.exclusive,
-partial(cancelUpgrade, sdUUID))
-requests.append(req)
-
-for req in requests:
-req.wait()
+self._domainsToUpgrade = []
@classmethod
def cleanupMasterMount(cls):
@@ -439,9 +422,6 @@
def _upgradePool(self, targetDomVersion):
with rmanager.acquireResource(STORAGE, "upgrade_" + self.spUUID,
rm.LockType.exclusive):
-if len(self._domainsToUpgrade) > 0:
-raise se.PoolUpgradeInProgress(self.spUUID)
-
sd.validateDomainVersion(targetDomVersion)
self.log.info("Trying to upgrade master domain `%s`",
self.masterDomain.sdUUID)
--
To view, visit https://gerrit.ovirt.org/57188
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8e14a3aa33bfab4751ab5d1e3becbeda892da4c3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: _upgradePool - concurrent.thread instead threading....
Liron Aravot has uploaded a new change for review.
Change subject: storage: _upgradePool - concurrent.thread instead
threading.Thread
..
storage: _upgradePool - concurrent.thread instead threading.Thread
Currently _upgradePool uses threading.Thread which doesn't print
the traceback in cae of error during the execution. This makes it
hard to debug errors that might rise on the started threads.
This patch changes it to use concurrent.Thread which already includes
the use of @utils.traceback that provides the logging.
Change-Id: I1bb080c25e3dbf3c93f84ce73417e7e64bcb4fc4
Bug-Url: https://bugzilla.redhat.com/1319523
Signed-off-by: Liron Aravot
---
M vdsm/storage/sp.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/57099/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index f5e69e8..421bb7a 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -460,7 +460,7 @@
self._upgradeCallback)
self.log.debug("Running initial domain upgrade threads")
for sdUUID in self._domainsToUpgrade:
-threading.Thread(target=self._upgradeCallback,
+concurrent.thread(target=self._upgradeCallback,
args=(sdUUID, True),
kwargs={"__securityOverride": True}).start()
--
To view, visit https://gerrit.ovirt.org/57099
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1bb080c25e3dbf3c93f84ce73417e7e64bcb4fc4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: monitor: Convert valid to read-only property
Liron Aravot has posted comments on this change. Change subject: monitor: Convert valid to read-only property .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/39088 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff27081041bbaa1e319539df67abb31f38367e7d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: monitor: Convert valid to read-only property
Liron Aravot has posted comments on this change. Change subject: monitor: Convert valid to read-only property .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/39088/3/tests/storageMonitorTests.py File tests/storageMonitorTests.py: Line 41: def test_deleting_attribute_raises(self): Line 42: for name in self.status.__slots__: Line 43: self.assertRaises(AssertionError, delattr, self.frozen, name) Line 44: Line 45: def test_valid(self): > valid is a property, so copying values is not related. This verify that it ok, so it seems that what we should verify here is that frozen.valid equals to status.valid Line 46: self.assertTrue(self.frozen.valid) Line 47: Line 48: Line 49: class StatusValidTests(VdsmTestCase): -- To view, visit https://gerrit.ovirt.org/39088 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff27081041bbaa1e319539df67abb31f38367e7d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO 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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: monitor: Convert valid to read-only property
Liron Aravot has posted comments on this change. Change subject: monitor: Convert valid to read-only property .. Patch Set 3: Code-Review-1 -- To view, visit https://gerrit.ovirt.org/39088 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff27081041bbaa1e319539df67abb31f38367e7d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: monitor: Convert valid to read-only property
Liron Aravot has posted comments on this change. Change subject: monitor: Convert valid to read-only property .. Patch Set 3: (2 comments) https://gerrit.ovirt.org/#/c/39088/3/tests/storageMonitorTests.py File tests/storageMonitorTests.py: Line 41: def test_deleting_attribute_raises(self): Line 42: for name in self.status.__slots__: Line 43: self.assertRaises(AssertionError, delattr, self.frozen, name) Line 44: Line 45: def test_valid(self): I don't see how this test is needed, the FrozenStatusTests is about making sure that the FrozenStatus is created properly. The frozen attribute will be checked with the rest of the attributes in test_copy_attributes Line 46: self.assertTrue(self.frozen.valid) Line 47: Line 48: Line 49: class StatusValidTests(VdsmTestCase): Line 49: class StatusValidTests(VdsmTestCase): Line 50: Line 51: def test_valid(self): Line 52: s = monitor.Status() Line 53: self.assertTrue(s.valid) lets add assertion that verifies that error is None Line 54: Line 55: def test_invalid(self): Line 56: s = monitor.Status() Line 57: s.error = Exception() -- To view, visit https://gerrit.ovirt.org/39088 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff27081041bbaa1e319539df67abb31f38367e7d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO 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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: add untrusted image verification.
Liron Aravot has posted comments on this change. Change subject: hsm: add untrusted image verification. .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/55746/4/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 1505: @public Line 1506: def verify_untrusted_volume(self, spUUID, sdUUID, imgUUID, volUUID): Line 1507: dom = sdCache.produce(sdUUID=sdUUID) Line 1508: vol = dom.produceVolume(imgUUID, volUUID) Line 1509: actual_info = qemuimg.info(vol.getVolumePath()) > great. *meta_format Line 1510: Line 1511: expected_format = volume.FMT2STR[vol.getFormat()] Line 1512: actual_format = actual_info["format"] Line 1513: if expected_format != actual_format: -- To view, visit https://gerrit.ovirt.org/55746 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf85061536eb4ddff021539c742a674f183a8984 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby 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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: add untrusted image verification.
Liron Aravot has posted comments on this change. Change subject: hsm: add untrusted image verification. .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/55746/4/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 1505: @public Line 1506: def verify_untrusted_volume(self, spUUID, sdUUID, imgUUID, volUUID): Line 1507: dom = sdCache.produce(sdUUID=sdUUID) Line 1508: vol = dom.produceVolume(imgUUID, volUUID) Line 1509: actual_info = qemuimg.info(vol.getVolumePath()) > I agree, the error should not mention upload. great. perhaps we should rename to qemu_info and metadata_info, that way its clear what is what. Line 1510: Line 1511: expected_format = volume.FMT2STR[vol.getFormat()] Line 1512: actual_format = actual_info["format"] Line 1513: if expected_format != actual_format: -- To view, visit https://gerrit.ovirt.org/55746 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf85061536eb4ddff021539c742a674f183a8984 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby 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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: add untrusted image verification.
Liron Aravot has posted comments on this change. Change subject: hsm: add untrusted image verification. .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/55746/4/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 1514: raise se.ImageVerificationError( Line 1515: "Uploaded image format is %s while the format specified " Line 1516: "by the user is %s" % (actual_format, expected_format) Line 1517: ) Line 1518: if "backingfile" in actual_info: > We cannot support this since a user cannot send us valid backingfile option I believe thats a relevant scenario that we need to support, if not now so in the future. Line 1519: raise se.ImageVerificationError( Line 1520: "'%s' is defined as a backingfile, while backingfile is not " Line 1521: "allowed for an uploaded image." Line 1522: % actual_info["backingfile"] -- To view, visit https://gerrit.ovirt.org/55746 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf85061536eb4ddff021539c742a674f183a8984 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby 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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: add untrusted image verification.
Liron Aravot has posted comments on this change. Change subject: hsm: add untrusted image verification. .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/55746/4/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 1505: @public Line 1506: def verify_untrusted_volume(self, spUUID, sdUUID, imgUUID, volUUID): Line 1507: dom = sdCache.produce(sdUUID=sdUUID) Line 1508: vol = dom.produceVolume(imgUUID, volUUID) Line 1509: actual_info = qemuimg.info(vol.getVolumePath()) > No, this is very wrong. The actual info is whats we have on storage, report I didn't get your comment - we have two "infos" here - one received from the uploaded image and one which resides on the storage. I don't mind what are the names, but it should be clear what is what. right now it isn't and one should go to the start of the method to check what is each one. If this is a generic method, the errors returned from here shouldn't contain "upload" specific comments. Line 1510: Line 1511: expected_format = volume.FMT2STR[vol.getFormat()] Line 1512: actual_format = actual_info["format"] Line 1513: if expected_format != actual_format: -- To view, visit https://gerrit.ovirt.org/55746 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf85061536eb4ddff021539c742a674f183a8984 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby 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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fileUtils: tarCopy - avoid extracting the file modified time
Liron Aravot has posted comments on this change. Change subject: fileUtils: tarCopy - avoid extracting the file modified time .. Patch Set 6: Verified+1 -- To view, visit https://gerrit.ovirt.org/55163 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I97e937aac233958db52b2fda685f5a5964fda2c8 Gerrit-PatchSet: 6 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: add untrusted image verification.
Liron Aravot has posted comments on this change. Change subject: hsm: add untrusted image verification. .. Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/55746/4/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 1505: @public Line 1506: def verify_untrusted_volume(self, spUUID, sdUUID, imgUUID, volUUID): Line 1507: dom = sdCache.produce(sdUUID=sdUUID) Line 1508: vol = dom.produceVolume(imgUUID, volUUID) Line 1509: actual_info = qemuimg.info(vol.getVolumePath()) i'd suggest to rename the info retrieved from the storage to actual_info, and the uploaded info to upload_info, those names seems clearer and makes reading the code to be more trivial Line 1510: Line 1511: expected_format = volume.FMT2STR[vol.getFormat()] Line 1512: actual_format = actual_info["format"] Line 1513: if expected_format != actual_format: Line 1514: raise se.ImageVerificationError( Line 1515: "Uploaded image format is %s while the format specified " Line 1516: "by the user is %s" % (actual_format, expected_format) Line 1517: ) Line 1518: if "backingfile" in actual_info: the problem with not supporting that is that we won't support upload of disks based on templates. perhaps we can find a solution that'll fit the security limitations while supporting that as well. Line 1519: raise se.ImageVerificationError( Line 1520: "'%s' is defined as a backingfile, while backingfile is not " Line 1521: "allowed for an uploaded image." Line 1522: % actual_info["backingfile"] -- To view, visit https://gerrit.ovirt.org/55746 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf85061536eb4ddff021539c742a674f183a8984 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Greg Padgett Gerrit-Reviewer: Idan Shaby 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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fileUtils: tarCopy - avoid extracting the file modified time
Liron Aravot has posted comments on this change. Change subject: fileUtils: tarCopy - avoid extracting the file modified time .. Patch Set 5: Rerun-hooks: all -- To view, visit https://gerrit.ovirt.org/55163 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I97e937aac233958db52b2fda685f5a5964fda2c8 Gerrit-PatchSet: 5 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fileUtils: tarCopy - avoid extracting the file modified time
Liron Aravot has posted comments on this change. Change subject: fileUtils: tarCopy - avoid extracting the file modified time .. Patch Set 5: I've verified by testing deactivation of the master domain which will migrate the master role to other domain, -- To view, visit https://gerrit.ovirt.org/55163 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I97e937aac233958db52b2fda685f5a5964fda2c8 Gerrit-PatchSet: 5 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fileUtils: tarCopy - avoid extracting the file modified time
Liron Aravot has posted comments on this change. Change subject: fileUtils: tarCopy - avoid extracting the file modified time .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/55163/1/vdsm/storage/fileUtils.py File vdsm/storage/fileUtils.py: Line 60: Line 61: tsrc = subprocess.Popen([constants.EXT_TAR, "cf", "-"] + Line 62: excludeArgs + ["-C", src, "."], Line 63: stdout=subprocess.PIPE) Line 64: tdst = subprocess.Popen([constants.EXT_TAR, "xf", "-", "-C", dst, "-m"], > Can you use the long option (--touch) instead? It is easier to debug code Done Line 65: stdin=tsrc.stdout, stderr=subprocess.PIPE, Line 66: stdout=subprocess.PIPE) Line 67: tsrc.stdout.close() Line 68: out, err = tdst.communicate() -- To view, visit https://gerrit.ovirt.org/55163 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I97e937aac233958db52b2fda685f5a5964fda2c8 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/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fileUtils: tarCopy - avoid extracting the file modified time
Liron Aravot has uploaded a new change for review. Change subject: fileUtils: tarCopy - avoid extracting the file modified time .. fileUtils: tarCopy - avoid extracting the file modified time We using fileUtils.tarCopy() when there are unsynced clocks between the source/destination tar might warn us that the source timestamp is in the future (when we copy from/to file). This error is logged to stderr, if the copy operation fails for some other reason the stderr might be flooeded with timestamp in the future errors which is confusing when analyzing the cause to the failure. As we don't case about the timestamp this patch adds the -m flag which doesn't extract the file modified date and by that eliminates the error. Change-Id: I97e937aac233958db52b2fda685f5a5964fda2c8 Signed-off-by: Liron Aravot --- M vdsm/storage/fileUtils.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/63/55163/1 diff --git a/vdsm/storage/fileUtils.py b/vdsm/storage/fileUtils.py index eb20c97..0d6946b 100644 --- a/vdsm/storage/fileUtils.py +++ b/vdsm/storage/fileUtils.py @@ -61,7 +61,7 @@ tsrc = subprocess.Popen([constants.EXT_TAR, "cf", "-"] + excludeArgs + ["-C", src, "."], stdout=subprocess.PIPE) -tdst = subprocess.Popen([constants.EXT_TAR, "xf", "-", "-C", dst], +tdst = subprocess.Popen([constants.EXT_TAR, "xf", "-", "-C", dst, "-m"], stdin=tsrc.stdout, stderr=subprocess.PIPE, stdout=subprocess.PIPE) tsrc.stdout.close() -- To view, visit https://gerrit.ovirt.org/55163 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I97e937aac233958db52b2fda685f5a5964fda2c8 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tmp
Liron Aravot has uploaded a new change for review. Change subject: tmp .. tmp aLonger description using lines' length under 72 chars. With multiple paragraphs if necessary. Change-Id: I47c816e9d453dca595fc7abb4a84b301c34fc624 Bug-Url: https://bugzilla.redhat.com/?? Signed-off-by: lara...@redhat.com --- M vdsm/rpc/vdsmapi-schema.json 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/48/39548/1 diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json index d901957..ee6b182 100644 --- a/vdsm/rpc/vdsmapi-schema.json +++ b/vdsm/rpc/vdsmapi-schema.json @@ -7809,7 +7809,7 @@ 'union': ['VdsmVolumeSpec', 'ImageSharingMethodArgsHttp']} ## -# @Volume.copyData: +# @Host.copyData: # # Copy data from a source location to a destination. # -- To view, visit https://gerrit.ovirt.org/39548 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I47c816e9d453dca595fc7abb4a84b301c34fc624 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tmp
Liron Aravot has abandoned this change. Change subject: tmp .. Abandoned -- To view, visit https://gerrit.ovirt.org/39548 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I47c816e9d453dca595fc7abb4a84b301c34fc624 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.activateHsm()
Liron Aravot has uploaded a new change for review.
Change subject: adding StorageDomain.activateHsm()
..
adding StorageDomain.activateHsm()
Change-Id: Ic661b44b060ddfeeb2a2caedb2662c6102b950e0
Signed-off-by: lara...@redhat.com
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/BindingXMLRPC.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
M vdsm/storage/sp.py
6 files changed, 96 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/43102/1
diff --git a/client/vdsClient.py b/client/vdsClient.py
index 01f2211..9c42228 100755
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -647,6 +647,14 @@
return dom['status']['code'], dom['status']['message']
return 0, ''
+def activateStorageDomainHsm(self, args):
+sdUUID = args[0]
+spUUID = args[1]
+dom = self.s.activateStorageDomainHsm(sdUUID, spUUID)
+if dom['status']['code']:
+return dom['status']['code'], dom['status']['message']
+return 0, ''
+
def deactivateStorageDomain(self, args):
sdUUID = args[0]
spUUID = args[1]
@@ -2502,6 +2510,10 @@
'Get list of VMs from the'
'given backup domain'
)),
+'updateStorageDomainVmData': (serv.updateStorageDomainVmData,
+ (' ',
+ 'Update VM on a Backup domain'
+ )),
'validateStorageDomain': (serv.validateStorageDomain,
('',
'Validate storage domain'
@@ -2511,6 +2523,12 @@
'Activate a storage domain that is already '
'a member in a storage pool.'
)),
+'activateStorageDomainHsm': (serv.activateStorageDomainHsm,
+ (' ',
+ 'Activate a storage domain that is '
+ 'already a member in a storage pool '
+ 'that operates without Spm.'
+ )),
'deactivateStorageDomain': (serv.deactivateStorageDomain,
(' ',
diff --git a/vdsm/API.py b/vdsm/API.py
index 8af2ad0..73f1a1f 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -991,6 +991,9 @@
def activate(self, storagepoolID):
return self._irs.activateStorageDomain(self._UUID, storagepoolID)
+def activateHsm(self, storagepoolID):
+return self._irs.activateStorageDomainHsm(self._UUID, storagepoolID)
+
def attach(self, storagepoolID):
return self._irs.attachStorageDomain(self._UUID, storagepoolID)
@@ -1108,7 +,8 @@
return self._irs.fenceSpmStorage(self._UUID, lastOwner, lastLver)
def getBackedUpVmsInfo(self, storagedomainID, vmList):
-return self._irs.getVmsInfo(self._UUID, storagedomainID, vmList)
+return self._irs.getBackedUpVmsInfo(self._UUID,
+storagedomainID, vmList)
def getBackedUpVmsList(self, storagedomainID):
return self._irs.getVmsList(self._UUID, storagedomainID)
diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py
index bd86804..0ab159f 100644
--- a/vdsm/rpc/BindingXMLRPC.py
+++ b/vdsm/rpc/BindingXMLRPC.py
@@ -607,6 +607,10 @@
domain = API.StorageDomain(sdUUID)
return domain.activate(spUUID)
+def domainActivateHsm(self, sdUUID, spUUID):
+domain = API.StorageDomain(sdUUID)
+return domain.activateHsm(spUUID)
+
def domainAttach(self, sdUUID, spUUID, options=None):
domain = API.StorageDomain(sdUUID)
return domain.attach(spUUID)
@@ -1101,6 +1105,7 @@
'upgradeStorageDomainVersion'),
(self.domainGetBackedUpVmsInfo,
'getStorageDomainBackedUpVmsInfo'),
+(self.domainActivateHsm, 'activateStorageDomainHsm'),
(self.domainUpdateVMData, 'updateStorageDomainVmData'))
def getIrsMethods(self):
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index 96d7661..2173ac2 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -4968,6 +4968,22 @@
'data': {'storagedomainID': 'UUID', 'storagepoolID': 'UUID'}}
##
+# @StorageDomain.activateHsm:
+#
+# Activate an attached but inactive Storage Domain.
+# Can be used only in Storage Pools that operate without SPM.
+#
+# @storagedomainID: The UUID of the Storage Domain
+#
+# @storagepoolID:The UUID of the Storage Pool to which the Storage Domain
+#is attached
+#
+# Since: 4.18.0
+##
+{'command': {'c
Change in vdsm[master]: adding StorageDomain.activateHsm()
Liron Aravot has abandoned this change. Change subject: adding StorageDomain.activateHsm() .. Abandoned -- To view, visit https://gerrit.ovirt.org/43153 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I68eccd300b8ba0cc45b2485f978eb3ae367c16e2 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.activateHsm()
Liron Aravot has uploaded a new change for review.
Change subject: adding StorageDomain.activateHsm()
..
adding StorageDomain.activateHsm()
Change-Id: I68eccd300b8ba0cc45b2485f978eb3ae367c16e2
Signed-off-by: lara...@redhat.com
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/BindingXMLRPC.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
M vdsm/storage/sp.py
M vdsm/storage/storage_exception.py
7 files changed, 134 insertions(+), 17 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/43153/4
diff --git a/client/vdsClient.py b/client/vdsClient.py
index d59ae7c..6169be7 100755
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -647,6 +647,14 @@
return dom['status']['code'], dom['status']['message']
return 0, ''
+def activateStorageDomainHsm(self, args):
+sdUUID = args[0]
+spUUID = args[1]
+dom = self.s.activateStorageDomainHsm(sdUUID, spUUID)
+if dom['status']['code']:
+return dom['status']['code'], dom['status']['message']
+return 0, ''
+
def deactivateStorageDomain(self, args):
sdUUID = args[0]
spUUID = args[1]
@@ -2502,6 +2510,10 @@
'Get list of VMs from the'
'given backup domain'
)),
+'updateStorageDomainVmData': (serv.updateStorageDomainVmData,
+ (' ',
+ 'Update VM on a Backup domain'
+ )),
'validateStorageDomain': (serv.validateStorageDomain,
('',
'Validate storage domain'
@@ -2511,6 +2523,12 @@
'Activate a storage domain that is already '
'a member in a storage pool.'
)),
+'activateStorageDomainHsm': (serv.activateStorageDomainHsm,
+ (' ',
+ 'Activate a storage domain that is '
+ 'already a member in a storage pool '
+ 'that operates without Spm.'
+ )),
'deactivateStorageDomain': (serv.deactivateStorageDomain,
(' ',
diff --git a/vdsm/API.py b/vdsm/API.py
index bc47e3f..3703d69 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -991,6 +991,9 @@
def activate(self, storagepoolID):
return self._irs.activateStorageDomain(self._UUID, storagepoolID)
+def activateHsm(self, storagepoolID):
+return self._irs.activateStorageDomainHsm(self._UUID, storagepoolID)
+
def attach(self, storagepoolID):
return self._irs.attachStorageDomain(self._UUID, storagepoolID)
@@ -1109,7 +1112,8 @@
return self._irs.fenceSpmStorage(self._UUID, lastOwner, lastLver)
def getBackedUpVmsInfo(self, storagedomainID, vmList):
-return self._irs.getVmsInfo(self._UUID, storagedomainID, vmList)
+return self._irs.getBackedUpVmsInfo(self._UUID,
+storagedomainID, vmList)
def getBackedUpVmsList(self, storagedomainID):
return self._irs.getVmsList(self._UUID, storagedomainID)
diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py
index 039d564..c6fb25d 100644
--- a/vdsm/rpc/BindingXMLRPC.py
+++ b/vdsm/rpc/BindingXMLRPC.py
@@ -607,6 +607,10 @@
domain = API.StorageDomain(sdUUID)
return domain.activate(spUUID)
+def domainActivateHsm(self, sdUUID, spUUID):
+domain = API.StorageDomain(sdUUID)
+return domain.activateHsm(spUUID)
+
def domainAttach(self, sdUUID, spUUID, options=None):
domain = API.StorageDomain(sdUUID)
return domain.attach(spUUID)
@@ -1099,7 +1103,8 @@
'detachAnyStoragePoolHsm'),
(self.domainGetStoredVmsInfo, 'getStorageDomainStoredVmsInfo'),
(self.domainUpgradeVersion, 'upgradeStorageDomainVersion'),
-(self.domainUpdateVMData, 'updateStorageDomainVmData'))
+(self.domainUpdateVMData, 'updateStorageDomainVmData'),
+(self.domainActivateHsm, 'activateStorageDomainHsm'))
def getIrsMethods(self):
return ((self.domainActivate, 'activateStorageDomain'),
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index df11b53..d65ae22 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -4968,6 +4968,22 @@
'data': {'storagedomainID': 'UUID', 'storagepoolID': 'UUID'}}
##
+# @StorageDomain.activateHsm:
+#
+# Activate an attached but inactive Storage Domain.
+# Can be used only in Storage Pools that operate without
Change in vdsm[master]: adding StorageDomain.removeVmData()
Liron Aravot has uploaded a new change for review.
Change subject: adding StorageDomain.removeVmData()
..
adding StorageDomain.removeVmData()
Change-Id: Ic964ac5c9cb57df881b895356a203529ab2a171a
Signed-off-by: lara...@redhat.com
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/BindingXMLRPC.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
M vdsm/storage/sp.py
6 files changed, 90 insertions(+), 10 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/03/43103/2
diff --git a/client/vdsClient.py b/client/vdsClient.py
index 9c42228..2ddb536 100755
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -978,6 +978,15 @@
return res['status']['code'], res['status']['message']
return 0, ''
+def removeStorageDomainVmData(self, args):
+spUUID = args[0]
+vmUUID = args[1]
+sdUUID = args[2]
+res = self.s.removeStorageDomainVmData(sdUUID, spUUID, vmUUID)
+if res['status']['code']:
+return res['status']['code'], res['status']['message']
+return 0, ''
+
def _parseDomainsMap(self, domMapString):
"""
Parse domains map string: "sdUUID1=status1,sdUUID2=status2,..."
diff --git a/vdsm/API.py b/vdsm/API.py
index 73f1a1f..0748598 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -1062,6 +1062,10 @@
return self._irs.updateStorageDomainVmData(self._UUID, storagepoolID,
vmID, vmData)
+def removeVMData(self, storagepoolID, vmID):
+return self._irs.removeStorageDomainVmData(self._UUID, storagepoolID,
+ vmID)
+
def validate(self):
return self._irs.validateStorageDomain(self._UUID)
diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py
index 0ab159f..d227d7f 100644
--- a/vdsm/rpc/BindingXMLRPC.py
+++ b/vdsm/rpc/BindingXMLRPC.py
@@ -644,6 +644,10 @@
domain = API.StorageDomain(sdUUID)
return domain.updateVMData(spUUID, vmUUID, vmData)
+def domainRemoveVMData(self, sdUUID, spUUID, vmUUID):
+domain = API.StorageDomain(sdUUID)
+return domain.removeVMData(spUUID, vmUUID)
+
def domainDetachAnyStoragePoolHsm(self, sdUUID, hostID):
domain = API.StorageDomain(sdUUID)
return domain.detachAnyStoragePoolHsm(sdUUID, hostID)
@@ -1106,7 +1110,8 @@
(self.domainGetBackedUpVmsInfo,
'getStorageDomainBackedUpVmsInfo'),
(self.domainActivateHsm, 'activateStorageDomainHsm'),
-(self.domainUpdateVMData, 'updateStorageDomainVmData'))
+(self.domainUpdateVMData, 'updateStorageDomainVmData'),
+(self.domainRemoveVMData, 'removeStorageDomainVmData'))
def getIrsMethods(self):
return ((self.domainActivate, 'activateStorageDomain'),
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index 2173ac2..dee5e09 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -5051,6 +5051,23 @@
'data': {'storagedomainID': 'UUID', 'storagepoolID': 'UUID',
'vmID': 'UUID', 'vmData' : 'str'}}
+##
+# @StorageDomain.removeVMData:
+#
+# Remove a previously saved virtual machine definitio from a backup domain.
+#
+# @storagedomainID: The backup Storage Domain id
+#
+# @storagepoolID:The UUID of the Storage Pool
+#
+# @vmUUID: The id of the VM
+#
+# Since: 4.18.0
+##
+{'command': {'class': 'StorageDomain', 'name': 'removeVMData'},
+ 'data': {'storagedomainID': 'UUID', 'storagepoolID': 'UUID',
+ 'vmUUID': 'UUID'}}
+
##
# @StorageDomainCreateArgumentsBlock:
@@ -6192,6 +6209,24 @@
'data': {'storagepoolID': 'UUID', 'vmUUID': 'UUID',
'*storagedomainID': 'UUID'}}
+
+##
+# @StoragePool.removeVM:
+#
+# Remove a previously saved virtual machine definition.
+#
+# @storagepoolID:The UUID of the Storage Pool
+#
+# @vmUUID: Remove the saved definition of the VM with this UUID
+#
+# @storagedomainID: #optional The Storage Domain where the VM is stored
+#
+# Since: 4.10.0
+##
+{'command': {'class': 'StoragePool', 'name': 'removeVM'},
+ 'data': {'storagepoolID': 'UUID', 'vmUUID': 'UUID',
+ '*storagedomainID': 'UUID'}}
+
## Category: @Task
##
# @Task:
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 8302a52..346544a 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1490,8 +1490,18 @@
pool = self._prepareUpdateVM(spUUID, sdUUID)
pool.updateVMHSM(sdUUID, vmUUID, vmData)
+def _prepareRemoveVm(self, spUUID, sdUUID, vmUUID):
+vars.task.getSharedLock(STORAGE, spUUID)
+pool = self.getPool(spUUID)
+if not sdUUID or sdUUID == sd.BLANK_UUID:
+sdUUID = pool.masterDomain.sdUUID
+
+vars.task.getSharedLock(STORAG
Change in vdsm[master]: adding StorageDomain.activateHsm()
Liron Aravot has abandoned this change. Change subject: adding StorageDomain.activateHsm() .. Abandoned -- To view, visit https://gerrit.ovirt.org/43102 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ic661b44b060ddfeeb2a2caedb2662c6102b950e0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.removeVmData()
Liron Aravot has abandoned this change. Change subject: adding StorageDomain.removeVmData() .. Abandoned -- To view, visit https://gerrit.ovirt.org/43103 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ic964ac5c9cb57df881b895356a203529ab2a171a Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.removeVmData()
Liron Aravot has uploaded a new change for review.
Change subject: adding StorageDomain.removeVmData()
..
adding StorageDomain.removeVmData()
Change-Id: Ic53b19a43850341ee488b50fbb339fdf68c1a048
Signed-off-by: lara...@redhat.com
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/BindingXMLRPC.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
M vdsm/storage/sp.py
6 files changed, 106 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/43154/4
diff --git a/client/vdsClient.py b/client/vdsClient.py
index 6169be7..e2123e1 100755
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -978,6 +978,15 @@
return res['status']['code'], res['status']['message']
return 0, ''
+def removeStorageDomainVmData(self, args):
+spUUID = args[0]
+vmUUID = args[1]
+sdUUID = args[2]
+res = self.s.removeStorageDomainVmData(sdUUID, spUUID, vmUUID)
+if res['status']['code']:
+return res['status']['code'], res['status']['message']
+return 0, ''
+
def _parseDomainsMap(self, domMapString):
"""
Parse domains map string: "sdUUID1=status1,sdUUID2=status2,..."
diff --git a/vdsm/API.py b/vdsm/API.py
index 3703d69..ca37aff 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -1063,6 +1063,10 @@
return self._irs.updateStorageDomainVmData(self._UUID, storagepoolID,
vmID, vmData)
+def removeVMData(self, storagepoolID, vmID):
+return self._irs.removeStorageDomainVmData(self._UUID, storagepoolID,
+ vmID)
+
def validate(self):
return self._irs.validateStorageDomain(self._UUID)
diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py
index c6fb25d..9ee5cdd 100644
--- a/vdsm/rpc/BindingXMLRPC.py
+++ b/vdsm/rpc/BindingXMLRPC.py
@@ -644,6 +644,10 @@
domain = API.StorageDomain(sdUUID)
return domain.updateVMData(spUUID, vmUUID, vmData)
+def domainRemoveVMData(self, sdUUID, spUUID, vmUUID):
+domain = API.StorageDomain(sdUUID)
+return domain.removeVMData(spUUID, vmUUID)
+
def domainDetachAnyStoragePoolHsm(self, sdUUID, hostID):
domain = API.StorageDomain(sdUUID)
return domain.detachAnyStoragePoolHsm(sdUUID, hostID)
@@ -1104,7 +1108,8 @@
(self.domainGetStoredVmsInfo, 'getStorageDomainStoredVmsInfo'),
(self.domainUpgradeVersion, 'upgradeStorageDomainVersion'),
(self.domainUpdateVMData, 'updateStorageDomainVmData'),
-(self.domainActivateHsm, 'activateStorageDomainHsm'))
+(self.domainActivateHsm, 'activateStorageDomainHsm'),
+(self.domainRemoveVMData, 'removeStorageDomainVmData'))
def getIrsMethods(self):
return ((self.domainActivate, 'activateStorageDomain'),
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index d65ae22..afbd960 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -5051,6 +5051,23 @@
'data': {'storagedomainID': 'UUID', 'storagepoolID': 'UUID',
'vmID': 'UUID', 'vmData' : 'str'}}
+##
+# @StorageDomain.removeVMData:
+#
+# Remove a previously saved virtual machine definitio from a backup domain.
+#
+# @storagedomainID: The backup Storage Domain id
+#
+# @storagepoolID:The UUID of the Storage Pool
+#
+# @vmUUID: The id of the VM
+#
+# Since: 4.18.0
+##
+{'command': {'class': 'StorageDomain', 'name': 'removeVMData'},
+ 'data': {'storagedomainID': 'UUID', 'storagepoolID': 'UUID',
+ 'vmUUID': 'UUID'}}
+
##
# @StorageDomainCreateArgumentsBlock:
@@ -6192,6 +6209,24 @@
'data': {'storagepoolID': 'UUID', 'vmUUID': 'UUID',
'*storagedomainID': 'UUID'}}
+
+##
+# @StoragePool.removeVM:
+#
+# Remove a previously saved virtual machine definition.
+#
+# @storagepoolID:The UUID of the Storage Pool
+#
+# @vmUUID: Remove the saved definition of the VM with this UUID
+#
+# @storagedomainID: #optional The Storage Domain where the VM is stored
+#
+# Since: 4.10.0
+##
+{'command': {'class': 'StoragePool', 'name': 'removeVM'},
+ 'data': {'storagepoolID': 'UUID', 'vmUUID': 'UUID',
+ '*storagedomainID': 'UUID'}}
+
## Category: @Task
##
# @Task:
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index db82566..dda8895 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1490,8 +1490,18 @@
pool = self._prepareUpdateVM(spUUID, sdUUID)
pool.updateVMHSM(sdUUID, vmUUID, vmData)
+def _prepareRemoveVm(self, spUUID, sdUUID, vmUUID):
+vars.task.getSharedLock(STORAGE, spUUID)
+pool = self.getPool(spUUID)
+if not sdUUID or sdUUID == sd.BLANK_UUID:
+sdUUID = pool.masterDomai
Change in vdsm[master]: adding StorageDomain.removeVmData()
Liron Aravot has abandoned this change. Change subject: adding StorageDomain.removeVmData() .. Abandoned -- To view, visit https://gerrit.ovirt.org/43154 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ic53b19a43850341ee488b50fbb339fdf68c1a048 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.updateVmData()
Liron Aravot has uploaded a new change for review.
Change subject: adding StorageDomain.updateVmData()
..
adding StorageDomain.updateVmData()
Change-Id: Id79e7d6375e60216d1eb9a58e5a5c087db98625f
Signed-off-by: lara...@redhat.com
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/BindingXMLRPC.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
M vdsm/storage/sp.py
6 files changed, 79 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/42930/5
diff --git a/client/vdsClient.py b/client/vdsClient.py
index c3628c5..d59ae7c 100755
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -709,6 +709,16 @@
return dom['status']['code'], dom['status']['message']
return 0, ''
+def updateStorageDomainVmData(self, args):
+sdUUID = args[0]
+spUUID = args[1]
+vmUUID = args[2]
+vmData = args[3]
+dom = self.s.updateStorageDomainVmData(sdUUID, spUUID, vmUUID, vmData)
+if dom['status']['code']:
+return dom['status']['code'], dom['status']['message']
+return 0, ''
+
def forcedDetachStorageDomain(self, args):
sdUUID = args[0]
spUUID = args[1]
diff --git a/vdsm/API.py b/vdsm/API.py
index 3007d93..bc47e3f 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -1056,6 +1056,10 @@
return self._irs.getStorageDomainStoredVmsInfo(storagepoolID,
self._UUID, vmList)
+def updateVMData(self, storagepoolID, vmID, vmData):
+return self._irs.updateStorageDomainVmData(self._UUID, storagepoolID,
+ vmID, vmData)
+
def validate(self):
return self._irs.validateStorageDomain(self._UUID)
diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py
index 5294e6a..039d564 100644
--- a/vdsm/rpc/BindingXMLRPC.py
+++ b/vdsm/rpc/BindingXMLRPC.py
@@ -636,6 +636,10 @@
domain = API.StorageDomain(sdUUID)
return domain.detachHsm(spUUID, force)
+def domainUpdateVMData(self, sdUUID, spUUID, vmUUID, vmData):
+domain = API.StorageDomain(sdUUID)
+return domain.updateVMData(spUUID, vmUUID, vmData)
+
def domainDetachAnyStoragePoolHsm(self, sdUUID, hostID):
domain = API.StorageDomain(sdUUID)
return domain.detachAnyStoragePoolHsm(sdUUID, hostID)
@@ -1094,7 +1098,8 @@
(self.domainDetachAnyStoragePoolHsm,
'detachAnyStoragePoolHsm'),
(self.domainGetStoredVmsInfo, 'getStorageDomainStoredVmsInfo'),
-(self.domainUpgradeVersion, 'upgradeStorageDomainVersion'))
+(self.domainUpgradeVersion, 'upgradeStorageDomainVersion'),
+(self.domainUpdateVMData, 'updateStorageDomainVmData'))
def getIrsMethods(self):
return ((self.domainActivate, 'activateStorageDomain'),
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index 7d514e3..df11b53 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -5017,6 +5017,26 @@
'returns': 'OVFMap'}
##
+# @StorageDomain.updateVMData:
+#
+# Store virtual machine OVF file on a backup Storage Domain
+#
+# @storagepoolID:The UUID of the Storage Pool
+#
+# @storagedomainID: The backup Storage Domain id
+#
+# @vmID: The id of the VM
+#
+# @vmData: The VM definition in OVF format
+#
+# Since: 4.18.0
+##
+{'command': {'class': 'StorageDomain', 'name': 'updateVMData'},
+ 'data': {'storagedomainID': 'UUID', 'storagepoolID': 'UUID',
+ 'vmID': 'UUID', 'vmData' : 'str'}}
+
+
+##
# @StorageDomainCreateArgumentsBlock:
#
# Creation argument for Block-based Storage Domains.
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index b1687da..1c76208 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1418,6 +1418,15 @@
pool = self.getPool(spUUID)
pool.setVolumeLegality(sdUUID, imgUUID, volUUID, legality)
+def _prepareUpdateVM(self, spUUID, sdUUID):
+vars.task.getSharedLock(STORAGE, spUUID)
+pool = self.getPool(spUUID)
+if not sdUUID or sdUUID == sd.BLANK_UUID:
+sdUUID = pool.masterDomain.sdUUID
+
+vars.task.getExclusiveLock(STORAGE, "vms_" + sdUUID)
+return pool
+
@public
def updateVM(self, spUUID, vmList, sdUUID=None, options=None):
"""
@@ -1438,15 +1447,31 @@
:type sdUUID: UUID
:param options: ?
"""
-vars.task.getSharedLock(STORAGE, spUUID)
-pool = self.getPool(spUUID)
-if not sdUUID or sdUUID == sd.BLANK_UUID:
-sdUUID = pool.masterDomain.sdUUID
-
-vars.task.getExclusiveLock(STORAGE, "vms_" + sdUUID)
+pool = self._prepareUpdateVM(spUUID, sdUUID)
pool.updateVM(vmList=vmList, sdUUID=sdUUID)
@public
+def updateStorageDomainVmData(s
Change in vdsm[master]: adding the StorageDomain.upgradeVersion verb
Liron Aravot has abandoned this change. Change subject: adding the StorageDomain.upgradeVersion verb .. Abandoned -- To view, visit https://gerrit.ovirt.org/42478 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Id8fb87c6e04e48072ec90d07a0f4c7a61411a808 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding the StorageDomain.upgradeVersion verb
Liron Aravot has uploaded a new change for review.
Change subject: adding the StorageDomain.upgradeVersion verb
..
adding the StorageDomain.upgradeVersion verb
Change-Id: Id8fb87c6e04e48072ec90d07a0f4c7a61411a808
Signed-off-by: lara...@redhat.com
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/BindingXMLRPC.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
M vdsm/storage/imageRepository/formatConverter.py
M vdsm/storage/sp.py
M vdsm/storage/storage_exception.py
8 files changed, 148 insertions(+), 24 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/78/42478/7
diff --git a/client/vdsClient.py b/client/vdsClient.py
index e17ba76..c3628c5 100755
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -682,6 +682,15 @@
return dom['status']['code'], dom['status']['message']
return 0, ''
+def upgradeStorageDomainVersion(self, args):
+sdUUID = args[0]
+spUUID = args[1]
+version = args[2]
+dom = self.s.upgradeStorageDomainVersion(sdUUID, spUUID, version)
+if dom['status']['code']:
+return dom['status']['code'], dom['status']['message']
+return 0, ''
+
def detachStorageDomainAnyStoragePoolHsm(self, args):
sdUUID = args[0]
hostID = args[1]
@@ -2521,6 +2530,12 @@
'domain metadata is not modified to'
'reflect its detached.'
)),
+'upgradeStorageDomainVersion': (serv.upgradeStorageDomainVersion,
+(' ',
+ 'Upgrades a storage domain that is'
+ 'part of connected storage pool to'
+ 'the given version.'
+ )),
'detachStorageDomainAnyStoragePoolHsm':
(serv.detachStorageDomainAnyStoragePoolHsm,
('' '',
diff --git a/vdsm/API.py b/vdsm/API.py
index 3212616..3007d93 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -1023,6 +1023,10 @@
def detachAnyStoragePoolHsm(self, hostID):
return self._irs.detachAnyStoragePoolHsm(self._UUID, hostID)
+def upgradeVersion(self, storagepoolID, version):
+return self._irs.upgradeStorageDomainVersion(self._UUID, storagepoolID,
+ version)
+
def extend(self, storagepoolID, devlist, force=False):
return self._irs.extendStorageDomain(self._UUID, storagepoolID,
devlist, force)
diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py
index 70db4ed..5294e6a 100644
--- a/vdsm/rpc/BindingXMLRPC.py
+++ b/vdsm/rpc/BindingXMLRPC.py
@@ -640,6 +640,10 @@
domain = API.StorageDomain(sdUUID)
return domain.detachAnyStoragePoolHsm(sdUUID, hostID)
+def domainUpgradeVersion(self, sdUUID, spUUID, version):
+domain = API.StorageDomain(sdUUID)
+return domain.upgradeVersion(spUUID, version)
+
def domainDetachForced(self, sdUUID, spUUID, options=None):
domain = API.StorageDomain(sdUUID)
return domain.detach(spUUID, None, None, force=True)
@@ -1089,7 +1093,8 @@
(self.domainDetachHsm, 'detachStorageDomainHsm'),
(self.domainDetachAnyStoragePoolHsm,
'detachAnyStoragePoolHsm'),
-(self.domainGetStoredVmsInfo, 'getStorageDomainStoredVmsInfo'))
+(self.domainGetStoredVmsInfo, 'getStorageDomainStoredVmsInfo'),
+(self.domainUpgradeVersion, 'upgradeStorageDomainVersion'))
def getIrsMethods(self):
return ((self.domainActivate, 'activateStorageDomain'),
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index 86cfb6e..7d514e3 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -5173,6 +5173,22 @@
'data': {'storagedomainID': 'UUID', 'hostID' : 'int'}}
##
+# @StorageDomain.upgradeVersion:
+#
+# Upgrades the StorageDomain version
+#
+# @storagedomainID: The UUID of the Storage Domain
+#
+# @storagepoolID:The UUID of the Storage Pool
+#
+# @version:The target version
+#
+# Since: 4.18.0
+##
+{'command': {'class': 'StorageDomain', 'name': 'upgradeVersion'},
+ 'data': {'storagedomainID': 'UUID', 'storagepoolID': 'UUID', 'version':
'int'}}
+
+##
# @StorageDomain.extend:
#
# Extend a block-based Storage Domain onto more block devices.
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 95c4803..b1687da 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -844,6 +844,11 @@
pool.detachSDHsm(sdUUID)
@public
+def upgradeStorageDomainVersion(self, sdUUID, spUUID, version):
+pool = self.getPool(spUUID)
+pool.upgradeDomainFormat(sdUUID, version)
+
+@publ
Change in vdsm[master]: adding StorageDomain.updateVmData()
Liron Aravot has abandoned this change. Change subject: adding StorageDomain.updateVmData() .. Abandoned -- To view, visit https://gerrit.ovirt.org/42930 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Id79e7d6375e60216d1eb9a58e5a5c087db98625f Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: Remove image run links in teardownImage
Liron Aravot has posted comments on this change. Change subject: hsm: Remove image run links in teardownImage .. Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/53298/2/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 1119: Line 1120: If the image is based on a template image it should be expressly Line 1121: deactivated. Line 1122: """ Line 1123: self.removeImageLinks(imgUUID) > when activating we create the links as the last operation, dont we prefer t i'm not sure if its preferred, because that way we have a stale link till the deactivation (link removal) is complete, on the other hand..if the lvs deactivation fails for some reason we remain without the link. Line 1124: allVols = self.getAllVolumes() Line 1125: volUUIDs = self._manifest._getImgExclusiveVols(imgUUID, allVols) Line 1126: lvm.deactivateLVs(self.sdUUID, volUUIDs) Line 1127: -- To view, visit https://gerrit.ovirt.org/53298 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4568aaac2ce26c67132a839d95b75dd97bb28f29 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Simone Tiraboschi Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hsm: Remove image run links in teardownImage
Liron Aravot has posted comments on this change. Change subject: hsm: Remove image run links in teardownImage .. Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/53298/2/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 1119: Line 1120: If the image is based on a template image it should be expressly Line 1121: deactivated. Line 1122: """ Line 1123: self.removeImageLinks(imgUUID) when activating we create the links as the last operation, dont we prefer to keep the symmetry and on deactivation have it as the last operation? Line 1124: allVols = self.getAllVolumes() Line 1125: volUUIDs = self._manifest._getImgExclusiveVols(imgUUID, allVols) Line 1126: lvm.deactivateLVs(self.sdUUID, volUUIDs) Line 1127: -- To view, visit https://gerrit.ovirt.org/53298 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4568aaac2ce26c67132a839d95b75dd97bb28f29 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Simone Tiraboschi Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: update domain links on state change
Liron Aravot has posted comments on this change.
Change subject: sp: update domain links on state change
..
Patch Set 7:
(1 comment)
https://gerrit.ovirt.org/#/c/51393/7/vdsm/storage/sp.py
File vdsm/storage/sp.py:
Line 145: domain = sdCache.produce(sdUUID)
Line 146: with rmanager.acquireResource(STORAGE, self.spUUID,
Line 147: rm.LockType.shared):
Line 148: if sdUUID not in self.getDomains(activeOnly=True):
Line 149: self.log.debug("Domain %s is not active, "
> +1
*a active*
Line 150:"skipping domain links refresh",
Line 151:sdUUID)
Line 152: return
Line 153: with rmanager.acquireResource(STORAGE, sdUUID +
"_refresh",
--
To view, visit https://gerrit.ovirt.org/51393
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Ala Hino
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Freddy Rolland
Gerrit-Reviewer: Idan Shaby
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: update domain links on state change
Liron Aravot has posted comments on this change.
Change subject: sp: update domain links on state change
..
Patch Set 7: Code-Review+1
(2 comments)
https://gerrit.ovirt.org/#/c/51393/7/vdsm/storage/sp.py
File vdsm/storage/sp.py:
Line 145: domain = sdCache.produce(sdUUID)
Line 146: with rmanager.acquireResource(STORAGE, self.spUUID,
Line 147: rm.LockType.shared):
Line 148: if sdUUID not in self.getDomains(activeOnly=True):
Line 149: self.log.debug("Domain %s is not active, "
consider /s/active/active pool domain
Line 150:"skipping domain links refresh",
Line 151:sdUUID)
Line 152: return
Line 153: with rmanager.acquireResource(STORAGE, sdUUID +
"_refresh",
Line 149: self.log.debug("Domain %s is not active, "
Line 150:"skipping domain links refresh",
Line 151:sdUUID)
Line 152: return
Line 153: with rmanager.acquireResource(STORAGE, sdUUID +
"_refresh",
consider to change to _refresh_links
Line 154: rm.LockType.exclusive):
Line 155: self.log.debug("Refreshing domain links for %s",
sdUUID)
Line 156: self._refreshDomainLinks(domain)
Line 157:
--
To view, visit https://gerrit.ovirt.org/51393
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Ala Hino
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Freddy Rolland
Gerrit-Reviewer: Idan Shaby
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: improve domainStateChange event handling
Liron Aravot has posted comments on this change. Change subject: sp: improve domainStateChange event handling .. Patch Set 6: -Code-Review -- To view, visit https://gerrit.ovirt.org/51393 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: update domain links on state change
Liron Aravot has posted comments on this change. Change subject: sp: update domain links on state change .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/27334/4/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 144: (): I know that this was merged to 3.4, but what will happen when domain is deactivated? will we refresh the domain links? seems to me like we need to use self.getDomain(activeOnly=True) -- To view, visit https://gerrit.ovirt.org/27334 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ac20e4b515472b24c35b2cccd2ad3dc98b3574c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Idan Shaby Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Xavi Francisco Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: sp: improve domainStateChange event handling
Liron Aravot has posted comments on this change. Change subject: sp: improve domainStateChange event handling .. Patch Set 3: Code-Review-1 (1 comment) -1 till my question is answered. https://gerrit.ovirt.org/#/c/51393/3/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 649: self.id = hostID Line 650: # Make sure SDCache doesn't have stale data (it can be in case of FC) Line 651: sdCache.invalidateStorage() Line 652: sdCache.refresh() Line 653: self._startWatchingDomainsState() what will happen if we are conencted to the pool and issue connect call again? what are the implications of registering twice? Line 654: try: Line 655: # Rebuild whole Pool Line 656: self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion) Line 657: except Exception: -- To view, visit https://gerrit.ovirt.org/51393 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: jobs: Return job id in Host.getJobs response
Liron Aravot has posted comments on this change. Change subject: jobs: Return job id in Host.getJobs response .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/51147 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50f0db0a2b1ba939a19ea232320e4eff1ca4dd39 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Freddy Rolland 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: jobs: Fix job error format
Liron Aravot has posted comments on this change. Change subject: jobs: Fix job error format .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/51145 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2fdc6db6ed7519e5860d34e3a602dbeecce9a998 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: jobs: Return job id in Host.getJobs response
Liron Aravot has posted comments on this change. Change subject: jobs: Return job id in Host.getJobs response .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/51147 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I50f0db0a2b1ba939a19ea232320e4eff1ca4dd39 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Freddy Rolland 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 https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: jobs: Fix job error format
Liron Aravot has posted comments on this change. Change subject: jobs: Fix job error format .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/51145 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2fdc6db6ed7519e5860d34e3a602dbeecce9a998 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fileSD: Keep deleted images names unique
Liron Aravot has posted comments on this change. Change subject: fileSD: Keep deleted images names unique .. Patch Set 2: Code-Review-1 this patch needs an update after https://gerrit.ovirt.org/48477 was merged. -- To view, visit https://gerrit.ovirt.org/48032 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia0582896d7ede028e9096da1a688e3e449ab0258 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: perform image deletion in task
Liron Aravot has posted comments on this change. Change subject: storage: perform image deletion in task .. Patch Set 8: Verified+1 Verified by Elad and myself. -- To view, visit https://gerrit.ovirt.org/48477 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e61d38439192d193f3f806e08661b61a65e7836 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: storage: perform image deletion in task
Liron Aravot has posted comments on this change. Change subject: storage: perform image deletion in task .. Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/48477/4/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 1539 Line 1540 Line 1541 Line 1542 Line 1543 > This comment explains why we delete the images in the synchronous state - w we still rename the image synchronously (which essentially deletes it), don't you prefer to leave the comment? Line 1532: else: Line 1533: if fakeTUUID: Line 1534: tParams = dom.produceVolume(imgUUID, fakeTUUID).\ Line 1535: getVolumeParams() Line 1536: > will be removed Done Line 1537: pool.deleteImage(dom, imgUUID, volsByImg) Line 1538: # This is a hack to keep the interface consistent Line 1539: # We currently have race conditions in delete image, to quickly fix Line 1540: # this we delete images in the "synchronous" state. This only works -- To view, visit https://gerrit.ovirt.org/48477 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e61d38439192d193f3f806e08661b61a65e7836 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fileSD: Do not try to remove non-existent lease file
Liron Aravot has posted comments on this change. Change subject: fileSD: Do not try to remove non-existent lease file .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/50589 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I525704678707dfef40e34497bc4917dd19d73034 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: API: Add api for getting host jobs info
Liron Aravot has posted comments on this change. Change subject: API: Add api for getting host jobs info .. Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/49451/2/lib/vdsm/jobs.py File lib/vdsm/jobs.py: Line 159: : why not keeping the old implementation and add the new condition? if we are changing this, why not use a dict (get by the id) rather then iterate over all of them? -- To view, visit https://gerrit.ovirt.org/49451 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I957938ea9ca55d2499d09dfed39821c85e3e9d48 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
