Change in vdsm[master]: osinfo: drop package buildtime
Michal Skrivanek has posted comments on this change. Change subject: osinfo: drop package buildtime .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65059 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: copy_data: Add qcow2_compat on convert.
gerrit-hooks has posted comments on this change. Change subject: copy_data: Add qcow2_compat on convert. .. Patch Set 24: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64373 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie26e5dcba6fc493b32ea7764889df2918c4dfdd3 Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer 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]: volume_artifacts: Add qcow2_compat on create.
gerrit-hooks has posted comments on this change. Change subject: volume_artifacts: Add qcow2_compat on create. .. Patch Set 24: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64372 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2dd2d53fba0dd69cdb4f60e152cf6d254cfb863a Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer 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]: osinfo: drop package buildtime
gerrit-hooks has posted comments on this change. Change subject: osinfo: drop package buildtime .. Patch Set 4: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65059 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: static: move vdsm bonding defaults under static
Martin Polednik has posted comments on this change. Change subject: static: move vdsm bonding defaults under static .. Patch Set 6: (1 comment) No path was hardcoded during the creation of this patch. :) https://gerrit.ovirt.org/#/c/62537/6/static/Makefile.am File static/Makefile.am: PS6, Line 24: /usr/share > Don't we have a predefined variable for this? It's referring to a source path, not the target. Target is vdsmdir. (see configure.ac) AC_SUBST([vdsmdir], ['${datarootdir}/vdsm']) -- To view, visit https://gerrit.ovirt.org/62537 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9cbd81e59aec49876e15337d7cd617e51316a604 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Ondřej Svoboda 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]: osinfo: un-nest kernel version gathering function
gerrit-hooks has posted comments on this change. Change subject: osinfo: un-nest kernel version gathering function .. Patch Set 2: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65379 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77f78882973855cc971bbdae66cc96c8068e8230 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal 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]: static: move man under static
Martin Polednik has posted comments on this change. Change subject: static: move man under static .. Patch Set 5: (3 comments) https://gerrit.ovirt.org/#/c/62549/5/static/Makefile.am File static/Makefile.am: PS5, Line 24: usr > same comment as parent patch about hardcoded paths. Same as parent patch, this is path within source tree, not the distribution target. PS5, Line 33: etc > hardcoding /etc is kinda fine, a lot of thing can broke otherwise, but stuf Ditto. PS5, Line 93: ./usr/lib/systemd > (yes, I noticed too late) Ditto. -- To view, visit https://gerrit.ovirt.org/62549 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I91fa36b38904d85fc60734ce966b8d993911b490 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: use assertXMLEqual in vmTests
Francesco Romani has uploaded a new change for review. Change subject: tests: use assertXMLEqual in vmTests .. tests: use assertXMLEqual in vmTests switch to assertXMLEqual to have better error messages. Change-Id: I6516404548eb000f9501818818c929df09c406e2 Signed-off-by: Francesco Romani --- M tests/vmTests.py 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/65417/1 diff --git a/tests/vmTests.py b/tests/vmTests.py index 6b7bb61..905a36e 100644 --- a/tests/vmTests.py +++ b/tests/vmTests.py @@ -633,7 +633,7 @@ """) -self.assertEqual(expected_xml, self._xml_sanitizer(dom._metadata)) +self.assertXMLEqual(expected_xml, self._xml_sanitizer(dom._metadata)) def testCpuTune(self): LIMIT = 50 @@ -829,7 +829,7 @@ """) -self.assertEqual(expected_xml, self._xml_sanitizer(dom._metadata)) +self.assertXMLEqual(expected_xml, self._xml_sanitizer(dom._metadata)) def testGetIoTunePolicy(self): with fake.VM() as machine: @@ -932,8 +932,8 @@ # Test that caches were properly updated self.assertEqual(drives[0].specParams["ioTune"], expected_io_tune[drives[0].name]) -self.assertEqual(self._xml_sanitizer(drives[0]._deviceXML), - self._xml_sanitizer(expected_xml)) +self.assertXMLEqual(self._xml_sanitizer(drives[0]._deviceXML), +self._xml_sanitizer(expected_xml)) def testSdIds(self): """ -- To view, visit https://gerrit.ovirt.org/65417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6516404548eb000f9501818818c929df09c406e2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani ___ 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]: tests: use assertXMLEqual in vmTests
gerrit-hooks has posted comments on this change. Change subject: tests: use assertXMLEqual in vmTests .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6516404548eb000f9501818818c929df09c406e2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani 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]: utils: Add missing Popen methods
Piotr Kliczewski has posted comments on this change. Change subject: utils: Add missing Popen methods .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65321 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d562663698d86b98b55d139e2691a0f51566ccf Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Wait for child process in tearDown
Piotr Kliczewski has posted comments on this change. Change subject: tests: Wait for child process in tearDown .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65322 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I248fa0b1c2129f69164be447402276d7908ed768 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Use Popen.poll() for running state
Piotr Kliczewski has posted comments on this change. Change subject: tests: Use Popen.poll() for running state .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/65323/4//COMMIT_MSG Commit Message: PS4, Line 9: exists works `exits` should be enough or we could use `exists/works` -- To view, visit https://gerrit.ovirt.org/65323 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I086e9317b5f8e5324d86bedeb04b485c7a09ad16 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: utils: Wait for terminated process
Piotr Kliczewski has posted comments on this change. Change subject: utils: Wait for terminated process .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65324 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida04e2c7ba092efdc2927ed9f460b0098ba2ad94 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: volume_artifacts: Add qcow2_compat on create.
Maor Lipchuk has posted comments on this change. Change subject: volume_artifacts: Add qcow2_compat on create. .. Patch Set 24: Verified+1 -- To view, visit https://gerrit.ovirt.org/64372 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2dd2d53fba0dd69cdb4f60e152cf6d254cfb863a Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer 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]: copy_data: Add qcow2_compat on convert.
Maor Lipchuk has posted comments on this change. Change subject: copy_data: Add qcow2_compat on convert. .. Patch Set 24: Verified+1 -- To view, visit https://gerrit.ovirt.org/64373 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie26e5dcba6fc493b32ea7764889df2918c4dfdd3 Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer 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]: tests: Refine TerminationTests names
Piotr Kliczewski has posted comments on this change. Change subject: tests: Refine TerminationTests names .. Patch Set 5: (3 comments) https://gerrit.ovirt.org/#/c/65326/5//COMMIT_MSG Commit Message: PS5, Line 9: Al For all the tests that test termination there is no need to repeat... PS5, Line 11: tests name test names PS5, Line 13: for other : failure tests what do you mean? how do you want to do it? (my questions based on the code in this patch) -- To view, visit https://gerrit.ovirt.org/65326 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: utils: Raise detectable error if termination fail
Piotr Kliczewski has posted comments on this change. Change subject: utils: Raise detectable error if termination fail .. Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG Commit Message: PS5, Line 18: handle the error handling the error without knowing the context (pid) is not useful. That is way we use new exception to pass a context (pid). This commit message is confusing. -- To view, visit https://gerrit.ovirt.org/65325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add tests for waiting on a zombie process
Piotr Kliczewski has posted comments on this change. Change subject: tests: Add tests for waiting on a zombie process .. Patch Set 6: Code-Review-1 (1 comment) https://gerrit.ovirt.org/#/c/65327/6/tests/utilsTests.py File tests/utilsTests.py: PS6, Line 105: def fail(): : raise RuntimeError("Attempt to kill a zombie process") in previous patch I though we wanted to reuse similar code but now I see that we copied and pasted it here. -- To view, visit https://gerrit.ovirt.org/65327 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add test for terminating a terminated process
Piotr Kliczewski has posted comments on this change. Change subject: tests: Add test for terminating a terminated process .. Patch Set 6: Code-Review-1 (1 comment) https://gerrit.ovirt.org/#/c/65328/6/tests/utilsTests.py File tests/utilsTests.py: PS6, Line 117: def fail(): : raise RuntimeError("Attempt to kill a terminated process") I see that copy and paste are our friends :) -- To view, visit https://gerrit.ovirt.org/65328 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f06c28857664cc49beb938f33ac3c9d07ca3b6 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: osinfo: drop package buildtime
Martin Polednik has posted comments on this change. Change subject: osinfo: drop package buildtime .. Patch Set 4: Verified+1 -- To view, visit https://gerrit.ovirt.org/65059 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add tests for poll and wait failures
Piotr Kliczewski has posted comments on this change. Change subject: tests: Add tests for poll and wait failures .. Patch Set 8: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/65294/8/tests/utilsTests.py File tests/utilsTests.py: PS8, Line 135: def fail(): : raise ExpectedFailure("Fake kill failure") Now we have it 3 times. PS8, Line 158: timing Wouldn't be better to wait to be sure that timing is correct instead of removing it? -- To view, visit https://gerrit.ovirt.org/65294 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: osinfo: un-nest kernel version gathering function
Martin Polednik has posted comments on this change. Change subject: osinfo: un-nest kernel version gathering function .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/65379/1/lib/vdsm/osinfo.py File lib/vdsm/osinfo.py: Line 245: Line 246: def _runtime_kernel_version(): Line 247: ret = os.uname() Line 248: try: Line 249: ver, rel = ret[2].split('-', 1) > I wonder if we can get this for free from the platform module Define free. We can get the exact same information with platform.uname()[2].split('-', 1) Unfortunately, as uname's implementation does roughly ... if release == 'unknown': release = '' ... if it cannot find the kernel release, it wouldn't really help us much. Line 250: except ValueError: Line 251: logging.error('kernel release not found', exc_info=True) Line 252: ver, rel = '0', '0' Line 253: -- To view, visit https://gerrit.ovirt.org/65379 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77f78882973855cc971bbdae66cc96c8068e8230 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal 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]: tests: Add tests for waiting on a zombie process
Nir Soffer has posted comments on this change. Change subject: tests: Add tests for waiting on a zombie process .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/65327/6/tests/utilsTests.py File tests/utilsTests.py: Line 102: self.proc.terminate() Line 103: wait_for_zombie(self.proc, 1) Line 104: Line 105: def fail(): Line 106: raise RuntimeError("Attempt to kill a zombie process") > in previous patch I though we wanted to reuse similar code but now I see th This raises rumtime error and will fail the test if raised. The other fail helper raise expected error and the test will fail if not raised. Please see the next patches for the complete picture. Line 107: Line 108: self.proc.kill = fail Line 109: with utils.terminating(self.proc): Line 110: pass -- To view, visit https://gerrit.ovirt.org/65327 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add test for terminating a terminated process
Nir Soffer has posted comments on this change. Change subject: tests: Add test for terminating a terminated process .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/65328/6/tests/utilsTests.py File tests/utilsTests.py: Line 114: self.proc.terminate() Line 115: self.proc.wait() Line 116: Line 117: def fail(): Line 118: raise RuntimeError("Attempt to kill a terminated process") > I see that copy and paste are our friends :) Did you notice that each exception has different text? Please suggest how to improve this while keeping the code clear. Line 119: Line 120: self.proc.kill = fail Line 121: with utils.terminating(self.proc): Line 122: pass -- To view, visit https://gerrit.ovirt.org/65328 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f06c28857664cc49beb938f33ac3c9d07ca3b6 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add tests for poll and wait failures
Nir Soffer has posted comments on this change. Change subject: tests: Add tests for poll and wait failures .. Patch Set 8: (2 comments) https://gerrit.ovirt.org/#/c/65294/8/tests/utilsTests.py File tests/utilsTests.py: Line 132: self.check_failure() Line 133: Line 134: def test_kill_failure(self): Line 135: def fail(): Line 136: raise ExpectedFailure("Fake kill failure") > Now we have it 3 times. 3 test, 3 fail texts, please suggest a simple and more clear code. Line 137: Line 138: self.proc.kill = fail Line 139: self.check_failure() Line 140: Line 154: self.assertEqual(type(e.exception.error), ExpectedFailure) Line 155: Line 156: # Note: We cannot check return code since AsyncProc.returncode is a Line 157: # property calling poll(). The return code here may be None or -9, Line 158: # depeending on timing. > Wouldn't be better to wait to be sure that timing is correct instead of rem There is no way to test this with AsyncProc, check how it imlements returncode. Line 159: Line 160: Line 161: class ExpectedFailure(Exception): Line 162: pass -- To view, visit https://gerrit.ovirt.org/65294 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: osinfo: drop package buildtime
Piotr Kliczewski has posted comments on this change. Change subject: osinfo: drop package buildtime .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65059 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: py3: make gluster_cli_tests pass
Piotr Kliczewski has posted comments on this change. Change subject: py3: make gluster_cli_tests pass .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65008 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Ramesh N Gerrit-Reviewer: Sahina Bose 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]: qemuimg: Expose API for qemuimg map
Ala Hino has posted comments on this change. Change subject: qemuimg: Expose API for qemuimg map .. Patch Set 4: (4 comments) https://gerrit.ovirt.org/#/c/65112/4/tests/qemuimg_test.py File tests/qemuimg_test.py: Line 497: # Empty run Line 498: run = img_map[2] Line 499: self.assertEqual(run["length"], size - offset - length) Line 500: self.assertEqual(run["data"], False) Line 501: self.assertEqual(run["zero"], True) > Please use the same style as in the qcow2 test for verifying. Done Line 502: Line 503: @permutations([ Line 504: # offset, length, expected_length, expected_start, qcow2_compat Line 505: (64 * 1024, 4 * 1024, 65536, 131072, "0.10"), Line 530: }, Line 531: # run 2 - data Line 532: { Line 533: "start": offset, Line 534: "offset": 327680, > Do you have a clue about this value? No idea :( Line 535: "length": expected_length, Line 536: "data": True, Line 537: "zero": False, Line 538: }, Line 537: "zero": False, Line 538: }, Line 539: # run 3 - empty Line 540: { Line 541: "start": expected_start, > Isn't this offset + expected_length? Fixed Line 542: "length": size - offset - expected_length, Line 543: "data": False, Line 544: "zero": True, Line 545: }, Line 540: { Line 541: "start": expected_start, Line 542: "length": size - offset - expected_length, Line 543: "data": False, Line 544: "zero": True, > Do we have an offsect in this run? what is the value? No offset here Line 545: }, Line 546: ] Line 547: Line 548: for actual, expected in zip(img_map, expected): -- To view, visit https://gerrit.ovirt.org/65112 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer 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]: qemuimg: Expose API for qemuimg map
gerrit-hooks has posted comments on this change. Change subject: qemuimg: Expose API for qemuimg map .. Patch Set 5: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65112 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer 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]: tests: Use Popen.poll() for running state
Nir Soffer has posted comments on this change. Change subject: tests: Use Popen.poll() for running state .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/65323/4//COMMIT_MSG Commit Message: Line 5: CommitDate: 2016-10-13 03:07:01 +0300 Line 6: Line 7: tests: Use Popen.poll() for running state Line 8: Line 9: Checking if /proc/pid exists works, but using poll() is simpler and more > `exits` should be enough or we could use `exists/works` Currently we check if /proc/pid exists, this checks works but we can use Popen.poll() instead. I son't see how I can remove the word "works" or how "exists/works" is corect. Line 10: clear. Line 11: Line 12: Since proc_path is used only in one test, move it the test using it. Line 13: -- To view, visit https://gerrit.ovirt.org/65323 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I086e9317b5f8e5324d86bedeb04b485c7a09ad16 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: utils: Raise detectable error if termination fail
Nir Soffer has posted comments on this change. Change subject: utils: Raise detectable error if termination fail .. Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG Commit Message: Line 14: job running the process must stay in running state to prevent engine Line 15: from starting the same operation on another machine. Line 16: Line 17: Typically when we cannot handle an error we should not handle it. But Line 18: in this special case, letting the caller handle the error is not useful > handling the error without knowing the context (pid) is not useful. That is Trying to make it more clear in the next version. Line 19: since it does not have any context. Line 20: Line 21: To make it possible to handle this critical error, we raise now a new Line 22: TerminatingFallure exception with process pid and the original error. -- To view, visit https://gerrit.ovirt.org/65325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: config: add tunables for container support
Francesco Romani has posted comments on this change. Change subject: config: add tunables for container support .. Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/64243/2//COMMIT_MSG Commit Message: Line 5: CommitDate: 2016-10-04 10:07:37 +0200 Line 6: Line 7: config: add tunables for container support Line 8: Line 9: Add the config items we'll need later on > Sentence regarding how were the defaults picked would be nice. Done Line 10: Line 11: Change-Id: I30e4372fc88ebb7e68cc3b982af3c2eefacef7ce -- To view, visit https://gerrit.ovirt.org/64243 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30e4372fc88ebb7e68cc3b982af3c2eefacef7ce Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: vmxml: export container metadata
Francesco Romani has posted comments on this change. Change subject: vmxml: export container metadata .. Patch Set 23: (6 comments) https://gerrit.ovirt.org/#/c/60481/23/vdsm/virt/vmxml.py File vdsm/virt/vmxml.py: PS23, Line 29: from vdsm.containersconnection import XML as xmlcont > This is added in the following patch, why is it here? rebase glitch, will fix. Line 28: from vdsm import constants Line 29: from vdsm.containersconnection import XML as xmlcont Line 30: from vdsm import cpuarch Line 31: from vdsm import utils Line 32: > unrelated and unneeded will remove Line 33: Line 34: METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0' Line 35: METADATA_VM_TUNE_ELEMENT = 'qos' Line 36: METADATA_VM_TUNE_PREFIX = 'ovirt' Line 33: Line 34: METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0' Line 35: METADATA_VM_TUNE_ELEMENT = 'qos' Line 36: METADATA_VM_TUNE_PREFIX = 'ovirt' Line 37: > unrelated and unneeded will remove Line 38: Line 39: _BOOT_MENU_TIMEOUT = 1 # milliseconds Line 40: Line 41: PS23, Line 296: six.iteritems(drive_map) > same as below Done PS23, Line 612: six.iteritems(custom) > Do you expect custom to be ever accessed concurrently? itemview seems reaso no concurrent access is expected in this flow, so I'll switch to items() PS23, Line 616: except ValueError: > If key starts with 'volume:', you'll always have 2 values e.g. Actually I don't, will just remove the try block -- To view, visit https://gerrit.ovirt.org/60481 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ade3c0c7d300c5ce33cb23723c3d0e59e4af664 Gerrit-PatchSet: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: utils: Raise detectable error if termination fail
Nir Soffer has posted comments on this change. Change subject: utils: Raise detectable error if termination fail .. Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG Commit Message: Line 14: job running the process must stay in running state to prevent engine Line 15: from starting the same operation on another machine. Line 16: Line 17: Typically when we cannot handle an error we should not handle it. But Line 18: in this special case, letting the caller handle the error is not useful > Trying to make it more clear in the next version. Tried, I cannot see what is confusing and how to improve this. Please suggest an improved text. Line 19: since it does not have any context. Line 20: Line 21: To make it possible to handle this critical error, we raise now a new Line 22: TerminatingFallure exception with process pid and the original error. -- To view, visit https://gerrit.ovirt.org/65325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Refine TerminationTests names
Nir Soffer has posted comments on this change. Change subject: tests: Refine TerminationTests names .. Patch Set 5: (3 comments) https://gerrit.ovirt.org/#/c/65326/5//COMMIT_MSG Commit Message: Line 5: CommitDate: 2016-10-13 03:07:01 +0300 Line 6: Line 7: tests: Refine TerminationTests names Line 8: Line 9: - Al the test are testing termination, there is no point in repeating > For all the tests that test termination there is no need to repeat... Done Line 10: the class name in the test names. Line 11: - Use the tests name to describe the situation tested; terminating a Line 12: running process, or kill failure. Line 13: - Rename fake_kill to fail; this name can be used later for other Line 7: tests: Refine TerminationTests names Line 8: Line 9: - Al the test are testing termination, there is no point in repeating Line 10: the class name in the test names. Line 11: - Use the tests name to describe the situation tested; terminating a > test names Done Line 12: running process, or kill failure. Line 13: - Rename fake_kill to fail; this name can be used later for other Line 14: failure tests. Line 15: Line 10: the class name in the test names. Line 11: - Use the tests name to describe the situation tested; terminating a Line 12: running process, or kill failure. Line 13: - Rename fake_kill to fail; this name can be used later for other Line 14: failure tests. > what do you mean? how do you want to do it? (my questions based on the code I was planning to move this to a module function, and reuse the same function all the tests. When I added the next tests, I discovered that we cannot reuse it since it should raise different exception and different text, so the best way is to have a nested function in the test. Hopefully more clear in the next version. Line 15: Line 16: Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d -- To view, visit https://gerrit.ovirt.org/65326 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Use Popen.poll() for running state
gerrit-hooks has posted comments on this change. Change subject: tests: Use Popen.poll() for running state .. Patch Set 5: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65323 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I086e9317b5f8e5324d86bedeb04b485c7a09ad16 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add test for terminating a terminated process
gerrit-hooks has posted comments on this change. Change subject: tests: Add test for terminating a terminated process .. Patch Set 7: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65328 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic0f06c28857664cc49beb938f33ac3c9d07ca3b6 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Wait for child process in tearDown
gerrit-hooks has posted comments on this change. Change subject: tests: Wait for child process in tearDown .. Patch Set 5: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65322 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I248fa0b1c2129f69164be447402276d7908ed768 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add tests for waiting on a zombie process
gerrit-hooks has posted comments on this change. Change subject: tests: Add tests for waiting on a zombie process .. Patch Set 7: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65327 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: utils: Raise detectable error if termination fail
gerrit-hooks has posted comments on this change. Change subject: utils: Raise detectable error if termination fail .. Patch Set 6: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Refine TerminationTests names
gerrit-hooks has posted comments on this change. Change subject: tests: Refine TerminationTests names .. Patch Set 6: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65326 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: utils: Wait for terminated process
gerrit-hooks has posted comments on this change. Change subject: utils: Wait for terminated process .. Patch Set 5: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65324 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida04e2c7ba092efdc2927ed9f460b0098ba2ad94 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: utils: Add missing Popen methods
gerrit-hooks has posted comments on this change. Change subject: utils: Add missing Popen methods .. Patch Set 5: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65321 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d562663698d86b98b55d139e2691a0f51566ccf Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add tests for poll and wait failures
gerrit-hooks has posted comments on this change. Change subject: tests: Add tests for poll and wait failures .. Patch Set 9: * Update Tracker::IGNORE, not relevant for branch: master * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65294 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: utils: Raise detectable error if termination fail
Nir Soffer has posted comments on this change. Change subject: utils: Raise detectable error if termination fail .. Patch Set 6: Verified+1 -- To view, visit https://gerrit.ovirt.org/65325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Refine TerminationTests names
Nir Soffer has posted comments on this change. Change subject: tests: Refine TerminationTests names .. Patch Set 6: Verified+1 -- To view, visit https://gerrit.ovirt.org/65326 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: build: Replace pep8 and pyflakes with flake8
Nir Soffer has posted comments on this change. Change subject: build: Replace pep8 and pyflakes with flake8 .. Patch Set 2: Edward, flake8 works on both python 2 and 3, but this is not relevant to our tests. -- To view, visit https://gerrit.ovirt.org/65403 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib42430f045e4475c31cd370b4196214094c29cd1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: build: Run the tests with tox
Nir Soffer has posted comments on this change. Change subject: build: Run the tests with tox .. Patch Set 8: Edward, this works with python 3 as you can see in the jenkins and travis runs. But in python 3 run, we are using the distro packages. In the distant future we will have a separate python 3 env running all the tests that can run with python 3, and then we would require nose for python 3. -- To view, visit https://gerrit.ovirt.org/65404 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I245a171940a5e869fd719a6410024ee77e8ad86c Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: py3: make gluster_cli_tests pass
Dan Kenigsberg has submitted this change and it was merged. Change subject: py3: make gluster_cli_tests pass .. py3: make gluster_cli_tests pass Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24 Signed-off-by: Dan Kenigsberg Reviewed-on: https://gerrit.ovirt.org/65008 Continuous-Integration: Jenkins CI Reviewed-by: Piotr Kliczewski --- M lib/vdsm/kaxmlrpclib.py M lib/vdsm/network/netlink/addr.py M tests/Makefile.am M tests/gluster_cli_tests.py M vdsm/gluster/storagedev.py 5 files changed, 14 insertions(+), 13 deletions(-) Approvals: Piotr Kliczewski: Looks good to me, but someone else must approve Jenkins CI: Passed CI tests Dan Kenigsberg: Verified; Looks good to me, approved -- To view, visit https://gerrit.ovirt.org/65008 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Ramesh N Gerrit-Reviewer: Sahina Bose Gerrit-Reviewer: gerrit-hooks ___ 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]: py3: make gluster_cli_tests pass
gerrit-hooks has posted comments on this change. Change subject: py3: make gluster_cli_tests pass .. Patch Set 6: * Update Tracker::IGNORE, no bug url/s found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/65008 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Ramesh N Gerrit-Reviewer: Sahina Bose 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]: py3: make gluster_cli_tests pass
Dan Kenigsberg has posted comments on this change. Change subject: py3: make gluster_cli_tests pass .. Patch Set 5: Code-Review+2 copying dropped scores, and raising. -- To view, visit https://gerrit.ovirt.org/65008 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Ramesh N Gerrit-Reviewer: Sahina Bose 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]: osinfo: drop package buildtime
Dan Kenigsberg has posted comments on this change. Change subject: osinfo: drop package buildtime .. Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/65059 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: osinfo: drop package buildtime
Dan Kenigsberg has submitted this change and it was merged. Change subject: osinfo: drop package buildtime .. osinfo: drop package buildtime Buildtime of host packages was never used and only causes us problems w/ parsing kernel cmdline. We are safe to drop it. Signed-off-by: Martin Polednik Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9 Reviewed-on: https://gerrit.ovirt.org/65059 Reviewed-by: Michal Skrivanek Continuous-Integration: Jenkins CI Reviewed-by: Piotr Kliczewski Reviewed-by: Dan Kenigsberg --- M lib/api/vdsm-api.yml M lib/vdsm/osinfo.py 2 files changed, 3 insertions(+), 15 deletions(-) Approvals: Piotr Kliczewski: Looks good to me, but someone else must approve Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Michal Skrivanek: Looks good to me, but someone else must approve Martin Polednik: Verified -- To view, visit https://gerrit.ovirt.org/65059 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks ___ 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]: osinfo: drop package buildtime
gerrit-hooks has posted comments on this change. Change subject: osinfo: drop package buildtime .. Patch Set 5: * Update Tracker::IGNORE, no bug url/s found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/65059 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: osinfo: un-nest kernel version gathering function
gerrit-hooks has posted comments on this change. Change subject: osinfo: un-nest kernel version gathering function .. Patch Set 3: * Update Tracker::IGNORE, no bug url/s found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/65379 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77f78882973855cc971bbdae66cc96c8068e8230 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal 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]: osinfo: un-nest kernel version gathering function
Dan Kenigsberg has submitted this change and it was merged. Change subject: osinfo: un-nest kernel version gathering function .. osinfo: un-nest kernel version gathering function Previous function, kernelDict, had unclear naming and was needlessly nested within package_versions function. This patch un-nests the function and renames it to _runtime_kernel_version. The new name hints why we don't look up the version via rpmdb. Change-Id: I77f78882973855cc971bbdae66cc96c8068e8230 Signed-off-by: Martin Polednik Reviewed-on: https://gerrit.ovirt.org/65379 Reviewed-by: Francesco Romani Continuous-Integration: Jenkins CI --- M lib/vdsm/osinfo.py 1 file changed, 12 insertions(+), 11 deletions(-) Approvals: Jenkins CI: Passed CI tests Francesco Romani: Looks good to me, approved Martin Polednik: Verified -- To view, visit https://gerrit.ovirt.org/65379 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I77f78882973855cc971bbdae66cc96c8068e8230 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: gerrit-hooks ___ 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]: qemuimg: Expose API for qemuimg map
Nir Soffer has posted comments on this change. Change subject: qemuimg: Expose API for qemuimg map .. Patch Set 5: (7 comments) Very nice! https://gerrit.ovirt.org/#/c/65112/5/tests/qemuimg_test.py File tests/qemuimg_test.py: Line 459: # Empty run Line 460: run = img_map[0] Line 461: self.assertEqual(run["length"], size) Line 462: self.assertEqual(run["data"], False) Line 463: self.assertEqual(run["zero"], True) Lets use the same format as in the other tests, and call the new shiny helper to verify. Line 464: Line 465: @permutations([ Line 466: # offset, qcow2_compat Line 467: (4 * 1024, "0.10"), Line 462: self.assertEqual(run["data"], False) Line 463: self.assertEqual(run["zero"], True) Line 464: Line 465: @permutations([ Line 466: # offset, qcow2_compat This is length, not offset. Line 467: (4 * 1024, "0.10"), Line 468: (4 * 1024, "1.1"), Line 469: (64 * 1024, "0.10"), Line 470: (64 * 1024, "1.1"), Line 479: qemu_pattern_write(image, fmt, offset=offset, len=length, Line 480:pattern=0xf0) Line 481: Line 482: img_map = qemuimg.map(image) Line 483: self.assertEqual(3, len(img_map)) We can move this check into the helper: self.assertEqual(len(actual), len(expected)) Line 484: Line 485: expected = [ Line 486: # run 1 - empty Line 487: { Line 508: ] Line 509: Line 510: for actual, expected in zip(img_map, expected): Line 511: for key in expected: Line 512: self.assertEqual(actual[key], expected[key]) We have same verification code twice, and we actually want to use it for the first test. Less move this into a helper and use it in all tests. Something like: self.check_map(actual, expected) Line 513: Line 514: @permutations([ Line 515: # offset, length, expected_length, expected_start, qcow2_compat Line 516: (64 * 1024, 4 * 1024, 65536, "0.10"), Line 528: qemu_pattern_write(image, fmt, offset=offset, len=length, Line 529:pattern=0xf0) Line 530: Line 531: img_map = qemuimg.map(image) Line 532: self.assertEqual(3, len(img_map)) Move to check_map helper Line 533: Line 534: expected = [ Line 535: # run 1 - empty Line 536: { Line 541: }, Line 542: # run 2 - data Line 543: { Line 544: "start": offset, Line 545: "offset": 327680, Lets add a comment abut this (no clue what is this value), or just remove it since we don't depend on this value. Line 546: "length": expected_length, Line 547: "data": True, Line 548: "zero": False, Line 549: }, Line 557: ] Line 558: Line 559: for actual, expected in zip(img_map, expected): Line 560: for key in expected: Line 561: self.assertEqual(actual[key], expected[key]) Repalce wiith call to check_map Line 562: Line 563: Line 564: def make_image(path, size, format, index, qcow2_compat, backing=None): Line 565: qemuimg.create(path, size=size, format=format, qcow2Compat=qcow2_compat, -- To view, visit https://gerrit.ovirt.org/65112 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer 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]: utils: Raise detectable error if termination fail
Piotr Kliczewski has posted comments on this change. Change subject: utils: Raise detectable error if termination fail .. Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG Commit Message: Line 14: job running the process must stay in running state to prevent engine Line 15: from starting the same operation on another machine. Line 16: Line 17: Typically when we cannot handle an error we should not handle it. But Line 18: in this special case, letting the caller handle the error is not useful > Tried, I cannot see what is confusing and how to improve this. Please sugge My confusion is that we say handling error is not useful but later we say that by creating new Exception and passing pid is fine. So I would go with: But in this special case, letting the caller to handle the error without context is not useful. By adding new TrminatingFailure exception with pid we solve missing context issue. Line 19: since it does not have any context. Line 20: Line 21: To make it possible to handle this critical error, we raise now a new Line 22: TerminatingFallure exception with process pid and the original error. -- To view, visit https://gerrit.ovirt.org/65325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Refine TerminationTests names
Piotr Kliczewski has posted comments on this change. Change subject: tests: Refine TerminationTests names .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65326 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add tests for waiting on a zombie process
Piotr Kliczewski has posted comments on this change. Change subject: tests: Add tests for waiting on a zombie process .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/65327/6/tests/utilsTests.py File tests/utilsTests.py: Line 102: self.proc.terminate() Line 103: wait_for_zombie(self.proc, 1) Line 104: Line 105: def fail(): Line 106: raise RuntimeError("Attempt to kill a zombie process") > This raises rumtime error and will fail the test if raised. In patch #65328 we use RuntimeError as well. We mentioned in commit message reuse of this method so let's do it. Line 107: Line 108: self.proc.kill = fail Line 109: with utils.terminating(self.proc): Line 110: pass -- To view, visit https://gerrit.ovirt.org/65327 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: virt: Try to detect non guest iniated shutdowns
Milan Zamazal has posted comments on this change. Change subject: virt: Try to detect non guest iniated shutdowns .. Patch Set 7: Code-Review+1 Maybe _shutdownLock is no longer necessary? -- To view, visit https://gerrit.ovirt.org/64991 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie04b9806fbf0a81dc576aa28cfdda5edb079ce29 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: net test: fix ovs_test:test_dry_run
Edward Haas has posted comments on this change. Change subject: net test: fix ovs_test:test_dry_run .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieb5c70c1498bbe85c5fdd9b0cec63761f2cf7c0d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI 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]: storagetestlib: Add sdVersion param to the test API.
Nir Soffer has posted comments on this change. Change subject: storagetestlib: Add sdVersion param to the test API. .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/65308/3/tests/storagetestlib_test.py File tests/storagetestlib_test.py: Line 91: self.assertEquals(3, env.sd_manifest.getVersion()) Line 92: Line 93: def test_block_domain_version_4(self): Line 94: with fake_env('block', sd_version=4) as env: Line 95: self.assertEquals(4, env.sd_manifest.getVersion()) Nice! But wouldn't it be even better if we merge to one test with permutations? @permutations([ # env_type, sd_version ("file", 3), ("file", 4), ("block", 3), ("block", 4), ]) def test_domain_version(self, env_type, sd_version): with fake_env(env_type, sd_version=sd_version) as env: self.assertEquals(sd_version, env.sd_manifest.getVersion()) And we can have another test for the default. Line 96: Line 97: def test_volume_structure(self): Line 98: with fake_file_env() as env: Line 99: img_id = make_uuid() -- To view, visit https://gerrit.ovirt.org/65308 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9759fa6d8b2bd7c359fd188d1e18ffbba5004941 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer 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]: tests: Add tests for poll and wait failures
Piotr Kliczewski has posted comments on this change. Change subject: tests: Add tests for poll and wait failures .. Patch Set 8: (1 comment) https://gerrit.ovirt.org/#/c/65294/8/tests/utilsTests.py File tests/utilsTests.py: Line 132: self.check_failure() Line 133: Line 134: def test_kill_failure(self): Line 135: def fail(): Line 136: raise ExpectedFailure("Fake kill failure") > 3 test, 3 fail texts, please suggest a simple and more clear code. Why not to have fail method as top level and use it 138 and in 145 line. We do not need to have it in two places. Line 137: Line 138: self.proc.kill = fail Line 139: self.check_failure() Line 140: -- To view, visit https://gerrit.ovirt.org/65294 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: hsm:Use sd compat instead of qemu conf compat.
Nir Soffer has posted comments on this change. Change subject: hsm:Use sd compat instead of qemu conf compat. .. Patch Set 10: (4 comments) https://gerrit.ovirt.org/#/c/64951/10/tests/storage_hsm_test.py File tests/storage_hsm_test.py: Line 98 Line 99 Line 100 Line 101 Line 102 But you check that the code raises?! Line 88: ('0.10', '0.10', 4), Line 89: ('1.1', '0.10', 4), Line 90: ('0.10', '0.10', 3), Line 91: )) Line 92: def test_disabled_compat_raises(self, hsm_compat, qemu_config, sd_version): This test check the invalid compat raises... Line 93: with self.fake_volume(sc.COW_FORMAT, sd_version) as vol: Line 94: create_conf = make_config([('irs', 'qcow2_compat', qemu_config)]) Line 95: info = {"format": qemuimg.FORMAT.QCOW2, "compat": hsm_compat} Line 96: with MonkeyPatchScope([(qemuimg, 'config', create_conf), Line 98: qemuimg.create(vol.volumePath, size=self.SIZE, Line 99:format=qemuimg.FORMAT.QCOW2) Line 100: h = FakeHSM() Line 101: self.assertNotRaises(h.verify_untrusted_volume, 'sp', Line 102: vol.sdUUID, vol.imgUUID, vol.volUUID) But you check that the code does not raise?! Line 103: Line 104: @permutations(( Line 105: ('1.1', '0.10', 3), Line 106: )) Line 103: Line 104: @permutations(( Line 105: ('1.1', '0.10', 3), Line 106: )) Line 107: def test_valid_compat(self, hsm_compat, qemu_config, sd_version): This test should test valid compats... Line 108: with self.fake_volume(sc.COW_FORMAT, sd_version) as vol: Line 109: create_conf = make_config([('irs', 'qcow2_compat', qemu_config)]) Line 110: info = {"format": qemuimg.FORMAT.QCOW2, "compat": hsm_compat} Line 111: with MonkeyPatchScope([(qemuimg, 'config', create_conf), -- To view, visit https://gerrit.ovirt.org/64951 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4b19a99e5d9a73c011bf6d8079e3855298561b9 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Amit Aviram Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer 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]: after_vm_destroy.py: migrate to jsonrpcvdscli
gerrit-hooks has posted comments on this change. Change subject: after_vm_destroy.py: migrate to jsonrpcvdscli .. Patch Set 7: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62383 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f3fa6479dde2c4a1298d0ae167d888d9f7e020a Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Irit Goihman Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: build: Replace pep8 and pyflakes with flake8
Edward Haas has posted comments on this change. Change subject: build: Replace pep8 and pyflakes with flake8 .. Patch Set 2: What do you mean it is not relevant? We need it installed once for py2 and once for py3 so tests will pass, no? Does tox take care of that somehow? -- To view, visit https://gerrit.ovirt.org/65403 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib42430f045e4475c31cd370b4196214094c29cd1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: vdsm: virt: add optional container support
Francesco Romani has posted comments on this change. Change subject: vdsm: virt: add optional container support .. Patch Set 51: (9 comments) https://gerrit.ovirt.org/#/c/53820/51/lib/vdsm/containersconnection.py File lib/vdsm/containersconnection.py: PS51, Line 33: _runtimes = frozenset() > I have inner hate for how this is further "calculated" (also considering th no, we don't really need a frozenset, we can probably kill it. https://gerrit.ovirt.org/#/c/53820/51/lib/vdsm/containerslib.py File lib/vdsm/containerslib.py: PS51, Line 99: def _is_cmd(argv, reqv): > Why this isn't attribute of SuperVdsmCommand class? It's slight implementat true, but OTOH this code doesn't use the SuperVdsmCommand state at all, so could also be a free function PS51, Line 113: def _call(ret): > Same as above, I believe it should be __call__ on top of that (internally c SubProcCommand.__call__ already does that! Again, this is a free function because it doesn't use any of the SuperVdsmCommand state. Anyway it's open for discussion, I'm not entirely happy with both designs. If you think it's clearer as method, I'll make this a method (same for _is_cmd above) PS51, Line 153: > container connection Done PS51, Line 161: ev > s/ev/event Done PS51, Line 177: For clearing connections during the tests. > Only? Runtime code shouldn't be polluted by test helpers. And for consistency with libvirtconnection. But we don't really care here, so let's kill this. https://gerrit.ovirt.org/#/c/53820/51/vdsm/virt/recovery.py File vdsm/virt/recovery.py: PS51, Line 167: all_vms > s/vms/domains ? yes, a bit better. Changed. PS51, Line 177: def _all_vms_from_runtime(cif): > s/vms/domains ? _all_running_domains(cif) is probably clearer and a better name https://gerrit.ovirt.org/#/c/53820/51/vdsm/virt/vm.py File vdsm/virt/vm.py: PS51, Line 1657: if is_kvm(self.conf): > My vote for split or helper function remains. Let's retry with a different partiotioning. -- To view, visit https://gerrit.ovirt.org/53820 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id236a30a5c875994c037b8d00c7463bceaab143f Gerrit-PatchSet: 51 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: copy_data: Add qcow2_compat on convert.
Nir Soffer has posted comments on this change. Change subject: copy_data: Add qcow2_compat on convert. .. Patch Set 24: (3 comments) Nice! https://gerrit.ovirt.org/#/c/64373/24/tests/storage_sdm_copy_data_test.py File tests/storage_sdm_copy_data_test.py: Line 76: Line 77: @contextmanager Line 78: def get_vols(self, storage_type, src_fmt, dst_fmt, chain_length=1, Line 79: size=DEFAULT_SIZE, sd_version=3, src_qcow2_compat='0.10'): Line 80: with fake_env(storage_type, sd_version) as env: sd_version is a kwarg, please always call it sd_version=value Line 81: rm = FakeResourceManager() Line 82: with MonkeyPatchScope([ Line 83: (guarded, 'context', fake_guarded_context()), Line 84: (storage.sdm.api.copy_data, 'sdCache', env.sdcache), Line 197: ('block', 'cow', 'cow', '1.1', 4), Line 198: ('block', 'cow', 'cow', '0.10', 3), Line 199: )) Line 200: def test_volume_qcow2v3(self, env_type, src_fmt, dst_fmt, src_qcow2_compat, Line 201: dst_sd_version): In this test we are actually testing coping in the same domain, so the source sd compat and destination sd compat are the same. It would be more interesting to test copying form old domain (version=3) to a new domain (version=4), or the opposite (from 4 to 3), but we don't have infrastructure for this yet. Please add a TODO for these tests. Line 202: src_fmt = sc.name2type(src_fmt) Line 203: dst_fmt = sc.name2type(dst_fmt) Line 204: job_id = make_uuid() Line 205: Line 220: job = storage.sdm.api.copy_data.Job(job_id, 0, source, dest) Line 221: Line 222: job.run() Line 223: wait_for_job(job) Line 224: self.assertEqual(jobs.STATUS.DONE, job.status) Please add: verify_qemu_chain(dst_chain) And also check that the destination images are using the correct compat. You can do this using qemuimg.info(path)["compat"] Line 225: Line 226: def test_bad_vm_configuration_volume(self): Line 227: """ Line 228: When copying a volume containing VM configuration information the -- To view, visit https://gerrit.ovirt.org/64373 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie26e5dcba6fc493b32ea7764889df2918c4dfdd3 Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Nir Soffer 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]: network: supervdsm: configure container networks
Francesco Romani has posted comments on this change. Change subject: network: supervdsm: configure container networks .. Patch Set 44: > The code is somewhat fine considering the TODOs, but question remains: if I > install vdsm-containers AFTER already using VDSM, do I explicitly have to > setup_networks again? How is user informed? This is material for another TODO :) -- To view, visit https://gerrit.ovirt.org/54998 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I338ca2d3abb0b1447c5a18c97afb9e14314f4107 Gerrit-PatchSet: 44 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: utils: Raise detectable error if termination fail
Nir Soffer has posted comments on this change. Change subject: utils: Raise detectable error if termination fail .. Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG Commit Message: Line 14: job running the process must stay in running state to prevent engine Line 15: from starting the same operation on another machine. Line 16: Line 17: Typically when we cannot handle an error we should not handle it. But Line 18: in this special case, letting the caller handle the error is not useful > My confusion is that we say handling error is not useful but later we say t OK, using your text. Line 19: since it does not have any context. Line 20: Line 21: To make it possible to handle this critical error, we raise now a new Line 22: TerminatingFallure exception with process pid and the original error. -- To view, visit https://gerrit.ovirt.org/65325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: utils: Raise detectable error if termination fail
Nir Soffer has posted comments on this change. Change subject: utils: Raise detectable error if termination fail .. Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG Commit Message: Line 14: job running the process must stay in running state to prevent engine Line 15: from starting the same operation on another machine. Line 16: Line 17: Typically when we cannot handle an error we should not handle it. But Line 18: in this special case, letting the caller handle the error is not useful > OK, using your text. With some modifications, hopefully this is more clear in the current version. Line 19: since it does not have any context. Line 20: Line 21: To make it possible to handle this critical error, we raise now a new Line 22: TerminatingFallure exception with process pid and the original error. -- To view, visit https://gerrit.ovirt.org/65325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add tests for waiting on a zombie process
Nir Soffer has posted comments on this change. Change subject: tests: Add tests for waiting on a zombie process .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/65327/6/tests/utilsTests.py File tests/utilsTests.py: Line 102: self.proc.terminate() Line 103: wait_for_zombie(self.proc, 1) Line 104: Line 105: def fail(): Line 106: raise RuntimeError("Attempt to kill a zombie process") > In patch #65328 we use RuntimeError as well. We mentioned in commit message Yes, but with different error text - this should make debugging the tests easier in case of errors. We can do something like this: def fail(exception, msg): raise exception(msg) And then n each test we can do: self.proc.kill = partial(fail, RuntimeError, "Attempt to kill a zombie process") But I find this much harder to understand compared to: def fail(): raise RuntimeError("Attempt to kill a zombie process") self.proc.kill = fail The best would be to use lambda: self.proc.kill = lambda: raise RuntimeError("Attempt to kill a zombie process") But lambda does not support this syntax. Line 107: Line 108: self.proc.kill = fail Line 109: with utils.terminating(self.proc): Line 110: pass -- To view, visit https://gerrit.ovirt.org/65327 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Add tests for poll and wait failures
Nir Soffer has posted comments on this change. Change subject: tests: Add tests for poll and wait failures .. Patch Set 8: (1 comment) https://gerrit.ovirt.org/#/c/65294/8/tests/utilsTests.py File tests/utilsTests.py: Line 132: self.check_failure() Line 133: Line 134: def test_kill_failure(self): Line 135: def fail(): Line 136: raise ExpectedFailure("Fake kill failure") > Why not to have fail method as top level and use it 138 and in 145 line. We Because it raises different exceptions with different message? Line 137: Line 138: self.proc.kill = fail Line 139: self.check_failure() Line 140: -- To view, visit https://gerrit.ovirt.org/65294 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: containers: add testsuite
gerrit-hooks has posted comments on this change. Change subject: tests: containers: add testsuite .. Patch Set 23: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/60678 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I27ba3cecbd71b7bbba94992d6bc63ca29333e313 Gerrit-PatchSet: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani 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]: vmxml: export container metadata
gerrit-hooks has posted comments on this change. Change subject: vmxml: export container metadata .. Patch Set 25: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/60481 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ade3c0c7d300c5ce33cb23723c3d0e59e4af664 Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: **WIP** tool: reconfigure containers networks
gerrit-hooks has posted comments on this change. Change subject: **WIP** tool: reconfigure containers networks .. Patch Set 19: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/60918 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6053e283c004cd61ba7727cea22ba73a631180ba Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani 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]: WIP: docs: add tutorial
gerrit-hooks has posted comments on this change. Change subject: WIP: docs: add tutorial .. Patch Set 2: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65400 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a9e947c30f2978fd9670fd64d99bf380181aa9c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani 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]: supervdsm: expose systemd utilities
gerrit-hooks has posted comments on this change. Change subject: supervdsm: expose systemd utilities .. Patch Set 33: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/56491 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I38e9a346da784fc200a82d9e5d9fdf665e752987 Gerrit-PatchSet: 33 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal 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]: network: supervdsm: configure container networks
gerrit-hooks has posted comments on this change. Change subject: network: supervdsm: configure container networks .. Patch Set 46: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/54998 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I338ca2d3abb0b1447c5a18c97afb9e14314f4107 Gerrit-PatchSet: 46 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: config: add tunables for container support
gerrit-hooks has posted comments on this change. Change subject: config: add tunables for container support .. Patch Set 4: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64243 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30e4372fc88ebb7e68cc3b982af3c2eefacef7ce Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: network: filter out 'veth' devices
gerrit-hooks has posted comments on this change. Change subject: network: filter out 'veth' devices .. Patch Set 20: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/60821 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I859c4bc885c0afd99fdaf741706d9bd1538850e6 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Francesco Romani 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]: core: containers: add the container support module
gerrit-hooks has posted comments on this change. Change subject: core: containers: add the container support module .. Patch Set 35: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/59824 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fb768ea97dd719cde9bd5e57e1b7cabe4b0f0ae Gerrit-PatchSet: 35 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: vdsm: virt: add optional container support
gerrit-hooks has posted comments on this change. Change subject: vdsm: virt: add optional container support .. Patch Set 53: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/53820 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id236a30a5c875994c037b8d00c7463bceaab143f Gerrit-PatchSet: 53 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: static: move vdsm bonding defaults under static
Francesco Romani has posted comments on this change. Change subject: static: move vdsm bonding defaults under static .. Patch Set 6: Code-Review+2 (1 comment) https://gerrit.ovirt.org/#/c/62537/6/static/Makefile.am File static/Makefile.am: PS6, Line 24: /usr/share > It's referring to a source path, not the target. Target is vdsmdir. Right, I was too hasty reading. -- To view, visit https://gerrit.ovirt.org/62537 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9cbd81e59aec49876e15337d7cd617e51316a604 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Ondřej Svoboda 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]: static: move man under static
Francesco Romani has posted comments on this change. Change subject: static: move man under static .. Patch Set 5: Code-Review+2 same answer as per parent patch, no reason to hold +2 -- To view, visit https://gerrit.ovirt.org/62549 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I91fa36b38904d85fc60734ce966b8d993911b490 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: use assertXMLEqual in vmTests
Nir Soffer has posted comments on this change. Change subject: tests: use assertXMLEqual in vmTests .. Patch Set 1: Looks good, need to make pep8 happy. ./tests/vmTests.py:636:80: E501 line too long (81 > 79 characters) ./tests/vmTests.py:832:80: E501 line too long (81 > 79 characters) -- To view, visit https://gerrit.ovirt.org/65417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6516404548eb000f9501818818c929df09c406e2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer 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]: tests: use assertXMLEqual in vmTests
Nir Soffer has posted comments on this change. Change subject: tests: use assertXMLEqual in vmTests .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65417 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6516404548eb000f9501818818c929df09c406e2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer 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]: [WIP] use libvirt domain xml prepared in advance
Francesco Romani has posted comments on this change. Change subject: [WIP] use libvirt domain xml prepared in advance .. Patch Set 2: Code-Review+1 (1 comment) https://gerrit.ovirt.org/#/c/65182/2/vdsm/virt/vmxml.py File vdsm/virt/vmxml.py: Line 219: self._devices = Element('devices') Line 220: self.dom.appendChild(self._devices) Line 221: Line 222: self.appendMetadata() Line 223: self.doc.appendChild(self.dom) I think this is redundant because minidom implicitely refers to the main Document. Or this line fixes an issue you actually found? Line 224: Line 225: def appendClock(self): Line 226: """ Line 227: Add element to domain: -- To view, visit https://gerrit.ovirt.org/65182 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I469fad3ca14a6b7f4675ef5c200175053f6dd4af Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: [WIP] use libvirt domain xml prepared in advance
Francesco Romani has posted comments on this change. Change subject: [WIP] use libvirt domain xml prepared in advance .. Patch Set 2: -Code-Review -- To view, visit https://gerrit.ovirt.org/65182 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I469fad3ca14a6b7f4675ef5c200175053f6dd4af Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: virt: Try to detect non guest iniated shutdowns
Francesco Romani has posted comments on this change. Change subject: virt: Try to detect non guest iniated shutdowns .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/64991 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie04b9806fbf0a81dc576aa28cfdda5edb079ce29 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: virt: Make DomainDescriptor use XML helpers
Francesco Romani has posted comments on this change. Change subject: virt: Make DomainDescriptor use XML helpers .. Patch Set 9: Code-Review+2 *Really* nice improvements. I like this. -- To view, visit https://gerrit.ovirt.org/55769 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib169735936d19171ff8b8d127666d7258c308f34 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer 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]: qemuimg: Expose API for qemuimg map
gerrit-hooks has posted comments on this change. Change subject: qemuimg: Expose API for qemuimg map .. Patch Set 6: * Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/65112 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer 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]: qemuimg: Expose API for qemuimg map
Ala Hino has posted comments on this change. Change subject: qemuimg: Expose API for qemuimg map .. Patch Set 5: (7 comments) https://gerrit.ovirt.org/#/c/65112/5/tests/qemuimg_test.py File tests/qemuimg_test.py: Line 459: # Empty run Line 460: run = img_map[0] Line 461: self.assertEqual(run["length"], size) Line 462: self.assertEqual(run["data"], False) Line 463: self.assertEqual(run["zero"], True) > Lets use the same format as in the other tests, and call the new shiny help Done Line 464: Line 465: @permutations([ Line 466: # offset, qcow2_compat Line 467: (4 * 1024, "0.10"), Line 462: self.assertEqual(run["data"], False) Line 463: self.assertEqual(run["zero"], True) Line 464: Line 465: @permutations([ Line 466: # offset, qcow2_compat > This is length, not offset. Done Line 467: (4 * 1024, "0.10"), Line 468: (4 * 1024, "1.1"), Line 469: (64 * 1024, "0.10"), Line 470: (64 * 1024, "1.1"), Line 479: qemu_pattern_write(image, fmt, offset=offset, len=length, Line 480:pattern=0xf0) Line 481: Line 482: img_map = qemuimg.map(image) Line 483: self.assertEqual(3, len(img_map)) > We can move this check into the helper: Done Line 484: Line 485: expected = [ Line 486: # run 1 - empty Line 487: { Line 508: ] Line 509: Line 510: for actual, expected in zip(img_map, expected): Line 511: for key in expected: Line 512: self.assertEqual(actual[key], expected[key]) > We have same verification code twice, and we actually want to use it for th Done Line 513: Line 514: @permutations([ Line 515: # offset, length, expected_length, expected_start, qcow2_compat Line 516: (64 * 1024, 4 * 1024, 65536, "0.10"), Line 528: qemu_pattern_write(image, fmt, offset=offset, len=length, Line 529:pattern=0xf0) Line 530: Line 531: img_map = qemuimg.map(image) Line 532: self.assertEqual(3, len(img_map)) > Move to check_map helper Done Line 533: Line 534: expected = [ Line 535: # run 1 - empty Line 536: { Line 541: }, Line 542: # run 2 - data Line 543: { Line 544: "start": offset, Line 545: "offset": 327680, > Lets add a comment abut this (no clue what is this value), or just remove i Removed ... Line 546: "length": expected_length, Line 547: "data": True, Line 548: "zero": False, Line 549: }, Line 557: ] Line 558: Line 559: for actual, expected in zip(img_map, expected): Line 560: for key in expected: Line 561: self.assertEqual(actual[key], expected[key]) > Repalce wiith call to check_map Done Line 562: Line 563: Line 564: def make_image(path, size, format, index, qcow2_compat, backing=None): Line 565: qemuimg.create(path, size=size, format=format, qcow2Compat=qcow2_compat, -- To view, visit https://gerrit.ovirt.org/65112 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer 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]: utils: Raise detectable error if termination fail
Francesco Romani has posted comments on this change. Change subject: utils: Raise detectable error if termination fail .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65325 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Refine TerminationTests names
Francesco Romani has posted comments on this change. Change subject: tests: Refine TerminationTests names .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65326 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: tests: Remove cPopenTests.py
Francesco Romani has posted comments on this change. Change subject: tests: Remove cPopenTests.py .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65407 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9dc384d6e84ce5f09573be32ea6247c6ef834836 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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]: config: add tunables for container support
Martin Polednik has posted comments on this change. Change subject: config: add tunables for container support .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/64243 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30e4372fc88ebb7e68cc3b982af3c2eefacef7ce Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik 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]: automation: Create coverage report sooner
Francesco Romani has posted comments on this change. Change subject: automation: Create coverage report sooner .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65414 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8688383eb5e99fdd0ee48815cc71afd6483819b4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer 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]: build: Replace pep8 and pyflakes with flake8
Francesco Romani has posted comments on this change. Change subject: build: Replace pep8 and pyflakes with flake8 .. Patch Set 2: Code-Review+1 good idea, I like this. -- To view, visit https://gerrit.ovirt.org/65403 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib42430f045e4475c31cd370b4196214094c29cd1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Irit Goihman Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim 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