Re: [Qemu-devel] [PULL v4 00/52] Ivshmem patches

[Peter Maydell]

On 26 October 2015 at 09:28, Marc-André Lureau
<address@hidden> wrote:
> Hi Peter,
>
> On Mon, Oct 19, 2015 at 5:57 PM, Peter Maydell <address@hidden> wrote:
>>
>> What is happening here is that we are looping infinitely in
>> mktempshmem() because shm_open() returns -1 with errno ENOSYS,
>> and there's no code in the loop that stops the loop on anything
>> except shm_open succeeding, or even prints anything out about
>> shm_open failing.
>>
>> I think this is failing for me because my system's chroot doesn't have
>> /dev/shm mounted. It would be nice if we could at a minimum handle
>> this reasonably gracefully...
>
> The following diff works for me:
>
> diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
> index efaa6e3..c8f0cf0 100644
> --- a/tests/ivshmem-test.c
> +++ b/tests/ivshmem-test.c
> @@ -441,13 +441,18 @@ static gchar *mktempshm(int size, int *fd)
>          }
>
>          g_free(name);
> +
> +        if (errno != EEXIST) {
> +            perror("shm_open");
> +            return NULL;
> +        }
>      }
>  }
>
>  int main(int argc, char **argv)
>  {
>      int ret, fd;
>      gchar dir[] = "/tmp/ivshmem-test.XXXXXX";
>
>  #if !GLIB_CHECK_VERSION(2, 31, 0)
>      if (!g_thread_supported()) {
> @@ -460,6 +465,9 @@ int main(int argc, char **argv)
>      qtest_add_abrt_handler(abrt_handler, NULL);
>      /* shm */
>      tmpshm = mktempshm(TMPSHMSIZE, &fd);
> +    if (!tmpshm) {
> +        return 0;
> +    }
>      tmpshmem = mmap(0, TMPSHMSIZE, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
>      g_assert(tmpshmem != MAP_FAILED);
>      /* server */

This will print a cryptic error message and then not fail the test,
which is not great. Maybe that's ok for the moment in the interests
of not keeping this huge patchset out of tree for too long[*], but
we should look at what glib's test framework provides in the way
of being able to report "skipped this test" outcomes.

[*] Incidentally this whole saga demonstrates why my general
recommendation is to keep pull requests at much less than
50 patches...

> I rebased and updated the tag.

If you mean by this "please retry the pull" you should send a fresh
coverletter email so my scripts will pick it up...

thanks
-- PMM