From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55689) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uuj9B-0008Nh-HF for qemu-devel@nongnu.org; Thu, 04 Jul 2013 09:00:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uuj96-0005hv-L8 for qemu-devel@nongnu.org; Thu, 04 Jul 2013 09:00:29 -0400 Received: from mail-ea0-x22c.google.com ([2a00:1450:4013:c01::22c]:59234) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uuj96-0005hg-FH for qemu-devel@nongnu.org; Thu, 04 Jul 2013 09:00:24 -0400 Received: by mail-ea0-f172.google.com with SMTP id q10so781407eaj.31 for ; Thu, 04 Jul 2013 06:00:23 -0700 (PDT) Date: Thu, 4 Jul 2013 15:00:20 +0200 From: Stefan Hajnoczi Message-ID: <20130704130020.GF4213@stefanha-thinkpad.redhat.com> References: <1372318700-25103-1-git-send-email-cngesaint@gmail.com> <1372318700-25103-2-git-send-email-cngesaint@gmail.com> <51CDF420.60202@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51CDF420.60202@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/2] Refine and export infinite loop checking in collect_image_info_list() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Xu Wang , Xu Wang , qemu-devel@nongnu.org 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