Re: [Qemu-devel] [PATCH V7 0/4] Refine and export backing file loop check

[Fam Zheng <famz@redhat.com>]

On 2013年11月13日 10:39, Xu Wang wrote:
> If there is loop exists in the backing file chain, many problems
> could be caused by it, such as no response and segment fault during
> system boot. Hence stopping backing file loop appear is very necessary.
> These patches refine and export loop checking function from collect_image_
> info_list() to block.c and build a independent function named bdrv_
> backing_file_loop_check(). Backing file loop checking is added before
> image created, before change backing file and before system boot.
>
> Updates from V6:
>    1. Combine bdrv_backing_chain_okay() and bdrv_new_chain_okay() into one.
>    2. Delete bdrv_find_format() in the backing_chain_has_loop().
>    3. Comments syntax and function naming updates.
>
> Updates from V5:
>    1. Simplify the function of loop checking (Just filename comparation.
>       Thanks Eric's suggestion).
>    2. Delete WIN32 platform support (There is no need to this patch now).
>    3. Adjust position of backing file loop checking (calling checking function
>       before change happen).
>    4. Function name updates and comments description fix.
>
> Updates from V4:
>    1. Add backing file loop check in bdrv_new_open().
>    2. Adjust open file logic of collect_image_info_list() (bdrv_new_open()
>       is called only once when opening the whole chain).
>    3. Remove redundant brackets in lnk file check logic.
>    4. Add error output in bdrv_img_create().
>    5. Remove MAX_PATH_LEN to use PATH_MAX instead.
>
> Updates from V3:
>    1. Comments fix for function bdrv_backing_file_loop_check().
>    2. Add ret check for fseek()/fread() in get_lnk_target_file().
>    3. Add limit of shortcuts filename length reading during comparing.
>    4. Add error_report() in driv_init().
>    5. Remove redundant loop check in qcow2/qed_change_backing_file().
>
> Updates from V2:
>    1. Removed parameter @chain from bdrv_backing_file_loop_check()
>    2. Comments and format fix, all patches were checked by checkpatch.pl
>    3. Fixed *bs leak.
>    4. Improved logic of .lnk file recognization.
>    5. Add filename lenth limit check in while()
>    6. Changed get_win_inode() to get_inode() and move all inode get method
>       into it to make logic more simpler.
>    7. Added value of @fmt as suggested.
>    8. Added backing file loop check in qcow2.c/qed.c
>
> Xu Wang (4):
>    block/qemu-img: Refine and export infinite loop checking in
>      collect_image_info_list()
>    block: Add check infinite loop in bdrv_img_create()
>    block: Add backing file loop check in change_backing_file()
>    blockdev: Add infinite loop check in drive_init()
>
>   block.c               | 90 ++++++++++++++++++++++++++++++++++++++++++++++++---
>   blockdev.c            |  6 ++++
>   include/block/block.h |  3 ++
>   qemu-img.c            | 52 ++++++++++++++---------------
>   4 files changed, 121 insertions(+), 30 deletions(-)
>

Thanks, this looks pretty good to me. I've left a few questions, mainly 
regarding to the error messages.

Thank you for your effort continuously improving your series!

Fam