Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Virt disks fixes #57350

Merged
merged 6 commits into from
May 18, 2020
Merged

Virt disks fixes #57350

merged 6 commits into from
May 18, 2020

Conversation

cbosdo
Copy link
Contributor

@cbosdo cbosdo commented May 18, 2020

What does this PR do?

Fixes miscellaneous problems related to the recent PR adding disk volume support in virt module and state.

  • Show proper URL for remote CDROMs in virt.get_disks()
  • Show libvirt pool name for rbd and gluster volumes in virt.get_disks()
  • Remove libvirt secret when removing rbd storage pool

What issues does this PR fix or reference?

Fixes: #56666

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

  • Docs
  • Changelog - Actually fixing another PR that hasn't been released yet
  • Tests written/updated

Commits signed with GPG?

Yes

From the user point of view the internal RBD pool name doesn't make
sense when listing the disks of a VM. We need then to resolve it to the
libvirt pool name if possible.

Move the corresponding code from purge to get_disks.
libvirt adds the index attribute in the XML definition when there is a
backing store file for a disk. We need to elude it before comparing the
sources to avoid trying to recreate disks in such cases.
cdrom devices can't use virtio. Set the default bus to ide for cdroms.
@cbosdo cbosdo requested a review from a team as a code owner May 18, 2020 10:07
@ghost ghost requested review from twangboy and removed request for a team May 18, 2020 10:07
virt._gen_xml converts the cdroms with remote URL into a network device,
but to be coherent with the user input the virt.get_disk() function
needs to reassemble the URL in thoses cases.
Some pool type require a secret for the authentication on the remote
source. Remove the secrets that were added by pool_defined but leave the
others in place.
@cbosdo
Copy link
Contributor Author

cbosdo commented May 18, 2020

@twangboy the ubuntu1604 failure seems completely unrelated to the PR, could you restart it?

Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small question, otherwise 👍

salt/modules/virt.py Show resolved Hide resolved
@waynew waynew added the ZRelease-Sodium retired label label May 18, 2020
@cbosdo
Copy link
Contributor Author

cbosdo commented May 18, 2020

yeah! all green 💚

@sagetherage sagetherage added this to In progress in Sodium via automation May 18, 2020
@dwoz dwoz merged commit 303824b into saltstack:master May 18, 2020
Sodium automation moved this from In progress to Done May 18, 2020
@cbosdo cbosdo deleted the virt-disks-fixes branch May 19, 2020 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ZRelease-Sodium retired label
Projects
No open projects
Sodium
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants