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

virttools: add new test type and virt-install test #3923

Merged
merged 1 commit into from
Jan 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions provider/vfio/ccw.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def read_write_operations_work(session, chpids, makefs=True):
Per default the device gets a new filesystem setup.
:param session: logged in guest session
:param chipds: string representing CHPIDs, e.g. 11122122
:param chpids: string representing CHPIDs, e.g. 11122122
:param makefs: if False, the device is expected to have a valid
filesystem already
:return: True on success
Expand Down Expand Up @@ -146,6 +146,21 @@ def mount(session):
raise TestError("Couldn't mount partition. %s" % out)


def set_device_offline(device_id, session=None):
"""
Sets device offline
:param device_id: cssid.ssid.devno, e.g. 0.0.560a
:param session: guest session, command is run on host if None
:raises TestError: if the device can't be set offline
"""

cmd = "chccwdev -d %s" % device_id
err, out = cmd_status_output(cmd, shell=True, session=session)
if err:
raise TestError("Could not set device offline. %s" % out)


def set_device_online(device_id, session=None):
"""
Sets device online
Expand All @@ -165,7 +180,7 @@ def get_first_device_identifiers(chpids, session):
"""
Gets the usual device identifier cssid.ssid.devno
:param chpids: chipids where the disk is connected, e.g. "11122122"
:param chpids: chpids where the disk is connected, e.g. "11122122"
:param session: guest session
:return: Pair of strings, "cssid.ssid.devno" "cssid.ssid.schid"
:raises TestError: if the device can't be found inside guest
Expand All @@ -176,7 +191,7 @@ def get_first_device_identifiers(chpids, session):
devices_inside_guest = [x for x in paths.devices
if x[paths.HEADER["CHPIDs"]] == chpids]
if not devices_inside_guest:
raise TestError("Device with chipds %s wasn't"
raise TestError("Device with chpids %s wasn't"
" found inside guest" % chpids)
first = devices_inside_guest[0]
return first[paths.HEADER["Device"]], first[paths.HEADER["Subchan."]]
Expand All @@ -188,7 +203,7 @@ def device_is_listed(session, chpids):
path ids.
:param session: guest console session
:param chipds: chpids where the disk is connected, e.g. "11122122"
:param chpids: chpids where the disk is connected, e.g. "11122122"
:return: True if device is listed
"""

Expand Down
9 changes: 9 additions & 0 deletions spell.ignore
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ chipset
chmod
chnaged
chown
chpid
chpids
chronyc
Chunfu
chwen
Expand Down Expand Up @@ -165,6 +167,8 @@ dac
DAC
darget
dargs
dasd
dasda
datetime
DAX
dbus
Expand Down Expand Up @@ -270,6 +274,7 @@ failover
fallocate
fc
fd
fdasd
fdisk
fds
fdset
Expand Down Expand Up @@ -528,6 +533,7 @@ LXC
lzop
macvlan
macvtap
makefs
Makesure
managedsave
Managedsave
Expand All @@ -549,6 +555,7 @@ mcast
MCS
md
mdev
mdevctl
mem
memballoon
memhog
Expand Down Expand Up @@ -833,6 +840,7 @@ sasl
sata
scenaries
schedinfo
schid
scp
scsi
sd
Expand Down Expand Up @@ -1032,6 +1040,7 @@ unprotect
Unprotect
Unregister
unresettable
Unsets
untar
upadte
Updae
Expand Down
11 changes: 11 additions & 0 deletions virttools/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
The test type 'virttools' is meant to cover test cases for the tools in the
virt-manager repository, e.g. virt-xml, virt-clone, virt-install.

By using avocado-vt we obtain access to many existing test functions for libvirt
and qemu. Please, note the related commit in avocado-vt to allow for the new
test type.

'virttools' uses the same basic setup as the tp-libvirt/libvirt test type assuming
most tests will suppose there's at least one existing vm 'avocado-vt-vm1' with
image in the default location (avocado-vt/.../images/jeos-27-s390x.qcow2), e.g.
for 'virt-xml avocado-vt-vm1 --add-device...', 'virt-clone avocado-vt-vm1'.
6 changes: 6 additions & 0 deletions virttools/tests/cfg/virt_install/hostdev_mdev.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
- virt_install.hostdev.mdev:
type = hostdev_mdev
variants:
- check_present_inside_guest:
only s390-virtio
mdev_type = vfio_ccw-io
200 changes: 200 additions & 0 deletions virttools/tests/src/virt_install/hostdev_mdev.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
import logging
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add 'LOG = log.getLogger('avocado.' + name)' and replace logging.xxx with LOG.xxx in the code. The current way does not log messages in the latest avocado. For details, please check #3833

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)


from time import sleep
from uuid import uuid4
from avocado.core.exceptions import TestError
from avocado.core.exceptions import TestFail
from provider.vfio import ccw
from virttest.libvirt_xml.vm_xml import VMXML
from virttest.utils_misc import cmd_status_output
from virttest import virsh

LOG = logging.getLogger('avocado.' + __name__)


class MdevHandler(object):
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious that in terms of MdevHandler, would you like maintain it as local test module in hostdev_mdev.py , or it could be more generic module shared across test modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chunfuwen I think your question is covered in discussion above #3923 (comment)

""" Base class for mdev type specific implementations """

def create_nodedev(self):
""" Creates the mdev and returns its name """
raise NotImplementedError()

def get_target_address(self):
""" Returns a target address to use for hostdev """
raise NotImplementedError()

def check_device_present_inside_guest(self, session):
"""
Checks if the host device is present inside the guest
:param session: guest session
"""
raise NotImplementedError()

def clean_up(self):
""" Stops the mediated device and returns resources to the host """
raise NotImplementedError()

@staticmethod
def from_type(mdev_type):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the function name is not very meaningful and explicit. Could we use one similar like 'get_instance_from_type()'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dzhengfy I would like to maintain it, IMO, it's a common python idiom, please check https://github.com/avocado-framework/avocado-vt/pull/2610/files#r445400984

"""
Creates implementing instance for mdev_type
:param mdev_type: The mediated device type as by nodedev API
"""
if mdev_type == "vfio_ccw-io":
return CcwMdevHandler()
else:
raise TestError("Test doesn't know how to handle %s." % mdev_type)


class CcwMdevHandler(MdevHandler):
Copy link
Contributor

Choose a reason for hiding this comment

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

How about moving CcwMdevHandler and MdevHandler to provider? I wonder if they can be used by other tests...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Yingshun At this point the base class doesn't do anything but only serves as interface to be implemented. Especially the get_target_address method at this point to me looks rather test specific. I'm not sure if somebody found those classes below the provider path they would know what to do with them without the context of the test implementation. Therefore, I'd like to wait for other tests to need these classes before moving them. Is that okay?

""" Class implementing test methods for vfio_ccw-io """

def __init__(self):
self.uuid = None
self.chpids = None
self.schid = None
self.target_address = None
self.expected_device_address = None
self.device_id = None
self.session = None

def create_nodedev(self):
"""
Creates a mediated device of a specific type
and returns its name from libvirt.
:return: name of mdev device as node device
"""
self.schid, self.chpids = ccw.get_device_info()
self.device_id, _ = ccw.get_first_device_identifiers(self.chpids, None)
ccw.set_override(self.schid)
self.uuid = str(uuid4())
ccw.start_device(self.uuid, self.schid)

return get_first_mdev_nodedev_name()

def get_target_address(self):
"""
Returns a valid target device address
:return: hostdev target address
"""
self.target_address = "address.type=ccw,address.cssid=0xfe,address.ssid=0x0,address.devno=0x1111"
self.expected_device_address = "0.0.1111"
return self.target_address

def check_device_present_inside_guest(self, session):
"""
Fails the test if the device can't be found inside the guest.
:param session: guest session
:raises: TestFail if device not found
"""
self.session = session
device, _ = ccw.get_first_device_identifiers(self.chpids, session)
if device != self.expected_device_address:
raise TestFail("Couldn't find device inside guest."
"Expected address %s, found %s." %
(self.expected_device_address, device))
LOG.debug("Device was found inside guest with"
" expected id %s." % device)

def clean_up(self):
"""
Returns the mdev resources to the host.
"""
if self.session:
self.session.close()
if self.uuid:
ccw.stop_device(self.uuid)
if self.schid:
ccw.unset_override(self.schid)
# need to sleep to avoid issue with setting device offline
# adding a wait_for would likely be more complicated
sleep(1)
if self.device_id:
ccw.set_device_offline(self.device_id)


def get_disk_for_import(vmxml):
"""
Returns the absolute path to a disk image for import.
Assume the boot image is the first disk and an image file.
:param vmxml: VMXML instance
:return: absolute path to the guest's first disk image file
"""
disks = vmxml.get_disk_all()
disk_list = list(disks.values())
first_disk = disk_list[0]
return first_disk.find('source').get('file')


def get_first_mdev_nodedev_name():
"""
Returns the first nodedev of type mdev known to libvirt
:return: the first listed mdev node device
"""
result = virsh.nodedev_list(cap="mdev", debug=True)
device_names = result.stdout.strip().splitlines()
if result.exit_status or len(device_names) == 0:
raise TestError("Couldn't create nodedev. %s. %s." %
(result.stderr, result.stdout))
return device_names[0]


def virt_install_with_hostdev(vm_name, mdev_nodedev, target_address, disk_path):
"""
Runs virt-install with hostdev
:param vm_name: guest name
:param mdev_nodedev: mdev name as node device
:param target_address: hostdev target address definition
:param disk_path: path to the disk image for import
"""
cmd = ("virt-install --import --name %s"
" --hostdev %s,%s"
" --disk %s"
" --vcpus 2 --memory 2048"
" --nographics --noautoconsole" %
(vm_name, mdev_nodedev, target_address, disk_path))
err, out = cmd_status_output(cmd, shell=True, verbose=True)
if err:
raise TestError("Couldn't install vm with hostdev: %s" % out)


def run(test, params, env):
"""
Confirm that a mediated device can be used by virt-install.
For this we import a disk we know will boot and check the
result inside the guest.
The mediated device is created by the test and assumed
to be the only mediated device in the test environment.
"""

vm_name = params.get("main_vm")
vm = env.get_vm(vm_name)
vmxml = VMXML.new_from_inactive_dumpxml(vm_name)
mdev_type = params.get("mdev_type", "vfio_ccw-io")
handler = None

try:

vm.undefine()
handler = MdevHandler.from_type(mdev_type)
disk = get_disk_for_import(vmxml)
mdev_nodedev = handler.create_nodedev()
target_address = handler.get_target_address()

virt_install_with_hostdev(vm_name, mdev_nodedev, target_address, disk)

session = vm.wait_for_login()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to start the vm before trying to loin?

Copy link
Contributor Author

@smitterl smitterl Jan 4, 2022

Choose a reason for hiding this comment

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

Do you need to start the vm before trying to loin?

No, the virt-install command boots the guest.

handler.check_device_present_inside_guest(session)

Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to close the session.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The session is closed now in the final clause handler.clean_up.

finally:
vmxml.sync()
if handler:
handler.clean_up()