Nir Soffer has posted comments on this change. Change subject: vm: enable disk stats collection on recovery ......................................................................
Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/32406/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 2895: else: Line 2896: for drive in devices[DISK_DEVICES]: Line 2897: if drive['device'] == 'disk' and isVdsmImage(drive): Line 2898: self.sdIds.append(drive['domainID']) Line 2899: self.startDisksStatsCollection() > Good point. I was under the impression something deeper was lurking behind, Sounds good, but I don't think we need to cache irs state. We also do not need to access irs, just use: if self._vm.cif.ready which does all the needed checks. Since this value starts at False and changes once to True, there is no problem in accessing from different threads. And please update the commit message about the effect of this patch. Opening a bug about this is also needed not also because we need to backport it, but for documentation. If we see cif.ready in profiles we can add local cache. Lets keep the code simple. Line 2900: Line 2901: for devType, devClass in self.DeviceMapping: Line 2902: for dev in devices[devType]: Line 2903: self._devices[devType].append(devClass(self.conf, self.log, -- To view, visit http://gerrit.ovirt.org/32406 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibd7d32373f5a0a7dd4ef88c5a0ad5a9f3d52938c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
