Change in vdsm[master]: containers: always add emulator to XML

mpolednik at redhat.com mpolednik at redhat.com
Mon Apr 4 11:12:35 UTC 2016


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 <fromani at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list