diff --git a/.zuul.yaml b/.zuul.yaml index f80a9eba226..70ecef6e906 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -118,6 +118,17 @@ python_version: 3.7 timeout: 3600 +- job: + name: nova-tox-validate-backport + parent: openstack-tox + description: | + Determine whether a backport is ready to be merged by checking whether it + has already been merged to master or more recent stable branches. + + Uses tox with the ``validate-backport`` environment. + vars: + tox_envlist: validate-backport + - job: name: nova-live-migration parent: nova-dsvm-multinode-base @@ -393,6 +404,8 @@ - nova-next - nova-tox-functional - nova-tox-functional-py36 + - nova-tox-validate-backport: + voting: false - tempest-integrated-compute: irrelevant-files: *dsvm-irrelevant-files - tempest-slow-py3: @@ -409,6 +422,7 @@ - nova-tox-functional-py36 - nova-multi-cell - nova-next + - nova-tox-validate-backport - tempest-integrated-compute: irrelevant-files: *dsvm-irrelevant-files - tempest-slow-py3: diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 2a0858f09ab..8bc699f575e 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -2449,9 +2449,10 @@ disk_available_least_total: disk_bus: description: | Disk bus type, some hypervisors (currently only libvirt) support - specify this parameter. Some example disk_bus values can be: `ide`, - `usb`, `virtio`, `scsi`. This is not an exhaustive list as it depends - on the virtualization driver, and may change as more support is added. + specify this parameter. Some example disk_bus values can be: ``fdc``, + ``ide``, ``sata``, ``scsi``, ``usb``, ``virtio``, ``xen``, ``lxc`` + and ``uml``. Support for each bus type depends on the virtualization driver + and underlying hypervisor. in: body required: false type: string diff --git a/doc/source/admin/pci-passthrough.rst b/doc/source/admin/pci-passthrough.rst index 52388db53a3..48abf1b2032 100644 --- a/doc/source/admin/pci-passthrough.rst +++ b/doc/source/admin/pci-passthrough.rst @@ -15,156 +15,188 @@ as multiple PCI devices. Virtual PCI devices are assigned to the same or different guests. In the case of PCI passthrough, the full physical device is assigned to only one guest and cannot be shared. +PCI devices are requested through flavor extra specs, specifically via the +``pci_passthrough:alias=`` flavor extra spec. This guide demonstrates +how to enable PCI passthrough for a type of PCI device with a vendor ID of +``8086`` and a product ID of ``154d`` - an Intel X520 Network Adapter - by +mapping them to the alias ``a1``. You should adjust the instructions for other +devices with potentially different capabilities. + .. note:: - For information on creating servers with virtual SR-IOV devices, refer to + For information on creating servers with SR-IOV network interfaces, refer to the :neutron-doc:`Networking Guide `. **Limitations** - * Attaching SR-IOV ports to existing servers is not currently supported, see - `bug 1708433 `_ for details. + * Attaching SR-IOV ports to existing servers is not currently supported. + This is now rejected by the API but previously it fail later in the + process. See `bug 1708433 `_ + for details. * Cold migration (resize) of servers with SR-IOV devices attached was not supported until the 14.0.0 Newton release, see `bug 1512800 `_ for details. -To enable PCI passthrough, follow the steps below: -#. Configure nova-scheduler (Controller) +Configure host (Compute) +------------------------ -#. Configure nova-api (Controller)** +To enable PCI passthrough on an x86, Linux-based compute node, the following +are required: -#. Configure a flavor (Controller) +* VT-d enabled in the BIOS +* IOMMU enabled on the host OS, e.g. by adding the ``intel_iommu=on`` or + ``amd_iommu=on`` parameter to the kernel parameters +* Assignable PCIe devices -#. Enable PCI passthrough (Compute) +To enable PCI passthrough on a Hyper-V compute node, the following are +required: -#. Configure PCI devices in nova-compute (Compute) +* Windows 10 or Windows / Hyper-V Server 2016 or newer +* VT-d enabled on the host +* Assignable PCI devices -.. note:: +In order to check the requirements above and if there are any assignable PCI +devices, run the following Powershell commands: - The PCI device with address ``0000:41:00.0`` is used as an example. This - will differ between environments. +.. code-block:: console -Configure nova-scheduler (Controller) -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + Start-BitsTransfer https://raw.githubusercontent.com/Microsoft/Virtualization-Documentation/master/hyperv-samples/benarm-powershell/DDA/survey-dda.ps1 + .\survey-dda.ps1 -#. Configure ``nova-scheduler`` as specified in :neutron-doc:`Configure - nova-scheduler - `. +If the compute node passes all the requirements, the desired assignable PCI +devices to be disabled and unmounted from the host, in order to be assignable +by Hyper-V. The following can be read for more details: `Hyper-V PCI +passthrough`__. -#. Restart the ``nova-scheduler`` service. +.. __: https://devblogs.microsoft.com/scripting/passing-through-devices-to-hyper-v-vms-by-using-discrete-device-assignment/ -Configure nova-api (Controller) -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -#. Specify the PCI alias for the device. +Configure ``nova-compute`` (Compute) +------------------------------------ - Configure a PCI alias ``a1`` to request a PCI device with a ``vendor_id`` of - ``0x8086`` and a ``product_id`` of ``0x154d``. The ``vendor_id`` and - ``product_id`` correspond the PCI device with address ``0000:41:00.0``. +Once PCI passthrough has been configured for the host, :program:`nova-compute` +must be configured to allow the PCI device to pass through to VMs. This is done +using the :oslo.config:option:`pci.passthrough_whitelist` option. For example, +assuming our sample PCI device has a PCI address of ``41:00.0`` on each host: - Edit ``/etc/nova/nova.conf``: +.. code-block:: ini - .. code-block:: ini + [pci] + passthrough_whitelist = { "address": "0000:41:00.0" } - [pci] - alias = { "vendor_id":"8086", "product_id":"154d", "device_type":"type-PF", "name":"a1" } +Refer to :oslo.config:option:`pci.passthrough_whitelist` for syntax information. - Refer to :oslo.config:option:`pci.alias` for syntax information. +Alternatively, to enable passthrough of all devices with the same product and +vendor ID: -#. Restart the ``nova-api`` service. +.. code-block:: ini -Configure a flavor (Controller) -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + [pci] + passthrough_whitelist = { "vendor_id": "8086", "product_id": "154d" } -Configure a flavor to request two PCI devices, each with ``vendor_id`` of -``0x8086`` and ``product_id`` of ``0x154d``: +If using vendor and product IDs, all PCI devices matching the ``vendor_id`` and +``product_id`` are added to the pool of PCI devices available for passthrough +to VMs. -.. code-block:: console +In addition, it is necessary to configure the :oslo.config:option:`pci.alias` +option, which is a JSON-style configuration option that allows you to map a +given device type, identified by the standard PCI ``vendor_id`` and (optional) +``product_id`` fields, to an arbitrary name or *alias*. This alias can then be +used to request a PCI device using the ``pci_passthrough:alias=`` flavor +extra spec, as discussed previously. For our sample device with a vendor ID of +``0x8086`` and a product ID of ``0x154d``, this would be: - # openstack flavor set m1.large --property "pci_passthrough:alias"="a1:2" +.. code-block:: ini -For more information about the syntax for ``pci_passthrough:alias``, refer to -:ref:`Flavors `. + [pci] + alias = { "vendor_id":"8086", "product_id":"154d", "device_type":"type-PF", "name":"a1" } -Enable PCI passthrough (Compute) -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +It's important to note the addition of the ``device_type`` field. This is +necessary because this PCI device supports SR-IOV. The ``nova-compute`` service +categorizes devices into one of three types, depending on the capabilities the +devices report: -Enable VT-d and IOMMU. For more information, refer to steps one and two in -:neutron-doc:`Create Virtual Functions -`. +``type-PF`` + The device supports SR-IOV and is the parent or root device. -For Hyper-V compute nodes, the requirements are as follows: +``type-VF`` + The device is a child device of a device that supports SR-IOV. -* Windows 10 or Windows / Hyper-V Server 2016 or newer. -* VT-d and SR-IOV enabled on the host. -* Assignable PCI devices. +``type-PCI`` + The device does not support SR-IOV. -In order to check the requirements above and if there are any assignable PCI -devices, run the following Powershell commands: +By default, it is only possible to attach ``type-PCI`` devices using PCI +passthrough. If you wish to attach ``type-PF`` or ``type-VF`` devices, you must +specify the ``device_type`` field in the config option. If the device was a +device that did not support SR-IOV, the ``device_type`` field could be omitted. -.. code-block:: console +Refer to :oslo.config:option:`pci.alias` for syntax information. - Start-BitsTransfer https://raw.githubusercontent.com/Microsoft/Virtualization-Documentation/master/hyperv-samples/benarm-powershell/DDA/survey-dda.ps1 - .\survey-dda.ps1 +.. important:: -If the compute node passes all the requirements, the desired assignable PCI -devices to be disabled and unmounted from the host, in order to be assignable -by Hyper-V. The following can be read for more details: `Hyper-V PCI -passthrough`__. + This option must also be configured on controller nodes. This is discussed later + in this document. -.. __: https://devblogs.microsoft.com/scripting/passing-through-devices-to-hyper-v-vms-by-using-discrete-device-assignment/ +Once configured, restart the :program:`nova-compute` service. + + +Configure ``nova-scheduler`` (Controller) +----------------------------------------- + +The :program:`nova-scheduler` service must be configured to enable the +``PciPassthroughFilter``. To do this, add this filter to the list of filters +specified in :oslo.config:option:`filter_scheduler.enabled_filters` and set +:oslo.config:option:`filter_scheduler.available_filters` to the default of +``nova.scheduler.filters.all_filters``. For example: -Configure PCI devices (Compute) -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +.. code-block:: ini -#. Configure ``nova-compute`` to allow the PCI device to pass through to - VMs. Edit ``/etc/nova/nova.conf``: + [filter_scheduler] + enabled_filters = ...,PciPassthroughFilter + available_filters = nova.scheduler.filters.all_filters - .. code-block:: ini +Once done, restart the :program:`nova-scheduler` service. - [pci] - passthrough_whitelist = { "address": "0000:41:00.0" } - Alternatively specify multiple PCI devices using whitelisting: +.. _pci-passthrough-alias: - .. code-block:: ini +Configure ``nova-api`` (Controller) +----------------------------------- - [pci] - passthrough_whitelist = { "vendor_id": "8086", "product_id": "10fb" } +It is necessary to also configure the :oslo.config:option:`pci.alias` config +option on the controller. This configuration should match the configuration +found on the compute nodes. For example: - All PCI devices matching the ``vendor_id`` and ``product_id`` are added to - the pool of PCI devices available for passthrough to VMs. +.. code-block:: ini - Refer to :oslo.config:option:`pci.passthrough_whitelist` for syntax - information. + [pci] + alias = { "vendor_id":"8086", "product_id":"154d", "device_type":"type-PF", "name":"a1" } -#. Specify the PCI alias for the device. +Refer to :oslo.config:option:`pci.alias` for syntax information. - From the Newton release, to resize guest with PCI device, configure the PCI - alias on the compute node as well. +Once configured, restart the :program:`nova-api` service. - Configure a PCI alias ``a1`` to request a PCI device with a ``vendor_id`` of - ``0x8086`` and a ``product_id`` of ``0x154d``. The ``vendor_id`` and - ``product_id`` correspond the PCI device with address ``0000:41:00.0``. - Edit ``/etc/nova/nova.conf``: +Configure a flavor (API) +------------------------ - .. code-block:: ini +Once the alias has been configured, it can be used for an flavor extra spec. +For example, to request two of the PCI devices referenced by alias ``a1``, run: - [pci] - alias = { "vendor_id":"8086", "product_id":"154d", "device_type":"type-PF", "name":"a1" } +.. code-block:: console + + $ openstack flavor set m1.large --property "pci_passthrough:alias"="a1:2" - Refer to :oslo.config:option:`pci.alias` for syntax information. +For more information about the syntax for ``pci_passthrough:alias``, refer to +:ref:`Flavors `. -#. Restart the ``nova-compute`` service. Create instances with PCI passthrough devices -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +--------------------------------------------- -The ``nova-scheduler`` selects a destination host that has PCI devices -available with the specified ``vendor_id`` and ``product_id`` that matches the -``alias`` from the flavor. +The :program:`nova-scheduler` service selects a destination host that has PCI +devices available that match the ``alias`` specified in the flavor. .. code-block:: console diff --git a/doc/source/admin/secure-live-migration-with-qemu-native-tls.rst b/doc/source/admin/secure-live-migration-with-qemu-native-tls.rst index 012d78e93be..fb76f656af8 100644 --- a/doc/source/admin/secure-live-migration-with-qemu-native-tls.rst +++ b/doc/source/admin/secure-live-migration-with-qemu-native-tls.rst @@ -120,10 +120,13 @@ Performing the migration (1) On all relevant compute nodes, enable the :oslo.config:option:`libvirt.live_migration_with_native_tls` - configuration attribute:: + configuration attribute and set the + :oslo.config:option:`libvirt.live_migration_scheme` + configuration attribute to tls:: [libvirt] live_migration_with_native_tls = true + live_migration_scheme = tls .. note:: Setting both @@ -131,6 +134,12 @@ Performing the migration :oslo.config:option:`libvirt.live_migration_tunnelled` at the same time is invalid (and disallowed). + .. note:: + Not setting + :oslo.config:option:`libvirt.live_migration_scheme` to ``tls`` + will result in libvirt using the unencrypted TCP connection + without displaying any error or a warning in the logs. + And restart the ``nova-compute`` service:: $ systemctl restart openstack-nova-compute diff --git a/doc/source/admin/virtual-persistent-memory.rst b/doc/source/admin/virtual-persistent-memory.rst index 73cc0f3b98e..95ad9a942f8 100644 --- a/doc/source/admin/virtual-persistent-memory.rst +++ b/doc/source/admin/virtual-persistent-memory.rst @@ -31,12 +31,23 @@ The following are required to support the vPMEM feature: ``dax_pmem``, ``nd_pmem``, ``device_dax``, ``nd_btt`` +.. note:: + + NVDIMM support is present in the Linux Kernel v4.0 or newer. It is + recommended to use Kernel version 4.2 or later since `NVDIMM support + `_ + is enabled by default. We met some bugs in older versions, and we have + done all verification works with OpenStack on 4.18 version, so 4.18 + version and newer will probably guarantee its functionality. + * QEMU version >= 3.1.0 * Libvirt version >= 5.0.0 * `ndctl`_ version >= 62 +* daxio version >= 1.6 + The vPMEM feature has been verified under the software and hardware listed above. diff --git a/doc/source/user/flavors.rst b/doc/source/user/flavors.rst index 3e24fc0072d..740a4edee02 100644 --- a/doc/source/user/flavors.rst +++ b/doc/source/user/flavors.rst @@ -706,10 +706,14 @@ Hiding hypervisor signature As of the 18.0.0 Rocky release, this is only supported by the libvirt driver. + Prior to the 21.0.0 Ussuri release, this was called + ``hide_hypervisor_id``. An alias is provided to provide backwards + compatibility. + .. code:: console $ openstack flavor set FLAVOR-NAME \ - --property hide_hypervisor_id=VALUE + --property hw:hide_hypervisor_id=VALUE Where: diff --git a/gate/live_migration/hooks/ceph.sh b/gate/live_migration/hooks/ceph.sh index 483f92b0e1a..c588f7c9b29 100755 --- a/gate/live_migration/hooks/ceph.sh +++ b/gate/live_migration/hooks/ceph.sh @@ -8,6 +8,7 @@ function prepare_ceph { configure_ceph #install ceph-common package on compute nodes $ANSIBLE subnodes --become -f 5 -i "$WORKSPACE/inventory" -m raw -a "executable=/bin/bash + export CEPH_RELEASE=nautilus source $BASE/new/devstack/functions source $BASE/new/devstack/functions-common git clone https://opendev.org/openstack/devstack-plugin-ceph /tmp/devstack-plugin-ceph diff --git a/gate/live_migration/hooks/run_tests.sh b/gate/live_migration/hooks/run_tests.sh index 00ad3416349..7a5027f2c9a 100755 --- a/gate/live_migration/hooks/run_tests.sh +++ b/gate/live_migration/hooks/run_tests.sh @@ -55,6 +55,11 @@ fi echo '4. test with Ceph for root + ephemeral disks' # Discover and set variables for the OS version so the devstack-plugin-ceph # scripts can find the correct repository to install the ceph packages. +# NOTE(lyarwood): Pin the CEPH_RELEASE to nautilus here as was the case +# prior to https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/777232 +# landing in the branchless plugin, we also have to pin in ceph.sh when +# configuring ceph on a remote node via ansible. +export CEPH_RELEASE=nautilus GetOSVersion prepare_ceph GLANCE_API_CONF=${GLANCE_API_CONF:-/etc/glance/glance-api.conf} diff --git a/lower-constraints.txt b/lower-constraints.txt index 18a15993fd3..597a4466c17 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -27,14 +27,14 @@ eventlet==0.20.0 extras==1.0.0 fasteners==0.14.1 fixtures==3.0.0 -flake8==2.5.5 +flake8==2.6.0 future==0.16.0 futurist==1.8.0 gabbi==1.35.0 gitdb2==2.0.3 GitPython==2.1.8 greenlet==0.4.10 -hacking==0.12.0 +hacking==1.1.0 idna==2.6 iso8601==0.1.11 Jinja2==2.10 @@ -149,7 +149,7 @@ SQLAlchemy==1.2.19 sqlalchemy-migrate==0.11.0 sqlparse==0.2.4 statsd==3.2.2 -stestr==1.0.0 +stestr==2.0.0 stevedore==1.20.0 suds-jurko==0.6 taskflow==2.16.0 diff --git a/nova/api/metadata/base.py b/nova/api/metadata/base.py index a53b51c3333..3cba366be06 100644 --- a/nova/api/metadata/base.py +++ b/nova/api/metadata/base.py @@ -122,8 +122,13 @@ def __init__(self, instance, address=None, content=None, extra_md=None, if not content: content = [] + # NOTE(gibi): this is not a cell targeted context even if we are called + # in a situation when the instance is in a different cell than the + # metadata service itself. ctxt = context.get_admin_context() + self.mappings = _format_instance_mapping(instance) + # NOTE(danms): Sanitize the instance to limit the amount of stuff # inside that may not pickle well (i.e. context). We also touch # some of the things we'll lazy load later to make sure we keep their @@ -145,8 +150,6 @@ def __init__(self, instance, address=None, content=None, extra_md=None, self.security_groups = secgroup_api.get_instance_security_groups( ctxt, instance) - self.mappings = _format_instance_mapping(ctxt, instance) - if instance.user_data is not None: self.userdata_raw = base64.decode_as_bytes(instance.user_data) else: @@ -683,9 +686,8 @@ def get_metadata_by_instance_id(instance_id, address, ctxt=None): return InstanceMetadata(instance, address) -def _format_instance_mapping(ctxt, instance): - bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( - ctxt, instance.uuid) +def _format_instance_mapping(instance): + bdms = instance.get_bdms() return block_device.instance_block_mapping(instance, bdms) diff --git a/nova/api/openstack/compute/aggregates.py b/nova/api/openstack/compute/aggregates.py index 4fce9e9ce2b..f522e323cec 100644 --- a/nova/api/openstack/compute/aggregates.py +++ b/nova/api/openstack/compute/aggregates.py @@ -28,6 +28,7 @@ from nova import exception from nova.i18n import _ from nova.policies import aggregates as aggr_policies +from nova import utils def _get_context(req): @@ -85,11 +86,17 @@ def create(self, req, body): return agg - @wsgi.expected_errors(404) + @wsgi.expected_errors((400, 404)) def show(self, req, id): """Shows the details of an aggregate, hosts and metadata included.""" context = _get_context(req) context.can(aggr_policies.POLICY_ROOT % 'show') + + try: + utils.validate_integer(id, 'id') + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e.format_message()) + try: aggregate = self.api.get_aggregate(context, id) except exception.AggregateNotFound as e: @@ -107,6 +114,11 @@ def update(self, req, id, body): if 'name' in updates: updates['name'] = common.normalize_name(updates['name']) + try: + utils.validate_integer(id, 'id') + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e.format_message()) + try: aggregate = self.api.update_aggregate(context, id, updates) except exception.AggregateNameExists as e: @@ -126,6 +138,12 @@ def delete(self, req, id): """Removes an aggregate by id.""" context = _get_context(req) context.can(aggr_policies.POLICY_ROOT % 'delete') + + try: + utils.validate_integer(id, 'id') + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e.format_message()) + try: self.api.delete_aggregate(context, id) except exception.AggregateNotFound as e: @@ -136,7 +154,7 @@ def delete(self, req, id): # NOTE(gmann): Returns 200 for backwards compatibility but should be 202 # for representing async API as this API just accepts the request and # request hypervisor driver to complete the same in async mode. - @wsgi.expected_errors((404, 409)) + @wsgi.expected_errors((400, 404, 409)) @wsgi.action('add_host') @validation.schema(aggregates.add_host) def _add_host(self, req, id, body): @@ -145,6 +163,12 @@ def _add_host(self, req, id, body): context = _get_context(req) context.can(aggr_policies.POLICY_ROOT % 'add_host') + + try: + utils.validate_integer(id, 'id') + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e.format_message()) + try: aggregate = self.api.add_host_to_aggregate(context, id, host) except (exception.AggregateNotFound, @@ -159,7 +183,7 @@ def _add_host(self, req, id, body): # NOTE(gmann): Returns 200 for backwards compatibility but should be 202 # for representing async API as this API just accepts the request and # request hypervisor driver to complete the same in async mode. - @wsgi.expected_errors((404, 409)) + @wsgi.expected_errors((400, 404, 409)) @wsgi.action('remove_host') @validation.schema(aggregates.remove_host) def _remove_host(self, req, id, body): @@ -168,6 +192,12 @@ def _remove_host(self, req, id, body): context = _get_context(req) context.can(aggr_policies.POLICY_ROOT % 'remove_host') + + try: + utils.validate_integer(id, 'id') + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e.format_message()) + try: aggregate = self.api.remove_host_from_aggregate(context, id, host) except (exception.AggregateNotFound, exception.AggregateHostNotFound, @@ -189,6 +219,11 @@ def _set_metadata(self, req, id, body): context = _get_context(req) context.can(aggr_policies.POLICY_ROOT % 'set_metadata') + try: + utils.validate_integer(id, 'id') + except exception.InvalidInput as e: + raise exc.HTTPBadRequest(explanation=e.format_message()) + metadata = body["set_metadata"]["metadata"] try: aggregate = self.api.update_aggregate_metadata(context, diff --git a/nova/api/openstack/compute/schemas/server_groups.py b/nova/api/openstack/compute/schemas/server_groups.py index c5724bd6873..102c6df869e 100644 --- a/nova/api/openstack/compute/schemas/server_groups.py +++ b/nova/api/openstack/compute/schemas/server_groups.py @@ -11,6 +11,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. + import copy from nova.api.validation import parameter_types @@ -27,15 +28,18 @@ 'name': parameter_types.name, 'policies': { # This allows only a single item and it must be one of the - # enumerated values. So this is really just a single string - # value, but for legacy reasons is an array. We could - # probably change the type from array to string with a - # microversion at some point but it's very low priority. + # enumerated values. It's changed to a single string value + # in 2.64. 'type': 'array', - 'items': [{ - 'type': 'string', - 'enum': ['anti-affinity', 'affinity']}], + 'items': [ + { + 'type': 'string', + 'enum': ['anti-affinity', 'affinity'], + }, + ], 'uniqueItems': True, + 'minItems': 1, + 'maxItems': 1, 'additionalItems': False, } }, diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 68af4750d44..ecc2473b76f 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -762,6 +762,7 @@ def create(self, req, body): exception.InvalidBDMEphemeralSize, exception.InvalidBDMFormat, exception.InvalidBDMSwapSize, + exception.InvalidBDMDiskBus, exception.VolumeTypeNotFound, exception.AutoDiskConfigDisabledByImage, exception.InstanceGroupNotFound, diff --git a/nova/compute/api.py b/nova/compute/api.py index 08ca453d04a..bc23cbeaa70 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1664,6 +1664,13 @@ def _validate_bdm(self, context, instance, instance_type, "(source: 'blank', dest: 'volume') need to have non-zero " "size")) + # NOTE(lyarwood): Ensure the disk_bus is at least known to Nova. + # The virt driver may reject this later but for now just ensure + # it's listed as an acceptable value of the DiskBus field class. + disk_bus = bdm.disk_bus if 'disk_bus' in bdm else None + if disk_bus and disk_bus not in fields_obj.DiskBus.ALL: + raise exception.InvalidBDMDiskBus(disk_bus=disk_bus) + ephemeral_size = sum(bdm.volume_size or instance_type['ephemeral_gb'] for bdm in block_device_mappings if block_device.new_format_is_ephemeral(bdm)) @@ -3385,6 +3392,8 @@ def _reset_image_metadata(): new_sys_metadata = utils.get_system_metadata_from_image( image, flavor) + new_sys_metadata.update({'image_base_image_ref': image_id}) + instance.system_metadata.update(new_sys_metadata) instance.save() return orig_sys_metadata @@ -3798,6 +3807,14 @@ def shelve(self, context, instance, clean_shutdown=True): hypervisor. """ instance.task_state = task_states.SHELVING + + # NOTE(aarents): Ensure image_base_image_ref is present as it will be + # needed during unshelve and instance rebuild done before Bug/1893618 + # Fix dropped it. + instance.system_metadata.update( + {'image_base_image_ref': instance.image_ref} + ) + instance.save(expected_task_state=[None]) self._record_action_start(context, instance, instance_actions.SHELVE) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 0dfd1592768..4b991ff962e 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1330,6 +1330,13 @@ def init_host(self): eventlet.semaphore.BoundedSemaphore( CONF.compute.max_concurrent_disk_ops) + if CONF.compute.max_disk_devices_to_attach == 0: + msg = _('[compute]max_disk_devices_to_attach has been set to 0, ' + 'which will prevent instances from being able to boot. ' + 'Set -1 for unlimited or set >= 1 to limit the maximum ' + 'number of disk devices.') + raise exception.InvalidConfiguration(msg) + self.driver.init_host(host=self.host) context = nova.context.get_admin_context() instances = objects.InstanceList.get_by_host( @@ -1580,7 +1587,11 @@ def _decode(f): return [_decode(f) for f in injected_files] def _validate_instance_group_policy(self, context, instance, - scheduler_hints): + scheduler_hints=None): + + if CONF.workarounds.disable_group_policy_check_upcall: + return + # NOTE(russellb) Instance group policy is enforced by the scheduler. # However, there is a race condition with the enforcement of # the policy. Since more than one instance may be scheduled at the @@ -1589,29 +1600,63 @@ def _validate_instance_group_policy(self, context, instance, # multiple instances with an affinity policy could end up on different # hosts. This is a validation step to make sure that starting the # instance here doesn't violate the policy. - group_hint = scheduler_hints.get('group') - if not group_hint: - return - - # The RequestSpec stores scheduler_hints as key=list pairs so we need - # to check the type on the value and pull the single entry out. The - # API request schema validates that the 'group' hint is a single value. - if isinstance(group_hint, list): - group_hint = group_hint[0] + if scheduler_hints is not None: + # only go through here if scheduler_hints is provided, even if it + # is empty. + group_hint = scheduler_hints.get('group') + if not group_hint: + return + else: + # The RequestSpec stores scheduler_hints as key=list pairs so + # we need to check the type on the value and pull the single + # entry out. The API request schema validates that + # the 'group' hint is a single value. + if isinstance(group_hint, list): + group_hint = group_hint[0] + + group = objects.InstanceGroup.get_by_hint(context, group_hint) + else: + # TODO(ganso): a call to DB can be saved by adding request_spec + # to rpcapi payload of live_migration, pre_live_migration and + # check_can_live_migrate_destination + try: + group = objects.InstanceGroup.get_by_instance_uuid( + context, instance.uuid) + except exception.InstanceGroupNotFound: + return - @utils.synchronized(group_hint) - def _do_validation(context, instance, group_hint): - group = objects.InstanceGroup.get_by_hint(context, group_hint) + @utils.synchronized(group['uuid']) + def _do_validation(context, instance, group): if group.policy and 'anti-affinity' == group.policy: + + # instances on host instances_uuids = objects.InstanceList.get_uuids_by_host( context, self.host) ins_on_host = set(instances_uuids) + + # instance param is just for logging, the nodename obtained is + # not actually related to the instance at all + nodename = self._get_nodename(instance) + + # instances being migrated to host + migrations = ( + objects.MigrationList.get_in_progress_by_host_and_node( + context, self.host, nodename)) + migration_vm_uuids = set([mig['instance_uuid'] + for mig in migrations]) + + total_instances = migration_vm_uuids | ins_on_host + + # refresh group to get updated members within locked block + group = objects.InstanceGroup.get_by_uuid(context, + group['uuid']) members = set(group.members) # Determine the set of instance group members on this host # which are not the instance in question. This is used to # determine how many other members from the same anti-affinity # group can be on this host. - members_on_host = ins_on_host & members - set([instance.uuid]) + members_on_host = (total_instances & members - + set([instance.uuid])) rules = group.rules if rules and 'max_server_per_host' in rules: max_server = rules['max_server_per_host'] @@ -1623,6 +1668,12 @@ def _do_validation(context, instance, group_hint): raise exception.RescheduledException( instance_uuid=instance.uuid, reason=msg) + + # NOTE(ganso): The check for affinity below does not work and it + # can easily be violated because the lock happens in different + # compute hosts. + # The only fix seems to be a DB lock to perform the check whenever + # setting the host field to an instance. elif group.policy and 'affinity' == group.policy: group_hosts = group.get_hosts(exclude=[instance.uuid]) if group_hosts and self.host not in group_hosts: @@ -1631,8 +1682,7 @@ def _do_validation(context, instance, group_hint): instance_uuid=instance.uuid, reason=msg) - if not CONF.workarounds.disable_group_policy_check_upcall: - _do_validation(context, instance, group_hint) + _do_validation(context, instance, group) def _log_original_error(self, exc_info, instance_uuid): LOG.error('Error: %s', exc_info[1], instance_uuid=instance_uuid, @@ -4766,10 +4816,24 @@ def prep_resize(self, context, image, instance, instance_type, with self._error_out_instance_on_exception( context, instance, instance_state=instance_state),\ errors_out_migration_ctxt(migration): + self._send_prep_resize_notifications( context, instance, fields.NotificationPhase.START, instance_type) try: + scheduler_hints = self._get_scheduler_hints(filter_properties, + request_spec) + # Error out if this host cannot accept the new instance due + # to anti-affinity. At this point the migration is already + # in-progress, so this is the definitive moment to abort due to + # the policy violation. Also, exploding here is covered by the + # cleanup methods in except block. + try: + self._validate_instance_group_policy(context, instance, + scheduler_hints) + except exception.RescheduledException as e: + raise exception.InstanceFaultRollback(inner_exception=e) + self._prep_resize(context, image, instance, instance_type, filter_properties, node, migration, request_spec, @@ -6455,9 +6519,33 @@ def _swap_volume(self, context, instance, bdm, connector, @wrap_instance_fault def swap_volume(self, context, old_volume_id, new_volume_id, instance, new_attachment_id): - """Swap volume for an instance.""" - context = context.elevated() + """Replace the old volume with the new volume within the active server + :param context: User request context + :param old_volume_id: Original volume id + :param new_volume_id: New volume id being swapped to + :param instance: Instance with original_volume_id attached + :param new_attachment_id: ID of the new attachment for new_volume_id + """ + @utils.synchronized(instance.uuid) + def _do_locked_swap_volume(context, old_volume_id, new_volume_id, + instance, new_attachment_id): + self._do_swap_volume(context, old_volume_id, new_volume_id, + instance, new_attachment_id) + _do_locked_swap_volume(context, old_volume_id, new_volume_id, instance, + new_attachment_id) + + def _do_swap_volume(self, context, old_volume_id, new_volume_id, + instance, new_attachment_id): + """Replace the old volume with the new volume within the active server + + :param context: User request context + :param old_volume_id: Original volume id + :param new_volume_id: New volume id being swapped to + :param instance: Instance with original_volume_id attached + :param new_attachment_id: ID of the new attachment for new_volume_id + """ + context = context.elevated() compute_utils.notify_about_volume_swap( context, instance, self.host, fields.NotificationPhase.START, @@ -6780,6 +6868,20 @@ def check_can_live_migrate_destination(self, ctxt, instance, :param limits: objects.SchedulerLimits object for this live migration. :returns: a LiveMigrateData object (hypervisor-dependent) """ + + # Error out if this host cannot accept the new instance due + # to anti-affinity. This check at this moment is not very accurate, as + # multiple requests may be happening concurrently and miss the lock, + # but when it works it provides a better user experience by failing + # earlier. Also, it should be safe to explode here, error becomes + # NoValidHost and instance status remains ACTIVE. + try: + self._validate_instance_group_policy(ctxt, instance) + except exception.RescheduledException as e: + msg = ("Failed to validate instance group policy " + "due to: {}".format(e)) + raise exception.MigrationPreCheckError(reason=msg) + src_compute_info = obj_base.obj_to_primitive( self._get_compute_info(ctxt, instance.host)) dst_compute_info = obj_base.obj_to_primitive( @@ -6801,15 +6903,18 @@ def check_can_live_migrate_destination(self, ctxt, instance, LOG.info('Destination was ready for NUMA live migration, ' 'but source is either too old, or is set to an ' 'older upgrade level.', instance=instance) - # Create migrate_data vifs - migrate_data.vifs = \ - migrate_data_obj.VIFMigrateData.create_skeleton_migrate_vifs( - instance.get_network_info()) - # Claim PCI devices for VIFs on destination (if needed) - port_id_to_pci = self._claim_pci_for_instance_vifs(ctxt, instance) - # Update migrate VIFs with the newly claimed PCI devices - self._update_migrate_vifs_profile_with_pci(migrate_data.vifs, - port_id_to_pci) + if self.network_api.supports_port_binding_extension(ctxt): + # Create migrate_data vifs + migrate_data.vifs = \ + migrate_data_obj.\ + VIFMigrateData.create_skeleton_migrate_vifs( + instance.get_network_info()) + # Claim PCI devices for VIFs on destination (if needed) + port_id_to_pci = self._claim_pci_for_instance_vifs( + ctxt, instance) + # Update migrate VIFs with the newly claimed PCI devices + self._update_migrate_vifs_profile_with_pci( + migrate_data.vifs, port_id_to_pci) finally: self.driver.cleanup_live_migration_destination_check(ctxt, dest_check_data) @@ -6915,6 +7020,13 @@ def pre_live_migration(self, context, instance, block_migration, disk, """ LOG.debug('pre_live_migration data is %s', migrate_data) + # Error out if this host cannot accept the new instance due + # to anti-affinity. At this point the migration is already in-progress, + # so this is the definitive moment to abort due to the policy + # violation. Also, it should be safe to explode here. The instance + # status remains ACTIVE, migration status failed. + self._validate_instance_group_policy(context, instance) + migrate_data.old_vol_attachment_ids = {} bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( context, instance.uuid) @@ -6977,8 +7089,12 @@ def pre_live_migration(self, context, instance, block_migration, disk, # determine if it should wait for a 'network-vif-plugged' event # from neutron before starting the actual guest transfer in the # hypervisor + using_multiple_port_bindings = ( + 'vifs' in migrate_data and migrate_data.vifs) migrate_data.wait_for_vif_plugged = ( - CONF.compute.live_migration_wait_for_vif_plug) + CONF.compute.live_migration_wait_for_vif_plug and + using_multiple_port_bindings + ) # NOTE(tr3buchet): setup networks on destination host self.network_api.setup_networks_on_host(context, instance, @@ -7038,8 +7154,8 @@ def _get_neutron_events_for_live_migration(instance): # We don't generate events if CONF.vif_plugging_timeout=0 # meaning that the operator disabled using them. if CONF.vif_plugging_timeout and utils.is_neutron(): - return [('network-vif-plugged', vif['id']) - for vif in instance.get_network_info()] + return (instance.get_network_info() + .get_live_migration_plug_time_events()) else: return [] @@ -7496,7 +7612,13 @@ def _post_live_migration(self, ctxt, instance, dest, # Releasing vlan. # (not necessary in current implementation?) - network_info = self.network_api.get_instance_nw_info(ctxt, instance) + # NOTE(artom) At this point in time we have not bound the ports to the + # destination host yet (this happens in migrate_instance_start() + # below). Therefore, the "old" source network info that's still in the + # instance info cache is safe to use here, since it'll be used below + # during driver.post_live_migration_at_source() to unplug the VIFs on + # the source. + network_info = instance.get_network_info() self._notify_about_instance_usage(ctxt, instance, "live_migration._post.start", diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index dc2b8444960..1c644cf299d 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -1307,8 +1307,37 @@ def _bury_in_cell0(self, context, request_spec, exc, updates = {'vm_state': vm_states.ERROR, 'task_state': None} for instance in instances_by_uuid.values(): + + inst_mapping = None + try: + # We don't need the cell0-targeted context here because the + # instance mapping is in the API DB. + inst_mapping = \ + objects.InstanceMapping.get_by_instance_uuid( + context, instance.uuid) + except exception.InstanceMappingNotFound: + # The API created the instance mapping record so it should + # definitely be here. Log an error but continue to create the + # instance in the cell0 database. + LOG.error('While burying instance in cell0, no instance ' + 'mapping was found.', instance=instance) + + # Perform a final sanity check that the instance is not mapped + # to some other cell already because of maybe some crazy + # clustered message queue weirdness. + if inst_mapping and inst_mapping.cell_mapping is not None: + LOG.error('When attempting to bury instance in cell0, the ' + 'instance is already mapped to cell %s. Ignoring ' + 'bury in cell0 attempt.', + inst_mapping.cell_mapping.identity, + instance=instance) + continue + with obj_target_cell(instance, cell0) as cctxt: instance.create() + if inst_mapping: + inst_mapping.cell_mapping = cell0 + inst_mapping.save() # Record an instance action with a failed event. self._create_instance_action_for_cell0( @@ -1328,16 +1357,6 @@ def _bury_in_cell0(self, context, request_spec, exc, self._set_vm_state_and_notify( cctxt, instance.uuid, 'build_instances', updates, exc, request_spec) - try: - # We don't need the cell0-targeted context here because the - # instance mapping is in the API DB. - inst_mapping = \ - objects.InstanceMapping.get_by_instance_uuid( - context, instance.uuid) - inst_mapping.cell_mapping = cell0 - inst_mapping.save() - except exception.InstanceMappingNotFound: - pass for build_request in build_requests: try: @@ -1506,13 +1525,8 @@ def schedule_and_build_instances(self, context, build_requests, instance.tags = instance_tags if instance_tags \ else objects.TagList() - # Update mapping for instance. Normally this check is guarded by - # a try/except but if we're here we know that a newer nova-api - # handled the build process and would have created the mapping - inst_mapping = objects.InstanceMapping.get_by_instance_uuid( - context, instance.uuid) - inst_mapping.cell_mapping = cell - inst_mapping.save() + # Update mapping for instance. + self._map_instance_to_cell(context, instance, cell) if not self._delete_build_request( context, build_request, instance, cell, instance_bdms, @@ -1540,6 +1554,37 @@ def schedule_and_build_instances(self, context, build_requests, host=host.service_host, node=host.nodename, limits=host.limits, host_list=host_list) + @staticmethod + def _map_instance_to_cell(context, instance, cell): + """Update the instance mapping to point at the given cell. + + During initial scheduling once a host and cell is selected in which + to build the instance this method is used to update the instance + mapping to point at that cell. + + :param context: nova auth RequestContext + :param instance: Instance object being built + :param cell: CellMapping representing the cell in which the instance + was created and is being built. + :returns: InstanceMapping object that was updated. + """ + inst_mapping = objects.InstanceMapping.get_by_instance_uuid( + context, instance.uuid) + # Perform a final sanity check that the instance is not mapped + # to some other cell already because of maybe some crazy + # clustered message queue weirdness. + if inst_mapping.cell_mapping is not None: + LOG.error('During scheduling instance is already mapped to ' + 'another cell: %s. This should not happen and is an ' + 'indication of bigger problems. If you see this you ' + 'should report it to the nova team. Overwriting ' + 'the mapping to point at cell %s.', + inst_mapping.cell_mapping.identity, cell.identity, + instance=instance) + inst_mapping.cell_mapping = cell + inst_mapping.save() + return inst_mapping + def _cleanup_build_artifacts(self, context, exc, instances, build_requests, request_specs, block_device_mappings, tags, cell_mapping_cache): diff --git a/nova/conf/compute.py b/nova/conf/compute.py index fccebabb370..6713f61d47b 100644 --- a/nova/conf/compute.py +++ b/nova/conf/compute.py @@ -946,10 +946,16 @@ The configured maximum is not enforced on shelved offloaded servers, as they have no compute host. +.. warning:: If this option is set to 0, the ``nova-compute`` service will fail + to start, as 0 disk devices is an invalid configuration that would + prevent instances from being able to boot. + Possible values: * -1 means unlimited -* Any integer >= 0 represents the maximum allowed +* Any integer >= 1 represents the maximum allowed. A value of 0 will cause the + ``nova-compute`` service to fail to start, as 0 disk devices is an invalid + configuration that would prevent instances from being able to boot. """), ] diff --git a/nova/conf/glance.py b/nova/conf/glance.py index b96f437b0f3..694855e219c 100644 --- a/nova/conf/glance.py +++ b/nova/conf/glance.py @@ -44,7 +44,7 @@ (i.e. "http://10.0.1.0:9292" or "https://my.glance.server/image"). """), cfg.IntOpt('num_retries', - default=0, + default=3, min=0, help=""" Enable glance operation retries. diff --git a/nova/conf/libvirt.py b/nova/conf/libvirt.py index 0ee880af477..1364c8ed0b4 100644 --- a/nova/conf/libvirt.py +++ b/nova/conf/libvirt.py @@ -1319,6 +1319,14 @@ This option is only usable for virtio-net device with vhost-user backend. Available only with QEMU/KVM. Requires libvirt v3.7 QEMU v2.10."""), + cfg.IntOpt('max_queues', default=None, min=1, help=""" +The maximum number of virtio queue pairs that can be enabled +when creating a multiqueue guest. The number of virtio queues +allocated will be the lesser of the CPUs requested by the guest +and the max value defined. By default, this value is set to none +meaning the legacy limits based on the reported kernel +major version will be used. +"""), ] diff --git a/nova/conf/pci.py b/nova/conf/pci.py index 28a68362612..b812b39676a 100644 --- a/nova/conf/pci.py +++ b/nova/conf/pci.py @@ -60,7 +60,8 @@ ``device_type`` Type of PCI device. Valid values are: ``type-PCI``, ``type-PF`` and - ``type-VF``. + ``type-VF``. Note that ``"device_type": "type-PF"`` **must** be specified + if you wish to passthrough a device that supports SR-IOV in its entirety. ``numa_policy`` Required NUMA affinity of device. Valid values are: ``legacy``, diff --git a/nova/conf/workarounds.py b/nova/conf/workarounds.py index c9ed0f2e4b5..20e4b5c9d61 100644 --- a/nova/conf/workarounds.py +++ b/nova/conf/workarounds.py @@ -265,6 +265,30 @@ * :oslo.config:option:`DEFAULT.instances_path` * :oslo.config:option:`image_cache_subdirectory_name` * :oslo.config:option:`update_resources_interval` +"""), + cfg.BoolOpt( + 'never_download_image_if_on_rbd', + default=False, + help=""" +When booting from an image on a ceph-backed compute node, if the image does not +already reside on the ceph cluster (as would be the case if glance is +also using the same cluster), nova will download the image from glance and +upload it to ceph itself. If using multiple ceph clusters, this may cause nova +to unintentionally duplicate the image in a non-COW-able way in the local +ceph deployment, wasting space. + +For more information, refer to the bug report: + +https://bugs.launchpad.net/nova/+bug/1858877 + +Enabling this option will cause nova to *refuse* to boot an instance if it +would require downloading the image from glance and uploading it to ceph +itself. + +Related options: + +* ``compute_driver`` (libvirt) +* ``[libvirt]/images_type`` (rbd) """), ] diff --git a/nova/console/websocketproxy.py b/nova/console/websocketproxy.py index e13b3c0fe15..7641a7cc086 100644 --- a/nova/console/websocketproxy.py +++ b/nova/console/websocketproxy.py @@ -19,6 +19,7 @@ ''' import copy +import os import socket import sys @@ -305,6 +306,22 @@ def socket(self, *args, **kwargs): # Fall back to the websockify <= v0.8.0 'socket' method location. return websockify.WebSocketServer.socket(*args, **kwargs) + def send_head(self): + # This code is copied from this example patch: + # https://bugs.python.org/issue32084#msg306545 + path = self.translate_path(self.path) + if os.path.isdir(path): + parts = urlparse.urlsplit(self.path) + if not parts.path.endswith('/'): + # Browsers interpret "Location: //uri" as an absolute URI + # like "http://URI" + if self.path.startswith('//'): + self.send_error(400, + "URI must not start with //") + return None + + return super(NovaProxyRequestHandler, self).send_head() + class NovaWebSocketProxy(websockify.WebSocketProxy): def __init__(self, *args, **kwargs): diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index cd47f18771a..a0c13fe3600 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -48,6 +48,7 @@ from sqlalchemy.orm import contains_eager from sqlalchemy.orm import joinedload from sqlalchemy.orm import noload +from sqlalchemy.orm import subqueryload from sqlalchemy.orm import undefer from sqlalchemy.schema import Table from sqlalchemy import sql @@ -1943,13 +1944,27 @@ def _build_instance_get(context, columns_to_join=None): continue if 'extra.' in column: query = query.options(undefer(column)) + elif column in ['metadata', 'system_metadata']: + # NOTE(melwitt): We use subqueryload() instead of joinedload() for + # metadata and system_metadata because of the one-to-many + # relationship of the data. Directly joining these columns can + # result in a large number of additional rows being queried if an + # instance has a large number of (system_)metadata items, resulting + # in a large data transfer. Instead, the subqueryload() will + # perform additional queries to obtain metadata and system_metadata + # for the instance. + query = query.options(subqueryload(column)) else: query = query.options(joinedload(column)) # NOTE(alaski) Stop lazy loading of columns not needed. for col in ['metadata', 'system_metadata']: if col not in columns_to_join: query = query.options(noload(col)) - return query + # NOTE(melwitt): We need to use order_by() so that the + # additional queries emitted by subqueryload() include the same ordering as + # used by the parent query. + # https://docs.sqlalchemy.org/en/13/orm/loading_relationships.html#the-importance-of-ordering + return query.order_by(models.Instance.id) def _instances_fill_metadata(context, instances, manual_joins=None): @@ -5591,7 +5606,11 @@ def _archive_deleted_rows_for_table(metadata, tablename, max_rows, before): # NOTE(jake): instance_actions_events doesn't have a instance_uuid column # but still needs to be archived as it is a FK constraint if ((max_rows is None or rows_archived < max_rows) and - ('instance_uuid' in columns or + # NOTE(melwitt): The pci_devices table uses the 'instance_uuid' + # column to track the allocated association of a PCI device and its + # records are not tied to the lifecycles of instance records. + (tablename != 'pci_devices' and + 'instance_uuid' in columns or tablename == 'instance_actions_events')): instances = models.BASE.metadata.tables['instances'] limit = max_rows - rows_archived if max_rows is not None else None diff --git a/nova/exception.py b/nova/exception.py index 74834284e46..d9533fe4c34 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -254,6 +254,11 @@ class TooManyDiskDevices(InvalidBDM): code = 403 +class InvalidBDMDiskBus(InvalidBDM): + msg_fmr = _("Block Device Mapping is invalid: The provided disk bus " + "%(disk_bus)s is not valid.") + + class InvalidAttribute(Invalid): msg_fmt = _("Attribute not supported: %(attr)s") @@ -615,6 +620,11 @@ class ImageBadRequest(Invalid): "%(response)s") +class ImageQuotaExceeded(NovaException): + msg_fmt = _("Quota exceeded or out of space for image %(image_id)s " + "in the image service.") + + class InstanceUnacceptable(Invalid): msg_fmt = _("Instance %(instance_id)s is unacceptable: %(reason)s") @@ -2536,7 +2546,7 @@ class PMEMNamespaceConfigInvalid(NovaException): "please check your conf file. ") -class GetPMEMNamespaceFailed(NovaException): +class GetPMEMNamespacesFailed(NovaException): msg_fmt = _("Get PMEM namespaces on host failed: %(reason)s.") diff --git a/nova/image/glance.py b/nova/image/glance.py index b2c5f9ae059..5c25bd4e41d 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -938,6 +938,8 @@ def _translate_image_exception(image_id, exc_value): if isinstance(exc_value, glanceclient.exc.BadRequest): return exception.ImageBadRequest(image_id=image_id, response=six.text_type(exc_value)) + if isinstance(exc_value, glanceclient.exc.HTTPOverLimit): + return exception.ImageQuotaExceeded(image_id=image_id) return exc_value diff --git a/nova/monkey_patch.py b/nova/monkey_patch.py index a07ff91dacb..160bb1a6611 100644 --- a/nova/monkey_patch.py +++ b/nova/monkey_patch.py @@ -22,6 +22,19 @@ def _monkey_patch(): + # See https://bugs.launchpad.net/nova/+bug/1164822 + # TODO(mdbooth): This feature was deprecated and removed in eventlet at + # some point but brought back in version 0.21.0, presumably because some + # users still required it to work round issues. However, there have been a + # number of greendns fixes in eventlet since then. Specifically, it looks + # as though the originally reported IPv6 issue may have been fixed in + # version 0.24.0. We should remove this when we can confirm that the + # original issue is fixed. + # NOTE(artom) eventlet processes environment variables at import-time. We + # therefore set this here, before importing eventlet, in order to correctly + # disable greendns. + os.environ['EVENTLET_NO_GREENDNS'] = 'yes' + # NOTE(mdbooth): Anything imported here will not be monkey patched. It is # important to take care not to import anything here which requires monkey # patching. @@ -39,16 +52,6 @@ def _monkey_patch(): problems = (set(['urllib3', 'oslo_context.context']) & set(sys.modules.keys())) - # See https://bugs.launchpad.net/nova/+bug/1164822 - # TODO(mdbooth): This feature was deprecated and removed in eventlet at - # some point but brought back in version 0.21.0, presumably because some - # users still required it to work round issues. However, there have been a - # number of greendns fixes in eventlet since then. Specifically, it looks - # as though the originally reported IPv6 issue may have been fixed in - # version 0.24.0. We should remove this when we can confirm that the - # original issue is fixed. - os.environ['EVENTLET_NO_GREENDNS'] = 'yes' - if debugger.enabled(): # turn off thread patching to enable the remote debugger eventlet.monkey_patch(thread=False) diff --git a/nova/network/model.py b/nova/network/model.py index d8119fae729..7ed9d2d1b86 100644 --- a/nova/network/model.py +++ b/nova/network/model.py @@ -469,6 +469,14 @@ def has_bind_time_event(self, migration): return (self.is_hybrid_plug_enabled() and not migration.is_same_host()) + @property + def has_live_migration_plug_time_event(self): + """Returns whether this VIF's network-vif-plugged external event will + be sent by Neutron at "plugtime" - in other words, as soon as neutron + completes configuring the network backend. + """ + return self.is_hybrid_plug_enabled() + def is_hybrid_plug_enabled(self): return self['details'].get(VIF_DETAILS_OVS_HYBRID_PLUG, False) @@ -530,15 +538,22 @@ def json(self): return jsonutils.dumps(self) def get_bind_time_events(self, migration): - """Returns whether any of our VIFs have "bind-time" events. See - has_bind_time_event() docstring for more details. + """Returns a list of external events for any VIFs that have + "bind-time" events during cold migration. """ return [('network-vif-plugged', vif['id']) for vif in self if vif.has_bind_time_event(migration)] + def get_live_migration_plug_time_events(self): + """Returns a list of external events for any VIFs that have + "plug-time" events during live migration. + """ + return [('network-vif-plugged', vif['id']) + for vif in self if vif.has_live_migration_plug_time_event] + def get_plug_time_events(self, migration): - """Complementary to get_bind_time_events(), any event that does not - fall in that category is a plug-time event. + """Returns a list of external events for any VIFs that have + "plug-time" events during cold migration. """ return [('network-vif-plugged', vif['id']) for vif in self if not vif.has_bind_time_event(migration)] diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 8f85a748f35..8508e63d78c 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -747,9 +747,15 @@ def _process_security_groups(self, instance, neutron, security_groups): # TODO(arosen) Should optimize more to do direct query for security # group if len(security_groups) == 1 if len(security_groups): + # NOTE(slaweq): fields other than name and id aren't really needed + # so asking only about those fields will allow Neutron to not + # prepare list of rules for each found security group. That may + # speed processing of this request a lot in case when tenant has + # got many security groups + sg_fields = ['id', 'name'] search_opts = {'tenant_id': instance.project_id} user_security_groups = neutron.list_security_groups( - **search_opts).get('security_groups') + fields=sg_fields, **search_opts).get('security_groups') for security_group in security_groups: name_match = None diff --git a/nova/network/neutronv2/constants.py b/nova/network/neutronv2/constants.py index 1bb4605c9f9..6a5e6b8e0ea 100644 --- a/nova/network/neutronv2/constants.py +++ b/nova/network/neutronv2/constants.py @@ -19,6 +19,7 @@ DNS_INTEGRATION = 'DNS Integration' MULTI_NET_EXT = 'Multi Provider Network' SUBSTR_PORT_FILTERING = 'IP address substring filtering' +PORT_BINDING = 'Port Binding' PORT_BINDING_EXTENDED = 'Port Bindings Extended' LIVE_MIGRATION = 'live-migration' DEFAULT_SECGROUP = 'default' diff --git a/nova/objects/fields.py b/nova/objects/fields.py index a768d0236af..366a970737c 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -339,6 +339,8 @@ class DiskBus(BaseNovaEnum): # NOTE(aspiers): If you change this, don't forget to update the # docs and metadata for hw_*_bus in glance. + # NOTE(lyarwood): Also update the possible values in the api-ref for the + # block_device_mapping_v2.disk_bus parameter. FDC = "fdc" IDE = "ide" SATA = "sata" diff --git a/nova/objects/instance_mapping.py b/nova/objects/instance_mapping.py index 0392f04770a..8b5f6ba92e7 100644 --- a/nova/objects/instance_mapping.py +++ b/nova/objects/instance_mapping.py @@ -15,6 +15,7 @@ from oslo_log import log as logging from oslo_utils import versionutils import six +from sqlalchemy.orm import exc as orm_exc from sqlalchemy.orm import joinedload from sqlalchemy.sql import false from sqlalchemy.sql import func @@ -161,8 +162,16 @@ def _save_in_db(context, instance_uuid, updates): def save(self): changes = self.obj_get_changes() changes = self._update_with_cell_id(changes) - db_mapping = self._save_in_db(self._context, self.instance_uuid, - changes) + try: + db_mapping = self._save_in_db(self._context, self.instance_uuid, + changes) + except orm_exc.StaleDataError: + # NOTE(melwitt): If the instance mapping has been deleted out from + # under us by conductor (delete requested while booting), we will + # encounter a StaleDataError after we retrieved the row and try to + # update it after it's been deleted. We can treat this like an + # instance mapping not found and allow the caller to handle it. + raise exception.InstanceMappingNotFound(uuid=self.instance_uuid) self._from_db_object(self._context, self, db_mapping) self.obj_reset_changes() diff --git a/nova/pci/manager.py b/nova/pci/manager.py index 3084643f5e8..05930b0bebb 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -225,6 +225,7 @@ def _set_hvdevs(self, devices): self.stale[new_value['address']] = new_value else: existed.update_device(new_value) + self.stats.update_device(existed) # Track newly discovered devices. for dev in [dev for dev in devices if diff --git a/nova/pci/stats.py b/nova/pci/stats.py index dcf26d3f74e..0a80ecedfc2 100644 --- a/nova/pci/stats.py +++ b/nova/pci/stats.py @@ -105,6 +105,32 @@ def _create_pool_keys_from_dev(self, dev): pool['parent_ifname'] = dev.extra_info['parent_ifname'] return pool + def _get_pool_with_device_type_mismatch(self, dev): + """Check for device type mismatch in the pools for a given device. + + Return (pool, device) if device type does not match or a single None + if the device type matches. + """ + for pool in self.pools: + for device in pool['devices']: + if device.address == dev.address: + if dev.dev_type != pool["dev_type"]: + return pool, device + return None + + return None + + def update_device(self, dev): + """Update a device to its matching pool.""" + pool_device_info = self._get_pool_with_device_type_mismatch(dev) + if pool_device_info is None: + return + + pool, device = pool_device_info + pool['devices'].remove(device) + self._decrease_pool_count(self.pools, pool) + self.add_device(dev) + def add_device(self, dev): """Add a device to its matching pool.""" dev_pool = self._create_pool_keys_from_dev(dev) diff --git a/nova/tests/functional/db/test_archive.py b/nova/tests/functional/db/test_archive.py index 79edf0b96ad..bc463c7eccc 100644 --- a/nova/tests/functional/db/test_archive.py +++ b/nova/tests/functional/db/test_archive.py @@ -137,6 +137,19 @@ def test_archive_deleted_rows_with_undeleted_residue(self): # Verify we have some system_metadata since we'll check that later. self.assertTrue(len(instance.system_metadata), 'No system_metadata for instance: %s' % server_id) + # Create a pci_devices record to simulate an instance that had a PCI + # device allocated at the time it was deleted. There is a window of + # time between deletion of the instance record and freeing of the PCI + # device in nova-compute's _complete_deletion method during RT update. + db.pci_device_update(admin_context, 1, 'fake-address', + {'compute_node_id': 1, + 'address': 'fake-address', + 'vendor_id': 'fake', + 'product_id': 'fake', + 'dev_type': 'fake', + 'label': 'fake', + 'status': 'allocated', + 'instance_uuid': instance.uuid}) # Now try and archive the soft deleted records. results, deleted_instance_uuids, archived = \ db.archive_deleted_rows(max_rows=100) @@ -151,6 +164,8 @@ def test_archive_deleted_rows_with_undeleted_residue(self): self.assertIn('instance_actions', results) self.assertIn('instance_actions_events', results) self.assertEqual(sum(results.values()), archived) + # Verify that the pci_devices record has not been dropped + self.assertNotIn('pci_devices', results) def _get_table_counts(self): engine = sqlalchemy_api.get_engine() diff --git a/nova/tests/functional/libvirt/test_numa_servers.py b/nova/tests/functional/libvirt/test_numa_servers.py index 9affbe9c198..6f23c02a8c0 100644 --- a/nova/tests/functional/libvirt/test_numa_servers.py +++ b/nova/tests/functional/libvirt/test_numa_servers.py @@ -343,12 +343,7 @@ def test_create_server_with_isolate_thread_policy_fails(self): } flavor_id = self._create_flavor(vcpu=2, extra_spec=extra_spec) - # FIXME(stephenfin): This should go to error status since there should - # not be a host available - expected_usage = { - 'DISK_GB': 20, 'MEMORY_MB': 2048, 'PCPU': 0, 'VCPU': 2, - } - self._run_build_test(flavor_id, expected_usage=expected_usage) + self._run_build_test(flavor_id, end_status='ERROR') def test_create_server_with_pcpu(self): """Create a server using an explicit 'resources:PCPU' request. diff --git a/nova/tests/functional/libvirt/test_pci_sriov_servers.py b/nova/tests/functional/libvirt/test_pci_sriov_servers.py index f203afec156..52b93c96192 100644 --- a/nova/tests/functional/libvirt/test_pci_sriov_servers.py +++ b/nova/tests/functional/libvirt/test_pci_sriov_servers.py @@ -13,11 +13,14 @@ # License for the specific language governing permissions and limitations # under the License. +import copy import fixtures import mock from oslo_log import log as logging from oslo_serialization import jsonutils +from nova import context +from nova import objects from nova.objects import fields from nova.tests.functional.libvirt import base from nova.tests.unit.virt.libvirt import fakelibvirt @@ -116,6 +119,20 @@ class SRIOVServersTest(_PCIServersTestBase): }, )] + def _disable_sriov_in_pf(self, pci_info): + # Check for PF and change the capability from virt_functions + # Delete all the VFs + vfs_to_delete = [] + + for device_name, device in pci_info.devices.items(): + if 'virt_functions' in device.pci_device: + device.generate_xml(skip_capability=True) + elif 'phys_function' in device.pci_device: + vfs_to_delete.append(device_name) + + for device in vfs_to_delete: + del pci_info.devices[device] + def test_create_server_with_VF(self): host_info = fakelibvirt.HostInfo(cpu_nodes=2, cpu_sockets=1, @@ -186,6 +203,69 @@ def test_create_server_with_VF_no_PF(self): self._run_build_test(flavor_id_vfs) self._run_build_test(flavor_id_pfs, end_status='ERROR') + def test_create_server_after_change_in_nonsriov_pf_to_sriov_pf(self): + # Starts a compute with PF not configured with SRIOV capabilities + # Updates the PF with SRIOV capability and restart the compute service + # Then starts a VM with the sriov port. The VM should be in active + # state with sriov port attached. + + # To emulate the device type changing, we first create a + # HostPCIDevicesInfo object with PFs and VFs. Then we make a copy + # and remove the VFs and the virt_function capability. This is + # done to ensure the physical function product id is same in both + # the versions. + host_info = fakelibvirt.HostInfo(cpu_nodes=2, cpu_sockets=1, + cpu_cores=2, cpu_threads=2, + kB_mem=15740000) + pci_info = fakelibvirt.HostPCIDevicesInfo(num_pfs=1, num_vfs=1) + pci_info_no_sriov = copy.deepcopy(pci_info) + + # Disable SRIOV capabilties in PF and delete the VFs + self._disable_sriov_in_pf(pci_info_no_sriov) + + fake_connection = self._get_connection(host_info, + pci_info=pci_info_no_sriov, + hostname='test_compute0') + self.mock_conn.return_value = fake_connection + + self.compute = self.start_service('compute', host='test_compute0') + + ctxt = context.get_admin_context() + pci_devices = objects.PciDeviceList.get_by_compute_node( + ctxt, + objects.ComputeNode.get_by_nodename( + ctxt, 'test_compute0', + ).id, + ) + self.assertEqual(1, len(pci_devices)) + self.assertEqual('type-PCI', pci_devices[0].dev_type) + + # Update connection with original pci info with sriov PFs + fake_connection = self._get_connection(host_info, + pci_info=pci_info, + hostname='test_compute0') + self.mock_conn.return_value = fake_connection + + # Restart the compute service + self.restart_compute_service(self.compute) + self.compute_started = True + + # Verify if PCI devices are of type type-PF or type-VF + pci_devices = objects.PciDeviceList.get_by_compute_node( + ctxt, + objects.ComputeNode.get_by_nodename( + ctxt, 'test_compute0', + ).id, + ) + for pci_device in pci_devices: + self.assertIn(pci_device.dev_type, ['type-PF', 'type-VF']) + + # Create a flavor + extra_spec = {"pci_passthrough:alias": "%s:1" % self.VFS_ALIAS_NAME} + flavor_id = self._create_flavor(extra_spec=extra_spec) + + self._run_build_test(flavor_id) + class GetServerDiagnosticsServerWithVfTestV21(_PCIServersTestBase): diff --git a/nova/tests/functional/regressions/test_bug_1888395.py b/nova/tests/functional/regressions/test_bug_1888395.py new file mode 100644 index 00000000000..69576fd094a --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_1888395.py @@ -0,0 +1,163 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import fixtures +import mock + +from lxml import etree +import six.moves.urllib.parse as urlparse + +from nova import context +from nova.network.neutronv2 import api as neutron +from nova.network.neutronv2 import constants as neutron_constants +from nova.tests.functional import integrated_helpers +from nova.tests.functional.libvirt import base as libvirt_base +from nova.tests.unit.virt.libvirt import fake_os_brick_connector +from nova.tests.unit.virt.libvirt import fakelibvirt +from nova.virt.libvirt import guest as libvirt_guest + + +class TestLiveMigrationWithoutMultiplePortBindings( + integrated_helpers.InstanceHelperMixin, + libvirt_base.ServersTestBase): + """Regression test for bug 1888395. + + This regression test asserts that Live migration works when + neutron does not support the binding-extended api extension + and the legacy single port binding workflow is used. + """ + + ADMIN_API = True + api_major_version = 'v2.1' + microversion = 'latest' + + def list_extensions(self, *args, **kwargs): + return { + 'extensions': [ + { + # Copied from neutron-lib portbindings.py + "updated": "2014-02-03T10:00:00-00:00", + "name": neutron_constants.PORT_BINDING, + "links": [], + "alias": "binding", + "description": "Expose port bindings of a virtual port to " + "external application" + } + ] + } + + def setUp(self): + self.flags(instances_path=self.useFixture(fixtures.TempDir()).path) + super(TestLiveMigrationWithoutMultiplePortBindings, self).setUp() + self.neutron.list_extensions = self.list_extensions + self.neutron_api = neutron.API() + # TODO(sean-k-mooney): remove after + # I275509eb0e0eb9eaf26fe607b7d9a67e1edc71f8 + # has merged. + self.useFixture(fixtures.MonkeyPatch( + 'nova.virt.libvirt.driver.connector', + fake_os_brick_connector)) + + self.computes = {} + for host in ['start_host', 'end_host']: + host_info = fakelibvirt.HostInfo( + cpu_nodes=1, cpu_sockets=1, cpu_cores=4, cpu_threads=2, + kB_mem=10740000) + fake_connection = self._get_connection( + host_info=host_info, hostname=host) + + # This is fun. Firstly we need to do a global'ish mock so we can + # actually start the service. + with mock.patch('nova.virt.libvirt.host.Host.get_connection', + return_value=fake_connection): + compute = self.start_service('compute', host=host) + + # Once that's done, we need to do some tweaks to each individual + # compute "service" to make sure they return unique objects + compute.driver._host.get_connection = lambda: fake_connection + self.computes[host] = compute + + self.ctxt = context.get_admin_context() + # TODO(sean-k-mooney): remove this when it is part of ServersTestBase + self.useFixture(fixtures.MonkeyPatch( + 'nova.tests.unit.virt.libvirt.fakelibvirt.Domain.migrateToURI3', + self._migrate_stub)) + self.useFixture(fixtures.MonkeyPatch( + 'nova.virt.libvirt.migration._update_serial_xml', + self._update_serial_xml_stub)) + + def _update_serial_xml_stub(self, xml_doc, migrate_data): + return xml_doc + + def _migrate_stub(self, domain, destination, params, flags): + """Stub out migrateToURI3.""" + + src_hostname = domain._connection.hostname + dst_hostname = urlparse.urlparse(destination).netloc + + # In a real live migration, libvirt and QEMU on the source and + # destination talk it out, resulting in the instance starting to exist + # on the destination. Fakelibvirt cannot do that, so we have to + # manually create the "incoming" instance on the destination + # fakelibvirt. + dst = self.computes[dst_hostname] + dst.driver._host.get_connection().createXML( + params['destination_xml'], + 'fake-createXML-doesnt-care-about-flags') + + src = self.computes[src_hostname] + conn = src.driver._host.get_connection() + + # because migrateToURI3 is spawned in a background thread, this method + # does not block the upper nova layers. Because we don't want nova to + # think the live migration has finished until this method is done, the + # last thing we do is make fakelibvirt's Domain.jobStats() return + # VIR_DOMAIN_JOB_COMPLETED. + server = etree.fromstring( + params['destination_xml'] + ).find('./uuid').text + dom = conn.lookupByUUIDString(server) + dom.complete_job() + + @mock.patch('nova.virt.libvirt.guest.Guest.get_job_info') + def test_live_migrate(self, mock_get_job_info): + mock_get_job_info.return_value = libvirt_guest.JobInfo( + type=fakelibvirt.VIR_DOMAIN_JOB_COMPLETED) + flavors = self.api.get_flavors() + flavor = flavors[0] + server_req = self._build_minimal_create_server_request( + self.api, 'some-server', flavor_id=flavor['id'], + image_uuid='155d900f-4e14-4e4c-a73d-069cbf4541e6', + networks=[{'port': self.neutron.port_1['id']}]) + server_req['availability_zone'] = 'nova:%s' % "start_host" + created_server = self.api.post_server({'server': server_req}) + server = self._wait_for_state_change( + self.api, created_server, 'ACTIVE') + self.assertFalse( + self.neutron_api.supports_port_binding_extension(self.ctxt)) + # TODO(sean-k-mooney): extend _live_migrate to support passing a host + self.api.post_server_action( + server['id'], + { + 'os-migrateLive': { + 'host': 'end_host', + 'block_migration': 'auto' + } + } + ) + + self._wait_for_server_parameter( + self.api, server, + {'OS-EXT-SRV-ATTR:host': 'end_host', 'status': 'ACTIVE'}) + + msg = "NotImplementedError: Cannot load 'vif_type' in the base class" + self.assertNotIn(msg, self.stdlog.logger.output) diff --git a/nova/tests/functional/regressions/test_bug_1894966.py b/nova/tests/functional/regressions/test_bug_1894966.py new file mode 100644 index 00000000000..e7e40690848 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_1894966.py @@ -0,0 +1,40 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from nova import test +from nova.tests import fixtures as nova_fixtures +from nova.tests.functional.api import client +from nova.tests.functional import integrated_helpers + + +class TestCreateServerGroupWithEmptyPolicies( + test.TestCase, integrated_helpers.InstanceHelperMixin, +): + """Demonstrate bug #1894966. + + Attempt to create a server group with an invalid 'policies' field. It + should fail cleanly. + """ + def setUp(self): + super(TestCreateServerGroupWithEmptyPolicies, self).setUp() + + api_fixture = self.useFixture(nova_fixtures.OSAPIFixture( + api_version='v2.1')) + self.api = api_fixture.api + self.api.microversion = '2.63' # the last version with the bug + + def test_create_with_empty_policies(self): + exc = self.assertRaises( + client.OpenStackApiException, + self.api.post_server_groups, + {'name': 'test group', 'policies': []}) + self.assertEqual(400, exc.response.status_code) diff --git a/nova/tests/functional/test_monkey_patch.py b/nova/tests/functional/test_monkey_patch.py new file mode 100644 index 00000000000..b471d333cf7 --- /dev/null +++ b/nova/tests/functional/test_monkey_patch.py @@ -0,0 +1,45 @@ +# Copyright 2020 Red Hat, Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +# NOTE(artom) This file exists to test eventlet monkeypatching. How and what +# eventlet monkeypatches can be controlled by environment variables that +# are processed by eventlet at import-time (for exmaple, EVENTLET_NO_GREENDNS). +# Nova manages all of this in nova.monkey_patch. Therefore, nova.monkey_patch +# must be the first thing to import eventlet. As nova.tests.functional.__init__ +# imports nova.monkey_patch, we're OK here. + +import socket +import traceback + +from nova import test + + +class TestMonkeyPatch(test.TestCase): + + def test_greendns_is_disabled(self): + """Try to resolve a fake fqdn. If we see greendns mentioned in the + traceback of the raised exception, it means we've not actually disabled + greendns. See the TODO and NOTE in nova.monkey_patch to understand why + greendns needs to be disabled. + """ + raised = False + try: + socket.gethostbyname('goat.fake') + except Exception: + tb = traceback.format_exc() + # NOTE(artom) If we've correctly disabled greendns, we expect the + # traceback to not contain any reference to it. + self.assertNotIn('greendns.py', tb) + raised = True + self.assertTrue(raised) diff --git a/nova/tests/unit/api/openstack/compute/test_aggregates.py b/nova/tests/unit/api/openstack/compute/test_aggregates.py index eb185849a64..c14f562dc72 100644 --- a/nova/tests/unit/api/openstack/compute/test_aggregates.py +++ b/nova/tests/unit/api/openstack/compute/test_aggregates.py @@ -299,7 +299,7 @@ def test_show_no_admin(self): self.controller.show, self.user_req, "1") - def test_show_with_invalid_id(self): + def test_show_with_bad_aggregate(self): side_effect = exception.AggregateNotFound(aggregate_id='2') with mock.patch.object(self.controller.api, 'get_aggregate', side_effect=side_effect) as mock_get: @@ -307,6 +307,10 @@ def test_show_with_invalid_id(self): self.req, "2") mock_get.assert_called_once_with(self.context, '2') + def test_show_with_invalid_id(self): + self.assertRaises(exc.HTTPBadRequest, self.controller.show, + self.req, 'foo') + def test_update(self): body = {"aggregate": {"name": "new_name", "availability_zone": "nova1"}} @@ -390,10 +394,11 @@ def test_update_with_empty_availability_zone(self): @mock.patch('nova.compute.api.AggregateAPI.update_aggregate') def test_update_with_none_availability_zone(self, mock_update_agg): - agg_id = uuidsentinel.aggregate + agg_id = 173 mock_update_agg.return_value = objects.Aggregate(self.context, name='test', - uuid=agg_id, + uuid=uuidsentinel.agg, + id=agg_id, hosts=[], metadata={}) body = {"aggregate": {"name": "test", @@ -414,6 +419,11 @@ def test_update_with_bad_aggregate(self): mock_update.assert_called_once_with(self.context, '2', body["aggregate"]) + def test_update_with_invalid_id(self): + body = {"aggregate": {"name": "test_name"}} + self.assertRaises(exc.HTTPBadRequest, self.controller.update, + self.req, 'foo', body=body) + def test_update_with_duplicated_name(self): body = {"aggregate": {"name": "test_name"}} side_effect = exception.AggregateNameExists(aggregate_name="test_name") @@ -433,7 +443,7 @@ def test_invalid_action(self): def test_update_with_invalid_action(self): with mock.patch.object(self.controller.api, "update_aggregate", side_effect=exception.InvalidAggregateAction( - action='invalid', aggregate_id='agg1', reason= "not empty")): + action='invalid', aggregate_id='1', reason= "not empty")): body = {"aggregate": {"availability_zone": "nova"}} self.assertRaises(exc.HTTPBadRequest, self.controller.update, self.req, "1", body=body) @@ -467,15 +477,20 @@ def test_add_host_with_already_added_host(self): def test_add_host_with_bad_aggregate(self): side_effect = exception.AggregateNotFound( - aggregate_id="bogus_aggregate") + aggregate_id="2") with mock.patch.object(self.controller.api, 'add_host_to_aggregate', side_effect=side_effect) as mock_add: self.assertRaises(exc.HTTPNotFound, eval(self.add_host), - self.req, "bogus_aggregate", + self.req, "2", body={"add_host": {"host": "host1"}}) - mock_add.assert_called_once_with(self.context, "bogus_aggregate", + mock_add.assert_called_once_with(self.context, "2", "host1") + def test_add_host_with_invalid_id(self): + body = {"add_host": {"host": "host1"}} + self.assertRaises(exc.HTTPBadRequest, eval(self.add_host), + self.req, 'foo', body=body) + def test_add_host_with_bad_host(self): side_effect = exception.ComputeHostNotFound(host="bogus_host") with mock.patch.object(self.controller.api, 'add_host_to_aggregate', @@ -534,16 +549,21 @@ def test_remove_host_no_admin(self): def test_remove_host_with_bad_aggregate(self): side_effect = exception.AggregateNotFound( - aggregate_id="bogus_aggregate") + aggregate_id="2") with mock.patch.object(self.controller.api, 'remove_host_from_aggregate', side_effect=side_effect) as mock_rem: self.assertRaises(exc.HTTPNotFound, eval(self.remove_host), - self.req, "bogus_aggregate", + self.req, "2", body={"remove_host": {"host": "host1"}}) - mock_rem.assert_called_once_with(self.context, "bogus_aggregate", + mock_rem.assert_called_once_with(self.context, "2", "host1") + def test_remove_host_with_invalid_id(self): + body = {"remove_host": {"host": "host1"}} + self.assertRaises(exc.HTTPBadRequest, eval(self.remove_host), + self.req, 'foo', body=body) + def test_remove_host_with_host_not_in_aggregate(self): side_effect = exception.AggregateHostNotFound(aggregate_id="1", host="host1") @@ -621,16 +641,21 @@ def test_set_metadata_no_admin(self): def test_set_metadata_with_bad_aggregate(self): body = {"set_metadata": {"metadata": {"foo": "bar"}}} - side_effect = exception.AggregateNotFound(aggregate_id="bad_aggregate") + side_effect = exception.AggregateNotFound(aggregate_id="2") with mock.patch.object(self.controller.api, 'update_aggregate_metadata', side_effect=side_effect) as mock_update: self.assertRaises(exc.HTTPNotFound, eval(self.set_metadata), - self.req, "bad_aggregate", body=body) - mock_update.assert_called_once_with(self.context, "bad_aggregate", + self.req, "2", body=body) + mock_update.assert_called_once_with(self.context, "2", body["set_metadata"]['metadata']) + def test_set_metadata_with_invalid_id(self): + body = {"set_metadata": {"metadata": {"foo": "bar"}}} + self.assertRaises(exc.HTTPBadRequest, eval(self.set_metadata), + self.req, 'foo', body=body) + def test_set_metadata_with_missing_metadata(self): body = {"asdf": {"foo": "bar"}} self.assertRaises(self.bad_request, eval(self.set_metadata), @@ -679,21 +704,25 @@ def test_delete_aggregate_no_admin(self): def test_delete_aggregate_with_bad_aggregate(self): side_effect = exception.AggregateNotFound( - aggregate_id="bogus_aggregate") + aggregate_id="2") with mock.patch.object(self.controller.api, 'delete_aggregate', side_effect=side_effect) as mock_del: self.assertRaises(exc.HTTPNotFound, self.controller.delete, - self.req, "bogus_aggregate") - mock_del.assert_called_once_with(self.context, "bogus_aggregate") + self.req, "2") + mock_del.assert_called_once_with(self.context, "2") + + def test_delete_with_invalid_id(self): + self.assertRaises(exc.HTTPBadRequest, self.controller.delete, + self.req, 'foo') def test_delete_aggregate_with_host(self): with mock.patch.object(self.controller.api, "delete_aggregate", side_effect=exception.InvalidAggregateAction( - action="delete", aggregate_id="agg1", + action="delete", aggregate_id="2", reason="not empty")): self.assertRaises(exc.HTTPBadRequest, self.controller.delete, - self.req, "agg1") + self.req, "2") def test_marshall_aggregate(self): # _marshall_aggregate() just basically turns the aggregate returned diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index 80af96ebb6d..d5736ceccef 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -4993,7 +4993,8 @@ def test_create_instance_bdm_api_validation_fails(self): (exception.InvalidBDMVolume, {'id': 'fake'}), (exception.InvalidBDMImage, {'id': 'fake'}), (exception.InvalidBDMBootSequence, {}), - (exception.InvalidBDMLocalsLimit, {})) + (exception.InvalidBDMLocalsLimit, {}), + (exception.InvalidBDMDiskBus, {'disk_bus': 'foo'})) ex_iter = iter(bdm_exceptions) diff --git a/nova/tests/unit/api/openstack/compute/test_shelve.py b/nova/tests/unit/api/openstack/compute/test_shelve.py index 8d41490d862..e51729935bf 100644 --- a/nova/tests/unit/api/openstack/compute/test_shelve.py +++ b/nova/tests/unit/api/openstack/compute/test_shelve.py @@ -54,6 +54,7 @@ def test_shelve_task_state_race(self, mock_save, get_instance_mock): self.req.environ['nova.context'], vm_state=vm_states.ACTIVE, task_state=None) instance.launched_at = instance.created_at + instance.system_metadata = {} get_instance_mock.return_value = instance mock_save.side_effect = exception.UnexpectedTaskStateError( instance_uuid=instance.uuid, expected=None, diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 6a833d4073f..e60373f1296 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -18,6 +18,7 @@ """Tests for compute service.""" import datetime +import fixtures as std_fixtures from itertools import chain import operator import sys @@ -6144,9 +6145,15 @@ def test_pre_live_migration_works_correctly(self, mock_get_bdms, # Confirm setup_compute_volume is called when volume is mounted. def stupid(*args, **kwargs): return fake_network.fake_get_instance_nw_info(self) + if CONF.use_neutron: self.stub_out( 'nova.network.neutronv2.api.API.get_instance_nw_info', stupid) + self.useFixture( + std_fixtures.MonkeyPatch( + 'nova.network.neutronv2.api.' + 'API.supports_port_binding_extension', + lambda *args: True)) else: self.stub_out('nova.network.api.API.get_instance_nw_info', stupid) @@ -6157,6 +6164,9 @@ def stupid(*args, **kwargs): fake_notifier.NOTIFICATIONS = [] migrate_data = objects.LibvirtLiveMigrateData( is_shared_instance_path=False) + vifs = migrate_data_obj.VIFMigrateData.create_skeleton_migrate_vifs( + stupid()) + migrate_data.vifs = vifs mock_pre.return_value = migrate_data with mock.patch.object(self.compute.network_api, @@ -6484,8 +6494,9 @@ def fakecleanup(*args, **kwargs): mock.patch.object( self.compute.network_api, 'setup_networks_on_host'), mock.patch.object(migration_obj, 'save'), + mock.patch.object(instance, 'get_network_info', return_value=[]), ) as ( - mock_migrate, mock_setup, mock_mig_save + mock_migrate, mock_setup, mock_mig_save, mock_get_nw_info ): self.compute._post_live_migration(c, instance, dest, migrate_data=migrate_data, @@ -6497,6 +6508,7 @@ def fakecleanup(*args, **kwargs): mock_migrate.assert_called_once_with(c, instance, migration) mock_post.assert_called_once_with(c, instance, False, dest) mock_clear.assert_called_once_with(mock.ANY) + mock_get_nw_info.assert_called() @mock.patch('nova.compute.utils.notify_about_instance_action') def test_post_live_migration_working_correctly(self, mock_notify): @@ -6539,12 +6551,15 @@ def test_post_live_migration_working_correctly(self, mock_notify): 'clear_events_for_instance'), mock.patch.object(self.compute, 'update_available_resource'), mock.patch.object(migration_obj, 'save'), + mock.patch.object(instance, 'get_network_info'), ) as ( post_live_migration, unfilter_instance, migrate_instance_start, post_live_migration_at_destination, post_live_migration_at_source, setup_networks_on_host, - clear_events, update_available_resource, mig_save + clear_events, update_available_resource, mig_save, get_nw_info, ): + nw_info = network_model.NetworkInfo.hydrate([]) + get_nw_info.return_value = nw_info self.compute._post_live_migration(c, instance, dest, migrate_data=migrate_data, source_bdms=bdms) @@ -6567,7 +6582,7 @@ def test_post_live_migration_working_correctly(self, mock_notify): post_live_migration_at_destination.assert_has_calls([ mock.call(c, instance, False, dest)]) post_live_migration_at_source.assert_has_calls( - [mock.call(c, instance, [])]) + [mock.call(c, instance, nw_info)]) clear_events.assert_called_once_with(instance) update_available_resource.assert_has_calls([mock.call(c)]) self.assertEqual('completed', migration_obj.status) @@ -9130,6 +9145,7 @@ def fake_rpc_rebuild(context, **kwargs): 'image_ramdisk_id': uuids.ramdisk_id, 'image_something_else': 'meow', 'preserved': 'preserve this!', + 'image_base_image_ref': image_ref, 'boot_roles': ''}, sys_meta) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 8db729b7746..c12f669bfb1 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -4529,6 +4529,22 @@ def test_validate_bdm_missing_volume_size(self, mock_get_image): bdms) mock_get_image.assert_called_once_with(self.context, image_id) + @mock.patch('nova.compute.api.API._get_image') + def test_validate_bdm_disk_bus(self, mock_get_image): + """Tests that _validate_bdm fail if an invalid disk_bus is provided + """ + instance = self._create_instance_obj() + bdms = objects.BlockDeviceMappingList(objects=[ + objects.BlockDeviceMapping( + boot_index=0, image_id=instance.image_ref, + source_type='image', destination_type='volume', + volume_type=None, snapshot_id=None, volume_id=None, + volume_size=1, disk_bus='virtio-scsi')]) + self.assertRaises(exception.InvalidBDMDiskBus, + self.compute_api._validate_bdm, + self.context, instance, objects.Flavor(), + bdms) + @mock.patch.object(objects.service, 'get_minimum_version_all_cells', return_value=compute_api.MIN_COMPUTE_VOLUME_TYPE) def test_the_specified_volume_type_id_assignment_to_name( diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index d608fcd00bc..954691b38cc 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -15,6 +15,7 @@ import contextlib import copy import datetime +import fixtures as std_fixtures import time from cinderclient import exceptions as cinder_exception @@ -993,6 +994,11 @@ def test_init_host_with_in_progress_evacuations(self, mock_destroy_evac, mock_error_interrupted.assert_called_once_with( self.context, {active_instance.uuid, evacuating_instance.uuid}) + def test_init_host_disk_devices_configuration_failure(self): + self.flags(max_disk_devices_to_attach=0, group='compute') + self.assertRaises(exception.InvalidConfiguration, + self.compute.init_host) + @mock.patch.object(objects.InstanceList, 'get_by_host', new=mock.Mock()) @mock.patch('nova.compute.manager.ComputeManager.' @@ -3220,22 +3226,80 @@ def _test_check_can_live_migrate_destination(self, do_raise=False, mock_event.assert_called_once_with( self.context, 'compute_check_can_live_migrate_destination', CONF.host, instance.uuid) + return result + @mock.patch('nova.objects.InstanceGroup.get_by_instance_uuid', mock.Mock( + side_effect=exception.InstanceGroupNotFound(group_uuid=''))) def test_check_can_live_migrate_destination_success(self): + self.useFixture(std_fixtures.MonkeyPatch( + 'nova.network.neutronv2.api.API.supports_port_binding_extension', + lambda *args: True)) self._test_check_can_live_migrate_destination() + @mock.patch('nova.objects.InstanceGroup.get_by_instance_uuid', mock.Mock( + side_effect=exception.InstanceGroupNotFound(group_uuid=''))) def test_check_can_live_migrate_destination_fail(self): + self.useFixture(std_fixtures.MonkeyPatch( + 'nova.network.neutronv2.api.API.supports_port_binding_extension', + lambda *args: True)) self.assertRaises( - test.TestingException, - self._test_check_can_live_migrate_destination, - do_raise=True) - + test.TestingException, + self._test_check_can_live_migrate_destination, + do_raise=True) + + @mock.patch('nova.objects.InstanceGroup.get_by_instance_uuid', mock.Mock( + side_effect=exception.InstanceGroupNotFound(group_uuid=''))) + def test_check_can_live_migrate_destination_contains_vifs(self): + self.useFixture(std_fixtures.MonkeyPatch( + 'nova.network.neutronv2.api.API.supports_port_binding_extension', + lambda *args: True)) + migrate_data = self._test_check_can_live_migrate_destination() + self.assertIn('vifs', migrate_data) + self.assertIsNotNone(migrate_data.vifs) + + @mock.patch('nova.objects.InstanceGroup.get_by_instance_uuid', mock.Mock( + side_effect=exception.InstanceGroupNotFound(group_uuid=''))) + def test_check_can_live_migrate_destination_no_binding_extended(self): + self.useFixture(std_fixtures.MonkeyPatch( + 'nova.network.neutronv2.api.API.supports_port_binding_extension', + lambda *args: False)) + migrate_data = self._test_check_can_live_migrate_destination() + self.assertNotIn('vifs', migrate_data) + + @mock.patch('nova.objects.InstanceGroup.get_by_instance_uuid', mock.Mock( + side_effect=exception.InstanceGroupNotFound(group_uuid=''))) def test_check_can_live_migrate_destination_src_numa_lm_false(self): + self.useFixture(std_fixtures.MonkeyPatch( + 'nova.network.neutronv2.api.API.supports_port_binding_extension', + lambda *args: True)) self._test_check_can_live_migrate_destination(src_numa_lm=False) + @mock.patch('nova.objects.InstanceGroup.get_by_instance_uuid', mock.Mock( + side_effect=exception.InstanceGroupNotFound(group_uuid=''))) def test_check_can_live_migrate_destination_src_numa_lm_true(self): + self.useFixture(std_fixtures.MonkeyPatch( + 'nova.network.neutronv2.api.API.supports_port_binding_extension', + lambda *args: True)) self._test_check_can_live_migrate_destination(src_numa_lm=True) + @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') + def test_check_can_live_migrate_destination_fail_group_policy( + self, mock_fail_db): + + instance = fake_instance.fake_instance_obj( + self.context, host=self.compute.host, vm_state=vm_states.ACTIVE, + node='fake-node') + + ex = exception.RescheduledException( + instance_uuid=instance.uuid, reason="policy violated") + + with mock.patch.object(self.compute, '_validate_instance_group_policy', + side_effect=ex): + self.assertRaises( + exception.MigrationPreCheckError, + self.compute.check_can_live_migrate_destination, + self.context, instance, None, None, None, None) + def test_dest_can_numa_live_migrate(self): positive_dest_check_data = objects.LibvirtLiveMigrateData( dst_supports_numa_live_migration=True) @@ -6975,7 +7039,8 @@ def test_prep_block_device_maintain_original_error_message(self, def test_validate_policy_honors_workaround_disabled(self, mock_get): instance = objects.Instance(uuid=uuids.instance) hints = {'group': 'foo'} - mock_get.return_value = objects.InstanceGroup(policy=None) + mock_get.return_value = objects.InstanceGroup(policy=None, + uuid=uuids.group) self.compute._validate_instance_group_policy(self.context, instance, hints) mock_get.assert_called_once_with(self.context, 'foo') @@ -7001,10 +7066,14 @@ def test_validate_instance_group_policy_handles_hint_list(self, mock_get): instance, hints) mock_get.assert_called_once_with(self.context, uuids.group_hint) + @mock.patch('nova.objects.InstanceGroup.get_by_uuid') @mock.patch('nova.objects.InstanceList.get_uuids_by_host') @mock.patch('nova.objects.InstanceGroup.get_by_hint') - def test_validate_instance_group_policy_with_rules(self, mock_get_by_hint, - mock_get_by_host): + @mock.patch.object(fake_driver.FakeDriver, 'get_available_nodes') + @mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node') + def test_validate_instance_group_policy_with_rules( + self, migration_list, nodes, mock_get_by_hint, mock_get_by_host, + mock_get_by_uuid): # Create 2 instance in same host, inst2 created before inst1 instance = objects.Instance(uuid=uuids.inst1) hints = {'group': [uuids.group_hint]} @@ -7013,17 +7082,26 @@ def test_validate_instance_group_policy_with_rules(self, mock_get_by_hint, mock_get_by_host.return_value = existing_insts # if group policy rules limit to 1, raise RescheduledException - mock_get_by_hint.return_value = objects.InstanceGroup( + group = objects.InstanceGroup( policy='anti-affinity', rules={'max_server_per_host': '1'}, - hosts=['host1'], members=members_uuids) + hosts=['host1'], members=members_uuids, + uuid=uuids.group) + mock_get_by_hint.return_value = group + mock_get_by_uuid.return_value = group + nodes.return_value = ['nodename'] + migration_list.return_value = [objects.Migration( + uuid=uuids.migration, instance_uuid=uuids.instance)] self.assertRaises(exception.RescheduledException, self.compute._validate_instance_group_policy, self.context, instance, hints) # if group policy rules limit change to 2, validate OK - mock_get_by_hint.return_value = objects.InstanceGroup( + group2 = objects.InstanceGroup( policy='anti-affinity', rules={'max_server_per_host': 2}, - hosts=['host1'], members=members_uuids) + hosts=['host1'], members=members_uuids, + uuid=uuids.group) + mock_get_by_hint.return_value = group2 + mock_get_by_uuid.return_value = group2 self.compute._validate_instance_group_policy(self.context, instance, hints) @@ -8550,6 +8628,8 @@ def test_max_concurrent_live_semaphore_unlimited(self, mock_executor): manager.ComputeManager() mock_executor.assert_called_once_with() + @mock.patch('nova.objects.InstanceGroup.get_by_instance_uuid', mock.Mock( + side_effect=exception.InstanceGroupNotFound(group_uuid=''))) def test_pre_live_migration_cinder_v3_api(self): # This tests that pre_live_migration with a bdm with an # attachment_id, will create a new attachment and update @@ -8627,6 +8707,8 @@ def _test(mock_attach, mock_get_bdms, mock_ivbi, _test() + @mock.patch('nova.objects.InstanceGroup.get_by_instance_uuid', mock.Mock( + side_effect=exception.InstanceGroupNotFound(group_uuid=''))) def test_pre_live_migration_exception_cinder_v3_api(self): # The instance in this test has 2 attachments. The second attach_create # will throw an exception. This will test that the first attachment @@ -8696,6 +8778,8 @@ def _test(mock_attach_create, mock_attach_delete, mock_get_bdms, self.assertGreater(len(m.mock_calls), 0) _test() + @mock.patch('nova.objects.InstanceGroup.get_by_instance_uuid', mock.Mock( + side_effect=exception.InstanceGroupNotFound(group_uuid=''))) def test_pre_live_migration_exceptions_delete_attachments(self): # The instance in this test has 2 attachments. The call to # driver.pre_live_migration will raise an exception. This will test @@ -8777,6 +8861,16 @@ def test_get_neutron_events_for_live_migration_empty(self): self.flags(vif_plugging_timeout=300, use_neutron=True) self.assertEqual( [], self.compute._get_neutron_events_for_live_migration([])) + # 4. no plug time events + with mock.patch.object(self.instance, 'get_network_info') as nw_info: + nw_info.return_value = network_model.NetworkInfo( + [network_model.VIF( + uuids.port1, details={ + network_model.VIF_DETAILS_OVS_HYBRID_PLUG: False})]) + self.assertFalse(nw_info.return_value[0].is_hybrid_plug_enabled()) + self.assertEqual( + [], self.compute._get_neutron_events_for_live_migration( + self.instance)) @mock.patch('nova.compute.rpcapi.ComputeAPI.pre_live_migration') @mock.patch('nova.compute.manager.ComputeManager._post_live_migration') @@ -8791,9 +8885,11 @@ def test_live_migration_wait_vif_plugged( wait_for_vif_plugged=True) mock_get_bdms.return_value = objects.BlockDeviceMappingList(objects=[]) mock_pre_live_mig.return_value = migrate_data + details = {network_model.VIF_DETAILS_OVS_HYBRID_PLUG: True} self.instance.info_cache = objects.InstanceInfoCache( network_info=network_model.NetworkInfo([ - network_model.VIF(uuids.port1), network_model.VIF(uuids.port2) + network_model.VIF(uuids.port1, details=details), + network_model.VIF(uuids.port2, details=details) ])) self.compute._waiting_live_migrations[self.instance.uuid] = ( self.migration, mock.MagicMock() @@ -8823,11 +8919,12 @@ def test_live_migration_wait_vif_plugged_old_dest_host( of not waiting. """ migrate_data = objects.LibvirtLiveMigrateData() + details = {network_model.VIF_DETAILS_OVS_HYBRID_PLUG: True} mock_get_bdms.return_value = objects.BlockDeviceMappingList(objects=[]) mock_pre_live_mig.return_value = migrate_data self.instance.info_cache = objects.InstanceInfoCache( network_info=network_model.NetworkInfo([ - network_model.VIF(uuids.port1)])) + network_model.VIF(uuids.port1, details=details)])) self.compute._waiting_live_migrations[self.instance.uuid] = ( self.migration, mock.MagicMock() ) @@ -8971,9 +9068,11 @@ def test_live_migration_aborted_before_running(self, mock_rpc, mock_get_bdms.return_value = source_bdms migrate_data = objects.LibvirtLiveMigrateData( wait_for_vif_plugged=True) + details = {network_model.VIF_DETAILS_OVS_HYBRID_PLUG: True} self.instance.info_cache = objects.InstanceInfoCache( network_info=network_model.NetworkInfo([ - network_model.VIF(uuids.port1), network_model.VIF(uuids.port2) + network_model.VIF(uuids.port1, details=details), + network_model.VIF(uuids.port2, details=details) ])) self.compute._waiting_live_migrations = {} fake_migration = objects.Migration( @@ -9317,6 +9416,7 @@ def test_post_live_migration_cinder_v3_api(self): source_node='src') image_bdm.attachment_id = uuids.attachment3 + @mock.patch.object(instance, 'get_network_info') @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.objects.ConsoleAuthToken.' 'clean_console_auths_for_instance') @@ -9335,7 +9435,7 @@ def test_post_live_migration_cinder_v3_api(self): def _test(mock_net_api, mock_notify, mock_driver, mock_rpc, mock_get_bdm_info, mock_attach_delete, mock_update_resource, mock_bdm_save, mock_ga, - mock_clean, mock_notify_action): + mock_clean, mock_notify_action, mock_get_nw_info): self._mock_rt() bdms = objects.BlockDeviceMappingList(objects=[vol_bdm, image_bdm]) @@ -9355,6 +9455,7 @@ def _test(mock_net_api, mock_notify, mock_driver, mock.call(self.context, instance, 'fake-mini', action='live_migration_post', phase='end')]) self.assertEqual(2, mock_notify_action.call_count) + mock_get_nw_info.assert_called() _test() @@ -10030,6 +10131,54 @@ def fake_reschedule_resize_or_reraise(*args, **kwargs): # (_error_out_instance_on_exception will set to ACTIVE by default). self.assertEqual(vm_states.STOPPED, instance.vm_state) + @mock.patch('nova.compute.utils.notify_usage_exists') + @mock.patch('nova.compute.manager.ComputeManager.' + '_notify_about_instance_usage') + @mock.patch('nova.compute.utils.notify_about_resize_prep_instance') + @mock.patch('nova.objects.Instance.save') + @mock.patch('nova.compute.manager.ComputeManager._revert_allocation') + @mock.patch('nova.compute.manager.ComputeManager.' + '_reschedule_resize_or_reraise') + @mock.patch('nova.compute.utils.add_instance_fault_from_exc') + # this is almost copy-paste from test_prep_resize_fails_rollback + def test_prep_resize_fails_group_validation( + self, add_instance_fault_from_exc, _reschedule_resize_or_reraise, + _revert_allocation, mock_instance_save, + notify_about_resize_prep_instance, _notify_about_instance_usage, + notify_usage_exists): + """Tests that if _validate_instance_group_policy raises + InstanceFaultRollback, the instance.vm_state is reset properly in + _error_out_instance_on_exception + """ + instance = fake_instance.fake_instance_obj( + self.context, host=self.compute.host, vm_state=vm_states.STOPPED, + node='fake-node', expected_attrs=['system_metadata', 'flavor']) + migration = mock.MagicMock(spec='nova.objects.Migration') + request_spec = mock.MagicMock(spec='nova.objects.RequestSpec') + ex = exception.RescheduledException( + instance_uuid=instance.uuid, reason="policy violated") + ex2 = exception.InstanceFaultRollback( + inner_exception=ex) + + def fake_reschedule_resize_or_reraise(*args, **kwargs): + raise ex2 + + _reschedule_resize_or_reraise.side_effect = ( + fake_reschedule_resize_or_reraise) + + with mock.patch.object(self.compute, '_validate_instance_group_policy', + side_effect=ex): + self.assertRaises( + # _error_out_instance_on_exception should reraise the + # RescheduledException inside InstanceFaultRollback. + exception.RescheduledException, self.compute.prep_resize, + self.context, instance.image_meta, instance, instance.flavor, + request_spec, filter_properties={}, node=instance.node, + clean_shutdown=True, migration=migration, host_list=[]) + # The instance.vm_state should remain unchanged + # (_error_out_instance_on_exception will set to ACTIVE by default). + self.assertEqual(vm_states.STOPPED, instance.vm_state) + @mock.patch('nova.compute.rpcapi.ComputeAPI.resize_instance') @mock.patch('nova.compute.resource_tracker.ResourceTracker.resize_claim') @mock.patch('nova.objects.Instance.save') diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 0f6689eba24..525d673f11e 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -2281,6 +2281,27 @@ def test_schedule_and_build_instances_fill_request_spec_noop( self.params['request_specs'][0].requested_resources = [] self._do_schedule_and_build_instances_test(self.params) + def test_map_instance_to_cell_already_mapped(self): + """Tests a scenario where an instance is already mapped to a cell + during scheduling. + """ + build_request = self.params['build_requests'][0] + instance = build_request.get_new_instance(self.ctxt) + # Simulate MQ split brain craziness by updating the instance mapping + # to point at cell0. + inst_mapping = objects.InstanceMapping.get_by_instance_uuid( + self.ctxt, instance.uuid) + inst_mapping.cell_mapping = self.cell_mappings['cell0'] + inst_mapping.save() + cell1 = self.cell_mappings['cell1'] + inst_mapping = self.conductor._map_instance_to_cell( + self.ctxt, instance, cell1) + # Assert that the instance mapping was updated to point at cell1 but + # also that an error was logged. + self.assertEqual(cell1.uuid, inst_mapping.cell_mapping.uuid) + self.assertIn('During scheduling instance is already mapped to ' + 'another cell', self.stdlog.logger.output) + @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') def test_cleanup_build_artifacts(self, inst_map_get): """Simple test to ensure the order of operations in the cleanup method @@ -2378,6 +2399,17 @@ def test_bury_in_cell0(self, mock_notify): build_requests = objects.BuildRequestList.get_all(self.ctxt) instances = objects.InstanceList.get_all(self.ctxt) + # Verify instance mappings. + inst_mappings = objects.InstanceMappingList.get_by_cell_id( + self.ctxt, self.cell_mappings['cell0'].id) + # bare_br is the only instance that has a mapping from setUp. + self.assertEqual(1, len(inst_mappings)) + # Since we did not setup instance mappings for the other fake build + # requests used in this test, we should see a message logged about + # there being no instance mappings. + self.assertIn('While burying instance in cell0, no instance mapping ' + 'was found', self.stdlog.logger.output) + self.assertEqual(0, len(build_requests)) self.assertEqual(4, len(instances)) inst_states = {inst.uuid: (inst.deleted, inst.vm_state) @@ -2475,6 +2507,32 @@ def test_bury_in_cell0_with_tags(self, mock_create_tags, mock_get_cell, test.MatchType(context.RequestContext), inst.uuid, self.params['tags']) + @mock.patch('nova.objects.Instance.create') + def test_bury_in_cell0_already_mapped(self, mock_inst_create): + """Tests a scenario where the instance mapping is already mapped to a + cell when we attempt to bury the instance in cell0. + """ + build_request = self.params['build_requests'][0] + # Simulate MQ split brain craziness by updating the instance mapping + # to point at cell1. + inst_mapping = objects.InstanceMapping.get_by_instance_uuid( + self.ctxt, build_request.instance_uuid) + inst_mapping.cell_mapping = self.cell_mappings['cell1'] + inst_mapping.save() + # Now attempt to bury the instance in cell0. + with mock.patch.object(inst_mapping, 'save') as mock_inst_map_save: + self.conductor._bury_in_cell0( + self.ctxt, self.params['request_specs'][0], + exc.NoValidHost(reason='idk'), build_requests=[build_request]) + # We should have exited without creating the instance in cell0 nor + # should the instance mapping have been updated to point at cell0. + mock_inst_create.assert_not_called() + mock_inst_map_save.assert_not_called() + # And we should have logged an error. + self.assertIn('When attempting to bury instance in cell0, the ' + 'instance is already mapped to cell', + self.stdlog.logger.output) + def test_reset(self): with mock.patch('nova.compute.rpcapi.ComputeAPI') as mock_rpc: old_rpcapi = self.conductor_manager.compute_rpcapi diff --git a/nova/tests/unit/console/test_websocketproxy.py b/nova/tests/unit/console/test_websocketproxy.py index 98e162d59cb..27677bbf78e 100644 --- a/nova/tests/unit/console/test_websocketproxy.py +++ b/nova/tests/unit/console/test_websocketproxy.py @@ -15,6 +15,7 @@ """Tests for nova websocketproxy.""" import copy +import io import socket import mock @@ -626,6 +627,70 @@ def test_tcp_rst_no_compute_rpcapi(self): self.wh.server.top_new_client(conn, address) self.assertIsNone(self.wh._compute_rpcapi) + def test_reject_open_redirect(self): + # This will test the behavior when an attempt is made to cause an open + # redirect. It should be rejected. + mock_req = mock.MagicMock() + mock_req.makefile().readline.side_effect = [ + b'GET //example.com/%2F.. HTTP/1.1\r\n', + b'' + ] + + client_addr = ('8.8.8.8', 54321) + mock_server = mock.MagicMock() + # This specifies that the server will be able to handle requests other + # than only websockets. + mock_server.only_upgrade = False + + # Constructing a handler will process the mock_req request passed in. + handler = websocketproxy.NovaProxyRequestHandler( + mock_req, client_addr, mock_server) + + # Collect the response data to verify at the end. The + # SimpleHTTPRequestHandler writes the response data to a 'wfile' + # attribute. + output = io.BytesIO() + handler.wfile = output + # Process the mock_req again to do the capture. + handler.do_GET() + output.seek(0) + result = output.readlines() + + # Verify no redirect happens and instead a 400 Bad Request is returned. + self.assertIn('400 URI must not start with //', result[0].decode()) + + def test_reject_open_redirect_3_slashes(self): + # This will test the behavior when an attempt is made to cause an open + # redirect. It should be rejected. + mock_req = mock.MagicMock() + mock_req.makefile().readline.side_effect = [ + b'GET ///example.com/%2F.. HTTP/1.1\r\n', + b'' + ] + + client_addr = ('8.8.8.8', 54321) + mock_server = mock.MagicMock() + # This specifies that the server will be able to handle requests other + # than only websockets. + mock_server.only_upgrade = False + + # Constructing a handler will process the mock_req request passed in. + handler = websocketproxy.NovaProxyRequestHandler( + mock_req, client_addr, mock_server) + + # Collect the response data to verify at the end. The + # SimpleHTTPRequestHandler writes the response data to a 'wfile' + # attribute. + output = io.BytesIO() + handler.wfile = output + # Process the mock_req again to do the capture. + handler.do_GET() + output.seek(0) + result = output.readlines() + + # Verify no redirect happens and instead a 400 Bad Request is returned. + self.assertIn('400 URI must not start with //', result[0].decode()) + class NovaWebsocketSecurityProxyTestCase(test.NoDBTestCase): diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 7f22102cc48..41b455f74ca 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -2086,6 +2086,14 @@ def test_instance_get_all_with_meta(self): sys_meta = utils.metadata_to_dict(inst['system_metadata']) self.assertEqual(sys_meta, self.sample_data['system_metadata']) + def test_instance_get_with_meta(self): + inst_id = self.create_instance_with_args().id + inst = db.instance_get(self.ctxt, inst_id) + meta = utils.metadata_to_dict(inst['metadata']) + self.assertEqual(meta, self.sample_data['metadata']) + sys_meta = utils.metadata_to_dict(inst['system_metadata']) + self.assertEqual(sys_meta, self.sample_data['system_metadata']) + def test_instance_update(self): instance = self.create_instance_with_args() metadata = {'host': 'bar', 'key2': 'wuff'} diff --git a/nova/tests/unit/image/test_glance.py b/nova/tests/unit/image/test_glance.py index c5e714c1b0a..59f5710d492 100644 --- a/nova/tests/unit/image/test_glance.py +++ b/nova/tests/unit/image/test_glance.py @@ -305,6 +305,11 @@ def test_client_httpnotfound_converts_to_imagenotfound(self): out_exc = glance._translate_image_exception('123', in_exc) self.assertIsInstance(out_exc, exception.ImageNotFound) + def test_client_httpoverlimit_converts_to_imagequotaexceeded(self): + in_exc = glanceclient.exc.HTTPOverLimit('123') + out_exc = glance._translate_image_exception('123', in_exc) + self.assertIsInstance(out_exc, exception.ImageQuotaExceeded) + class TestGlanceSerializer(test.NoDBTestCase): def test_serialize(self): diff --git a/nova/tests/unit/objects/test_instance_mapping.py b/nova/tests/unit/objects/test_instance_mapping.py index ec50517a20c..2c877c0a1f5 100644 --- a/nova/tests/unit/objects/test_instance_mapping.py +++ b/nova/tests/unit/objects/test_instance_mapping.py @@ -12,6 +12,7 @@ import mock from oslo_utils import uuidutils +from sqlalchemy.orm import exc as orm_exc from nova import exception from nova import objects @@ -151,6 +152,14 @@ def test_save(self, save_in_db): comparators={ 'cell_mapping': self._check_cell_map_value}) + @mock.patch.object(instance_mapping.InstanceMapping, '_save_in_db') + def test_save_stale_data_error(self, save_in_db): + save_in_db.side_effect = orm_exc.StaleDataError + mapping_obj = objects.InstanceMapping(self.context) + mapping_obj.instance_uuid = uuidutils.generate_uuid() + + self.assertRaises(exception.InstanceMappingNotFound, mapping_obj.save) + @mock.patch.object(instance_mapping.InstanceMapping, '_destroy_in_db') def test_destroy(self, destroy_in_db): uuid = uuidutils.generate_uuid() diff --git a/nova/tests/unit/pci/test_stats.py b/nova/tests/unit/pci/test_stats.py index be867783fb9..0375ccc63f6 100644 --- a/nova/tests/unit/pci/test_stats.py +++ b/nova/tests/unit/pci/test_stats.py @@ -533,6 +533,30 @@ def test_remove_device(self): self.pci_stats.remove_device(dev2) self._assertPools() + def test_update_device(self): + # Update device type of one of the device from type-PCI to + # type-PF. Verify if the existing pool is updated and a new + # pool is created with dev_type type-PF. + self._create_pci_devices() + dev1 = self.pci_tagged_devices.pop() + dev1.dev_type = 'type-PF' + self.pci_stats.update_device(dev1) + self.assertEqual(3, len(self.pci_stats.pools)) + self._assertPoolContent(self.pci_stats.pools[0], '1137', '0072', + len(self.pci_untagged_devices)) + self.assertEqual(self.pci_untagged_devices, + self.pci_stats.pools[0]['devices']) + self._assertPoolContent(self.pci_stats.pools[1], '1137', '0071', + len(self.pci_tagged_devices), + physical_network='physnet1') + self.assertEqual(self.pci_tagged_devices, + self.pci_stats.pools[1]['devices']) + self._assertPoolContent(self.pci_stats.pools[2], '1137', '0071', + 1, + physical_network='physnet1') + self.assertEqual(dev1, + self.pci_stats.pools[2]['devices'][0]) + class PciDeviceVFPFStatsTestCase(test.NoDBTestCase): diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index e942b3a0739..186c8383a0c 100644 --- a/nova/tests/unit/test_hacking.py +++ b/nova/tests/unit/test_hacking.py @@ -1007,7 +1007,7 @@ def test_useless_assertion(self): expected_errors=errors, filename="nova/tests/unit/test_context.py") # Check no errors in other than 'nova/tests' directory. self._assert_has_no_errors( - code, checks.nonexistent_assertion_methods_and_attributes, + code, checks.useless_assertion, filename="nova/compute/api.py") code = """ self.assertIsNone(None_test_var, "Fails") diff --git a/nova/tests/unit/test_metadata.py b/nova/tests/unit/test_metadata.py index 7a6ecf3af24..c14544ba152 100644 --- a/nova/tests/unit/test_metadata.py +++ b/nova/tests/unit/test_metadata.py @@ -329,12 +329,14 @@ def test_format_instance_mapping(self): 'uuid': 'e5fe5518-0288-4fa3-b0c4-c79764101b85', 'root_device_name': None, 'default_ephemeral_device': None, - 'default_swap_device': None}) + 'default_swap_device': None, + 'context': self.context}) instance_ref1 = objects.Instance(**{'id': 0, 'uuid': 'b65cee2f-8c69-4aeb-be2f-f79742548fc2', 'root_device_name': '/dev/sda1', 'default_ephemeral_device': None, - 'default_swap_device': None}) + 'default_swap_device': None, + 'context': self.context}) def fake_bdm_get(ctxt, uuid): return [fake_block_device.FakeDbBlockDeviceDict( @@ -373,10 +375,12 @@ def fake_bdm_get(ctxt, uuid): 'swap': '/dev/sdc', 'ebs0': '/dev/sdh'} - self.assertEqual(base._format_instance_mapping(self.context, - instance_ref0), block_device._DEFAULT_MAPPINGS) - self.assertEqual(base._format_instance_mapping(self.context, - instance_ref1), expected) + self.assertEqual( + base._format_instance_mapping(instance_ref0), + block_device._DEFAULT_MAPPINGS) + self.assertEqual( + base._format_instance_mapping(instance_ref1), + expected) def test_pubkey(self): md = fake_InstanceMetadata(self, self.instance.obj_clone()) diff --git a/nova/tests/unit/virt/libvirt/fake_imagebackend.py b/nova/tests/unit/virt/libvirt/fake_imagebackend.py index d73a396ab5e..4a940fa45e0 100644 --- a/nova/tests/unit/virt/libvirt/fake_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/fake_imagebackend.py @@ -184,11 +184,17 @@ def image_init(instance=None, disk_name=None, path=None): # class. image_init.SUPPORTS_CLONE = False - # Ditto for the 'is_shared_block_storage' function + # Ditto for the 'is_shared_block_storage' function and + # 'is_file_in_instance_path' def is_shared_block_storage(): return False + def is_file_in_instance_path(): + return False + setattr(image_init, 'is_shared_block_storage', is_shared_block_storage) + setattr(image_init, 'is_file_in_instance_path', + is_file_in_instance_path) return image_init diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py index d10a44c7386..676a3542b1f 100644 --- a/nova/tests/unit/virt/libvirt/fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py @@ -251,60 +251,71 @@ def __init__(self, dev_type, slot, function, iommu_group, numa_node, applicable if ``dev_type`` is one of: ``PF``, ``VF``. """ - if dev_type == 'PCI': - if vf_ratio: + self.dev_type = dev_type + self.slot = slot + self.function = function + self.iommu_group = iommu_group + self.numa_node = numa_node + self.vf_ratio = vf_ratio + self.generate_xml() + + def generate_xml(self, skip_capability=False): + capability = '' + if self.dev_type == 'PCI': + if self.vf_ratio: raise ValueError('vf_ratio does not apply for PCI devices') prod_id = PCI_PROD_ID prod_name = PCI_PROD_NAME driver = PCI_DRIVER_NAME - capability = '' - elif dev_type == 'PF': + elif self.dev_type == 'PF': prod_id = PF_PROD_ID prod_name = PF_PROD_NAME driver = PF_DRIVER_NAME - capability = self.cap_templ % { - 'cap_type': PF_CAP_TYPE, - 'addresses': '\n'.join([ - self.addr_templ % { - # these are the slot, function values of the child VFs - # we can only assign 8 functions to a slot (0-7) so - # bump the slot each time we exceed this - 'slot': slot + (x // 8), - # ...and wrap the function value - 'function': x % 8, - # the offset is because the PF is occupying function 0 - } for x in range(1, vf_ratio + 1)]) - } - elif dev_type == 'VF': + if not skip_capability: + capability = self.cap_templ % { + 'cap_type': PF_CAP_TYPE, + 'addresses': '\n'.join([ + self.addr_templ % { + # these are the slot, function values of the child + # VFs, we can only assign 8 functions to a slot + # (0-7) so bump the slot each time we exceed this + 'slot': self.slot + (x // 8), + # ...and wrap the function value + 'function': x % 8, + # the offset is because the PF is occupying function 0 + } for x in range(1, self.vf_ratio + 1)]) + } + elif self.dev_type == 'VF': prod_id = VF_PROD_ID prod_name = VF_PROD_NAME driver = VF_DRIVER_NAME - capability = self.cap_templ % { - 'cap_type': VF_CAP_TYPE, - 'addresses': self.addr_templ % { - # this is the slot, function value of the parent PF - # if we're e.g. device 8, we'll have a different slot - # to our parent so reverse this - 'slot': slot - ((vf_ratio + 1) // 8), - # the parent PF is always function 0 - 'function': 0, + if not skip_capability: + capability = self.cap_templ % { + 'cap_type': VF_CAP_TYPE, + 'addresses': self.addr_templ % { + # this is the slot, function value of the parent PF + # if we're e.g. device 8, we'll have a different slot + # to our parent so reverse this + 'slot': self.slot - ((self.vf_ratio + 1) // 8), + # the parent PF is always function 0 + 'function': 0, + } } - } else: raise ValueError('Expected one of: PCI, VF, PCI') self.pci_device = self.pci_device_template % { - 'slot': slot, - 'function': function, + 'slot': self.slot, + 'function': self.function, 'vend_id': PCI_VEND_ID, 'vend_name': PCI_VEND_NAME, 'prod_id': prod_id, 'prod_name': prod_name, 'driver': driver, 'capability': capability, - 'iommu_group': iommu_group, - 'numa_node': numa_node, + 'iommu_group': self.iommu_group, + 'numa_node': self.numa_node, } def XMLDesc(self, flags): @@ -759,6 +770,7 @@ def __init__(self, connection, xml, running=False, transient=False): self._has_saved_state = False self._snapshots = {} self._id = self._connection._id_counter + self._job_type = VIR_DOMAIN_JOB_UNBOUNDED def _parse_definition(self, xml): try: @@ -847,6 +859,20 @@ def _parse_definition(self, xml): nic_info['source'] = source.get('network') elif nic_info['type'] == 'bridge': nic_info['source'] = source.get('bridge') + elif nic_info['type'] == 'hostdev': + # is for VF when vnic_type + # is direct. Add sriov vf pci information in nic_info + address = source.find('./address') + pci_type = address.get('type') + pci_domain = address.get('domain').replace('0x', '') + pci_bus = address.get('bus').replace('0x', '') + pci_slot = address.get('slot').replace('0x', '') + pci_function = address.get('function').replace( + '0x', '') + pci_device = "%s_%s_%s_%s_%s" % (pci_type, pci_domain, + pci_bus, pci_slot, + pci_function) + nic_info['source'] = pci_device nics_info += [nic_info] @@ -888,11 +914,32 @@ def _parse_definition(self, xml): return definition + def verify_hostdevs_interface_are_vfs(self): + """Verify for interface type hostdev if the pci device is VF or not. + """ + + error_message = ("Interface type hostdev is currently supported on " + "SR-IOV Virtual Functions only") + + nics = self._def['devices'].get('nics', []) + for nic in nics: + if nic['type'] == 'hostdev': + pci_device = nic['source'] + pci_info_from_connection = self._connection.pci_info.devices[ + pci_device] + if 'phys_function' not in pci_info_from_connection.pci_device: + raise make_libvirtError( + libvirtError, + error_message, + error_code=VIR_ERR_CONFIG_UNSUPPORTED, + error_domain=VIR_FROM_DOMAIN) + def create(self): self.createWithFlags(0) def createWithFlags(self, flags): # FIXME: Not handling flags at the moment + self.verify_hostdevs_interface_are_vfs() self._state = VIR_DOMAIN_RUNNING self._connection._mark_running(self) self._has_saved_state = False @@ -1016,7 +1063,7 @@ def XMLDesc(self, flags): nics = '' for nic in self._def['devices']['nics']: - if 'source' in nic: + if 'source' in nic and nic['type'] != 'hostdev': nics += ''' @@ -1191,7 +1238,17 @@ def jobInfo(self): return [0] * 12 def jobStats(self, flags=0): - return {} + # NOTE(artom) By returning VIR_DOMAIN_JOB_UNBOUNDED, we're pretending a + # job is constantly running. Tests are expected to call the + # complete_job or fail_job methods when they're ready for jobs (read: + # live migrations) to "complete". + return {'type': self._job_type} + + def complete_job(self): + self._job_type = VIR_DOMAIN_JOB_COMPLETED + + def fail_job(self): + self._job_type = VIR_DOMAIN_JOB_FAILED def injectNMI(self, flags=0): return 0 @@ -1656,6 +1713,16 @@ def make_libvirtError(error_class, msg, error_code=None, virNWFilter = NWFilter +# A private libvirt-python class and global only provided here for testing to +# ensure it's not returned by libvirt.host.Host.get_libvirt_proxy_classes. +class FakeHandler(object): + def __init__(self): + pass + + +_EventAddHandleFunc = FakeHandler + + class FakeLibvirtFixture(fixtures.Fixture): """Performs global setup/stubbing for all libvirt tests. """ diff --git a/nova/tests/unit/virt/libvirt/test_config.py b/nova/tests/unit/virt/libvirt/test_config.py index bc30776561d..968582dc15b 100644 --- a/nova/tests/unit/virt/libvirt/test_config.py +++ b/nova/tests/unit/virt/libvirt/test_config.py @@ -338,6 +338,26 @@ def test_config_simple(self): """) + def test_config_parse_require(self): + xml = """ + + """ + xmldoc = etree.fromstring(xml) + obj = config.LibvirtConfigCPUFeature() + obj.parse_dom(xmldoc) + + self.assertEqual(obj.policy, "require") + + def test_config_parse_disable(self): + xml = """ + + """ + xmldoc = etree.fromstring(xml) + obj = config.LibvirtConfigCPUFeature() + obj.parse_dom(xmldoc) + + self.assertEqual(obj.policy, "disable") + class LibvirtConfigGuestCPUFeatureTest(LibvirtConfigBaseTest): @@ -436,6 +456,27 @@ def test_config_complex(self): """) + def test_config_disabled_features(self): + obj = config.LibvirtConfigCPU() + obj.model = "Penryn" + obj.vendor = "Intel" + obj.arch = obj_fields.Architecture.X86_64 + + disabled_feature = config.LibvirtConfigCPUFeature("mtrr") + disabled_feature.policy = "disable" + obj.add_feature(disabled_feature) + obj.add_feature(config.LibvirtConfigCPUFeature("apic")) + + xml = obj.to_xml() + self.assertXmlEqual(xml, """ + + x86_64 + Penryn + Intel + + + """) + def test_only_uniq_cpu_featues(self): obj = config.LibvirtConfigCPU() obj.model = "Penryn" diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 0b1f8961c55..93b41583042 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -4445,6 +4445,19 @@ def test_get_guest_config_windows_hyperv_feature2(self): self._test_get_guest_config_windows_hyperv() def test_get_guest_config_windows_hyperv_all_hide_flv(self): + # Similar to test_get_guest_config_windows_hyperv_feature2 + # but also test hiding the HyperV signature with the flavor + # extra_spec "hw:hide_hypervisor_id" + flavor_hide_id = fake_flavor.fake_flavor_obj(self.context, + extra_specs={"hw:hide_hypervisor_id": "true"}, + expected_attrs={"extra_specs"}) + # this works for kvm (the default, tested below) and qemu + self.flags(virt_type='qemu', group='libvirt') + + self._test_get_guest_config_windows_hyperv( + flavor=flavor_hide_id, hvid_hidden=True) + + def test_get_guest_config_windows_hyperv_all_hide_flv_old(self): # Similar to test_get_guest_config_windows_hyperv_feature2 # but also test hiding the HyperV signature with the flavor # extra_spec "hide_hypervisor_id" @@ -4471,10 +4484,10 @@ def test_get_guest_config_windows_hyperv_all_hide_img(self): def test_get_guest_config_windows_hyperv_all_hide_flv_img(self): # Similar to test_get_guest_config_windows_hyperv_feature2 # but also test hiding the HyperV signature with both the flavor - # extra_spec "hide_hypervisor_id" and the image property + # extra_spec "hw:hide_hypervisor_id" and the image property # "img_hide_hypervisor_id" flavor_hide_id = fake_flavor.fake_flavor_obj(self.context, - extra_specs={"hide_hypervisor_id": "true"}, + extra_specs={"hw:hide_hypervisor_id": "true"}, expected_attrs={"extra_specs"}) self.flags(virt_type='qemu', group='libvirt') @@ -6087,7 +6100,7 @@ def test_get_guest_config_with_video_driver_vram(self): self.assertEqual(cfg.devices[4].type, "spice") self.assertEqual(cfg.devices[5].type, "qxl") - self.assertEqual(cfg.devices[5].vram, 64 * units.Mi / units.Ki) + self.assertEqual(cfg.devices[5].vram, 65536) def _test_add_video_driver(self, model): self.flags(virt_type='kvm', group='libvirt') @@ -6098,15 +6111,19 @@ def _test_add_video_driver(self, model): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) guest = vconfig.LibvirtConfigGuest() - instance_ref = objects.Instance(**self.test_instance) - flavor = instance_ref.get_flavor() + flavor = objects.Flavor( + extra_specs={'hw_video:ram_max_mb': '512'}) image_meta = objects.ImageMeta.from_dict({ - 'properties': {'hw_video_model': model}}) + 'properties': { + 'hw_video_model': model, + 'hw_video_ram': 8, + }, + }) self.assertTrue(drvr._guest_add_video_device(guest)) - video = drvr._add_video_driver(guest, image_meta, - flavor) + video = drvr._add_video_driver(guest, image_meta, flavor) self.assertEqual(model, video.type) + self.assertEqual(8192, video.vram) # should be in bytes def test__add_video_driver(self): self._test_add_video_driver('qxl') @@ -6518,7 +6535,7 @@ def test_get_guest_config_with_hiding_hypervisor_id_flavor_extra_specs( self): # Input to the test: flavor extra_specs flavor_hide_id = fake_flavor.fake_flavor_obj(self.context, - extra_specs={"hide_hypervisor_id": "true"}, + extra_specs={"hw:hide_hypervisor_id": "true"}, expected_attrs={"extra_specs"}) self.flags(virt_type='kvm', group='libvirt') @@ -6544,7 +6561,7 @@ def test_get_guest_config_with_hiding_hypervisor_id_img_and_flavor( # Input to the test: image metadata (true) and flavor # extra_specs (true) flavor_hide_id = fake_flavor.fake_flavor_obj(self.context, - extra_specs={"hide_hypervisor_id": "true"}, + extra_specs={"hw:hide_hypervisor_id": "true"}, expected_attrs={"extra_specs"}) image_meta = objects.ImageMeta.from_dict({ "disk_format": "raw", @@ -6571,7 +6588,7 @@ def test_get_guest_config_with_hiding_hypervisor_id_img_or_flavor( # Input to the test: image metadata (false) and flavor # extra_specs (true) flavor_hide_id = fake_flavor.fake_flavor_obj(self.context, - extra_specs={"hide_hypervisor_id": "true"}, + extra_specs={"hw:hide_hypervisor_id": "true"}, expected_attrs={"extra_specs"}) image_meta = objects.ImageMeta.from_dict({ "disk_format": "raw", @@ -6596,7 +6613,7 @@ def test_get_guest_config_with_hiding_hypervisor_id_img_or_flavor( # Input to the test: image metadata (true) and flavor # extra_specs (false) flavor_hide_id = fake_flavor.fake_flavor_obj(self.context, - extra_specs={"hide_hypervisor_id": "false"}, + extra_specs={"hw:hide_hypervisor_id": "false"}, expected_attrs={"extra_specs"}) image_meta = objects.ImageMeta.from_dict({ "disk_format": "raw", @@ -6643,7 +6660,7 @@ def test_get_guest_config_without_hiding_hypervisor_id(self): def test_get_guest_config_without_hiding_hypervisor_id_flavor_extra_specs( self): flavor_hide_id = fake_flavor.fake_flavor_obj(self.context, - extra_specs={"hide_hypervisor_id": "false"}, + extra_specs={"hw:hide_hypervisor_id": "false"}, expected_attrs={"extra_specs"}) self.flags(virt_type='qemu', group='libvirt') @@ -17727,6 +17744,51 @@ def test_set_cache_mode_shareable(self): drvr._set_cache_mode(fake_conf) self.assertEqual('none', fake_conf.driver_cache) + def _make_fake_conf(self, cache=None): + if cache: + self.flags(disk_cachemodes=['block=' + cache], group='libvirt') + else: + self.flags(group='libvirt') + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + fake_conf = FakeConfigGuestDisk() + fake_conf.source_type = 'block' + fake_conf.driver_io = 'native' + drvr._set_cache_mode(fake_conf) + return fake_conf + + def test_set_cache_mode_driver_io_writeback(self): + """Tests that when conf.driver_io is 'native' and driver_cache is + 'writeback', then conf.driver_io is forced to 'threads' + """ + fake_conf = self._make_fake_conf('writeback') + self.assertEqual('writeback', fake_conf.driver_cache) + self.assertEqual('threads', fake_conf.driver_io) + + def test_set_cache_mode_driver_io_writethrough(self): + """Tests that when conf.driver_io is 'native' and driver_cache is + 'writethrough', then conf.driver_io is forced to 'threads' + """ + fake_conf = self._make_fake_conf('writethrough') + self.assertEqual('writethrough', fake_conf.driver_cache) + self.assertEqual('threads', fake_conf.driver_io) + + def test_set_cache_mode_driver_io_unsafe(self): + """Tests that when conf.driver_io is 'native' and driver_cache is + 'unsafe', then conf.driver_io is forced to 'threads' + """ + fake_conf = self._make_fake_conf('unsafe') + self.assertEqual('unsafe', fake_conf.driver_cache) + self.assertEqual('threads', fake_conf.driver_io) + + def test_without_set_cache_mode_driver_io(self): + """Tests that when conf.driver_io is 'native' and driver_cache is + not set(this is default settings), then conf.driver_io is kept as + 'native' + """ + fake_conf = self._make_fake_conf() + self.assertIsNone(fake_conf.driver_cache) + self.assertEqual('native', fake_conf.driver_io) + def test_set_cache_mode_invalid_mode(self): self.flags(disk_cachemodes=['file=FAKE'], group='libvirt') drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) @@ -19142,10 +19204,11 @@ def test_swap_volume_disconnect_new_volume_on_pivot_error(self, disconnect_volume.assert_called_once_with(self.context, mock.sentinel.new_connection_info, instance) + @mock.patch.object(fileutils, 'delete_if_exists') @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete') @mock.patch('nova.privsep.path.chown') def _test_live_snapshot( - self, mock_chown, mock_is_job_complete, + self, mock_chown, mock_is_job_complete, mock_delete, can_quiesce=False, require_quiesce=False): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) mock_dom = mock.MagicMock() @@ -19203,6 +19266,7 @@ def _test_live_snapshot( mock_chown.assert_called_once_with(dltfile, uid=os.getuid()) mock_snapshot.assert_called_once_with(dltfile, "qcow2", dstfile, "qcow2") + mock_delete.assert_called_once_with(dltfile) mock_define.assert_called_once_with(xmldoc) mock_quiesce.assert_any_call(mock.ANY, self.test_instance, mock.ANY, True) @@ -21734,6 +21798,74 @@ def test_unshelve_rbd_image_flatten_during_fetch_image_cache(self): mock_rbd_driver.flatten.assert_called_once_with( mock.sentinel.rbd_name, pool=mock.sentinel.rbd_pool) + @mock.patch.object(libvirt_driver.LibvirtDriver, '_try_fetch_image_cache') + @mock.patch.object(libvirt_driver.LibvirtDriver, '_rebase_with_qemu_img') + def _test_unshelve_qcow2_rebase_image_during_create(self, + mock_rebase, mock_fetch, original_image_in_glance=True): + self.flags(images_type='qcow2', group='libvirt') + + # Original image ref from where instance was created, before SHELVE + # occurs, base_root_fname is related backing file name. + base_image_ref = 'base_image_ref' + base_root_fname = imagecache.get_cache_fname(base_image_ref) + # Snapshot image ref created during SHELVE. + shelved_image_ref = 'shelved_image_ref' + shelved_root_fname = imagecache.get_cache_fname(shelved_image_ref) + + # Instance state during unshelve spawn(). + inst_params = { + 'image_ref': shelved_image_ref, + 'vm_state': vm_states.SHELVED_OFFLOADED, + 'system_metadata': {'image_base_image_ref': base_image_ref} + } + + instance = self._create_instance(params=inst_params) + disk_images = {'image_id': instance.image_ref} + instance_dir = libvirt_utils.get_instance_path(instance) + disk_path = os.path.join(instance_dir, 'disk') + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + + if original_image_in_glance: + # We expect final backing file is original image, not shelved one. + expected_backing_file = os.path.join( + imagecache.ImageCacheManager().cache_dir, + base_root_fname) + else: + # None means rebase will merge backing file into disk(flatten). + expected_backing_file = None + mock_fetch.side_effect = [ + None, + exception.ImageNotFound(image_id=base_image_ref) + ] + + drvr._create_and_inject_local_root( + self.context, instance, False, '', disk_images, None, None) + + mock_fetch.assert_has_calls([ + mock.call(test.MatchType(nova.virt.libvirt.imagebackend.Qcow2), + libvirt_utils.fetch_image, + self.context, shelved_root_fname, shelved_image_ref, + instance, instance.root_gb * units.Gi, None), + mock.call(test.MatchType(nova.virt.libvirt.imagebackend.Qcow2), + libvirt_utils.fetch_image, + self.context, base_root_fname, base_image_ref, + instance, None)]) + mock_rebase.assert_called_once_with(disk_path, expected_backing_file) + + def test_unshelve_qcow2_rebase_image_during_create(self): + # Original image is present in Glance. In that case the 2nd + # fetch succeeds and we rebase instance disk to original image backing + # file, instance is back to nominal state: after unshelve, + # instance.image_ref will match current backing file. + self._test_unshelve_qcow2_rebase_image_during_create() + + def test_unshelve_qcow2_rebase_image_during_create_notfound(self): + # Original image is no longer available in Glance, so 2nd fetch + # will failed (HTTP 404). In that case qemu-img rebase will merge + # backing file into disk, removing backing file dependency. + self._test_unshelve_qcow2_rebase_image_during_create( + original_image_in_glance=False) + @mock.patch('nova.virt.libvirt.driver.imagebackend') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._inject_data') @mock.patch('nova.virt.libvirt.driver.imagecache') @@ -21752,6 +21884,52 @@ def test_data_not_injects_with_configdrive(self, mock_image, mock_inject, None) self.assertFalse(mock_inject.called) + @mock.patch('nova.virt.libvirt.utils.fetch_image') + @mock.patch('nova.virt.libvirt.storage.rbd_utils.RBDDriver') + @mock.patch.object(imagebackend, 'IMAGE_API') + def test_create_fetch_image_ceph_workaround(self, mock_image, mock_rbd, + mock_fetch): + # Make sure that rbd clone will fail as un-clone-able + mock_rbd.is_cloneable.return_value = False + # Make sure the rbd code thinks the image does not already exist + mock_rbd.return_value.exists.return_value = False + # Make sure the rbd code says the image is small + mock_rbd.return_value.size.return_value = 128 * units.Mi + # Make sure IMAGE_API.get() returns a raw image + mock_image.get.return_value = {'locations': [], 'disk_format': 'raw'} + + instance = self._create_instance() + disk_images = {'image_id': 'foo'} + self.flags(images_type='rbd', group='libvirt') + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + + def do_create(): + # Reset the fetch mock and run our driver method so we can + # check for called-ness after each attempt + mock_fetch.reset_mock() + drvr._create_and_inject_local_root(self.context, + instance, + False, + '', + disk_images, + get_injection_info(), + None) + + # Do an image create with rbd + do_create() + # Make sure it tried fetch, which implies that it tried and + # failed to clone. + mock_fetch.assert_called() + + # Enable the workaround + self.flags(never_download_image_if_on_rbd=True, + group='workarounds') + # Ensure that we raise the original ImageUnacceptable from the + # failed clone... + self.assertRaises(exception.ImageUnacceptable, do_create) + # ...and ensure that we did _not_ try to fetch + mock_fetch.assert_not_called() + @mock.patch('nova.virt.netutils.get_injected_network_template') @mock.patch('nova.virt.disk.api.inject_data') @mock.patch.object(libvirt_driver.LibvirtDriver, "_conn") @@ -23794,6 +23972,25 @@ def test_cpu_traits_with_mode_none_on_power(self, mock_baseline, mock_cap): '''], 1) + @mock.patch('nova.virt.images.qemu_img_info', + return_value=mock.Mock(file_format="fake_fmt")) + @mock.patch('oslo_concurrency.processutils.execute') + def test_rebase_with_qemu_img(self, mock_execute, mock_qemu_img_info): + """rebasing disk image to another backing file""" + self.drvr._rebase_with_qemu_img("disk", "backing_file") + mock_qemu_img_info.assert_called_once_with("backing_file") + mock_execute.assert_called_once_with('qemu-img', 'rebase', + '-b', 'backing_file', '-F', + 'fake_fmt', 'disk') + + # Flatten disk image when no backing file is given. + mock_qemu_img_info.reset_mock() + mock_execute.reset_mock() + self.drvr._rebase_with_qemu_img("disk", None) + self.assertEqual(0, mock_qemu_img_info.call_count) + mock_execute.assert_called_once_with('qemu-img', 'rebase', + '-b', '', 'disk') + class LibvirtVolumeUsageTestCase(test.NoDBTestCase): """Test for LibvirtDriver.get_all_volume_usage.""" @@ -24413,8 +24610,23 @@ def test_volume_snapshot_delete_when_dom_not_running(self, mock_execute, not running should trigger a blockRebase using qemu-img not libvirt. In this test, we rebase the image with another image as backing file. """ + dom_xml = """ + + + + + + 0e38683e-f0af-418f-a3f1-6b67ea0f919d + + + + + + + """ % self.inst['uuid'] + mock_domain, guest = self._setup_block_rebase_domain_and_guest_mocks( - self.dom_xml) + dom_xml) instance = objects.Instance(**self.inst) snapshot_id = 'snapshot-1234' @@ -24425,10 +24637,13 @@ def test_volume_snapshot_delete_when_dom_not_running(self, mock_execute, self.delete_info_1) mock_disk_op_sema.__enter__.assert_called_once() - mock_qemu_img_info.assert_called_once_with("snap.img") - mock_execute.assert_called_once_with('qemu-img', 'rebase', - '-b', 'snap.img', '-F', - 'fake_fmt', 'disk1_file') + mock_qemu_img_info.assert_called_once_with( + "/var/lib/nova/instances/%s/snap.img" % instance.uuid) + mock_execute.assert_called_once_with( + 'qemu-img', 'rebase', + '-b', '/var/lib/nova/instances/%s/snap.img' % instance.uuid, + '-F', 'fake_fmt', + '/var/lib/nova/instances/%s/disk1_file' % instance.uuid) @mock.patch.object(compute_utils, 'disk_ops_semaphore') @mock.patch.object(host.Host, "has_min_version", @@ -25424,6 +25639,15 @@ def test_vpmems_duplicated_config(self, mock_get): self.assertRaises(exception.PMEMNamespaceConfigInvalid, drvr._discover_vpmems, vpmem_conf) + @mock.patch('nova.privsep.libvirt.get_pmem_namespaces') + def test_get_vpmems_on_host__exception(self, mock_get_ns): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + mock_get_ns.side_effect = Exception('foo') + + self.assertRaises( + exception.GetPMEMNamespacesFailed, + drvr._get_vpmems_on_host) + @mock.patch('nova.virt.hardware.get_vpmems') def test_get_ordered_vpmems(self, mock_labels): # get orgered vpmems based on flavor extra_specs diff --git a/nova/tests/unit/virt/libvirt/test_host.py b/nova/tests/unit/virt/libvirt/test_host.py index c9411d6eb75..4c4b1d45c8b 100644 --- a/nova/tests/unit/virt/libvirt/test_host.py +++ b/nova/tests/unit/virt/libvirt/test_host.py @@ -1322,8 +1322,9 @@ def test_get_libvirt_proxy_classes(self): self.assertIn(fakelibvirt.virSecret, proxy_classes) self.assertIn(fakelibvirt.virNWFilter, proxy_classes) - # Assert that we filtered out libvirtError + # Assert that we filtered out libvirtError and any private classes self.assertNotIn(fakelibvirt.libvirtError, proxy_classes) + self.assertNotIn(fakelibvirt._EventAddHandleFunc, proxy_classes) def test_tpool_get_connection(self): # Test that Host.get_connection() returns a tpool.Proxy diff --git a/nova/tests/unit/virt/libvirt/test_migration.py b/nova/tests/unit/virt/libvirt/test_migration.py index 7adfb0ef65b..07d11f94e55 100644 --- a/nova/tests/unit/virt/libvirt/test_migration.py +++ b/nova/tests/unit/virt/libvirt/test_migration.py @@ -955,7 +955,48 @@ def test_update_vif_xml_no_matching_vif(self): doc = etree.fromstring(original_xml) ex = self.assertRaises(KeyError, migration._update_vif_xml, doc, data, get_vif_config) - self.assertIn("CA:FE:DE:AD:BE:EF", six.text_type(ex)) + self.assertIn("ca:fe:de:ad:be:ef", six.text_type(ex)) + + def test_update_vif_xml_lower_case_mac(self): + """Tests that the vif in the migrate data is not found in the existing + guest interfaces. + """ + conf = vconfig.LibvirtConfigGuestInterface() + conf.net_type = "bridge" + conf.source_dev = "qbra188171c-ea" + conf.target_dev = "tapa188171c-ea" + conf.mac_addr = "DE:AD:BE:EF:CA:FE" + conf.model = "virtio" + original_xml = """ + 3de6550a-8596-4937-8046-9d862036bca5 + + + + + + + + + +
+ + + """ % uuids.ovs + expected_xml = """ + 3de6550a-8596-4937-8046-9d862036bca5 + + + + + + +
+ + + """ + self._test_update_vif_xml(conf, original_xml, expected_xml) class MigrationMonitorTestCase(test.NoDBTestCase): diff --git a/nova/tests/unit/virt/libvirt/test_vif.py b/nova/tests/unit/virt/libvirt/test_vif.py index 648d6dc408a..379948d95e6 100644 --- a/nova/tests/unit/virt/libvirt/test_vif.py +++ b/nova/tests/unit/virt/libvirt/test_vif.py @@ -657,6 +657,21 @@ def test_virtio_multiqueue_in_kernel_3(self, mock_uname): def test_virtio_multiqueue_in_kernel_4(self, mock_uname): self._test_virtio_multiqueue(10, '10') + @mock.patch('os.uname', return_value=('Linux', '', '2.6.32-21-generic')) + def test_virtio_multiqueue_in_kernel_2_max_queues(self, mock_uname): + self.flags(max_queues=2, group='libvirt') + self._test_virtio_multiqueue(10, '2') + + @mock.patch('os.uname', return_value=('Linux', '', '3.19.0-47-generic')) + def test_virtio_multiqueue_in_kernel_3_max_queues(self, mock_uname): + self.flags(max_queues=2, group='libvirt') + self._test_virtio_multiqueue(10, '2') + + @mock.patch('os.uname', return_value=('Linux', '', '4.2.0-35-generic')) + def test_virtio_multiqueue_in_kernel_4_max_queues(self, mock_uname): + self.flags(max_queues=2, group='libvirt') + self._test_virtio_multiqueue(10, '2') + def test_vhostuser_os_vif_multiqueue(self): d = vif.LibvirtGenericVIFDriver() hostimpl = host.Host("qemu:///system") @@ -1123,10 +1138,71 @@ def test_tap_ethernet_vif_driver(self): @mock.patch('nova.privsep.linux_net.device_exists', return_value=True) @mock.patch('nova.privsep.linux_net.set_device_mtu') @mock.patch('nova.privsep.linux_net.create_tap_dev') - def test_plug_tap(self, mock_create_tap_dev, mock_set_mtu, + def test_plug_tap_kvm_virtio(self, mock_create_tap_dev, mock_set_mtu, mock_device_exists): - d = vif.LibvirtGenericVIFDriver() - d.plug(self.instance, self.vif_tap) + + d1 = vif.LibvirtGenericVIFDriver() + ins = objects.Instance( + id=1, uuid='f0000000-0000-0000-0000-000000000001', + project_id=723, system_metadata={} + ) + d1.plug(ins, self.vif_tap) + mock_create_tap_dev.assert_called_once_with('tap-xxx-yyy-zzz', None, + multiqueue=False) + + mock_create_tap_dev.reset_mock() + + d2 = vif.LibvirtGenericVIFDriver() + mq_ins = objects.Instance( + id=1, uuid='f0000000-0000-0000-0000-000000000001', + project_id=723, system_metadata={ + 'image_hw_vif_multiqueue_enabled': 'True' + } + ) + d2.plug(mq_ins, self.vif_tap) + mock_create_tap_dev.assert_called_once_with('tap-xxx-yyy-zzz', None, + multiqueue=True) + + @mock.patch('nova.privsep.linux_net.device_exists', return_value=True) + @mock.patch('nova.privsep.linux_net.set_device_mtu') + @mock.patch('nova.privsep.linux_net.create_tap_dev') + def test_plug_tap_mq_ignored_virt_type( + self, mock_create_tap_dev, mock_set_mtu, mock_device_exists): + + self.flags(use_virtio_for_bridges=True, + virt_type='xen', + group='libvirt') + + d1 = vif.LibvirtGenericVIFDriver() + ins = objects.Instance( + id=1, uuid='f0000000-0000-0000-0000-000000000001', + project_id=723, system_metadata={ + 'image_hw_vif_multiqueue_enabled': 'True' + } + ) + d1.plug(ins, self.vif_tap) + mock_create_tap_dev.assert_called_once_with('tap-xxx-yyy-zzz', + None, + multiqueue=False) + + @mock.patch('nova.privsep.linux_net.device_exists', return_value=True) + @mock.patch('nova.privsep.linux_net.set_device_mtu') + @mock.patch('nova.privsep.linux_net.create_tap_dev') + def test_plug_tap_mq_ignored_vif_model( + self, mock_create_tap_dev, mock_set_mtu, mock_device_exists): + + d1 = vif.LibvirtGenericVIFDriver() + ins = objects.Instance( + id=1, uuid='f0000000-0000-0000-0000-000000000001', + project_id=723, system_metadata={ + 'image_hw_vif_multiqueue_enabled': 'True', + 'image_hw_vif_model': 'e1000', + } + ) + d1.plug(ins, self.vif_tap) + mock_create_tap_dev.assert_called_once_with('tap-xxx-yyy-zzz', + None, + multiqueue=False) def test_unplug_tap(self): d = vif.LibvirtGenericVIFDriver() diff --git a/nova/tests/unit/virt/test_hardware.py b/nova/tests/unit/virt/test_hardware.py index 9201177fe27..772f05b2188 100644 --- a/nova/tests/unit/virt/test_hardware.py +++ b/nova/tests/unit/virt/test_hardware.py @@ -3020,10 +3020,13 @@ def test_get_pinning_host_siblings_large_instance_odd_fit(self): got_topo = objects.VirtCPUTopology(sockets=1, cores=5, threads=1) self.assertEqualTopology(got_topo, inst_pin.cpu_topology) + # TODO(stephenfin): Remove when we drop support for vcpu_pin_set def test_get_pinning_isolate_policy_too_few_fully_free_cores(self): host_pin = objects.NUMACell( id=0, - cpuset=set(), + # Simulate a legacy host with vcpu_pin_set configuration, + # meaning cpuset == pcpuset + cpuset=set([0, 1, 2, 3]), pcpuset=set([0, 1, 2, 3]), memory=4096, memory_usage=0, @@ -3038,10 +3041,13 @@ def test_get_pinning_isolate_policy_too_few_fully_free_cores(self): inst_pin = hw._numa_fit_instance_cell_with_pinning(host_pin, inst_pin) self.assertIsNone(inst_pin) + # TODO(stephenfin): Remove when we drop support for vcpu_pin_set def test_get_pinning_isolate_policy_no_fully_free_cores(self): host_pin = objects.NUMACell( id=0, - cpuset=set(), + # Simulate a legacy host with vcpu_pin_set configuration, + # meaning cpuset == pcpuset + cpuset=set([0, 1, 2, 3]), pcpuset=set([0, 1, 2, 3]), memory=4096, memory_usage=0, @@ -3056,10 +3062,13 @@ def test_get_pinning_isolate_policy_no_fully_free_cores(self): inst_pin = hw._numa_fit_instance_cell_with_pinning(host_pin, inst_pin) self.assertIsNone(inst_pin) + # TODO(stephenfin): Remove when we drop support for vcpu_pin_set def test_get_pinning_isolate_policy_fits(self): host_pin = objects.NUMACell( id=0, - cpuset=set(), + # Simulate a legacy host with vcpu_pin_set configuration, + # meaning cpuset == pcpuset + cpuset=set([0, 1, 2, 3]), pcpuset=set([0, 1, 2, 3]), memory=4096, memory_usage=0, @@ -3076,10 +3085,13 @@ def test_get_pinning_isolate_policy_fits(self): got_topo = objects.VirtCPUTopology(sockets=1, cores=2, threads=1) self.assertEqualTopology(got_topo, inst_pin.cpu_topology) + # TODO(stephenfin): Remove when we drop support for vcpu_pin_set def test_get_pinning_isolate_policy_fits_ht_host(self): host_pin = objects.NUMACell( id=0, - cpuset=set(), + # Simulate a legacy host with vcpu_pin_set configuration, + # meaning cpuset == pcpuset + cpuset=set([0, 1, 2, 3]), pcpuset=set([0, 1, 2, 3]), memory=4096, memory_usage=0, @@ -3096,10 +3108,13 @@ def test_get_pinning_isolate_policy_fits_ht_host(self): got_topo = objects.VirtCPUTopology(sockets=1, cores=2, threads=1) self.assertEqualTopology(got_topo, inst_pin.cpu_topology) + # TODO(stephenfin): Remove when we drop support for vcpu_pin_set def test_get_pinning_isolate_policy_fits_w_usage(self): host_pin = objects.NUMACell( id=0, - cpuset=set(), + # Simulate a legacy host with vcpu_pin_set configuration, + # meaning cpuset == pcpuset + cpuset=set([0, 1, 2, 3, 4, 5, 6, 7]), pcpuset=set([0, 1, 2, 3, 4, 5, 6, 7]), memory=4096, memory_usage=0, @@ -3116,6 +3131,38 @@ def test_get_pinning_isolate_policy_fits_w_usage(self): got_topo = objects.VirtCPUTopology(sockets=1, cores=2, threads=1) self.assertEqualTopology(got_topo, inst_pin.cpu_topology) + # TODO(stephenfin): Remove when we drop support for vcpu_pin_set + @mock.patch.object(hw, 'LOG') + def test_get_pinning_isolate_policy_bug_1889633(self, mock_log): + host_pin = objects.NUMACell( + id=0, + cpuset={0, 1, 4, 5}, + pcpuset={2, 3, 6, 7}, + memory=4096, + memory_usage=0, + pinned_cpus=set(), + siblings=[{0, 4}, {1, 5}, {2, 6}, {3, 7}], + mempages=[], + ) + inst_pin = objects.InstanceNUMACell( + cpuset=set(), + pcpuset={0, 1}, + memory=2048, + cpu_policy=fields.CPUAllocationPolicy.DEDICATED, + cpu_thread_policy=fields.CPUThreadAllocationPolicy.ISOLATE, + ) + limits = objects.NUMATopologyLimits( + cpu_allocation_ratio=2, ram_allocation_ratio=2, + ) + + # host has hyperthreads, which means no NUMA topology should be built + inst_topo = hw._numa_fit_instance_cell(host_pin, inst_pin, limits) + self.assertIsNone(inst_topo) + self.assertIn( + 'Host supports hyperthreads, but instance requested no', + mock_log.warning.call_args[0][0], + ) + class CPUPinningTestCase(test.NoDBTestCase, _CPUPinningTestCaseBase): def test_host_numa_fit_instance_to_host_single_cell(self): @@ -3823,11 +3870,14 @@ def test_isolate_full_usage(self): self.assertEqual(set([0, 1]), host_topo.cells[0].pinned_cpus) + # TODO(stephenfin): Remove when we drop support for vcpu_pin_set def test_isolate_w_isolate_thread_alloc(self): host_topo = objects.NUMATopology(cells=[ objects.NUMACell( id=0, - cpuset=set(), + # Simulate a legacy host with vcpu_pin_set configuration, + # meaning cpuset == pcpuset + cpuset=set([0, 1, 2, 3, 4, 5]), pcpuset=set([0, 1, 2, 3, 4, 5]), memory=2048, cpu_usage=0, @@ -3850,11 +3900,14 @@ def test_isolate_w_isolate_thread_alloc(self): self.assertEqual({0: 0, 1: 2}, inst_topo.cells[0].cpu_pinning) self.assertEqual(set([4]), inst_topo.cells[0].cpuset_reserved) + # TODO(stephenfin): Remove when we drop support for vcpu_pin_set def test_isolate_w_isolate_thread_alloc_usage(self): host_topo = objects.NUMATopology(cells=[ objects.NUMACell( id=0, - cpuset=set(), + # Simulate a legacy host with vcpu_pin_set configuration, + # meaning cpuset == pcpuset + cpuset=set([0, 1, 2, 3, 4, 5]), pcpuset=set([0, 1, 2, 3, 4, 5]), memory=2048, cpu_usage=0, @@ -3910,11 +3963,14 @@ def test_asymmetric_host(self): self.assertEqual({0: 2, 1: 3}, inst_topo.cells[0].cpu_pinning) self.assertEqual(set([1]), inst_topo.cells[0].cpuset_reserved) + # TODO(stephenfin): Remove when we drop support for vcpu_pin_set def test_asymmetric_host_w_isolate_thread_alloc(self): host_topo = objects.NUMATopology(cells=[ objects.NUMACell( id=0, - cpuset=set(), + # Simulate a legacy host with vcpu_pin_set configuration, + # meaning cpuset == pcpuset + cpuset=set([1, 2, 3, 4, 5]), pcpuset=set([1, 2, 3, 4, 5]), memory=2048, cpu_usage=0, @@ -3924,8 +3980,7 @@ def test_asymmetric_host_w_isolate_thread_alloc(self): mempages=[objects.NUMAPagesTopology( size_kb=4, total=524288, used=0)])]) inst_topo = objects.InstanceNUMATopology( - emulator_threads_policy=( - fields.CPUEmulatorThreadsPolicy.ISOLATE), + emulator_threads_policy=fields.CPUEmulatorThreadsPolicy.ISOLATE, cells=[objects.InstanceNUMACell( id=0, cpuset=set([0, 1]), memory=2048, diff --git a/nova/tests/unit/virt/test_virt_drivers.py b/nova/tests/unit/virt/test_virt_drivers.py index c457b7b89bb..ff69b3f9c93 100644 --- a/nova/tests/unit/virt/test_virt_drivers.py +++ b/nova/tests/unit/virt/test_virt_drivers.py @@ -39,6 +39,7 @@ from nova.tests.unit import fake_block_device from nova.tests.unit.image import fake as fake_image from nova.tests.unit import utils as test_utils +from nova.tests.unit.virt.libvirt import fakelibvirt from nova.virt import block_device as driver_block_device from nova.virt import event as virtevent from nova.virt import fake @@ -617,6 +618,10 @@ def test_unfilter_instance(self): network_info = test_utils.get_test_network_info() self.connection.unfilter_instance(instance_ref, network_info) + @mock.patch( + 'nova.tests.unit.virt.libvirt.fakelibvirt.Domain.jobStats', + new=mock.Mock(return_value={ + 'type': fakelibvirt.VIR_DOMAIN_JOB_COMPLETED})) def test_live_migration(self): instance_ref, network_info = self._get_running_instance() fake_context = context.RequestContext('fake', 'fake') diff --git a/nova/tests/unit/virt/vmwareapi/test_vm_util.py b/nova/tests/unit/virt/vmwareapi/test_vm_util.py index ef19152d005..aa26346b380 100644 --- a/nova/tests/unit/virt/vmwareapi/test_vm_util.py +++ b/nova/tests/unit/virt/vmwareapi/test_vm_util.py @@ -1569,6 +1569,96 @@ def test_vm_create_spec_with_profile_spec(self): profile_spec='fake_profile_spec') self.assertEqual(['fake_profile_spec'], create_spec.vmProfile) + def test_vm_create_spec_with_multi_vifs(self): + datastore = ds_obj.Datastore('fake-ds-ref', 'fake-ds-name') + extra_specs = vm_util.ExtraSpecs() + vif_info = {'network_name': 'br100', + 'mac_address': '00:00:00:ca:fe:01', + 'network_ref': {'type': 'DistributedVirtualPortgroup', + 'dvsw': 'fake-network-id1', + 'dvpg': 'fake-group'}, + 'iface_id': 7, + 'vif_model': 'VirtualE1000'} + vif_info2 = {'network_name': 'br101', + 'mac_address': '00:00:00:ca:fe:02', + 'network_ref': {'type': 'DistributedVirtualPortgroup', + 'dvsw': 'fake-network-id2', + 'dvpg': 'fake-group'}, + 'iface_id': 7, + 'vif_model': 'VirtualE1000'} + create_spec = vm_util.get_vm_create_spec(fake.FakeFactory(), + self._instance, + datastore.name, + [vif_info, vif_info2], + extra_specs) + + port = 'ns0:DistributedVirtualSwitchPortConnection' + backing = 'ns0:VirtualEthernetCardDistributedVirtualPortBackingInfo' + + device_changes = [] + fake_factory = fake.FakeFactory() + device_change = fake_factory.create('ns0:VirtualDeviceConfigSpec') + device_change.operation = 'add' + + device = fake_factory.create('ns0:VirtualE1000') + device.key = -47 + device.macAddress = '00:00:00:ca:fe:01' + device.addressType = 'manual' + device.wakeOnLanEnabled = True + + device.backing = fake_factory.create(backing) + device.backing.port = fake_factory.create(port) + device.backing.port.portgroupKey = 'fake-group' + device.backing.port.switchUuid = 'fake-network-id1' + + device.resourceAllocation = fake_factory.create( + 'ns0:VirtualEthernetCardResourceAllocation') + device.resourceAllocation.share = fake_factory.create( + 'ns0:SharesInfo') + device.resourceAllocation.share.level = None + device.resourceAllocation.share.shares = None + + connectable = fake_factory.create('ns0:VirtualDeviceConnectInfo') + connectable.allowGuestControl = True + connectable.connected = True + connectable.startConnected = True + device.connectable = connectable + device_change.device = device + + device_changes.append(device_change) + + device_change = fake_factory.create('ns0:VirtualDeviceConfigSpec') + device_change.operation = 'add' + + device = fake_factory.create('ns0:VirtualE1000') + device.key = -48 + device.macAddress = '00:00:00:ca:fe:02' + device.addressType = 'manual' + device.wakeOnLanEnabled = True + + device.backing = fake_factory.create(backing) + device.backing.port = fake_factory.create(port) + device.backing.port.portgroupKey = 'fake-group' + device.backing.port.switchUuid = 'fake-network-id2' + + device.resourceAllocation = fake_factory.create( + 'ns0:VirtualEthernetCardResourceAllocation') + device.resourceAllocation.share = fake_factory.create( + 'ns0:SharesInfo') + device.resourceAllocation.share.level = None + device.resourceAllocation.share.shares = None + + connectable = fake_factory.create('ns0:VirtualDeviceConnectInfo') + connectable.allowGuestControl = True + connectable.connected = True + connectable.startConnected = True + device.connectable = connectable + device_change.device = device + + device_changes.append(device_change) + + self.assertEqual(device_changes, create_spec.deviceChange) + @mock.patch.object(pbm, 'get_profile_id_by_name') def test_get_storage_profile_spec(self, mock_retrieve_profile_id): fake_profile_id = fake.DataObject() diff --git a/nova/tests/unit/virt/xenapi/image/test_glance.py b/nova/tests/unit/virt/xenapi/image/test_glance.py index 3994af17926..be49622e733 100644 --- a/nova/tests/unit/virt/xenapi/image/test_glance.py +++ b/nova/tests/unit/virt/xenapi/image/test_glance.py @@ -148,7 +148,7 @@ def test_upload_image(self, mock_upload, mock_sr_path, mock_extra_header, mock_sr_path.assert_called_once_with(self.session) mock_extra_header.assert_called_once_with(self.context) mock_upload.assert_called_once_with( - self.session, 0, mock.ANY, mock.ANY, 'fake_image_uuid', + self.session, 3, mock.ANY, mock.ANY, 'fake_image_uuid', 'fake_sr_path', 'fake_extra_header', **params) @mock.patch.object(utils, 'get_auto_disk_config_from_instance') @@ -169,7 +169,7 @@ def test_upload_image_None_os_type(self, mock_upload, mock_sr_path, mock_sr_path.assert_called_once_with(self.session) mock_extra_header.assert_called_once_with(self.context) mock_upload.assert_called_once_with( - self.session, 0, mock.ANY, mock.ANY, 'fake_image_uuid', + self.session, 3, mock.ANY, mock.ANY, 'fake_image_uuid', 'fake_sr_path', 'fake_extra_header', **params) mock_disk_config.assert_called_once_with(self.instance) @@ -190,7 +190,7 @@ def test_upload_image_no_os_type(self, mock_upload, mock_sr_path, mock_sr_path.assert_called_once_with(self.session) mock_extra_header.assert_called_once_with(self.context) mock_upload.assert_called_once_with( - self.session, 0, mock.ANY, mock.ANY, 'fake_image_uuid', + self.session, 3, mock.ANY, mock.ANY, 'fake_image_uuid', 'fake_sr_path', 'fake_extra_header', **params) mock_disk_config.assert_called_once_with(self.instance) @@ -211,7 +211,7 @@ def test_upload_image_auto_config_disk_disabled( mock_sr_path.assert_called_once_with(self.session) mock_extra_header.assert_called_once_with(self.context) mock_upload.assert_called_once_with( - self.session, 0, mock.ANY, mock.ANY, 'fake_image_uuid', + self.session, 3, mock.ANY, mock.ANY, 'fake_image_uuid', 'fake_sr_path', 'fake_extra_header', **params) @mock.patch.object(common_glance, 'generate_identity_headers') @@ -231,7 +231,7 @@ def test_upload_image_raises_exception(self, mock_upload, mock_sr_path, mock_sr_path.assert_called_once_with(self.session) mock_extra_header.assert_called_once_with(self.context) mock_upload.assert_called_once_with( - self.session, 0, mock.ANY, mock.ANY, 'fake_image_uuid', + self.session, 3, mock.ANY, mock.ANY, 'fake_image_uuid', 'fake_sr_path', 'fake_extra_header', **params) @mock.patch.object(time, 'sleep') @@ -330,5 +330,5 @@ def test_upload_image_raises_exception_image_not_found(self, mock_sr_path.assert_called_once_with(self.session) mock_extra_header.assert_called_once_with(self.context) mock_upload.assert_called_once_with( - self.session, 0, mock.ANY, mock.ANY, 'fake_image_uuid', + self.session, 3, mock.ANY, mock.ANY, 'fake_image_uuid', 'fake_sr_path', 'fake_extra_header', **params) diff --git a/nova/tests/unit/virt/xenapi/test_vm_utils.py b/nova/tests/unit/virt/xenapi/test_vm_utils.py index ef2d687e8f8..ef29e27e36b 100644 --- a/nova/tests/unit/virt/xenapi/test_vm_utils.py +++ b/nova/tests/unit/virt/xenapi/test_vm_utils.py @@ -258,7 +258,7 @@ def _assert_call_plugin_serialized_with_retry(self): self.mock_call_plugin.assert_called_once_with( 'glance.py', 'download_vhd2', - 0, + 3, mock.ANY, mock.ANY, extra_headers={'X-Auth-Token': 'auth_token', diff --git a/nova/virt/hardware.py b/nova/virt/hardware.py index 4a275b6beb0..1a22c6b5b17 100644 --- a/nova/virt/hardware.py +++ b/nova/virt/hardware.py @@ -902,6 +902,30 @@ def _get_reserved(sibling_set, vcpus_pinning, num_cpu_reserved=0, 'for the isolate policy without this.') return + # TODO(stephenfin): Drop this when we drop support for 'vcpu_pin_set' + # NOTE(stephenfin): This is total hack. We're relying on the fact that + # the libvirt driver, which is the only one that currently supports + # pinned CPUs, will set cpuset and pcpuset to the same value if using + # legacy configuration, i.e. 'vcpu_pin_set', as part of + # '_get_host_numa_topology'. They can't be equal otherwise since + # 'cpu_dedicated_set' and 'cpu_shared_set' must be disjoint. Therefore, + # if these are equal, the host that this NUMA cell corresponds to is + # using legacy configuration and it's okay to use the old, "pin a core + # and reserve its siblings" implementation of the 'isolate' policy. If + # they're not, the host is using new-style configuration and we've just + # hit bug #1889633 + if threads_per_core != 1 and host_cell.pcpuset != host_cell.cpuset: + LOG.warning( + "Host supports hyperthreads, but instance requested no " + "hyperthreads. This should have been rejected by the " + "scheduler but we likely got here due to the fallback VCPU " + "query. Consider setting '[workarounds] " + "disable_fallback_pcpu_query' to 'True' once hosts are no " + "longer using 'vcpu_pin_set'. Refer to bug #1889633 for more " + "information." + ) + return + pinning = _get_pinning( 1, # we only want to "use" one thread per core sibling_sets[threads_per_core], diff --git a/nova/virt/libvirt/config.py b/nova/virt/libvirt/config.py index d0f9f215558..39c4da88478 100644 --- a/nova/virt/libvirt/config.py +++ b/nova/virt/libvirt/config.py @@ -596,11 +596,13 @@ def __init__(self, name=None, **kwargs): **kwargs) self.name = name + self.policy = "require" def parse_dom(self, xmldoc): super(LibvirtConfigCPUFeature, self).parse_dom(xmldoc) self.name = xmldoc.get("name") + self.policy = xmldoc.get("policy", "require") def format_dom(self): ft = super(LibvirtConfigCPUFeature, self).format_dom() @@ -652,7 +654,8 @@ def parse_dom(self, xmldoc): elif c.tag == "feature": f = LibvirtConfigCPUFeature() f.parse_dom(c) - self.add_feature(f) + if f.policy != "disable": + self.add_feature(f) def format_dom(self): cpu = super(LibvirtConfigCPU, self).format_dom() @@ -675,7 +678,8 @@ def format_dom(self): # sorting the features to allow more predictable tests for f in sorted(self.features, key=lambda x: x.name): - cpu.append(f.format_dom()) + if f.policy != "disable": + cpu.append(f.format_dom()) return cpu diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 5043589073e..da988294531 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -597,6 +597,22 @@ def _set_cache_mode(self, conf): driver_cache) conf.driver_cache = cache_mode + # NOTE(acewit): If the [libvirt]disk_cachemodes is set as + # `block=writeback` or `block=writethrough` or `block=unsafe`, + # whose correponding Linux's IO semantic is not O_DIRECT in + # file nova.conf, then it will result in an attachment failure + # because of the libvirt bug + # (https://bugzilla.redhat.com/show_bug.cgi?id=1086704) + if ((getattr(conf, 'driver_io', None) == "native") and + conf.driver_cache not in [None, 'none', 'directsync']): + conf.driver_io = "threads" + LOG.warning("The guest disk driver io mode has fallen back " + "from 'native' to 'threads' because the " + "disk cache mode is set as %(cachemode)s, which does" + "not use O_DIRECT. See the following bug report " + "for more details: https://launchpad.net/bugs/1841363", + {'cachemode': conf.driver_cache}) + def _do_quality_warnings(self): """Warn about potential configuration issues. @@ -2621,6 +2637,10 @@ def _live_snapshot(self, context, instance, guest, disk_path, out_path, libvirt_utils.extract_snapshot(disk_delta, 'qcow2', out_path, image_format) + # Remove the disk_delta file once the snapshot extracted, so that + # it doesn't hang around till the snapshot gets uploaded + fileutils.delete_if_exists(disk_delta) + def _volume_snapshot_update_status(self, context, snapshot_id, status): """Send a snapshot status update to Cinder. @@ -2841,32 +2861,15 @@ def _wait_for_snapshot(): timer.start(interval=0.5).wait() @staticmethod - def _rebase_with_qemu_img(guest, device, active_disk_object, - rebase_base): - """Rebase a device tied to a guest using qemu-img. - - :param guest:the Guest which owns the device being rebased - :type guest: nova.virt.libvirt.guest.Guest - :param device: the guest block device to rebase - :type device: nova.virt.libvirt.guest.BlockDevice - :param active_disk_object: the guest block device to rebase - :type active_disk_object: nova.virt.libvirt.config.\ - LibvirtConfigGuestDisk + def _rebase_with_qemu_img(source_path, rebase_base): + """Rebase a disk using qemu-img. + + :param source_path: the disk source path to rebase + :type source_path: string :param rebase_base: the new parent in the backing chain :type rebase_base: None or string """ - # It's unsure how well qemu-img handles network disks for - # every protocol. So let's be safe. - active_protocol = active_disk_object.source_protocol - if active_protocol is not None: - msg = _("Something went wrong when deleting a volume snapshot: " - "rebasing a %(protocol)s network disk using qemu-img " - "has not been fully tested") % {'protocol': - active_protocol} - LOG.error(msg) - raise exception.InternalError(msg) - if rebase_base is None: # If backing_file is specified as "" (the empty string), then # the image is rebased onto no backing file (i.e. it will exist @@ -2877,11 +2880,20 @@ def _rebase_with_qemu_img(guest, device, active_disk_object, # If the rebased image is going to have a backing file then # explicitly set the backing file format to avoid any security # concerns related to file format auto detection. - backing_file = rebase_base + if os.path.isabs(rebase_base): + backing_file = rebase_base + else: + # this is a probably a volume snapshot case where the + # rebase_base is relative. See bug + # https://bugs.launchpad.net/nova/+bug/1885528 + backing_file_name = os.path.basename(rebase_base) + volume_path = os.path.dirname(source_path) + backing_file = os.path.join(volume_path, backing_file_name) + b_file_fmt = images.qemu_img_info(backing_file).file_format qemu_img_extra_arg = ['-F', b_file_fmt] - qemu_img_extra_arg.append(active_disk_object.source_path) + qemu_img_extra_arg.append(source_path) # execute operation with disk concurrency semaphore with compute_utils.disk_ops_semaphore: processutils.execute("qemu-img", "rebase", "-b", backing_file, @@ -3044,7 +3056,18 @@ def _get_snap_dev(filename, backing_store): else: LOG.debug('Guest is not running so doing a block rebase ' 'using "qemu-img rebase"', instance=instance) - self._rebase_with_qemu_img(guest, dev, active_disk_object, + + # It's unsure how well qemu-img handles network disks for + # every protocol. So let's be safe. + active_protocol = active_disk_object.source_protocol + if active_protocol is not None: + msg = _("Something went wrong when deleting a volume " + "snapshot: rebasing a %(protocol)s network disk " + "using qemu-img has not been fully tested" + ) % {'protocol': active_protocol} + LOG.error(msg) + raise exception.InternalError(msg) + self._rebase_with_qemu_img(active_disk_object.source_path, rebase_base) else: @@ -4001,9 +4024,24 @@ def _create_and_inject_local_root(self, context, instance, backend.create_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME) if backend.SUPPORTS_CLONE: def clone_fallback_to_fetch(*args, **kwargs): + refuse_fetch = ( + CONF.libvirt.images_type == 'rbd' and + CONF.workarounds.never_download_image_if_on_rbd) try: backend.clone(context, disk_images['image_id']) except exception.ImageUnacceptable: + if refuse_fetch: + # Re-raise the exception from the failed + # ceph clone. The compute manager expects + # ImageUnacceptable as a possible result + # of spawn(), from which this is called. + with excutils.save_and_reraise_exception(): + LOG.warning( + 'Image %s is not on my ceph and ' + '[workarounds]/' + 'never_download_image_if_on_rbd=True;' + ' refusing to fetch and upload.', + disk_images['image_id']) libvirt_utils.fetch_image(*args, **kwargs) fetch_func = clone_fallback_to_fetch else: @@ -4012,6 +4050,14 @@ def clone_fallback_to_fetch(*args, **kwargs): root_fname, disk_images['image_id'], instance, size, fallback_from_host) + # During unshelve on Qcow2 backend, we spawn() using snapshot image + # created during shelve. Extra work is needed in order to rebase + # disk image to its original image_ref. Disk backing file will + # then represent back image_ref instead of shelved image. + if (instance.vm_state == vm_states.SHELVED_OFFLOADED and + isinstance(backend, imagebackend.Qcow2)): + self._finalize_unshelve_qcow2_image(context, instance, backend) + if need_inject: self._inject_data(backend, instance, injection_info) @@ -4021,6 +4067,36 @@ def clone_fallback_to_fetch(*args, **kwargs): return created_disks + def _finalize_unshelve_qcow2_image(self, context, instance, backend): + # NOTE(aarents): During qcow2 instance unshelve, backing file + # represents shelved image, not original instance.image_ref. + # We rebase here instance disk to original image. + # This second fetch call does nothing except downloading original + # backing file if missing, as image disk have already been + # created/resized by first fetch call. + base_dir = self.image_cache_manager.cache_dir + base_image_ref = instance.system_metadata.get('image_base_image_ref') + root_fname = imagecache.get_cache_fname(base_image_ref) + base_backing_fname = os.path.join(base_dir, root_fname) + + try: + self._try_fetch_image_cache(backend, libvirt_utils.fetch_image, + context, root_fname, base_image_ref, + instance, None) + except exception.ImageNotFound: + # We must flatten here in order to remove dependency with an orphan + # backing file (as shelved image will be dropped once unshelve + # is successfull). + LOG.warning('Current disk image is created on top of shelved ' + 'image and cannot be rebased to original image ' + 'because it is no longer available in the image ' + 'service, disk will be consequently flattened.', + instance=instance) + base_backing_fname = None + + LOG.info('Rebasing disk image.', instance=instance) + self._rebase_with_qemu_img(backend.path, base_backing_fname) + def _create_configdrive(self, context, instance, injection_info, rescue=False): # As this method being called right after the definition of a @@ -5106,7 +5182,9 @@ def _set_features(self, guest, os_type, caps, virt_type, image_meta, flavor): hide_hypervisor_id = (strutils.bool_from_string( flavor.extra_specs.get('hide_hypervisor_id')) or - image_meta.properties.get('img_hide_hypervisor_id')) + strutils.bool_from_string( + flavor.extra_specs.get('hw:hide_hypervisor_id')) or + image_meta.properties.get('img_hide_hypervisor_id')) if virt_type == "xen": # PAE only makes sense in X86 @@ -5214,7 +5292,7 @@ def _add_video_driver(self, guest, image_meta, flavor): raise exception.RequestedVRamTooHigh(req_vram=video_ram, max_vram=max_vram) if max_vram and video_ram: - video.vram = video_ram * units.Mi / units.Ki + video.vram = video_ram * units.Mi // units.Ki guest.add_device(video) # NOTE(sean-k-mooney): return the video device we added diff --git a/nova/virt/libvirt/guest.py b/nova/virt/libvirt/guest.py index 318294c659a..a7fbc50ef2c 100644 --- a/nova/virt/libvirt/guest.py +++ b/nova/virt/libvirt/guest.py @@ -367,8 +367,8 @@ def get_all_devices(self, devtype=None): return devs def detach_device_with_retry(self, get_device_conf_func, device, live, - max_retry_count=7, inc_sleep_time=2, - max_sleep_time=30, + max_retry_count=7, inc_sleep_time=10, + max_sleep_time=60, alternative_device_name=None, supports_device_missing_error_code=False): """Detaches a device from the guest. After the initial detach request, diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py index 603ab9925fd..d8078dd3873 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -124,15 +124,15 @@ def __init__(self, uri, read_only=False, @staticmethod def _get_libvirt_proxy_classes(libvirt_module): """Return a tuple for tpool.Proxy's autowrap argument containing all - classes defined by the libvirt module except libvirtError. + public vir* classes defined by the libvirt module. """ # Get a list of (name, class) tuples of libvirt classes classes = inspect.getmembers(libvirt_module, inspect.isclass) - # Return a list of just the classes, filtering out libvirtError because - # we don't need to proxy that - return tuple([cls[1] for cls in classes if cls[0] != 'libvirtError']) + # Return a list of just the vir* classes, filtering out libvirtError + # and any private globals pointing at private internal classes. + return tuple([cls[1] for cls in classes if cls[0].startswith("vir")]) def _wrap_libvirt_proxy(self, obj): """Return an object wrapped in a tpool.Proxy using autowrap appropriate diff --git a/nova/virt/libvirt/migration.py b/nova/virt/libvirt/migration.py index 7b7214712c8..36a9d4bcd9b 100644 --- a/nova/virt/libvirt/migration.py +++ b/nova/virt/libvirt/migration.py @@ -346,14 +346,21 @@ def _update_vif_xml(xml_doc, migrate_data, get_vif_config): instance_uuid = xml_doc.findtext('uuid') parser = etree.XMLParser(remove_blank_text=True) interface_nodes = xml_doc.findall('./devices/interface') - migrate_vif_by_mac = {vif.source_vif['address']: vif + # MAC address stored for port in neutron DB and in domain XML + # might be in different cases, so to harmonize that + # we convert MAC to lower case for dict key. + migrate_vif_by_mac = {vif.source_vif['address'].lower(): vif for vif in migrate_data.vifs} for interface_dev in interface_nodes: mac = interface_dev.find('mac') mac = mac if mac is not None else {} mac_addr = mac.get('address') if mac_addr: - migrate_vif = migrate_vif_by_mac[mac_addr] + # MAC address stored in libvirt should always be normalized + # and stored in lower case. But just to be extra safe here + # we still normalize MAC retrieved from XML to be absolutely + # sure it will be the same with the Neutron provided one. + migrate_vif = migrate_vif_by_mac[mac_addr.lower()] vif = migrate_vif.get_dest_vif() # get_vif_config is a partial function of # nova.virt.libvirt.vif.LibvirtGenericVIFDriver.get_config diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py index 89c259a6181..c03548d007d 100644 --- a/nova/virt/libvirt/vif.py +++ b/nova/virt/libvirt/vif.py @@ -126,6 +126,24 @@ def get_vif_devname(self, vif): return vif['devname'] return ("nic" + vif['id'])[:network_model.NIC_NAME_LEN] + def get_vif_model(self, image_meta=None, vif_model=None): + + model = vif_model + + # If the user has specified a 'vif_model' against the + # image then honour that model + if image_meta: + model = osinfo.HardwareProperties(image_meta).network_model + + # If the virt type is KVM/QEMU/VZ(Parallels), then use virtio according + # to the global config parameter + if (model is None and CONF.libvirt.virt_type in + ('kvm', 'qemu', 'parallels') and + CONF.libvirt.use_virtio_for_bridges): + model = network_model.VIF_MODEL_VIRTIO + + return model + def get_base_config(self, instance, mac, image_meta, inst_type, virt_type, vnic_type, host): # TODO(sahid): We should rewrite it. This method handles too @@ -146,16 +164,9 @@ def get_base_config(self, instance, mac, image_meta, conf, mac, model, driver, vhost_queues, rx_queue_size) return conf - # If the user has specified a 'vif_model' against the - # image then honour that model - if image_meta: - model = osinfo.HardwareProperties(image_meta).network_model - - # If the virt type is KVM/QEMU/VZ(Parallels), then use virtio according - # to the global config parameter - if (model is None and virt_type in ('kvm', 'qemu', 'parallels') and - CONF.libvirt.use_virtio_for_bridges): - model = network_model.VIF_MODEL_VIRTIO + # if model has already been defined, + # image_meta contents will override it + model = self.get_vif_model(image_meta=image_meta, vif_model=model) if not is_vif_model_valid_for_virt(virt_type, model): raise exception.UnsupportedHardware(model=model, virt=virt_type) @@ -212,10 +223,7 @@ def _get_virtio_mq_settings(self, image_meta, flavor): """ driver = None vhost_queues = None - if not isinstance(image_meta, objects.ImageMeta): - image_meta = objects.ImageMeta.from_dict(image_meta) - img_props = image_meta.properties - if img_props.get('hw_vif_multiqueue_enabled'): + if self._requests_multiqueue(image_meta): driver = 'vhost' max_tap_queues = self._get_max_tap_queues() if max_tap_queues: @@ -226,7 +234,27 @@ def _get_virtio_mq_settings(self, image_meta, flavor): return (driver, vhost_queues) + def _requests_multiqueue(self, image_meta): + """Check if multiqueue property is set in the image metadata.""" + + if not isinstance(image_meta, objects.ImageMeta): + image_meta = objects.ImageMeta.from_dict(image_meta) + + img_props = image_meta.properties + + if img_props.get('hw_vif_multiqueue_enabled'): + return True + + return False + def _get_max_tap_queues(self): + # Note(sean-k-mooney): some linux distros have backported + # changes for newer kernels which make the kernel version + # number unreliable to determine the max queues supported + # To address this without making the code distro dependent + # we introduce a new config option and prefer it if set. + if CONF.libvirt.max_queues: + return CONF.libvirt.max_queues # NOTE(kengo.sakai): In kernels prior to 3.0, # multiple queues on a tap interface is not supported. # In kernels 3.x, the number of queues on a tap interface @@ -690,7 +718,13 @@ def plug_tap(self, instance, vif): """Plug a VIF_TYPE_TAP virtual interface.""" dev = self.get_vif_devname(vif) mac = vif['details'].get(network_model.VIF_DETAILS_TAP_MAC_ADDRESS) - nova.privsep.linux_net.create_tap_dev(dev, mac) + image_meta = instance.image_meta + vif_model = self.get_vif_model(image_meta=image_meta) + # TODO(ganso): explore whether multiqueue works for other vif models + # that go through this code path. + multiqueue = (self._requests_multiqueue(image_meta) and + vif_model == network_model.VIF_MODEL_VIRTIO) + nova.privsep.linux_net.create_tap_dev(dev, mac, multiqueue=multiqueue) network = vif.get('network') mtu = network.get_meta('mtu') if network else None nova.privsep.linux_net.set_device_mtu(dev, mtu) diff --git a/nova/virt/vmwareapi/vm_util.py b/nova/virt/vmwareapi/vm_util.py index 22820d5d843..ac3c1b9762e 100644 --- a/nova/virt/vmwareapi/vm_util.py +++ b/nova/virt/vmwareapi/vm_util.py @@ -242,9 +242,9 @@ def get_vm_create_spec(client_factory, instance, data_store_name, config_spec.firmware = extra_specs.firmware devices = [] - for vif_info in vif_infos: + for i, vif_info in enumerate(vif_infos): vif_spec = _create_vif_spec(client_factory, vif_info, - extra_specs.vif_limits) + extra_specs.vif_limits, i) devices.append(vif_spec) serial_port_spec = create_serial_port_spec(client_factory) @@ -416,7 +416,7 @@ def convert_vif_model(name): return name -def _create_vif_spec(client_factory, vif_info, vif_limits=None): +def _create_vif_spec(client_factory, vif_info, vif_limits=None, offset=0): """Builds a config spec for the addition of a new network adapter to the VM. """ @@ -478,7 +478,7 @@ def _create_vif_spec(client_factory, vif_info, vif_limits=None): # The Server assigns a Key to the device. Here we pass a -ve temporary key. # -ve because actual keys are +ve numbers and we don't # want a clash with the key that server might associate with the device - net_device.key = -47 + net_device.key = -47 - offset net_device.addressType = "manual" net_device.macAddress = mac_address net_device.wakeOnLanEnabled = True diff --git a/releasenotes/notes/avoid_muli_ceph_download-4083decf501dba40.yaml b/releasenotes/notes/avoid_muli_ceph_download-4083decf501dba40.yaml new file mode 100644 index 00000000000..f79c2781196 --- /dev/null +++ b/releasenotes/notes/avoid_muli_ceph_download-4083decf501dba40.yaml @@ -0,0 +1,19 @@ +--- +other: + - | + Nova now has a config option called + ``[workarounds]/never_download_image_if_on_rbd`` which helps to + avoid pathological storage behavior with multiple ceph clusters. + Currently, Nova does *not* support multiple ceph clusters + properly, but Glance can be configured with them. If an instance + is booted from an image residing in a ceph cluster other than the + one Nova knows about, it will silently download it from Glance and + re-upload the image to the local ceph privately for that + instance. Unlike the behavior you expect when configuring Nova and + Glance for ceph, Nova will continue to do this over and over for + the same image when subsequent instances are booted, consuming a + large amount of storage unexpectedly. The new workaround option + will cause Nova to refuse to do this download/upload behavior and + instead fail the instance boot. It is simply a stop-gap effort to + allow unsupported deployments with multiple ceph clusters from + silently consuming large amounts of disk space. diff --git a/releasenotes/notes/bug-1821755-7bd03319e34b6b10.yaml b/releasenotes/notes/bug-1821755-7bd03319e34b6b10.yaml new file mode 100644 index 00000000000..4c6135311bb --- /dev/null +++ b/releasenotes/notes/bug-1821755-7bd03319e34b6b10.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Improved detection of anti-affinity policy violation when performing live + and cold migrations. Most of the violations caused by race conditions due + to performing concurrent live or cold migrations should now be addressed + by extra checks in the compute service. Upon detection, cold migration + operations are automatically rescheduled, while live migrations have two + checks and will be rescheduled if detected by the first one, otherwise the + live migration will fail cleanly and revert the instance state back to its + previous value. diff --git a/releasenotes/notes/bug-1841363-fallback-to-threaded-io-when-native-io-is-not-supported-fe56014e9648a518.yaml b/releasenotes/notes/bug-1841363-fallback-to-threaded-io-when-native-io-is-not-supported-fe56014e9648a518.yaml new file mode 100644 index 00000000000..8ea68c97419 --- /dev/null +++ b/releasenotes/notes/bug-1841363-fallback-to-threaded-io-when-native-io-is-not-supported-fe56014e9648a518.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + Since Libvirt v.1.12.0 and the introduction of the `libvirt issue`_ , + there is a fact that if we set cache mode whose write semantic is not + O_DIRECT (i.e. "unsafe", "writeback" or "writethrough"), there will + be a problem with the volume drivers (i.e. LibvirtISCSIVolumeDriver, + LibvirtNFSVolumeDriver and so on), which designate native io explicitly. + + When the driver_cache (default is none) has been configured as neither + "none" nor "directsync", the libvirt driver will ensure the driver_io + to be "threads" to avoid an instance spawning failure. + + .. _`libvirt issue`: https://bugzilla.redhat.com/show_bug.cgi?id=1086704 diff --git a/releasenotes/notes/bug-1841932-c871ac7b3b05d67e.yaml b/releasenotes/notes/bug-1841932-c871ac7b3b05d67e.yaml new file mode 100644 index 00000000000..d54be4f03a9 --- /dev/null +++ b/releasenotes/notes/bug-1841932-c871ac7b3b05d67e.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Add support for the ``hw:hide_hypervisor_id`` extra spec. This is an + alias for the ``hide_hypervisor_id`` extra spec, which was not + compatible with the ``AggregateInstanceExtraSpecsFilter`` scheduler + filter. See + `bug 1841932 `_ for more + details. diff --git a/releasenotes/notes/bug-1889633-37e524fb6c20fbdf.yaml b/releasenotes/notes/bug-1889633-37e524fb6c20fbdf.yaml new file mode 100644 index 00000000000..77c86013e07 --- /dev/null +++ b/releasenotes/notes/bug-1889633-37e524fb6c20fbdf.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + An issue that could result in instances with the ``isolate`` thread policy + (``hw:cpu_thread_policy=isolate``) being scheduled to hosts with SMT + (HyperThreading) and consuming ``VCPU`` instead of ``PCPU`` has been + resolved. See `bug #1889633`__ for more information. + + .. __: https://bugs.launchpad.net/nova/+bug/1889633 diff --git a/releasenotes/notes/bug-1892361-pci-deivce-type-update-c407a66fd37f6405.yaml b/releasenotes/notes/bug-1892361-pci-deivce-type-update-c407a66fd37f6405.yaml new file mode 100644 index 00000000000..ba35c25b02d --- /dev/null +++ b/releasenotes/notes/bug-1892361-pci-deivce-type-update-c407a66fd37f6405.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + Fixes `bug 1892361`_ in which the pci stat pools are not updated when an + existing device is enabled with SRIOV capability. Restart of nova-compute + service updates the pci device type from type-PCI to type-PF but the pools + still maintain the device type as type-PCI. And so the PF is considered for + allocation to instance that requests vnic_type=direct. With this fix, the + pci device type updates are detected and the pci stat pools are updated + properly. + + .. _bug 1892361: https://bugs.launchpad.net/nova/+bug/1892361 diff --git a/releasenotes/notes/bug-1893263-769acadc4b6141d0.yaml b/releasenotes/notes/bug-1893263-769acadc4b6141d0.yaml new file mode 100644 index 00000000000..3874489513b --- /dev/null +++ b/releasenotes/notes/bug-1893263-769acadc4b6141d0.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Addressed an issue that prevented instances using multiqueue feature from + being created successfully when their vif_type is TAP. diff --git a/releasenotes/notes/bug-1894966-d25c12b1320cb910.yaml b/releasenotes/notes/bug-1894966-d25c12b1320cb910.yaml new file mode 100644 index 00000000000..f87cc7806da --- /dev/null +++ b/releasenotes/notes/bug-1894966-d25c12b1320cb910.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Resolved an issue whereby providing an empty list for the ``policies`` + field in the request body of the ``POST /os-server-groups`` API would + result in a server error. This only affects the 2.1 to 2.63 microversions, + as the 2.64 microversion replaces the ``policies`` list field with a + ``policy`` string field. See `bug #1894966`__ for more information. + + .. __: https://bugs.launchpad.net/nova/+bug/1894966 diff --git a/releasenotes/notes/console-proxy-reject-open-redirect-4ac0a7895acca7eb.yaml b/releasenotes/notes/console-proxy-reject-open-redirect-4ac0a7895acca7eb.yaml new file mode 100644 index 00000000000..ce05b9a8670 --- /dev/null +++ b/releasenotes/notes/console-proxy-reject-open-redirect-4ac0a7895acca7eb.yaml @@ -0,0 +1,19 @@ +--- +security: + - | + A vulnerability in the console proxies (novnc, serial, spice) that allowed + open redirection has been `patched`_. The novnc, serial, and spice console + proxies are implemented as websockify servers and the request handler + inherits from the python standard SimpleHTTPRequestHandler. There is a + `known issue`_ in the SimpleHTTPRequestHandler which allows open redirects + by way of URLs in the following format:: + + http://vncproxy.my.domain.com//example.com/%2F.. + + which if visited, will redirect a user to example.com. + + The novnc, serial, and spice console proxies will now reject requests that + pass a redirection URL beginning with "//" with a 400 Bad Request. + + .. _patched: https://bugs.launchpad.net/nova/+bug/1927677 + .. _known issue: https://bugs.python.org/issue32084 diff --git a/releasenotes/notes/increase_glance_num_retries-ddfcd7053631882b.yaml b/releasenotes/notes/increase_glance_num_retries-ddfcd7053631882b.yaml new file mode 100644 index 00000000000..72b5467bcd2 --- /dev/null +++ b/releasenotes/notes/increase_glance_num_retries-ddfcd7053631882b.yaml @@ -0,0 +1,11 @@ +--- +upgrade: + - | + The default for ``[glance] num_retries`` has changed from ``0`` to ``3``. + The option controls how many times to retry a Glance API call in response + to a HTTP connection failure. When deploying Glance behind HAproxy it is + possible for a response to arrive just after the HAproxy idle time. As a + result, an exception will be raised when the connection is closed resulting + in a failed request. By increasing the default value, Nova can be more + resilient to this scenario were HAproxy is misconfigured by retrying the + request. diff --git a/releasenotes/notes/restore-rocky-portbinding-semantics-48e9b1fa969cc5e9.yaml b/releasenotes/notes/restore-rocky-portbinding-semantics-48e9b1fa969cc5e9.yaml new file mode 100644 index 00000000000..dc33e3c61db --- /dev/null +++ b/releasenotes/notes/restore-rocky-portbinding-semantics-48e9b1fa969cc5e9.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + In the Rocky (18.0.0) release support was added to nova to use neutron's + multiple port binding feature when the binding-extended API extension + is available. In the Train (20.0.0) release the SR-IOV live migration + feature broke the semantics of the vifs field in the ``migration_data`` + object that signals if the new multiple port binding workflow should + be used by always populating it even when the ``binding-extended`` API + extension is not present. This broke live migration for any deployment + that did not support the optional ``binding-extended`` API extension. + The Rocky behavior has now been restored enabling live migration + using the single port binding workflow when multiple port bindings + are not available. diff --git a/releasenotes/notes/virtio-max-queues-27f73e988c7e66ba.yaml b/releasenotes/notes/virtio-max-queues-27f73e988c7e66ba.yaml new file mode 100644 index 00000000000..845f0c258fd --- /dev/null +++ b/releasenotes/notes/virtio-max-queues-27f73e988c7e66ba.yaml @@ -0,0 +1,15 @@ +--- +other: + - | + The nova libvirt virt driver supports creating instances with multi-queue + virtio network interfaces. In previous releases nova has based the maximum + number of virtio queue pairs that can be allocated on the reported kernel + major version. It has been reported in `bug #1847367`_ that some distros have + backported changes from later major versions that make major version + number no longer suitable to determine the maximum virtio queue pair count. + A new config option has been added to the libvirt section of the nova.conf. + When defined nova will now use the ``[libvirt]/max_queues`` option to + define the max queues that can be configured, if undefined it will + fallback to the previous kernel version approach. + + .. _`bug #1847367`: https://bugs.launchpad.net/nova/+bug/1847367 diff --git a/test-requirements.txt b/test-requirements.txt index a3e369f85a6..98a6bba99ab 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -16,12 +16,12 @@ python-barbicanclient>=4.5.2 # Apache-2.0 python-ironicclient!=2.7.1,>=2.7.0 # Apache-2.0 requests-mock>=1.2.0 # Apache-2.0 oslotest>=3.8.0 # Apache-2.0 -stestr>=1.0.0 # Apache-2.0 +stestr>=2.0.0 # Apache-2.0 osprofiler>=1.4.0 # Apache-2.0 testresources>=2.0.0 # Apache-2.0/BSD testscenarios>=0.4 # Apache-2.0/BSD testtools>=2.2.0 # MIT -bandit>=1.1.0 # Apache-2.0 +bandit>=1.1.0,<=1.6.2 # Apache-2.0 gabbi>=1.35.0 # Apache-2.0 wsgi-intercept>=1.7.0 # MIT License diff --git a/tools/check-cherry-picks.sh b/tools/check-cherry-picks.sh index 32627e59b60..5a449c520b7 100755 --- a/tools/check-cherry-picks.sh +++ b/tools/check-cherry-picks.sh @@ -32,7 +32,7 @@ if [ $checked -eq 0 ]; then echo "Checked $checked cherry-pick hashes: OK" exit 0 else - if ! git show --format='%B' --quiet | grep -qi 'stable.*only'; then + if ! git show --format='%B' --quiet $commit_hash | grep -qi 'stable.*only'; then echo 'Stable branch requires either cherry-pick -x headers or [stable-only] tag!' exit 1 fi diff --git a/tox.ini b/tox.ini index 209875c44f7..47e6d1e88cb 100644 --- a/tox.ini +++ b/tox.ini @@ -1,10 +1,14 @@ [tox] -minversion = 3.1.1 +minversion = 3.2.0 envlist = py27,py37,functional,pep8 # Automatic envs (pyXX) will only use the python version appropriate to that # env and ignore basepython inherited from [testenv] if we set # ignore_basepython_conflict. ignore_basepython_conflict = True +# Pin the virtualenv and therefore the setuptools version used for the env +# creation. This results in a new tox being installed in .tox/.tox virtualenv +# and the tox on the host will delegate all the calls to the tox in that env. +requires = virtualenv<20.8 [testenv] basepython = python3 @@ -57,7 +61,6 @@ commands = bash -c "! find doc/ -type f -name *.json | xargs grep -U -n $'\r'" # Check that all included JSON files are valid JSON bash -c '! find doc/ -type f -name *.json | xargs -t -n1 python -m json.tool 2>&1 > /dev/null | grep -B1 -v ^python' - bash tools/check-cherry-picks.sh [testenv:fast8] description = @@ -66,6 +69,15 @@ envdir = {toxworkdir}/shared commands = bash tools/flake8wrap.sh -HEAD +[testenv:validate-backport] +description = + Determine whether a backport is ready to be merged by checking whether it has + already been merged to master or more recent stable branches. +deps = +skipsdist = true +commands = + bash tools/check-cherry-picks.sh + [testenv:functional] # TODO(melwitt): This can be removed when functional tests are gating with # python 3.x