[PATCH v2 16/49] binman: Detect when valid images are not produced
Bin Meng
bmeng.cn at gmail.com
Mon Jun 29 08:54:32 CEST 2020
Hi Simon,
On Sun, Jun 14, 2020 at 10:57 AM Simon Glass <sjg at chromium.org> wrote:
>
> When external blobs are missing, show a message indicating that the images
> are not functional.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> (no changes since v1)
>
> tools/binman/control.py | 16 +++++++++++--
> tools/binman/entry.py | 21 +++++++++++++++++
> tools/binman/etype/blob_ext.py | 1 +
> tools/binman/etype/section.py | 16 ++++++++++++-
> tools/binman/ftest.py | 14 ++++++++++-
> .../binman/test/159_blob_ext_missing_sect.dts | 23 +++++++++++++++++++
> tools/patman/tout.py | 1 +
> 7 files changed, 88 insertions(+), 4 deletions(-)
> create mode 100644 tools/binman/test/159_blob_ext_missing_sect.dts
>
> diff --git a/tools/binman/control.py b/tools/binman/control.py
> index 8c6eae83f1..343b0a0c35 100644
> --- a/tools/binman/control.py
> +++ b/tools/binman/control.py
> @@ -403,6 +403,9 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
> allow_resize: True to allow entries to change size (this does a re-pack
> of the entries), False to raise an exception
> allow_missing: Allow blob_ext objects to be missing
> +
> + Returns:
> + True if one or more external blobs are missing, False if all are present
> """
> if get_contents:
> image.SetAllowMissing(allow_missing)
> @@ -450,6 +453,12 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
> image.BuildImage()
> if write_map:
> image.WriteMap()
> + missing_list = []
> + image.CheckMissing(missing_list)
> + if missing_list:
> + tout.Warning("Image '%s' is missing external blobs and is non-functional: %s" %
> + (image.name, ' '.join([e.name for e in missing_list])))
> + return bool(missing_list)
>
>
> def Binman(args):
> @@ -524,14 +533,17 @@ def Binman(args):
>
> images = PrepareImagesAndDtbs(dtb_fname, args.image,
> args.update_fdt)
> + missing = False
> for image in images.values():
> - ProcessImage(image, args.update_fdt, args.map,
> - allow_missing=args.allow_missing)
> + missing |= ProcessImage(image, args.update_fdt, args.map,
> + allow_missing=args.allow_missing)
>
> # Write the updated FDTs to our output files
> for dtb_item in state.GetAllFdts():
> tools.WriteFile(dtb_item._fname, dtb_item.GetContents())
>
> + if missing:
> + tout.Warning("Some images are invalid")
> finally:
> tools.FinaliseOutputDir()
> finally:
> diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> index 90ffd27617..4a42e0bf34 100644
> --- a/tools/binman/entry.py
> +++ b/tools/binman/entry.py
> @@ -84,6 +84,7 @@ class Entry(object):
> self.image_pos = None
> self._expand_size = False
> self.compress = 'none'
> + self.missing = False
>
> @staticmethod
> def Lookup(node_path, etype):
> @@ -794,3 +795,23 @@ features to produce new behaviours.
> elif self == entries[-1]:
> return 'end'
> return 'middle'
> +
> + def SetAllowMissing(self, allow_missing):
> + """Set whether a section allows missing external blobs
> +
> + Args:
> + allow_missing: True if allowed, False if not allowed
> + """
> + # This is meaningless for
for what?
> + self._allow_missing = allow_missing
> +
Should the above changes be in patch "[v2,14/49] binman: Allow
external binaries to be missing" ?
> + def CheckMissing(self, missing_list):
> + """Check if any entries in this section have missing external blobs
> +
> + If there are missing blobs, the entries are added to the list
> +
> + Args:
> + missing_list: List of Entry objects to be added to
> + """
> + if self.missing:
> + missing_list.append(self)
> diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py
> index 51779c88c9..8d641001a9 100644
> --- a/tools/binman/etype/blob_ext.py
> +++ b/tools/binman/etype/blob_ext.py
> @@ -34,5 +34,6 @@ class Entry_blob_ext(Entry_blob):
> # Allow the file to be missing
> if not self._pathname:
> self.SetContents(b'')
> + self.missing = True
> return True
> return super().ObtainContents()
> diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
> index 44b13b2575..3b42a5890f 100644
> --- a/tools/binman/etype/section.py
> +++ b/tools/binman/etype/section.py
> @@ -50,6 +50,7 @@ class Entry_section(Entry):
> self._skip_at_start = None
> self._end_4gb = False
> self._allow_missing = False
> + self.missing = False
>
> def ReadNode(self):
> """Read properties from the image node"""
> @@ -543,7 +544,9 @@ class Entry_section(Entry):
> Args:
> allow_missing: True if allowed, False if not allowed
> """
> - self._allow_missing = allow_missing
> + super().SetAllowMissing(allow_missing)
> + for entry in self._entries.values():
> + entry.SetAllowMissing(allow_missing)
>
> def GetAllowMissing(self):
> """Get whether a section allows missing external blobs
> @@ -552,3 +555,14 @@ class Entry_section(Entry):
> True if allowed, False if not allowed
> """
> return self._allow_missing
> +
> + def CheckMissing(self, missing_list):
> + """Check if any entries in this section have missing external blobs
> +
> + If there are missing blobs, the entries are added to the list
> +
> + Args:
> + missing_list: List of Entry objects to be added to
> + """
> + for entry in self._entries.values():
> + entry.CheckMissing(missing_list)
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 928d3608a3..cc551c9f17 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -3380,7 +3380,19 @@ class TestFunctional(unittest.TestCase):
>
> def testExtblobMissingOk(self):
> """Test an image with an missing external blob that is allowed"""
> - self._DoTestFile('158_blob_ext_missing.dts', allow_missing=True)
> + with test_util.capture_sys_output() as (stdout, stderr):
> + self._DoTestFile('158_blob_ext_missing.dts', allow_missing=True)
> + err = stderr.getvalue()
> + self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")
> +
> + def testExtblobMissingOkSect(self):
> + """Test an image with an missing external blob that is allowed"""
> + with test_util.capture_sys_output() as (stdout, stderr):
> + self._DoTestFile('159_blob_ext_missing_sect.dts',
> + allow_missing=True)
> + err = stderr.getvalue()
> + self.assertRegex(err, "Image 'main-section'.*missing.*: "
> + "blob-ext blob-ext2")
>
>
> if __name__ == "__main__":
> diff --git a/tools/binman/test/159_blob_ext_missing_sect.dts b/tools/binman/test/159_blob_ext_missing_sect.dts
> new file mode 100644
> index 0000000000..5f14c54138
> --- /dev/null
> +++ b/tools/binman/test/159_blob_ext_missing_sect.dts
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + binman {
> + size = <0x80>;
> +
> + section {
> + blob-ext {
> + filename = "missing-file";
> + };
> + };
> +
> + blob-ext2 {
> + type = "blob-ext";
> + filename = "missing-file2";
> + };
> + };
> +};
> diff --git a/tools/patman/tout.py b/tools/patman/tout.py
> index 91a53f4073..33305263d8 100644
> --- a/tools/patman/tout.py
> +++ b/tools/patman/tout.py
> @@ -171,6 +171,7 @@ def Init(_verbose=WARNING, stdout=sys.stdout):
>
> # TODO(sjg): Move this into Chromite libraries when we have them
> stdout_is_tty = hasattr(sys.stdout, 'isatty') and sys.stdout.isatty()
> + stderr_is_tty = hasattr(sys.stderr, 'isatty') and sys.stderr.isatty()
>
> def Uninit():
> ClearProgress()
> --
Regards,
Bin
More information about the U-Boot
mailing list