[v1] make vm-build-freebsd fixes

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

Posted by Ilya Leoshkevich 10 months ago

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

FreeBSD itself disables this functionality in the respective port [1].
So do it upstream too.

[1] https://cgit.freebsd.org/ports/tree/emulators/qemu-devel/files/patch-util_meson.build?id=984366c18f1bc54e38751afc29be08c596b83696

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index d0329966f1b..3d67d78b522 100644
--- a/meson.build
+++ b/meson.build
@@ -2403,6 +2403,7 @@ config_host_data.set('CONFIG_GETRANDOM',
 config_host_data.set('CONFIG_INOTIFY',
                      cc.has_header_symbol('sys/inotify.h', 'inotify_init'))
 config_host_data.set('CONFIG_INOTIFY1',
+                     host_os != 'freebsd' and
                      cc.has_header_symbol('sys/inotify.h', 'inotify_init1'))
 config_host_data.set('CONFIG_PRCTL_PR_SET_TIMERSLACK',
                      cc.has_header_symbol('sys/prctl.h', 'PR_SET_TIMERSLACK'))
-- 
2.43.0

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

Posted by Philippe Mathieu-Daudé 9 months, 4 weeks ago

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

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

Wouldn't it be better to use a runtime check in qemu_file_monitor_new()?

> FreeBSD itself disables this functionality in the respective port [1].
> So do it upstream too.
> 
> [1] https://cgit.freebsd.org/ports/tree/emulators/qemu-devel/files/patch-util_meson.build?id=984366c18f1bc54e38751afc29be08c596b83696
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   meson.build | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/meson.build b/meson.build
> index d0329966f1b..3d67d78b522 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2403,6 +2403,7 @@ config_host_data.set('CONFIG_GETRANDOM',
>   config_host_data.set('CONFIG_INOTIFY',
>                        cc.has_header_symbol('sys/inotify.h', 'inotify_init'))
>   config_host_data.set('CONFIG_INOTIFY1',
> +                     host_os != 'freebsd' and
>                        cc.has_header_symbol('sys/inotify.h', 'inotify_init1'))
>   config_host_data.set('CONFIG_PRCTL_PR_SET_TIMERSLACK',
>                        cc.has_header_symbol('sys/prctl.h', 'PR_SET_TIMERSLACK'))

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

Posted by Daniel P. Berrangé 9 months, 4 weeks ago

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

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

Posted by Warner Losh 9 months, 3 weeks ago

On Wed, Jan 31, 2024 at 9:42 AM Daniel P. Berrangé <berrange@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?

Warner

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

Posted by Daniel P. Berrangé 9 months, 3 weeks ago

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é <berrange@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
-- 
|: 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 :|

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

Posted by Kyle Evans 9 months, 3 weeks ago

On 2/5/24 09:31, 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é <berrange@redhat.com>
>> wrote:
>>
>>> On Wed, Jan 31, 2024 at 05:24:10PM +0100, Philippe Mathieu-Daudé wrote:
 > [... snip ...]
>>>> 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.
> 

FWIW, +1... unless we miss the follow-up notification that it's been 
created, I'd personally put it into the WONTFIX bucket pretty quickly.

Barring some kind of NOTE_COVER (bad name) that can be emitted if a file 
is simply replaced by another, a directory being reported as shortened 
then extended is a valid and useful representation of the situation 
(even if not completely accurate) to avoid consumers missing the action 
entirely.

Thanks,

Kyle Evans

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

Posted by Ilya Leoshkevich 9 months, 3 weeks ago

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é
> > <berrange@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.

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

Posted by Thomas Huth 10 months ago

On 25/01/2024 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
> 
> FreeBSD itself disables this functionality in the respective port [1].
> So do it upstream too.
> 
> [1] https://cgit.freebsd.org/ports/tree/emulators/qemu-devel/files/patch-util_meson.build?id=984366c18f1bc54e38751afc29be08c596b83696
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   meson.build | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/meson.build b/meson.build
> index d0329966f1b..3d67d78b522 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2403,6 +2403,7 @@ config_host_data.set('CONFIG_GETRANDOM',
>   config_host_data.set('CONFIG_INOTIFY',
>                        cc.has_header_symbol('sys/inotify.h', 'inotify_init'))
>   config_host_data.set('CONFIG_INOTIFY1',
> +                     host_os != 'freebsd' and
>                        cc.has_header_symbol('sys/inotify.h', 'inotify_init1'))
>   config_host_data.set('CONFIG_PRCTL_PR_SET_TIMERSLACK',
>                        cc.has_header_symbol('sys/prctl.h', 'PR_SET_TIMERSLACK'))

Reviewed-by: Thomas Huth <thuth@redhat.com>