Re: [PATCH] tests/migration: use the common library function

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 11 Nov 2019 at 14:41, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 11/11/2019 15.11, Alex Bennée wrote:
>> >
>> > Thomas Huth <thuth@redhat.com> writes:
>> >
>> >> On 11/11/2019 13.55, Alex Bennée wrote:
>> >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> >>
>> >> Could you please add at least a short patch description? (Why is this
>> >> change necessary / a good idea?)
>> >
>> > It's just a minor clean-up Dave happened to comment on last week. Using
>> > the helper function is preferable given it abstracts away any system
>> > differences for the same information.
>>
>> But this also changes the behavior on non-Linux systems (i.e. the *BSDs
>> and macOS), since they will now use getpid() instead of gettid ... is
>> that the intended change here?
>
> Does the 'stress' program work on those OSes? For that matter,
> does it work on Linux?
>
> As far as I can tell we don't compile stress.c on any host,
> since the only thing that depends on tests/migration/stress$(EXESUF)
> is tests/migration/initrd-stress.img, and nothing depends on that.
>
> Nothing creates tests/migration/ in the build dir so trying
> to build tests/migration/stress in an out-of-tree config fails:
>
>   CC      tests/migration/stress.o
> /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration/stress.c:359:1:
> fatal error: opening dependency file tests/migration/stress.d: No such
> file or directory
>  }
>  ^
> compilation terminated.
>
> ...and if I fix that by manually creating the directory then
> it fails to link:
>
>   CC      tests/migration/stress.o
>   LINK    tests/migration/stress
> tests/migration/stress.o: In function `get_command_arg_str':
> /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration/stress.c:107:
> undefined reference to `g_strndup'
> /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration/stress.c:109:
> undefined reference to `g_strdup'
> tests/migration/stress.o: In function `get_command_arg_ull':
> /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration/stress.c:129:
> undefined reference to `g_free'
> /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration/stress.c:132:
> undefined reference to `g_free'
> tests/migration/stress.o: In function `stress':
> /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration/stress.c:253:
> undefined reference to `pthread_create'
> collect2: error: ld returned 1 exit status
> /home/petmay01/linaro/qemu-from-laptop/qemu/tests/Makefile.include:849:
> recipe for target 'tests/migration/stress' failed
>
> Is this dead code ?

It was introduced around 3 years ago by Daniel for stress testing. The
instructions in:

  409437e16df273fc5f78f6cd1cb53023eaeb9b72
  Author:     Daniel P. Berrangé <berrange@redhat.com>
  AuthorDate: Wed Jul 20 14:23:13 2016 +0100
  Commit:     Amit Shah <amit.shah@redhat.com>
  CommitDate: Fri Jul 22 13:23:39 2016 +0530

  tests: introduce a framework for testing migration performance

say to use:

  make tests/migration/initrd-stress.img

And that has indeed bitrotted over time. All the other tweaks since are
passing through clean ups.

>
> thanks
> -- PMM


--
Alex Bennée