On Mon, 2024-02-05 at 15:31 +0000, Daniel P. Berrangé wrote: > On Mon, Feb 05, 2024 at 08:23:41AM -0700, Warner Losh wrote: > > On Wed, Jan 31, 2024 at 9:42 AM Daniel P. Berrangé > > <berra...@redhat.com> > > wrote: > > > > > 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 }, > > > > > > > I agree this is likely the best course of action. Has anybody filed > > a bug > > at https://bugs.freebsd.org? > > I've not, and I'm not even sure I would class it a FreeBSD bug. Other > than the fact that it differs from Linux behaviour, it feels like it > is reasonble semantics to emit a 'delete' event in this scenario so > that an event consumer can detect replacement of an existing file. > > With regards, > Daniel
Sounds reasonable; I will send a v2 with the meson adjustments and with the test fix.
