Yotam Oron has posted comments on this change.
Change subject: vds_bootstrap: fix usage string
......................................................................
Patch Set 2: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.usersys.redhat.com/982
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8950764778dc61d04caeed4c74e792d52bba3378
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: vds_bootstrap: fix usage string
......................................................................
Patch Set 2: (2 inline comments)
Yotam, I've sent this patch just because you (duly) asked to fix the 'usage' string. I do not plan to rewrite vds_bootstrap - this is TEXTONLY.
....................................................
File vds_bootstrap/vds_bootstrap.py
Line 909: -u {true|false} -- use rhev-m-deployed yum repo
Line 910: -f -- override firewall rules by default ovirt rules.
Line 911: obsolete options:
Line 912: -n <netconsole_host:port>
Line 913: -r <rev_num>
well moving to optparse is something completely different.
Line 914: """
Line 915: try:
Line 916: rev_num = None
Line 917: orgName = 'Red Hat Inc.'
Line 926: orgName = v
Line 927: if o == "-t":
Line 928: systime = v
Line 929: if o == "-n":
Line 930: # TODO: remove me as soon as possible (BZ#689726)
we have to keep this, as long as RHEV-M sends it. this is why it is listed as obsolete.
Line 931: pass
Line 932: if o == "-u":
Line 933: usevdcrepo = (v.upper() == 'TRUE')
Line 934: elif o == '-f':
--
To view, visit http://gerrit.usersys.redhat.com/982
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8950764778dc61d04caeed4c74e792d52bba3378
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com>
Yotam Oron has posted comments on this change.
Change subject: vds_bootstrap: fix usage string
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File vds_bootstrap/vds_bootstrap.py
Line 909: -u {true|false} -- use rhev-m-deployed yum repo
Line 910: -f -- override firewall rules by default ovirt rules.
Line 911: obsolete options:
Line 912: -n <netconsole_host:port>
Line 913: -r <rev_num>
I guess a better way to do it would have been to use the Python command line parser thing we use.
Line 914: """
Line 915: try:
Line 916: rev_num = None
Line 917: orgName = 'Red Hat Inc.'
Line 926: orgName = v
Line 927: if o == "-t":
Line 928: systime = v
Line 929: if o == "-n":
Line 930: # TODO: remove me as soon as possible (BZ#689726)
BZ#689726 is verified, I guess this netconsole genius can be gone for good.
Line 931: pass
Line 932: if o == "-u":
Line 933: usevdcrepo = (v.upper() == 'TRUE')
Line 934: elif o == '-f':
--
To view, visit http://gerrit.usersys.redhat.com/982
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8950764778dc61d04caeed4c74e792d52bba3378
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Yotam Oron <yoron(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: Related to BZ#738120 - Logging in OOP.
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File vdsm/storage/processPool.py
Line 293: pass # Nothing we can do
Line 294:
Line 295: # Add logger handler (via Queue)
Line 296: hdlr = QueueHandler(logQueue)
Line 297: hdlr.setLevel(logging.DEBUG)
it would be nicer to inherit log level from main vdsm
Line 298: formatter = logging.Formatter("%(asctime)s %(filename)s, %(lineno)d, %(funcName)s: %(message)s")
Line 299: hdlr.setFormatter(formatter)
Line 300: logging.root.addHandler(hdlr)
Line 301:
Line 294:
Line 295: # Add logger handler (via Queue)
Line 296: hdlr = QueueHandler(logQueue)
Line 297: hdlr.setLevel(logging.DEBUG)
Line 298: formatter = logging.Formatter("%(asctime)s %(filename)s, %(lineno)d, %(funcName)s: %(message)s")
same goes to formatter - more polite to take the configurable one from main process. However we must log pid too, or the lines of unrelated oops would mix.
Line 299: hdlr.setFormatter(formatter)
Line 300: logging.root.addHandler(hdlr)
Line 301:
Line 302: poller = select.poll()
--
To view, visit http://gerrit.usersys.redhat.com/985
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iabe8abf31f46469e3c6fdac670f0ff03fe9cefe8
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Saggi Mizrahi has posted comments on this change.
Change subject: [WIP] Logging in OOP
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm/storage/processPool.py
Line 39: class Timeout(RuntimeError): pass
Line 40: class NoFreeHelpersError(RuntimeError): pass
Line 41: class PoolClosedError(RuntimeError): pass
Line 42:
Line 43: class QueueHandler(logging.Handler):
Move to logUtils. We might use it in supervdsm as well
Line 44: """
Line 45: This handler sends events to a queue. Typically, it would be used together
Line 46: with a multiprocessing Queue to centralise logging to file in one process
Line 47: (in a multi-process application), so as to avoid file write contention
--
To view, visit http://gerrit.usersys.redhat.com/985
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iabe8abf31f46469e3c6fdac670f0ff03fe9cefe8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Dan Kenigsberg has submitted this change and it was merged.
Change subject: BZ#742267 server_connection: checking access is enough
......................................................................
BZ#742267 server_connection: checking access is enough
There is no need to verify that a directory/mountpoint is owned by
vdsm:kvm. All we need is vdsm accessibility. Ownership requirement was
added in downstream commit 9fb05359 for local storage, and later
(http://gerrit.usersys.redhat.com/776) extended to all storage types.
Change-Id: I799374f13cc9d5fe193d76479f738e0a5fa02836
---
M vdsm/storage/fileUtils.py
M vdsm/storage/storage_connection.py
2 files changed, 0 insertions(+), 12 deletions(-)
Approvals:
Dan Kenigsberg: Looks good to me, approved
Igor Lvovsky: Looks good to me, but someone else must approve
David Naori: Verified
--
To view, visit http://gerrit.usersys.redhat.com/983
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I799374f13cc9d5fe193d76479f738e0a5fa02836
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: David Naori <dnaori(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#742267 server_connection: checking access is enough
......................................................................
Patch Set 1: Looks good to me, approved
--
To view, visit http://gerrit.usersys.redhat.com/983
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I799374f13cc9d5fe193d76479f738e0a5fa02836
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: David Naori <dnaori(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: [WIP] Logging in OOP
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File vdsm/storage/fileUtils.py
Line 94: raise se.MountTypeError()
Line 95:
Line 96: log.error(">>>>>>>1 cmd=%s", str(cmd))
Line 97: rc, out, err = misc.execCmd(cmd)
Line 98: log.error(">>>>>>>2 rc=%s out=%s err=%s", rc, out, err)
do we really want this in production code? I think execCmd logs this anyway.
Line 99: return rc
Line 100:
Line 101: def tarCopy(src, dst, exclude=[]):
Line 102: excludeArgs = ["--exclude=%s" % path for path in exclude]
--
To view, visit http://gerrit.usersys.redhat.com/985
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iabe8abf31f46469e3c6fdac670f0ff03fe9cefe8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: [WIP] Logging in OOP
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
drop [wip] since it's ready for use.
....................................................
File vdsm/storage/processPool.py
Line 33: from config import config
Line 34:
Line 35: MANAGE_PORT = config.getint("addresses", "management_port")
Line 36:
Line 37: _log = logging.getLogger("ProcessPool")
anything using this?
Line 38:
Line 39: class Timeout(RuntimeError): pass
Line 40: class NoFreeHelpersError(RuntimeError): pass
Line 41: class PoolClosedError(RuntimeError): pass
Line 148: # This is best utilized by starting all child processes at once.
Line 149: self._helperPool = [Helper(self._logQueue) for i in range(self._maxSubProcess)]
Line 150: self._lockPool = [Lock() for i in range(self._maxSubProcess)]
Line 151: self._closed = False
Line 152: # Add proccess for looging from oop
proccess --> thread, loogin --> logging
Line 153: self.logProc = threading.Thread(target=_helperLoggerLoop, args=(self._logQueue,))
Line 154: self.logProc.daemon = True
Line 155: self.logProc.start()
Line 156:
Line 285: if hasattr(log, 'handlers'): del log.handlers[:]
Line 286:
Line 287: # Close all file-descriptors we don't need
Line 288: # Logging won't work past this point
Line 289: fdsWhiteList = [0, 1, 2, lifeLine, parentLifelineFD, pipe.fileno(),
minor: ALLCAPS and (tupple) would have been nicer.
Line 290: logQueue._reader.fileno(), logQueue._writer.fileno()]
Line 291: for fd in misc.getfds():
Line 292: if fd not in fdsWhiteList:
Line 293: try:
--
To view, visit http://gerrit.usersys.redhat.com/985
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iabe8abf31f46469e3c6fdac670f0ff03fe9cefe8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>