From patchwork Thu Oct 20 11:16:20 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 120800 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 00CFDB70B2 for ; Thu, 20 Oct 2011 23:09:32 +1100 (EST) Received: from localhost ([::1]:49676 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGqcj-0001fJ-FY for incoming@patchwork.ozlabs.org; Thu, 20 Oct 2011 07:17:21 -0400 Received: from eggs.gnu.org ([140.186.70.92]:55611) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGqcB-0000Fh-ND for qemu-devel@nongnu.org; Thu, 20 Oct 2011 07:16:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RGqcA-0001Re-GM for qemu-devel@nongnu.org; Thu, 20 Oct 2011 07:16:47 -0400 Received: from mail-iy0-f173.google.com ([209.85.210.173]:57898) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RGqcA-0001Qa-2z for qemu-devel@nongnu.org; Thu, 20 Oct 2011 07:16:46 -0400 Received: by mail-iy0-f173.google.com with SMTP id l21so3766477iak.4 for ; Thu, 20 Oct 2011 04:16:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=wOx2R75CmIh4Fsg6ZG8ZvZn66Z6FDMChqg9hkVEEzgs=; b=Xw9vpAvQtRTywppd4uBH4kXjF5FdeQSFbc9twlIjxJ3/h6D+mkc4zHSuETneUT0O5h WPK6oWo20Orl/aiVdJ11P/Wu62KIdcyI794iwGhOEuf/RatK0BOWas/bMjrPheEg3a+D 1F1EcTUuy46YdjF/KLoy1mHjwgiNlYDDo97B8= Received: by 10.42.136.196 with SMTP id v4mr18454696ict.3.1319109405846; Thu, 20 Oct 2011 04:16:45 -0700 (PDT) Received: from localhost.localdomain (93-34-218-143.ip51.fastwebnet.it. [93.34.218.143]) by mx.google.com with ESMTPS id n30sm22979754ibl.4.2011.10.20.04.16.42 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 20 Oct 2011 04:16:44 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Thu, 20 Oct 2011 13:16:20 +0200 Message-Id: <1319109385-7927-3-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.7.6 In-Reply-To: <1319109385-7927-1-git-send-email-pbonzini@redhat.com> References: <1319109385-7927-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.210.173 Cc: kwolf@redhat.com, stefanha@linux.vnet.ibm.com Subject: [Qemu-devel] [PATCH v2 2/7] vmdk: clean up open X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Move vmdk_parent_open to vmdk_open. There's another path how vmdk_parent_open can be reached: vmdk_parse_extents() -> vmdk_open_sparse() -> vmdk_open_vmdk4() -> vmdk_open_desc_file(). If that can happen, however, the code is bogus. vmdk_parent_open reads from bs->file: if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) { but it is always called with s->desc_offset == 0 and with the same bs->file. So the data that vmdk_parent_open reads comes always from the same place, and anyway there is only one place where it can write it, namely bs->backing_file. So, if it cannot happen, the patched code is okay. It is also possible that the recursive call can happen, but only once. In that case there would still be a bug in vmdk_open_desc_file setting s->desc_offset = 0, but the patched code is okay. Finally, in the case where multiple recursive calls can happen the code would need to be rewritten anyway. It is likely that this would anyway involve adding several parameters to vmdk_parent_open, and calling it from vmdk_open_vmdk4. Signed-off-by: Paolo Bonzini --- block/vmdk.c | 37 +++++++++++++++---------------------- 1 files changed, 15 insertions(+), 22 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index ea00938..ace2977 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -624,20 +624,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, return -ENOTSUP; } s->desc_offset = 0; - ret = vmdk_parse_extents(buf, bs, bs->file->filename); - if (ret) { - vmdk_free_extents(bs); - return ret; - } - - /* try to open parent images, if exist */ - ret = vmdk_parent_open(bs); - if (ret) { - vmdk_free_extents(bs); - return ret; - } - s->parent_cid = vmdk_read_cid(bs, 1); - return 0; + return vmdk_parse_extents(buf, bs, bs->file->filename); } static int vmdk_open(BlockDriverState *bs, int flags) @@ -647,17 +634,23 @@ static int vmdk_open(BlockDriverState *bs, int flags) if (vmdk_open_sparse(bs, bs->file, flags) == 0) { s->desc_offset = 0x200; - /* try to open parent images, if exist */ - ret = vmdk_parent_open(bs); + } else { + ret = vmdk_open_desc_file(bs, flags, 0); if (ret) { - vmdk_free_extents(bs); - return ret; + goto fail; } - s->parent_cid = vmdk_read_cid(bs, 1); - return 0; - } else { - return vmdk_open_desc_file(bs, flags, 0); } + /* try to open parent images, if exist */ + ret = vmdk_parent_open(bs); + if (ret) { + goto fail; + } + s->parent_cid = vmdk_read_cid(bs, 1); + return ret; + +fail: + vmdk_free_extents(bs); + return ret; } static int get_whole_cluster(BlockDriverState *bs,