[05/17] block: Convert BB interface to byte-based discards

Commit Message

Eric Blake June 22, 2016, 3:51 p.m. UTC

Change sector-based blk_discard(), blk_co_discard(), and
blk_aio_discard() to instead be byte-based *_pdiscard()
functions.  NBD gets a lot simpler now that ignoring the
unaligned portion of a byte-based discard request is handled
under the hood by the block layer.

Signed-off-by: Eric Blake <[e-mail]>
---
 include/sysemu/block-backend.h |  9 ++++-----
 block/block-backend.c          | 25 +++++++++++--------------
 block/mirror.c                 |  5 +++--
 hw/block/xen_disk.c            |  7 ++++---
 hw/ide/core.c                  |  6 ++++--
 hw/scsi/scsi-disk.c            |  8 ++++----
 nbd/server.c                   | 19 +++++--------------
 qemu-io-cmds.c                 |  3 +--
 8 files changed, 36 insertions(+), 46 deletions(-)

Comments

Stefan Hajnoczi July 14, 2016, 12:16 p.m. UTC | #1

On Wed, Jun 22, 2016 at 09:51:02AM -0600, Eric Blake wrote:
> Change sector-based blk_discard(), blk_co_discard(), and
> blk_aio_discard() to instead be byte-based *_pdiscard()
> functions.  NBD gets a lot simpler now that ignoring the
> unaligned portion of a byte-based discard request is handled
> under the hood by the block layer.
> 
> Signed-off-by: Eric Blake <[e-mail]>
> ---
>  include/sysemu/block-backend.h |  9 ++++-----
>  block/block-backend.c          | 25 +++++++++++--------------
>  block/mirror.c                 |  5 +++--
>  hw/block/xen_disk.c            |  7 ++++---
>  hw/ide/core.c                  |  6 ++++--
>  hw/scsi/scsi-disk.c            |  8 ++++----
>  nbd/server.c                   | 19 +++++--------------
>  qemu-io-cmds.c                 |  3 +--
>  8 files changed, 36 insertions(+), 46 deletions(-)

Reviewed-by: Stefan Hajnoczi <[e-mail]>

Patch

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 2469a1c..778a994 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -138,15 +138,14 @@  BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset,
                             BlockCompletionFunc *cb, void *opaque);
 BlockAIOCB *blk_aio_flush(BlockBackend *blk,
                           BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *blk_aio_discard(BlockBackend *blk,
-                            int64_t sector_num, int nb_sectors,
-                            BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, int64_t offset, int count,
+                             BlockCompletionFunc *cb, void *opaque);
 void blk_aio_cancel(BlockAIOCB *acb);
 void blk_aio_cancel_async(BlockAIOCB *acb);
 int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
 BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
                           BlockCompletionFunc *cb, void *opaque);
-int blk_co_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors);
+int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int count);
 int blk_co_flush(BlockBackend *blk);
 int blk_flush(BlockBackend *blk);
 int blk_flush_all(void);
@@ -206,7 +205,7 @@  int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
 int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
                          const uint8_t *buf, int nb_sectors);
 int blk_truncate(BlockBackend *blk, int64_t offset);
-int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors);
+int blk_pdiscard(BlockBackend *blk, int64_t offset, int count);
 int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
                      int64_t pos, int size);
 int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
diff --git a/block/block-backend.c b/block/block-backend.c
index 0a488f7..bf125c5 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1060,17 +1060,16 @@  BlockAIOCB *blk_aio_flush(BlockBackend *blk,
     return bdrv_aio_flush(blk_bs(blk), cb, opaque);
 }

-BlockAIOCB *blk_aio_discard(BlockBackend *blk,
-                            int64_t sector_num, int nb_sectors,
-                            BlockCompletionFunc *cb, void *opaque)
+BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk,
+                             int64_t offset, int count,
+                             BlockCompletionFunc *cb, void *opaque)
 {
-    int ret = blk_check_request(blk, sector_num, nb_sectors);
+    int ret = blk_check_byte_request(blk, offset, count);
     if (ret < 0) {
         return blk_abort_aio_request(blk, cb, opaque, ret);
     }

-    return bdrv_aio_pdiscard(blk_bs(blk), sector_num << BDRV_SECTOR_BITS,
-                             nb_sectors << BDRV_SECTOR_BITS, cb, opaque);
+    return bdrv_aio_pdiscard(blk_bs(blk), offset, count, cb, opaque);
 }

 void blk_aio_cancel(BlockAIOCB *acb)
@@ -1102,15 +1101,14 @@  BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
     return bdrv_aio_ioctl(blk_bs(blk), req, buf, cb, opaque);
 }

-int blk_co_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
+int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int count)
 {
-    int ret = blk_check_request(blk, sector_num, nb_sectors);
+    int ret = blk_check_byte_request(blk, offset, count);
     if (ret < 0) {
         return ret;
     }

-    return bdrv_co_pdiscard(blk_bs(blk), sector_num << BDRV_SECTOR_BITS,
-                            nb_sectors << BDRV_SECTOR_BITS);
+    return bdrv_co_pdiscard(blk_bs(blk), offset, count);
 }

 int blk_co_flush(BlockBackend *blk)
@@ -1500,15 +1498,14 @@  int blk_truncate(BlockBackend *blk, int64_t offset)
     return bdrv_truncate(blk_bs(blk), offset);
 }

-int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
+int blk_pdiscard(BlockBackend *blk, int64_t offset, int count)
 {
-    int ret = blk_check_request(blk, sector_num, nb_sectors);
+    int ret = blk_check_byte_request(blk, offset, count);
     if (ret < 0) {
         return ret;
     }

-    return bdrv_pdiscard(blk_bs(blk), sector_num << BDRV_SECTOR_BITS,
-                         nb_sectors << BDRV_SECTOR_BITS);
+    return bdrv_pdiscard(blk_bs(blk), offset, count);
 }

 int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
diff --git a/block/mirror.c b/block/mirror.c
index a04ed9c..92e821a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -297,8 +297,9 @@  static void mirror_do_zero_or_discard(MirrorBlockJob *s,
     s->in_flight++;
     s->sectors_in_flight += nb_sectors;
     if (is_discard) {
-        blk_aio_discard(s->target, sector_num, op->nb_sectors,
-                        mirror_write_complete, op);
+        blk_aio_pdiscard(s->target, sector_num << BDRV_SECTOR_BITS,
+                         op->nb_sectors << BDRV_SECTOR_BITS,
+                         mirror_write_complete, op);
     } else {
         blk_aio_pwrite_zeroes(s->target, sector_num * BDRV_SECTOR_SIZE,
                               op->nb_sectors * BDRV_SECTOR_SIZE,
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 90aca73..3b8ad33 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -574,9 +574,10 @@  static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
     {
         struct blkif_request_discard *discard_req = (void *)&ioreq->req;
         ioreq->aio_inflight++;
-        blk_aio_discard(blkdev->blk,
-                        discard_req->sector_number, discard_req->nr_sectors,
-                        qemu_aio_complete, ioreq);
+        blk_aio_pdiscard(blkdev->blk,
+                         discard_req->sector_number << BDRV_SECTOR_BITS,
+                         discard_req->nr_sectors << BDRV_SECTOR_BITS,
+                         qemu_aio_complete, ioreq);
         break;
     }
     default:
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 029f6b9..d3ee0c3 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -423,8 +423,10 @@  static void ide_issue_trim_cb(void *opaque, int ret)
                 }

                 /* Got an entry! Submit and exit.  */
-                iocb->aiocb = blk_aio_discard(iocb->blk, sector, count,
-                                              ide_issue_trim_cb, opaque);
+                iocb->aiocb = blk_aio_pdiscard(iocb->blk,
+                                               sector << BDRV_SECTOR_BITS,
+                                               count << BDRV_SECTOR_BITS,
+                                               ide_issue_trim_cb, opaque);
                 return;
             }

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 36f8a85..3e94884 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1609,10 +1609,10 @@  static void scsi_unmap_complete_noio(UnmapCBData *data, int ret)
             goto done;
         }

-        r->req.aiocb = blk_aio_discard(s->qdev.conf.blk,
-                                       sector_num * (s->qdev.blocksize / 512),
-                                       nb_sectors * (s->qdev.blocksize / 512),
-                                       scsi_unmap_complete, data);
+        r->req.aiocb = blk_aio_pdiscard(s->qdev.conf.blk,
+                                        sector_num * s->qdev.blocksize,
+                                        nb_sectors * s->qdev.blocksize,
+                                        scsi_unmap_complete, data);
         data->count--;
         data->inbuf += 16;
         return;
diff --git a/nbd/server.c b/nbd/server.c
index a677e26..2f2d9dd 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1182,20 +1182,11 @@  static void nbd_trip(void *opaque)
         break;
     case NBD_CMD_TRIM:
         TRACE("Request type is TRIM");
-        /* Ignore unaligned head or tail, until block layer adds byte
-         * interface */
-        if (request.len >= BDRV_SECTOR_SIZE) {
-            request.len -= (request.from + request.len) % BDRV_SECTOR_SIZE;
-            ret = blk_co_discard(exp->blk,
-                                 DIV_ROUND_UP(request.from + exp->dev_offset,
-                                              BDRV_SECTOR_SIZE),
-                                 request.len / BDRV_SECTOR_SIZE);
-            if (ret < 0) {
-                LOG("discard failed");
-                reply.error = -ret;
-            }
-        } else {
-            TRACE("trim request too small, ignoring");
+        ret = blk_co_pdiscard(exp->blk, request.from + exp->dev_offset,
+                              request.len);
+        if (ret < 0) {
+            LOG("discard failed");
+            reply.error = -ret;
         }
         if (nbd_co_send_reply(req, &reply, 0) < 0) {
             goto out;
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 09e879f..224deff 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1696,8 +1696,7 @@  static int discard_f(BlockBackend *blk, int argc, char **argv)
     }

     gettimeofday(&t1, NULL);
-    ret = blk_discard(blk, offset >> BDRV_SECTOR_BITS,
-                      count >> BDRV_SECTOR_BITS);
+    ret = blk_pdiscard(blk, offset, count);
     gettimeofday(&t2, NULL);

     if (ret < 0) {