autotest / virt-test

Linux Virtualization Tests
Other
97 stars 140 forks source link

virttest.qemu_qtree: Update guess_type() for QtreeDev #2220

Closed spcui closed 9 years ago

spcui commented 9 years ago

Make the get method can get the correct QtreeDisk device on different platform.

ID: 1211151 1231011

CongLi commented 9 years ago

Hi @spcui

The request is that virtio-blk-pci info in qtree is different in RHEL.6 and RHEL.7, we need update the code to the root cause I think. I don't think that disabling the virtio-blk-pci can solve the problem.

Please be free to correct me if I'm misunderstanding.

Thanks.

spcui commented 9 years ago

Hi @CongSmile

For rhel7 host, self.qtree['type'] == 'virtio-blk-device', and return disk, for rhel6 host, self.qtree['type'] == 'virtio-blk-pci', and elif branch can return the correct type.

CongLi commented 9 years ago

Thx for the explanation at first.

For the elif branch, you don't judge "virtio-blk-pci", right? which means that no matter qtree['type'] equals to "virtio-blk-pci", it will return the QtreeDisk.

But actually, it qtree['type'] != "virtio-blk-pci", it should return QtreeDev, right?

Please be free to correct me if I'm misunderstanding.

Thanks.

CongLi commented 9 years ago

Have confirmed with @xutian , this PR needs update.

guess_type() for Qdisk should be in Qdisk.

xutian commented 9 years ago

@spcui

for guess_type in Qdev, it just get dev objects in qtree, for guess_type in qdisk, it should be overwrite in Qdisk not in Qdev. so Nack this PR.

Thanks, Xu

CongLi commented 9 years ago

Hi @spcui

CI check failed, could you fix it at first?

Thx.

vipmike007 commented 9 years ago

inspekt style /home/travis/build/autotest/virt-test/virttest/qemu_qtree.py:364:5: E125 20 PEP8 check fail: /home/travis/build/autotest/virt-test/virttest/qemu_qtree.py Trying to fix errors with autopep8 PEP8 compliance FAIL The command "inspekt style" exited with 1.

lmr commented 9 years ago

Hi @spcui. As you might know, we're moving virt-test to avocado-vt, so I please ask you to consider re-implementing this functionality in the later. The new repo is:

https://github.com/avocado-framework/avocado-vt

Please keep in mind the following guidelines when creating the new PR:

1) Most of the time you can pick the patches of your original PR and merge them on a new branch with minimal changes. 3) Don't use autotest APIs, since we're moving away from using them. Please review the code and change things like utils.system -> process.system, utils.run -> process.run, so on and so forth.

Thanks, and I'm sorry for any inconveniences we might have caused your team.