[libvirt] [PATCH] Check for private symbols presence as well
Currently, we are checking if libvirt.so contains public symbols. However, sometimes we rename an internal symbol and forget to change libvirt_private.syms accordingly. Hence, it's safer to check for internal symbols as well. --- src/Makefile.am | 7 ++++++- src/check-symfile.pl | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index ae3d491..c5840c0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -318,8 +318,13 @@ if WITH_LINUX check-symfile: libvirt.syms libvirt.la $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt.syms \ .libs/libvirt.so + +check-private-symfile: libvirt_private.syms libvirt.la + $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt_private.syms \ + .libs/libvirt.so else check-symfile: +check-private-symfile: endif PROTOCOL_STRUCTS = \ @@ -344,7 +349,7 @@ else !WITH_REMOTE check-protocol: endif EXTRA_DIST += $(PROTOCOL_STRUCTS) check-symfile.pl -check-local: check-protocol check-symfile +check-local: check-protocol check-symfile check-private-symfile .PHONY: check-protocol $(PROTOCOL_STRUCTS:structs=struct) # Mock driver, covering domains, storage, networks, etc diff --git a/src/check-symfile.pl b/src/check-symfile.pl index ac37b46..435e045 100755 --- a/src/check-symfile.pl +++ b/src/check-symfile.pl @@ -44,7 +44,7 @@ foreach my $elflib (@elflibs) { close NM; } -foreach my $sym (@wantsyms) { +foreach my $sym (keys(%wantsyms)) { next if exists $gotsyms{$sym}; print STDERR "Expected symbol $sym is not in ELF library\n"; -- 1.7.8.6
On 10/05/2012 07:05 AM, Michal Privoznik wrote:
Currently, we are checking if libvirt.so contains public symbols. However, sometimes we rename an internal symbol and forget to change libvirt_private.syms accordingly. Hence, it's safer to check for internal symbols as well. --- src/Makefile.am | 7 ++++++- src/check-symfile.pl | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-)
ACK. I confirmed that this catches the problem fixed by your commit fcd6477. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05.10.2012 15:13, Eric Blake wrote:
On 10/05/2012 07:05 AM, Michal Privoznik wrote:
Currently, we are checking if libvirt.so contains public symbols. However, sometimes we rename an internal symbol and forget to change libvirt_private.syms accordingly. Hence, it's safer to check for internal symbols as well. --- src/Makefile.am | 7 ++++++- src/check-symfile.pl | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-)
ACK. I confirmed that this catches the problem fixed by your commit fcd6477.
Thanks, pushed. Michal
On 10/05/2012 07:05 AM, Michal Privoznik wrote:
Currently, we are checking if libvirt.so contains public symbols. However, sometimes we rename an internal symbol and forget to change libvirt_private.syms accordingly. Hence, it's safer to check for internal symbols as well. --- src/Makefile.am | 7 ++++++- src/check-symfile.pl | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-)
Phooey. I had only tested in-tree builds when I gave ack.
check-symfile: libvirt.syms libvirt.la $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt.syms \ .libs/libvirt.so
This works in VPATH builds, since libvirt.syms is version-controlled...
+ +check-private-symfile: libvirt_private.syms libvirt.la + $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt_private.syms \ + .libs/libvirt.so
...but this fails in a VPATH build, since libvirt_private.syms is generated into builddir. I'm working on a followup. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/05/2012 08:42 AM, Eric Blake wrote:
On 10/05/2012 07:05 AM, Michal Privoznik wrote:
Currently, we are checking if libvirt.so contains public symbols. However, sometimes we rename an internal symbol and forget to change libvirt_private.syms accordingly. Hence, it's safer to check for internal symbols as well. --- src/Makefile.am | 7 ++++++- src/check-symfile.pl | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-)
Phooey. I had only tested in-tree builds when I gave ack.
check-symfile: libvirt.syms libvirt.la $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt.syms \ .libs/libvirt.so
This works in VPATH builds, since libvirt.syms is version-controlled...
+ +check-private-symfile: libvirt_private.syms libvirt.la + $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt_private.syms \ + .libs/libvirt.so
...but this fails in a VPATH build, since libvirt_private.syms is generated into builddir. I'm working on a followup.
I got that backwards. libvirt.syms is generated, and in builddir, and _includes_ all of libvirt_public.syms AND libvirt_private.syms. libvirt_private.syms is in srcdir only, so the VPATH build is failing to find it. The reason that your patch "worked" at detecting the failure is not because of the new makefile rule, but because of the fix to use keys(%wantsyms) instead of @wantsyms in the perl script, where check-symfile, rather than the new check-private-symfile, was detecting the error. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This reverts part of commit 5468594f4655d20674a9429a0671f6afa3538274; the perl changes were sufficient. Since libvirt.syms is already a generated file created from libvirt_private.syms, we don't need a second pass over libvirt_private.syms in isolation. --- Pushing under the build-breaker rule. src/Makefile.am | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index c5840c0..ae3d491 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -318,13 +318,8 @@ if WITH_LINUX check-symfile: libvirt.syms libvirt.la $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt.syms \ .libs/libvirt.so - -check-private-symfile: libvirt_private.syms libvirt.la - $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt_private.syms \ - .libs/libvirt.so else check-symfile: -check-private-symfile: endif PROTOCOL_STRUCTS = \ @@ -349,7 +344,7 @@ else !WITH_REMOTE check-protocol: endif EXTRA_DIST += $(PROTOCOL_STRUCTS) check-symfile.pl -check-local: check-protocol check-symfile check-private-symfile +check-local: check-protocol check-symfile .PHONY: check-protocol $(PROTOCOL_STRUCTS:structs=struct) # Mock driver, covering domains, storage, networks, etc -- 1.7.11.4
participants (2)
-
Eric Blake -
Michal Privoznik