From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35458) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UWroI-0000CG-E9 for qemu-devel@nongnu.org; Mon, 29 Apr 2013 13:24:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UWroD-0003UL-Up for qemu-devel@nongnu.org; Mon, 29 Apr 2013 13:24:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12590) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UWroD-0003S4-Ec for qemu-devel@nongnu.org; Mon, 29 Apr 2013 13:24:13 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r3THOCAs030041 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 29 Apr 2013 13:24:12 -0400 Date: Mon, 29 Apr 2013 13:24:10 -0400 From: Jeff Cody Message-ID: <20130429172410.GD3525@localhost.localdomain> References: <56a0e0347fde5396fb6b2a260de5b1a867a588d6.1366726446.git.jcody@redhat.com> <20130428095855.GA28366@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130428095855.GA28366@localhost.localdomain> Subject: Re: [Qemu-devel] [PATCH v2 3/5] block: initial VHDX driver support framework - supports open and probe List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, kwolf@redhat.com, stefanha@redhat.com On Sun, Apr 28, 2013 at 05:58:55PM +0800, Fam Zheng wrote: > On Tue, 04/23 10:24, Jeff Cody wrote: > > +/* opens the specified header block from the VHDX file header section */ > > +static int vhdx_parse_header(BlockDriverState *bs, BDRVVHDXState *s) > > +{ > > + int ret = 0; > > + vhdx_header *header1; > > + vhdx_header *header2; > > + uint64_t h1_seq = 0; > > + uint64_t h2_seq = 0; > > + uint8_t *buffer; > > + > > + header1 = qemu_blockalign(bs, sizeof(vhdx_header)); > > + header2 = qemu_blockalign(bs, sizeof(vhdx_header)); > > + > > + buffer = qemu_blockalign(bs, VHDX_HEADER_SIZE); > > + > > + s->headers[0] = header1; > > + s->headers[1] = header2; > > + > Logic for header1 and header2 are completely the same, IMO it might be > better to avoid code with a loop. > > + /* We have to read the whole VHDX_HEADER_SIZE instead of > > + * sizeof(vhdx_header), because the checksum is over the whole > > + * region */ > > + ret = bdrv_pread(bs->file, VHDX_HEADER1_OFFSET, buffer, VHDX_HEADER_SIZE); > > + if (ret < 0) { > > + goto fail; > > + } > > + /* copy over just the relevant portion that we need */ > > + memcpy(header1, buffer, sizeof(vhdx_header)); > > + vhdx_header_le_import(header1); > > + > > + if (vhdx_checksum_is_valid(buffer, VHDX_HEADER_SIZE, 4) && > > + header1->signature == VHDX_HDR_MAGIC) { > > + h1_seq = header1->sequence_number; > > + } > Do we need to check the version here? As the specification page 15 says: > > The Version field specifies the version of the VHDX format used > within the VHDX file. This field must be set to 1. If it is not, a > parser must not attempt to parse the file using the details from > this format specification. > Yes, you are correct - I will add that in for v3. > > + > > + ret = bdrv_pread(bs->file, VHDX_HEADER2_OFFSET, buffer, VHDX_HEADER_SIZE); > > + if (ret < 0) { > > + goto fail; > > + } > > + /* copy over just the relevant portion that we need */ > > + memcpy(header2, buffer, sizeof(vhdx_header)); > > + vhdx_header_le_import(header2); > > + > > + if (vhdx_checksum_is_valid(buffer, VHDX_HEADER_SIZE, 4) && > > + header2->signature == VHDX_HDR_MAGIC) { > > + h2_seq = header2->sequence_number; > > + } > Same as above. > > + > > + if (h1_seq > h2_seq) { > > + s->curr_header = 0; > > + } else if (h2_seq > h1_seq) { > > + s->curr_header = 1; > > + } else { > > + printf("NO VALID HEADER\n"); > > + ret = -EINVAL; > > + goto fail; > > + } > > + > > + ret = 0; > > + > > + goto exit; > > + > > +fail: > > + qemu_vfree(header1); > > + qemu_vfree(header2); > > + s->headers[0] = NULL; > > + s->headers[1] = NULL; > > +exit: > > + qemu_vfree(buffer); > > + return ret; > > +} > > -- > Fam