Martin Polednik has posted comments on this change.
Change subject: lib: grub2 module ......................................................................
Patch Set 3:
(3 comments)
https://gerrit.ovirt.org/#/c/47946/3/lib/vdsm/grub2.py File lib/vdsm/grub2.py:
Line 45: return "Process failed with rc=%d out=%r err=%r" % ( Line 46: self.rc, self.out, self.err) Line 47: Line 48: Line 49: def mkconfig(args):
please add docstrings to functions. No need to document every parameter or
Added. No wiki yet, let me think about it. Line 50: cmd = [_GRUB2_MKCONFIG.cmd] Line 51: cmd.extend(args) Line 52: rc, out, err = utils.execCmd(_GRUB2_MKCONFIG, raw=True) Line 53: if rc != 0:
Line 160: Line 161: device_vendor = device_vendor.lstrip('0x') Line 162: device_product = device_product.lstrip('0x') Line 163: Line 164: if stub_ids:
does the ordering matter? can we use a set?
Possibly! Line 165: for vendor, product in stub_ids: Line 166: if device_vendor == vendor and device_product == product: Line 167: return Line 168:
https://gerrit.ovirt.org/#/c/47946/3/tests/grub2Tests.py File tests/grub2Tests.py:
Line 71: def testAddMultipleStubs(self): Line 72: with tempfile.NamedTemporaryFile() as f: Line 73: grub2.add_to_stub( Line 74: ['cafe', '0xdead', '0xf00d', 'b002'], Line 75: ['0xbeef', 'feed', '0xc00l', 'fa11'],
I missed this reviewing the code! so question: why is it helpful to support
Discussed with engine, that will be the approach for next revision. Line 76: cfg_source=self._test_file('grub2_default.out'), Line 77: cfg_dest=f.name) Line 78: Line 79: data = f.read()
vdsm-patches@lists.fedorahosted.org