diff mbox series

[v2,07/13] of: unittest: Cleanup partially-applied overlays

Message ID 594a6a8934e5569bf96d317a6a3c0a9129a2ae20.1690533838.git.geert+renesas@glider.be (mailing list archive)
State Mainlined
Commit 0676aeeca537740a03ecdb8b699b37e98ec60289
Delegated to: Geert Uytterhoeven
Headers show
Series of: overlay/unittest: Miscellaneous fixes and improvements | expand

Commit Message

Geert Uytterhoeven July 28, 2023, 8:50 a.m. UTC
When of_overlay_fdt_apply() fails, the changeset may be partially
applied, and the caller is still expected to call of_overlay_remove() to
clean up this partial state.  However, overlay_17 is the only test that
takes care of cleaning up after an (expected) failure.

Instead of adding cleanup code to each individual test, extend
overlay_info with the optional expected return value of
of_overlay_remove(), and handle cleanup in the overlay_data_apply()
helper.  While at it, simplify the end marker in the overlay_info table.

Update the expected error output for errors during the newly cleanup.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/of/unittest.c | 130 +++++++++++++++++++++++++++---------------
 1 file changed, 83 insertions(+), 47 deletions(-)

Comments

Rob Herring Aug. 1, 2023, 8:50 p.m. UTC | #1
On Fri, Jul 28, 2023 at 2:50 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> When of_overlay_fdt_apply() fails, the changeset may be partially
> applied, and the caller is still expected to call of_overlay_remove() to
> clean up this partial state.  However, overlay_17 is the only test that
> takes care of cleaning up after an (expected) failure.
>
> Instead of adding cleanup code to each individual test, extend
> overlay_info with the optional expected return value of
> of_overlay_remove(), and handle cleanup in the overlay_data_apply()
> helper.  While at it, simplify the end marker in the overlay_info table.
>
> Update the expected error output for errors during the newly cleanup.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - No changes.
> ---
>  drivers/of/unittest.c | 130 +++++++++++++++++++++++++++---------------
>  1 file changed, 83 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index b23a44de091bd044..f9c09d5787362601 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -2964,12 +2964,6 @@ static void __init of_unittest_overlay_notify(void)
>
>         unittest(ovcs_id, "ovcs_id not created for overlay_17\n");
>
> -       if (ovcs_id) {
> -               ret = of_overlay_remove(&ovcs_id);
> -               unittest(!ret,
> -                       "overlay_17 of_overlay_remove(), ret = %d\n", ret);
> -       }
> -
>         /* ---  overlay 18  --- */
>
>         unittest(overlay_data_apply("overlay_18", &ovcs_id),
> @@ -3257,17 +3251,19 @@ static void __init of_unittest_lifecycle(void)
>         extern uint8_t __dtbo_##overlay_name##_begin[]; \
>         extern uint8_t __dtbo_##overlay_name##_end[]
>
> -#define OVERLAY_INFO(overlay_name, expected) \
> -{      .dtbo_begin       = __dtbo_##overlay_name##_begin, \
> -       .dtbo_end         = __dtbo_##overlay_name##_end, \
> -       .expected_result = expected, \
> -       .name            = #overlay_name, \
> +#define OVERLAY_INFO(overlay_name, expected, expected_remove) \
> +{      .dtbo_begin             = __dtbo_##overlay_name##_begin, \
> +       .dtbo_end               = __dtbo_##overlay_name##_end, \
> +       .expected_result        = expected, \
> +       .expected_result_remove = expected_remove, \
> +       .name                   = #overlay_name, \
>  }
>
>  struct overlay_info {
>         uint8_t         *dtbo_begin;
>         uint8_t         *dtbo_end;
>         int             expected_result;
> +       int             expected_result_remove; /* if apply failed */
>         int             ovcs_id;
>         char            *name;
>  };
> @@ -3307,40 +3303,40 @@ OVERLAY_INFO_EXTERN(overlay_bad_symbol);
>
>  /* entries found by name */
>  static struct overlay_info overlays[] = {
> -       OVERLAY_INFO(overlay_base, -9999),
> -       OVERLAY_INFO(overlay, 0),
> -       OVERLAY_INFO(overlay_0, 0),
> -       OVERLAY_INFO(overlay_1, 0),
> -       OVERLAY_INFO(overlay_2, 0),
> -       OVERLAY_INFO(overlay_3, 0),
> -       OVERLAY_INFO(overlay_4, 0),
> -       OVERLAY_INFO(overlay_5, 0),
> -       OVERLAY_INFO(overlay_6, 0),
> -       OVERLAY_INFO(overlay_7, 0),
> -       OVERLAY_INFO(overlay_8, 0),
> -       OVERLAY_INFO(overlay_9, 0),
> -       OVERLAY_INFO(overlay_10, 0),
> -       OVERLAY_INFO(overlay_11, 0),
> -       OVERLAY_INFO(overlay_12, 0),
> -       OVERLAY_INFO(overlay_13, 0),
> -       OVERLAY_INFO(overlay_15, 0),
> -       OVERLAY_INFO(overlay_16, -EBUSY),
> -       OVERLAY_INFO(overlay_17, -EEXIST),
> -       OVERLAY_INFO(overlay_18, 0),
> -       OVERLAY_INFO(overlay_19, 0),
> -       OVERLAY_INFO(overlay_20, 0),
> -       OVERLAY_INFO(overlay_gpio_01, 0),
> -       OVERLAY_INFO(overlay_gpio_02a, 0),
> -       OVERLAY_INFO(overlay_gpio_02b, 0),
> -       OVERLAY_INFO(overlay_gpio_03, 0),
> -       OVERLAY_INFO(overlay_gpio_04a, 0),
> -       OVERLAY_INFO(overlay_gpio_04b, 0),
> -       OVERLAY_INFO(overlay_bad_add_dup_node, -EINVAL),
> -       OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL),
> -       OVERLAY_INFO(overlay_bad_phandle, -EINVAL),
> -       OVERLAY_INFO(overlay_bad_symbol, -EINVAL),
> +       OVERLAY_INFO(overlay_base, -9999, 0),
> +       OVERLAY_INFO(overlay, 0, 0),
> +       OVERLAY_INFO(overlay_0, 0, 0),
> +       OVERLAY_INFO(overlay_1, 0, 0),
> +       OVERLAY_INFO(overlay_2, 0, 0),
> +       OVERLAY_INFO(overlay_3, 0, 0),
> +       OVERLAY_INFO(overlay_4, 0, 0),
> +       OVERLAY_INFO(overlay_5, 0, 0),
> +       OVERLAY_INFO(overlay_6, 0, 0),
> +       OVERLAY_INFO(overlay_7, 0, 0),
> +       OVERLAY_INFO(overlay_8, 0, 0),
> +       OVERLAY_INFO(overlay_9, 0, 0),
> +       OVERLAY_INFO(overlay_10, 0, 0),
> +       OVERLAY_INFO(overlay_11, 0, 0),
> +       OVERLAY_INFO(overlay_12, 0, 0),
> +       OVERLAY_INFO(overlay_13, 0, 0),
> +       OVERLAY_INFO(overlay_15, 0, 0),
> +       OVERLAY_INFO(overlay_16, -EBUSY, 0),
> +       OVERLAY_INFO(overlay_17, -EEXIST, 0),
> +       OVERLAY_INFO(overlay_18, 0, 0),
> +       OVERLAY_INFO(overlay_19, 0, 0),
> +       OVERLAY_INFO(overlay_20, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_01, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_02a, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_02b, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_03, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_04a, 0, 0),
> +       OVERLAY_INFO(overlay_gpio_04b, 0, 0),
> +       OVERLAY_INFO(overlay_bad_add_dup_node, -EINVAL, -ENODEV),
> +       OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL, -ENODEV),
> +       OVERLAY_INFO(overlay_bad_phandle, -EINVAL, 0),
> +       OVERLAY_INFO(overlay_bad_symbol, -EINVAL, -ENODEV),
>         /* end marker */
> -       {.dtbo_begin = NULL, .dtbo_end = NULL, .expected_result = 0, .name = NULL}
> +       { }
>  };
>
>  static struct device_node *overlay_base_root;
> @@ -3435,8 +3431,9 @@ void __init unittest_unflatten_overlay_base(void)
>  static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
>  {
>         struct overlay_info *info;
> +       int passed = 1;
>         int found = 0;
> -       int ret;
> +       int ret, ret2;
>         u32 size;
>
>         for (info = overlays; info && info->name; info++) {
> @@ -3463,11 +3460,24 @@ static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
>         pr_debug("%s applied\n", overlay_name);
>
>  out:
> -       if (ret != info->expected_result)
> +       if (ret != info->expected_result) {
>                 pr_err("of_overlay_fdt_apply() expected %d, ret=%d, %s\n",
>                        info->expected_result, ret, overlay_name);
> +               passed = 0;
> +       }
> +
> +       if (ret < 0) {
> +               /* changeset may be partially applied */
> +               ret2 = of_overlay_remove(&info->ovcs_id);
> +               if (ret2 != info->expected_result_remove) {
> +                       pr_err("of_overlay_remove() expected %d, ret=%d, %s\n",
> +                              info->expected_result_remove, ret2,
> +                              overlay_name);
> +                       passed = 0;
> +               }
> +       }
>
> -       return (ret == info->expected_result);
> +       return passed;
>  }
>
>  /*
> @@ -3660,10 +3670,18 @@ static __init void of_unittest_overlay_high_level(void)
>                      "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller");
>         EXPECT_BEGIN(KERN_ERR,
>                      "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/controller/name");
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: Error reverting changeset (-19)");
>
>         unittest(overlay_data_apply("overlay_bad_add_dup_node", NULL),
>                  "Adding overlay 'overlay_bad_add_dup_node' failed\n");
>
> +       EXPECT_END(KERN_ERR,
> +                  "OF: Error reverting changeset (-19)");
> +       EXPECT_END(KERN_ERR,
> +                  "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/controller/name");
>         EXPECT_END(KERN_ERR,
>                    "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
>         EXPECT_END(KERN_ERR,
> @@ -3675,10 +3693,18 @@ static __init void of_unittest_overlay_high_level(void)
>                      "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/rpm_avail");
>         EXPECT_BEGIN(KERN_ERR,
>                      "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/electric/name");
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: Error reverting changeset (-19)");
>
>         unittest(overlay_data_apply("overlay_bad_add_dup_prop", NULL),
>                  "Adding overlay 'overlay_bad_add_dup_prop' failed\n");
>
> +       EXPECT_END(KERN_ERR,
> +                  "OF: Error reverting changeset (-19)");
> +       EXPECT_END(KERN_ERR,
> +                  "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/electric/name");
>         EXPECT_END(KERN_ERR,
>                    "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
>         EXPECT_END(KERN_ERR,
> @@ -3689,9 +3715,19 @@ static __init void of_unittest_overlay_high_level(void)
>         unittest(overlay_data_apply("overlay_bad_phandle", NULL),
>                  "Adding overlay 'overlay_bad_phandle' failed\n");
>
> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");

I noticed my printing patch will have an issue on this because it
prints the changeset pointer value. I guess we have to manually check
that if EXPECT can't handle some type of wildcard.

> +       EXPECT_BEGIN(KERN_ERR,
> +                    "OF: resolver: overlay phandle fixup failed: -22");
> +
>         unittest(overlay_data_apply("overlay_bad_symbol", NULL),
>                  "Adding overlay 'overlay_bad_symbol' failed\n");
>
> +       EXPECT_END(KERN_ERR,
> +                  "OF: resolver: overlay phandle fixup failed: -22");

I'm seeing "OF: Error reverting changeset (-19)" here instead.
Cut-n-paste error?

Rob
Geert Uytterhoeven Aug. 14, 2023, 9:21 a.m. UTC | #2
Hi Rob,

On Tue, Aug 1, 2023 at 10:50 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Fri, Jul 28, 2023 at 2:50 AM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > When of_overlay_fdt_apply() fails, the changeset may be partially
> > applied, and the caller is still expected to call of_overlay_remove() to
> > clean up this partial state.  However, overlay_17 is the only test that
> > takes care of cleaning up after an (expected) failure.
> >
> > Instead of adding cleanup code to each individual test, extend
> > overlay_info with the optional expected return value of
> > of_overlay_remove(), and handle cleanup in the overlay_data_apply()
> > helper.  While at it, simplify the end marker in the overlay_info table.
> >
> > Update the expected error output for errors during the newly cleanup.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > v2:
> >   - No changes.

> > --- a/drivers/of/unittest.c
> > +++ b/drivers/of/unittest.c

> > @@ -3689,9 +3715,19 @@ static __init void of_unittest_overlay_high_level(void)
> >         unittest(overlay_data_apply("overlay_bad_phandle", NULL),
> >                  "Adding overlay 'overlay_bad_phandle' failed\n");
> >
> > +       EXPECT_BEGIN(KERN_ERR,
> > +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
> > +       EXPECT_BEGIN(KERN_ERR,
> > +                    "OF: resolver: overlay phandle fixup failed: -22");
> > +
> >         unittest(overlay_data_apply("overlay_bad_symbol", NULL),
> >                  "Adding overlay 'overlay_bad_symbol' failed\n");
> >
> > +       EXPECT_END(KERN_ERR,
> > +                  "OF: resolver: overlay phandle fixup failed: -22");
>
> I'm seeing "OF: Error reverting changeset (-19)" here instead.
> Cut-n-paste error?

Indeed. Thanks for pointing this out!

Gr{oetje,eeting}s,

                        Geert
Rob Herring Aug. 24, 2023, 1:06 a.m. UTC | #3
On Mon, Aug 14, 2023 at 4:21 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Rob,
>
> On Tue, Aug 1, 2023 at 10:50 PM Rob Herring <robh+dt@kernel.org> wrote:
> > On Fri, Jul 28, 2023 at 2:50 AM Geert Uytterhoeven
> > <geert+renesas@glider.be> wrote:
> > > When of_overlay_fdt_apply() fails, the changeset may be partially
> > > applied, and the caller is still expected to call of_overlay_remove() to
> > > clean up this partial state.  However, overlay_17 is the only test that
> > > takes care of cleaning up after an (expected) failure.
> > >
> > > Instead of adding cleanup code to each individual test, extend
> > > overlay_info with the optional expected return value of
> > > of_overlay_remove(), and handle cleanup in the overlay_data_apply()
> > > helper.  While at it, simplify the end marker in the overlay_info table.
> > >
> > > Update the expected error output for errors during the newly cleanup.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > ---
> > > v2:
> > >   - No changes.
>
> > > --- a/drivers/of/unittest.c
> > > +++ b/drivers/of/unittest.c
>
> > > @@ -3689,9 +3715,19 @@ static __init void of_unittest_overlay_high_level(void)
> > >         unittest(overlay_data_apply("overlay_bad_phandle", NULL),
> > >                  "Adding overlay 'overlay_bad_phandle' failed\n");
> > >
> > > +       EXPECT_BEGIN(KERN_ERR,
> > > +                    "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
> > > +       EXPECT_BEGIN(KERN_ERR,
> > > +                    "OF: resolver: overlay phandle fixup failed: -22");
> > > +
> > >         unittest(overlay_data_apply("overlay_bad_symbol", NULL),
> > >                  "Adding overlay 'overlay_bad_symbol' failed\n");
> > >
> > > +       EXPECT_END(KERN_ERR,
> > > +                  "OF: resolver: overlay phandle fixup failed: -22");
> >
> > I'm seeing "OF: Error reverting changeset (-19)" here instead.
> > Cut-n-paste error?
>
> Indeed. Thanks for pointing this out!

I've fixed this up along with the other EXPECTs that changed due to my
rework and applied the series.

Thanks,
Rob
diff mbox series

Patch

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index b23a44de091bd044..f9c09d5787362601 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2964,12 +2964,6 @@  static void __init of_unittest_overlay_notify(void)
 
 	unittest(ovcs_id, "ovcs_id not created for overlay_17\n");
 
-	if (ovcs_id) {
-		ret = of_overlay_remove(&ovcs_id);
-		unittest(!ret,
-			"overlay_17 of_overlay_remove(), ret = %d\n", ret);
-	}
-
 	/* ---  overlay 18  --- */
 
 	unittest(overlay_data_apply("overlay_18", &ovcs_id),
@@ -3257,17 +3251,19 @@  static void __init of_unittest_lifecycle(void)
 	extern uint8_t __dtbo_##overlay_name##_begin[]; \
 	extern uint8_t __dtbo_##overlay_name##_end[]
 
-#define OVERLAY_INFO(overlay_name, expected) \
-{	.dtbo_begin       = __dtbo_##overlay_name##_begin, \
-	.dtbo_end         = __dtbo_##overlay_name##_end, \
-	.expected_result = expected, \
-	.name            = #overlay_name, \
+#define OVERLAY_INFO(overlay_name, expected, expected_remove) \
+{	.dtbo_begin		= __dtbo_##overlay_name##_begin, \
+	.dtbo_end		= __dtbo_##overlay_name##_end, \
+	.expected_result	= expected, \
+	.expected_result_remove	= expected_remove, \
+	.name			= #overlay_name, \
 }
 
 struct overlay_info {
 	uint8_t		*dtbo_begin;
 	uint8_t		*dtbo_end;
 	int		expected_result;
+	int		expected_result_remove;	/* if apply failed */
 	int		ovcs_id;
 	char		*name;
 };
@@ -3307,40 +3303,40 @@  OVERLAY_INFO_EXTERN(overlay_bad_symbol);
 
 /* entries found by name */
 static struct overlay_info overlays[] = {
-	OVERLAY_INFO(overlay_base, -9999),
-	OVERLAY_INFO(overlay, 0),
-	OVERLAY_INFO(overlay_0, 0),
-	OVERLAY_INFO(overlay_1, 0),
-	OVERLAY_INFO(overlay_2, 0),
-	OVERLAY_INFO(overlay_3, 0),
-	OVERLAY_INFO(overlay_4, 0),
-	OVERLAY_INFO(overlay_5, 0),
-	OVERLAY_INFO(overlay_6, 0),
-	OVERLAY_INFO(overlay_7, 0),
-	OVERLAY_INFO(overlay_8, 0),
-	OVERLAY_INFO(overlay_9, 0),
-	OVERLAY_INFO(overlay_10, 0),
-	OVERLAY_INFO(overlay_11, 0),
-	OVERLAY_INFO(overlay_12, 0),
-	OVERLAY_INFO(overlay_13, 0),
-	OVERLAY_INFO(overlay_15, 0),
-	OVERLAY_INFO(overlay_16, -EBUSY),
-	OVERLAY_INFO(overlay_17, -EEXIST),
-	OVERLAY_INFO(overlay_18, 0),
-	OVERLAY_INFO(overlay_19, 0),
-	OVERLAY_INFO(overlay_20, 0),
-	OVERLAY_INFO(overlay_gpio_01, 0),
-	OVERLAY_INFO(overlay_gpio_02a, 0),
-	OVERLAY_INFO(overlay_gpio_02b, 0),
-	OVERLAY_INFO(overlay_gpio_03, 0),
-	OVERLAY_INFO(overlay_gpio_04a, 0),
-	OVERLAY_INFO(overlay_gpio_04b, 0),
-	OVERLAY_INFO(overlay_bad_add_dup_node, -EINVAL),
-	OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL),
-	OVERLAY_INFO(overlay_bad_phandle, -EINVAL),
-	OVERLAY_INFO(overlay_bad_symbol, -EINVAL),
+	OVERLAY_INFO(overlay_base, -9999, 0),
+	OVERLAY_INFO(overlay, 0, 0),
+	OVERLAY_INFO(overlay_0, 0, 0),
+	OVERLAY_INFO(overlay_1, 0, 0),
+	OVERLAY_INFO(overlay_2, 0, 0),
+	OVERLAY_INFO(overlay_3, 0, 0),
+	OVERLAY_INFO(overlay_4, 0, 0),
+	OVERLAY_INFO(overlay_5, 0, 0),
+	OVERLAY_INFO(overlay_6, 0, 0),
+	OVERLAY_INFO(overlay_7, 0, 0),
+	OVERLAY_INFO(overlay_8, 0, 0),
+	OVERLAY_INFO(overlay_9, 0, 0),
+	OVERLAY_INFO(overlay_10, 0, 0),
+	OVERLAY_INFO(overlay_11, 0, 0),
+	OVERLAY_INFO(overlay_12, 0, 0),
+	OVERLAY_INFO(overlay_13, 0, 0),
+	OVERLAY_INFO(overlay_15, 0, 0),
+	OVERLAY_INFO(overlay_16, -EBUSY, 0),
+	OVERLAY_INFO(overlay_17, -EEXIST, 0),
+	OVERLAY_INFO(overlay_18, 0, 0),
+	OVERLAY_INFO(overlay_19, 0, 0),
+	OVERLAY_INFO(overlay_20, 0, 0),
+	OVERLAY_INFO(overlay_gpio_01, 0, 0),
+	OVERLAY_INFO(overlay_gpio_02a, 0, 0),
+	OVERLAY_INFO(overlay_gpio_02b, 0, 0),
+	OVERLAY_INFO(overlay_gpio_03, 0, 0),
+	OVERLAY_INFO(overlay_gpio_04a, 0, 0),
+	OVERLAY_INFO(overlay_gpio_04b, 0, 0),
+	OVERLAY_INFO(overlay_bad_add_dup_node, -EINVAL, -ENODEV),
+	OVERLAY_INFO(overlay_bad_add_dup_prop, -EINVAL, -ENODEV),
+	OVERLAY_INFO(overlay_bad_phandle, -EINVAL, 0),
+	OVERLAY_INFO(overlay_bad_symbol, -EINVAL, -ENODEV),
 	/* end marker */
-	{.dtbo_begin = NULL, .dtbo_end = NULL, .expected_result = 0, .name = NULL}
+	{ }
 };
 
 static struct device_node *overlay_base_root;
@@ -3435,8 +3431,9 @@  void __init unittest_unflatten_overlay_base(void)
 static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
 {
 	struct overlay_info *info;
+	int passed = 1;
 	int found = 0;
-	int ret;
+	int ret, ret2;
 	u32 size;
 
 	for (info = overlays; info && info->name; info++) {
@@ -3463,11 +3460,24 @@  static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
 	pr_debug("%s applied\n", overlay_name);
 
 out:
-	if (ret != info->expected_result)
+	if (ret != info->expected_result) {
 		pr_err("of_overlay_fdt_apply() expected %d, ret=%d, %s\n",
 		       info->expected_result, ret, overlay_name);
+		passed = 0;
+	}
+
+	if (ret < 0) {
+		/* changeset may be partially applied */
+		ret2 = of_overlay_remove(&info->ovcs_id);
+		if (ret2 != info->expected_result_remove) {
+			pr_err("of_overlay_remove() expected %d, ret=%d, %s\n",
+			       info->expected_result_remove, ret2,
+			       overlay_name);
+			passed = 0;
+		}
+	}
 
-	return (ret == info->expected_result);
+	return passed;
 }
 
 /*
@@ -3660,10 +3670,18 @@  static __init void of_unittest_overlay_high_level(void)
 		     "OF: overlay: ERROR: multiple fragments add and/or delete node /testcase-data-2/substation@100/motor-1/controller");
 	EXPECT_BEGIN(KERN_ERR,
 		     "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/controller/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: Error reverting changeset (-19)");
 
 	unittest(overlay_data_apply("overlay_bad_add_dup_node", NULL),
 		 "Adding overlay 'overlay_bad_add_dup_node' failed\n");
 
+	EXPECT_END(KERN_ERR,
+		   "OF: Error reverting changeset (-19)");
+	EXPECT_END(KERN_ERR,
+		   "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/controller/name");
 	EXPECT_END(KERN_ERR,
 		   "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/controller/name");
 	EXPECT_END(KERN_ERR,
@@ -3675,10 +3693,18 @@  static __init void of_unittest_overlay_high_level(void)
 		     "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/rpm_avail");
 	EXPECT_BEGIN(KERN_ERR,
 		     "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/electric/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: Error reverting changeset (-19)");
 
 	unittest(overlay_data_apply("overlay_bad_add_dup_prop", NULL),
 		 "Adding overlay 'overlay_bad_add_dup_prop' failed\n");
 
+	EXPECT_END(KERN_ERR,
+		   "OF: Error reverting changeset (-19)");
+	EXPECT_END(KERN_ERR,
+		   "OF: changeset: remove_property failed @/testcase-data-2/substation@100/motor-1/electric/name");
 	EXPECT_END(KERN_ERR,
 		   "OF: overlay: ERROR: multiple fragments add, update, and/or delete property /testcase-data-2/substation@100/motor-1/electric/name");
 	EXPECT_END(KERN_ERR,
@@ -3689,9 +3715,19 @@  static __init void of_unittest_overlay_high_level(void)
 	unittest(overlay_data_apply("overlay_bad_phandle", NULL),
 		 "Adding overlay 'overlay_bad_phandle' failed\n");
 
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
+	EXPECT_BEGIN(KERN_ERR,
+		     "OF: resolver: overlay phandle fixup failed: -22");
+
 	unittest(overlay_data_apply("overlay_bad_symbol", NULL),
 		 "Adding overlay 'overlay_bad_symbol' failed\n");
 
+	EXPECT_END(KERN_ERR,
+		   "OF: resolver: overlay phandle fixup failed: -22");
+	EXPECT_END(KERN_ERR,
+		   "OF: changeset: remove_property failed @/testcase-data-2/substation@100/hvac-medium-2/name");
+
 	return;
 
 err_unlock: