Dan Kenigsberg has posted comments on this change.
Change subject: Remove unused TaskResource class
......................................................................
Patch Set 1: Verified; Looks good to me, approved
should have been dropped by
BZ#581243 - Refactored resource system to more easily allow new features
--
To view, visit http://gerrit.usersys.redhat.com/980
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib487ff8661c771b765afed1f97327ef88c798745
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Correct typos for 'successfully'
......................................................................
Correct typos for 'successfully'
Change-Id: I8e5bfc5911f3a981e20bf721fb348c0bcd4e9d5d
---
M vdsm/storage/hsm.py
M vdsm/storage/task.py
2 files changed, 3 insertions(+), 3 deletions(-)
Approvals:
Dan Kenigsberg: Verified; Looks good to me, approved
--
To view, visit http://gerrit.usersys.redhat.com/979
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I8e5bfc5911f3a981e20bf721fb348c0bcd4e9d5d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: Correct typos for 'successfully'
......................................................................
Patch Set 1: Verified; Looks good to me, approved
better rush it in before Saggi's changes...
--
To view, visit http://gerrit.usersys.redhat.com/979
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e5bfc5911f3a981e20bf721fb348c0bcd4e9d5d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#741658 Move VM to Down if libvirt reports VIR_ERR_NO_DOMAIN
......................................................................
Patch Set 2: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.usersys.redhat.com/984
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6262fb2504534c56fdec246937423d629f92e13
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: Made spm securing logic more generic
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/storage/securable.py
Line -3
Line -2
Line -1
Line 0
Line 1: from threading import Event
Let's live up to the Berrange bar and add a GPL notice here.
Line 2: from functools import wraps
Line 3:
Line 4: class SecureError(RuntimeError): pass
Line 5:
Line -1
Line 0
Line 1: from threading import Event
Line 2: from functools import wraps
Line 3:
and a docstring would be great.
Line 4: class SecureError(RuntimeError): pass
Line 5:
Line 6: class Securable(type):
Line 7: def __new__(mcs, name, bases, fdict):
....................................................
File vdsm/storage/spm.py
Line 293: self.log.debug("Registering with state change event")
Line 294: sp.StatsThread.onDomainConnectivityStateChange.register(self._upgradePoolDomain)
Line 295: self.log.debug("Running initial domain upgrade threads")
Line 296: for sdUUID in self._domainsToUpgrade:
Line 297: threading.Thread(target=self._upgradePoolDomain, args=(sdUUID, True), kwargs={"__securityOverride": True}).start()
unrelated, but still: WARNING, nondeamonic long thread.
Line 298:
Line 299: @secured
Line 300: def _upgradePoolDomain(self, sdUUID, isValid):
Line 301: # This method is called everytime the onDomainConnectivityStateChange
--
To view, visit http://gerrit.usersys.redhat.com/991
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e16feafc03d6efe5c904b18d4d27177dd14b905
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: Made the logging mechanism more generic
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
but a happy step forward.
....................................................
File vdsm/logUtils.py
Line 45:
Line 46: def call2str(func, args, kwargs, printers={}):
Line 47: kwargs = kwargs.copy()
Line 48: varnames = func.func_code.co_varnames[:func.func_code.co_argcount]
Line 49: if hasattr(func, "im_func"):
suddenly you do not like ismethod()?
Line 50: args = [func.im_self] + list(args)
Line 51: func = func.im_func
Line 52:
Line 53: for name, val in zip(varnames, args):
Line 52:
Line 53: for name, val in zip(varnames, args):
Line 54: kwargs[name] = val
Line 55:
Line 56: for name, val in zip(reversed(varnames), reversed(func.func_defaults) if func.func_defaults else []):
after several minutes I figured out why you reverse the lists.
zip( varnames[-len(func.func_defaults or []):], func.func_defaults or [])
would be clearer.
Line 57: if name not in kwargs:
Line 58: kwargs[name] = val
Line 59:
Line 60: argsStrs = []
Line 58: kwargs[name] = val
Line 59:
Line 60: argsStrs = []
Line 61: for argName in varnames:
Line 62: if argName == "self":
not very nice, since "self" has no syntactic power. couldn't you behead varnames after line 49?
Line 63: continue
Line 64:
Line 65: val = kwargs[argName]
Line 66: printer = printers.get(argName, repr)
....................................................
File vdsm/storage/spm.py
Line 55: import fileUtils
Line 56: from processPool import Timeout
Line 57: import logUtils
Line 58:
Line 59: logged = partial(logUtils.logcall, "dispatcher", "Run and protect: %s",
logged = hsm.logged would avoid the sin of code dup.
Line 60: resPattern="Run and protect: %(name)s, Return response: %(result)s")
Line 61:
Line 62: rmanager = rm.ResourceManager.getInstance()
Line 63:
--
To view, visit http://gerrit.usersys.redhat.com/990
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I38b4a16803b03043144acf62e9fb65c3b1b0a3c5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: Add partial vms list support to the regular 'list' query.
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/clientIF.py
Line 692: d = vm.status()
Line 693: if full:
Line 694: return d
Line 695: else:
Line 696: return {'vmId': d['vmId'], 'status': d['status']}
not very important, but for large "vms", it would be better to make
vms = set(vms)
Line 697: return {'status': doneCode,
Line 698: 'vmList': [reportedStatus(vm, full) for vm in self.vmContainer.values()
Line 699: if not vms or vm.id in vms]}
Line 700:
....................................................
File vdsm_cli/vdsClient.py
Line 207: view = 'long' #Default view
Line 208: vms = []
Line 209:
Line 210: if args:
Line 211: if args[0].startswith('vms:'):
now it should be args[1], right?
Line 212: vms = _vmsParser(args[0])
Line 213: else:
Line 214: view = args[0]
Line 215: if len(args) > 1 and args[1].startswith('vms:'):
Line 222: #if not vms:
Line 223: for res in self.s.getAllVmStats()['statsList']:
Line 224: if not vms or res['vmId'] in vms:
Line 225: allStats[ res['vmId'] ] = res
Line 226: #else:
probably meant to clean the dead comment
Line 227: # for vm in vms:
Line 228: # res = self.s.getVmStats(vm)['statsList']
Line 229: # allStats[vm] = res[0]
Line 230:
--
To view, visit http://gerrit.usersys.redhat.com/956
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5051c42ca9e4ce706eff3cfec0de04cd76b2b95f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: Related to BZ#743272 - dirList IS and WAS dirList.
......................................................................
Patch Set 1: Looks good to me, but someone else must approve
(1 inline comment)
....................................................
File vdsm/storage/taskManager.py
Line 197: def loadDumpedTasks(self, store):
Line 198: if not os.path.exists(store):
Line 199: self.log.debug("task dump path %s does not exist.", store)
Line 200: return
Line 201: #taskID is the root part of each (root.ext) entry in the dump task dir
why did you remove the former code? it was so clear and succinct!
Line 202: tasksIDs = [os.path.splitext(tid)[0] for tid in os.listdir(store)]
Line 203: for taskID in tasksIDs:
Line 204: self.log.debug("Loading dumped task %s", taskID)
Line 205: try:
--
To view, visit http://gerrit.usersys.redhat.com/993
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fbcdb4076a5c16fd74754350464f382636ff419
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#732980 Initialize the block sizes metadata
......................................................................
Patch Set 1: Looks good to me, but someone else must approve
(1 inline comment)
....................................................
File vdsm/storage/blockSD.py
Line 242: self.metavol = lvm.lvPath(self.sdUUID, sd.METADATA)
Line 243:
Line 244: try:
Line 245: self.logBlkSize = self.getMetaParam(DMDK_LOGBLKSIZE)
Line 246: self.phyBlkSize = self.getMetaParam(DMDK_PHYBLKSIZE)
should we worry a case when one value is missing, but the both are read from vg?
I know it is far too late to ask - but why do we even need to store the values in our metadata if they are known to vg?
Line 247: except KeyError:
Line 248: # Initialize the block sizes metadata if not defined
Line 249: self.logBlkSize, self.phyBlkSize = lvm.getVGBlockSizes(sdUUID)
Line 250: self.setMetaParam(DMDK_LOGBLKSIZE, self.logBlkSize)
--
To view, visit http://gerrit.usersys.redhat.com/992
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0baa4addabc31389ea1656099f7c9d2d70cd85a0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#740887 Tune cache dirty ratio
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm/vdsmd.in
Line 366: if [ $? -ne 0 ]; then
Line 367: echo $"Configuring sysctl for vdsm..."
Line 368: sed -e '/vm.dirty_ratio/d' \
Line 369: -e '/vm.dirty_background_ratio/d' \
Line 370: -e '/# VDSM section begin/,/# VDSM section end/d' \
do we want to make this persistent? what is the benefit over setting it when vdsmd starts?
currently it remains even after vdsm is removed..
Line 371: /etc/sysctl.conf > /etc/sysctl.conf.vdsm
Line 372: cat >> /etc/sysctl.conf.vdsm <<EOF
Line 373: $VDSM_SYSCTL_HEAD
Line 374: vm.dirty_ratio = 5
--
To view, visit http://gerrit.usersys.redhat.com/970
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf5c8e4c0637c60092b89fba103b96b37bdafaa0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>