Skip to contentSkip to conversation

libblktapctl.so is unversioned

PATCH In review Olaf Hering 3 participants 7 messages Jul 28, 2010 – Aug 13, 2010

lore
Share & tools
lore
#0 lore raw

Hello,

In my builds /usr/sbin/tap-ctl and /usr/sbin/xl link against this shared lib. Is there a reason why libblktapctl.so has no SONAME and no version number, like libblktapctl.so.1?

Olaf

#1 lore raw

It ought to be versioned, especially since I think the interface is going to change to hide some OS dependencies.

K.

Quoted 14 lines · Olaf Hering

Hello,

In my builds /usr/sbin/tap-ctl and /usr/sbin/xl link against this shared lib. Is there a reason why libblktapctl.so has no SONAME and no version number, like libblktapctl.so.1?

Olaf

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#2 lore raw

PATCH 1/1 add SONAME to libblktapctl.so

Add an SONAME to libblktapctl.so.
Install static library as data to avoid executable permissions in the .a file.

---
 tools/blktap2/control/Makefile |   35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)
tools/blktap2/control/Makefile +25 -10
@@ -1,6 +1,11 @@
 XEN_ROOT := ../../../
 include $(XEN_ROOT)/tools/Rules.mk
 
+MAJOR              = 1.0
+MINOR              = 0
+LIBNAME            = libblktapctl
+LIBSONAME          = $(LIBNAME).so.$(MAJOR)
+
 IBIN               = tap-ctl
 
 CFLAGS            += -Werror
@@ -35,29 +40,39 @@ CTL_PICS  = $(patsubst %.o,%.opic,$(CTL_
 OBJS = $(CTL_OBJS) tap-ctl.o
 PICS = $(CTL_PICS)
 
-LIBS = libblktapctl.a libblktapctl.so
+LIB_STATIC = $(LIBNAME).a
+LIB_SHARED = $(LIBSONAME).$(MINOR)
 IBIN = tap-ctl
 
 all: build
 
-build: $(IBIN) $(LIBS)
+build: $(IBIN) $(LIB_STATIC) $(LIB_SHARED)
+
+$(LIBNAME).so: $(LIBSONAME)
+	ln -sf $< $@
+
+$(LIBSONAME): $(LIB_SHARED)
+	ln -sf $< $@
 
-tap-ctl: tap-ctl.o libblktapctl.so
-	$(CC) $(CFLAGS) -o $@ $^
+tap-ctl: tap-ctl.o $(LIBNAME).so
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^
 
-libblktapctl.a: $(CTL_OBJS)
+$(LIB_STATIC): $(CTL_OBJS)
 	$(AR) r $@ $^
 
-libblktapctl.so: $(CTL_PICS)
-	$(CC) $(CFLAGS) -fPIC -shared -rdynamic $^ -o $@
+$(LIB_SHARED): $(CTL_PICS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -fPIC  -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) -rdynamic $^ -o $@
 
-install: $(IBIN) $(LIBS)
+install: $(IBIN) $(LIB_STATIC) $(LIB_SHARED)
 	$(INSTALL_DIR) -p $(DESTDIR)$(SBINDIR)
 	$(INSTALL_PROG) $(IBIN) $(DESTDIR)$(SBINDIR)
-	$(INSTALL_PROG) $(LIBS) $(DESTDIR)$(LIBDIR)
+	$(INSTALL_DATA) $(LIB_STATIC) $(DESTDIR)$(LIBDIR)
+	$(INSTALL_PROG) $(LIB_SHARED) $(DESTDIR)$(LIBDIR)
+	ln -sf $(LIBSONAME) $(DESTDIR)$(LIBDIR)/$(LIBNAME).so
+	ln -sf $(LIB_SHARED) $(DESTDIR)$(LIBDIR)/$(LIBSONAME)
 
 clean:
-	rm -f $(OBJS) $(PICS) $(DEPS) $(IBIN) $(LIBS)
+	rm -f $(OBJS) $(PICS) $(DEPS) $(IBIN) $(LIB_STATIC) $(LIB_SHARED)
 	rm -f *~
 
 .PHONY: all build clean install
#3 lore raw

Re: [PATCH] add SONAME to libblktapctl.so

Olaf Hering writes ("[Xen-devel] [PATCH] add SONAME to libblktapctl.so"):

Quoted 2 lines

Add an SONAME to libblktapctl.so. Install static library as data to avoid executable permissions in the .a file.

Thanks. I have a few comments:

tools/blktap2/control/Makefile CTL_PICS = $(patsubst %.o,%.opic,$(CTL_ · line 64

Quoted 4 lines
tools/blktap2/control/Makefile +2 -2
-libblktapctl.so: $(CTL_PICS)
-	$(CC) $(CFLAGS) -fPIC -shared -rdynamic $^ -o $@
+$(LIB_SHARED): $(CTL_PICS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -fPIC  -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) -rdynamic $^ -o $@

I'm not sure I understand why -fPIC appears here as a literal, rather than being included via some more general make variable. And does it have any effect on the shared library creation link line anyway ? Other places that make shared libraries don't have -fPIC there.

Likewise they don't say -rdynamic.

I appreciate that you're not doing anything here that wasn't in the makefiles before. So really I'm just asking if you or anyone else knows anything about the reasons for this.

Thanks,
Ian.

#4 lore raw

Re: [PATCH] add SONAME to libblktapctl.so

Quoted 11 lines · Ian Jackson

tools/blktap2/control/Makefile CTL_PICS = $(patsubst %.o,%.opic,$(CTL_ · line 64

Quoted 4 lines
tools/blktap2/control/Makefile +2 -2
-libblktapctl.so: $(CTL_PICS)
-	$(CC) $(CFLAGS) -fPIC -shared -rdynamic $^ -o $@
+$(LIB_SHARED): $(CTL_PICS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -fPIC  -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) -rdynamic $^ -o $@

I'm not sure I understand why -fPIC appears here as a literal, rather than being included via some more general make variable. And does it have any effect on the shared library creation link line anyway ? Other places that make shared libraries don't have -fPIC there.

Likewise they don't say -rdynamic.

Ian,

maybe someone has over-done things here. I'm sure libblktapctl is most likely not different from all the other xen libraries. So I would just drop -fPIC and -rdynamic from the options.

The gcc manual states that linking should be done with the same options used to compile the individual objects, to get predictable results. So if for example -fPIC wasnt there before (in CFLAGS), it wont make the resulting lib any better.

Olaf

Acked-by
#5 lore raw

Re: [PATCH] add SONAME to libblktapctl.so

Olaf Hering writes ("Re: [Xen-devel] [PATCH] add SONAME to libblktapctl.so"):

Quoted 8 lines

maybe someone has over-done things here. I'm sure libblktapctl is most likely not different from all the other xen libraries. So I would just drop -fPIC and -rdynamic from the options.

The gcc manual states that linking should be done with the same options used to compile the individual objects, to get predictable results. So if for example -fPIC wasnt there before (in CFLAGS), it wont make the resulting lib any better.

That's a convincing explanation. In which case, as regards your patch,

and we should have a separate changeset to remove -fPIC and -rdynamic.

Ian.

#6 lore raw

Re: [PATCH] add SONAME to libblktapctl.so

Ian Jackson writes ("Re: [Xen-devel] [PATCH] add SONAME to libblktapctl.so"):

Quoted 6 lines

That's a convincing explanation. In which case, as regards your patch,

and we should have a separate changeset to remove -fPIC and -rdynamic.

And here that one is:

tools/blktap2: Remove unnecessary -fPIC and -rdynamic from link line

Where these options are needed they will be in the appropriate *FLAGS variables; they should not appear in specific Makefiles.

tools/blktap2/control/Makefile +1 -1
diff -r ddb9f47ef2e2 tools/blktap2/control/Makefile
@@ -61,7 +61,7 @@ tap-ctl: tap-ctl.o $(LIBNAME).so
 	$(AR) r $@ $^
 
 $(LIB_SHARED): $(CTL_PICS)
-	$(CC) $(CFLAGS) $(LDFLAGS) -fPIC  -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) -rdynamic $^ -o $@
+	$(CC) $(CFLAGS) $(LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,$(LIBSONAME) $(SHLIB_LDFLAGS) $^ -o $@
 
 install: $(IBIN) $(LIB_STATIC) $(LIB_SHARED)
 	$(INSTALL_DIR) -p $(DESTDIR)$(SBINDIR)