Martin Polednik has posted comments on this change. Change subject: containers: always add emulator to XML ......................................................................
Patch Set 14: (3 comments) Some minor comments, nothing -1 worthy except maybe that I don't like guessing host supported emulators. Postponing rating since I need to see more of the series. https://gerrit.ovirt.org/#/c/54176/14//COMMIT_MSG Commit Message: PS14, Line 9: to for https://gerrit.ovirt.org/#/c/54176/14/vdsm/virt/vmxml.py File vdsm/virt/vmxml.py: PS14, Line 539: and not is_kvm(self.conf) This may have future implication if we decide to ever support (at least upstream) emulation. What we should really do is report emulators to the engine and let it decide which one to use (/usr/bin/qemu-system-*, /usr/libexec/qemu-kvm, runc(?), rkt(?), ...). As that is (my)RFE itself, leaving that our of rating. PS14, Line 541: else: : # else no need for the <emulator> element : emulator_value = None Stylistic, but I'd rather initialize emulator_value = None *before* the branching and leave out the else clause. Thoughts? (the idea is to emphasize that we don't care about emulator except for some platforms) Minor: s/value/path as contents of <emulator> is absolute path to the binary. -- To view, visit https://gerrit.ovirt.org/54176 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie40c6a3e2cdbb72053db71056092d3b7684b994d Gerrit-PatchSet: 14 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/mailman/listinfo/vdsm-patches