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@xxxxxxxxxx>
> > ---
> > 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.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
