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 init disks #56666

Merged
merged 25 commits into from
May 7, 2020
Merged

Virt init disks #56666

merged 25 commits into from
May 7, 2020

Conversation

cbosdo
Copy link
Contributor

@cbosdo cbosdo commented Apr 16, 2020

What does this PR do?

Enables volume support in virt module and states.

What issues does this PR fix or reference?

Fixes: #57005

Previous Behavior

virt.running, virt.purge, etc could only handle file disk images.

New Behavior

virt.running, virt.purge, etc can also handle volume disk images. This allows to use RBD, iSCSI, etc volumes in VMs.

Merge requirements satisfied?

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

  • Docs
  • Changelog
  • Tests written/updated

Commits signed with GPG?

Yes

@cbosdo cbosdo requested a review from a team as a code owner April 16, 2020 15:45
@cbosdo cbosdo changed the title Virt init disks [WIP] Virt init disks Apr 16, 2020
@cbosdo cbosdo marked this pull request as draft April 16, 2020 15:47
@Ch3LL Ch3LL requested review from DmitryKuzmenko and removed request for a team April 16, 2020 15:49
@cbosdo
Copy link
Contributor Author

cbosdo commented Apr 16, 2020

@DmitryKuzmenko don't spend too much time reviewing that PR for the while: the code is still subject to changes

@cbosdo
Copy link
Contributor Author

cbosdo commented Apr 20, 2020

@dwoz Don't loose time to blacken this PR for the while: code it still subject to changes: I'll handle the blackening before removing the WIP flag.

@cbosdo cbosdo force-pushed the virt-init-disks branch 9 times, most recently from 364b63a to 4e3f391 Compare April 24, 2020 07:01
@cbosdo cbosdo force-pushed the virt-init-disks branch 3 times, most recently from f429f99 to 2ed480d Compare April 30, 2020 11:44
@cbosdo cbosdo marked this pull request as ready for review April 30, 2020 11:45
@cbosdo cbosdo changed the title [WIP] Virt init disks Virt init disks Apr 30, 2020
@cbosdo cbosdo force-pushed the virt-init-disks branch 2 times, most recently from 6f01327 to 55097b7 Compare April 30, 2020 13:34
@cbosdo
Copy link
Contributor Author

cbosdo commented Apr 30, 2020

@DmitryKuzmenko This PR is ready for review now

@twangboy
Copy link
Contributor

@cbosdo Looks like we got some test failures on Windows

@cbosdo cbosdo force-pushed the virt-init-disks branch from 55097b7 to 6083e7d Compare May 4, 2020 14:59
@cbosdo
Copy link
Contributor Author

cbosdo commented May 4, 2020

@twangboy should be fixed with the latest push... sadly urlparse is considering the windows path as valid and parses the drive letter as the URL scheme.

@cbosdo
Copy link
Contributor Author

cbosdo commented May 5, 2020

All green now :)

DmitryKuzmenko
DmitryKuzmenko previously approved these changes May 6, 2020
cbosdo added 22 commits May 7, 2020 09:16
In the near future gen_vol_xml will be able to handle many volume types,
not only for ESX volumes. For this, clean up the function from all the
ESX-specifics code and move them to the caller code.

The volume key and target path values have also been removed since those
are read-only elements that should not be provided for volume creation
as per https://libvirt.org/formatstorage.html#StorageVol
In order to generate almost all the volumes that libvirt can handle, add
the type, backingStore, permissions and allocation parameters to the
virt._gen_vol_xml() function.

Also make the format parameter optional since libvirt has default values
depending on the storage backend.
In order to avoid connection multiple times when reusing this function
in the virt module, create _define_vol_xml_str not caring about the
connection opening and closing.
In the same vein than pool_define and network_define, expose a
volume_define function to let users create a volume without needing to
know all of libvirt's XML details.
When using volumes the user can just copy the template disk image into
the target path. In such a case, the image needs to be uploaded into the
volume.
Extract the libvirt-handling code from virt.capabilities into a
virt._capabilities function accepting an opened libvirt connection.
This allows reusing the code in other functions with easy connection
handling.
Te virt.pool_capabilities function computes a lot of interesting values
on the virtual storage pool types. Extract the logic of it into
virt._pool_capabilities for reuse.
Since the next commits will introduce more uses of the libvirt
connection in virt.init(), start sharing it now.
In order to support creating VMs with disk on more storage pools like
iSCSI, disks, LVM, RBD, etc, virt.init needs to handle disks not only as
files, but also as libvirt volumes.
libvirtError is not defined, libvirt.libvirtError should be used
instead.
There is no need to generate MAC addresses in the virt module if the
user hasn't provided any. This only makes it harder to make the
difference between a real mac address change from the user and a new
generated one.

Now the mac address is not written in the domain XML definition if not
provided by the user. This avoids unnecessary changes when applying
virt.running.
Since it could be useful to know whether a volume has a backing store,
output the path and format of the backing store if any is defined.
Since the format of a volume may be of interest and could help to tell
if two volumes are similar, output this information in the
virt.volume_infos function.
In order to help creating storage volumes in virtual storage pools from
states, add a virt.volume_defined state.
If a virtual machine has disks of volume types, they will now be
reported by the virt._get_disk() function and all the user-exposed
functions using it like virt.get_disks(), virt.vm_info() and
virt.full_info().
virt.purge will now remove not only the file disks, but also the disk volumes.
libvirt doesn't support attaching RBD storage pool volumes to a VM.
For instance, a disk with such a source is invalid:

    <source pool='rbd-pool' volume='rbd-volume'/>

And needs to be replaced with:

    <source protocol="rbd" name="rbd-pool-name/rbd-volume">
      <host name="hostname" port="7000"/>
      <auth username='myuser'>
        <secret type='ceph' usage='mypassid'/>
      </auth>
    </source>

This means that we need to fetch the connection data from the pool
definition and convert the volume disk definition into a network one
when creating the VM.

This also means that when purging the VM we need to search for the
pool by looking in every pool's XML to delete the volume.
Since volumes in a virtual storage pool of type 'disk' are partitions,
then need to be named after the disk name with sequential numbers rather
than using the VM and disk names.

Also, the format passed by the user is the one used when creating the volume.
However in the VM definition for logical and disk volumes, the format should
be set to raw.
Libvirt allows to use network images for cdroms. Use them if the image
is a remote URL for a cdrom device.
Live attaching / detaching removable devices results in failures.
To change the source of those, we need to call updateDeviceFlags instead.
@cbosdo cbosdo force-pushed the virt-init-disks branch from f87a5d3 to 9a70d2a Compare May 7, 2020 07:16
@dwoz dwoz merged commit 7d18001 into saltstack:master May 7, 2020
@cbosdo cbosdo deleted the virt-init-disks branch May 15, 2020 07:50
@cbosdo cbosdo mentioned this pull request May 18, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ZRelease-Sodium retired label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create VMs with disks on volume disks
4 participants