Commit d62d9dc4 lifted streamOptimized images's version to 3, but we
now refuse to open version 3 images read-write.  We need to make
streamOptimized an exception to allow converting to it. This fixes the
accidentally broken iotests case 059 for the same reason.
Signed-off-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Signed-off-by: NMax Reitz <mreitz@redhat.com>

VMware products accept only version 3 for streamOptimized, let's bump
the version.
Reported-by: NRadoslav Gerganov <rgerganov@vmware.com>
Signed-off-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

vmdk_parse_extents() reports parse errors like this:

    error_setg(errp, "Invalid extent lines:\n%s", p);

where p points to the beginning of the malformed line in the image
descriptor.  This results in a multi-line error message

    Invalid extent lines:
    <first line that doesn't parse>
    <remaining text that may or may not parse, if any>

Error messages should not have newlines embedded.  Since the remaining
text is not helpful, we can simply report:

    Invalid extent line: <first line that doesn't parse>

Cc: Fam Zheng <famz@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-19-git-send-email-armbru@redhat.com>
Reviewed-by: NFam Zheng <famz@redhat.com>

Factor out loop stepping to turn a while-loop with goto into a
for-loop with continue.

Cc: Fam Zheng <famz@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Message-Id: <1450452927-8346-18-git-send-email-armbru@redhat.com>

Commit 6daf194d, be62a2eb and 312fd5f2 got rid of a bunch, but they
keep coming back.  Tracked down with the Coccinelle semantic patch
from commit 312fd5f2.

Cc: Fam Zheng <famz@redhat.com>
Cc: Peter Crosthwaite <crosthwaitepeter@gmail.com>
Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
Cc: Dominik Dingel <dingel@linux.vnet.ibm.com>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Changchun Ouyang <changchun.ouyang@intel.com>
Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>
Cc: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: NMarkus Armbruster <armbru@pond.sub.org>
Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: NCornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: NBharata B Rao <bharata@linux.vnet.ibm.com>
Acked-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Message-Id: <1450452927-8346-17-git-send-email-armbru@redhat.com>

We have two issues with our qapi union layout:
1) Even though the QMP wire format spells the tag 'type', the
C code spells it 'kind', requiring some hacks in the generator.
2) The C struct uses an anonymous union, which places all tag
values in the same namespace as all non-variant members. This
leads to spurious collisions if a tag value matches a non-variant
member's name.

Make the conversion to the new layout for block-related code.
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-16-git-send-email-eblake@redhat.com>
[Commit message tweaked slightly]
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

This is the final step in converting all of the BlockDriverState
pointers that block drivers use to BdrvChild.

After this patch, bs->children contains the full list of child nodes
that are referenced by a given BDS, and these children are only
referenced through BdrvChild, so that updating the pointer in there is
enough for changing edges in the graph.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NAlberto Garcia <berto@igalia.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>

This patch removes the temporary duplication between bs->file and
bs->file_child by converting everything to BdrvChild.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NAlberto Garcia <berto@igalia.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NAlberto Garcia <berto@igalia.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NJeff Cody <jcody@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>

When the VMDK is streamOptimized (or compressed), the
next_cluster_sector must not be incremented by a fixed number of
sectors. Instead of this, it must be rounded up to the next consecutive
sector. Fixing this results in much smaller compressed images.
Signed-off-by: NRadoslav Gerganov <rgerganov@vmware.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Now that this parameter is effectively unused, we can drop it and just
pass NULL on to bdrv_open_inherit().
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NAlberto Garcia <berto@igalia.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

In particular, don't include it into headers.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>

These macros expand into error class enumeration constant, comma,
string.  Unclean.  Has been that way since commit 13f59ae8.

The error class is always ERROR_CLASS_GENERIC_ERROR since the previous
commit.

Clean up as follows:

* Prepend every use of a QERR_ macro by ERROR_CLASS_GENERIC_ERROR, and
  delete it from the QERR_ macro.  No change after preprocessing.

* Rewrite error_set(ERROR_CLASS_GENERIC_ERROR, ...) into
  error_setg(...).  Again, no change after preprocessing.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>

When reopening an image, the block layer already takes care to reopen
bs->file as well with recalculated inherited flags. The same must happen
for any other child (most notably missing before this patch: backing
files).

If bs->file (or any other child) didn't originally inherit from bs, e.g.
because it was created separately and then only referenced, it must not
inherit flags on reopen either, so check the inherited_from field before
propagation the reopen down.

VMDK already reopened its extents manually; this code can now be
dropped.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Instead of letting every caller of bdrv_open() determine the right flags
for its child node manually and pass them to the function, pass the
parent node and the role of the newly opened child (like backing file,
protocol layer, etc.).
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>

Besides standardising on a single interface for opening child nodes,
this patch allows the user to specify options to individual extent
nodes. Overriding file names isn't possible with this yet, so it's of
limited usefulness, but still a step forward.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NJeff Cody <jcody@redhat.com>

Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

It has the similar issue with b1649fae. Since the calculation
is repeated for a few times already, introduce a function so it can be
reused.
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Richard Jones caught this bug with afl fuzzer.

In fact, that's the only possible value to overflow (extent->l1_size =
0x20000000) l1_size:

l1_size = extent->l1_size * sizeof(long) => 0x80000000;

g_try_malloc returns NULL because l1_size is interpreted as negative
during type casting from 'int' to 'gsize', which yields a enormous
value. Hence, by coincidence, we get a "not too bad" behavior:

qemu-img: Could not open '/tmp/afl6.img': Could not open
'/tmp/afl6.img': Cannot allocate memory

Values larger than 0x20000000 will be refused by the validation in
vmdk_add_extent.

Values smaller than 0x20000000 will not overflow l1_size.

Cc: qemu-stable@nongnu.org
Reported-by: NRichard W.M. Jones <rjones@redhat.com>
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Tested-by: NRichard W.M. Jones <rjones@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

This fixes the bug introduced by commit c6ac36e1 (vmdk: Optimize cluster
allocation).

Sometimes, write_len could be larger than cluster size, because it
contains both data and marker.  We must advance next_cluster_sector in
this case, otherwise the image gets corrupted.

Cc: qemu-stable@nongnu.org
Reported-by: NAntoni Villalonga <qemu-list@friki.cat>
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Coverity spotted this.

The field is 32 bits, but if it's possible to overflow in 32 bit
left shift.
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NJohn Snow <jsnow@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

There are several error messages that identify a BlockDriverState by
its device name. However those errors can be produced in nodes that
don't have a device name associated.

In those cases we should use bdrv_get_device_or_node_name() to fall
back to the node name and produce a more meaningful message. The
messages are also updated to use the more generic term 'node' instead
of 'device'.
Signed-off-by: NAlberto Garcia <berto@igalia.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Message-id: 9823a1f0514fdb0692e92868661c38a9e00a12d6.1428485266.git.berto@igalia.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The size compared should be PATH_MAX, rather than sizeof(char *).
Reported-by: NPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: NJeff Cody <jcody@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Message-id: 46d873261433f4527e88885582f96942d61758d6.1423592487.git.jcody@redhat.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Functions 'vmdk_parse_extents' and 'vmdk_create' allocate several
PATH_MAX sized arrays on the stack.  Make these dynamically allocated.
Signed-off-by: NJeff Cody <jcody@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Keep the variable 'ret' something that is returned by the function it is
defined in.  For the return value of 'sscanf', use a more meaningful
variable name.
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: NJohn Snow <jsnow@redhat.com>
Signed-off-by: NJeff Cody <jcody@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

When a vmdk image is created with a backing file, it is opened to check
whether it is indeed a vmdk file by letting qemu probe it. When doing
so, the backing filename is relative to the image's base directory so it
should be interpreted accordingly.
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

If vmdk blindly tries to use path_combine() using bs->file->filename as
the base file name, this will result in a bad error message for JSON
file names when calling bdrv_open(). It is better to only try
bs->file->exact_filename; if that is empty, bs->file->filename will be
useless for path_combine() and an error should be emitted (containing
bs->file->filename because desc_file_path (which is
bs->file->exact_filename) is empty).
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Message-id: 1417615043-26174-2-git-send-email-mreitz@redhat.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Reported-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NDon Koch <dkoch@verizon.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Message-id: 1417649314-13704-7-git-send-email-famz@redhat.com
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

It will be assigned to the return value of vmdk_read_desc.
Suggested-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NDon Koch <dkoch@verizon.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Message-id: 1417649314-13704-6-git-send-email-famz@redhat.com
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Since a too small file cannot be a valid VMDK image, and also since the
buffer's first 4 bytes will be unconditionally examined by
vmdk_open_sparse, let's error out the small file case to be clear.
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NDon Koch <dkoch@verizon.com>
Message-id: 1417649314-13704-5-git-send-email-famz@redhat.com
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Zeroing a buffer that will be filled right after is not necessary, and
allocating a power of two + 1 is naughty.
Suggested-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NDon Koch <dkoch@verizon.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Message-id: 1417649314-13704-4-git-send-email-famz@redhat.com
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

commit 04d542c8 (vmdk: support vmfs files) added support of VMFS extent
type but the comment above the changed code is left out. Update the
comment so they are consistent.
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NDon Koch <dkoch@verizon.com>
Message-id: 1417649314-13704-3-git-send-email-famz@redhat.com
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

This replaces two "time(NULL)" invocations with "g_random_int()".
According to VMDK spec, CID "is a random 32‐bit value updated the first
time the content of the virtual disk is modified after the virtual disk
is opened". Using "seconds since epoch" is just a "lame way" to generate
it, and not completely safe because of the low precision.
Suggested-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NDon Koch <dkoch@verizon.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Message-id: 1417649314-13704-2-git-send-email-famz@redhat.com
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

When extent types don't match, we return -ENOTSUP. In this case, be
polite to the caller and don't modify bdi.
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Message-id: 1415938161-16217-1-git-send-email-famz@redhat.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

device_name[] can become non-empty only in bdrv_new_root() and
bdrv_move_feature_fields().  The latter is used only to undo damage
done by bdrv_swap().  The former is called only by blk_new_with_bs().
Therefore, when a BlockDriverState's device_name[] is non-empty, then
it's been created with a BlockBackend, and vice versa.  Furthermore,
blk_new_with_bs() keeps the two names equal.

Therefore, device_name[] is redundant.  Eliminate it.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

This fixes the bug introduced by commit c6ac36e1 (vmdk: Optimize cluster
allocation).

$ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k'
write failed: Invalid argument
Reported-by: NMark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NFam Zheng <famz@redhat.com>
Message-id: 1411437381-11234-1-git-send-email-famz@redhat.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Currently the file size requested by user is rounded down to nearest
sector, causing the actual file size could be a bit less than the size
user requested. Since some formats (like qcow2) record virtual disk
size in bytes, this can make the last few bytes cannot be accessed.

This patch fixes it by rounding up file size to nearest sector so that
the actual file size is no less than the requested file size.
Signed-off-by: NHu Tao <hutao@cn.fujitsu.com>
Reviewed-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

vmdk_open_sparse() does not take ownership of buf so the caller always
needs to free it.
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NFam Zheng <famz@redhat.com>