Fabian Deutsch has posted comments on this change. Change subject: find_grub_cfg(): Add elif statement to get correct cfg_path for efi_boot ......................................................................
Patch Set 1: Code-Review-1 (1 comment) http://gerrit.ovirt.org/#/c/26418/1/src/ovirt/node/utils/system.py File src/ovirt/node/utils/system.py: Line 742: cfg_path = None Line 743: import ovirtnode.ovirtfunctions as _functions Line 744: if Filesystem.by_label("Boot"): Line 745: cfg_path = "/boot/grub/grub.conf" Line 746: elif _functions.is_efi_boot(): I see the need for this. But this is getting messy again. First - please don't import ovirtfunctions anymore. system.is_efi already exists. Now we only need a clean implementation for mount_efi. And for this specific problem. IIUIC we've got two variables (path and filename) which depend on three or four other (el6, fedora, efi, iscsi) variables. Couldn't we rework the logic to determin the path and the cfg filename separately and join both independently determined values? We can even write doctests for this. Line 747: _functions.mount_efi(target="/liveos") Line 748: if os.path.exists("/liveos/EFI/redhat"): Line 749: cfg_path = "/liveos/EFI/redhat/grub.conf" Line 750: else: -- To view, visit http://gerrit.ovirt.org/26418 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I299fb4906d8d2e947130daea1de570a9347bb202 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: hadong <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
