Re: [Qemu-devel] [PATCH v4 4/6] introduce new vma archive format

[Eric Blake]

On 02/20/2013 02:32 AM, Dietmar Maurer wrote:
> This is a very simple archive format, see docs/specs/vma_spec.txt
> 
> Signed-off-by: Dietmar Maurer <address@hidden>
> ---

> +++ b/docs/specs/vma_spec.txt
> @@ -0,0 +1,24 @@
> +=Virtual Machine Archive format (VMA)=
> +
> +This format contains a header which includes the VM configuration as
> +binary blobs, and a list of devices (dev_id, name).

Is there a magic number, for quickly identifying whether a file is
likely to be a vma?  What endianness are multi-byte numbers interpreted
with?  Does the overall header file leave ample room for adding later
extensions in a manner that is reliably detected as an unsupported
feature in older tools?

> +
> +The actual VM image data is stored inside extents. An extent contains
> +up to 64 clusters, and start with a 512 byte header containing
> +additional information for those clusters.

Doesn't this create alignment slowdowns on modern disks that prefer 4k
alignment?  Wouldn't it be better to have the header be 4096 bytes, so
that each of the 64 clusters in an extent is also 4096-aligned?  If you
_do_ go with a 4096 header per extent, then it might be better to go
with 16 bytes per cluster and 256 clusters per extent, instead of 8
bytes per cluster and 512 clusters per extent.

> +
> +We use a cluster size of 65536, and use 8 bytes for each
> +cluster in the header to store the following information:
> +
> +* 1 byte dev_id (to identity the drive)
> +* 2 bytes zero indicator (mark zero regions (16x4096))
> +* 4 bytes cluster number

Is that sufficient, or are we artificially limiting the maximum size of
a disk image that can be stored to 64k*4G = 128T? Again, going with
16-bytes per cluster instead of 8 bytes per cluster, to get up to a
4096-byte header alignment so that all clusters fall on nice alignment
boundaries, would leave you room to supply a 64-bit offset instead of a
32-bit cluster number.  Don't know if that would be helpful or not, but
food for thought.

> +* 1 byte not used (reserved)

Can these be rearranged to 'dev_id, reserved, zero indicator, cluster
number' to achieve natural alignment when reading the cluster number?

> +
> +We only store non-zero blocks (such block is 4096 bytes).

So if I understand correctly, your current layout is divided into
extents of  up to 4194816 bytes each (512 header, then 4M divided into
64 clusters of 64k each).  Then, if the zero indicator is all 0, then
the corresponding cluster will be 64k bytes on the wire; if it is
0x0001, then the first 4096 bytes of the corresponding cluster will be
all zeros, and the cluster itself will occupy only 60k of the vma file?

> +
> +Each archive is marked with a uuid. The archive header and all
> +extent headers includes that uuid and a MD5 checksum (over header
> +data).

Layout of this header?

Hint - look at how the qcow2 file format is specified.  You need a lot
more details - enough that someone could independently implement a
program to read and create vma images that would be compatible with what
your implementation produces.

> +++ b/vma-reader.c
> @@ -0,0 +1,799 @@
> +/*
> + * VMA: Virtual Machine Archive
> + *
> + * Copyright (C) 2012 Proxmox Server Solutions

It's 2013.

> +
> +#define BITS_PER_LONG  (sizeof(unsigned long) * 8)

8 is a magic number; you should be using CHAR_BIT from limits.h instead.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature