On Fri, May 15, 2015 at 02:45:06PM +0100, John Garbutt wrote: > On 15 May 2015 at 13:28, Daniel P. Berrange <berra...@redhat.com> wrote: > > On Fri, May 15, 2015 at 11:51:22AM +0100, Daniel P. Berrange wrote: > >> On Thu, May 14, 2015 at 02:23:25PM -0500, Matt Riedemann wrote: > >> > There are some workarounds in the code [3] I'd like to see removed by > >> > bumping the minimum required version. > >> > >> Sure, its nice to remove workarounds from a cleanliness POV, but I'm > >> generally > >> pretty conservative about doing so, because in the majority of case (while > >> it > >> looks ugly) it is not really a significant burden on maintainers to keep it > >> around. > >> > >> This example is really just that. It certainly looks ugly, but we have the > >> code there now, it is doing the job for people who have that problem and it > >> isn't really having any measurable impact on our ability to maintain the > >> libvirt code. Removing this code won't lessen our maintainance burden in > >> any way, but it will unquestionably impact our users by removing support > >> for > >> the platform they may be currently deployed on. > > > > BTW, the code you quote here: > > > > > > http://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/host.py?id=2015.1.0#n754 > > > > is not actually working around a bug the libvirt hypervisor. It is in fact > > a bug in the libvirt-python API binding. As such we don't actually need to > > increase the min required libvirt to be able to remove that check. In fact > > increasing the min required libvirt is the wrong thing todo, because it is > > possible for someone to have the min required libvirt, but by accessing it > > via an older libvirt-python which still has the bug. > > > > So what's really needed is a dep on libvirt-python >= 1.2.0, not libvirt. > > > > We don't express min required versions for libvirt-python in the > > requirements.txt file though, since it is an optional package and we > > don't have any mechanism for recording min versions for those AFAIK. > > Does this mean we can drop the above [3] code? > https://github.com/openstack/requirements/blob/master/global-requirements.txt#L56
Hmm, I didn't know it was listed in global-requirements.txt - I only checked the requirements.txt and test-requirements.txt in Nova itself which does not list libvirt-python. Previously test-requirements.txt did have it, but we dropped it, since the unit tests now exclusively use fakelibvirt. To answer your question though, if global-requirements.txt is enforcing that we have libvirt-python 1.2.5, then we can drop that particular workaround. 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 :| _______________________________________________ OpenStack-operators mailing list OpenStack-operators@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-operators