All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, vsementsov@virtuozzo.com,
	"open list:qed" <qemu-block@nongnu.org>,
	stefanha@redhat.com, mreitz@redhat.com
Subject: [PATCH v2 10/9] qed: Simplify backing reads
Date: Thu,  7 May 2020 09:45:13 -0500	[thread overview]
Message-ID: <20200507144513.348470-1-eblake@redhat.com> (raw)
In-Reply-To: <20200507084800.20596-1-vsementsov@virtuozzo.com>

The other four drivers that support backing files (qcow, qcow2,
parallels, vmdk) all rely on the block layer to populate zeroes when
reading beyond EOF of a short backing file.  We can simplify the qed
code by doing likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I noticed this during my audit that v1 of Vladimir's series was correct.

No change in iotests results (test 274 is currently failing for qed,
but for other reasons:
+Traceback (most recent call last):
+  File "274", line 24, in <module>
+    iotests.verify_image_format(supported_fmts=['qcow2'])
+AttributeError: module 'iotests' has no attribute 'verify_image_format'
)

 block/qed.h |  1 -
 block/qed.c | 64 +++++------------------------------------------------
 2 files changed, 6 insertions(+), 59 deletions(-)

diff --git a/block/qed.h b/block/qed.h
index 42c115d8220c..3d12bf78d412 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -140,7 +140,6 @@ typedef struct QEDAIOCB {

     /* Current cluster scatter-gather list */
     QEMUIOVector cur_qiov;
-    QEMUIOVector *backing_qiov;
     uint64_t cur_pos;               /* position on block device, in bytes */
     uint64_t cur_cluster;           /* cluster offset in image file */
     unsigned int cur_nclusters;     /* number of clusters being accessed */
diff --git a/block/qed.c b/block/qed.c
index 927382995a0c..bea4b9f6cc97 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -849,56 +849,18 @@ static BDRVQEDState *acb_to_s(QEDAIOCB *acb)
  * @s:              QED state
  * @pos:            Byte position in device
  * @qiov:           Destination I/O vector
- * @backing_qiov:   Possibly shortened copy of qiov, to be allocated here
- * @cb:             Completion function
- * @opaque:         User data for completion function
  *
  * This function reads qiov->size bytes starting at pos from the backing file.
  * If there is no backing file then zeroes are read.
  */
 static int coroutine_fn qed_read_backing_file(BDRVQEDState *s, uint64_t pos,
-                                              QEMUIOVector *qiov,
-                                              QEMUIOVector **backing_qiov)
+                                              QEMUIOVector *qiov)
 {
-    uint64_t backing_length = 0;
-    size_t size;
-    int ret;
-
-    /* If there is a backing file, get its length.  Treat the absence of a
-     * backing file like a zero length backing file.
-     */
     if (s->bs->backing) {
-        int64_t l = bdrv_getlength(s->bs->backing->bs);
-        if (l < 0) {
-            return l;
-        }
-        backing_length = l;
-    }
-
-    /* Zero all sectors if reading beyond the end of the backing file */
-    if (pos >= backing_length ||
-        pos + qiov->size > backing_length) {
-        qemu_iovec_memset(qiov, 0, 0, qiov->size);
-    }
-
-    /* Complete now if there are no backing file sectors to read */
-    if (pos >= backing_length) {
-        return 0;
-    }
-
-    /* If the read straddles the end of the backing file, shorten it */
-    size = MIN((uint64_t)backing_length - pos, qiov->size);
-
-    assert(*backing_qiov == NULL);
-    *backing_qiov = g_new(QEMUIOVector, 1);
-    qemu_iovec_init(*backing_qiov, qiov->niov);
-    qemu_iovec_concat(*backing_qiov, qiov, 0, size);
-
-    BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING_AIO);
-    ret = bdrv_co_preadv(s->bs->backing, pos, size, *backing_qiov, 0);
-    if (ret < 0) {
-        return ret;
+        BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING_AIO);
+        return bdrv_co_preadv(s->bs->backing, pos, qiov->size, qiov, 0);
     }
+    qemu_iovec_memset(qiov, 0, 0, qiov->size);
     return 0;
 }

@@ -915,7 +877,6 @@ static int coroutine_fn qed_copy_from_backing_file(BDRVQEDState *s,
                                                    uint64_t offset)
 {
     QEMUIOVector qiov;
-    QEMUIOVector *backing_qiov = NULL;
     int ret;

     /* Skip copy entirely if there is no work to do */
@@ -925,13 +886,7 @@ static int coroutine_fn qed_copy_from_backing_file(BDRVQEDState *s,

     qemu_iovec_init_buf(&qiov, qemu_blockalign(s->bs, len), len);

-    ret = qed_read_backing_file(s, pos, &qiov, &backing_qiov);
-
-    if (backing_qiov) {
-        qemu_iovec_destroy(backing_qiov);
-        g_free(backing_qiov);
-        backing_qiov = NULL;
-    }
+    ret = qed_read_backing_file(s, pos, &qiov);

     if (ret) {
         goto out;
@@ -1339,8 +1294,7 @@ static int coroutine_fn qed_aio_read_data(void *opaque, int ret,
         qemu_iovec_memset(&acb->cur_qiov, 0, 0, acb->cur_qiov.size);
         r = 0;
     } else if (ret != QED_CLUSTER_FOUND) {
-        r = qed_read_backing_file(s, acb->cur_pos, &acb->cur_qiov,
-                                  &acb->backing_qiov);
+        r = qed_read_backing_file(s, acb->cur_pos, &acb->cur_qiov);
     } else {
         BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
         r = bdrv_co_preadv(bs->file, offset, acb->cur_qiov.size,
@@ -1365,12 +1319,6 @@ static int coroutine_fn qed_aio_next_io(QEDAIOCB *acb)
     while (1) {
         trace_qed_aio_next_io(s, acb, 0, acb->cur_pos + acb->cur_qiov.size);

-        if (acb->backing_qiov) {
-            qemu_iovec_destroy(acb->backing_qiov);
-            g_free(acb->backing_qiov);
-            acb->backing_qiov = NULL;
-        }
-
         acb->qiov_offset += acb->cur_qiov.size;
         acb->cur_pos += acb->cur_qiov.size;
         qemu_iovec_reset(&acb->cur_qiov);
-- 
2.26.2



  parent reply	other threads:[~2020-05-07 14:46 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-07  8:47 [PATCH v2 0/9] drop unallocated_blocks_are_zero Vladimir Sementsov-Ogievskiy
2020-05-07  8:47 ` [PATCH v2 1/9] qemu-img: convert: don't use unallocated_blocks_are_zero Vladimir Sementsov-Ogievskiy
2020-05-07  8:47 ` [PATCH v2 2/9] block: inline bdrv_unallocated_blocks_are_zero() Vladimir Sementsov-Ogievskiy
2020-05-07 14:08   ` Eric Blake
2020-05-28  9:31     ` Vladimir Sementsov-Ogievskiy
2020-05-07  8:47 ` [PATCH v2 3/9] block/vdi: return ZERO block-status when appropriate Vladimir Sementsov-Ogievskiy
2020-05-07 14:10   ` Eric Blake
2020-05-07  8:47 ` [PATCH v2 4/9] block/vpc: " Vladimir Sementsov-Ogievskiy
2020-05-07 14:11   ` Eric Blake
2020-05-07  8:47 ` [PATCH v2 5/9] block/crypto: drop unallocated_blocks_are_zero Vladimir Sementsov-Ogievskiy
2020-05-07  8:47 ` [PATCH v2 6/9] block/iscsi: " Vladimir Sementsov-Ogievskiy
2020-05-07  8:47 ` [PATCH v2 7/9] block/file-posix: " Vladimir Sementsov-Ogievskiy
2020-05-07  8:47 ` [PATCH v2 8/9] block/vhdx: " Vladimir Sementsov-Ogievskiy
2020-05-07  8:48 ` [PATCH v2 9/9] block: " Vladimir Sementsov-Ogievskiy
2020-05-07 14:21   ` Eric Blake
2020-05-07 14:45 ` Eric Blake [this message]
2020-05-07 15:22   ` [PATCH v2 10/9] qed: Simplify backing reads Eric Blake
2020-05-07 18:22   ` Vladimir Sementsov-Ogievskiy
2020-05-19 17:40 ` [PATCH v2 0/9] drop unallocated_blocks_are_zero Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200507144513.348470-1-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.