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

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

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 5608 bytes --]

The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:

  Merge remote-tracking branch
'remotes/armbru/tags/pull-qapi-2015-10-12' into staging (2015-10-13
10:42:06 +0100)

are available in the git repository at:

  https://github.com/elmarco/qemu tags/ivshmem-pull-request

for you to fetch changes up to e229096d2e43e823e30dde1b6cac6d863abd30f9:

  doc: document ivshmem & hugepages (2015-10-14 12:43:39 +0200)

----------------------------------------------------------------
v4:
- fix server for posix compatibility
- fix macos test failure with double ftruncate()
- replace custom pow2 code with pow2ceil()
- make server silent again (--verbose)
- make server verbose earlier
- use g_test_verbose() instead of QTEST_LOG
- fix x86 build warnings
- make server test more flexible to timeout with 5s max
- the diff with v3 is attached

----------------------------------------------------------------
Andreas Färber (1):
      tests: Add ivshmem qtest

David Marchand (3):
      contrib: add ivshmem client and server
      docs: update ivshmem device spec
      ivshmem: add check on protocol version in QEMU

Marc-André Lureau (48):
      config: enable ivshmem on POSIX
      char: add qemu_chr_free()
      msix: add VMSTATE_MSIX_TEST
      ivhsmem: read do not accept more than sizeof(long)
      ivshmem: fix number of bytes to push to fifo
      ivshmem: factor out the incoming fifo handling
      ivshmem: remove unnecessary dup()
      ivshmem: remove superflous ivshmem_attr field
      ivshmem: remove useless doorbell field
      ivshmem: more qdev conversion
      ivshmem: remove last exit(1)
      ivshmem: limit maximum number of peers to G_MAXUINT16
      ivshmem: simplify around increase_dynamic_storage()
      ivshmem: allocate eventfds in resize_peers()
      ivshmem: remove useless ivshmem_update_irq() val argument
      ivshmem: initialize max_peer to -1
      ivshmem: remove max_peer field
      ivshmem: improve debug messages
      ivshmem: improve error handling
      ivshmem: print error on invalid peer id
      ivshmem: simplify a bit the code
      ivshmem: use common return
      ivshmem: use common is_power_of_2()
      ivshmem: migrate with VMStateDescription
      ivshmem: shmfd can be 0
      ivshmem: check shm isn't already initialized
      ivshmem: add device description
      ivshmem: fix pci_ivshmem_exit()
      ivshmem: replace 'guest' for 'peer' appropriately
      ivshmem: error on too many eventfd received
      ivshmem: reset mask on device reset
      util: const event_notifier_get_fd() argument
      ivshmem-client: check the number of vectors
      ivshmem-server: use a uint16 for client ID
      ivshmem-server: fix hugetlbfs support
      contrib: remove unnecessary strdup()
      msix: implement pba write (but read-only)
      qtest: add qtest_add_abrt_handler()
      glib-compat: add 2.38/2.40/2.46 asserts
      tests: add ivshmem qtest
      ivshmem: do not keep shm_fd open
      ivshmem: use qemu_strtosz()
      ivshmem: add hostmem backend
      ivshmem: remove EventfdEntry.vector
      ivshmem: rename MSI eventfd_table
      ivshmem: use kvm irqfd for msi notifications
      ivshmem: use little-endian int64_t for the protocol
      doc: document ivshmem & hugepages

 Makefile                                |   8 +
 Makefile.objs                           |   5 +
 configure                               |   1 +
 contrib/ivshmem-client/Makefile.objs    |   1 +
 contrib/ivshmem-client/ivshmem-client.c | 446
++++++++++++++++++++++++++++++++++
 contrib/ivshmem-client/ivshmem-client.h | 213 +++++++++++++++++
 contrib/ivshmem-client/main.c           | 240 +++++++++++++++++++
 contrib/ivshmem-server/Makefile.objs    |   1 +
 contrib/ivshmem-server/ivshmem-server.c | 491
+++++++++++++++++++++++++++++++++++++
 contrib/ivshmem-server/ivshmem-server.h | 167 +++++++++++++
 contrib/ivshmem-server/main.c           | 263 ++++++++++++++++++++
 default-configs/pci.mak                 |   2 +-
 docs/specs/ivshmem_device_spec.txt      | 127 +++++++---
 hw/misc/ivshmem.c                       | 837
++++++++++++++++++++++++++++++++++++++++++++--------------------
 hw/pci/msix.c                           |   6 +
 include/glib-compat.h                   |  61 +++++
 include/hw/misc/ivshmem.h               |  25 ++
 include/hw/pci/msix.h                   |  16 +-
 include/qemu/event_notifier.h           |   2 +-
 include/sysemu/char.h                   |  10 +-
 qemu-char.c                             |   9 +-
 qemu-doc.texi                           |  23 +-
 tests/Makefile                          |   3 +
 tests/ivshmem-test.c                    | 483
+++++++++++++++++++++++++++++++++++++
 tests/libqtest.c                        |  37 ++-
 tests/libqtest.h                        |   2 +
 util/event_notifier-posix.c             |   2 +-
 27 files changed, 3160 insertions(+), 321 deletions(-)
 create mode 100644 contrib/ivshmem-client/Makefile.objs
 create mode 100644 contrib/ivshmem-client/ivshmem-client.c
 create mode 100644 contrib/ivshmem-client/ivshmem-client.h
 create mode 100644 contrib/ivshmem-client/main.c
 create mode 100644 contrib/ivshmem-server/Makefile.objs
 create mode 100644 contrib/ivshmem-server/ivshmem-server.c
 create mode 100644 contrib/ivshmem-server/ivshmem-server.h
 create mode 100644 contrib/ivshmem-server/main.c
 create mode 100644 include/hw/misc/ivshmem.h
 create mode 100644 tests/ivshmem-test.c


-- 
Marc-André Lureau

[-- Attachment #2: ivshmem.diff --]
[-- Type: text/plain, Size: 8699 bytes --]

diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
index f143d41..5e5239c 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -191,7 +191,7 @@ ivshmem_server_handle_new_conn(IvshmemServer *server)
     QTAILQ_FOREACH(other_peer, &server->peer_list, next) {
         for (i = 0; i < peer->vectors_count; i++) {
             ivshmem_server_send_one_msg(other_peer->sock_fd, peer->id,
-                event_notifier_get_fd(&peer->vectors[i]));
+                                        peer->vectors[i].wfd);
         }
     }
 
@@ -199,7 +199,7 @@ ivshmem_server_handle_new_conn(IvshmemServer *server)
     QTAILQ_FOREACH(other_peer, &server->peer_list, next) {
         for (i = 0; i < peer->vectors_count; i++) {
             ivshmem_server_send_one_msg(peer->sock_fd, other_peer->id,
-                event_notifier_get_fd(&other_peer->vectors[i]));
+                                        other_peer->vectors[i].wfd);
         }
     }
 
@@ -232,15 +232,14 @@ static int
 ivshmem_server_ftruncate(int fd, unsigned shmsize)
 {
     int ret;
+    struct stat mapstat;
 
     /* align shmsize to next power of 2 */
-    shmsize--;
-    shmsize |= shmsize >> 1;
-    shmsize |= shmsize >> 2;
-    shmsize |= shmsize >> 4;
-    shmsize |= shmsize >> 8;
-    shmsize |= shmsize >> 16;
-    shmsize++;
+    shmsize = pow2ceil(shmsize);
+
+    if (fstat(fd, &mapstat) != -1 && mapstat.st_size == shmsize) {
+        return 0;
+    }
 
     while (shmsize <= IVSHMEM_SERVER_MAX_HUGEPAGE_SIZE) {
         ret = ftruncate(fd, shmsize);
@@ -262,6 +261,7 @@ ivshmem_server_init(IvshmemServer *server, const char *unix_sock_path,
     int ret;
 
     memset(server, 0, sizeof(*server));
+    server->verbose = verbose;
 
     ret = snprintf(server->unix_sock_path, sizeof(server->unix_sock_path),
                    "%s", unix_sock_path);
@@ -278,7 +278,6 @@ ivshmem_server_init(IvshmemServer *server, const char *unix_sock_path,
 
     server->shm_size = shm_size;
     server->n_vectors = n_vectors;
-    server->verbose = verbose;
 
     QTAILQ_INIT(&server->peer_list);
 
@@ -299,10 +298,6 @@ static long gethugepagesize(const char *path)
     } while (ret != 0 && errno == EINTR);
 
     if (ret != 0) {
-        if (errno != ENOENT) {
-            fprintf(stderr, "cannot stat shm file %s: %s\n", path,
-                    strerror(errno));
-        }
         return -1;
     }
 
@@ -326,16 +321,22 @@ ivshmem_server_start(IvshmemServer *server)
     long hpagesize;
 
     hpagesize = gethugepagesize(server->shm_path);
+    if (hpagesize < 0 && errno != ENOENT) {
+        IVSHMEM_SERVER_DEBUG(server, "cannot stat shm file %s: %s\n",
+                             server->shm_path, strerror(errno));
+    }
+
     if (hpagesize > 0) {
         gchar *filename = g_strdup_printf("%s/ivshmem.XXXXXX", server->shm_path);
-        fprintf(stdout, "Using hugepages: %s\n", server->shm_path);
+        IVSHMEM_SERVER_DEBUG(server, "Using hugepages: %s\n", server->shm_path);
         shm_fd = mkstemp(filename);
         unlink(filename);
         g_free(filename);
     } else
 #endif
     {
-        fprintf(stdout, "Using POSIX shared memory: %s\n", server->shm_path);
+        IVSHMEM_SERVER_DEBUG(server, "Using POSIX shared memory: %s\n",
+                             server->shm_path);
         shm_fd = shm_open(server->shm_path, O_CREAT|O_RDWR, S_IRWXU);
     }
 
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index 7e10903..f250119 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -35,5 +35,5 @@ CONFIG_SDHCI=y
 CONFIG_EDU=y
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
-CONFIG_IVSHMEM=$(CONFIG_KVM)
+CONFIG_IVSHMEM=$(CONFIG_POSIX)
 CONFIG_ROCKER=y
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 227a4db..b18f422 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -623,13 +623,14 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
     }
 
     if (incoming_posn < -1) {
-        IVSHMEM_DPRINTF("invalid incoming_posn %ld\n", incoming_posn);
+        IVSHMEM_DPRINTF("invalid incoming_posn %" PRId64 "\n", incoming_posn);
         return;
     }
 
     /* pick off s->server_chr->msgfd and store it, posn should accompany msg */
     incoming_fd = qemu_chr_fe_get_msgfd(s->server_chr);
-    IVSHMEM_DPRINTF("posn is %ld, fd is %d\n", incoming_posn, incoming_fd);
+    IVSHMEM_DPRINTF("posn is %" PRId64 ", fd is %d\n",
+                    incoming_posn, incoming_fd);
 
     /* make sure we have enough space for this peer */
     if (incoming_posn >= s->nb_peers) {
@@ -651,7 +652,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
             s->vm_id = incoming_posn;
         } else {
             /* otherwise an fd == -1 means an existing peer has gone away */
-            IVSHMEM_DPRINTF("posn %ld has gone away\n", incoming_posn);
+            IVSHMEM_DPRINTF("posn %" PRId64 " has gone away\n", incoming_posn);
             close_peer_eventfds(s, incoming_posn);
         }
         return;
@@ -697,7 +698,6 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
     /* each peer has an associated array of eventfds, and we keep
      * track of how many eventfds received so far */
     /* get a new eventfd: */
-    /* get a new eventfd */
     if (peer->nb_eventfds >= s->vectors) {
         error_report("Too many eventfd received, device has %d vectors",
                      s->vectors);
@@ -708,7 +708,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
     new_eventfd = peer->nb_eventfds++;
 
     /* this is an eventfd for a particular peer VM */
-    IVSHMEM_DPRINTF("eventfds[%ld][%d] = %d\n", incoming_posn,
+    IVSHMEM_DPRINTF("eventfds[%" PRId64 "][%d] = %d\n", incoming_posn,
                     new_eventfd, incoming_fd);
     event_notifier_init_fd(&peer->eventfds[new_eventfd], incoming_fd);
     fcntl_setfl(incoming_fd, O_NONBLOCK); /* msix/irqfd poll non block */
diff --git a/tests/Makefile b/tests/Makefile
index e7b0218..73403eb 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -149,7 +149,7 @@ gcov-files-pci-y += hw/display/virtio-gpu-pci.c
 gcov-files-pci-$(CONFIG_VIRTIO_VGA) += hw/display/virtio-vga.c
 check-qtest-pci-y += tests/intel-hda-test$(EXESUF)
 gcov-files-pci-y += hw/audio/intel-hda.c hw/audio/hda-codec.c
-check-qtest-pci-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
+check-qtest-pci-$(CONFIG_POSIX) += tests/ivshmem-test$(EXESUF)
 gcov-files-pci-y += hw/misc/ivshmem.c
 
 check-qtest-i386-y = tests/endianness-test$(EXESUF)
diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
index a57fb08..efaa6e3 100644
--- a/tests/ivshmem-test.c
+++ b/tests/ivshmem-test.c
@@ -288,11 +288,12 @@ static void test_ivshmem_server(void)
     IvshmemServer server;
     int ret, vm1, vm2;
     int nvectors = 2;
+    guint64 end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
 
     memset(tmpshmem, 0x42, TMPSHMSIZE);
     ret = ivshmem_server_init(&server, tmpserver, tmpshm,
                               TMPSHMSIZE, nvectors,
-                              getenv("QTEST_LOG") != NULL);
+                              g_test_verbose());
     g_assert_cmpint(ret, ==, 0);
 
     ret = ivshmem_server_start(&server);
@@ -315,7 +316,7 @@ static void test_ivshmem_server(void)
     g_assert(thread.thread != NULL);
 
     /* waiting until mapping is done */
-    while (true) {
+    while (g_get_monotonic_time() < end_time) {
         g_usleep(1000);
 
         if (qtest_readb(s1->qtest, (uintptr_t)s1->mem_base) == 0x42 &&
@@ -337,8 +338,10 @@ static void test_ivshmem_server(void)
     ret = qpci_msix_pending(s1->dev, 0);
     g_assert_cmpuint(ret, ==, 0);
     out_reg(s2, DOORBELL, vm1 << 16);
-    g_usleep(10000);
-    ret = qpci_msix_pending(s1->dev, 0);
+    do {
+        g_usleep(10000);
+        ret = qpci_msix_pending(s1->dev, 0);
+    } while (ret == 0 && g_get_monotonic_time() < end_time);
     g_assert_cmpuint(ret, !=, 0);
 
     /* ping vm1 -> vm2 */
@@ -346,15 +349,13 @@ static void test_ivshmem_server(void)
     ret = qpci_msix_pending(s2->dev, 0);
     g_assert_cmpuint(ret, ==, 0);
     out_reg(s1, DOORBELL, vm2 << 16);
-    g_usleep(10000);
-    ret = qpci_msix_pending(s2->dev, 0);
+    do {
+        g_usleep(10000);
+        ret = qpci_msix_pending(s2->dev, 0);
+    } while (ret == 0 && g_get_monotonic_time() < end_time);
     g_assert_cmpuint(ret, !=, 0);
 
-    /* remove vm2 */
     qtest_quit(s2->qtest);
-    /* XXX wait enough time for vm1 to be notified */
-    g_usleep(1000);
-
     qtest_quit(s1->qtest);
 
     if (qemu_write_full(thread.pipe[1], "q", 1) != 1) {

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

[Peter Maydell]

On 14 October 2015 at 15:49, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:
>
>   Merge remote-tracking branch
> 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging (2015-10-13
> 10:42:06 +0100)
>
> are available in the git repository at:
>
>   https://github.com/elmarco/qemu tags/ivshmem-pull-request
>
> for you to fetch changes up to e229096d2e43e823e30dde1b6cac6d863abd30f9:
>
>   doc: document ivshmem & hugepages (2015-10-14 12:43:39 +0200)
>
> ----------------------------------------------------------------
> v4:
> - fix server for posix compatibility
> - fix macos test failure with double ftruncate()
> - replace custom pow2 code with pow2ceil()
> - make server silent again (--verbose)
> - make server verbose earlier
> - use g_test_verbose() instead of QTEST_LOG
> - fix x86 build warnings
> - make server test more flexible to timeout with 5s max
> - the diff with v3 is attached
>
> ----------------------------------------------------------------

On 32-bit ARM the test-ivshmem program got stuck somehow (it had been running
for an hour when I killed it). It didn't write anything to the log,
I'm afraid.

thanks
-- PMM

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

[Peter Maydell]

On 16 October 2015 at 12:26, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 14 October 2015 at 15:49, Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
>> The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:
>>
>>   Merge remote-tracking branch
>> 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging (2015-10-13
>> 10:42:06 +0100)
>>
>> are available in the git repository at:
>>
>>   https://github.com/elmarco/qemu tags/ivshmem-pull-request
>>
>> for you to fetch changes up to e229096d2e43e823e30dde1b6cac6d863abd30f9:
>>
>>   doc: document ivshmem & hugepages (2015-10-14 12:43:39 +0200)
>>
>> ----------------------------------------------------------------
>> v4:
>> - fix server for posix compatibility
>> - fix macos test failure with double ftruncate()
>> - replace custom pow2 code with pow2ceil()
>> - make server silent again (--verbose)
>> - make server verbose earlier
>> - use g_test_verbose() instead of QTEST_LOG
>> - fix x86 build warnings
>> - make server test more flexible to timeout with 5s max
>> - the diff with v3 is attached
>>
>> ----------------------------------------------------------------
>
> On 32-bit ARM the test-ivshmem program got stuck somehow (it had been running
> for an hour when I killed it). It didn't write anything to the log,
> I'm afraid.

This may be something funny with this build machine; I will put
this back on my list to try again at some point.

-- PMM

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

[Peter Maydell]

On 16 October 2015 at 15:47, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 16 October 2015 at 12:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 14 October 2015 at 15:49, Marc-André Lureau
>> <marcandre.lureau@gmail.com> wrote:
>>> The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:
>>>
>>>   Merge remote-tracking branch
>>> 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging (2015-10-13
>>> 10:42:06 +0100)
>>>
>>> are available in the git repository at:
>>>
>>>   https://github.com/elmarco/qemu tags/ivshmem-pull-request
>>>
>>> for you to fetch changes up to e229096d2e43e823e30dde1b6cac6d863abd30f9:
>>>
>>>   doc: document ivshmem & hugepages (2015-10-14 12:43:39 +0200)
>>>
>>> ----------------------------------------------------------------
>>> v4:
>>> - fix server for posix compatibility
>>> - fix macos test failure with double ftruncate()
>>> - replace custom pow2 code with pow2ceil()
>>> - make server silent again (--verbose)
>>> - make server verbose earlier
>>> - use g_test_verbose() instead of QTEST_LOG
>>> - fix x86 build warnings
>>> - make server test more flexible to timeout with 5s max
>>> - the diff with v3 is attached
>>>
>>> ----------------------------------------------------------------
>>
>> On 32-bit ARM the test-ivshmem program got stuck somehow (it had been running
>> for an hour when I killed it). It didn't write anything to the log,
>> I'm afraid.
>
> This may be something funny with this build machine; I will put
> this back on my list to try again at some point.

Nope, retry gave the same thing -- test-ivshmem running for hours,
using 100% CPU. Sorry, no backtrace, I don't have gdb on that box
right now (I will ask for it to be installed).

thanks
-- PMM

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

[Peter Maydell]

On 14 October 2015 at 15:49, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:
>
>   Merge remote-tracking branch
> 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging (2015-10-13
> 10:42:06 +0100)
>
> are available in the git repository at:
>
>   https://github.com/elmarco/qemu tags/ivshmem-pull-request
>
> for you to fetch changes up to e229096d2e43e823e30dde1b6cac6d863abd30f9:
>
>   doc: document ivshmem & hugepages (2015-10-14 12:43:39 +0200)

NB that you'll need to rebase this before resubmitting, as
kvm_irqchip_update_msi_route now has an extra argument.

I'm going to have a look at the test case hangs I've been seeing.

thanks
-- PMM

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

[Peter Maydell]

On 16 October 2015 at 12:26, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 32-bit ARM the test-ivshmem program got stuck somehow (it had been running
> for an hour when I killed it). It didn't write anything to the log,
> I'm afraid.

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...

thanks
-- PMM

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

[Marc-André Lureau]

Hi Peter,

On Mon, Oct 19, 2015 at 5:57 PM, Peter Maydell <peter.maydell@linaro.org> 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 */

I rebased and updated the tag.


-- 
Marc-André Lureau

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

[Peter Maydell]

On 26 October 2015 at 09:28, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> Hi Peter,
>
> On Mon, Oct 19, 2015 at 5:57 PM, Peter Maydell <peter.maydell@linaro.org> 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

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

[Marc-André Lureau]

Hi

On Mon, Oct 26, 2015 at 10:58 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> 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.

g_test_skip() is since 2.38 (and can't be added in compat, because it
uses internal variable etc)

Furthermore, the shm error is a precondition for all the tests, it
doesn't fit well with g_test_skip() which is inside the individual
unit tests.

> [*] 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...

ok




-- 
Marc-André Lureau