[PATCH] scripts: Make check-symfile.py work on alpha
The script expects each of the symbols that it looks for to be in one of three sections, which in nm(1) are described as follows: T - The symbol is in the text (code) section. B - The symbol is in the BSS data section. This section typically contains zero-initialized or uninitialized data, although the exact behavior is system dependent. D - The symbol is in the initialized data section. When building on alpha, however, some of the symbols show up in one of two additional sections, specifically: S - The symbol is in an uninitialized or zero-initialized data section for small objects. G - The symbol is in an initialized data section for small objects. In other words, S is the same as B and G is the same as D, except with some optimization for small objects that for some reason is applied on alpha but not on other architectures. I have confirmed that, for all the symbols that the script complained about being missing on alpha, the section is the expected one, that is, symbols that are reported as B on x86 are reported as S on alpha, and symbols that are reported as D on x86 are reported as G on alpha. Note that, while the B section doesn't seem to be used at all on alpha, at least in our case, the D section still is. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- scripts/check-symfile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py index 0f6e780df0..c2ee405118 100755 --- a/scripts/check-symfile.py +++ b/scripts/check-symfile.py @@ -61,7 +61,7 @@ for elflib in elflibs: for line in nm: line = line.decode("utf-8") - symmatch = re.search(r'''^\S+\s(?:[TBD])\s(\S+)\s*$''', line) + symmatch = re.search(r'''^\S+\s(?:[TBSDG])\s(\S+)\s*$''', line) if symmatch is None: continue -- 2.43.0
On Sat, Jan 20, 2024 at 11:30:10PM +0100, Andrea Bolognani wrote:
The script expects each of the symbols that it looks for to be in one of three sections, which in nm(1) are described as follows:
T - The symbol is in the text (code) section. B - The symbol is in the BSS data section. This section typically contains zero-initialized or uninitialized data, although the exact behavior is system dependent. D - The symbol is in the initialized data section.
When building on alpha, however, some of the symbols show up in one of two additional sections, specifically:
Again alpha is irrelevant as an architecture, so this patch is not needed. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Sun, Jan 21, 2024 at 07:50:02PM +0000, Daniel P. Berrangé wrote:
On Sat, Jan 20, 2024 at 11:30:10PM +0100, Andrea Bolognani wrote:
The script expects each of the symbols that it looks for to be in one of three sections, which in nm(1) are described as follows:
T - The symbol is in the text (code) section. B - The symbol is in the BSS data section. This section typically contains zero-initialized or uninitialized data, although the exact behavior is system dependent. D - The symbol is in the initialized data section.
When building on alpha, however, some of the symbols show up in one of two additional sections, specifically:
Again alpha is irrelevant as an architecture, so this patch is not needed.
Debian builds packages on alpha and, even though it's no longer considered a release architecture, it works just fine and keeps up with updated software components: $ uname -a Linux debian-alpha 6.6.8-alpha-generic #1 Debian 6.6.8-1 (2023-12-22) alpha GNU/Linux That's from a Debian installation running under qemu-system-alpha that I've created over the weekend. Bit slow, as one would expect when TCG is involved, but other than that perfectly functional. In fact, this tiny patch is all that's needed to get the Debian package for libvirt to build successfully. And it works too, at least when it comes to the client part: $ ip a show dev enp0s2 2: enp0s2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP group default qlen 1000 link/ether 52:54:00:fd:42:9b brd ff:ff:ff:ff:ff:ff inet 192.168.124.126/24 brd 192.168.124.255 scope global dynamic enp0s2 valid_lft 3379sec preferred_lft 3379sec inet6 fe80::5054:ff:fefd:429b/64 scope link proto kernel_ll valid_lft forever preferred_lft forever $ virsh -c qemu+ssh://192.168.124.1/system list Id Name State ------------------------------ 1 debian-alpha running $ virsh -c qemu+ssh://192.168.124.1/system dumpxml debian-alpha --xpath /domain/devices/interface/mac <mac address="52:54:00:fd:42:9b"/> Here I'm connecting to the host's libvirtd from within the guest, just for convenience. I could just as easily connect to a remote hypervisor. I could of course keep this patch as a downstream-only change in the Debian package, but I see no point in not accepting it upstream, especially since the maintenance burden it causes is literally zero. Please reconsider :) -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Jan 22, 2024 at 01:24:41AM -0800, Andrea Bolognani wrote:
On Sun, Jan 21, 2024 at 07:50:02PM +0000, Daniel P. Berrangé wrote:
On Sat, Jan 20, 2024 at 11:30:10PM +0100, Andrea Bolognani wrote:
The script expects each of the symbols that it looks for to be in one of three sections, which in nm(1) are described as follows:
T - The symbol is in the text (code) section. B - The symbol is in the BSS data section. This section typically contains zero-initialized or uninitialized data, although the exact behavior is system dependent. D - The symbol is in the initialized data section.
When building on alpha, however, some of the symbols show up in one of two additional sections, specifically:
Again alpha is irrelevant as an architecture, so this patch is not needed.
Debian builds packages on alpha and, even though it's no longer considered a release architecture, it works just fine and keeps up with updated software components:
$ uname -a Linux debian-alpha 6.6.8-alpha-generic #1 Debian 6.6.8-1 (2023-12-22) alpha GNU/Linux
That's from a Debian installation running under qemu-system-alpha that I've created over the weekend. Bit slow, as one would expect when TCG is involved, but other than that perfectly functional.
In fact, this tiny patch is all that's needed to get the Debian package for libvirt to build successfully. And it works too, at least when it comes to the client part:
$ ip a show dev enp0s2 2: enp0s2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP group default qlen 1000 link/ether 52:54:00:fd:42:9b brd ff:ff:ff:ff:ff:ff inet 192.168.124.126/24 brd 192.168.124.255 scope global dynamic enp0s2 valid_lft 3379sec preferred_lft 3379sec inet6 fe80::5054:ff:fefd:429b/64 scope link proto kernel_ll valid_lft forever preferred_lft forever
$ virsh -c qemu+ssh://192.168.124.1/system list Id Name State ------------------------------ 1 debian-alpha running
$ virsh -c qemu+ssh://192.168.124.1/system dumpxml debian-alpha --xpath /domain/devices/interface/mac <mac address="52:54:00:fd:42:9b"/>
Here I'm connecting to the host's libvirtd from within the guest, just for convenience. I could just as easily connect to a remote hypervisor.
I could of course keep this patch as a downstream-only change in the Debian package, but I see no point in not accepting it upstream, especially since the maintenance burden it causes is literally zero.
Please reconsider :)
Can you please take another look at this patch (as well as the other one that contains alpha-specific changes) in light of the arguments that I've provided above? Thanks. -- Andrea Bolognani / Red Hat / Virtualization
On 1/26/24 11:21, Andrea Bolognani wrote:
On Mon, Jan 22, 2024 at 01:24:41AM -0800, Andrea Bolognani wrote:
Please reconsider :)
Can you please take another look at this patch (as well as the other one that contains alpha-specific changes) in light of the arguments that I've provided above? Thanks.
I don't want to override Dan, but to me this makes sense. Unless he disagrees, you have my: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On Tue, Jan 30, 2024 at 12:39:29PM +0100, Michal Prívozník wrote:
On 1/26/24 11:21, Andrea Bolognani wrote:
On Mon, Jan 22, 2024 at 01:24:41AM -0800, Andrea Bolognani wrote:
Please reconsider :)
Can you please take another look at this patch (as well as the other one that contains alpha-specific changes) in light of the arguments that I've provided above? Thanks.
I don't want to override Dan, but to me this makes sense. Unless he disagrees, you have my:
Go for it. I still maintain no one should waste a second of effort on an architecture that was dead before libvirt even started as a project, but won't block this. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Jan 30, 2024 at 12:18:43PM +0000, Daniel P. Berrangé wrote:
On Tue, Jan 30, 2024 at 12:39:29PM +0100, Michal Prívozník wrote:
On 1/26/24 11:21, Andrea Bolognani wrote:
Can you please take another look at this patch (as well as the other one that contains alpha-specific changes) in light of the arguments that I've provided above? Thanks.
I don't want to override Dan, but to me this makes sense. Unless he disagrees, you have my:
Go for it.
I still maintain no one should waste a second of effort on an architecture that was dead before libvirt even started as a project, but won't block this.
Thanks, I appreciate that! Any chance either of you could look at the other patch[1] as well? The connection to alpha is sort of tangential in that case, what's more important is that we're currently not using -fstack-protector on aarch64 even though we should. [1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/R5C57... -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Jan 30, 2024 at 07:42:10 -0800, Andrea Bolognani wrote:
On Tue, Jan 30, 2024 at 12:18:43PM +0000, Daniel P. Berrangé wrote:
On Tue, Jan 30, 2024 at 12:39:29PM +0100, Michal Prívozník wrote:
On 1/26/24 11:21, Andrea Bolognani wrote:
Can you please take another look at this patch (as well as the other one that contains alpha-specific changes) in light of the arguments that I've provided above? Thanks.
I don't want to override Dan, but to me this makes sense. Unless he disagrees, you have my:
Go for it.
I still maintain no one should waste a second of effort on an architecture that was dead before libvirt even started as a project, but won't block this.
Thanks, I appreciate that!
Any chance either of you could look at the other patch[1] as well? The connection to alpha is sort of tangential in that case, what's more important is that we're currently not using -fstack-protector on aarch64 even though we should.
IMO there you should post a version which just enables -fstack-protector everywhere, as in ... drop the alpha hack. That one is starting to go beyond the 'trivial' change.
On Tue, Jan 30, 2024 at 05:36:03PM +0100, Peter Krempa wrote:
On Tue, Jan 30, 2024 at 07:42:10 -0800, Andrea Bolognani wrote:
Any chance either of you could look at the other patch[1] as well? The connection to alpha is sort of tangential in that case, what's more important is that we're currently not using -fstack-protector on aarch64 even though we should.
IMO there you should post a version which just enables -fstack-protector everywhere, as in ... drop the alpha hack. That one is starting to go beyond the 'trivial' change.
That's not my preference, clearly, but I'm okay doing that. While this patch is strictly necessary to make libvirt build successfully on alpha, the alpha-related part of the other one only removes a number of annoying, but ultimately harmless warnings. One last chance for someone to ACK that patch as-is. If not, I'll cave in and post an updated, alpha-free version :) -- Andrea Bolognani / Red Hat / Virtualization
On 1/30/24 18:04, Andrea Bolognani wrote:
On Tue, Jan 30, 2024 at 05:36:03PM +0100, Peter Krempa wrote:
On Tue, Jan 30, 2024 at 07:42:10 -0800, Andrea Bolognani wrote:
Any chance either of you could look at the other patch[1] as well? The connection to alpha is sort of tangential in that case, what's more important is that we're currently not using -fstack-protector on aarch64 even though we should.
IMO there you should post a version which just enables -fstack-protector everywhere, as in ... drop the alpha hack. That one is starting to go beyond the 'trivial' change.
That's not my preference, clearly, but I'm okay doing that.
While this patch is strictly necessary to make libvirt build successfully on alpha, the alpha-related part of the other one only removes a number of annoying, but ultimately harmless warnings.
One last chance for someone to ACK that patch as-is. If not, I'll cave in and post an updated, alpha-free version :)
While one could argue that this check-symfile.py patch is: a) trivial, b) useful for other arches than just alpha (possibly, in case linker decides to put memory into different places like S or G), it's hard to use the same argument for -fstack-protector IMO. Michal
On Tue, Jan 30, 2024 at 06:31:11PM +0100, Michal Prívozník wrote:
On 1/30/24 18:04, Andrea Bolognani wrote:
On Tue, Jan 30, 2024 at 05:36:03PM +0100, Peter Krempa wrote:
On Tue, Jan 30, 2024 at 07:42:10 -0800, Andrea Bolognani wrote:
Any chance either of you could look at the other patch[1] as well? The connection to alpha is sort of tangential in that case, what's more important is that we're currently not using -fstack-protector on aarch64 even though we should.
IMO there you should post a version which just enables -fstack-protector everywhere, as in ... drop the alpha hack. That one is starting to go beyond the 'trivial' change.
That's not my preference, clearly, but I'm okay doing that.
While this patch is strictly necessary to make libvirt build successfully on alpha, the alpha-related part of the other one only removes a number of annoying, but ultimately harmless warnings.
One last chance for someone to ACK that patch as-is. If not, I'll cave in and post an updated, alpha-free version :)
While one could argue that this check-symfile.py patch is: a) trivial, b) useful for other arches than just alpha (possibly, in case linker decides to put memory into different places like S or G), it's hard to use the same argument for -fstack-protector IMO.
Heard. v2 posted. -- Andrea Bolognani / Red Hat / Virtualization
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Michal Prívozník -
Peter Krempa