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() -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <mskri...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
