[libvirt] [PATCH 2/3]: Log argv passed to virExec and virRun

[Daniel P. Berrange]

On Mon, Nov 03, 2008 at 11:21:26AM -0500, Cole Robinson wrote:
> Daniel P. Berrange wrote:
> > On Thu, Oct 30, 2008 at 02:06:20PM -0400, Cole Robinson wrote:
> >   
> >> The attached patch logs the the argv's passed to
> >> the virExec and virRun functions. There's a bit of
> >> trickery here: since virRun is just a wrapper for
> >> virExec, we don't want the argv string to be logged
> >> twice. 
> >>
> >> I addressed this by renaming virExec to __virExec,
> >> and keeping the original function name to simply
> >> debug the argv and then hand off control. This
> >> means anytime virExec is explictly called, the
> >> argv will be logged, but if functions wish to by
> >> pass that they can just call __virExec (which is
> >> what virRun does.)
> >>     
> >
> > I'm a little confused about why we can't just put the logging
> > calling directly in the existing virExec() function. Since the
> > first thing virRun() does is to call virExec() this would 
> > seem to be sufficient without need of a wrapper.
> >
> > Daniel
> >   
> 
> Two small benefits of the way this patch does it:
> 
> - We can tell by the debug output whether the argv
>   is coming from virRun or from virExec called
>   explicitly.
> 
> - We want the argv string available in virRun for
>   error reporting. The patch allows us to avoid
>   converting the argv to string twice.
> 
> I can rework the patch if you'd like, the above
> points aren't deal breakers.

Nah, I understand the purpose of your original approach now - ACK to the
patch.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|