Bug 1459075 - [RFE][CodeChange] Replace python-cpopen usage with python-subprocess32
[RFE][CodeChange] Replace python-cpopen usage with python-subprocess32
Status: CLOSED CURRENTRELEASE
Product: vdsm
Classification: oVirt
Component: RFEs (Show other bugs)
---
Unspecified Unspecified
low Severity medium (vote)
: ovirt-4.2.0
: ---
Assigned To: Nir Soffer
Lilach Zitnitski
: CodeChange, FutureFeature
Depends On: 1470218
Blocks:
  Show dependency treegraph
 
Reported: 2017-06-06 04:49 EDT by Dan Kenigsberg
Modified: 2017-12-20 06:16 EST (History)
15 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2017-12-20 06:16:08 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: Storage
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rule-engine: ovirt‑4.2+
ratamir: testing_plan_complete-
ylavi: planning_ack+
rule-engine: devel_ack+
ratamir: testing_ack+


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
oVirt gerrit 75009 None None None 2017-11-28 05:35 EST
oVirt gerrit 83458 None None None 2017-11-28 05:37 EST
oVirt gerrit 83643 None None None 2017-11-28 05:37 EST

  None (edit)
Description Dan Kenigsberg 2017-06-06 04:49:15 EDT
Description of problem:
python2's subprocess is buggy, which prompted us to replace it with python-cpopen. Since then, python-subprocess32[1] has provided a better solution: it backports python3's subprocess to python2.

Let us dump our own implementation, and replace it with a python3-compatible solution that is developed upstream.

The package is built in CentOS[2] so importing its RPM to RHV should be relatively easy.

[1] https://pypi.python.org/pypi/subprocess32/
[2] https://cbs.centos.org/koji/packageinfo?packageID=3725
Comment 3 Yaniv Bronhaim 2017-06-26 08:09:18 EDT
I started to check what is still missing for such https://gerrit.ovirt.org/78659 replacement. We still have in both imageSharing.py qemuimg.py a "deathSignal" usage that we need to eliminate. Except that, I will send PIPE as default stdout and stdin when we call execCmd, and the rest is fine.. I'll continue verification after imagesharing.py and qemuimg.py will be fixed. I'm not sure what is the reason for still counting on signal when we run with systemctl.. I though we're already fine there
Comment 4 Nir Soffer 2017-07-12 10:45:50 EDT
We will remove the deathSignal from the few places using it.
https://gerrit.ovirt.org/75009 is removing one instance.
Comment 5 Yaniv Bronhaim 2017-07-20 03:50:02 EDT
Moving the intention of this bug to be about the vdsm usages in python-copen - As we discussed few weeks ago, seems like all left locations that need modification are storage areas and requires storage team followup.
The build in centos is ready (see Bug 1470218). If something still missing let me know
Comment 6 Yaniv Lavi 2017-11-26 09:09:19 EST
What is the right target for this?
Comment 7 Nir Soffer 2017-11-28 05:33:55 EST
(In reply to Yaniv Lavi from comment #6)
> What is the right target for this?

The work was completed in vdsm few weeks ago, I think this can be in 4.2.0 and
mark as modified/onqa.
Comment 8 Nir Soffer 2017-11-28 05:40:59 EST
This is included in current master, so can be moved to ON_QA when qa have time to test it.

There is no behavior change, running automation is enough to verify the changes.
Comment 11 Nir Soffer 2017-11-28 05:55:44 EST
There was a similar effort in ioprocess - version 1.0 (working on the release)
also uses now suprocess32:
https://gerrit.ovirt.org/84291
Comment 14 Lilach Zitnitski 2017-12-12 08:04:08 EST
(In reply to Nir Soffer from comment #8)
> This is included in current master, so can be moved to ON_QA when qa have
> time to test it.
> 
> There is no behavior change, running automation is enough to verify the
> changes.

Based on this comment and since we haven't seen any unusual behavior in out automation tests, moving to verified.
Comment 15 Sandro Bonazzola 2017-12-20 06:16:08 EST
This bugzilla is included in oVirt 4.2.0 release, published on Dec 20th 2017.

Since the problem described in this bug report should be
resolved in oVirt 4.2.0 release, published on Dec 20th 2017, it has been closed with a resolution of CURRENT RELEASE.

If the solution does not work for you, please open a new bug report.

Note You need to log in before you can comment on or make changes to this bug.