virt.vm_info and other functions in virt module

[waheedi]

What does this PR do?

• It fixes few bugs with the current module and sticks to the standard way of implementing virt module functions, there were inconsistencies in the implementation of few functions like get_on_poweroff, get_on_crash, get_on_reboot, get_uuid and get_profiles.

• removed function get_xml as it does not belong here

What issues does this PR fix or reference?

#52431

Previous Behavior

salt '*' virt.vm_info
The VM "<libvirt.virDomain object at 0x7f24e9e73198>" is not present

New Behavior

salt '*' virt.vm_info

minion:
    ----------
    minion-1:
        ----------
        cpu:
            1
        cputime:
            0
        disks:
            ----------
            vda:
                ----------
                cluster size:
                    65536
                disk size:
                    3065716736
                file:
                    /var/lib/libvirt/images/ubuntu16.04.qcow2
                file format:
                    qcow2
                type:
                    disk
                virtual size:
                    10737418240
        graphics:
            ----------
            autoport:
                yes
            keymap:
                None
            listen:
                None
            port:
                None
            type:
                spice
        loader:
            ----------
            path:
                None
        maxMem:
            819200
        mem:
            819200
        nics:
            ----------
            52:54:00:70:b7:63:
                ----------
                address:
                    ----------
                    bus:
                        0x00
                    domain:
                        0x0000
                    function:
                        0x0
                    slot:
                        0x03
                    type:
                        pci
                mac:
                    52:54:00:70:b7:63
                model:
                    virtio
                source:
                    ----------
                    network:
                        default
                type:
                    network
        on_crash:
            destroy
        on_poweroff:
            destroy
        on_reboot:
            restart
        state:
            shutdown
        uuid:
            e6e3f990-8997-4a5e-8cb7-ea835eae4bbe

Tests written?

No (but tested)

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

[gtmanfred]

You can't just delete function, salt maintains backwards compatiblity for at least 2 major releases, and this needs to include deprecation warnings for anything that is being deleted.

https://docs.saltstack.com/en/latest/topics/development/deprecations.html

[waheedi]

@gtmanfred I re-added get_xml function, thanks for taking a look.

[gtmanfred]

Why are all these having to be redefined?

What changed? This has worked in the past. Do you have a reason to not use the get_xml() function that already exists, and looks like it is doing the same thing?

I want to make sure we are not breaking other peoples environments.

[waheedi]

Thanks @gtmanfred, I'm pretty sure this will not break other peoples environments, few reasons:

• the previous version of the code used to reference dom as string a "name" while it should be an actual domain object.
• all the other _get functions are expecting a domain object not a string and many functions are raising exceptions because of treating an object as a string, then failing with ERROR: The VM "<libvirt.virDomain object at 0x7f24e9e73198>" is not present

take a look at this one here for example:

    def _info(dom):
        '''
        Compute the infos of a domain
        '''
        raw = dom.info()
        return {'cpu': raw[3],
                'cputime': int(raw[4]),
                'disks': _get_disks(dom),
                'graphics': _get_graphics(dom),
                'nics': _get_nics(dom),
                'uuid': _get_uuid(dom),
                'loader': _get_loader(dom),
                'on_crash': _get_on_crash(dom),
                'on_reboot': _get_on_reboot(dom),
                'on_poweroff': _get_on_poweroff(dom),
                'maxMem': int(raw[1]),
                'mem': int(raw[2]),
                'state': VIRT_STATE_NAME_MAP.get(raw[0], 'unknown')}

dom is a domain object and not a domain name string.