Yeela Kaplan has posted comments on this change.

Change subject: oop: Add an option to configure oop implementation
......................................................................


Patch Set 26:

(4 comments)

http://gerrit.ovirt.org/#/c/26576/26/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 359:             multipath.setupMultipath()
Line 360: 
Line 361:         self.__validateLvmLockingType()
Line 362: 
Line 363:         oop.setDefaultImpl(config.get('irs', 'oop_impl'))
> you can read it directly in outOfProcess.py as you read other config values
Saggi referred to this comment in a previous patchset If I remember correctly.


We want the config file to be accessed only in hsm...
Line 364: 
Line 365:         self.domainStateChangeCallbacks = set()
Line 366: 
Line 367:         # cleanStorageRepoitory uses tasksDir value, this must be 
assigned


http://gerrit.ovirt.org/#/c/26576/26/vdsm/storage/outOfProcess.py
File vdsm/storage/outOfProcess.py:

Line 1: #
Line 2: # Copyright 2011 Red Hat, Inc.
> date should be updated
Done
Line 3: #
Line 4: # This program is free software; you can redistribute it and/or modify
Line 5: # it under the terms of the GNU General Public License as published by
Line 6: # the Free Software Foundation; either version 2 of the License, or


Line 20: import types
Line 21: import os
Line 22: import errno
Line 23: import logging
Line 24: import stat
> imports are better when kept sorted (it's easier to handle parallel commits
Done
Line 25: import sys
Line 26: 
Line 27: from vdsm.config import config
Line 28: import threading


Line 107:         return partial(self._procPool.callCrabRPCFunction, 
self._timeout,
Line 108:                        fullName)
Line 109: 
Line 110: 
Line 111: def OopWrapper(procPool, ioproc=None):
> isn't the OopWrapper private and being used only in this file ?
Done
Line 112:     return _ModuleWrapper("oop", procPool, ioproc, DEFAULT_TIMEOUT,
Line 113:                           (("os",
Line 114:                             ("path",)),
Line 115:                            "glob",


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd756afd43d23631dc7ed4bac64bec9a81b358b4
Gerrit-PatchSet: 26
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykap...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to