[PATCH] Restrict payload kernel versions to kernels in the payload (#1074358)

David Shea dshea at redhat.com
Wed Jan 21 14:47:03 UTC 2015


On 01/21/2015 02:25 AM, Vratislav Podzimek wrote:
>
> @@ -334,16 +316,8 @@ class Payload(object):
>   
>       @property
>       def kernelVersionList(self):
> -        if not self._kernelVersionList:
> -            self._updateKernelVersionList()
> -
> -        return self._kernelVersionList[0]
> -
> -    @property
> -    def rescueKernelList(self):
> -        # do re-scan if looking for rescue kernel
> -        self._updateKernelVersionList()
> -        return self._kernelVersionList[1]
> +        """ An iterator of the kernel versions installed by the payload. """
> +        raise NotImplementedError()
> I don't think all kernelVersionList implementations return an iterator.
> We should either change this docstring to 'iterable' or really return
> iterators from all the implementations.

I'll change the comment to say iterable, which is what I meant anyway. I 
didn't want to say list, which was the type for the property in Payload, 
since most of them were returning generators.


>   
> @@ -180,6 +186,17 @@ class LiveImagePayload(ImagePayload):
>       def spaceRequired(self):
>           return Size(iutil.getDirSize("/")*1024)
>   
> +    def _updateKernelVersionList(self):
> +        files = glob.glob(INSTALL_TREE + "/boot/vmlinuz-*")
> +        files.extend(glob.glob(INSTALL_TREE + "/boot/efi/EFI/%s/vmlinuz-*" % self.instclass.efi_dir))
> +
> +        self._kernelVersionList = sorted([f.split("/")[-1][8:] for f in files
> +           if os.path.isfile(f) and "-rescue-" not in f], cmp=versionCmp)
> You can use generator instead of list comprehension in the sorted()
> call.

Fair enough. I had this one as a list to ensure that all the data was 
gathered when the method is called, but glob returns a list anyway, so 
there's no danger of having a half-iterated generator when the image is 
umounted.

>
> @@ -493,3 +513,19 @@ class LiveImageKSPayload(LiveImagePayload):
>               return Size(self._min_size)
>           else:
>               return Size(1024*1024*1024)
> +
> +    @property
> +    def kernelVersionList(self):
> +        # If it doesn't look like a tarfile use the super's kernelVersionList
> +        if not self.is_tarfile:
> +            return super(LiveImageKSPayload, self).kernelVersionList
> +
> +        import tarfile
> +        with tarfile.open(self.image_path) as archive:
> +            names = archive.getnames()
> +
> +            # Strip out vmlinuz- from the names
> +            kernels = [n.split("/")[-1][8:] for n in names if "boot/vmlinuz-" in n]
> +            kernels.sort(cmp=versionCmp)
> sorted((n.split()....), cmp=versionCmp) would better match the other
> implementations of this property and it would not use a temporary list.

Will do.

>
>> +
>> +            return kernels
>> diff --git a/pyanaconda/packaging/tarpayload.py b/pyanaconda/packaging/tarpayload.py
>> index b6328e0..3082530 100644
>> --- a/pyanaconda/packaging/tarpayload.py
>> +++ b/pyanaconda/packaging/tarpayload.py
>> @@ -36,7 +36,7 @@ except ImportError:
>>       log.error("import of tarfile failed")
>>       tarfile = None
>>   
>> -from pyanaconda.packaging import ArchivePayload, PayloadError
>> +from pyanaconda.packaging import ArchivePayload, PayloadError, versionCmp
>>   from pyanaconda import iutil
>>   
>>   # TarPayload is not yet fully implemented
>> @@ -73,7 +73,11 @@ class TarPayload(ArchivePayload):
>>       @property
>>       def kernelVersionList(self):
>>           names = self.archive.getnames()
>> -        kernels = [n for n in names if "boot/vmlinuz-" in n]
>> +
>> +        # Strip out vmlinuz- from the names
>> +        kernels = [n.split("/")[-1][8:] for n in names if "boot/vmlinuz-" in n]
>> +        kernels.sort(cmp=versionCmp)
> Same here.

And same here.



More information about the anaconda-patches mailing list