← Back to team overview

yahoo-eng-team team mailing list archive

[Bug 1783551] Re: _set_config_VIFGeneric should accept os-vif object

 

Reviewed:  https://review.openstack.org/571461
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=6bfb1959184d6e262e63c25e9b1ef7d1fdf63a8e
Submitter: Zuul
Branch:    master

commit 6bfb1959184d6e262e63c25e9b1ef7d1fdf63a8e
Author: Jan Gutter <jan.gutter@xxxxxxxxxxxxx>
Date:   Thu May 31 13:10:50 2018 +0200

    Use vif.vif_name in _set_config_VIFGeneric
    
    During IVS os-vif migration, _set_config_VIFGeneric was added:
    https://review.openstack.org/#/c/534371/
    
    It appears that some legacy plugging code still slipped through, since
    the preferred method to pass the VIF name (i.e. device name) is via the
    os-vif object. Crucially, this is not a Nova VIF object, so it has
    different fields.
    
    * This change makes _set_config_VIFGeneric behave the way other os-vif
      methods do, by taking the name from the os-vif object.
    * This change adds back some VIF unit tests that would have caught this.
    
    Closes-Bug: #1783551
    Change-Id: I468a9e78c948209cc2fde795ffffa0e99906fbef
    Signed-off-by: Jan Gutter <jan.gutter@xxxxxxxxxxxxx>


** Changed in: nova
       Status: In Progress => Fix Released

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1783551

Title:
  _set_config_VIFGeneric should accept os-vif object

Status in OpenStack Compute (nova):
  Fix Released

Bug description:
  Description
  ===========

  While working on adjacent codepaths exercising
  nova.virt.libvirt.vif._set_config_VIFGeneric I discovered that it
  appears to expect a Nova VIF object instead of an os-vif object. Since
  the _set_config_* functions are called to convert os-vif objects to
  libvirt xml config, this trips up. It's not caught in production or
  tests because the tests were removed (mistakenly thought to be
  dangling). For example, re-introducing a test shows the issue:

   nova.tests.unit.virt.libvirt.test_vif.LibvirtVifTestCase.test_ivs_ethernet_driver
   ---------------------------------------------------------------------------------

   Captured traceback:
   ~~~~~~~~~~~~~~~~~~~
       Traceback (most recent call last):
         File "nova/tests/unit/virt/libvirt/test_vif.py", line 1395, in test_ivs_ethernet_driver
           xml = self._get_instance_xml(d, self.vif_ivs)
         File "nova/tests/unit/virt/libvirt/test_vif.py", line 572, in _get_instance_xml
           hostimpl)
         File "nova/virt/libvirt/vif.py", line 583, in get_config
           vnic_type)
         File "nova/virt/libvirt/vif.py", line 555, in _get_config_os_vif
           func(instance, vif, conf, host)
         File "nova/virt/libvirt/vif.py", line 463, in _set_config_VIFGeneric
           dev = self.get_vif_devname(vif)
         File "nova/virt/libvirt/vif.py", line 108, in get_vif_devname
           return ("nic" + vif['id'])[:network_model.NIC_NAME_LEN]
       TypeError: 'VIFGeneric' object has no attribute '__getitem__'

  This happened in https://review.openstack.org/#/c/534371/ during the
  Rocky cycle, so it's likely that the impact was not experienced
  externally (or tested much).

  Steps to Reproduce
  ==================

  It's likely to hit practice if the third-party IVS os-vif plugin
  exercises this codepath. Other than that, the way to reproduce this
  would be to add back the tests.

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/1783551/+subscriptions


References