Re: [PATCH for-5.2 05/10] vhost-user-blk-test: close fork child file descriptors

[Coiby Xu]

On Thu, Dec 03, 2020 at 01:57:21PM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 24, 2020 at 08:08:26PM +0800, Coiby Xu wrote:
> > Hi Stefan,
> >
> > On Wed, Nov 11, 2020 at 12:43:26PM +0000, Stefan Hajnoczi wrote:
> > > Do not leave stdin and stdout open after fork. stdout is the
> > > tap-driver.pl pipe. If we keep the pipe open then tap-driver.pl will not
> > > detect that qos-test has terminated and it will hang.
> >
> > I wonder under which situation this would happen. I couldn't re-produce
> > this issue locally.
>
> Try adding g_assert_not_reached() to one of the tests so the qos-test
> process aborts. Then tap-driver.pl will hang because the pipe hasn't
> been closed and "make check" never completes.

Thank you for the explanation! I thought closing fork child file is
for dealing with a subtle race condition. So it's actually for dealing
with the situation when g_assert_* fails.

Do you think g_spawn_async_with_pipes is a better way than fork/exec
since g_spawn_* could help us take care of closing standard file
descriptors?

    g_test_message("starting vhost-user backend: %s",
                   storage_daemon_command->str);

    GPid child_pid;
    g_autoptr(GError) error = NULL;
    const gchar * const argv[] = { "/bin/sh", "-c", 
storage_daemon_command->str, NULL };
    g_spawn_async_with_pipes (NULL, argv, NULL, G_SPAWN_STDOUT_TO_DEV_NULL, 
NULL,
            NULL, &child_pid, NULL, NULL,
            NULL, &error);

    if (error != NULL)
    {
        g_error ("Starting qemu-storage-daemon failed: %s", error->message);
        abort();
    }
    g_string_free(storage_daemon_command, true);



--
Best regards,
Coiby