[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [master f17-branch 1/2] restore the GPT blacklist code



This looks good, but I have a possibly-minor concern about the matching,
below:

On Mon, 2012-04-02 at 11:00 -0700, Brian C. Lane wrote:

> +    def blackListGPT(self):
> +        """ Remove GPT disk label as an option on systems where their BIOS
> +            doesn't boot from GPT labeled disks.
> +
> +            Currently this includes: Lenovo
> +        """
> +        if not os.path.isfile(DMI_CHASSIS_VENDOR):
> +            return
> +        buf = open(DMI_CHASSIS_VENDOR).read()
> +        if "LENOVO" in buf.splitlines() and "gpt" in self._disklabel_types:

The splitlines() is confusing here.. is LENOVO supposed to be a
substring match, or an exact match? Also: are you sure it's always
uppercase? 

Maybe this should be a bit looser, and the match a bit clearer?

if "gpt" in self._disklabel_types:
    vendor = open(DMI_CHASSIS_VENDOR).read().strip().lower()
    if "lenovo" in vendor:

I'm sure it works as-is but I'd rather not have to revise the patch once
we find out that *some* Lenovo systems have "Lenovo" and some "LENOVO"
and some "Lenovo, Inc.", etc, etc..  Unless there's some standard/spec
that says that'll never happen?

-w




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]