From 2ce193e16d7ca5b93671d7f0d2e3b35761f5d386 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Thu, 22 Sep 2016 12:41:46 -0400 Subject: [PATCH] libvirt: ignore conflict when defining network filters We have a latent race in the libvirt firewall code when setting up static filters which is now an error with libvirt>=1.2.7, which is why we started seeing this in CI failures starting in newton which run on xenial nodes that have libvirt 1.3.1 (but didn't see it on trusty nodes with libvirt 1.2.2). Libvirt commit 46a811db0731cedaea0153fc223faa6096cee5b5 checks for an existing filter with the same name but a different uuid when defining network filters and raises an error if found. That was added in the 1.2.7 release. This change simply handles the error and ignores it so we don't fail to boot the instance. Unfortunately we don't have a specific error code from libvirt when this happens so the best we can do is compare the error message from the libvirt error which is only going to work for English locales because the error message from libvirt is translated. Change-Id: I161be26d605351f168e351d3ed3d308234346f6f Closes-Bug: #1612875 --- nova/tests/unit/virt/libvirt/test_firewall.py | 37 +++++++++++++++++++ nova/virt/libvirt/firewall.py | 16 +++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/nova/tests/unit/virt/libvirt/test_firewall.py b/nova/tests/unit/virt/libvirt/test_firewall.py index 27e9f6d40ded..3f5be4e294e3 100644 --- a/nova/tests/unit/virt/libvirt/test_firewall.py +++ b/nova/tests/unit/virt/libvirt/test_firewall.py @@ -713,3 +713,40 @@ class NWFilterTestCase(test.NoDBTestCase): self.assertEqual(2, debug.call_count) self.assertEqual(u"Cannot find UUID for filter '%(name)s': '%(e)s'", debug.call_args_list[0][0][0]) + + def test_define_filter_already_exists(self): + """Tests that we ignore a libvirt error when the nw filter already + exists for a given name. + """ + error = fakelibvirt.libvirtError('already exists') + error.err = (fakelibvirt.VIR_ERR_OPERATION_FAILED, None, + "filter 'nova-no-nd-reflection' already exists with uuid " + "e740c5ec-c715-4f73-9874-630cc73d4ac2",) + with mock.patch.object(self.fw._conn, 'nwfilterDefineXML', + side_effect=error) as define: + self.fw._define_filter(mock.sentinel.xml) + define.assert_called_once_with(mock.sentinel.xml) + + def test_define_filter_fails_wrong_message(self): + """Tests that we reraise the libvirt error for an operational failure + if the error message is something unexpected. + """ + error = fakelibvirt.libvirtError('already exists') + error.err = (fakelibvirt.VIR_ERR_OPERATION_FAILED, None, 'oops',) + with mock.patch.object(self.fw._conn, 'nwfilterDefineXML', + side_effect=error) as define: + self.assertRaises(fakelibvirt.libvirtError, + self.fw._define_filter, mock.sentinel.xml) + define.assert_called_once_with(mock.sentinel.xml) + + def test_define_filter_fails_wrong_code(self): + """Tests that we reraise the libvirt error for an operational failure + if the error code is something unexpected. + """ + error = fakelibvirt.libvirtError('already exists') + error.err = (fakelibvirt.VIR_ERR_OPERATION_TIMEOUT, None, 'timeout',) + with mock.patch.object(self.fw._conn, 'nwfilterDefineXML', + side_effect=error) as define: + self.assertRaises(fakelibvirt.libvirtError, + self.fw._define_filter, mock.sentinel.xml) + define.assert_called_once_with(mock.sentinel.xml) diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py index c3112de34172..207b3b945b16 100644 --- a/nova/virt/libvirt/firewall.py +++ b/nova/virt/libvirt/firewall.py @@ -20,6 +20,7 @@ import uuid from eventlet import greenthread from lxml import etree from oslo_log import log as logging +from oslo_utils import excutils from oslo_utils import importutils from nova.cloudpipe import pipelib @@ -259,7 +260,20 @@ class NWFilterFirewall(base_firewall.FirewallDriver): def _define_filter(self, xml): if callable(xml): xml = xml() - self._conn.nwfilterDefineXML(xml) + try: + self._conn.nwfilterDefineXML(xml) + except libvirt.libvirtError as ex: + with excutils.save_and_reraise_exception() as ctxt: + errcode = ex.get_error_code() + if errcode == libvirt.VIR_ERR_OPERATION_FAILED: + # Since libvirt 1.2.7 this operation can fail if the filter + # with the same name already exists for the given uuid. + # Unfortunately there is not a specific error code for this + # so we have to parse the error message to see if that was + # the failure. + errmsg = ex.get_error_message() + if 'already exists with uuid' in errmsg: + ctxt.reraise = False def unfilter_instance(self, instance, network_info): """Clear out the nwfilter rules."""