Change in vdsm[master]: sd: Introduce StorageDomainChecker

nsoffer at redhat.com nsoffer at redhat.com
Mon Mar 14 17:58:09 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: sd: Introduce StorageDomainChecker
......................................................................


Patch Set 2:

(3 comments)

https://gerrit.ovirt.org/#/c/54375/2/tests/manifest_tests.py
File tests/manifest_tests.py:

Line 36
Line 37
Line 38
Line 39
Line 40
> You will need to add tests for the checker.
Sure, if we agree that this is the right approach.


https://gerrit.ovirt.org/#/c/54375/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 420:         if now - self._last_vg_check > timeout:
Line 421:             self._last_vg_check = now
Line 422:             lvm.chkVG(self._sd_uuid)
Line 423:         elif lvm.getVG(self._sd_uuid).partial != lvm.VG_OK:
Line 424:             raise se.StorageDomainAccessError(self._sd_uuid)
> It's not obvious to me how simply moving the same checks from the BlockSD i
Moving vgck here does not improve anything, I think it is a mistake. The checker should only do the read delay check, so it can never block because of lame thread management in vdsm, only because storage is not accessible.
Line 425: 
Line 426:     def stats(self):
Line 427:         return vg_stats(self._sd_uuid)
Line 428: 


Line 423:         elif lvm.getVG(self._sd_uuid).partial != lvm.VG_OK:
Line 424:             raise se.StorageDomainAccessError(self._sd_uuid)
Line 425: 
Line 426:     def stats(self):
Line 427:         return vg_stats(self._sd_uuid)
I'm not sure these stats are even correct since they are cached - it would be best to remove them from the monitor response, since they are not about storage availability.

The stats are probably correct only when we invalidate the lvm cache, which old used used to every 5 minutes, so we probably check them in another thread, scheduled every 5 minutes or so, not in the monitor thread.
Line 428: 
Line 429:     def __repr__(self):
Line 430:         return ("<{self.__class__.__name__} "
Line 431:                 "sd_uuid={self._sd_uuid}, "


-- 
To view, visit https://gerrit.ovirt.org/54375
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib801c54515baaa6b04098f97c8df70c711535200
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list