Nir Soffer has posted comments on this change. Change subject: storage: Resize devices on Storage Pool Connect ......................................................................
Patch Set 2: (2 comments) Nice! https://gerrit.ovirt.org/#/c/40864/2/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 1195: # create domain special volumes folder Line 1196: fileUtils.createdir(os.path.join(domaindir, sd.DOMAIN_META_DATA)) Line 1197: fileUtils.createdir(os.path.join(domaindir, sd.DOMAIN_IMAGES)) Line 1198: # resize devices if needed Line 1199: for pv in blockSD.lvm.listPVNames(domUUID): This is not a pv, this a pvname - we should be specific and consistent with these terms. Line 1200: try: Line 1201: multipath.resize_if_needed(os.path.basename(pv)) Line 1202: except Exception: Line 1203: self.log.exception("Could not resize device %s ", pv) Line 1197: fileUtils.createdir(os.path.join(domaindir, sd.DOMAIN_IMAGES)) Line 1198: # resize devices if needed Line 1199: for pv in blockSD.lvm.listPVNames(domUUID): Line 1200: try: Line 1201: multipath.resize_if_needed(os.path.basename(pv)) We don't want to handle errors in os.path.basename, and we also want to make it clear why we call it. Using explaining parameter achieve both goals: guid = os.path.basename(pvname) try: multipath.resize_if_needed(guid) Also check if we have some api (maybe in lvm.py) for extracting the pv guid from a pv name. os.path.basename works but means the the caller know the implementation details. Line 1202: except Exception: Line 1203: self.log.exception("Could not resize device %s ", pv) Line 1204: Line 1205: # Add the file domains -- To view, visit https://gerrit.ovirt.org/40864 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia574a59047d2fb5dd3ac7360c2c3a9dde05237ef Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Fred Rolland <froll...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
