[libvirt] RFC: spec file cleanup ideas

Hi all, I wanted to float some ideas about cleaning up the RPM spec file. None of it is urgent but I'll end up scratching the itch eventually and I want to make sure people are on board Old distro bits: * Drop support for building on rhel5... this was ACKd previously by eblake: http://www.redhat.com/archives/libvir-list/2015-November/msg00175.html * Drop support for end-of-life Fedora. There's lots of Fedora conditionals going back to Fedora 13. I don't think it's really interesting to try to support building an RPM on a distribution that is no longer receiving updates. But we could be a bit conservative and give a couple release window, dropping support for less than Fedora 20 for example (which went EOL in June 2015) Configuration variables: some of these could still feasibly be 'useful' even if they aren't programmatically set by any distro conditional, but I think unless someone actively uses them we should seek to have as few conditionals as possible, rather than conditionalize everything by default which seems more common. Some ideas: * Drop with_* conditionals that are only used by old Fedora or RHEL5. I didn't audit all of them but the obvious ones are straight forward like: with_systemd_macros, with_polkit, with_capng, with_netcf, with_yajl, with_capng, with_avahi, with_hal/all HAL support * Drop client_only/server_drivers/with_libvirtd ... do these serve any good purpose? (once the rhel5 bits are gone) * Drop with_driver_modules: it's unconditionally enabled and has been since it was introduced Old spec bits cleanup: * drop defattr(), it's long since not needed * drop manual removal of the buildroot, it's long since not needed * use %global consistently instead of %define * consider using %{with/without X} pattern instead of defining variables as with_X * The top of the spec has this block: # If neither fedora nor rhel was defined, try to guess them from dist %if !0%{?rhel} && !0%{?fedora} %{expand:%(echo "%{?dist}" | \ sed -ne 's/^\.el\([0-9]\+\).*/%%define rhel \1/p')} %{expand:%(echo "%{?dist}" | \ sed -ne 's/^\.fc\?\([0-9]\+\).*/%%define fedora \1/p')} %endif Is it actually still relevant? It was added 5 years ago... I don't know in what case rhel or fedora macros won't be defined

On Fri, 2016-04-08 at 19:13 -0400, Cole Robinson wrote:
Hi all, I wanted to float some ideas about cleaning up the RPM spec file. None of it is urgent but I'll end up scratching the itch eventually and I want to make sure people are on board
So I actually just skimmed your proposal... I think the idea of dropping from the .spec file stuff that we've already dropped from the build system is a sound one, so unless there are concerns about specific bits I'd say we should move forward with this. On the other hand... Why do we ship the .spec file in the first place? Is Fedora, or any other distribution for that matter, using it as-is? Or is it meant to be just for developers' convenience? Personally, I'd be happy having to maintain a single build system and leaving downstream packaging to distributions. But maybe that's just a consequence of my background in Debian packaging :) Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team

On 04/15/2016 06:03 AM, Andrea Bolognani wrote:
On Fri, 2016-04-08 at 19:13 -0400, Cole Robinson wrote:
Hi all,
I wanted to float some ideas about cleaning up the RPM spec file. None of it is urgent but I'll end up scratching the itch eventually and I want to make sure people are on board
So I actually just skimmed your proposal... I think the idea of dropping from the .spec file stuff that we've already dropped from the build system is a sound one, so unless there are concerns about specific bits I'd say we should move forward with this.
On the other hand... Why do we ship the .spec file in the first place? Is Fedora, or any other distribution for that matter, using it as-is? Or is it meant to be just for developers' convenience?
Personally, I'd be happy having to maintain a single build system and leaving downstream packaging to distributions. But maybe that's just a consequence of my background in Debian packaging :)
We do actually share the spec file between RHEL and Fedora with local changes largely only for changelog and adding patches. I love the way it works for libvirt because it distributes the burden of maintaining packaging among the whole team (at least those employed by redhat). Especially when we needed to build for RHEL5 up to latest Fedora. It also allows for 'make rpm' which is handy for a whole bunch of reasons, especially for new contributors on fedora/rhel who want to test a bug fix without having to figure out the proper configure invocation to get a working setup. - Cole

On Fri, 2016-04-15 at 06:40 -0400, Cole Robinson wrote:
On the other hand... Why do we ship the .spec file in the first place? Is Fedora, or any other distribution for that matter, using it as-is? Or is it meant to be just for developers' convenience?
Personally, I'd be happy having to maintain a single build system and leaving downstream packaging to distributions. But maybe that's just a consequence of my background in Debian packaging :)
We do actually share the spec file between RHEL and Fedora with local changes largely only for changelog and adding patches. I love the way it works for libvirt because it distributes the burden of maintaining packaging among the whole team (at least those employed by redhat). Especially when we needed to build for RHEL5 up to latest Fedora.
It also allows for 'make rpm' which is handy for a whole bunch of reasons, especially for new contributors on fedora/rhel who want to test a bug fix without having to figure out the proper configure invocation to get a working setup.
Very well then. Thanks for explaining! Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team

On Fri, Apr 08, 2016 at 07:13:07PM -0400, Cole Robinson wrote:
Hi all,
I wanted to float some ideas about cleaning up the RPM spec file. None of it is urgent but I'll end up scratching the itch eventually and I want to make sure people are on board
Old distro bits:
* Drop support for building on rhel5... this was ACKd previously by eblake: http://www.redhat.com/archives/libvir-list/2015-November/msg00175.html
Yep
* Drop support for end-of-life Fedora. There's lots of Fedora conditionals going back to Fedora 13. I don't think it's really interesting to try to support building an RPM on a distribution that is no longer receiving updates. But we could be a bit conservative and give a couple release window, dropping support for less than Fedora 20 for example (which went EOL in June 2015)
There's a surprising/disappointing number of people who carry on running EOL Fedora releases, so I think we should definitely be conservative in this respect. 1 year grace after Fedora EOL seems sufficient though, so dropping less than F20 would be OK.
Configuration variables: some of these could still feasibly be 'useful' even if they aren't programmatically set by any distro conditional, but I think unless someone actively uses them we should seek to have as few conditionals as possible, rather than conditionalize everything by default which seems more common. Some ideas:
Every conditional we have increases potential for bugs in the spec file and I'm pretty sure we have some in there, because no one ever tests most of these conditionals when disabled.
* Drop with_* conditionals that are only used by old Fedora or RHEL5. I didn't audit all of them but the obvious ones are straight forward like: with_systemd_macros, with_polkit, with_capng, with_netcf, with_yajl, with_capng, with_avahi, with_hal/all HAL support
For that matter I think we can drop HAL *code* entirely since it was EOL after RHEL5, replaced by the udev driver.
* Drop client_only/server_drivers/with_libvirtd ... do these serve any good purpose? (once the rhel5 bits are gone)
The client stuff I'm a bit less sure about. The original rational for this was that RHEL was only shipping libvirt with hypervisor drivers enabled on certain architectures, and we wanted to enable other architectures todo a client only build. eg to allow an ppc64 client to talk to a x86_64 host. Now that we have LXC enabled unconditionally on all archs though, there's probably not a hugely compelling reason for a client only build anymore.
* Drop with_driver_modules: it's unconditionally enabled and has been since it was introduced
Yep, that was there in case we needed to turn off drive modules for compatilbity reasons on existing RHEL platforms. In the end we just switched RHEL over to use them anyway, so they don't serve much use.
Old spec bits cleanup:
* drop defattr(), it's long since not needed * drop manual removal of the buildroot, it's long since not needed * use %global consistently instead of %define * consider using %{with/without X} pattern instead of defining variables as with_X
Assuming %global is supported by RHEL6, its fine.
* The top of the spec has this block:
# If neither fedora nor rhel was defined, try to guess them from dist %if !0%{?rhel} && !0%{?fedora} %{expand:%(echo "%{?dist}" | \ sed -ne 's/^\.el\([0-9]\+\).*/%%define rhel \1/p')} %{expand:%(echo "%{?dist}" | \ sed -ne 's/^\.fc\?\([0-9]\+\).*/%%define fedora \1/p')} %endif
Is it actually still relevant? It was added 5 years ago... I don't know in what case rhel or fedora macros won't be defined
In RHEL5 vintage build roots, %rhel/%fedora were not defined IIRC. No longer an issue. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Thanks for the comments On 04/15/2016 06:14 AM, Daniel P. Berrange wrote:
* Drop with_* conditionals that are only used by old Fedora or RHEL5. I didn't audit all of them but the obvious ones are straight forward like: with_systemd_macros, with_polkit, with_capng, with_netcf, with_yajl, with_capng, with_avahi, with_hal/all HAL support
For that matter I think we can drop HAL *code* entirely since it was EOL after RHEL5, replaced by the udev driver.
Maybe BSD still uses it? CCing Roman Also Maxim fixed a WITH_HAL build issue in November, not sure if they are using it. CCed - Cole

Cole Robinson wrote:
Thanks for the comments
On 04/15/2016 06:14 AM, Daniel P. Berrange wrote:
* Drop with_* conditionals that are only used by old Fedora or RHEL5. I didn't audit all of them but the obvious ones are straight forward like: with_systemd_macros, with_polkit, with_capng, with_netcf, with_yajl, with_capng, with_avahi, with_hal/all HAL support
For that matter I think we can drop HAL *code* entirely since it was EOL after RHEL5, replaced by the udev driver.
Maybe BSD still uses it? CCing Roman
Also Maxim fixed a WITH_HAL build issue in November, not sure if they are using it. CCed
HAL nodedev driver builds fine on FreeBSD and basic stuff like nodedev-list or nodedev-dumpxml works, but that's the only things that I test for nodedev. Also, I don't actually know what are the usage scenarios for the nodedev APIs, I assume the main function is to use it to manage devices passthrough to guests. For that case I doubt that important things like device detaching work on FreeBSD, but probably I'm wrong because I didn't look closer at this. Also, I didn't test PCI passthrough on FreeBSD at all. Anyway, once somebody gets to implement passthrough related things for libvirt on FreeBSD, probably it'd be good to base these things on the existing HAL driver. On the other hand, I'm not aware if somebody's going to implement it soon and it's not on the top of my todo list either, so it would not be a huge lost if it gets removed (not hard to revive it when needed). Roman Bogorodskiy

On Fri, Apr 15, 2016 at 02:40:29PM +0300, Roman Bogorodskiy wrote:
Cole Robinson wrote:
Thanks for the comments
On 04/15/2016 06:14 AM, Daniel P. Berrange wrote:
* Drop with_* conditionals that are only used by old Fedora or RHEL5. I didn't audit all of them but the obvious ones are straight forward like: with_systemd_macros, with_polkit, with_capng, with_netcf, with_yajl, with_capng, with_avahi, with_hal/all HAL support
For that matter I think we can drop HAL *code* entirely since it was EOL after RHEL5, replaced by the udev driver.
Maybe BSD still uses it? CCing Roman
Also Maxim fixed a WITH_HAL build issue in November, not sure if they are using it. CCed
HAL nodedev driver builds fine on FreeBSD and basic stuff like nodedev-list or nodedev-dumpxml works, but that's the only things that I test for nodedev.
Also, I don't actually know what are the usage scenarios for the nodedev APIs, I assume the main function is to use it to manage devices passthrough to guests. For that case I doubt that important things like device detaching work on FreeBSD, but probably I'm wrong because I didn't look closer at this. Also, I didn't test PCI passthrough on FreeBSD at all.
Anyway, once somebody gets to implement passthrough related things for libvirt on FreeBSD, probably it'd be good to base these things on the existing HAL driver.
On the other hand, I'm not aware if somebody's going to implement it soon and it's not on the top of my todo list either, so it would not be a huge lost if it gets removed (not hard to revive it when needed).
Ok, I didn't realize HAL had been ported to FreeBSD. I assume the udev driver isn't going to be much use for FreeBSD, so we should certainly *not* remove HAL for as long as its used on FreeBSD as that would be a feature regression. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 04/15/2016 07:40 AM, Roman Bogorodskiy wrote:
Cole Robinson wrote:
Thanks for the comments
On 04/15/2016 06:14 AM, Daniel P. Berrange wrote:
* Drop with_* conditionals that are only used by old Fedora or RHEL5. I didn't audit all of them but the obvious ones are straight forward like: with_systemd_macros, with_polkit, with_capng, with_netcf, with_yajl, with_capng, with_avahi, with_hal/all HAL support
For that matter I think we can drop HAL *code* entirely since it was EOL after RHEL5, replaced by the udev driver.
Maybe BSD still uses it? CCing Roman
Also Maxim fixed a WITH_HAL build issue in November, not sure if they are using it. CCed
HAL nodedev driver builds fine on FreeBSD and basic stuff like nodedev-list or nodedev-dumpxml works, but that's the only things that I test for nodedev.
Also, I don't actually know what are the usage scenarios for the nodedev APIs, I assume the main function is to use it to manage devices passthrough to guests. For that case I doubt that important things like device detaching work on FreeBSD, but probably I'm wrong because I didn't look closer at this. Also, I didn't test PCI passthrough on FreeBSD at all.
Anyway, once somebody gets to implement passthrough related things for libvirt on FreeBSD, probably it'd be good to base these things on the existing HAL driver.
virt-manager uses the nodedev APIs to enumerate PCI (passthrough), USB (passthrough), physical network devices (macvtap, network and interface creation), and scsi devices (storage pool creation). - Cole
participants (4)
-
Andrea Bolognani
-
Cole Robinson
-
Daniel P. Berrange
-
Roman Bogorodskiy