------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
*From:* Vladimir Sementsov-Ogievskiy <address@hidden>
*Sent:* Thursday, April 9, 2020 9:39 PM
*To:* Alberto Garcia <address@hidden>; Andrey Shinkevich <address@hidden>; address@hidden
<address@hidden>; address@hidden <address@hidden>
*Cc:* address@hidden <address@hidden>; address@hidden <address@hidden>; address@hidden
<address@hidden>; Denis Lunev <address@hidden>
*Subject:* Re: [PATCH v12 2/3] qcow2: Allow writing compressed data of multiple
clusters
09.04.2020 19:50, Alberto Garcia wrote:
On Mon 02 Dec 2019 01:15:05 PM CET, Andrey Shinkevich wrote:
+static coroutine_fn int
+qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset)
+{
+ BDRVQcow2State *s = bs->opaque;
+ AioTaskPool *aio = NULL;
+ int ret = 0;
+
+ if (has_data_file(bs)) {
+ return -ENOTSUP;
+ }
+
+ if (bytes == 0) {
+ /*
+ * align end of file to a sector boundary to ease reading with
+ * sector based I/Os
+ */
+ int64_t len = bdrv_getlength(bs->file->bs);
+ if (len < 0) {
+ return len;
+ }
+ return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, NULL);
+ }
+
+ if (offset_into_cluster(s, offset)) {
+ return -EINVAL;
+ }
+
+ while (bytes && aio_task_pool_status(aio) == 0) {
+ uint64_t chunk_size = MIN(bytes, s->cluster_size);
+
+ if (!aio && chunk_size != bytes) {
+ aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
+ }
+
+ ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_compressed_task_entry,
+ 0, 0, offset, chunk_size, qiov, qiov_offset,
NULL);
+ if (ret < 0) {
+ break;
+ }
+ qiov_offset += chunk_size;
+ offset += chunk_size;
+ bytes -= chunk_size;
+ }
This patch allows the user to write more than one cluster of compressed
data at a time, and it does so by splitting the request into many
cluster-sized requests and using qcow2_add_task() for each one of them.
What happens however is that there's no guarantee that the requests are
processed in the same order that they were added.
One consequence is that running on an empty qcow2 file a command as
simple as this one:
qemu-io -c 'write -c 0 256k' image.qcow2
does not always produce the same results.
This does not have any user-visible consequences for the guest. In all
cases the data is correctly written, it's just that the ordering of the
compressed clusters (and therefore the contents of the L2 entries) will
be different each time.
Because of this a test cannot expect that running the same commands on
an empty image produces always the same results.
Is this something that we should be concerned about?
Parallel writing compressed clusters is significant improvement, as it allow
compressing in really parallel threads.
Generally, async parallel issuing of several requests gives more performance
than handling peaces one-by-one, mirror works on this basis and it is fast.
I've already moved qcow2 to this idea (aio tasks in qcow2 code), and in
progress of moving backup job. So, I think that asynchrony and ambiguity would
be native for block-layer anyway.
Hmm. Still, what about cluster sequence? For normal clusters there may be
simple thing to do: preallocation (at least of metadata). So, we can pre-create
cluster sequence.. But what to do with compressed clusters if we want specific
order for them, I don't know. On the other hand, ordering of normal cluster may
make sence: it should increase performnace of following IO. But for compressed
clusters it's not the case.
So, I don't think we should make specific workaround for testing... What
exactly is the case?
--
Best regards,
Vladimir