From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0Vls-0005Cd-NR for qemu-devel@nongnu.org; Mon, 15 Dec 2014 08:33:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y0Vlm-0004nz-LV for qemu-devel@nongnu.org; Mon, 15 Dec 2014 08:33:08 -0500 Received: from relay.parallels.com ([195.214.232.42]:54429) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0Vlm-0004nl-EM for qemu-devel@nongnu.org; Mon, 15 Dec 2014 08:33:02 -0500 Message-ID: <548EE300.6050903@openvz.org> Date: Mon, 15 Dec 2014 16:32:48 +0300 From: "Denis V. Lunev" MIME-Version: 1.0 References: <1418632081-20667-1-git-send-email-den@openvz.org> <1418632081-20667-15-git-send-email-den@openvz.org> <20141215124544.GI4411@noname.str.redhat.com> In-Reply-To: <20141215124544.GI4411@noname.str.redhat.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 14/16] block/parallels: introduce ParallelsSnapshot data structure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Jeff Cody , qemu-devel@nongnu.org, Stefan Hajnoczi On 15/12/14 15:45, Kevin Wolf wrote: > Am 15.12.2014 um 09:27 hat Denis V. Lunev geschrieben: >> In order to support snapshots of parallels images we should maintain >> snapshots list in BDRVParallelsState. Snapshots actually forms tree. >> Though, in read-only case, we do need to traverse only from current >> snapshot leaf to the snapshot tree root. Thus interesting for us >> snapshots forms old good linked list. >> >> This patch just introduces the structure for this and fills it with >> a signle image as was done before. > s/signle/single/ > >> True parsing be done in the next patch. >> >> Signed-off-by: Denis V. Lunev >> CC: Jeff Cody >> CC: Kevin Wolf >> CC: Stefan Hajnoczi > If I understand correctly, this is what actually describes the backing > file relationship. We should probably use the normal infrastructure for > this. > > The challenge here seems to be that the single descriptor XML file > describes the complete chain of backing files. This is different from > the image formats that we support until now. > > I think we need some design discussion here first before we even look at > code. Did you consider making the snapshots regular backing files, and > if so, what were the reasons that let you prefer a purely internal > solution? > > Kevin This implementation is borrowed from the current VMDK support. The idea is exactly the same, see below. I have taken this as a source of architecture approach as format is quite similar. Anyway, I am very open to a discussion and solid architecture approach here would be very good. Regards, Den static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename) { magic = be32_to_cpu(*(uint32_t *)buf); if (magic == VMDK3_MAGIC || magic == VMDK4_MAGIC) { return 100; } else { /* test descriptor parsing */ } } static int vmdk_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { char *buf; int ret; BDRVVmdkState *s = bs->opaque; uint32_t magic; buf = vmdk_read_desc(bs->file, 0, errp); if (!buf) { return -EINVAL; } magic = ldl_be_p(buf); switch (magic) { case VMDK3_MAGIC: case VMDK4_MAGIC: ret = vmdk_open_sparse(bs, bs->file, flags, buf, errp); s->desc_offset = 0x200; break; default: ret = vmdk_open_desc_file(bs, flags, buf, errp); break; } if (ret) { goto fail; } ... static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf, Error **errp) { .... ret = vmdk_parse_extents(buf, bs, bs->file->filename, errp); .... } static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, const char *desc_file_path, Error **errp) { .... while (*p) { .... path_combine(extent_path, sizeof(extent_path), desc_file_path, fname); extent_file = NULL; ret = bdrv_open(&extent_file, extent_path, NULL, NULL, bs->open_flags | BDRV_O_PROTOCOL, NULL, errp); ... /* save to extents array */ }