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 <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- [email protected] To unsubscribe send an email to [email protected]
