[node-patches] Change in ovirt-node[master]: find_grub_cfg(): Add elif statement to get correct cfg_path ...

[fabiand at fedoraproject.org]

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 <hadong0720 at gmail.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes