Double semicolons should be single.
Signed-off-by: NDong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Now that bdrv_co_is_allocated() is available we can use it instead of
the synchronous bdrv_is_allocated() interface.  This is a follow-up that
Kevin Wolf <kwolf@redhat.com> pointed out after applying the series that
introduces bdrv_co_is_allocated().

It is safe to make cow_read() a coroutine_fn because its only caller is
a coroutine_fn.
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

It's common to wake up all waiting coroutines.  Introduce the
qemu_co_queue_restart_all() function to do this instead of looping over
qemu_co_queue_next() in every caller.
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The cow block driver does not keep internal state for cluster lookups.
This means it is safe to perform cluster lookups in coroutine context
without risk of race conditions that corrupt internal state.
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

It is trivial to switch from the synchronous .bdrv_is_allocated()
interface to .bdrv_co_is_allocated() since vdi_is_allocated() does not
block.
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

It is trivial to switch from the synchronous .bdrv_is_allocated()
interface to .bdrv_co_is_allocated() since vvfat_is_allocated() does not
block.
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The qcow2, qcow, and vmdk block drivers are based on coroutines.  They have a
coroutine mutex which protects internal state.  We can convert the
.bdrv_is_allocated() function to .bdrv_co_is_allocated() by holding the mutex
around the cluster lookup operation.
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The bdrv_qed_is_allocated() function is a synchronous wrapper around
qed_find_cluster(), which performs the cluster lookup.  In order to
convert the synchronous function to a coroutine function we yield
instead of using qemu_aio_wait().  Note that QED's cache is already safe
for parallel requests so no locking is needed.
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

If the bdrv_read() of the snapshot's L1 table fails, return the right
error code and make sure that the old L1 table is still loaded and we
don't break the BlockDriverState completely.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

First the snapshot must be deleted and only then the refcounts can be
decreased.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

The refcount updates must be moved so that in the worst case we can get
cluster leaks, but refcounts may never be too low.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Besides fixing the return code, this adds some comments that make clear
how the code works and that it potentially breaks images if we fail in
the wrong place. Actually fixing this is left for the next patch.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Increase refcounts only after allocating a new L1 table has succeeded in
order to make leaks less likely. If writing the snapshot table fails,
revert in-memory state to be consistent with that on disk.

While at it, make it return the real error codes instead of -1.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

sn->id_str could be leaked before this. The rest of this patch changes
comments, fixes coding style or removes checks that are unnecessary with
g_malloc.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Failing in the middle wouldn't help with the integrity of the image, so
doing everything in a single request seems better.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Doesn't immediately fix anything as the callers don't use the return
value, but they will be fixed next.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Looks better when reviewing these source files.
Signed-off-by: NDong Xu Wang <wdongxu@linux.vnet.ibm.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Since common file operation functions lack of error detection,
so change them to bdrv series functions.
Signed-off-by: NLi Zhi Hui <zhihuili@linux.vnet.ibm.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

This patch is only to refactor some lines of codes to get better and more robust codes.

As you have seen, in qed_read_table_cb() it's nice to
use qiov->size because that function doesn't obviously use a single
struct iovec.

In other two functions, if qiov use more than one struct iovec, the existing way will get wrong nb_sectors.
To make the code more robust, it will be nicer to refactor the existing way as below.
Signed-off-by: NZhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Acked-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

A BlockDriverState should not issue requests on itself through the
public block layer interface.  Nested, or reentrant, requests are
problematic because they do I/O throttling and request tracking twice.

Features like block layer copy-on-read use request tracking to avoid
race conditions between concurrent requests.  The reentrant request will
have to "wait" for its parent request to complete.  But the parent is
waiting for the reentrant request to make progress so we have reached
deadlock.

The solution is for block drivers to avoid the public block layer
interfaces for reentrant requests.   Instead they should call their own
internal functions if they wish to perform reentrant requests.

This is also a good opportunity to make copy_sectors() a true
coroutine_fn.  That means calling bdrv_co_writev() instead of
bdrv_write().  Behavior is unchanged but we're being explicit that this
executes in coroutine context.
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Unlocking during COW allows for more parallelism. One change it requires is
that buffers are dynamically allocated instead of just using a per-image
buffer.

While touching the code, drop the synchronous qcow2_read() function and replace
it by a bdrv_read() call.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: NDong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@linux.vnet.ibm.com>

The block map is allocated in vdi_open, but was never freed.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

vvfat caches more or less everything when in writable mode. For migration
to work, it would have to be invalidated. Block migration for now when
in writable mode (default is readonly).
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

vpc caches the BAT. For migration to work, it would have to be
invalidated. Block migration for now.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

VMDK caches L2 tables. For migration to work, they would have to be
invalidated. Block migration for now.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

vdi caches the block map. For migration to work, it would have to be
invalidated. Block migration for now.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

qcow caches L2 tables. For migration to work, they would have to be
invalidated. Block migration for now.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

s->lock should be unlocked before leaving add_aio_request.
Signed-off-by: NDong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

zlib.h is not a local include file, therefore it should be included
using <> instead of "".
Signed-off-by: NStefan Weil <sw@weilnetz.de>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Now when you try to migrate with qed, you get:

(qemu) migrate tcp:localhost:1025
Block format 'qed' used by device 'ide0-hd0' does not support feature 'live migration'
(qemu)
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

We don't reopen the actual file, but instead invoke the close and open routines.
We specifically ignore the backing file since it's contents are read-only and
therefore immutable.
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

qcow2 has a writeback metadata cache, so flushing a qcow2 image actually
consists of writing back that cache to the protocol and only then flushes the
protocol in order to get everything stable on disk.

This introduces a separate bdrv_co_flush_to_os to reflect the split.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

There are two different types of flush that you can do: Flushing one level up
to the OS (i.e. writing data to the host page cache) or flushing it all the way
down to the disk. The existing functions flush to the disk, reflect this in the
function name.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The Data Offset field in the Dynamic Disk Header is an 8 byte field.
Although the specification (2006-10-11) gives an example of initializing
only the first 4 bytes, images generated by Microsoft on Windows initialize
all 8 bytes.

Failure to initialize all 8 bytes results in errors from utilities
like Citrix's vhd-util which checks specifically for the proper Data
Offset field initialization.
Signed-off-by: NCharles Arnold <carnold@suse.com>
Reviewed-by: NAndreas Färber <afaerber@suse.de>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

vvfat used to directly call into the qcow2 block driver instead of using the
block.c wrappers. With the coroutine conversion, this stopped working.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>

First determine FAT12/16/32, then compute geometry from that for both
FDD and HDD.  For 1.44MB floppies, and 2.88MB floppies using FAT16,
change to 1 sector/cluster.  The default remains 2.88MB with FAT12
and 2 sectors/cluster.  Both DOS and mkdosfs by default format a 2.88MB
floppy as FAT12.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>