Re: [Qemu-devel] [PATCH v7 04/16] qcow2: add qcow2_co_pwritev_compressed

[Pavel Butsykin]

On 08.08.2016 16:44, Kevin Wolf wrote:
> Am 22.07.2016 um 10:17 hat Denis V. Lunev geschrieben:
> > From: Pavel Butsykin <address@hidden>
> >
> > Added implementation of the qcow2_co_pwritev_compressed function that
> > will allow us to safely use compressed writes for the qcow2 from running
> > VMs.
> >
> > Signed-off-by: Pavel Butsykin <address@hidden>
> > Reviewed-by: Stefan Hajnoczi <address@hidden>
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > CC: Jeff Cody <address@hidden>
> > CC: Markus Armbruster <address@hidden>
> > CC: Eric Blake <address@hidden>
> > CC: John Snow <address@hidden>
> > CC: Stefan Hajnoczi <address@hidden>
> > CC: Kevin Wolf <address@hidden>
>
> >   /* XXX: put compressed sectors first, then all the cluster aligned
> >      tables to avoid losing bytes in alignment */
> > -static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num,
> > -                                  const uint8_t *buf, int nb_sectors)
> > +static coroutine_fn int
> > +qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
> > +                            uint64_t bytes, QEMUIOVector *qiov)
> >   {
> >       BDRVQcow2State *s = bs->opaque;
> > +    QEMUIOVector hd_qiov;
> > +    struct iovec iov;
> >       z_stream strm;
> >       int ret, out_len;
> > -    uint8_t *out_buf;
> > +    uint8_t *buf, *out_buf;
> >       uint64_t cluster_offset;
> >
> > -    if (nb_sectors == 0) {
> > +    if (bytes == 0) {
> >           /* align end of file to a sector boundary to ease reading with
> >              sector based I/Os */
> >           cluster_offset = bdrv_getlength(bs->file->bs);
> >           return bdrv_truncate(bs->file->bs, cluster_offset);
> >       }
> >
> > -    if (nb_sectors != s->cluster_sectors) {
> > +    if (bytes != s->cluster_size) {
> >           ret = -EINVAL;
> >
> >           /* Zero-pad last write if image size is not cluster aligned */
> > -        if (sector_num + nb_sectors == bs->total_sectors &&
> > -            nb_sectors < s->cluster_sectors) {
> > +        if (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS &&
> > +            bytes < s->cluster_size)
> > +        {
> >               uint8_t *pad_buf = qemu_blockalign(bs, s->cluster_size);
> >               memset(pad_buf, 0, s->cluster_size);
> > -            memcpy(pad_buf, buf, nb_sectors * BDRV_SECTOR_SIZE);
> > -            ret = qcow2_write_compressed(bs, sector_num,
> > -                                         pad_buf, s->cluster_sectors);
> > +            qemu_iovec_to_buf(qiov, 0, pad_buf, s->cluster_size);
>
> I think, bytes would be more exact than s->cluster_size here. Anyway,
> shouldn't hurt because we stop at the end of the qiov anyway and its
> size should match bytes.

Yes, I can send follow-up patches.
> > +            iov = (struct iovec) {
> > +                .iov_base   = pad_buf,
> > +                .iov_len    = s->cluster_size,
> > +            };
> > +            qemu_iovec_init_external(&hd_qiov, &iov, 1);
> > +            ret = qcow2_co_pwritev_compressed(bs, offset, bytes, &hd_qiov);
> >               qemu_vfree(pad_buf);
> >           }
> >           return ret;
> >       }
>
> Kevin