[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2 13/17] vmdk: Implement .bdrv_co_pwritev() int
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH v2 13/17] vmdk: Implement .bdrv_co_pwritev() interface |
Date: |
Fri, 29 Apr 2016 09:41:28 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 29.04.2016 um 05:08 hat Fam Zheng geschrieben:
> On Thu, 04/28 15:16, Kevin Wolf wrote:
> > +typedef struct VmdkWriteCompressedCo {
> > + BlockDriverState *bs;
> > + int64_t sector_num;
> > + const uint8_t *buf;
> > + int nb_sectors;
> > + int ret;
> > +} VmdkWriteCompressedCo;
> > +
> > +static void vmdk_co_write_compressed(void *opaque)
> > +{
> > + VmdkWriteCompressedCo *co = opaque;
> > + QEMUIOVector local_qiov;
> > + uint64_t offset = co->sector_num * BDRV_SECTOR_SIZE;
> > + uint64_t bytes = co->nb_sectors * BDRV_SECTOR_SIZE;
> > +
> > + struct iovec iov = (struct iovec) {
> > + .iov_base = (uint8_t*) co->buf,
> > + .iov_len = bytes,
> > + };
> > + qemu_iovec_init_external(&local_qiov, &iov, 1);
> > +
> > + co->ret = vmdk_pwritev(co->bs, offset, bytes, &local_qiov, false,
> > false);
>
> Should it acquire s->lock?
Yes, probably, if only for consistency.
It didn't acquire the lock before, so it didn't seem appropriate to add
it in this patch. Not taking the lock is fine because this function is
only called by qemu-img, where there is no concurrency.
There was a proposal recently to allow compressed writes during a normal
VM run. If we want to enable this, the locking would be needed. (The
same is true for qcow2 and qcow2.)
> > +}
> > +
> > static int vmdk_write_compressed(BlockDriverState *bs,
> > int64_t sector_num,
> > const uint8_t *buf,
> > int nb_sectors)
> > {
> > BDRVVmdkState *s = bs->opaque;
> > +
> > if (s->num_extents == 1 && s->extents[0].compressed) {
> > - return vmdk_write(bs, sector_num, buf, nb_sectors, false, false);
> > + Coroutine *co;
> > + AioContext *aio_context = bdrv_get_aio_context(bs);
> > + VmdkWriteCompressedCo data = {
> > + .bs = bs,
> > + .sector_num = sector_num,
> > + .buf = buf,
> > + .nb_sectors = nb_sectors,
> > + .ret = -EINPROGRESS,
> > + };
> > + co = qemu_coroutine_create(vmdk_co_write_compressed);
> > + qemu_coroutine_enter(co, &data);
> > + while (data.ret == -EINPROGRESS) {
> > + aio_poll(aio_context, true);
> > + }
> > + return data.ret;
>
> Don't you have a plan to make the creation of coroutine for compressed write
> in
> in block layer? Or will bdrv_co_pwritev gain a "compressed" flag
> (BDRV_REQ_COMPRESSED) in the future?
Well, there was this recent discussion, but personally I don't have any
plans. If we want to do compressed writes from running VMs, we will need
the conversion because we must not block the vcpu.
Or maybe the flag would actually be the nicer option, yes. I don't have
a strong opinion on this yet.
Kevin
- [Qemu-block] [PATCH v2 10/17] vdi: Implement .bdrv_co_pwritev() interface, (continued)
- [Qemu-block] [PATCH v2 10/17] vdi: Implement .bdrv_co_pwritev() interface, Kevin Wolf, 2016/04/28
- [Qemu-block] [PATCH v2 11/17] vmdk: Add vmdk_find_offset_in_cluster(), Kevin Wolf, 2016/04/28
- [Qemu-block] [PATCH v2 15/17] vpc: Implement .bdrv_co_pwritev() interface, Kevin Wolf, 2016/04/28
- [Qemu-block] [PATCH v2 17/17] block: Remove BlockDriver.bdrv_read/write, Kevin Wolf, 2016/04/28
- [Qemu-block] [PATCH v2 12/17] vmdk: Implement .bdrv_co_preadv() interface, Kevin Wolf, 2016/04/28
- [Qemu-block] [PATCH v2 14/17] vpc: Implement .bdrv_co_preadv() interface, Kevin Wolf, 2016/04/28
- [Qemu-block] [PATCH v2 16/17] vvfat: Implement .bdrv_co_preadv/pwritev interfaces, Kevin Wolf, 2016/04/28
- [Qemu-block] [PATCH v2 13/17] vmdk: Implement .bdrv_co_pwritev() interface, Kevin Wolf, 2016/04/28
Re: [Qemu-block] [PATCH v2 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev, Fam Zheng, 2016/04/28
Re: [Qemu-block] [PATCH v2 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev, Kevin Wolf, 2016/04/29