Change in vdsm[master]: blockSD: Ensure active volumes are refreshed before use.

nsoffer at redhat.com nsoffer at redhat.com
Sun Nov 17 19:04:14 UTC 2013


Nir Soffer has posted comments on this change.

Change subject: blockSD: Ensure active volumes are refreshed before use.
......................................................................


Patch Set 6:

(3 comments)

This looks like the right direction.

I would happier if the changes are separated to multiple patches:

1. Support multiple lvs in refreshLVs.
2. Make lvm._isActiveLV public
3. Use new api to refresh active lvs instead of activate them

....................................................
File vdsm/storage/blockSD.py
Line 1067: 
Line 1068:         If the image is based on a template image it will be activated.
Line 1069:         """
Line 1070:         toRefresh = [vol for vol in volUUIDs if vol not in SPECIAL_LVS and
Line 1071:                      os.path.exists(lvm.lvPath(self.sdUUID, vol))]
I think we need lvm.isActiveLV(vgname, lvname) instead of using os.path.exists in upper layers.

There is a private lvm._isActiveLV that can be change to be public.
Line 1072:         lvm.activateLVs(self.sdUUID, volUUIDs)
Line 1073: 
Line 1074:         if toRefresh:
Line 1075:             log.warning("The following volumes are already active and will be"


Line 1068:         If the image is based on a template image it will be activated.
Line 1069:         """
Line 1070:         toRefresh = [vol for vol in volUUIDs if vol not in SPECIAL_LVS and
Line 1071:                      os.path.exists(lvm.lvPath(self.sdUUID, vol))]
Line 1072:         lvm.activateLVs(self.sdUUID, volUUIDs)
If you know that some lvs are already active, why do you try to activate them again?
Line 1073: 
Line 1074:         if toRefresh:
Line 1075:             log.warning("The following volumes are already active and will be"
Line 1076:                         " refreshed before use : %s", toRefresh)


....................................................
File vdsm/storage/lvm.py
Line 1148:     _lvminfo._lvs.pop((vg, oldlv), None)
Line 1149:     _lvminfo._reloadlvs(vg, newlv)
Line 1150: 
Line 1151: 
Line 1152: def refreshLV(vgName, lvNames):
Rename to refreshLVs - as you used in blockSD.py
Line 1153:     # If  the  logical  volumes  are active, reload their metadata.
Line 1154:     cmd = ['lvchange', '--refresh']
Line 1155:     cmd.extend("%s/%s" % (vgName, lv) for lv in lvNames)
Line 1156:     rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))


-- 
To view, visit http://gerrit.ovirt.org/19871
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ecc64b8ca0133b030ba5bfa37f1a2c55067dd5d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lee Yarwood <lyarwood at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Lee Yarwood <lyarwood at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list