Re: [Qemu-devel] [PATCH 1/2] Refine and export infinite loop checking in collect_image_info_list()

[Stefan Hajnoczi <stefanha@gmail.com>]

On Fri, Jun 28, 2013 at 02:37:52PM -0600, Eric Blake wrote:
> On 06/27/2013 01:38 AM, Xu Wang wrote:
> > +    filenames = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
> > +
> > +    /* If backing file exists, filename will insert into hash table and seek
> > +     * the whole backing file chain from @backing_file.
> > +     */
> > +    if (backing_file) {
> > +        g_hash_table_insert(filenames, (gpointer)filename, NULL);
> 
> Does this have any false positives (perhaps mishandling due to relative
> names) or false negatives (perhaps hard links allow different spellings
> of the same file to create a loop, although the difference in names
> won't indicate the problem)?  I'd really like to see you add a testcase
> before this patch gets committed, although I agree that a patch along
> these lines is worthwhile.  For example, make sure the following chain
> is not rejected:
> 
> /dir1/base.img <- /dir1/wrap.img(relative backing 'base.img') <-
> /dir2/base.img (absolute backing '/dir1/base.img') <-
> /dir2/wrap.img(relative backing 'base.img')
> 
> whether opened in /dir2/ via relative name 'wrap.img' or absolute name
> '/dir2/wrap.img'.  Likewise, make sure you can detect this loop:
> 
> create directory 'dir'
> create './dir/b.img'
> create './b.img' with relative backing 'dir/b.img'
> remove ./dir/b.img and dir
> ln -s . dir
> now 'b.img' refers to itself as backing file, even though the names
> ./b.img and ./dir/b.img are not equal by strcmp.

Yes, a test case should be added in tests/qemu-iotests/.  Please see
this wiki page for documentation:

http://qemu-project.org/Documentation/QemuIoTests

Stefan