[PATCH 0/9] More truncate improvements

[PATCH 0/9] More truncate improvements

[Eric Blake]

Based-on: <20200424125448.63318-1-kwolf@redhat.com>
[PATCH v7 00/10] block: Fix resize (extending) of short overlays

After reviewing Kevin's work, I questioned if we had a redundancy with
bdrv_has_zero_init_truncate.  It turns out we do, and this is the result.

Patch 1 has been previously posted [1] and reviewed, the rest is new.
I did not address Neils' comment that modern gluster also always
0-initializes [2], as I am not set up to verify it (my changes to the
other drivers are semantic no-ops, so I don't feel as bad about
posting them with less rigourous testing).

[1] https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg08070.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg04266.html

Eric Blake (9):
  gluster: Drop useless has_zero_init callback
  file-win32: Support BDRV_REQ_ZERO_WRITE for truncate
  nfs: Support BDRV_REQ_ZERO_WRITE for truncate
  rbd: Support BDRV_REQ_ZERO_WRITE for truncate
  sheepdog: Support BDRV_REQ_ZERO_WRITE for truncate
  ssh: Support BDRV_REQ_ZERO_WRITE for truncate
  parallels: Rework truncation logic
  vhdx: Rework truncation logic
  block: Drop unused .bdrv_has_zero_init_truncate

 include/block/block.h     |  1 -
 include/block/block_int.h |  7 ---
 block.c                   | 21 ---------
 block/file-posix.c        |  1 -
 block/file-win32.c        |  4 +-
 block/gluster.c           | 14 ------
 block/nfs.c               |  4 +-
 block/parallels.c         | 23 ++++++----
 block/qcow2.c             |  1 -
 block/qed.c               |  1 -
 block/raw-format.c        |  6 ---
 block/rbd.c               |  4 +-
 block/sheepdog.c          |  4 +-
 block/ssh.c               |  5 ++-
 block/vhdx.c              | 89 ++++++++++++++++++++++-----------------
 15 files changed, 80 insertions(+), 105 deletions(-)

-- 
2.26.2

[PATCH 1/9] gluster: Drop useless has_zero_init callback

[Eric Blake]

block.c already defaults to 0 if we don't provide a callback; there's
no need to write a callback that always fails.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block/gluster.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index d06df900f692..31233cac696a 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1359,12 +1359,6 @@ static int64_t qemu_gluster_allocated_file_size(BlockDriverState *bs)
     }
 }

-static int qemu_gluster_has_zero_init(BlockDriverState *bs)
-{
-    /* GlusterFS volume could be backed by a block device */
-    return 0;
-}
-
 /*
  * Find allocation range in @bs around offset @start.
  * May change underlying file descriptor's file offset.
@@ -1569,8 +1563,6 @@ static BlockDriver bdrv_gluster = {
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
     .bdrv_co_flush_to_disk        = qemu_gluster_co_flush_to_disk,
-    .bdrv_has_zero_init           = qemu_gluster_has_zero_init,
-    .bdrv_has_zero_init_truncate  = qemu_gluster_has_zero_init,
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_pdiscard             = qemu_gluster_co_pdiscard,
 #endif
@@ -1601,8 +1593,6 @@ static BlockDriver bdrv_gluster_tcp = {
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
     .bdrv_co_flush_to_disk        = qemu_gluster_co_flush_to_disk,
-    .bdrv_has_zero_init           = qemu_gluster_has_zero_init,
-    .bdrv_has_zero_init_truncate  = qemu_gluster_has_zero_init,
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_pdiscard             = qemu_gluster_co_pdiscard,
 #endif
@@ -1633,8 +1623,6 @@ static BlockDriver bdrv_gluster_unix = {
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
     .bdrv_co_flush_to_disk        = qemu_gluster_co_flush_to_disk,
-    .bdrv_has_zero_init           = qemu_gluster_has_zero_init,
-    .bdrv_has_zero_init_truncate  = qemu_gluster_has_zero_init,
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_pdiscard             = qemu_gluster_co_pdiscard,
 #endif
@@ -1671,8 +1659,6 @@ static BlockDriver bdrv_gluster_rdma = {
     .bdrv_co_readv                = qemu_gluster_co_readv,
     .bdrv_co_writev               = qemu_gluster_co_writev,
     .bdrv_co_flush_to_disk        = qemu_gluster_co_flush_to_disk,
-    .bdrv_has_zero_init           = qemu_gluster_has_zero_init,
-    .bdrv_has_zero_init_truncate  = qemu_gluster_has_zero_init,
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_pdiscard             = qemu_gluster_co_pdiscard,
 #endif
-- 
2.26.2

[PATCH 2/9] file-win32: Support BDRV_REQ_ZERO_WRITE for truncate

[Eric Blake]

When using bdrv_file, .bdrv_has_zero_init_truncate always returns 1;
therefore, we can behave just like file-posix, and always implement
BDRV_REQ_ZERO_WRITE by ignoring it since the OS gives it to us for
free (note that file-posix.c had to use an 'if' because it shared code
between regular files and block devices, but in file-win32.c,
bdrv_host_device uses a separate .bdrv_file_open).

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/file-win32.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/file-win32.c b/block/file-win32.c
index a6b0dda5c302..fa569685d8bc 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -408,6 +408,9 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
         win32_aio_attach_aio_context(s->aio, bdrv_get_aio_context(bs));
     }

+    /* When extending regular files, we get zeros from the OS */
+    bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
+
     ret = 0;
 fail:
     qemu_opts_del(opts);
-- 
2.26.2

[PATCH 3/9] nfs: Support BDRV_REQ_ZERO_WRITE for truncate

[Eric Blake]

Our .bdrv_has_zero_init_truncate returns 1 if we detect that the OS
always 0-fills; we can use that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it when the OS gives it to us for
free.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/nfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/nfs.c b/block/nfs.c
index 2393fbfe6bcc..b93989265630 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -623,6 +623,9 @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags,
     }

     bs->total_sectors = ret;
+    if (client->has_zero_init) {
+        bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
+    }
     ret = 0;
     return ret;
 }
-- 
2.26.2

[PATCH 4/9] rbd: Support BDRV_REQ_ZERO_WRITE for truncate

[Eric Blake]

Our .bdrv_has_zero_init_truncate always returns 1 because rbd always
0-fills; we can use that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/rbd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/rbd.c b/block/rbd.c
index f2d52091c702..331c45adb2b2 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -817,6 +817,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
         }
     }

+    /* When extending regular files, we get zeros from the OS */
+    bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
+
     r = 0;
     goto out;

-- 
2.26.2

[PATCH 5/9] sheepdog: Support BDRV_REQ_ZERO_WRITE for truncate

[Eric Blake]

Our .bdrv_has_zero_init_truncate always returns 1 because sheepdog
always 0-fills; we can use that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/sheepdog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index ef0a6e743e27..26fd22c7f07d 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1654,6 +1654,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
     memcpy(&s->inode, buf, sizeof(s->inode));

     bs->total_sectors = s->inode.vdi_size / BDRV_SECTOR_SIZE;
+    bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
     pstrcpy(s->name, sizeof(s->name), vdi);
     qemu_co_mutex_init(&s->lock);
     qemu_co_mutex_init(&s->queue_lock);
-- 
2.26.2

[PATCH 6/9] ssh: Support BDRV_REQ_ZERO_WRITE for truncate

[Eric Blake]

Our .bdrv_has_zero_init_truncate can detect when the remote side
always zero fills; we can reuse that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it when the server gives it to us for
free.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/ssh.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/ssh.c b/block/ssh.c
index 9eb33df8598c..f9e08a490069 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -883,6 +883,10 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
     /* Go non-blocking. */
     ssh_set_blocking(s->session, 0);

+    if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) {
+        bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
+    }
+
     qapi_free_BlockdevOptionsSsh(opts);

     return 0;
-- 
2.26.2

[PATCH 7/9] parallels: Rework truncation logic

[Eric Blake]

The parallels driver tries to use truncation for image growth, but can
only do so when reads are guaranteed as zero.  Now that we have a way
to request zero contents from truncation, we can defer the decision to
actual allocation attempts rather than up front, reducing the number
of places that still use bdrv_has_zero_init_truncate.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/parallels.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 2be92cf41708..9dadaa3217b9 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -196,14 +196,24 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
     }
     if (s->data_end + space > (len >> BDRV_SECTOR_BITS)) {
         space += s->prealloc_size;
+        /*
+         * We require the expanded size to read back as zero. If the
+         * user permitted truncation, we try that; but if it fails, we
+         * force the safer-but-slower fallocate.
+         */
+        if (s->prealloc_mode == PRL_PREALLOC_MODE_TRUNCATE) {
+            ret = bdrv_truncate(bs->file,
+                                (s->data_end + space) << BDRV_SECTOR_BITS,
+                                false, PREALLOC_MODE_OFF, BDRV_REQ_ZERO_WRITE,
+                                NULL);
+            if (ret == -ENOTSUP) {
+                s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
+            }
+        }
         if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
             ret = bdrv_pwrite_zeroes(bs->file,
                                      s->data_end << BDRV_SECTOR_BITS,
                                      space << BDRV_SECTOR_BITS, 0);
-        } else {
-            ret = bdrv_truncate(bs->file,
-                                (s->data_end + space) << BDRV_SECTOR_BITS,
-                                false, PREALLOC_MODE_OFF, 0, NULL);
         }
         if (ret < 0) {
             return ret;
@@ -828,6 +838,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         qemu_opt_get_size_del(opts, PARALLELS_OPT_PREALLOC_SIZE, 0);
     s->prealloc_size = MAX(s->tracks, s->prealloc_size >> BDRV_SECTOR_BITS);
     buf = qemu_opt_get_del(opts, PARALLELS_OPT_PREALLOC_MODE);
+    /* prealloc_mode can be downgraded later during allocate_clusters */
     s->prealloc_mode = qapi_enum_parse(&prealloc_mode_lookup, buf,
                                        PRL_PREALLOC_MODE_FALLOCATE,
                                        &local_err);
@@ -836,10 +847,6 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail_options;
     }

-    if (!bdrv_has_zero_init_truncate(bs->file->bs)) {
-        s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
-    }
-
     if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_INACTIVE)) {
         s->header->inuse = cpu_to_le32(HEADER_INUSE_MAGIC);
         ret = parallels_update_header(bs);
-- 
2.26.2

[PATCH 8/9] vhdx: Rework truncation logic

[Eric Blake]

The vhdx driver uses truncation for image growth, with a special case
for blocks that already read as zero but which are only being
partially written.  But with a bit of rearranging, it's just as easy
to defer the decision on whether truncation resulted in zeroes to the
actual allocation attempt, reducing the number of places that still
use bdrv_has_zero_init_truncate.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/vhdx.c | 89 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 51 insertions(+), 38 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 21497f731878..fe544abaf52a 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1241,12 +1241,16 @@ exit:
 /*
  * Allocate a new payload block at the end of the file.
  *
- * Allocation will happen at 1MB alignment inside the file
+ * Allocation will happen at 1MB alignment inside the file.
+ *
+ * If @need_zero is set on entry but not cleared on return, then truncation
+ * could not guarantee that the new portion reads as zero, and the caller
+ * will take care of it instead.
  *
  * Returns the file offset start of the new payload block
  */
 static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
-                                    uint64_t *new_offset)
+                               uint64_t *new_offset, bool *need_zero)
 {
     int64_t current_len;

@@ -1263,6 +1267,17 @@ static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
         return -EINVAL;
     }

+    if (*need_zero) {
+        int ret;
+
+        ret = bdrv_truncate(bs->file, *new_offset + s->block_size, false,
+                            PREALLOC_MODE_OFF, BDRV_REQ_ZERO_WRITE, NULL);
+        if (ret != -ENOTSUP) {
+            *need_zero = false;
+            return ret;
+        }
+    }
+
     return bdrv_truncate(bs->file, *new_offset + s->block_size, false,
                          PREALLOC_MODE_OFF, 0, NULL);
 }
@@ -1356,18 +1371,38 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
                 /* in this case, we need to preserve zero writes for
                  * data that is not part of this write, so we must pad
                  * the rest of the buffer to zeroes */
-
-                /* if we are on a posix system with ftruncate() that extends
-                 * a file, then it is zero-filled for us.  On Win32, the raw
-                 * layer uses SetFilePointer and SetFileEnd, which does not
-                 * zero fill AFAIK */
-
-                /* Queue another write of zero buffers if the underlying file
-                 * does not zero-fill on file extension */
-
-                if (bdrv_has_zero_init_truncate(bs->file->bs) == 0) {
-                    use_zero_buffers = true;
-
+                use_zero_buffers = true;
+                /* fall through */
+            case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
+            case PAYLOAD_BLOCK_UNMAPPED:
+            case PAYLOAD_BLOCK_UNMAPPED_v095:
+            case PAYLOAD_BLOCK_UNDEFINED:
+                bat_prior_offset = sinfo.file_offset;
+                ret = vhdx_allocate_block(bs, s, &sinfo.file_offset,
+                                          &use_zero_buffers);
+                if (ret < 0) {
+                    goto exit;
+                }
+                /*
+                 * once we support differencing files, this may also be
+                 * partially present
+                 */
+                /* update block state to the newly specified state */
+                vhdx_update_bat_table_entry(bs, s, &sinfo, &bat_entry,
+                                            &bat_entry_offset,
+                                            PAYLOAD_BLOCK_FULLY_PRESENT);
+                bat_update = true;
+                /*
+                 * Since we just allocated a block, file_offset is the
+                 * beginning of the payload block. It needs to be the
+                 * write address, which includes the offset into the
+                 * block, unless the entire block needs to read as
+                 * zeroes but truncation was not able to provide them,
+                 * in which case we need to fill in the rest.
+                 */
+                if (!use_zero_buffers) {
+                    sinfo.file_offset += sinfo.block_offset;
+                } else {
                     /* zero fill the front, if any */
                     if (sinfo.block_offset) {
                         iov1.iov_len = sinfo.block_offset;
@@ -1379,7 +1414,7 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
                     }

                     /* our actual data */
-                    qemu_iovec_concat(&hd_qiov, qiov,  bytes_done,
+                    qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
                                       sinfo.bytes_avail);

                     /* zero fill the back, if any */
@@ -1394,29 +1429,7 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
                         sectors_to_write += iov2.iov_len >> BDRV_SECTOR_BITS;
                     }
                 }
-                /* fall through */
-            case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */
-            case PAYLOAD_BLOCK_UNMAPPED:
-            case PAYLOAD_BLOCK_UNMAPPED_v095:
-            case PAYLOAD_BLOCK_UNDEFINED:
-                bat_prior_offset = sinfo.file_offset;
-                ret = vhdx_allocate_block(bs, s, &sinfo.file_offset);
-                if (ret < 0) {
-                    goto exit;
-                }
-                /* once we support differencing files, this may also be
-                 * partially present */
-                /* update block state to the newly specified state */
-                vhdx_update_bat_table_entry(bs, s, &sinfo, &bat_entry,
-                                            &bat_entry_offset,
-                                            PAYLOAD_BLOCK_FULLY_PRESENT);
-                bat_update = true;
-                /* since we just allocated a block, file_offset is the
-                 * beginning of the payload block. It needs to be the
-                 * write address, which includes the offset into the block */
-                if (!use_zero_buffers) {
-                    sinfo.file_offset += sinfo.block_offset;
-                }
+
                 /* fall through */
             case PAYLOAD_BLOCK_FULLY_PRESENT:
                 /* if the file offset address is in the header zone,
-- 
2.26.2

[PATCH 9/9] block: Drop unused .bdrv_has_zero_init_truncate

[Eric Blake]

Now that there are no clients of bdrv_has_zero_init_truncate, none of
the drivers need to worry about providing it.

What's more, this eliminates a source of some confusion: a literal
reading of the documentation as written in ceaca56f and implemented in
commit 1dcaf527 claims that a driver which returns 0 for
bdrv_has_zero_init_truncate() must not return 1 for
bdrv_has_zero_init(); this condition was violated for parallels, qcow,
and sometimes for vdi, although in practice it did not matter since
those drivers also lacked .bdrv_co_truncate.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/block/block.h     |  1 -
 include/block/block_int.h |  7 -------
 block.c                   | 21 ---------------------
 block/file-posix.c        |  1 -
 block/file-win32.c        |  1 -
 block/nfs.c               |  1 -
 block/qcow2.c             |  1 -
 block/qed.c               |  1 -
 block/raw-format.c        |  6 ------
 block/rbd.c               |  1 -
 block/sheepdog.c          |  3 ---
 block/ssh.c               |  1 -
 12 files changed, 45 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 8b62429aa4a9..4de8d8f8a6b2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -430,7 +430,6 @@ int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);
 int bdrv_co_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);
 int bdrv_has_zero_init_1(BlockDriverState *bs);
 int bdrv_has_zero_init(BlockDriverState *bs);
-int bdrv_has_zero_init_truncate(BlockDriverState *bs);
 bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs);
 bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
 int bdrv_block_status(BlockDriverState *bs, int64_t offset,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 92335f33c750..df6d0273d679 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -449,16 +449,9 @@ struct BlockDriver {
     /*
      * Returns 1 if newly created images are guaranteed to contain only
      * zeros, 0 otherwise.
-     * Must return 0 if .bdrv_has_zero_init_truncate() returns 0.
      */
     int (*bdrv_has_zero_init)(BlockDriverState *bs);

-    /*
-     * Returns 1 if new areas added by growing the image with
-     * PREALLOC_MODE_OFF contain only zeros, 0 otherwise.
-     */
-    int (*bdrv_has_zero_init_truncate)(BlockDriverState *bs);
-
     /* Remove fd handlers, timers, and other event loop callbacks so the event
      * loop is no longer in use.  Called with no in-flight requests and in
      * depth-first traversal order with parents before child nodes.
diff --git a/block.c b/block.c
index 03cc5813a292..fea646d33dc3 100644
--- a/block.c
+++ b/block.c
@@ -5291,27 +5291,6 @@ int bdrv_has_zero_init(BlockDriverState *bs)
     return 0;
 }

-int bdrv_has_zero_init_truncate(BlockDriverState *bs)
-{
-    if (!bs->drv) {
-        return 0;
-    }
-
-    if (bs->backing) {
-        /* Depends on the backing image length, but better safe than sorry */
-        return 0;
-    }
-    if (bs->drv->bdrv_has_zero_init_truncate) {
-        return bs->drv->bdrv_has_zero_init_truncate(bs);
-    }
-    if (bs->file && bs->drv->is_filter) {
-        return bdrv_has_zero_init_truncate(bs->file->bs);
-    }
-
-    /* safe default */
-    return 0;
-}
-
 bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs)
 {
     BlockDriverInfo bdi;
diff --git a/block/file-posix.c b/block/file-posix.c
index 1dca220a81ba..84012be18f4d 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3099,7 +3099,6 @@ BlockDriver bdrv_file = {
     .bdrv_co_create = raw_co_create,
     .bdrv_co_create_opts = raw_co_create_opts,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
     .bdrv_co_block_status = raw_co_block_status,
     .bdrv_co_invalidate_cache = raw_co_invalidate_cache,
     .bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes,
diff --git a/block/file-win32.c b/block/file-win32.c
index fa569685d8bc..221aaf713e24 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -641,7 +641,6 @@ BlockDriver bdrv_file = {
     .bdrv_close         = raw_close,
     .bdrv_co_create_opts = raw_co_create_opts,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,

     .bdrv_aio_preadv    = raw_aio_preadv,
     .bdrv_aio_pwritev   = raw_aio_pwritev,
diff --git a/block/nfs.c b/block/nfs.c
index b93989265630..2d3474c1e051 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -876,7 +876,6 @@ static BlockDriver bdrv_nfs = {
     .create_opts                    = &nfs_create_opts,

     .bdrv_has_zero_init             = nfs_has_zero_init,
-    .bdrv_has_zero_init_truncate    = nfs_has_zero_init,
     .bdrv_get_allocated_file_size   = nfs_get_allocated_file_size,
     .bdrv_co_truncate               = nfs_file_co_truncate,

diff --git a/block/qcow2.c b/block/qcow2.c
index 2ba0b17c391c..9acdbaeb3ab8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5596,7 +5596,6 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_co_create_opts  = qcow2_co_create_opts,
     .bdrv_co_create       = qcow2_co_create,
     .bdrv_has_zero_init   = qcow2_has_zero_init,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
     .bdrv_co_block_status = qcow2_co_block_status,

     .bdrv_co_preadv_part    = qcow2_co_preadv_part,
diff --git a/block/qed.c b/block/qed.c
index b0fdb8f56508..82483c870d26 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1675,7 +1675,6 @@ static BlockDriver bdrv_qed = {
     .bdrv_co_create           = bdrv_qed_co_create,
     .bdrv_co_create_opts      = bdrv_qed_co_create_opts,
     .bdrv_has_zero_init       = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
     .bdrv_co_block_status     = bdrv_qed_co_block_status,
     .bdrv_co_readv            = bdrv_qed_co_readv,
     .bdrv_co_writev           = bdrv_qed_co_writev,
diff --git a/block/raw-format.c b/block/raw-format.c
index 351f2d91c6b6..9108e4369628 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -414,11 +414,6 @@ static int raw_has_zero_init(BlockDriverState *bs)
     return bdrv_has_zero_init(bs->file->bs);
 }

-static int raw_has_zero_init_truncate(BlockDriverState *bs)
-{
-    return bdrv_has_zero_init_truncate(bs->file->bs);
-}
-
 static int coroutine_fn raw_co_create_opts(BlockDriver *drv,
                                            const char *filename,
                                            QemuOpts *opts,
@@ -582,7 +577,6 @@ BlockDriver bdrv_raw = {
     .bdrv_co_ioctl        = &raw_co_ioctl,
     .create_opts          = &raw_create_opts,
     .bdrv_has_zero_init   = &raw_has_zero_init,
-    .bdrv_has_zero_init_truncate = &raw_has_zero_init_truncate,
     .strong_runtime_opts  = raw_strong_runtime_opts,
     .mutable_opts         = mutable_opts,
 };
diff --git a/block/rbd.c b/block/rbd.c
index 331c45adb2b2..617553b022c7 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1313,7 +1313,6 @@ static BlockDriver bdrv_rbd = {
     .bdrv_co_create         = qemu_rbd_co_create,
     .bdrv_co_create_opts    = qemu_rbd_co_create_opts,
     .bdrv_has_zero_init     = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate = bdrv_has_zero_init_1,
     .bdrv_get_info          = qemu_rbd_getinfo,
     .create_opts            = &qemu_rbd_create_opts,
     .bdrv_getlength         = qemu_rbd_getlength,
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 26fd22c7f07d..34a6b97f0164 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -3233,7 +3233,6 @@ static BlockDriver bdrv_sheepdog = {
     .bdrv_co_create               = sd_co_create,
     .bdrv_co_create_opts          = sd_co_create_opts,
     .bdrv_has_zero_init           = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate  = bdrv_has_zero_init_1,
     .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
     .bdrv_co_truncate             = sd_co_truncate,
@@ -3272,7 +3271,6 @@ static BlockDriver bdrv_sheepdog_tcp = {
     .bdrv_co_create               = sd_co_create,
     .bdrv_co_create_opts          = sd_co_create_opts,
     .bdrv_has_zero_init           = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate  = bdrv_has_zero_init_1,
     .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
     .bdrv_co_truncate             = sd_co_truncate,
@@ -3311,7 +3309,6 @@ static BlockDriver bdrv_sheepdog_unix = {
     .bdrv_co_create               = sd_co_create,
     .bdrv_co_create_opts          = sd_co_create_opts,
     .bdrv_has_zero_init           = bdrv_has_zero_init_1,
-    .bdrv_has_zero_init_truncate  = bdrv_has_zero_init_1,
     .bdrv_getlength               = sd_getlength,
     .bdrv_get_allocated_file_size = sd_get_allocated_file_size,
     .bdrv_co_truncate             = sd_co_truncate,
diff --git a/block/ssh.c b/block/ssh.c
index f9e08a490069..098dbe03c15b 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -1397,7 +1397,6 @@ static BlockDriver bdrv_ssh = {
     .bdrv_co_create_opts          = ssh_co_create_opts,
     .bdrv_close                   = ssh_close,
     .bdrv_has_zero_init           = ssh_has_zero_init,
-    .bdrv_has_zero_init_truncate  = ssh_has_zero_init,
     .bdrv_co_readv                = ssh_co_readv,
     .bdrv_co_writev               = ssh_co_writev,
     .bdrv_getlength               = ssh_getlength,
-- 
2.26.2

Re: [PATCH 0/9] More truncate improvements

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200428202905.770727-1-eblake@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      block/file-posix.o
  CC      block/linux-aio.o
/tmp/qemu-test/src/block/parallels.c: In function 'parallels_co_writev':
/tmp/qemu-test/src/block/parallels.c:218:12: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         if (ret < 0) {
            ^
/tmp/qemu-test/src/block/parallels.c:169:9: note: 'ret' was declared here
     int ret;
         ^
cc1: all warnings being treated as errors
make: *** [block/parallels.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=17b76be9fd0a432985a07807c0fce033', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-_1eoo1y7/src/docker-src.2020-04-28-22.22.17.10859:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=17b76be9fd0a432985a07807c0fce033
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-_1eoo1y7/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m2.980s
user    0m8.950s


The full log is available at
http://patchew.org/logs/20200428202905.770727-1-eblake@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 0/9] More truncate improvements

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200428202905.770727-1-eblake@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      block/io.o
  CC      block/create.o
/tmp/qemu-test/src/block/parallels.c: In function 'parallels_co_writev':
/tmp/qemu-test/src/block/parallels.c:218:12: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         if (ret < 0) {
            ^
/tmp/qemu-test/src/block/parallels.c:169:9: note: 'ret' was declared here
     int ret;
         ^~~
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: block/parallels.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=b6fe059fceb24ee6af44e5ec70624428', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-3enfstzu/src/docker-src.2020-04-28-22.21.44.9392:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=b6fe059fceb24ee6af44e5ec70624428
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-3enfstzu/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m45.121s
user    0m8.436s


The full log is available at
http://patchew.org/logs/20200428202905.770727-1-eblake@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 6/9] ssh: Support BDRV_REQ_ZERO_WRITE for truncate

[Richard W.M. Jones]

On Tue, Apr 28, 2020 at 03:29:02PM -0500, Eric Blake wrote:
> Our .bdrv_has_zero_init_truncate can detect when the remote side
> always zero fills; we can reuse that same knowledge to implement
> BDRV_REQ_ZERO_WRITE by ignoring it when the server gives it to us for
> free.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  block/ssh.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/block/ssh.c b/block/ssh.c
> index 9eb33df8598c..f9e08a490069 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -883,6 +883,10 @@ static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
>      /* Go non-blocking. */
>      ssh_set_blocking(s->session, 0);
> 
> +    if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) {
> +        bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
> +    }
> +

As the libssh sftp API is written this is all fine, so ACK:

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

On the other hand the actual openssh / libssh code is a bit hairy.
Openssh simply copies the st_mode field onto the wire (with
byte-swapping).  Libssh assumes that S_IFREG == 0100000 and translates
this bit to SSH_FILEXFER_TYPE_REGULAR.  This happens to be true on
Linux and *BSD but as far as I can tell isn't defined in POSIX.
Anyway I guess it'll work in all places that we care about.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Re: [PATCH 9/9] block: Drop unused .bdrv_has_zero_init_truncate

[Richard W.M. Jones]

On Tue, Apr 28, 2020 at 03:29:05PM -0500, Eric Blake wrote:
> Now that there are no clients of bdrv_has_zero_init_truncate, none of
> the drivers need to worry about providing it.
> 
> What's more, this eliminates a source of some confusion: a literal
> reading of the documentation as written in ceaca56f and implemented in
> commit 1dcaf527 claims that a driver which returns 0 for
> bdrv_has_zero_init_truncate() must not return 1 for
> bdrv_has_zero_init(); this condition was violated for parallels, qcow,
> and sometimes for vdi, although in practice it did not matter since
> those drivers also lacked .bdrv_co_truncate.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
...
> diff --git a/block/ssh.c b/block/ssh.c
> index f9e08a490069..098dbe03c15b 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -1397,7 +1397,6 @@ static BlockDriver bdrv_ssh = {
>      .bdrv_co_create_opts          = ssh_co_create_opts,
>      .bdrv_close                   = ssh_close,
>      .bdrv_has_zero_init           = ssh_has_zero_init,
> -    .bdrv_has_zero_init_truncate  = ssh_has_zero_init,
>      .bdrv_co_readv                = ssh_co_readv,
>      .bdrv_co_writev               = ssh_co_writev,
>      .bdrv_getlength               = ssh_getlength,

This part seems fine, so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [PATCH 0/9] More truncate improvements

[Eric Blake]

On 4/28/20 9:24 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200428202905.770727-1-eblake@redhat.com/
> 

> /tmp/qemu-test/src/block/parallels.c: In function 'parallels_co_writev':
> /tmp/qemu-test/src/block/parallels.c:218:12: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>           if (ret < 0) {
>              ^
> /tmp/qemu-test/src/block/parallels.c:169:9: note: 'ret' was declared here
>       int ret;
>           ^

False positive: the code is roughly:

int ret;
if (cond1) {
   ret = ...;
}
if (cond2) {
   ret = ...;
}
if (ret < 0)

but the compiler can't prove that cond1 + cond2 covers all 
possibilities.  The obvious fix is to initialize ret; squash this into 7/9:

diff --git i/block/parallels.c w/block/parallels.c
index eb6c6c01b998..e7717c508e62 100644
--- i/block/parallels.c
+++ w/block/parallels.c
@@ -166,7 +166,7 @@ static int64_t block_status(BDRVParallelsState *s, 
int64_t sector_num,
  static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
                                   int nb_sectors, int *pnum)
  {
-    int ret;
+    int ret = 0;
      BDRVParallelsState *s = bs->opaque;
      int64_t pos, space, idx, to_allocate, i, len;





-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/9] More truncate improvements

[Eric Blake]

On 4/28/20 3:28 PM, Eric Blake wrote:
> Based-on: <20200424125448.63318-1-kwolf@redhat.com>
> [PATCH v7 00/10] block: Fix resize (extending) of short overlays
> 
> After reviewing Kevin's work, I questioned if we had a redundancy with
> bdrv_has_zero_init_truncate.  It turns out we do, and this is the result.
> 
> Patch 1 has been previously posted [1] and reviewed, the rest is new.
> I did not address Neils' comment that modern gluster also always
> 0-initializes [2], as I am not set up to verify it (my changes to the
> other drivers are semantic no-ops, so I don't feel as bad about
> posting them with less rigourous testing).
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg08070.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg04266.html
> 
> Eric Blake (9):
>    gluster: Drop useless has_zero_init callback
>    file-win32: Support BDRV_REQ_ZERO_WRITE for truncate
>    nfs: Support BDRV_REQ_ZERO_WRITE for truncate
>    rbd: Support BDRV_REQ_ZERO_WRITE for truncate
>    sheepdog: Support BDRV_REQ_ZERO_WRITE for truncate
>    ssh: Support BDRV_REQ_ZERO_WRITE for truncate
>    parallels: Rework truncation logic
>    vhdx: Rework truncation logic
>    block: Drop unused .bdrv_has_zero_init_truncate
> 

Ping.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/9] More truncate improvements

[Kevin Wolf]

Am 28.04.2020 um 22:28 hat Eric Blake geschrieben:
> Based-on: <20200424125448.63318-1-kwolf@redhat.com>
> [PATCH v7 00/10] block: Fix resize (extending) of short overlays
> 
> After reviewing Kevin's work, I questioned if we had a redundancy with
> bdrv_has_zero_init_truncate.  It turns out we do, and this is the result.
> 
> Patch 1 has been previously posted [1] and reviewed, the rest is new.
> I did not address Neils' comment that modern gluster also always
> 0-initializes [2], as I am not set up to verify it (my changes to the
> other drivers are semantic no-ops, so I don't feel as bad about
> posting them with less rigourous testing).
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg08070.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg04266.html

block/parallels.c: In function 'parallels_co_writev':
block/parallels.c:218:12: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  218 |         if (ret < 0) {
      |            ^
block/parallels.c:169:9: note: 'ret' was declared here
  169 |     int ret;
      |         ^~~
cc1: all warnings being treated as errors

Apart from that, you can add:

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [PATCH 7/9] parallels: Rework truncation logic

[Denis V. Lunev]

On 4/28/20 11:29 PM, Eric Blake wrote:
> The parallels driver tries to use truncation for image growth, but can
> only do so when reads are guaranteed as zero.  Now that we have a way
> to request zero contents from truncation, we can defer the decision to
> actual allocation attempts rather than up front, reducing the number
> of places that still use bdrv_has_zero_init_truncate.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  block/parallels.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 2be92cf41708..9dadaa3217b9 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -196,14 +196,24 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
>      }
>      if (s->data_end + space > (len >> BDRV_SECTOR_BITS)) {
>          space += s->prealloc_size;
> +        /*
> +         * We require the expanded size to read back as zero. If the
> +         * user permitted truncation, we try that; but if it fails, we
> +         * force the safer-but-slower fallocate.
> +         */
> +        if (s->prealloc_mode == PRL_PREALLOC_MODE_TRUNCATE) {
> +            ret = bdrv_truncate(bs->file,
> +                                (s->data_end + space) << BDRV_SECTOR_BITS,
> +                                false, PREALLOC_MODE_OFF, BDRV_REQ_ZERO_WRITE,
> +                                NULL);
> +            if (ret == -ENOTSUP) {
> +                s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
> +            }
> +        }
>          if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
>              ret = bdrv_pwrite_zeroes(bs->file,
>                                       s->data_end << BDRV_SECTOR_BITS,
>                                       space << BDRV_SECTOR_BITS, 0);
> -        } else {
> -            ret = bdrv_truncate(bs->file,
> -                                (s->data_end + space) << BDRV_SECTOR_BITS,
> -                                false, PREALLOC_MODE_OFF, 0, NULL);
>          }
>          if (ret < 0) {
>              return ret;
> @@ -828,6 +838,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
>          qemu_opt_get_size_del(opts, PARALLELS_OPT_PREALLOC_SIZE, 0);
>      s->prealloc_size = MAX(s->tracks, s->prealloc_size >> BDRV_SECTOR_BITS);
>      buf = qemu_opt_get_del(opts, PARALLELS_OPT_PREALLOC_MODE);
> +    /* prealloc_mode can be downgraded later during allocate_clusters */
>      s->prealloc_mode = qapi_enum_parse(&prealloc_mode_lookup, buf,
>                                         PRL_PREALLOC_MODE_FALLOCATE,
>                                         &local_err);
> @@ -836,10 +847,6 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
>          goto fail_options;
>      }
>
> -    if (!bdrv_has_zero_init_truncate(bs->file->bs)) {
> -        s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
> -    }
> -
>      if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_INACTIVE)) {
>          s->header->inuse = cpu_to_le32(HEADER_INUSE_MAGIC);
>          ret = parallels_update_header(bs);
Reviewed-by: Denis V. Lunev <den@openvz.org>

Re: [PATCH 0/9] More truncate improvements

[Eric Blake]

On 5/7/20 5:15 AM, Kevin Wolf wrote:
> Am 28.04.2020 um 22:28 hat Eric Blake geschrieben:
>> Based-on: <20200424125448.63318-1-kwolf@redhat.com>
>> [PATCH v7 00/10] block: Fix resize (extending) of short overlays
>>
>> After reviewing Kevin's work, I questioned if we had a redundancy with
>> bdrv_has_zero_init_truncate.  It turns out we do, and this is the result.
>>
>> Patch 1 has been previously posted [1] and reviewed, the rest is new.
>> I did not address Neils' comment that modern gluster also always
>> 0-initializes [2], as I am not set up to verify it (my changes to the
>> other drivers are semantic no-ops, so I don't feel as bad about
>> posting them with less rigourous testing).
>>
>> [1] https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg08070.html
>> [2] https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg04266.html
> 
> block/parallels.c: In function 'parallels_co_writev':
> block/parallels.c:218:12: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    218 |         if (ret < 0) {
>        |            ^
> block/parallels.c:169:9: note: 'ret' was declared here
>    169 |     int ret;
>        |         ^~~
> cc1: all warnings being treated as errors

Yep, fixup posted here:
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg05199.html

> 
> Apart from that, you can add:
> 
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Do you need me to send a v2?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/9] More truncate improvements

[Kevin Wolf]

Am 07.05.2020 um 16:29 hat Eric Blake geschrieben:
> On 5/7/20 5:15 AM, Kevin Wolf wrote:
> > Am 28.04.2020 um 22:28 hat Eric Blake geschrieben:
> > > Based-on: <20200424125448.63318-1-kwolf@redhat.com>
> > > [PATCH v7 00/10] block: Fix resize (extending) of short overlays
> > > 
> > > After reviewing Kevin's work, I questioned if we had a redundancy with
> > > bdrv_has_zero_init_truncate.  It turns out we do, and this is the result.
> > > 
> > > Patch 1 has been previously posted [1] and reviewed, the rest is new.
> > > I did not address Neils' comment that modern gluster also always
> > > 0-initializes [2], as I am not set up to verify it (my changes to the
> > > other drivers are semantic no-ops, so I don't feel as bad about
> > > posting them with less rigourous testing).
> > > 
> > > [1] https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg08070.html
> > > [2] https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg04266.html
> > 
> > block/parallels.c: In function 'parallels_co_writev':
> > block/parallels.c:218:12: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >    218 |         if (ret < 0) {
> >        |            ^
> > block/parallels.c:169:9: note: 'ret' was declared here
> >    169 |     int ret;
> >        |         ^~~
> > cc1: all warnings being treated as errors
> 
> Yep, fixup posted here:
> https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg05199.html
> 
> > 
> > Apart from that, you can add:
> > 
> > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> 
> Do you need me to send a v2?

Ah, sorry, I missed that you had already sent a fixup. I'll squash it
in.

Kevin