[libvirt] [PATCH 1/5] tests: qemuxml2argvmock: Don't mock virCommandPassFD

[Peter Krempa]

On Thu, Aug 16, 2018 at 12:08:12 +0200, Michal Privoznik wrote:
> On 08/14/2018 03:21 PM, Peter Krempa wrote:
> > This function does not modify the host. It merely puts the file
> > descriptor into a list in virCommandPtr.
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >  tests/qemuxml2argvmock.c | 8 --------
> >  1 file changed, 8 deletions(-)
> > 
> > diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
> > index 4df92cf396..c8a5f186d5 100644
> > --- a/tests/qemuxml2argvmock.c
> > +++ b/tests/qemuxml2argvmock.c
> > @@ -184,14 +184,6 @@ virNetDevRunEthernetScript(const char *ifname ATTRIBUTE_UNUSED,
> >      return 0;
> >  }
> > 
> > -void
> > -virCommandPassFD(virCommandPtr cmd ATTRIBUTE_UNUSED,
> > -                 int fd ATTRIBUTE_UNUSED,
> > -                 unsigned int flags ATTRIBUTE_UNUSED)
> > -{
> > -    /* nada */
> > -}
> > -
> >  int
> >  virNetDevOpenvswitchGetVhostuserIfname(const char *path ATTRIBUTE_UNUSED,
> >                                         char **ifname)
> > 
> 
> NACK, this indeed causes caller to close wrong FD:
> 
> 
> libvirt.git/tests $ ../run valgrind --trace-children=yes ./qemuxml2argvtest
> 
> ==210382== Warning: invalid file descriptor 1729 in syscall close()

I've posted a v2 of these yesterday in the morning.

Also in this case we know that fd 1729 is in fact invalid as we check
that is unused prior to this and the test would abort if it was used.

The new approach in v2 whitelists only the two FDs (which are still
invalid though) to be passed in virCommand since otherwise the command
line generator will not work.

Also note that when forking subprocesses we close a bunch of invalid FDs
anyways so it should not be a problem if we know they are not used
elsewhere.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180816/7c89bfa4/attachment-0001.sig>