Francesco Romani has posted comments on this change.
Change subject: v2v: Log detailed output of virt-v2v
......................................................................
Patch Set 6: Code-Review-1
(3 comments)
This version is an improvement, I'm not still completely happy about doing shell
tricks in Vdsm, but we head in a better direction now.
-1 for visibility of the comments (please also check the past versions)
https://gerrit.ovirt.org/#/c/59834/4//COMMIT_MSG
Commit Message:
Line 13: It would be cleaner to modify execCmd() to store the log output, but
Line 14: this can dangerous without the function being refactored first.
Line 15: Therefore we resort to a dirty hack with shell to redirect stderr to
Line 16: stdout and store everything in a log file. This hack should be cleaned
Line 17: once execCmd() is updated.
As I see it, there are two or three problems that sholdn't be
mixed togethe
1. agreed. Do we have a BZ about this? just for tracking purposes (we
could use a
Related-To: XXX
here)
2. please check inline comment about watchCmd - just to make sure we discuss this
3. IIUC we need to preserve the right ordering between stdout or stderr, right?
I mean, does it help to store all the stdout in one place, all the stderr in another
place, and then merge the two once virt-v2v is done?
Or doing so we lose some key information? (e.g. which stderr line refers to which stdout
line)
just asking for my understanding.
Last question, do you have one example of virt-v2v debug log handy? (
fpaste.org is fine)
Line 18:
Line 19: Change-Id: I6a8d9284316a551edeaffdd66dfcd299fa02478e
Line 20: Bug-Url:
https://bugzilla.redhat.com/1350465
https://gerrit.ovirt.org/#/c/59834/6/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:
PS6, Line 399: # XXX: This is a rather hacky way to redirect stderr to stdout. It
: # should be rewritten once execCmd has some clean way to do that.
ybronhei suggested to use watchCmd. Perhaps it could help. Did you tried it?
PS6, Line 855: ln = self._stream.__iter__().next()
why not
ln = next(self._stream)
?
--
To view, visit
https://gerrit.ovirt.org/59834
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a8d9284316a551edeaffdd66dfcd299fa02478e
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky <tgolembi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: Tomas Golembiovsky <tgolembi(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes