Change in vdsm[master]: net: sb validator: raise on missing sb device.

[Code Review]

From Dan Kenigsberg :

Dan Kenigsberg has posted comments on this change.

Change subject: net: sb validator: raise on missing sb device.
..


Patch Set 20: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/79953/20//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: net: sb validator: raise on missing sb device.
Line 8: 
Line 9: If no southbound device was given to a bridgless network,
Line 10: raise.
commit message is still wrong.
Line 11: 
Line 12: Change-Id: I6f3503faa5c84155ed667e2a474d1dc148798e35


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f3503faa5c84155ed667e2a474d1dc148798e35
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net: Centralize call to pre-config validation

[Code Review]

From Dan Kenigsberg :

Dan Kenigsberg has posted comments on this change.

Change subject: net: Centralize call to pre-config validation
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icd8fab6168816cd03545f268f8402f06e1fe84d6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: drop execCmd(raw)

[Code Review]

From Dan Kenigsberg :

Dan Kenigsberg has posted comments on this change.

Change subject: drop execCmd(raw)
..


Patch Set 2: Code-Review-1

nope, there's still plenty of code using the depending on raw=False.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ab6d9dca564da2dd61923888fad0be94afee8c9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: Openstacknet hooks must check provider type to run

[Code Review]

From Dan Kenigsberg :

Dan Kenigsberg has posted comments on this change.

Change subject: Openstacknet hooks must check provider type to run
..


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/80155/1//COMMIT_MSG
Commit Message:

PS1, Line 9: drive
driver


Line 9: When an openstacknet drive is installed along with another
Line 10: external network provider, its hooks were failing when an
Line 11: external network nic (non openstack) was handled.
Line 12: This patch modifies the hooks to check if the nic handled
Line 13: is of type OPENSTACK_NET_PROVIDER_TYPE before doing
which value is passed by the OVN provider? I thought that only the pluginType 
would differ.
Line 14: any other operation.
Line 15: 
Line 16: Change-Id: Ie88b6793fde74d9cd3b6a308c75c10d02c05a627


Line 11: external network nic (non openstack) was handled.
Line 12: This patch modifies the hooks to check if the nic handled
Line 13: is of type OPENSTACK_NET_PROVIDER_TYPE before doing
Line 14: any other operation.
Line 15: 
wasn't there an open bug about this?
Line 16: Change-Id: Ie88b6793fde74d9cd3b6a308c75c10d02c05a627


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie88b6793fde74d9cd3b6a308c75c10d02c05a627
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Marcin Mirecki 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dominik Holler 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tool.service: avoid execCmd(raw=False)

[Code Review]

From Dan Kenigsberg :

Dan Kenigsberg has uploaded a new change for review.

Change subject: tool.service: avoid execCmd(raw=False)
..

tool.service: avoid execCmd(raw=False)

tool.service is the only place where we use raw=False to split lines in
command output. However, we concatenate these lines immediately
afterwards, or ignore them.

This patch would enable the removal of the `raw` argument from execCmd
in a future patch.

Change-Id: I7530451496c41d1f3568e263f1087a92a8f0f3bb
Signed-off-by: Dan Kenigsberg 
---
M lib/vdsm/tool/service.py
1 file changed, 4 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/80213/1

diff --git a/lib/vdsm/tool/service.py b/lib/vdsm/tool/service.py
index b720b4a..f7fd9e7 100644
--- a/lib/vdsm/tool/service.py
+++ b/lib/vdsm/tool/service.py
@@ -30,11 +30,7 @@
 
 from vdsm.common.cmdutils import CommandPath
 from . import expose, UsageError, ExtraArgsError
-from ..commands import execCmd as _execCmd
-
-
-def execCmd(argv, raw=True, *args, **kwargs):
-return _execCmd(argv, raw=raw, *args, **kwargs)
+from ..commands import execCmd
 
 
 _SYSTEMCTL = CommandPath("systemctl",
@@ -108,10 +104,10 @@
 @functools.wraps(systemctlFun)
 def wrapper(srvName):
 cmd = [_SYSTEMCTL.cmd, "--no-pager", "list-unit-files"]
-rc, out, err = execCmd(cmd, raw=False)
+rc, out, err = execCmd(cmd)
 if rc != 0:
 raise ServiceOperationError(
-"Error listing unit files", '\n'.join(out), '\n'.join(err))
+"Error listing unit files", out, err)
 fullName = srvName
 # If unit file type was specified, don't override it.
 if srvName.count('.') < 1:
@@ -179,7 +175,7 @@
 @functools.wraps(initctlFun)
 def wrapper(srvName):
 cmd = [_INITCTL.cmd, "usage", srvName]
-rc, out, err = execCmd(cmd, raw=False)
+rc, out, err = execCmd(cmd)
 if rc != 0:
 raise ServiceNotExistError("%s is not an Upstart service" %
srvName)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7530451496c41d1f3568e263f1087a92a8f0f3bb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: checkimages hook: avoid using execCmd(sync=False)

[Code Review]

From Dan Kenigsberg :

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 
---
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 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: drop execCmd(raw)

[Code Review]

From Dan Kenigsberg :

Dan Kenigsberg has uploaded a new change for review.

Change subject: drop execCmd(raw)
..

drop execCmd(raw)

The `raw` argument was ill-conceived, as it modified the type of the
output of execCmd. Nowadays we use only raw=True, so we can safely drop
it.

Change-Id: I2ab6d9dca564da2dd61923888fad0be94afee8c9
Signed-off-by: Dan Kenigsberg 
---
M lib/vdsm/commands.py
M lib/vdsm/gluster/gfapi.py
M lib/vdsm/hooks.py
M lib/vdsm/host/__init__.py
M lib/vdsm/mkimage.py
M lib/vdsm/numa.py
M lib/vdsm/storage/clusterlock.py
M lib/vdsm/storage/fuser.py
M lib/vdsm/storage/mailbox.py
M lib/vdsm/storage/misc.py
M lib/vdsm/storage/multipath.py
M lib/vdsm/storage/qemuimg.py
M lib/vdsm/supervdsm_api/containers.py
M lib/vdsm/tool/configurators/certificates.py
M lib/vdsm/tool/configurators/lvm.py
M lib/vdsm/tool/configurators/sanlock.py
M lib/vdsm/udevadm.py
M lib/vdsm/v2v.py
M lib/vdsm/virt/containers/command.py
M tests/cmdutils_test.py
M tests/commands_test.py
M tests/containers/conttestlib.py
M tests/loopback.py
M tests/mkimage_test.py
M tests/storage/mount_test.py
M tests/storage/storagetestlib.py
M tests/v2v_test.py
M vdsm_hooks/hostusb/after_vm_destroy.py
M vdsm_hooks/hostusb/before_vm_start.py
M vdsm_hooks/localdisk/after_disk_prepare
M vdsm_hooks/localdisk/localdisk-helper
M vdsm_hooks/openstacknet/openstacknet_utils.py
M vdsm_hooks/promisc/after_vm_start.py
M vdsm_hooks/promisc/before_vm_destroy.py
M vdsm_hooks/scratchpad/before_vm_start.py
35 files changed, 53 insertions(+), 61 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/14/80214/1

diff --git a/lib/vdsm/commands.py b/lib/vdsm/commands.py
index 2041b30..438e839 100644
--- a/lib/vdsm/commands.py
+++ b/lib/vdsm/commands.py
@@ -42,7 +42,7 @@
 BUFFSIZE = 1024
 
 
-def execCmd(command, sudo=False, cwd=None, data=None, raw=False,
+def execCmd(command, sudo=False, cwd=None, data=None,
 printable=None, env=None, sync=True, nice=None, ioclass=None,
 ioclassdata=None, setsid=False, execCmdLogger=logging.root,
 deathSignal=None, resetCpuAffinity=True):
@@ -92,10 +92,6 @@
 out = ""
 
 execCmdLogger.debug(retcode_log_line(p.returncode, err=err))
-
-if not raw:
-out = out.splitlines(False)
-err = err.splitlines(False)
 
 return p.returncode, out, err
 
diff --git a/lib/vdsm/gluster/gfapi.py b/lib/vdsm/gluster/gfapi.py
index dc455be..00eaf19 100644
--- a/lib/vdsm/gluster/gfapi.py
+++ b/lib/vdsm/gluster/gfapi.py
@@ -227,7 +227,7 @@
 env['PYTHONPATH'] = ":".join(map(os.path.abspath,
  env['PYTHONPATH'].split(":")))
 
-rc, out, err = commands.execCmd(command, raw=True, env=env)
+rc, out, err = commands.execCmd(command, env=env)
 if rc != 0:
 raise ge.GlfsStatvfsException(rc, [out], [err])
 res = json.loads(out)
@@ -258,7 +258,7 @@
 env['PYTHONPATH'] = ":".join(map(os.path.abspath,
  env['PYTHONPATH'].split(":")))
 
-rc, out, err = commands.execCmd(command, raw=True, env=env)
+rc, out, err = commands.execCmd(command, env=env)
 if rc != 0:
 raise ge.GlusterVolumeEmptyCheckFailedException(rc, [out], [err])
 return out.upper() == "TRUE"
diff --git a/lib/vdsm/hooks.py b/lib/vdsm/hooks.py
index f7e9fdf..5d5e4d1 100644
--- a/lib/vdsm/hooks.py
+++ b/lib/vdsm/hooks.py
@@ -104,7 +104,7 @@
 
 errorSeen = False
 for s in scripts:
-rc, out, err = commands.execCmd([s], raw=True,
+rc, out, err = commands.execCmd([s],
 env=scriptenv)
 logging.info(err)
 if rc != 0:
diff --git a/lib/vdsm/host/__init__.py b/lib/vdsm/host/__init__.py
index 14e01b8..47f8c0d 100644
--- a/lib/vdsm/host/__init__.py
+++ b/lib/vdsm/host/__init__.py
@@ -44,7 +44,6 @@
 ret, out, err = execCmd([constants.EXT_DMIDECODE,
  "-s",
  "system-uuid"],
-raw=True,
 sudo=True)
 out = '\n'.join(line for line in out.splitlines()
 if not line.startswith('#'))
diff --git a/lib/vdsm/mkimage.py b/lib/vdsm/mkimage.py
index a76fca5..c630f24 100644
--- a/lib/vdsm/mkimage.py
+++ b/lib/vdsm/mkimage.py
@@ -123,7 +123,7 @@
 command = [EXT_MKFS_MSDOS, '-C', floppy, '1440']
 if volumeName is not None:
 command.extend(['-n', volumeName])
-rc, out, err = execCmd(command, raw=True)
+rc, out, err = execCmd(command)
 if rc:
 raise OSError(errno.EIO, "could not create floppy file: "
   "code %s, out %s\nerr %s" % (rc, out, err))
@@ -158,7 +158,7 @@
 fd = os.open(isopath, os.O_CREAT |