Change in vdsm[ovirt-3.5.0]: vdsm-reg: Fix closed log file descriptors

Hello Dima Kuznetsov, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/35121

to review the following change.

Change subject: vdsm-reg: Fix closed log file descriptors
......................................................................

vdsm-reg: Fix closed log file descriptors

When calling createDaemon(), all file descriptors are closed, including
the file descriptors held by log handlers. This patch moves logging
configuration below the daemonization to make sure the file descriptors
stay open.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1150238
Change-Id: Ida11e61aea18fbb2990176889423d2c4b6292a09
Signed-off-by: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/34981
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M vdsm_reg/vdsm-reg-setup.in
1 file changed, 9 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/35121/1

diff --git a/vdsm_reg/vdsm-reg-setup.in b/vdsm_reg/vdsm-reg-setup.in
index 804183f..7fc8943 100644
--- a/vdsm_reg/vdsm-reg-setup.in
+++ b/vdsm_reg/vdsm-reg-setup.in
@@ -352,19 +352,21 @@
     pidfile = config.get('vars', 'pidfile')
     sleepTime = float(config.get('vars', 'reg_req_interval'))
 
-    loggerConf = config.get('vars', 'logger_conf')
-    logging.config.fileConfig(loggerConf, disable_existing_loggers=False)
-    log = logging.getLogger('')
-
-    _createUpgradeDir(os.path.dirname(config.get('vars',
'upgrade_iso_file')))
-
     try:
         if daemonize:
             createDaemon.createDaemon()
+
+        loggerConf = config.get('vars', 'logger_conf')
+        logging.config.fileConfig(loggerConf, disable_existing_loggers=False)
+        log = logging.getLogger('')
+
         if daemonize:
             log = logging.getLogger('vdsRegistrator')
         if not daemonize:
             log.handlers.append(logging.StreamHandler())
+
+        _createUpgradeDir(
+            os.path.dirname(config.get('vars', 'upgrade_iso_file')))
 
         log.info("After daemonize - My pid is %d", os.getpid())
         file(pidfile, 'w').write(str(os.getpid()) + "\n")
@@ -396,10 +398,10 @@
                 itr,
                 sleepTime+nRandom
             )
+        log.info("Exiting ....")
     finally:
         if pidfile and os.path.exists(pidfile):
             os.unlink(pidfile)
-        log.info("Exiting ....")
 
 
 def usage():


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ida11e61aea18fbb2990176889423d2c4b6292a09
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5.0
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;

Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-reg: Fix closed log file descriptors
......................................................................


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida11e61aea18fbb2990176889423d2c4b6292a09
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5.0
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-reg: Fix closed log file descriptors
......................................................................


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida11e61aea18fbb2990176889423d2c4b6292a09
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5.0
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Fabian Deutsch <fabiand(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Fabian Deutsch has posted comments on this change.

Change subject: vdsm-reg: Fix closed log file descriptors
......................................................................


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida11e61aea18fbb2990176889423d2c4b6292a09
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5.0
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Fabian Deutsch <fabiand(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Yaniv Bronhaim has submitted this change and it was merged.

Change subject: vdsm-reg: Fix closed log file descriptors
......................................................................


vdsm-reg: Fix closed log file descriptors

When calling createDaemon(), all file descriptors are closed, including
the file descriptors held by log handlers. This patch moves logging
configuration below the daemonization to make sure the file descriptors
stay open.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1150238
Change-Id: Ida11e61aea18fbb2990176889423d2c4b6292a09
Signed-off-by: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/34981
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/35121
Reviewed-by: Fabian Deutsch <fabiand(a)redhat.com&gt;
---
M vdsm_reg/vdsm-reg-setup.in
1 file changed, 9 insertions(+), 7 deletions(-)

Approvals:
  Fabian Deutsch: Looks good to me, but someone else must approve
  Yaniv Bronhaim: Verified; Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ida11e61aea18fbb2990176889423d2c4b6292a09
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5.0
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com&gt;
Gerrit-Reviewer: Fabian Deutsch <fabiand(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org