Hi Oksana, On Fri, Mar 20, 2020 at 12:15 PM Oksana Vohchana <ovosh...@redhat.com> wrote: > > Provides new functions related to the rdma migration test > Adds functions to check if service RDMA is enabled and gets > the ip address on the interface where it was configured > > Signed-off-by: Oksana Vohchana <ovosh...@redhat.com> > --- > tests/acceptance/migration.py | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py > index e4c39b85a1..a783f3915b 100644 > --- a/tests/acceptance/migration.py > +++ b/tests/acceptance/migration.py > @@ -11,12 +11,17 @@ > > > import tempfile > +import json > from avocado_qemu import Test > from avocado import skipUnless > > from avocado.utils import network > from avocado.utils import wait > from avocado.utils.path import find_command > +from avocado.utils.network.interfaces import NetworkInterface > +from avocado.utils.network.hosts import LocalHost > +from avocado.utils import service > +from avocado.utils import process > > > class Migration(Test): > @@ -58,6 +63,31 @@ class Migration(Test): > self.cancel('Failed to find a free port') > return port > > + def _if_rdma_enable(self): > + rdma_stat = service.ServiceManager() > + rdma = rdma_stat.status('rdma') > + return rdma
You can just `return rdma_stat.status('rdma')` here! Also, as you are not using any of the class attributes or methods, if you make this method static, you don't need to call it with `None` as you did on patch 3 of this series. > + > + def _get_interface_rdma(self): > + cmd = 'rdma link show -j' > + out = json.loads(process.getoutput(cmd)) > + try: > + for i in out: > + if i['state'] == 'ACTIVE': > + return i['netdev'] > + except KeyError: > + return None Same comment about making this method static. Actually, if you are not using any of the attributes or methods from the Migration class on these two methods, I think you can define them as functions, outside of the Class. Does it make sense? > + > + def _get_ip_rdma(self, interface): > + local = LocalHost() > + network_in = NetworkInterface(interface, local) > + try: > + ip = network_in._get_interface_details() > + if ip: > + return ip[0]['addr_info'][0]['local'] > + except: > + self.cancel("Incorrect interface configuration or device name") > + If you change the logic a bit and raise an exception here, instead of doing a `self.cancel`, you can also make this method static, or move it outside of the class. The cancel can be handled in the test, with the exception raised here. > > def test_migration_with_tcp_localhost(self): > dest_uri = 'tcp:localhost:%u' % self._get_free_port() > -- > 2.21.1 > > Let me know if the comments do not make sense. Willian