Change in vdsm[master]: virt: Set timeout on boot menu

mpolednik at redhat.com mpolednik at redhat.com
Tue Apr 26 15:00:00 UTC 2016


Martin Polednik has posted comments on this change.

Change subject: virt: Set timeout on boot menu
......................................................................


Patch Set 3: Code-Review+1

(2 comments)

Some disagreement not worth blocking for.

https://gerrit.ovirt.org/#/c/56393/3/tests/vmTests.py
File tests/vmTests.py:

PS3, Line 197: testOSXMLBootMenuTimeout
(totally minor and not very relevant nit) - I was trying to figure out what OSX ML means...


https://gerrit.ovirt.org/#/c/56393/3/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

Line 302:             timeout = config.getint('vars', 'boot_menu_timeout') * 1000
Line 303:             if timeout < 0:
Line 304:                 timeout = 0
Line 305:             elif timeout > 65535:
Line 306:                 timeout = 65535
> You can raise here vdsm.exception.InvalidConfiguration
We should let libvirt fail/deal with invalid value OR do as Nir said, I don't like the idea of hiding configuration change.
Line 307:             oselem.appendChildWithArgs('bootmenu', enable='yes',
Line 308:                                        timeout=unicode(timeout))
Line 309: 
Line 310:         if use_serial_console and cpuarch.is_x86(self.arch):


-- 
To view, visit https://gerrit.ovirt.org/56393
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f3501e8500e366e785f5a8ddfdf78fd34c997a2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Arik Hadas <ahadas at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken 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: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list