Martin Polednik has posted comments on this change. Change subject: virt: utils: add is_kvm helper ......................................................................
Patch Set 28: Code-Review-1 (4 comments) https://gerrit.ovirt.org/#/c/55647/28/lib/vdsm/virt/utils.py File lib/vdsm/virt/utils.py: PS28, Line 205: conf s/conf/vm_conf PS28, Line 210: vm_type = conf.get('vmType') Idea: is there an issue with 'vmType': 'container'? PS28, Line 215: eturn vm_type == 'kvm' If we consider above comment, the check could be relaxed to vm_type != 'container' and function renamed to is_qemu. I'm not sure if that is an improvement. https://gerrit.ovirt.org/#/c/55647/28/tests/vmUtilsTests.py File tests/vmUtilsTests.py: PS28, Line 224: def test_container_type(self): : self.assertFalse(utils.is_kvm({ : 'vmType': 'kvm', : 'custom': { : 'containerType': 'rkt', : }, : })) The usage of different key and it's implicit exclusivity with vmType makes me further unhappy. -- To view, visit https://gerrit.ovirt.org/55647 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f9e1b8f9326e565ee7324d0b328100ca86c6967 Gerrit-PatchSet: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
