On Thu, 21 Jul 2016 08:24:57 -0400 (EDT) Francesco Romani <[email protected]> wrote:
> ----- Original Message ----- > > From: "Nir Soffer" <[email protected]> > > To: "Tomáš Golembiovský" <[email protected]> > > Cc: "Francesco Romani" <[email protected]>, "devel" <[email protected]>, > > "Yaniv Bronheim" <[email protected]>, > > "Shahar Havivi" <[email protected]> > > Sent: Thursday, July 21, 2016 2:17:19 PM > > Subject: Re: [ovirt-devel] execCmd() and storing stdout and stderr in log > > file > > > > On Thu, Jul 21, 2016 at 1:51 PM, Tomáš Golembiovský <[email protected]> > > wrote: > > > On Thu, 21 Jul 2016 03:26:44 -0400 (EDT) > > > Francesco Romani <[email protected]> wrote: > [...] > > >> 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. > > > > Until we integrate with subprocess32, you should use compat.CPopen. > > > > When subprocess32 is ready, the correct way would be compat.Popen, > > hiding the subprocess32 import on Python 2.7. > > > > > - since Yaniv said updated cpopen won't be available for 4.0 > > > > We don't need the update for 4.0, cpopen 1.4.1, available in 4.0 should be > > good enough. I still see only 1.4 in repos. But if there will be 1.4.1 for 4.0 it is good enough. > > > > > - and since we're aiming for subprocess.Popen in a log run anyway (if > > > my > > > understanding is correct) > > > > Correct, but we did not integrated with it yet > > > > > - and since the package for python-subprocess32 is available in our > > > 4.0 > > > dependencies repo already > > > > Really? > > > > I think subprocess32 is not available yet on rhel, so we cannot use it yet. > > > > Tomas, could you please check about the availability in RHEL? > So, turns out we have only one option (and half? :)) which is good, it > simplifies > the things. It turned out the python-subprocess32 is available from our centos-ovirt40-candidate[1] repo. Not in RHEL as such. > > > >> 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) > > > > This is not an option, execCmd is too big and ugly today, we are not going > > to add more features to it. > > > > This helper is the way to use for the common case of running a process, > > collecting the data from both stdout and stderr, and returning the results. > > > > Code that have special needs should not use execCmd. I didn't know that. > > > > See qemuimg.QemuImgOperation, and storage.check.DirectioChecker for > > examles. > > > > You should use the helpers in cmdutils for consistent logging when starting > > and finishing a process. > > Fine, and thanks for the examples. > > > > > >> 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 > > > > No, one is enough > > Indeed it is, going to abandon 60660 and bury it very deep as soon as we > reach consensus. > > > > > >> 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 > > > > I would not use the shell if I don't have to. > > ok, let's keep this as very last resort. > > > There are other issues - where do you save the import log > > (per-import log file?, v2v.log?) and how the logs are deleted. > > Indeed they are, but I'm less worried about those since are been already > solved > in the past and we already have tools to deal with them Logs will be stored in /var/log/vdsm/imports. > > Maybe keep per-import log files, in /var/log/vdsm/v2v, and use logrotate > > configuration to rotate them? > > I think we though about this (me and Tomas) in the past, and that was the > plan; > Anyway I agree with this, let's make this our plan if it wasn't already :) I didn't bother with setup for logrotate yet. The import process is not something happening automatically or regularly. It's unlikely the user will end up with full /var partition because of that. If you believe it's necessary it can be done. [1] http://cbs.centos.org/repos/virt7-ovirt-40-candidate/x86_64/os/ -- Tomáš Golembiovský <[email protected]> _______________________________________________ Devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/devel
