Open Stack

On Fri, May 15, 2015 at 02:45:06PM +0100, John Garbutt wrote:
> On 15 May 2015 at 13:28, Daniel P. Berrange <berrange at 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 :|