From Dan Kenigsberg <dan...@redhat.com>:

Dan Kenigsberg has uploaded a new change for review.

Change subject: checkimages hook: avoid using execCmd(sync=False)
......................................................................

checkimages hook: avoid using execCmd(sync=False)

I would like to move hooking.py to the common package. However, common
does not have the complex execCmd, and is not likely to ever have
something with sync=False. This patch reimplements exec-with-timeout
using threading.Timer instead of sync=False.

Change-Id: I2ec70891901a0473f5a863ca8ffd0c1df8046962
Signed-off-by: Dan Kenigsberg <dan...@redhat.com>
---
M vdsm_hooks/checkimages/before_vm_start.py
1 file changed, 12 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/80212/1

diff --git a/vdsm_hooks/checkimages/before_vm_start.py 
b/vdsm_hooks/checkimages/before_vm_start.py
index 31b123d..8dbcdb4 100755
--- a/vdsm_hooks/checkimages/before_vm_start.py
+++ b/vdsm_hooks/checkimages/before_vm_start.py
@@ -4,7 +4,10 @@
 import sys
 import traceback
 import fcntl
+import signal
+import subprocess
 import struct
+import threading
 import hooking
 
 BLKGETSIZE64 = 0x80081272  # Obtain device size in bytes
@@ -75,18 +78,20 @@
 
     # Check the image using qemu-img. Enforce check termination
     # on timeout expiration
-    p = hooking.execCmd(cmd, raw=True, sync=False)
+    p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
+    t = threading.Timer(timeout, p.kill)
+    t.start()
 
-    if not p.wait(timeout):
-        p.kill()
+    ((out, err), rc) = (p.communicate(), p.returncode)
+
+    t.cancel()
+
+    if rc == -signal.SIGKILL:
         sys.stderr.write('checkimages: %s image check operation timed out.' %
                          path)
         sys.stderr.write('Increate timeout or check image availability.')
         sys.exit(2)
-
-    ((out, err), rc) = (p.communicate(), p.returncode)
-
-    if rc == 0:
+    elif rc == 0:
         sys.stderr.write('checkimages: %s image check returned: %s\n' %
                          (path, out))
     else:


-- 
To view, visit https://gerrit.ovirt.org/80212
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2ec70891901a0473f5a863ca8ffd0c1df8046962
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <dan...@redhat.com>
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Reply via email to