[SRU] Virsh disk attach errors silently ignored

Bug #1825882 reported by Gustavo Randich on 2019-04-22
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Medium
Lee Yarwood
Queens
Medium
Lee Yarwood
Rocky
Medium
Lee Yarwood
Stein
Medium
Lee Yarwood
Ubuntu Cloud Archive
Status tracked in Stein
Queens
Medium
Unassigned
Rocky
Medium
Unassigned
Stein
Medium
Sahid Orentino
nova (Ubuntu)
Medium
Sahid Orentino
Bionic
Medium
Sahid Orentino
Cosmic
Medium
Sahid Orentino
Disco
Medium
Sahid Orentino

Bug Description

[Impact]

The following commit (1) is causing volume attachments which fail due to libvirt device attach erros to be silently ignored and Nova report the attachment as successful.

It seems that the original intention of the commit was to log a condition and re-raise the exeption, but if the exception is of type libvirt.libvirtError and does not contain the searched pattern, the exception is ignored. If you unindent the raise statement, errors are reported again.

In our case we had ceph/apparmor configuration problems in compute nodes which prevented virsh attaching the device; volumes appeared as successfully attached but the corresponding block device missing in guests VMs. Other libvirt attach error conditions are ignored also, as when you have already occuppied device names (i.e. 'Target vdb already exists', device is busy, etc.)

(1) https://github.com/openstack/nova/commit/78891c2305bff6e16706339a9c5eca99a84e409c

[Test Case]

* Deploy any OpenStack version up to Pike , which includes ceph backed cinder
* Create a guest VM (openstack server ...)
* Create a test cinder volume

$ openstack volume create test --size 10

* Force a drop on ceph traffic. Run the following command on the nova hypervisor on which the server runs.

$ iptables -A OUTPUT -d ceph-mon-addr -p tcp --dport 6800 -j DROP

* Attach the volume to a running instance.

$ openstack server add volume 7151f507-a6b7-4f6d-a4cc-fd223d9feb5d 742ff117-21ae-4d1b-a52b-5b37955716ff

* This should cause the volume attachment to fail

$ virsh domblklist instance-xxxxx
Target Source
------------------------------------------------
vda nova/7151f507-a6b7-4f6d-a4cc-fd223d9feb5d_disk

No volume should attached after this step.

* If the behavior is fixed:

   * Check that openstack server show , doesn't displays the displays the volume as attached.
   * Check that proper log entries states the libvirt exception and error.

* If the behavior isn't fixed:

   * openstack server show <ID> , will display the volume in the volumes_attached property.

[Expected result]

* Volume attach fails and a proper exception is logged.

[Actual result]

* Volume attach fails but remains connected to the host and no further
exception gets logged.

[Regression Potential]

* The patches have been cherry-picked from upstream which helps to reduce the regression potential of these fixes.

[Other Info]

* N/A

Description

tags: added: libvirt
Matt Riedemann (mriedem) wrote :

Yeah indeed it looks like the raise was used improperly. Probably a better thing to do here would be to use the excutils.save_and_reraise_exception() which provides a context manager that you can control if the exception should be re-raised or not.

Changed in nova:
status: New → Triaged
importance: Undecided → Medium

Fix proposed to branch: master
Review: https://review.opendev.org/655696

Changed in nova:
assignee: nobody → Lee Yarwood (lyarwood)
status: Triaged → In Progress

Reviewed: https://review.opendev.org/655696
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=bc57ae50734fa6a70115b6369e867079fb5eb4b8
Submitter: Zuul
Branch: master

commit bc57ae50734fa6a70115b6369e867079fb5eb4b8
Author: Lee Yarwood <email address hidden>
Date: Thu Apr 25 14:42:09 2019 +0100

    libvirt: Stop ignoring unknown libvirtError exceptions during volume attach

    Id346bce6e47431988cce7001abcf29a9faf2936a attempted to introduce a
    simple breadcrumb in the logs to highlight a known Libvirt issue.
    Unfortunately this change resulted in libvirtError exceptions that
    didn't match the known issue being silently ignored.

    This change corrects this by using excutils.save_and_reraise_exception
    to ensure all libvirtError exceptions are logged and raised regardless
    of being linked to the known issue.

    Change-Id: Ib440f4f2e484312af5f393722363846f6c95b760
    Closes-Bug: #1825882

Changed in nova:
status: In Progress → Fix Released

Reviewed: https://review.opendev.org/657049
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=b3dedefcc58a1fc76ba37f9f8bb1ef7d238aaceb
Submitter: Zuul
Branch: stable/stein

commit b3dedefcc58a1fc76ba37f9f8bb1ef7d238aaceb
Author: Lee Yarwood <email address hidden>
Date: Thu Apr 25 14:42:09 2019 +0100

    libvirt: Stop ignoring unknown libvirtError exceptions during volume attach

    Id346bce6e47431988cce7001abcf29a9faf2936a attempted to introduce a
    simple breadcrumb in the logs to highlight a known Libvirt issue.
    Unfortunately this change resulted in libvirtError exceptions that
    didn't match the known issue being silently ignored.

    This change corrects this by using excutils.save_and_reraise_exception
    to ensure all libvirtError exceptions are logged and raised regardless
    of being linked to the known issue.

    Change-Id: Ib440f4f2e484312af5f393722363846f6c95b760
    Closes-Bug: #1825882
    (cherry picked from commit bc57ae50734fa6a70115b6369e867079fb5eb4b8)

Reviewed: https://review.opendev.org/657051
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=34b4220448395f10eb2fd39d68b3f527339ab414
Submitter: Zuul
Branch: stable/rocky

commit 34b4220448395f10eb2fd39d68b3f527339ab414
Author: Lee Yarwood <email address hidden>
Date: Thu Apr 25 14:42:09 2019 +0100

    libvirt: Stop ignoring unknown libvirtError exceptions during volume attach

    Id346bce6e47431988cce7001abcf29a9faf2936a attempted to introduce a
    simple breadcrumb in the logs to highlight a known Libvirt issue.
    Unfortunately this change resulted in libvirtError exceptions that
    didn't match the known issue being silently ignored.

    This change corrects this by using excutils.save_and_reraise_exception
    to ensure all libvirtError exceptions are logged and raised regardless
    of being linked to the known issue.

    Change-Id: Ib440f4f2e484312af5f393722363846f6c95b760
    Closes-Bug: #1825882
    (cherry picked from commit bc57ae50734fa6a70115b6369e867079fb5eb4b8)
    (cherry picked from commit b3dedefcc58a1fc76ba37f9f8bb1ef7d238aaceb)

Reviewed: https://review.opendev.org/657050
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=63b45a87b82655824e61641016430a6613cee001
Submitter: Zuul
Branch: stable/queens

commit 63b45a87b82655824e61641016430a6613cee001
Author: Lee Yarwood <email address hidden>
Date: Thu Apr 25 14:42:09 2019 +0100

    libvirt: Stop ignoring unknown libvirtError exceptions during volume attach

    Id346bce6e47431988cce7001abcf29a9faf2936a attempted to introduce a
    simple breadcrumb in the logs to highlight a known Libvirt issue.
    Unfortunately this change resulted in libvirtError exceptions that
    didn't match the known issue being silently ignored.

    This change corrects this by using excutils.save_and_reraise_exception
    to ensure all libvirtError exceptions are logged and raised regardless
    of being linked to the known issue.

    Change-Id: Ib440f4f2e484312af5f393722363846f6c95b760
    Closes-Bug: #1825882
    (cherry picked from commit bc57ae50734fa6a70115b6369e867079fb5eb4b8)
    (cherry picked from commit b3dedefcc58a1fc76ba37f9f8bb1ef7d238aaceb)
    (cherry picked from commit 34b4220448395f10eb2fd39d68b3f527339ab414)

description: updated
summary: - Virsh disk attach errors silently ignored
+ [SRU] Virsh disk attach errors silently ignored
Changed in nova (Ubuntu Disco):
status: New → Triaged
Changed in nova (Ubuntu Cosmic):
status: New → Triaged
Changed in nova (Ubuntu Bionic):
status: New → Triaged
Changed in nova (Ubuntu):
status: New → Triaged
importance: Undecided → Medium
Changed in nova (Ubuntu Bionic):
importance: Undecided → Medium
Changed in nova (Ubuntu Cosmic):
importance: Undecided → Medium
Changed in nova (Ubuntu Disco):
importance: Undecided → Medium
Changed in cloud-archive:
assignee: nobody → Sahid Orentino (sahid-ferdjaoui)
Changed in nova (Ubuntu):
assignee: nobody → Sahid Orentino (sahid-ferdjaoui)
Changed in nova (Ubuntu Bionic):
assignee: nobody → Sahid Orentino (sahid-ferdjaoui)
Changed in nova (Ubuntu Disco):
assignee: nobody → Sahid Orentino (sahid-ferdjaoui)
Changed in nova (Ubuntu Cosmic):
assignee: nobody → Sahid Orentino (sahid-ferdjaoui)
description: updated

Hello Gustavo, or anyone else affected,

Accepted nova into disco-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/nova/2:19.0.0-0ubuntu2.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-disco to verification-done-disco. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-disco. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in nova (Ubuntu Disco):
status: Triaged → Fix Committed
tags: added: verification-needed verification-needed-disco
Łukasz Zemczak (sil2100) wrote :

Hello Gustavo, or anyone else affected,

Accepted nova into cosmic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/nova/2:18.1.0-0ubuntu3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-cosmic to verification-done-cosmic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-cosmic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in nova (Ubuntu Cosmic):
status: Triaged → Fix Committed
tags: added: verification-needed-cosmic
Corey Bryant (corey.bryant) wrote :

Hello Gustavo, or anyone else affected,

Accepted nova into stein-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:stein-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-stein-needed to verification-stein-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-stein-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-stein-needed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nova - 2:19.0.0-0ubuntu5

---------------
nova (2:19.0.0-0ubuntu5) eoan; urgency=medium

  * d/p/bug_1825882.patch: Cherry-picked from upstream to ensure
    virsh disk attach does not fail silently (LP: #1825882).
  * d/p/bug_1826523.patch: Cherry-picked from upstream to ensure
    always disconnect volumes after libvirt exceptions (LP: #1826523).

 -- Sahid Orentino Ferdjaoui <email address hidden> Thu, 16 May 2019 10:41:27 +0200

Changed in nova (Ubuntu):
status: Triaged → Fix Released
Corey Bryant (corey.bryant) wrote :

Hello Gustavo, or anyone else affected,

Accepted nova into rocky-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:rocky-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-rocky-needed to verification-rocky-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-rocky-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-rocky-needed
Łukasz Zemczak (sil2100) wrote :

Hello Gustavo, or anyone else affected,

Accepted nova into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/nova/2:17.0.9-0ubuntu3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in nova (Ubuntu Bionic):
status: Triaged → Fix Committed
tags: added: verification-needed-bionic
Corey Bryant (corey.bryant) wrote :

Hello Gustavo, or anyone else affected,

Accepted nova into queens-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:queens-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-queens-needed to verification-queens-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-queens-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-queens-needed
tags: added: sts-sru-needed
tags: added: sts
Jorge Niedbalski (niedbalski) wrote :

I have verified the following versions:

* bionic: 17.0.9-0ubuntu3
* cosmic: 18.1.0-0ubuntu3
* disco: 19.0.0-0ubuntu2.3

The following exception is raised when the traffic to the ceph-mon is dropped by
iptables.

 libvirt.libvirtError: Timed out during operation: cannot acquire state change lock (held by remoteDispatchDomainAttachDeviceFlags)

No attachment has been found at the vms after this operation fails, no volume added to
volumes_attached property.

tags: added: verification-done-bionic verification-done-cosmic verification-done-disco
removed: sts sts-sru-needed verification-needed-bionic verification-needed-cosmic verification-needed-disco
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nova - 2:17.0.9-0ubuntu3

---------------
nova (2:17.0.9-0ubuntu3) bionic; urgency=medium

  * d/p/bug_1825882.patch: Cherry-picked from upstream to ensure
    virsh disk attach does not fail silently (LP: #1825882).
  * d/p/bug_1826523.patch: Cherry-picked from upstream to ensure
    always disconnect volumes after libvirt exceptions (LP: #1826523).

nova (2:17.0.9-0ubuntu2) bionic; urgency=medium

  * d/p/xenapi-agent-change-openssl-error-handling.patch: Cherry-picked from
    upstream to ensure xenapi agent only raises a RuntimeError exception
    when openssl returns a non-zero exit code (LP: #1771506).
  * d/p/skip-double-word-hacking-test.patch: Cherry-picked from upstream
    to work-around test_hacking failures with new versions of python
    (LP: #1782786).

 -- Sahid Orentino Ferdjaoui <email address hidden> Thu, 16 May 2019 11:06:11 +0200

Changed in nova (Ubuntu Bionic):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nova - 2:18.1.0-0ubuntu3

---------------
nova (2:18.1.0-0ubuntu3) cosmic; urgency=medium

  * d/p/bug_1825882.patch: Cherry-picked from upstream to ensure
    virsh disk attach does not fail silently (LP: #1825882).
  * d/p/bug_1826523.patch: Cherry-picked from upstream to ensure
    always disconnect volumes after libvirt exceptions (LP: #1826523).

nova (2:18.1.0-0ubuntu2) cosmic; urgency=medium

  * d/p/xenapi-agent-change-openssl-error-handling.patch: Cherry-picked from
    upstream to ensure xenapi agent only raises a RuntimeError exception
    when openssl returns a non-zero exit code (LP: #1771506).

 -- Sahid Orentino Ferdjaoui <email address hidden> Thu, 16 May 2019 10:58:45 +0200

Changed in nova (Ubuntu Cosmic):
status: Fix Committed → Fix Released

This issue was fixed in the openstack/nova 19.0.1 release.

Jorge Niedbalski (niedbalski) wrote :

OK, verified S/Q/R as commented on https://bugs.launchpad.net/bugs/1826523

The following exception is raised when the traffic to the ceph-mon is dropped by
iptables.

** libvirt.libvirtError: Timed out during operation: cannot acquire state change lock (held by remoteDispatchDomainAttachDeviceFlags)
** No attachment has been found at the vms after this operation fails, no volume added to
volumes_attached property. (https://pastebin.canonical.com/p/kkmRcfrsjT/)

tags: added: verification-done verification-queens-done verification-rocky-done verification-stein-done
removed: verification-needed verification-queens-needed verification-rocky-needed verification-stein-needed

This issue was fixed in the openstack/nova 18.2.1 release.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers