| Message ID | 1452174718-13233-1-git-send-email-ian.campbell@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/01/16 13:51, Ian Campbell wrote: > The map files highlight a number of namespacing inconsistencies > (particularly with libxenguest using xc_* a significant amount). > > It also seems to highlight a bunch of libxenguest.so functionalty > which appears to want to be exported (xc_*) but is not used in tree. > The initial list was based on what was needed to compile everything in > tree. I then looked through the list for xc_* and checked if any were > exported in a public header, leading to adding the following functions > which are intended to be public but not used in tree to the > libxenguest.map: > - xc_cpuid_to_str > - xc_compression_add_page > - xc_compression_compress_pages > - xc_compression_create_context > - xc_compression_free_context > - xc_compression_reset_pagebuf > - xc_compression_uncompress_page These compression functions became unused when I dropped legacy migration. > diff --git a/tools/libxc/libxenctrl.map b/tools/libxc/libxenctrl.map > new file mode 100644 > index 0000000..cc93a5b > --- /dev/null > +++ b/tools/libxc/libxenctrl.map > @@ -0,0 +1,18 @@ > +{ > + global: > + xc_*; > + > + /* > + * Supposedly internal functions which are also used > + * by libxenguest (only, it seems) > + */ > + read_exact; > + write_exact; > + writev_exact; read/write_exact are used in libxc by xc_tmem.c, but only because the tmem part of legacy migration split across the two libraries. In the long term, they should move to being xenguest private. ~Andrew > + > + /* Other un-namespaced functions used elsewhere in tree */ > + do_xen_hypercall; > + do_memory_op; > + > + local: *; /* Do not expose anything by default */ > +};
>>> On 07.01.16 at 14:51, <ian.campbell@citrix.com> wrote: > --- /dev/null > +++ b/tools/libxc/libxenctrl.map > @@ -0,0 +1,18 @@ > +{ > + global: No actual version identifier / name space? Jan
On Thu, 2016-01-07 at 14:36 +0000, Andrew Cooper wrote: > On 07/01/16 13:51, Ian Campbell wrote: > > The map files highlight a number of namespacing inconsistencies > > (particularly with libxenguest using xc_* a significant amount). > > > > It also seems to highlight a bunch of libxenguest.so functionalty > > which appears to want to be exported (xc_*) but is not used in tree. > > The initial list was based on what was needed to compile everything in > > tree. I then looked through the list for xc_* and checked if any were > > exported in a public header, leading to adding the following functions > > which are intended to be public but not used in tree to the > > libxenguest.map: > > - xc_cpuid_to_str > > - xc_compression_add_page > > - xc_compression_compress_pages > > - xc_compression_create_context > > - xc_compression_free_context > > - xc_compression_reset_pagebuf > > - xc_compression_uncompress_page > > These compression functions became unused when I dropped legacy > migration. Indeed, I anticipated them either coming back or getting removed at some point. > > diff --git a/tools/libxc/libxenctrl.map b/tools/libxc/libxenctrl.map > > new file mode 100644 > > index 0000000..cc93a5b > > --- /dev/null > > +++ b/tools/libxc/libxenctrl.map > > @@ -0,0 +1,18 @@ > > +{ > > + global: > > + xc_*; > > + > > + /* > > + * Supposedly internal functions which are also used > > + * by libxenguest (only, it seems) > > + */ > > + read_exact; > > + write_exact; > > + writev_exact; > > read/write_exact are used in libxc by xc_tmem.c, but only because the > tmem part of legacy migration split across the two libraries. They are also used by various osdep backend stuff in libxc, which should go away with my split of those into other libraries and by xc_core.c (which is in libxc, perhaps wrongly). > In the long term, they should move to being xenguest private. > > ~Andrew > > > + > > + /* Other un-namespaced functions used elsewhere in > > tree */ > > + do_xen_hypercall; > > + do_memory_op; > > + > > + local: *; /* Do not expose anything by default */ > > +}; >
On Thu, 2016-01-07 at 07:49 -0700, Jan Beulich wrote: > > > > On 07.01.16 at 14:51, <ian.campbell@citrix.com> wrote: > > --- /dev/null > > +++ b/tools/libxc/libxenctrl.map > > @@ -0,0 +1,18 @@ > > +{ > > + global: > > No actual version identifier / name space? Correct, this is only being used to hide internal symbols, not to to provide any kind of ABI stability (which libxc and libxg lack). Maybe libxenstore should be a) done separately and b) have a namespace as you suggest and be declared stable in the same way as the things I am adding under tools/libs are, but given that it uses libxenctrl there's probably a little more work to do before that becomes possible. Ian.
diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile index 940708f..8882aa1 100644 --- a/tools/libxc/Makefile +++ b/tools/libxc/Makefile @@ -206,8 +206,9 @@ libxenctrl.so: libxenctrl.so.$(MAJOR) libxenctrl.so.$(MAJOR): libxenctrl.so.$(MAJOR).$(MINOR) $(SYMLINK_SHLIB) $< $@ -libxenctrl.so.$(MAJOR).$(MINOR): $(CTRL_PIC_OBJS) - $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenctrl.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ $(LDLIBS_libxentoollog) $(DLOPEN_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS) +libxenctrl.so.$(MAJOR).$(MINOR): SHLIB_LDFLAGS += -Wl,--version-script=libxenctrl.map +libxenctrl.so.$(MAJOR).$(MINOR): $(CTRL_PIC_OBJS) libxenctrl.map + $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenctrl.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $(CTRL_PIC_OBJS) $(LDLIBS_libxentoollog) $(DLOPEN_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS) # libxenguest @@ -228,8 +229,9 @@ endif xc_dom_bzimageloader.o: CFLAGS += $(call zlib-options,D) xc_dom_bzimageloader.opic: CFLAGS += $(call zlib-options,D) +libxenguest.so.$(MAJOR).$(MINOR): SHLIB_LDFLAGS += -Wl,--version-script=libxenguest.map libxenguest.so.$(MAJOR).$(MINOR): COMPRESSION_LIBS = $(call zlib-options,l) -libxenguest.so.$(MAJOR).$(MINOR): $(GUEST_PIC_OBJS) libxenctrl.so +libxenguest.so.$(MAJOR).$(MINOR): $(GUEST_PIC_OBJS) libxenctrl.so libxenguest.map $(CC) $(LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenguest.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $(GUEST_PIC_OBJS) $(COMPRESSION_LIBS) -lz $(LDLIBS_libxenctrl) $(PTHREAD_LIBS) $(APPEND_LDFLAGS) xenctrl_osdep_ENOSYS.so: $(OSDEP_PIC_OBJS) libxenctrl.so diff --git a/tools/libxc/libxenctrl.map b/tools/libxc/libxenctrl.map new file mode 100644 index 0000000..cc93a5b --- /dev/null +++ b/tools/libxc/libxenctrl.map @@ -0,0 +1,18 @@ +{ + global: + xc_*; + + /* + * Supposedly internal functions which are also used + * by libxenguest (only, it seems) + */ + read_exact; + write_exact; + writev_exact; + + /* Other un-namespaced functions used elsewhere in tree */ + do_xen_hypercall; + do_memory_op; + + local: *; /* Do not expose anything by default */ +}; diff --git a/tools/libxc/libxenguest.map b/tools/libxc/libxenguest.map new file mode 100644 index 0000000..5aa3605 --- /dev/null +++ b/tools/libxc/libxenguest.map @@ -0,0 +1,37 @@ +{ + global: + xg_*; + elf_*; /* Stuff from libelf */ + xc_dom_*; /* Domain builder */ + xc_linux_build; /* Compat for xc_dom_* */ + + /* Either wrongly namespaced or in the wrong library */ + xc_elf_set_logfile; + + xc_domain_save; + xc_domain_restore; + + xc_exchange_page; + xc_mark_page_online; + xc_mark_page_offline; + xc_query_page_offline_status; + + xc_await_suspend; + xc_suspend_evtchn_init_exclusive; + xc_suspend_evtchn_init_sane; + xc_suspend_evtchn_release; + + xc_map_m2p; + + xc_dom_check_gzip; + xc_dom_do_gunzip; + + xc_cpuid_apply_policy; + xc_cpuid_set; + xc_cpuid_check; + xc_cpuid_to_str; + + xc_domain_get_native_protocol; + + local: *; /* Do not expose anything by default */ +}; diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile index 1b4a494..87c2719 100644 --- a/tools/xenstore/Makefile +++ b/tools/xenstore/Makefile @@ -102,8 +102,9 @@ libxenstore.so.$(MAJOR): libxenstore.so.$(MAJOR).$(MINOR) xs.opic: CFLAGS += -DUSE_PTHREAD -libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic - $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ $(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS) +libxenstore.so.$(MAJOR).$(MINOR): SHLIB_LDFLAGS += -Wl,--version-script=libxenstore.map +libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic libxenstore.map + $(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ xs.opic xs_lib.opic $(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS) libxenstore.a: xs.o xs_lib.o $(AR) rcs $@ $^ diff --git a/tools/xenstore/libxenstore.map b/tools/xenstore/libxenstore.map new file mode 100644 index 0000000..c406207 --- /dev/null +++ b/tools/xenstore/libxenstore.map @@ -0,0 +1,15 @@ +{ + global: + xs_*; + + /* + * Un-namespaced functions, may only be used from + * xenstore_client.c? Unclear if they might be useful + * from other potentially client applications. + */ + sanitise_value; + unsanitise_value; + expanding_buffer_ensure; + + local: *; /* Do not expose anything by default */ +};
... and limit the exported symbols to just those which are used in tree. This was partly just an interest driven excercise to see how much internal cruft these libraries were actually exporting (it's not actually as bad as I expected). According to diffing before and after: nm libfoo.so | awk '{ if ($2 == "T") { print $3 } }' The following symbols are no longer exported: - libxenstore.so: - _fini - _init - libxenctrl.so: - _fini - _init - bitmap_64_to_byte - bitmap_byte_to_64 - discard_file_cache - libxenguest.so: - _fini - _init - cr3_to_mfn - csum_page - dhdr_type_to_str - dump_bad_pseudophysmap_entry - handle_tsc_info - lz4_decompress_unknownoutputsize - mfn_in_pseudophysmap - mfn_to_cr3 - mfn_to_pfn - pin_table - populate_pfns - rec_type_to_str - write_split_record - write_tsc_info - x86_pv_domain_info - x86_pv_map_m2p - xc_inflate_buffer - xc_read_image - xc_try_lz4_decode The map files highlight a number of namespacing inconsistencies (particularly with libxenguest using xc_* a significant amount). It also seems to highlight a bunch of libxenguest.so functionalty which appears to want to be exported (xc_*) but is not used in tree. The initial list was based on what was needed to compile everything in tree. I then looked through the list for xc_* and checked if any were exported in a public header, leading to adding the following functions which are intended to be public but not used in tree to the libxenguest.map: - xc_cpuid_to_str - xc_compression_add_page - xc_compression_compress_pages - xc_compression_create_context - xc_compression_free_context - xc_compression_reset_pagebuf - xc_compression_uncompress_page The remainder are internal functions using the external namespace (of another library, no less). Since the relevant libfoo.map is added to the Makefile dependencies of the library, no longer use $^ to get the objects for libxenctrl and libxenstore, libxenguest already didn't. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- This was something of an academic exercise, I'm not sure it's really the right thing, especially for the unstable libraries. --- tools/libxc/Makefile | 8 +++++--- tools/libxc/libxenctrl.map | 18 ++++++++++++++++++ tools/libxc/libxenguest.map | 37 +++++++++++++++++++++++++++++++++++++ tools/xenstore/Makefile | 5 +++-- tools/xenstore/libxenstore.map | 15 +++++++++++++++ 5 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 tools/libxc/libxenctrl.map create mode 100644 tools/libxc/libxenguest.map create mode 100644 tools/xenstore/libxenstore.map