[node-patches] Change in ovirt-node[master]: Add utils.system.Bootloader

[fabiand at fedoraproject.org]

Fabian Deutsch has posted comments on this change.

Change subject: Add utils.system.Bootloader
......................................................................


Patch Set 8: Code-Review-1

(3 comments)

Basically amazing. Just three things.

....................................................
File src/ovirt/node/utils/system.py
Line 88:     """
Line 89:     return os.path.exists("/dev/HostVG")
Line 90: 
Line 91: 
Line 92: def has_filesystem(label):
Could we either rename the function to reflect that a fslabel is expected, e.g.:

    def has_filesystem_with_label(...) # or: ..._for_label()

Or create a class, e.g.:

    class Filesystem(...):
        @staticmethod
        def by_label(label):
Line 93:     """Determines whether a filesystem with a given label is present on this
Line 94:     system
Line 95:     """
Line 96:     process.call(["partprobe"] + [x for x in glob.glob("/dev/mapper/*")


Line 550:             self.path = path
Line 551:         else:
Line 552:             raise RuntimeError("Must be called with a mount point")
Line 553: 
Line 554:     def remount(self, ro=True, rw=False):
ro is not used below, I'd probably use

    def remount(self, ro=False):
Line 555:         # EL6 won't let you remount if it's not in mtab or fstab
Line 556:         # So we'll parse /proc/mounts ourselves to find it
Line 557:         device = self._find_device()
Line 558:         try:


Line 565:         except:
Line 566:             self.logger.exception("Can't remount %s on %s!" % (device,
Line 567:                                                                self.path))
Line 568: 
Line 569:     def _find_device(self):
Is this for finding the device for a mountpoint? We can probably use

   findmnt -o SOURCE -n /tmp $PATH
Line 570:         mounts = [mount for mount in File("/proc/mounts")]
Line 571:         m = re.compile(r'^(/dev.*?)\s+%s\s+.*' % self.path)
Line 572:         for mount in mounts:
Line 573:             if m.match(mount):


-- 
To view, visit http://gerrit.ovirt.org/19811
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a796a4c07bd99b927070ec62a7b1d990d4e1f9b
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes