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

[fabiand at redhat.com]

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 7: Code-Review-1

(2 comments)

I like the idea of the patch, to first find the root where we need to search and then find the correct config file.

But the patch is missing that sometimes a grub2 subdir is used.

Please enhance the patch to use something like a find method to find the correct configfile in a tree

http://gerrit.ovirt.org/#/c/26418/7/src/ovirt/node/utils/system.py
File src/ovirt/node/utils/system.py:

Line 829:     @staticmethod
Line 830:     def find_grub_cfg():
Line 831:         cfg = None
Line 832:         if Filesystem.by_label("Boot"):
Line 833:             cfg_path = "/boot/grub/"
This will also not catch grub2 subdirs
Line 834:         elif is_efi():
Line 835:             mount_efi(target="/liveos")
Line 836:             cfg_path = "/liveos/EFI"
Line 837:         elif os.path.ismount("/dev/.initramfs/live"):


Line 836:             cfg_path = "/liveos/EFI"
Line 837:         elif os.path.ismount("/dev/.initramfs/live"):
Line 838:             cfg_path = "/dev/.initramfs/live"
Line 839:         elif os.path.ismount("/run/initramfs/.live"):
Line 840:             cfg_path = "/liveos/grub"
This will not catch grub2 subdirs
Line 841: 
Line 842:         for dirName, subdirList, fileList in os.walk(cfg_path):
Line 843:             for f in fileList:
Line 844:                 if f.endswith(".conf") or f.endswith(".cfg"):


-- 
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: 7
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: hadong <hadong0720 at gmail.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at redhat.com>
Gerrit-Reviewer: Joey Boggs <jboggs at redhat.com>
Gerrit-Reviewer: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes