Change in vdsm[master]: Invalidate filters on HSMs before rescanning extended VG

nsoffer at redhat.com nsoffer at redhat.com
Wed Oct 30 19:43:55 UTC 2013


Nir Soffer has posted comments on this change.

Change subject: Invalidate filters  on HSMs before rescanning extended VG
......................................................................


Patch Set 16: Code-Review-1

(2 comments)

Looks good, but I think it can more correct.

....................................................
File vdsm/storage/hsm.py
Line 1985:     def scanDevicesVisibility(self, guids):
Line 1986:         visible = lambda guid: (guid, os.path.exists(
Line 1987:                                 os.path.join("/dev/mapper", guid)))
Line 1988:         visibleDevs = map(visible, guids)
Line 1989:         if not all(visibleDevs):
This seems to be the correct way to invalidate the filters - we found a device which is not visible.

Can you validate this?
Line 1990:             multipath.rescan()
Line 1991:             visibleDevs = map(visible, guids)
Line 1992:         return dict(visibleDevs)
Line 1993: 


Line 2004:                   boolean
Line 2005:         :rtype: dict
Line 2006:         """
Line 2007:         visibility = self.scanDevicesVisibility(guids)
Line 2008:         lvm.invalidateFilter()
I suspect that this is not the most correct way to invalidate the filter. If all guids are visible, the filters are probably correct as well.

If we choose to invalidate the filters here anyway, it should be done either before calling scanDevicesVisibility, or after processing visibility dict. Not sure why you chose to insert the call in the middle of the existing code.
Line 2009:         for guid in guids:
Line 2010:             if visibility[guid]:
Line 2011:                 visibility[guid] = (os.stat('/dev/mapper/' + guid).st_mode &
Line 2012:                                     stat.S_IRUSR != 0)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov <pzhukov 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: Pavel Zhukov <pzhukov at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Tomáš Došek <tdosek 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