Re: [Qemu-devel] [PATCH] tests: fix tpm-crb tpm-tis tests race

[Stefan Berger]

On 03/16/2018 10:30 AM, Daniel P. Berrangé wrote:
> On Fri, Mar 16, 2018 at 03:29:04PM +0100, Marc-André Lureau wrote:
> > Hi
> >
> > On Fri, Mar 16, 2018 at 3:25 PM, Daniel P. Berrangé <address@hidden> wrote:
> > > On Fri, Mar 16, 2018 at 03:22:23PM +0100, Marc-André Lureau wrote:
> > > > No need to close the TPM data socket on the emulator end, qemu will
> > > > close it after a SHUTDOWN. This avoids a race between close() and
> > > > read() in the TPM data thread.
> > > >
> > > > Reported-by: Peter Maydell <address@hidden>
> > > > Signed-off-by: Marc-André Lureau <address@hidden>
> > > > ---
> > > >   tests/tpm-emu.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/tests/tpm-emu.c b/tests/tpm-emu.c
> > > > index 4dada76834..8c2bd53cad 100644
> > > > --- a/tests/tpm-emu.c
> > > > +++ b/tests/tpm-emu.c
> > > > @@ -125,7 +125,7 @@ void *tpm_emu_ctrl_thread(void *data)
> > > >           case CMD_SHUTDOWN: {
> > > >               ptm_res res = 0;
> > > >               qio_channel_write(ioc, (char *)&res, sizeof(res), &error_abort);
> > > > -            qio_channel_close(s->tpm_ioc, &error_abort);
> > > > +            /* the tpm data thread is expected to finish now */
> > > >               g_thread_join(s->emu_tpm_thread);
> > > Won't this leave an orphaed FD open in the test suite ? Is it perhaps
> > > sufficient to just swap the order of the g_thread_join and qio_channel_close
> > > calls, so that we join the thread before we close the channel that the
> > > thread is using ?
> > Isn't the socket close on the last unref? (at end of tpm_emu_tpm_thread())
> Oh yes, that is true. I should have looked at more than just diff context.
> In that case
>
>    Reviewed-by: Daniel P. Berrangé <address@hidden>

Ha. It's already done :-)

Reviewed-by: Stefan Berger <address@hidden>


> Regards,
> Daniel