1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] Makefile: don't use LINKPROG for tests/migration/stress

Alexey Perevalov posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1506933058-28313-2-git-send-email-a.perevalov@samsung.com
Test checkpatch passed
Test docker passed
Test s390x passed
tests/Makefile.include | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PATCH] Makefile: don't use LINKPROG for tests/migration/stress
Posted by Alexey Perevalov 6 years, 6 months ago 
This patch just replaces  LINKPROG for CC.
LINKPROG invokes both CXX and CC.
So in case of static linking of C source, static libstdc++
is required by build, but it could be missing in system.
And static libstdc++ may not be needed for whole QEMU.

Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index abc6707..bf94516 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -803,7 +803,7 @@ tests/numa-test$(EXESUF): tests/numa-test.o
 tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
-	$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
+	$(call quiet-command, $(CC) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
 
 INITRD_WORK_DIR=tests/migration/initrd
 
-- 
2.7.4
Re: [Qemu-devel] [PATCH] Makefile: don't use LINKPROG for tests/migration/stress
Posted by Peter Maydell 6 years, 6 months ago 
On 2 October 2017 at 09:30, Alexey Perevalov <a.perevalov@samsung.com> wrote:
> This patch just replaces  LINKPROG for CC.
> LINKPROG invokes both CXX and CC.
> So in case of static linking of C source, static libstdc++
> is required by build, but it could be missing in system.
> And static libstdc++ may not be needed for whole QEMU.
>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index abc6707..bf94516 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -803,7 +803,7 @@ tests/numa-test$(EXESUF): tests/numa-test.o
>  tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
>
>  tests/migration/stress$(EXESUF): tests/migration/stress.o
> -       $(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
> +       $(call quiet-command, $(CC) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
>
>  INITRD_WORK_DIR=tests/migration/initrd

Why does this executable need to define its own link rule anyway?
Ideally it should just use the standard rules.mak LINK function
like everything else. If it does need to do something weird
it should have a comment saying why it's weird...

thanks
-- PMM
Re: [Qemu-devel] [PATCH] Makefile: don't use LINKPROG for tests/migration/stress
Posted by Daniel P. Berrange 6 years, 6 months ago 
On Mon, Oct 02, 2017 at 12:14:39PM +0100, Peter Maydell wrote:
> On 2 October 2017 at 09:30, Alexey Perevalov <a.perevalov@samsung.com> wrote:
> > This patch just replaces  LINKPROG for CC.
> > LINKPROG invokes both CXX and CC.
> > So in case of static linking of C source, static libstdc++
> > is required by build, but it could be missing in system.
> > And static libstdc++ may not be needed for whole QEMU.
> >
> > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> > ---
> >  tests/Makefile.include | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index abc6707..bf94516 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -803,7 +803,7 @@ tests/numa-test$(EXESUF): tests/numa-test.o
> >  tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
> >
> >  tests/migration/stress$(EXESUF): tests/migration/stress.o
> > -       $(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
> > +       $(call quiet-command, $(CC) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
> >
> >  INITRD_WORK_DIR=tests/migration/initrd
> 
> Why does this executable need to define its own link rule anyway?
> Ideally it should just use the standard rules.mak LINK function
> like everything else. If it does need to do something weird
> it should have a comment saying why it's weird...

Primarily because we're static linking this binary so that it can be put
into an initrd without needing any extra files added. It doesn't use any
of the 3rd party libraries the rest of QEMU uses - ie no glib2 in particular.

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: [Qemu-devel] [PATCH] Makefile: don't use LINKPROG for tests/migration/stress
Posted by Peter Maydell 6 years, 6 months ago 
On 2 October 2017 at 12:17, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Mon, Oct 02, 2017 at 12:14:39PM +0100, Peter Maydell wrote:
>> Why does this executable need to define its own link rule anyway?
>> Ideally it should just use the standard rules.mak LINK function
>> like everything else. If it does need to do something weird
>> it should have a comment saying why it's weird...
>
> Primarily because we're static linking this binary so that it can be put
> into an initrd without needing any extra files added. It doesn't use any
> of the 3rd party libraries the rest of QEMU uses - ie no glib2 in particular.

Oh, are we committing our usual error of trying to build guest
binaries with the host toolchain?

thanks
-- PMM
Re: [Qemu-devel] [PATCH] Makefile: don't use LINKPROG for tests/migration/stress
Posted by Daniel P. Berrange 6 years, 6 months ago 
On Mon, Oct 02, 2017 at 12:19:54PM +0100, Peter Maydell wrote:
> On 2 October 2017 at 12:17, Daniel P. Berrange <berrange@redhat.com> wrote:
> > On Mon, Oct 02, 2017 at 12:14:39PM +0100, Peter Maydell wrote:
> >> Why does this executable need to define its own link rule anyway?
> >> Ideally it should just use the standard rules.mak LINK function
> >> like everything else. If it does need to do something weird
> >> it should have a comment saying why it's weird...
> >
> > Primarily because we're static linking this binary so that it can be put
> > into an initrd without needing any extra files added. It doesn't use any
> > of the 3rd party libraries the rest of QEMU uses - ie no glib2 in particular.
> 
> Oh, are we committing our usual error of trying to build guest
> binaries with the host toolchain?

That's not an error here - this test suite is only intended to run with
matching host/guest arch, and boots with the host kernel + custom initrd
containing only this binary.

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.