vdsm-patches November 2015

vdsm-patches@lists.fedorahosted.org

47 participants
406 discussions

Change in vdsm[master]: WIP
by fromani＠redhat.com 04 Nov '15

04 Nov '15

Francesco Romani has uploaded a new change for review. Change subject: WIP ...................................................................... WIP Change-Id: I0beb75cc1d4a09b07402d16e4b8057e391daad8c Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M lib/vdsm/taskset.py M lib/vdsm/udevadm.py M lib/vdsm/utils.py M lib/vdsm/virtsparsify.py 4 files changed, 11 insertions(+), 15 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/47967/1 diff --git a/lib/vdsm/taskset.py b/lib/vdsm/taskset.py index 2ec6a16..c33c6cf 100644 --- a/lib/vdsm/taskset.py +++ b/lib/vdsm/taskset.py @@ -37,10 +37,7 @@ """ command = [constants.EXT_TASKSET, '--pid', str(pid)] - rc, out, err = utils.execCmd(command, resetCpuAffinity=False) - - if rc != 0: - raise Error(rc, out, err) + utils.exec_cmd(command, resetCpuAffinity=False) return _cpu_set_from_output(out[-1]) @@ -65,10 +62,7 @@ str(pid) )) - rc, out, err = utils.execCmd(command, resetCpuAffinity=False) - - if rc != 0: - raise Error(rc, out, err) + utils.exec_cmd(command, resetCpuAffinity=False) def _cpu_set_from_output(line): diff --git a/lib/vdsm/udevadm.py b/lib/vdsm/udevadm.py index e4e3a9c..dd13120 100644 --- a/lib/vdsm/udevadm.py +++ b/lib/vdsm/udevadm.py @@ -100,6 +100,4 @@ def _run_command(args): cmd = [_UDEVADM.cmd] cmd.extend(args) - rc, out, err = utils.execCmd(cmd, raw=True) - if rc != 0: - raise Error(rc, out, err) + utils.exec_cmd(cmd, raw=True) diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index d16f059..e447529 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -687,6 +687,13 @@ self.rc, self.out, self.err) +def exec_cmd(cmd, *a, **kw): + rc, out, err = execCmd(cmd, *a, **kw) + if rc != 0: + raise Error(rc, out, err) + return rc, out, err + + def stripNewLines(lines): return [l[:-1] if l.endswith('\n') else l for l in lines] diff --git a/lib/vdsm/virtsparsify.py b/lib/vdsm/virtsparsify.py index 187e3bd..35099dc 100644 --- a/lib/vdsm/virtsparsify.py +++ b/lib/vdsm/virtsparsify.py @@ -50,7 +50,4 @@ cmd.extend((src_vol, dst_vol)) - rc, out, err = utils.execCmd(cmd, deathSignal=signal.SIGKILL) - - if rc != 0: - raise Error(rc, out, err) + utils.exec_cmd(cmd, deathSignal=signal.SIGKILL) -- To view, visit https://gerrit.ovirt.org/47967 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0beb75cc1d4a09b07402d16e4b8057e391daad8c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org

2 3

Change in vdsm[master]: virt: assign system-wide name to threads
by Nir Soffer 04 Nov '15

04 Nov '15

Nir Soffer has posted comments on this change. Change subject: virt: assign system-wide name to threads ...................................................................... Patch Set 10: Note that we have this topic: https://gerrit.ovirt.org/#/q/status:open+project:vdsm+branch:master+topic:t… Once all code uses concurrent.thread, it will be easy to set thread names automatically for all users. -- To view, visit https://gerrit.ovirt.org/41058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa6f2afec140c9ae54601151cd51d87cba3ee232 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org Gerrit-HasComments: No

1 0

Change in vdsm[master]: virt: assign system-wide name to threads
by automation＠ovirt.org 04 Nov '15

04 Nov '15

automation(a)ovirt.org has posted comments on this change. Change subject: virt: assign system-wide name to threads ...................................................................... Patch Set 10: * Update tracker::IGNORE, no Bug-Url found -- To view, visit https://gerrit.ovirt.org/41058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa6f2afec140c9ae54601151cd51d87cba3ee232 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org Gerrit-HasComments: No

1 0

Change in vdsm[master]: virt: assign system-wide name to threads
by fromani＠redhat.com 04 Nov '15

04 Nov '15

Francesco Romani has abandoned this change. Change subject: virt: assign system-wide name to threads ...................................................................... Abandoned we will have a fresh (re)start later. -- To view, visit https://gerrit.ovirt.org/41058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ifa6f2afec140c9ae54601151cd51d87cba3ee232 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org

1 0

Change in vdsm[master]: lib: set system name for threads
by automation＠ovirt.org 04 Nov '15

04 Nov '15

automation(a)ovirt.org has posted comments on this change. Change subject: lib: set system name for threads ...................................................................... Patch Set 10: * Update tracker::IGNORE, no Bug-Url found -- To view, visit https://gerrit.ovirt.org/41057 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e8ffe2c598221d93ca154e4ee4f345568c5b35f Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org Gerrit-HasComments: No

1 0

Change in vdsm[master]: lib: set system name for threads
by fromani＠redhat.com 04 Nov '15

04 Nov '15

Francesco Romani has abandoned this change. Change subject: lib: set system name for threads ...................................................................... Abandoned we will have a fresh (re)start later. -- To view, visit https://gerrit.ovirt.org/41057 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I9e8ffe2c598221d93ca154e4ee4f345568c5b35f Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org

1 0

Change in vdsm[master]: config: Resolve crash when IPv6 disabled in kernel
by ibarkan＠redhat.com 04 Nov '15

04 Nov '15

Ido Barkan has posted comments on this change. Change subject: config: Resolve crash when IPv6 disabled in kernel ...................................................................... Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/47978/1/vdsm/network/configurators/ifcfg.py File vdsm/network/configurators/ifcfg.py: Line 102: self._addSourceRoute(bridge) Line 103: _ifup(bridge) Line 104: Line 105: # Check if ipv6 is disabled by kernel arguments Line 106: has_ipv6 = True > hmm, I think that we'd better expose this as a memoized function in caps.py agreed. Line 107: try: Line 108: socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) Line 109: except socket.error: Line 110: has_ipv6 = False -- To view, visit https://gerrit.ovirt.org/47978 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I774cac868ddee08ac072adf2104a811594779052 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ollie Armstrong <ollie(a)fubra.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ollie Armstrong <ollie(a)fubra.com> Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org Gerrit-HasComments: Yes

1 0

Change in vdsm[master]: lib: grub2 module
by fromani＠redhat.com 03 Nov '15

03 Nov '15

Francesco Romani has posted comments on this change. Change subject: lib: grub2 module ...................................................................... Patch Set 3: (9 comments) few comments, -1 for visibility Please add one-liners (or more, of course, if you need) defining "workaround" and "stub". https://gerrit.ovirt.org/#/c/47946/3/lib/vdsm/grub2.py File lib/vdsm/grub2.py: Line 45: return "Process failed with rc=%d out=%r err=%r" % ( Line 46: self.rc, self.out, self.err) Line 47: Line 48: Line 49: def mkconfig(args): please add docstrings to functions. No need to document every parameter or to be verbose, just outline what functions should do. Do we have a wiki page or anything describing the flow? I mean the intended flow, not the implemented flow. Line 50: cmd = [_GRUB2_MKCONFIG.cmd] Line 51: cmd.extend(args) Line 52: rc, out, err = utils.execCmd(_GRUB2_MKCONFIG, raw=True) Line 53: if rc != 0: Line 160: Line 161: device_vendor = device_vendor.lstrip('0x') Line 162: device_product = device_product.lstrip('0x') Line 163: Line 164: if stub_ids: does the ordering matter? can we use a set? Line 165: for vendor, product in stub_ids: Line 166: if device_vendor == vendor and device_product == product: Line 167: return Line 168: Line 163: Line 164: if stub_ids: Line 165: for vendor, product in stub_ids: Line 166: if device_vendor == vendor and device_product == product: Line 167: return this check you implemented is duplicating the default behaviour of python. I think you can just check like this: if [vendor, product] in stub_ids: return Line 168: Line 169: stub_ids.append([device_vendor, device_product]) Line 170: Line 171: Line 165: for vendor, product in stub_ids: Line 166: if device_vendor == vendor and device_product == product: Line 167: return Line 168: Line 169: stub_ids.append([device_vendor, device_product]) why list and not tuple? Line 170: Line 171: Line 172: def _del_from_stub(stub_ids, device_vendor, device_product): Line 173: Line 173: Line 174: device_vendor = device_vendor.lstrip('0x') Line 175: device_product = device_product.lstrip('0x') Line 176: Line 177: if stub_ids: does it work without the if? To iterate in an empty collection should exit immediately. Line 178: for idx, (vendor, product) in enumerate(stub_ids): Line 179: if device_vendor == vendor and device_product == product: Line 180: del(stub_ids[idx]) Line 181: break Line 193: Line 194: Line 195: def _add_workaround(workarounds, workaround): Line 196: try: Line 197: workarounds.index(workaround) same: do we care about the ordering? If so, please document it why. Otherwise, let's just use a set Line 198: except ValueError: Line 199: workarounds.append(workaround) Line 200: Line 201: https://gerrit.ovirt.org/#/c/47946/3/tests/grub2Tests.py File tests/grub2Tests.py: Line 55: Line 56: self._assertIntegrity(data) Line 57: self.assertIn('"pci-stub.ids=cafe:beef"', data) Line 58: Line 59: def testAddDuplicateStub2(self): please use a more describing name, and higlight the differences with respect to the other test Line 60: with tempfile.NamedTemporaryFile() as f: Line 61: grub2.add_to_stub( Line 62: ['cafe', '0xcafe'], ['beef', '0xbeef'], Line 63: cfg_source=self._test_file('grub2_default.out'), Line 71: def testAddMultipleStubs(self): Line 72: with tempfile.NamedTemporaryFile() as f: Line 73: grub2.add_to_stub( Line 74: ['cafe', '0xdead', '0xf00d', 'b002'], Line 75: ['0xbeef', 'feed', '0xc00l', 'fa11'], I missed this reviewing the code! so question: why is it helpful to support lists and not just one argument? I mean, what's the use case? Furthermore, maybe having a single list of pairs (tuples) is a bit more easier to read. Did you try that approach? Line 76: cfg_source=self._test_file('grub2_default.out'), Line 77: cfg_dest=f.name) Line 78: Line 79: data = f.read() Line 169: Line 170: self._assertIntegrity(data) Line 171: self.assertIn('VDSM_WORKAROUNDS="a=b"', data) Line 172: Line 173: def testAddDuplicateWorkaround2(self): same as for stub Line 174: with tempfile.NamedTemporaryFile() as f: Line 175: grub2.add_workaround( Line 176: ['a=b', 'a=b'], Line 177: cfg_source=self._test_file('grub2_default.out'), -- To view, visit https://gerrit.ovirt.org/47946 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e7e0b327882f64196a71fe20dcbd99017b80800 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com> Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com> Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com> Gerrit-Reviewer: Michal Skrivanek <mskrivan(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org Gerrit-HasComments: Yes

1 0

Change in vdsm[master]: lib: grub2 module
by fromani＠redhat.com 03 Nov '15

03 Nov '15

Francesco Romani has posted comments on this change. Change subject: lib: grub2 module ...................................................................... Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/47946/1/lib/vdsm/grub2.py File lib/vdsm/grub2.py: Line 143: if not val: Line 144: continue Line 145: workarounds.append(val) Line 146: Line 147: return workarounds > Not really, as split would introduce empty element. uhm I probably miss something but: workarounds = [ val for val in (grub_cfg[_CFG_NAME_WORKAROUNDS].strip('"').split(' ')) if val ] ? btw, it is very minor Line 148: Line 149: Line 150: def _add_workaround(workarounds, workaround): Line 151: try: -- To view, visit https://gerrit.ovirt.org/47946 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e7e0b327882f64196a71fe20dcbd99017b80800 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com> Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com> Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com> Gerrit-Reviewer: Michal Skrivanek <mskrivan(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org Gerrit-HasComments: Yes

1 0

Change in vdsm[master]: vdsm: introduce cpuinfo module
by fromani＠redhat.com 03 Nov '15

03 Nov '15

Francesco Romani has posted comments on this change. Change subject: vdsm: introduce cpuinfo module ...................................................................... Patch Set 10: nice, we are asking changes in the opposite directions :) to unlock the checkmate, let's step back and do what Dan asks, which is pretty much what you had before my comment. We'll see in a later patch if my idea works better or not -- To view, visit https://gerrit.ovirt.org/46912 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaa702b05f3825ebdcfed16d86d39a8c38fcf224c Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com> Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com> Gerrit-Reviewer: Michal Skrivanek <mskrivan(a)redhat.com> Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com> Gerrit-Reviewer: automation(a)ovirt.org Gerrit-HasComments: No

1 0

← Newer
1
...
33
34
35
36
37
38
39
40
41
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches November 2015