fix bdrv_open_child return value check

Commit Message

Dmitry Frolov Aug. 31, 2023, 12:59 p.m. UTC

bdrv_open_child() may return NULL.
Usually return value is checked for this function.
Check for return value is more reliable.

Fixes: 24bc15d1f6 ("vmdk: Use BdrvChild instead of BDS for references to extents")

Signed-off-by: Dmitry Frolov <[e-mail]>
---
 block/vmdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kevin Wolf Sept. 1, 2023, 11:15 a.m. UTC | #1

Am 31.08.2023 um 14:59 hat Dmitry Frolov geschrieben:
> bdrv_open_child() may return NULL.
> Usually return value is checked for this function.
> Check for return value is more reliable.
> 
> Fixes: 24bc15d1f6 ("vmdk: Use BdrvChild instead of BDS for references to extents")
> 
> Signed-off-by: Dmitry Frolov <[e-mail]>

Did you actually see some failure because of this? If so, what is the
specific case that failed?

Or is this just cleanup based on code inspection?

Kevin

Dmitry Frolov Sept. 1, 2023, 11:26 a.m. UTC | #2

Hello, Kevin.

This was just cleanup, based on the inspection.

Dmitry.

01.09.2023 14:15, Kevin Wolf пишет:
> Am 31.08.2023 um 14:59 hat Dmitry Frolov geschrieben:
>> bdrv_open_child() may return NULL.
>> Usually return value is checked for this function.
>> Check for return value is more reliable.
>>
>> Fixes: 24bc15d1f6 ("vmdk: Use BdrvChild instead of BDS for references to extents")
>>
>> Signed-off-by: Dmitry Frolov <[e-mail]>
> Did you actually see some failure because of this? If so, what is the
> specific case that failed?
>
> Or is this just cleanup based on code inspection?
>
> Kevin
>

Kevin Wolf Sept. 1, 2023, 1:57 p.m. UTC | #3

Am 01.09.2023 um 13:26 hat Дмитрий Фролов geschrieben:
> Hello, Kevin.
> 
> This was just cleanup, based on the inspection.

Thanks for clarifying. I've updated the subject line to "vmdk: Clean up
bdrv_open_child() return value check" to avoid misunderstandings and
applied this to my block branch.

Kevin

Patch

diff --git a/block/vmdk.c b/block/vmdk.c
index 70066c2b01..58ce290e9c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1207,7 +1207,7 @@  static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
                                       bs, &child_of_bds, extent_role, false,
                                       &local_err);
         g_free(extent_path);
-        if (local_err) {
+        if (!extent_file) {
             error_propagate(errp, local_err);
             ret = -EINVAL;
             goto out;