Change in vdsm[master]: lib: grub2 module
fromani at redhat.com
fromani at redhat.com
Mon Nov 2 10:59:02 UTC 2015
Francesco Romani has posted comments on this change.
Change subject: lib: grub2 module
......................................................................
Patch Set 1: Code-Review-1
(3 comments)
-1 for visibility
I started reviewing, and gave initial comments, than realized that here we are partially replicating a facility that should be system wide.
So, +1 for the feature, but I feel that we need to draw a line, construct the parameters we want (or need), but then the actual job of handling the grub2 config should _enterely_ shed on some other system wide tool (grub2-mkconfig is not enough, too low level), on which we should depend on.
https://gerrit.ovirt.org/#/c/47946/1/lib/vdsm/grub2.py
File lib/vdsm/grub2.py:
Line 22: import collections
Line 23:
Line 24: from . import utils
Line 25:
Line 26: _GRUB_CONFIG = os.path.abspath('/etc/default/grub')
but this isn't already an abs path? :)
Line 27:
Line 28: _CFG_NAME_PCI_STUB = 'VDSM_PCI_STUB_DEVICES'
Line 29: _CFG_NAME_WORKAROUNDS = 'VDSM_WORKAROUNDS'
Line 30: _CFG_NAME_CMDLINE = 'GRUB_CMDLINE_LINUX'
Line 29: _CFG_NAME_WORKAROUNDS = 'VDSM_WORKAROUNDS'
Line 30: _CFG_NAME_CMDLINE = 'GRUB_CMDLINE_LINUX'
Line 31: _CMDLINE_STUB_IDS = 'pci-stub.ids='
Line 32:
Line 33: _GRUB2_MKCONFIG = utils.CommandPath('/usr/sbin/grub2-mkconfig')
maybe just grub2-mkconfig and let CommandPath figure out the path?
Line 34:
Line 35:
Line 36: class Error(Exception):
Line 37:
Line 32:
Line 33: _GRUB2_MKCONFIG = utils.CommandPath('/usr/sbin/grub2-mkconfig')
Line 34:
Line 35:
Line 36: class Error(Exception):
For another patch, not necessarily from yours: let's factor this Error copied everywhere once for all.
Line 37:
Line 38: def __init__(self, rc, out, err):
Line 39: self.rc = rc
Line 40: self.out = out
--
To view, visit https://gerrit.ovirt.org/47946
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e7e0b327882f64196a71fe20dcbd99017b80800
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list