On Thu, 21 Jul 2016 03:26:44 -0400 (EDT) Francesco Romani <[email protected]> wrote:
> ----- Original Message ----- > > From: "Nir Soffer" <[email protected]> > > To: "Yaniv Bronheim" <[email protected]> > > Cc: "Shahar Havivi" <[email protected]>, "Richard Jones" > > <[email protected]>, "devel" <[email protected]> > > Sent: Tuesday, July 19, 2016 4:18:31 PM > > Subject: Re: [ovirt-devel] execCmd() and storing stdout and stderr in log > > file > > > > On Tue, Jul 19, 2016 at 5:07 PM, Yaniv Bronheim <[email protected]> > > wrote: > > > > > > On Tue, Jul 19, 2016 at 4:56 PM, Tomáš Golembiovský <[email protected]> > > > wrote: > > >> > > >> On Thu, 14 Jul 2016 17:25:28 +0300 > > >> Nir Soffer <[email protected]> wrote: > > >> > > >> > After https://gerrit.ovirt.org/#/c/46733/ you should be able to create > > >> > the pipeline in python like this: > > >> > > > >> > v2v = Popen(["virt-v2v", ...], stdout=PIPE, stderr=STDOUT) > > >> > tee = Popen(["tee", "-a", logfile], stdin=v2v.stdout, stdout=PIPE, > > >> > stderr=PIPE) > > >> > > > >> > Now we can read output from tee.stdout, and when tee is finished, we > > >> > can > > >> > wait > > >> > for v2v to get the exit code. > > >> > > > >> > Since all output would go to tee stdout and stderr may only contain tee > > >> > usage > > >> > errors, we don't need to use AsyncProc, making this code python 3 > > >> > compatible. > > >> > > >> > > >> Yes, this may actualy work. And do we plan to adopt the cpopen 1.4.1, > > >> where > > >> this is fixed, in VDSM? > > > > > > > > > cpopen 1.5.1 - and yes but not in ovirt-4.0 > > > > It is included in 1.4.1, released ages ago. > > > > $ git log --oneline --decorate > > 826b5ef (HEAD -> master, origin/master, origin/HEAD) Raise version for 1.5 > > build > > a9d2a72 (inherit-fds) Add tests for disabling the close-on-exec flag > > 7163e79 Do not close inherited fds from parent > > 1b170fb (gerrit/master) Raising release number and tagging cpopen 1.4.1 > > 69ae841 Sort imports to make it easier to add new imports > > 1116830 Simplify tests using CPopen.communicate() > > 58e1b43 Remove unneeded and wrong retry after fork > > fbf9ff3 Remove wrong and unneeded retry for execv > > 74ee4a0 Do not retry close() after interrupt > > 1abde2b Use _exit to terminate child after failure > > 8709c42 Fix incorrect fd closing > > > > $ git show 1b170fb --format=fuller > > commit 1b170fb8f2d204457ca66e53936fd3a059f4ed4b > > Author: Yaniv Bronhaim <[email protected]> > > AuthorDate: Wed Oct 14 15:40:07 2015 +0300 > > Commit: Yaniv Bronhaim <[email protected]> > > CommitDate: Wed Oct 14 09:36:31 2015 -0400 > > > > Raising release number and tagging cpopen 1.4.1 > > > > Recent patches include only bug fixes. see commit messages > > > > Change-Id: Ia1d32dd5bd7f60681b64251cef217c8d1e41b14b > > Signed-off-by: Yaniv Bronhaim <[email protected]> > > Thanks to everyone for comments and insights. > > I think is time to wrap up so Tomas can finish his work. > > So, the options on the table are: > 1. Build pipelines in python (Nir's approach above) > - looks like the the fix is in cpopen 1.4.1, available X. One more option that occured to me is: use Nir's approach but use subprocess.Popen directly. - since Yaniv said updated cpopen won't be available for 4.0 - and since we're aiming for subprocess.Popen in a log run anyway (if my understanding is correct) - and since the package for python-subprocess32 is available in our 4.0 dependencies repo already > 2. add options to execCmd to redirect stderr > - make execCmd even bigger, makes everyone grumpy > - could require a couple of simple fixes (talked with > Tomas privately, nothing groundbreaking - like one between stdout > and stderr could be None and we must cope with that) > 3. DUP execCmd just for v2v and just to avoid making it more complex > - see https://gerrit.ovirt.org/#/c/60660/ > - even worse than extending execCmd? > - still needs the fixes above > 4. use shell tricks and reuse existing execCmd > - reviewers/future selves needs to wrap their head on that shell magic > - PERHAPS fragile? I don't really know > 5. something else I forgot? > > > I think the approaches here are ordered by likeness and feasibility, > so I'd actually try in this order (1->2->3->4) > > Please ACK/NACK the above, so Tomas can conclude this patch > for next monday (20160725) > > -- > Francesco Romani > RedHat Engineering Virtualization R & D > Phone: 8261328 > IRC: fromani -- Tomáš Golembiovský <[email protected]> _______________________________________________ Devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/devel
