Re: [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU

[Avi Kivity]

On 03/30/2010 04:54 PM, Anthony Liguori wrote:
> On 03/30/2010 08:28 AM, Avi Kivity wrote:
> > > But that offers no advantage to what we have which fails the 
> > > proof-by-example that threading makes the situation better. 
> >
> >
> > It has an advantage, qcow2 is currently synchronous in parts:
> >
> > block/qcow2-cluster.c:    ret = bdrv_write(s->hd, (cluster_offset >> 
> > 9) + n_start,
> > block/qcow2-cluster.c:                    ret = 
> bdrv_read(bs->backing_hd, sector_num, buf, n1);
>
> The two of these happen in copy_sectors().  copy_sectors() runs from 
> qcow2_alloc_cluster_link_l2() which is called from qcow_aio_write_cb() 
> and preallocate().
>
> > block/qcow2.c:        bdrv_write(s->hd, (meta.cluster_offset >> 9) + 
> > num - 1, buf, 1);
>
> This only happens during creation (for preallocation).
>
> > block/qcow2.c:        bdrv_write(bs, sector_num, buf, 
> > s->cluster_sectors);
> > block/qcow2-cluster.c:        ret = bdrv_read(s->hd, coffset >> 9, 
> > s->cluster_data, nb_csectors);
>
> These two are only for compressed images.

Are compressed images supposed to block vcpus?

> So it looks like we really only have one operation 
> (qcow2_alloc_cluster_link_l2) that blocks.  Do we really think that 
> it's sufficiently difficult to make this function asynchronous that it 
> justifies threading the block layer?

There are also tons of bdrv_pread()s and bdrv_pwrite()s.  Isn't growing 
some of the tables synchronous? how about creating a snapshot?

If it were just one operation in qcow2, threading would be a big 
overkill.  But threading also fixes all of the other format drivers, and 
makes working on qcow2 easier.

--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.