[Qemu-devel] Re: [PATCH v3 1/2] Add bdrv_aio_multiwrite

[Kevin Wolf]

Am 10.09.2009 01:08, schrieb Juan Quintela:
> Kevin Wolf <address@hidden> wrote:
> 
> Hi again
> 
> 
>> +            QEMUIOVector *qiov = qemu_mallocz(sizeof(*qiov));
>> +            qemu_iovec_init(qiov,
>> +                reqs[outidx].qiov->niov + reqs[i].qiov->niov + 1);
>> +
>> +            // Add the first request to the merged one. If the requests are
>> +            // overlapping, drop the last sectors of the first request.
>> +            size = (reqs[i].sector - reqs[outidx].sector) << 9;
>> +            qemu_iovec_concat(qiov, reqs[outidx].qiov, size);
>> +
>> +            // We might need to add some zeros between the two requests
>> +            if (reqs[i].sector > oldreq_last) {
>> +                size_t zero_bytes = (reqs[i].sector - oldreq_last) << 9;
>> +                uint8_t *buf = qemu_blockalign(bs, zero_bytes);
>> +                memset(buf, 0, zero_bytes);
>> +                qemu_iovec_add(qiov, buf, zero_bytes);
>> +                mcb->callbacks[i].free_buf = buf;
>> +            }
>> +
>> +            // Add the second request
>> +            qemu_iovec_concat(qiov, reqs[i].qiov, reqs[i].qiov->size);
>> +
>> +            reqs[outidx].nb_sectors += reqs[i].nb_sectors;
>> +            reqs[outidx].qiov = qiov;
> 
> What frees reqs[outidx].qiov previous value, or new one for that matter?

Your decision where to stop quoting the code is kind of funny. The very
next line is:

mcb->callbacks[i].free_qiov = reqs[outidx].qiov;

This saves each newly allocated qiov. It is later freed in
multiwrite_user_cb. To free the original value of qiov is the task of
the caller.

> I can't see where it is done.  As far as I can see, we are losing both
> the ones that we are overwritten and the ones for the request that got merged.

Hope you see them now. :-)

Kevin