Re: [PATCH 3/3] meson: Disable CONFIG_NOTIFY1 on FreeBSD

[Daniel P . Berrangé]

On Wed, Jan 31, 2024 at 05:24:10PM +0100, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> Warner, do you remember what this is about?
> 
> (https://cgit.freebsd.org/ports/commit/emulators/qemu-devel/files/patch-util_meson.build?id=2ab482e2c8f51eae7ffd747685b7f181fe1b3809
> isn't very verbose).

That's simply going to workaround our incomplete feature
check. We look for sys/inotify.h and if present, we
assume that is in the C library. That's true on Linux,
but not true on *BSD, hence the undefined symbol.

We need to augment the header file check with a linker
symbol check for the C library.

If we wanted to also check for -linotify that'd make
it portable to BSD, but not the behaviour difference
mentioned below.

> 
> On 25/1/24 20:48, Ilya Leoshkevich wrote:
> > make vm-build-freebsd fails with:
> > 
> >      ld: error: undefined symbol: inotify_init1
> >      >>> referenced by filemonitor-inotify.c:183 
> > (../src/util/filemonitor-inotify.c:183)
> >      >>>               util_filemonitor-inotify.c.o:(qemu_file_monitor_new) 
> > in archive libqemuutil.a
> > 
> > On FreeBSD inotify functions are defined in libinotify.so, so it might
> > be tempting to add it to the dependencies. Doing so, however, reveals
> > that this library handles rename events differently from Linux:
> > 
> >      $ FILEMONITOR_DEBUG=1 build/tests/unit/test-util-filemonitor
> >      Rename /tmp/test-util-filemonitor-K13LI2/fish/one.txt -> 
> > /tmp/test-util-filemonitor-K13LI2/two.txt
> >      Event id=200000000 event=2 file=one.txt
> >      Queue event id 200000000 event 2 file one.txt
> >      Queue event id 100000000 event 2 file two.txt
> >      Queue event id 100000002 event 2 file two.txt
> >      Queue event id 100000000 event 0 file two.txt
> >      Queue event id 100000002 event 0 file two.txt
> >      Event id=100000000 event=0 file=two.txt
> >      Expected event 0 but got 2

Interesting. So In the "Rename" test, the destination already exists.

BSD is thus reporting that 'two.txt' is deleted, before being (re)created
Linux is only reporting 'two.txt' is created.

I don't think we can easily paper over this difference. The easiest is
probably to conditionalize the test

 git diff
diff --git a/tests/unit/test-util-filemonitor.c 
b/tests/unit/test-util-filemonitor.c
index a22de27595..c3b2006365 100644
--- a/tests/unit/test-util-filemonitor.c
+++ b/tests/unit/test-util-filemonitor.c
@@ -281,6 +281,14 @@ test_file_monitor_events(void)
         { .type = QFILE_MONITOR_TEST_OP_EVENT,
           .filesrc = "one.txt", .watchid = &watch1,
           .eventid = QFILE_MONITOR_EVENT_DELETED },
+#ifdef __FreeBSD__
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "two.txt", .watchid = &watch0,
+          .eventid = QFILE_MONITOR_EVENT_DELETED },
+        { .type = QFILE_MONITOR_TEST_OP_EVENT,
+          .filesrc = "two.txt", .watchid = &watch2,
+          .eventid = QFILE_MONITOR_EVENT_DELETED },
+#endif
         { .type = QFILE_MONITOR_TEST_OP_EVENT,
           .filesrc = "two.txt", .watchid = &watch0,
           .eventid = QFILE_MONITOR_EVENT_CREATED },


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|