[Qemu-devel] Re: [PATCH v2 6/7] qed: Read/write support

[Avi Kivity <avi@redhat.com>]

  On 10/08/2010 05:48 PM, Stefan Hajnoczi wrote:
> This patch implements the read/write state machine.  Operations are
> fully asynchronous and multiple operations may be active at any time.
>
> Allocating writes lock tables to ensure metadata updates do not
> interfere with each other.  If two allocating writes need to update the
> same L2 table they will run sequentially.  If two allocating writes need
> to update different L2 tables they will run in parallel.
>

Shouldn't there be a flush between an allocating write and an L2 
update?  Otherwise a reuse of a cluster can move logical sectors from 
one place to another, causing a data disclosure.

Can be skipped if the new cluster is beyond the physical image size.

> +
> +/*
> + * Table locking works as follows:
> + *
> + * Reads and non-allocating writes do not acquire locks because they do not
> + * modify tables and only see committed L2 cache entries.

What about a non-allocating write that follows an allocating write?

1 Guest writes to sector 0
2 Host reads backing image (or supplies zeros), sectors 1-127
3 Host writes sectors 0-127
4 Guest writes sector 1
5 Host writes sector 1

There needs to be a barrier that prevents the host and the disk from 
reordering operations 3 and 5, or guest operation 4 is lost.  As far as 
the guest is concerned no overlapping writes were issued, so it isn't 
required to provide any barriers.

(based on the comment only, haven't read the code)

-- 
error compiling committee.c: too many arguments to function