| Message ID | 2897c32f34b415aadcf43a5ae296cf5f6e15e757.1501280035.git.alistair.francis@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Alistair, On 07/28/2017 07:16 PM, Alistair Francis wrote: > Convert any remaining uses of fprintf(stderr, "warning:"... > to use warn_report() instead. This helps standardise on a single > method of printing warnings to the user. > > All of the warnings were changed using this command: > find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] |warn_report("|Ig' {} + > > The #include lines and chagnes to the test Makefile were manually > updated to allow the code to compile. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > > tests/Makefile.include | 4 ++-- > util/cutils.c | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 7af278db55..4886caf565 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y) > tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y) > tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) $(test-crypto-obj-y) > tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o > -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-util-obj-y) > +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-qom-obj-y) I don't understand what was not working in the previous line. > -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o > +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o $(test-qom-obj-y) Here adding $(util-obj-y) should be enough. But I did not test it :P Regards, Phil. > tests/test-int128$(EXESUF): tests/test-int128.o > tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y) > tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y) > diff --git a/util/cutils.c b/util/cutils.c > index 1534682083..b33ede83d1 100644 > --- a/util/cutils.c > +++ b/util/cutils.c > @@ -30,6 +30,7 @@ > #include "qemu/iov.h" > #include "net/net.h" > #include "qemu/cutils.h" > +#include "qemu/error-report.h" > > void strpadcpy(char *buf, int buf_size, const char *str, char pad) > { > @@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int initial) > return initial; > } > if (debug < 0 || debug > max || errno != 0) { > - fprintf(stderr, "warning: %s not in [0, %d]", name, max); > + warn_report("%s not in [0, %d]", name, max); > return initial; > } > return debug; >
On Fri, Jul 28, 2017 at 4:57 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Hi Alistair, > > On 07/28/2017 07:16 PM, Alistair Francis wrote: >> >> Convert any remaining uses of fprintf(stderr, "warning:"... >> to use warn_report() instead. This helps standardise on a single >> method of printing warnings to the user. >> >> All of the warnings were changed using this command: >> find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] >> |warn_report("|Ig' {} + >> >> The #include lines and chagnes to the test Makefile were manually >> updated to allow the code to compile. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> >> tests/Makefile.include | 4 ++-- >> util/cutils.c | 3 ++- >> 2 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index 7af278db55..4886caf565 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): >> tests/test-thread-pool.o $(test-block-obj-y) >> tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y) >> tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) >> $(test-crypto-obj-y) >> tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o >> -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o >> migration/page_cache.o $(test-util-obj-y) >> +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o >> migration/page_cache.o $(test-qom-obj-y) > > > I don't understand what was not working in the previous line. > >> -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o >> +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o >> $(test-qom-obj-y) > > > Here adding $(util-obj-y) should be enough. > > But I did not test it :P I didn't understand it either. It is possible I was doing something really wrong, but I couldn't get it to work otherwise. Thanks, Alistair > > Regards, > > Phil. > > >> tests/test-int128$(EXESUF): tests/test-int128.o >> tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y) >> tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y) >> diff --git a/util/cutils.c b/util/cutils.c >> index 1534682083..b33ede83d1 100644 >> --- a/util/cutils.c >> +++ b/util/cutils.c >> @@ -30,6 +30,7 @@ >> #include "qemu/iov.h" >> #include "net/net.h" >> #include "qemu/cutils.h" >> +#include "qemu/error-report.h" >> void strpadcpy(char *buf, int buf_size, const char *str, char pad) >> { >> @@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int >> initial) >> return initial; >> } >> if (debug < 0 || debug > max || errno != 0) { >> - fprintf(stderr, "warning: %s not in [0, %d]", name, max); >> + warn_report("%s not in [0, %d]", name, max); >> return initial; >> } >> return debug; >> >
PATCH 3/5 has the exact same subject. Why are the two separate? Alistair Francis <alistair.francis@xilinx.com> writes: > Convert any remaining uses of fprintf(stderr, "warning:"... > to use warn_report() instead. This helps standardise on a single > method of printing warnings to the user. > > All of the warnings were changed using this command: > find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] |warn_report("|Ig' {} + > > The #include lines and chagnes to the test Makefile were manually changes > updated to allow the code to compile. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > > tests/Makefile.include | 4 ++-- > util/cutils.c | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 7af278db55..4886caf565 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y) > tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y) > tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) $(test-crypto-obj-y) > tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o > -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-util-obj-y) > -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o > +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-qom-obj-y) > +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o $(test-qom-obj-y) No. What symbols exactly is the linker missing? > tests/test-int128$(EXESUF): tests/test-int128.o > tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y) > tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y) > diff --git a/util/cutils.c b/util/cutils.c > index 1534682083..b33ede83d1 100644 > --- a/util/cutils.c > +++ b/util/cutils.c > @@ -30,6 +30,7 @@ > #include "qemu/iov.h" > #include "net/net.h" > #include "qemu/cutils.h" > +#include "qemu/error-report.h" > > void strpadcpy(char *buf, int buf_size, const char *str, char pad) > { > @@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int initial) > return initial; > } > if (debug < 0 || debug > max || errno != 0) { > - fprintf(stderr, "warning: %s not in [0, %d]", name, max); > + warn_report("%s not in [0, %d]", name, max); > return initial; > } > return debug;
On Mon, Aug 14, 2017 at 6:34 AM, Markus Armbruster <armbru@redhat.com> wrote: > PATCH 3/5 has the exact same subject. Why are the two separate? You are right, that is a mess. This one doesn't check for newlines at the end while the earlier one checked for and removed new lines. > > Alistair Francis <alistair.francis@xilinx.com> writes: > >> Convert any remaining uses of fprintf(stderr, "warning:"... >> to use warn_report() instead. This helps standardise on a single >> method of printing warnings to the user. >> >> All of the warnings were changed using this command: >> find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] |warn_report("|Ig' {} + >> >> The #include lines and chagnes to the test Makefile were manually > > changes Fixed. > >> updated to allow the code to compile. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> >> tests/Makefile.include | 4 ++-- >> util/cutils.c | 3 ++- >> 2 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index 7af278db55..4886caf565 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y) >> tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y) >> tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) $(test-crypto-obj-y) >> tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o >> -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-util-obj-y) >> -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o >> +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-qom-obj-y) >> +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o $(test-qom-obj-y) > > No. What symbols exactly is the linker missing? Without the change, this is the error I see when running make check: CC tests/test-x86-cpuid.o LINK tests/test-x86-cpuid GTESTER tests/test-x86-cpuid CC tests/test-xbzrle.o LINK tests/test-xbzrle libqemustub.a(monitor.o): In function `monitor_get_fd': /scratch/alistai/master-qemu/stubs/monitor.c:10: undefined reference to `error_setg_internal' collect2: error: ld returned 1 exit status /scratch/alistai/master-qemu/rules.mak:121: recipe for target 'tests/test-xbzrle' failed make: *** [tests/test-xbzrle] Error 1 If only the xbzrle change is made then I see this: LINK tests/test-xbzrle GTESTER tests/test-xbzrle CC tests/test-vmstate.o LINK tests/test-vmstate GTESTER tests/test-vmstate CC tests/test-cutils.o LINK tests/test-cutils util/cutils.o: In function `parse_debug_env': /scratch/alistai/master-qemu/util/cutils.c:605: undefined reference to `warn_report' collect2: error: ld returned 1 exit status /scratch/alistai/master-qemu/rules.mak:121: recipe for target 'tests/test-cutils' failed make: *** [tests/test-cutils] Error 1 Thanks, Alistair > >> tests/test-int128$(EXESUF): tests/test-int128.o >> tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y) >> tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y) >> diff --git a/util/cutils.c b/util/cutils.c >> index 1534682083..b33ede83d1 100644 >> --- a/util/cutils.c >> +++ b/util/cutils.c >> @@ -30,6 +30,7 @@ >> #include "qemu/iov.h" >> #include "net/net.h" >> #include "qemu/cutils.h" >> +#include "qemu/error-report.h" >> >> void strpadcpy(char *buf, int buf_size, const char *str, char pad) >> { >> @@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int initial) >> return initial; >> } >> if (debug < 0 || debug > max || errno != 0) { >> - fprintf(stderr, "warning: %s not in [0, %d]", name, max); >> + warn_report("%s not in [0, %d]", name, max); >> return initial; >> } >> return debug;
Paolo, there's a Make puzzle for you below. Alistair Francis <alistair.francis@xilinx.com> writes: > On Mon, Aug 14, 2017 at 6:34 AM, Markus Armbruster <armbru@redhat.com> wrote: >> PATCH 3/5 has the exact same subject. Why are the two separate? > > You are right, that is a mess. > > This one doesn't check for newlines at the end while the earlier one > checked for and removed new lines. > >> >> Alistair Francis <alistair.francis@xilinx.com> writes: [...] >>> diff --git a/tests/Makefile.include b/tests/Makefile.include >>> index 7af278db55..4886caf565 100644 >>> --- a/tests/Makefile.include >>> +++ b/tests/Makefile.include >>> @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y) >>> tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y) >>> tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) $(test-crypto-obj-y) >>> tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o >>> -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-util-obj-y) >>> -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o >>> +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-qom-obj-y) >>> +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o $(test-qom-obj-y) >> >> No. What symbols exactly is the linker missing? > > Without the change, this is the error I see when running make check: > > CC tests/test-x86-cpuid.o > LINK tests/test-x86-cpuid > GTESTER tests/test-x86-cpuid > CC tests/test-xbzrle.o > LINK tests/test-xbzrle > libqemustub.a(monitor.o): In function `monitor_get_fd': > /scratch/alistai/master-qemu/stubs/monitor.c:10: undefined reference > to `error_setg_internal' > collect2: error: ld returned 1 exit status > /scratch/alistai/master-qemu/rules.mak:121: recipe for target > 'tests/test-xbzrle' failed > make: *** [tests/test-xbzrle] Error 1 The root cause is "obvious" ;-P The linker searches each .a just once. We link with test-util-obj-y = libqemuutil.a libqemustub.a i.e. the linker first pulls whatever it needs out of libqemuutil.a, then moves on to pull whatever it now needs out of libqemustub.a. If libqemustub.a needs something from libqemuutil.a that hasn't been pulled already, linking fails. The linker explains its doings (--print-map): Archive member included to satisfy reference by file (symbol) libqemuutil.a(cutils.o) tests/test-xbzrle.o (uleb128_encode_small) libqemuutil.a(qemu-error.o) libqemuutil.a(cutils.o) (warn_report) libqemustub.a(error-printf.o) libqemuutil.a(qemu-error.o) (error_vprintf) libqemustub.a(monitor.o) libqemuutil.a(qemu-error.o) (cur_mon) Let me explain the linker's explanation: test-xbzrle.o pulls in libqemuutil.a(cutils.o) for uleb128_encode_small libqemuutil.a(cutils.o) pulls in libqemuutil.a(qemu-error.o) for warn_report libqemuutil.a(qemu-error.o) pulls in libqemustub.a(error_printf.o) and libqemustub.a(monitor.o) for error_vprintf and cur_mon libqemuutil(monitor.o) references error_setg_internal, which can't be resolved. The stupid fix is to repeat libraries until the link succeeds: test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a You may have seen this with -lX11 if you're old enough. ld(1) suggests the linker can do it for us: -( archives -) --start-group archives --end-group The archives should be a list of archive files. They may be either explicit file names, or -l options. The specified archives are searched repeatedly until no new undefined references are created. Normally, an archive is searched only once in the order that it is specified on the command line. If a symbol in that archive is needed to resolve an undefined symbol referred to by an object in an archive that appears later on the command line, the linker would not be able to resolve that reference. By grouping the archives, they all be searched repeatedly until all possible references are resolved. Using this option has a significant performance cost. It is best to use it only when there are unavoidable circular references between two or more archives. Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1 doesn't work for me, though. The smart solution is not to have .a reference each other. Paolo, what do you think? > If only the xbzrle change is made then I see this: > > LINK tests/test-xbzrle > GTESTER tests/test-xbzrle > CC tests/test-vmstate.o > LINK tests/test-vmstate > GTESTER tests/test-vmstate > CC tests/test-cutils.o > LINK tests/test-cutils > util/cutils.o: In function `parse_debug_env': > /scratch/alistai/master-qemu/util/cutils.c:605: undefined reference to > `warn_report' > collect2: error: ld returned 1 exit status > /scratch/alistai/master-qemu/rules.mak:121: recipe for target > 'tests/test-cutils' failed > make: *** [tests/test-cutils] Error 1
On 15/08/2017 09:30, Markus Armbruster wrote: > The stupid fix is to repeat libraries until the link succeeds: > > test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a > > You may have seen this with -lX11 if you're old enough. > > ld(1) suggests the linker can do it for us: > > -( archives -) > --start-group archives --end-group > The archives should be a list of archive files. They may be either > explicit file names, or -l options. > > The specified archives are searched repeatedly until no new > undefined references are created. Normally, an archive is searched > only once in the order that it is specified on the command line. > If a symbol in that archive is needed to resolve an undefined > symbol referred to by an object in an archive that appears later on > the command line, the linker would not be able to resolve that > reference. By grouping the archives, they all be searched > repeatedly until all possible references are resolved. > > Using this option has a significant performance cost. It is best > to use it only when there are unavoidable circular references > between two or more archives. > > Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1 > doesn't work for me, though. > > The smart solution is not to have .a reference each other. Nah, I think we should teach those new kids on the block about -lX11 instead. :) > Paolo, what do you think? Another possibility is to just merge the two static libraries into one. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 15/08/2017 09:30, Markus Armbruster wrote: >> The stupid fix is to repeat libraries until the link succeeds: >> >> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a >> >> You may have seen this with -lX11 if you're old enough. >> >> ld(1) suggests the linker can do it for us: >> >> -( archives -) >> --start-group archives --end-group >> The archives should be a list of archive files. They may be either >> explicit file names, or -l options. >> >> The specified archives are searched repeatedly until no new >> undefined references are created. Normally, an archive is searched >> only once in the order that it is specified on the command line. >> If a symbol in that archive is needed to resolve an undefined >> symbol referred to by an object in an archive that appears later on >> the command line, the linker would not be able to resolve that >> reference. By grouping the archives, they all be searched >> repeatedly until all possible references are resolved. >> >> Using this option has a significant performance cost. It is best >> to use it only when there are unavoidable circular references >> between two or more archives. >> >> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1 >> doesn't work for me, though. >> >> The smart solution is not to have .a reference each other. > > Nah, I think we should teach those new kids on the block about -lX11 > instead. :) > >> Paolo, what do you think? > > Another possibility is to just merge the two static libraries into one. Sounds good to me!
On Thu, Aug 17, 2017 at 10:02 AM, Markus Armbruster <armbru@redhat.com> wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > >> On 15/08/2017 09:30, Markus Armbruster wrote: >>> The stupid fix is to repeat libraries until the link succeeds: >>> >>> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a >>> >>> You may have seen this with -lX11 if you're old enough. >>> >>> ld(1) suggests the linker can do it for us: >>> >>> -( archives -) >>> --start-group archives --end-group >>> The archives should be a list of archive files. They may be either >>> explicit file names, or -l options. >>> >>> The specified archives are searched repeatedly until no new >>> undefined references are created. Normally, an archive is searched >>> only once in the order that it is specified on the command line. >>> If a symbol in that archive is needed to resolve an undefined >>> symbol referred to by an object in an archive that appears later on >>> the command line, the linker would not be able to resolve that >>> reference. By grouping the archives, they all be searched >>> repeatedly until all possible references are resolved. >>> >>> Using this option has a significant performance cost. It is best >>> to use it only when there are unavoidable circular references >>> between two or more archives. >>> >>> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1 >>> doesn't work for me, though. >>> >>> The smart solution is not to have .a reference each other. >> >> Nah, I think we should teach those new kids on the block about -lX11 >> instead. :) This sounds scary... >> >>> Paolo, what do you think? >> >> Another possibility is to just merge the two static libraries into one. > > Sounds good to me! I feel like I have opened a can of worms. I can try and combine libqemustub.a into libqemuutil.a is that the solution? I just want to make sure before I start this. Thanks, Alistair >
On 08/17/2017 02:02 PM, Markus Armbruster wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: >> On 15/08/2017 09:30, Markus Armbruster wrote: >>> The stupid fix is to repeat libraries until the link succeeds: >>> >>> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a >>> >>> You may have seen this with -lX11 if you're old enough. >>> [...] >>> >>> The smart solution is not to have .a reference each other. >> >> Nah, I think we should teach those new kids on the block about -lX11 >> instead. :) haha >> >>> Paolo, what do you think? >> >> Another possibility is to just merge the two static libraries into one. > > Sounds good to me! it makes sens to only keep libqemuutil.a with stub included.
On 08/17/2017 02:55 PM, Alistair Francis wrote: >>> On 15/08/2017 09:30, Markus Armbruster wrote: >>>> The stupid fix is to repeat libraries until the link succeeds: >>>> >>>> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a >>>> [...] >>>> >>>> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1 >>>> doesn't work for me, though. >>>> >>>> The smart solution is not to have .a reference each other. >>> >>> Nah, I think we should teach those new kids on the block about -lX11 >>> instead. :) > > This sounds scary... > >>> >>>> Paolo, what do you think? >>> >>> Another possibility is to just merge the two static libraries into one. >> >> Sounds good to me! > > I feel like I have opened a can of worms. you are good at it! IIRC it all started with a 1-line change in tcp_chr_wait_connected() more than 2 months ago :) > > I can try and combine libqemustub.a into libqemuutil.a is that the > solution? I just want to make sure before I start this. IMHO your series is OK like this, add a "TODO remove once libqemuutil.a circular dep is resolved" comment in the Makefile is enough, and let this issue for another time. Regards, Phil.
Philippe Mathieu-Daudé <f4bug@amsat.org> writes: > On 08/17/2017 02:55 PM, Alistair Francis wrote: >>>> On 15/08/2017 09:30, Markus Armbruster wrote: >>>>> The stupid fix is to repeat libraries until the link succeeds: >>>>> >>>>> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a >>>>> > [...] >>>>> >>>>> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1 >>>>> doesn't work for me, though. >>>>> >>>>> The smart solution is not to have .a reference each other. >>>> >>>> Nah, I think we should teach those new kids on the block about -lX11 >>>> instead. :) >> >> This sounds scary... >> >>>> >>>>> Paolo, what do you think? >>>> >>>> Another possibility is to just merge the two static libraries into one. >>> >>> Sounds good to me! >> >> I feel like I have opened a can of worms. > > you are good at it! IIRC it all started with a 1-line change in > tcp_chr_wait_connected() more than 2 months ago :) > >> >> I can try and combine libqemustub.a into libqemuutil.a is that the >> solution? I just want to make sure before I start this. > > IMHO your series is OK like this, add a "TODO remove once > libqemuutil.a circular dep is resolved" comment in the Makefile is > enough, and let this issue for another time. I disagree. If merging the two .a is beyond your reach (I hope it isn't), then the spot to mess up is this one: # TODO bla bla explain bla test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a
On Thu, Aug 17, 2017 at 10:32 PM, Markus Armbruster <armbru@redhat.com> wrote: > Philippe Mathieu-Daudé <f4bug@amsat.org> writes: > >> On 08/17/2017 02:55 PM, Alistair Francis wrote: >>>>> On 15/08/2017 09:30, Markus Armbruster wrote: >>>>>> The stupid fix is to repeat libraries until the link succeeds: >>>>>> >>>>>> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a >>>>>> >> [...] >>>>>> >>>>>> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1 >>>>>> doesn't work for me, though. >>>>>> >>>>>> The smart solution is not to have .a reference each other. >>>>> >>>>> Nah, I think we should teach those new kids on the block about -lX11 >>>>> instead. :) >>> >>> This sounds scary... >>> >>>>> >>>>>> Paolo, what do you think? >>>>> >>>>> Another possibility is to just merge the two static libraries into one. >>>> >>>> Sounds good to me! >>> >>> I feel like I have opened a can of worms. >> >> you are good at it! IIRC it all started with a 1-line change in >> tcp_chr_wait_connected() more than 2 months ago :) >> >>> >>> I can try and combine libqemustub.a into libqemuutil.a is that the >>> solution? I just want to make sure before I start this. >> >> IMHO your series is OK like this, add a "TODO remove once >> libqemuutil.a circular dep is resolved" comment in the Makefile is >> enough, and let this issue for another time. > > I disagree. > > If merging the two .a is beyond your reach (I hope it isn't), then the > spot to mess up is this one: This actually seems pretty easy to do. I'm going to split this patch and the Makefile changes into a separate series and send those so I don't end up spamming everyone with the earlier patches in the series. Then after 2.10 I can combine them all and send the full series. Thanks, Alistair > > # TODO bla bla explain bla > test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a >
On 08/18/2017 02:32 AM, Markus Armbruster wrote: > Philippe Mathieu-Daudé <f4bug@amsat.org> writes: > >> On 08/17/2017 02:55 PM, Alistair Francis wrote: >>>>> On 15/08/2017 09:30, Markus Armbruster wrote: >>>>>> The stupid fix is to repeat libraries until the link succeeds: >>>>>> >>>>>> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a >>>>>> >> [...] >>>>>> >>>>>> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1 >>>>>> doesn't work for me, though. >>>>>> >>>>>> The smart solution is not to have .a reference each other. >>>>> >>>>> Nah, I think we should teach those new kids on the block about -lX11 >>>>> instead. :) >>> >>> This sounds scary... >>> >>>>> >>>>>> Paolo, what do you think? >>>>> >>>>> Another possibility is to just merge the two static libraries into one. >>>> >>>> Sounds good to me! >>> >>> I feel like I have opened a can of worms. >> >> you are good at it! IIRC it all started with a 1-line change in >> tcp_chr_wait_connected() more than 2 months ago :) >> >>> >>> I can try and combine libqemustub.a into libqemuutil.a is that the >>> solution? I just want to make sure before I start this. >> >> IMHO your series is OK like this, add a "TODO remove once >> libqemuutil.a circular dep is resolved" comment in the Makefile is >> enough, and let this issue for another time. > > I disagree. > > If merging the two .a is beyond your reach (I hope it isn't), then the > spot to mess up is this one: > > # TODO bla bla explain bla > test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a > I was talking about the first patch: +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o $(test-qom-obj-y) not merging circularly :)
diff --git a/tests/Makefile.include b/tests/Makefile.include index 7af278db55..4886caf565 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y) tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y) tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) $(test-crypto-obj-y) tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-util-obj-y) -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o migration/page_cache.o $(test-qom-obj-y) +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o $(test-qom-obj-y) tests/test-int128$(EXESUF): tests/test-int128.o tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y) tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y) diff --git a/util/cutils.c b/util/cutils.c index 1534682083..b33ede83d1 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -30,6 +30,7 @@ #include "qemu/iov.h" #include "net/net.h" #include "qemu/cutils.h" +#include "qemu/error-report.h" void strpadcpy(char *buf, int buf_size, const char *str, char pad) { @@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int initial) return initial; } if (debug < 0 || debug > max || errno != 0) { - fprintf(stderr, "warning: %s not in [0, %d]", name, max); + warn_report("%s not in [0, %d]", name, max); return initial; } return debug;
Convert any remaining uses of fprintf(stderr, "warning:"... to use warn_report() instead. This helps standardise on a single method of printing warnings to the user. All of the warnings were changed using this command: find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] |warn_report("|Ig' {} + The #include lines and chagnes to the test Makefile were manually updated to allow the code to compile. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- tests/Makefile.include | 4 ++-- util/cutils.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-)