[libvirt] version script and non-existent symbols
The current version script refers to symbols that don't exist in the code. What is the purpose of this? The Solaris linker (quite correctly, I think) generates hanging references to such symbols. If it's just for documentation, such symbols should be commented out in the script. But there is another more basic problem. If I configure out various components, such as storage, which is necessary in order to compile, then /those/ symbols don't exist, and the build fails as above. I think the solution to the latter is to provide stubs that return failure codes for every symbol that can be ./configured out. So we'd introduce a stubs.c file that has these for everything, and compile each of the stubs if the relevant WITH_ option isn't set. Finally, C-style comments aren't supported with the Solaris linker. Since both linkers support # as a comment delimiter, any objections to changing it to do that? regards john
On Sat, Dec 13, 2008 at 05:03:33PM +0000, John Levon wrote:
The current version script refers to symbols that don't exist in the code. What is the purpose of this? The Solaris linker (quite correctly, I think) generates hanging references to such symbols.
That's a bug - can you tell me which you see as missing. We can just remove them.
But there is another more basic problem. If I configure out various components, such as storage, which is necessary in order to compile, then /those/ symbols don't exist, and the build fails as above.
I think the solution to the latter is to provide stubs that return failure codes for every symbol that can be ./configured out. So we'd introduce a stubs.c file that has these for everything, and compile each of the stubs if the relevant WITH_ option isn't set.
I think I'd be more inclined to split up the libvirt_sym.version.in file into pieces, one providing all the public API symbols, which would never change. Then for all the private symbols have them in a file matching the source file, eg for domain_conf.h, have a file domain_conf.sym. Finally have a rule to merge all the .sym files using the same #ifdef as we use to turn on/off various files eg, libvirt_SYMBOLS = libvirt_public.sym if WITH_XEN libvirt_SYMBOLS += xen_internal.sym xend_internal.sym endif if WITH_STORAGE libvirt_SYMBOLS += storage_conf.sym end
Finally, C-style comments aren't supported with the Solaris linker. Since both linkers support # as a comment delimiter, any objections to changing it to do that?
Fine by me. # is nicer anyway because you don't have to remember to terminate it :-) Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Sat, Dec 13, 2008 at 06:19:52PM +0000, Daniel P. Berrange wrote:
I think the solution to the latter is to provide stubs that return failure codes for every symbol that can be ./configured out. So we'd introduce a stubs.c file that has these for everything, and compile each of the stubs if the relevant WITH_ option isn't set.
I think I'd be more inclined to split up the libvirt_sym.version.in file into pieces, one providing all the public API symbols, which would never change. Then for all the private symbols have them in a file matching the source file, eg for domain_conf.h, have a file domain_conf.sym. Finally have a rule to merge all the .sym files using the same #ifdef as we use to turn on/off various files
It's actually not clear to me why some of these are even in the version script. For example, why is brSetEnableSTP() there? regards john
On Mon, Dec 15, 2008 at 08:50:33PM +0000, John Levon wrote:
On Sat, Dec 13, 2008 at 06:19:52PM +0000, Daniel P. Berrange wrote:
I think the solution to the latter is to provide stubs that return failure codes for every symbol that can be ./configured out. So we'd introduce a stubs.c file that has these for everything, and compile each of the stubs if the relevant WITH_ option isn't set.
I think I'd be more inclined to split up the libvirt_sym.version.in file into pieces, one providing all the public API symbols, which would never change. Then for all the private symbols have them in a file matching the source file, eg for domain_conf.h, have a file domain_conf.sym. Finally have a rule to merge all the .sym files using the same #ifdef as we use to turn on/off various files
It's actually not clear to me why some of these are even in the version script. For example, why is brSetEnableSTP() there?
It is used from the network_driver.c file, and that is linked to the daemon, while brSetEnableSTP is part of the .so, so it has to be added to the private exports section. About 70% of them are there for that reason. Then, if we build drivers as dlopen()'able modules, even more needed to be exported in the private section. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, Dec 15, 2008 at 09:32:03PM +0000, Daniel P. Berrange wrote:
It's actually not clear to me why some of these are even in the version script. For example, why is brSetEnableSTP() there?
It is used from the network_driver.c file, and that is linked to the daemon
Hmm I'd missed that. A weird source code setup?
reason. Then, if we build drivers as dlopen()'able modules, even more
Oh, I forgot about that. OK. What is the purpose of the dlopen() version? Are people to be expected to deliver modules separately? Is it just a minimisation thing? regards john
On Mon, Dec 15, 2008 at 09:51:16PM +0000, John Levon wrote:
On Mon, Dec 15, 2008 at 09:32:03PM +0000, Daniel P. Berrange wrote:
It's actually not clear to me why some of these are even in the version script. For example, why is brSetEnableSTP() there?
It is used from the network_driver.c file, and that is linked to the daemon
Hmm I'd missed that. A weird source code setup?
Yeah, its a little wierd, but then moving source files around in CVS is not too pleasant either. Ultimately the dlopen() option makes is neccessary regardless
reason. Then, if we build drivers as dlopen()'able modules, even more
Oh, I forgot about that. OK. What is the purpose of the dlopen() version? Are people to be expected to deliver modules separately? Is it just a minimisation thing?
No, it doesn't allow for 3rd party modules, because we fix the list of modules we probe for, and also guarentee ABI incompatability in the internal module interface in every release. It was basically to allow smaller footprint deployments, and allow sysadmins to guarentee that undesired drivers will never be activated on a host even if the distro maintainer wants to compile all of them by default. For example, if you've compiled it in, the UserModeLinux driver will always activate itself, but many people won't want it around. That said, we're not actively using dlopen() currently... Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Daniel P. Berrange -
John Levon