All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read
@ 2016-05-03 15:42 Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 01/14] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
                   ` (13 more replies)
  0 siblings, 14 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf

2.7 material, depends on Kevin's block-next:
git://repo.or.cz/qemu/kevin.git block-next

Previously posted as part of a larger NBD series [1] and as v4 [2].
Mostly orthogonal to Kevin's recent work to also kill sector
interfaces from the driver.

[1] https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg03526.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg04636.html

Also available as a tag at this location:
git fetch git://repo.or.cz/qemu/ericb.git nbd-block-v5

Changes since then:
Actually compile-tested everything (no --target-list helps)
address Kevin's reviews on 3, 4, 6
fix indentation on 13

001/14:[----] [--] 'block: Allow BDRV_REQ_FUA through blk_pwrite()'
002/14:[----] [--] 'fdc: Switch to byte-based block access'
003/14:[0006] [FC] 'nand: Switch to byte-based block access'
004/14:[0013] [FC] 'onenand: Switch to byte-based block access'
005/14:[----] [--] 'pflash: Switch to byte-based block access'
006/14:[0007] [FC] 'sd: Switch to byte-based block access'
007/14:[----] [--] 'm25p80: Switch to byte-based block access'
008/14:[----] [--] 'atapi: Switch to byte-based block access'
009/14:[----] [--] 'nbd: Switch to byte-based block access'
010/14:[----] [--] 'qemu-img: Switch to byte-based block access'
011/14:[----] [--] 'qemu-io: Switch to byte-based block access'
012/14:[----] [--] 'block: Switch blk_read_unthrottled() to byte interface'
013/14:[0002] [FC] 'block: Switch blk_write_zeroes() to byte interface'
014/14:[----] [-C] 'block: Kill blk_write(), blk_read()'

Eric Blake (14):
  block: Allow BDRV_REQ_FUA through blk_pwrite()
  fdc: Switch to byte-based block access
  nand: Switch to byte-based block access
  onenand: Switch to byte-based block access
  pflash: Switch to byte-based block access
  sd: Switch to byte-based block access
  m25p80: Switch to byte-based block access
  atapi: Switch to byte-based block access
  nbd: Switch to byte-based block access
  qemu-img: Switch to byte-based block access
  qemu-io: Switch to byte-based block access
  block: Switch blk_read_unthrottled() to byte interface
  block: Switch blk_write_zeroes() to byte interface
  block: Kill blk_write(), blk_read()

 include/sysemu/block-backend.h | 15 ++++----
 block/block-backend.c          | 47 +++++++-------------------
 block/crypto.c                 |  2 +-
 block/parallels.c              |  5 +--
 block/qcow.c                   |  8 ++---
 block/qcow2.c                  |  4 +--
 block/qed.c                    |  6 ++--
 block/sheepdog.c               |  2 +-
 block/vdi.c                    |  4 +--
 block/vhdx.c                   |  5 +--
 block/vmdk.c                   | 10 +++---
 block/vpc.c                    | 10 +++---
 hw/block/fdc.c                 | 25 +++++++++-----
 hw/block/hd-geometry.c         |  2 +-
 hw/block/m25p80.c              |  3 +-
 hw/block/nand.c                | 36 +++++++++++++-------
 hw/block/onenand.c             | 41 ++++++++++++++--------
 hw/block/pflash_cfi01.c        | 12 +++----
 hw/block/pflash_cfi02.c        | 12 +++----
 hw/ide/atapi.c                 | 19 ++++++-----
 hw/nvram/spapr_nvram.c         |  4 +--
 hw/sd/sd.c                     | 51 +++-------------------------
 nbd/server.c                   |  2 +-
 qemu-img.c                     | 31 +++++++++++------
 qemu-io-cmds.c                 | 77 ++++++++++--------------------------------
 qemu-nbd.c                     | 11 +++---
 26 files changed, 191 insertions(+), 253 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 01/14] block: Allow BDRV_REQ_FUA through blk_pwrite()
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 02/14] fdc: Switch to byte-based block access Eric Blake
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, kwolf, Max Reitz, Stefan Hajnoczi, Denis V. Lunev,
	Hitoshi Mitake, Liu Yuan, Jeff Cody, Stefan Weil, Fam Zheng,
	David Gibson, Alexander Graf, Paolo Bonzini, open list:Sheepdog,
	open list:sPAPR

We have several block drivers that understand BDRV_REQ_FUA,
and emulate it in the block layer for the rest by a full flush.
But without a way to actually request BDRV_REQ_FUA during a
pass-through blk_pwrite(), FUA-aware block drivers like NBD are
forced to repeat the emulation logic of a full flush regardless
of whether the backend they are writing to could do it more
efficiently.

This patch just wires up a flags argument; followup patches
will actually make use of it in the NBD driver and in qemu-io.

Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Denis V. Lunev <den@openvz.org>
---
 include/sysemu/block-backend.h |  3 ++-
 block/block-backend.c          |  6 ++++--
 block/crypto.c                 |  2 +-
 block/parallels.c              |  2 +-
 block/qcow.c                   |  8 ++++----
 block/qcow2.c                  |  4 ++--
 block/qed.c                    |  6 +++---
 block/sheepdog.c               |  2 +-
 block/vdi.c                    |  4 ++--
 block/vhdx.c                   |  5 +++--
 block/vmdk.c                   | 10 +++++-----
 block/vpc.c                    | 10 +++++-----
 hw/nvram/spapr_nvram.c         |  4 ++--
 nbd/server.c                   |  2 +-
 qemu-io-cmds.c                 |  2 +-
 15 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index c62b6fe..6991b26 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -102,7 +102,8 @@ BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
                                  int nb_sectors, BdrvRequestFlags flags,
                                  BlockCompletionFunc *cb, void *opaque);
 int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int count);
-int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count);
+int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count,
+               BdrvRequestFlags flags);
 int64_t blk_getlength(BlockBackend *blk);
 void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr);
 int64_t blk_nb_sectors(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index a7623e8..96c1d7c 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -953,9 +953,11 @@ int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int count)
     return count;
 }

-int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count)
+int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count,
+               BdrvRequestFlags flags)
 {
-    int ret = blk_prw(blk, offset, (void*) buf, count, blk_write_entry, 0);
+    int ret = blk_prw(blk, offset, (void *) buf, count, blk_write_entry,
+                      flags);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/crypto.c b/block/crypto.c
index 1903e84..32ba17c 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -91,7 +91,7 @@ static ssize_t block_crypto_write_func(QCryptoBlock *block,
     struct BlockCryptoCreateData *data = opaque;
     ssize_t ret;

-    ret = blk_pwrite(data->blk, offset, buf, buflen);
+    ret = blk_pwrite(data->blk, offset, buf, buflen, 0);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not write encryption header");
         return ret;
diff --git a/block/parallels.c b/block/parallels.c
index 324ed43..2d8bc87 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -512,7 +512,7 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
     memset(tmp, 0, sizeof(tmp));
     memcpy(tmp, &header, sizeof(header));

-    ret = blk_pwrite(file, 0, tmp, BDRV_SECTOR_SIZE);
+    ret = blk_pwrite(file, 0, tmp, BDRV_SECTOR_SIZE, 0);
     if (ret < 0) {
         goto exit;
     }
diff --git a/block/qcow.c b/block/qcow.c
index 60ddb12..d6dc1b0 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -853,14 +853,14 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
     }

     /* write all the data */
-    ret = blk_pwrite(qcow_blk, 0, &header, sizeof(header));
+    ret = blk_pwrite(qcow_blk, 0, &header, sizeof(header), 0);
     if (ret != sizeof(header)) {
         goto exit;
     }

     if (backing_file) {
         ret = blk_pwrite(qcow_blk, sizeof(header),
-            backing_file, backing_filename_len);
+                         backing_file, backing_filename_len, 0);
         if (ret != backing_filename_len) {
             goto exit;
         }
@@ -869,8 +869,8 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
     tmp = g_malloc0(BDRV_SECTOR_SIZE);
     for (i = 0; i < ((sizeof(uint64_t)*l1_size + BDRV_SECTOR_SIZE - 1)/
         BDRV_SECTOR_SIZE); i++) {
-        ret = blk_pwrite(qcow_blk, header_size +
-            BDRV_SECTOR_SIZE*i, tmp, BDRV_SECTOR_SIZE);
+        ret = blk_pwrite(qcow_blk, header_size + BDRV_SECTOR_SIZE * i,
+                         tmp, BDRV_SECTOR_SIZE, 0);
         if (ret != BDRV_SECTOR_SIZE) {
             g_free(tmp);
             goto exit;
diff --git a/block/qcow2.c b/block/qcow2.c
index 470734b..3090538 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2207,7 +2207,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
             cpu_to_be64(QCOW2_COMPAT_LAZY_REFCOUNTS);
     }

-    ret = blk_pwrite(blk, 0, header, cluster_size);
+    ret = blk_pwrite(blk, 0, header, cluster_size, 0);
     g_free(header);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not write qcow2 header");
@@ -2217,7 +2217,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     /* Write a refcount table with one refcount block */
     refcount_table = g_malloc0(2 * cluster_size);
     refcount_table[0] = cpu_to_be64(2 * cluster_size);
-    ret = blk_pwrite(blk, cluster_size, refcount_table, 2 * cluster_size);
+    ret = blk_pwrite(blk, cluster_size, refcount_table, 2 * cluster_size, 0);
     g_free(refcount_table);

     if (ret < 0) {
diff --git a/block/qed.c b/block/qed.c
index 0af5274..6cfd4c1 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -601,18 +601,18 @@ static int qed_create(const char *filename, uint32_t cluster_size,
     }

     qed_header_cpu_to_le(&header, &le_header);
-    ret = blk_pwrite(blk, 0, &le_header, sizeof(le_header));
+    ret = blk_pwrite(blk, 0, &le_header, sizeof(le_header), 0);
     if (ret < 0) {
         goto out;
     }
     ret = blk_pwrite(blk, sizeof(le_header), backing_file,
-                     header.backing_filename_size);
+                     header.backing_filename_size, 0);
     if (ret < 0) {
         goto out;
     }

     l1_table = g_malloc0(l1_size);
-    ret = blk_pwrite(blk, header.l1_table_offset, l1_table, l1_size);
+    ret = blk_pwrite(blk, header.l1_table_offset, l1_table, l1_size, 0);
     if (ret < 0) {
         goto out;
     }
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 33e0a33..625f876 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1678,7 +1678,7 @@ static int sd_prealloc(const char *filename, Error **errp)
         if (ret < 0) {
             goto out;
         }
-        ret = blk_pwrite(blk, idx * buf_size, buf, buf_size);
+        ret = blk_pwrite(blk, idx * buf_size, buf, buf_size, 0);
         if (ret < 0) {
             goto out;
         }
diff --git a/block/vdi.c b/block/vdi.c
index e5fe4e8..54e1144 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -827,7 +827,7 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
     vdi_header_print(&header);
 #endif
     vdi_header_to_le(&header);
-    ret = blk_pwrite(blk, offset, &header, sizeof(header));
+    ret = blk_pwrite(blk, offset, &header, sizeof(header), 0);
     if (ret < 0) {
         error_setg(errp, "Error writing header to %s", filename);
         goto exit;
@@ -848,7 +848,7 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
                 bmap[i] = VDI_UNALLOCATED;
             }
         }
-        ret = blk_pwrite(blk, offset, bmap, bmap_size);
+        ret = blk_pwrite(blk, offset, bmap, bmap_size, 0);
         if (ret < 0) {
             error_setg(errp, "Error writing bmap to %s", filename);
             goto exit;
diff --git a/block/vhdx.c b/block/vhdx.c
index 2b7b332..ec778fe 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1856,13 +1856,14 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
     creator = g_utf8_to_utf16("QEMU v" QEMU_VERSION, -1, NULL,
                               &creator_items, NULL);
     signature = cpu_to_le64(VHDX_FILE_SIGNATURE);
-    ret = blk_pwrite(blk, VHDX_FILE_ID_OFFSET, &signature, sizeof(signature));
+    ret = blk_pwrite(blk, VHDX_FILE_ID_OFFSET, &signature, sizeof(signature),
+                     0);
     if (ret < 0) {
         goto delete_and_exit;
     }
     if (creator) {
         ret = blk_pwrite(blk, VHDX_FILE_ID_OFFSET + sizeof(signature),
-                         creator, creator_items * sizeof(gunichar2));
+                         creator, creator_items * sizeof(gunichar2), 0);
         if (ret < 0) {
             goto delete_and_exit;
         }
diff --git a/block/vmdk.c b/block/vmdk.c
index f243527..908d619 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1808,12 +1808,12 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
     header.check_bytes[3] = 0xa;

     /* write all the data */
-    ret = blk_pwrite(blk, 0, &magic, sizeof(magic));
+    ret = blk_pwrite(blk, 0, &magic, sizeof(magic), 0);
     if (ret < 0) {
         error_setg(errp, QERR_IO_ERROR);
         goto exit;
     }
-    ret = blk_pwrite(blk, sizeof(magic), &header, sizeof(header));
+    ret = blk_pwrite(blk, sizeof(magic), &header, sizeof(header), 0);
     if (ret < 0) {
         error_setg(errp, QERR_IO_ERROR);
         goto exit;
@@ -1833,7 +1833,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
         gd_buf[i] = cpu_to_le32(tmp);
     }
     ret = blk_pwrite(blk, le64_to_cpu(header.rgd_offset) * BDRV_SECTOR_SIZE,
-                     gd_buf, gd_buf_size);
+                     gd_buf, gd_buf_size, 0);
     if (ret < 0) {
         error_setg(errp, QERR_IO_ERROR);
         goto exit;
@@ -1845,7 +1845,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
         gd_buf[i] = cpu_to_le32(tmp);
     }
     ret = blk_pwrite(blk, le64_to_cpu(header.gd_offset) * BDRV_SECTOR_SIZE,
-                     gd_buf, gd_buf_size);
+                     gd_buf, gd_buf_size, 0);
     if (ret < 0) {
         error_setg(errp, QERR_IO_ERROR);
         goto exit;
@@ -2108,7 +2108,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)

     blk_set_allow_write_beyond_eof(new_blk, true);

-    ret = blk_pwrite(new_blk, desc_offset, desc, desc_len);
+    ret = blk_pwrite(new_blk, desc_offset, desc, desc_len, 0);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not write description");
         goto exit;
diff --git a/block/vpc.c b/block/vpc.c
index 2da4126..0379813 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -788,13 +788,13 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t *buf,
     block_size = 0x200000;
     num_bat_entries = (total_sectors + block_size / 512) / (block_size / 512);

-    ret = blk_pwrite(blk, offset, buf, HEADER_SIZE);
+    ret = blk_pwrite(blk, offset, buf, HEADER_SIZE, 0);
     if (ret < 0) {
         goto fail;
     }

     offset = 1536 + ((num_bat_entries * 4 + 511) & ~511);
-    ret = blk_pwrite(blk, offset, buf, HEADER_SIZE);
+    ret = blk_pwrite(blk, offset, buf, HEADER_SIZE, 0);
     if (ret < 0) {
         goto fail;
     }
@@ -804,7 +804,7 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t *buf,

     memset(buf, 0xFF, 512);
     for (i = 0; i < (num_bat_entries * 4 + 511) / 512; i++) {
-        ret = blk_pwrite(blk, offset, buf, 512);
+        ret = blk_pwrite(blk, offset, buf, 512, 0);
         if (ret < 0) {
             goto fail;
         }
@@ -831,7 +831,7 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t *buf,
     /* Write the header */
     offset = 512;

-    ret = blk_pwrite(blk, offset, buf, 1024);
+    ret = blk_pwrite(blk, offset, buf, 1024, 0);
     if (ret < 0) {
         goto fail;
     }
@@ -853,7 +853,7 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t *buf,
         return ret;
     }

-    ret = blk_pwrite(blk, total_size - HEADER_SIZE, buf, HEADER_SIZE);
+    ret = blk_pwrite(blk, total_size - HEADER_SIZE, buf, HEADER_SIZE, 0);
     if (ret < 0) {
         return ret;
     }
diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
index 802636e..019f25d 100644
--- a/hw/nvram/spapr_nvram.c
+++ b/hw/nvram/spapr_nvram.c
@@ -124,7 +124,7 @@ static void rtas_nvram_store(PowerPCCPU *cpu, sPAPRMachineState *spapr,

     alen = len;
     if (nvram->blk) {
-        alen = blk_pwrite(nvram->blk, offset, membuf, len);
+        alen = blk_pwrite(nvram->blk, offset, membuf, len, 0);
     }

     assert(nvram->buf);
@@ -190,7 +190,7 @@ static int spapr_nvram_post_load(void *opaque, int version_id)
     sPAPRNVRAM *nvram = VIO_SPAPR_NVRAM(opaque);

     if (nvram->blk) {
-        int alen = blk_pwrite(nvram->blk, 0, nvram->buf, nvram->size);
+        int alen = blk_pwrite(nvram->blk, 0, nvram->buf, nvram->size, 0);

         if (alen < 0) {
             return alen;
diff --git a/nbd/server.c b/nbd/server.c
index 2184c64..fa862cd 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1115,7 +1115,7 @@ static void nbd_trip(void *opaque)
         TRACE("Writing to device");

         ret = blk_pwrite(exp->blk, request.from + exp->dev_offset,
-                        req->data, request.len);
+                         req->data, request.len, 0);
         if (ret < 0) {
             LOG("writing to file failed");
             reply.error = -ret;
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index e34f777..e26e543 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -474,7 +474,7 @@ static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset,
         return -ERANGE;
     }

-    *total = blk_pwrite(blk, offset, (uint8_t *)buf, count);
+    *total = blk_pwrite(blk, offset, (uint8_t *)buf, count, 0);
     if (*total < 0) {
         return *total;
     }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 02/14] fdc: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 01/14] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 03/14] nand: " Eric Blake
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, John Snow, Max Reitz

Sector-based blk_write() should die; switch to byte-based
blk_pwrite() instead.  Likewise for blk_read().

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

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 3722275..f73af7d 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -223,6 +223,13 @@ static int fd_sector(FDrive *drv)
                           NUM_SIDES(drv));
 }

+/* Returns current position, in bytes, for given drive */
+static int fd_offset(FDrive *drv)
+{
+    g_assert(fd_sector(drv) < INT_MAX >> BDRV_SECTOR_BITS);
+    return fd_sector(drv) << BDRV_SECTOR_BITS;
+}
+
 /* Seek to a new position:
  * returns 0 if already on right track
  * returns 1 if track changed
@@ -1629,8 +1636,8 @@ static int fdctrl_transfer_handler (void *opaque, int nchan,
         if (fdctrl->data_dir != FD_DIR_WRITE ||
             len < FD_SECTOR_LEN || rel_pos != 0) {
             /* READ & SCAN commands and realign to a sector for WRITE */
-            if (blk_read(cur_drv->blk, fd_sector(cur_drv),
-                         fdctrl->fifo, 1) < 0) {
+            if (blk_pread(cur_drv->blk, fd_offset(cur_drv),
+                          fdctrl->fifo, BDRV_SECTOR_SIZE) < 0) {
                 FLOPPY_DPRINTF("Floppy: error getting sector %d\n",
                                fd_sector(cur_drv));
                 /* Sure, image size is too small... */
@@ -1657,8 +1664,8 @@ static int fdctrl_transfer_handler (void *opaque, int nchan,

             k->read_memory(fdctrl->dma, nchan, fdctrl->fifo + rel_pos,
                            fdctrl->data_pos, len);
-            if (blk_write(cur_drv->blk, fd_sector(cur_drv),
-                          fdctrl->fifo, 1) < 0) {
+            if (blk_pwrite(cur_drv->blk, fd_offset(cur_drv),
+                           fdctrl->fifo, BDRV_SECTOR_SIZE, 0) < 0) {
                 FLOPPY_DPRINTF("error writing sector %d\n",
                                fd_sector(cur_drv));
                 fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
@@ -1741,7 +1748,8 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
                                    fd_sector(cur_drv));
                     return 0;
                 }
-            if (blk_read(cur_drv->blk, fd_sector(cur_drv), fdctrl->fifo, 1)
+            if (blk_pread(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
+                          BDRV_SECTOR_SIZE)
                 < 0) {
                 FLOPPY_DPRINTF("error getting sector %d\n",
                                fd_sector(cur_drv));
@@ -1820,7 +1828,8 @@ static void fdctrl_format_sector(FDCtrl *fdctrl)
     }
     memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
     if (cur_drv->blk == NULL ||
-        blk_write(cur_drv->blk, fd_sector(cur_drv), fdctrl->fifo, 1) < 0) {
+        blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
+                   BDRV_SECTOR_SIZE, 0) < 0) {
         FLOPPY_DPRINTF("error formatting sector %d\n", fd_sector(cur_drv));
         fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
     } else {
@@ -2243,8 +2252,8 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
         if (pos == FD_SECTOR_LEN - 1 ||
             fdctrl->data_pos == fdctrl->data_len) {
             cur_drv = get_cur_drv(fdctrl);
-            if (blk_write(cur_drv->blk, fd_sector(cur_drv), fdctrl->fifo, 1)
-                < 0) {
+            if (blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
+                           BDRV_SECTOR_SIZE, 0) < 0) {
                 FLOPPY_DPRINTF("error writing sector %d\n",
                                fd_sector(cur_drv));
                 break;
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 03/14] nand: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 01/14] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 02/14] fdc: Switch to byte-based block access Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 04/14] onenand: " Eric Blake
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz

Sector-based blk_write() should die; switch to byte-based
blk_pwrite() instead.  Likewise for blk_read().

This file is doing some complex computations to map various
flash page sizes (256, 512, and 2048) atop generic uses of
512-byte sector operations.  Perhaps someone will want to tidy
up the file for fewer gymnastics in managing addresses and
offsets, and less wasteful visits of 256-byte pages, but it
was out of scope for this series, where I just went with the
mechanical conversion.

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

---
v5: fix missing edit
---
 hw/block/nand.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/hw/block/nand.c b/hw/block/nand.c
index 29c6596..c69e675 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -663,7 +663,8 @@ static void glue(nand_blk_write_, PAGE_SIZE)(NANDFlashState *s)
         sector = SECTOR(s->addr);
         off = (s->addr & PAGE_MASK) + s->offset;
         soff = SECTOR_OFFSET(s->addr);
-        if (blk_read(s->blk, sector, iobuf, PAGE_SECTORS) < 0) {
+        if (blk_pread(s->blk, sector << BDRV_SECTOR_BITS, iobuf,
+                      PAGE_SECTORS << BDRV_SECTOR_BITS) < 0) {
             printf("%s: read error in sector %" PRIu64 "\n", __func__, sector);
             return;
         }
@@ -675,21 +676,24 @@ static void glue(nand_blk_write_, PAGE_SIZE)(NANDFlashState *s)
                             MIN(OOB_SIZE, off + s->iolen - PAGE_SIZE));
         }

-        if (blk_write(s->blk, sector, iobuf, PAGE_SECTORS) < 0) {
+        if (blk_pwrite(s->blk, sector << BDRV_SECTOR_BITS, iobuf,
+                       PAGE_SECTORS << BDRV_SECTOR_BITS, 0) < 0) {
             printf("%s: write error in sector %" PRIu64 "\n", __func__, sector);
         }
     } else {
         off = PAGE_START(s->addr) + (s->addr & PAGE_MASK) + s->offset;
         sector = off >> 9;
         soff = off & 0x1ff;
-        if (blk_read(s->blk, sector, iobuf, PAGE_SECTORS + 2) < 0) {
+        if (blk_pread(s->blk, sector << BDRV_SECTOR_BITS, iobuf,
+                      (PAGE_SECTORS + 2) << BDRV_SECTOR_BITS) < 0) {
             printf("%s: read error in sector %" PRIu64 "\n", __func__, sector);
             return;
         }

         mem_and(iobuf + soff, s->io, s->iolen);

-        if (blk_write(s->blk, sector, iobuf, PAGE_SECTORS + 2) < 0) {
+        if (blk_pwrite(s->blk, sector << BDRV_SECTOR_BITS, iobuf,
+                       (PAGE_SECTORS + 2) << BDRV_SECTOR_BITS, 0) < 0) {
             printf("%s: write error in sector %" PRIu64 "\n", __func__, sector);
         }
     }
@@ -716,17 +720,20 @@ static void glue(nand_blk_erase_, PAGE_SIZE)(NANDFlashState *s)
         i = SECTOR(addr);
         page = SECTOR(addr + (1 << (ADDR_SHIFT + s->erase_shift)));
         for (; i < page; i ++)
-            if (blk_write(s->blk, i, iobuf, 1) < 0) {
+            if (blk_pwrite(s->blk, i << BDRV_SECTOR_BITS, iobuf,
+                           BDRV_SECTOR_SIZE, 0) < 0) {
                 printf("%s: write error in sector %" PRIu64 "\n", __func__, i);
             }
     } else {
         addr = PAGE_START(addr);
         page = addr >> 9;
-        if (blk_read(s->blk, page, iobuf, 1) < 0) {
+        if (blk_pread(s->blk, page << BDRV_SECTOR_BITS, iobuf,
+                      BDRV_SECTOR_SIZE) < 0) {
             printf("%s: read error in sector %" PRIu64 "\n", __func__, page);
         }
         memset(iobuf + (addr & 0x1ff), 0xff, (~addr & 0x1ff) + 1);
-        if (blk_write(s->blk, page, iobuf, 1) < 0) {
+        if (blk_pwrite(s->blk, page << BDRV_SECTOR_BITS, iobuf,
+                       BDRV_SECTOR_SIZE, 0) < 0) {
             printf("%s: write error in sector %" PRIu64 "\n", __func__, page);
         }

@@ -734,18 +741,20 @@ static void glue(nand_blk_erase_, PAGE_SIZE)(NANDFlashState *s)
         i = (addr & ~0x1ff) + 0x200;
         for (addr += ((PAGE_SIZE + OOB_SIZE) << s->erase_shift) - 0x200;
                         i < addr; i += 0x200) {
-            if (blk_write(s->blk, i >> 9, iobuf, 1) < 0) {
+            if (blk_pwrite(s->blk, i, iobuf, BDRV_SECTOR_SIZE, 0) < 0) {
                 printf("%s: write error in sector %" PRIu64 "\n",
                        __func__, i >> 9);
             }
         }

         page = i >> 9;
-        if (blk_read(s->blk, page, iobuf, 1) < 0) {
+        if (blk_pread(s->blk, page << BDRV_SECTOR_BITS, iobuf,
+                      BDRV_SECTOR_SIZE) < 0) {
             printf("%s: read error in sector %" PRIu64 "\n", __func__, page);
         }
         memset(iobuf, 0xff, ((addr - 1) & 0x1ff) + 1);
-        if (blk_write(s->blk, page, iobuf, 1) < 0) {
+        if (blk_pwrite(s->blk, page << BDRV_SECTOR_BITS, iobuf,
+                       BDRV_SECTOR_SIZE, 0) < 0) {
             printf("%s: write error in sector %" PRIu64 "\n", __func__, page);
         }
     }
@@ -760,7 +769,8 @@ static void glue(nand_blk_load_, PAGE_SIZE)(NANDFlashState *s,

     if (s->blk) {
         if (s->mem_oob) {
-            if (blk_read(s->blk, SECTOR(addr), s->io, PAGE_SECTORS) < 0) {
+            if (blk_pread(s->blk, SECTOR(addr) << BDRV_SECTOR_BITS, s->io,
+                          PAGE_SECTORS << BDRV_SECTOR_BITS) < 0) {
                 printf("%s: read error in sector %" PRIu64 "\n",
                                 __func__, SECTOR(addr));
             }
@@ -769,8 +779,8 @@ static void glue(nand_blk_load_, PAGE_SIZE)(NANDFlashState *s,
                             OOB_SIZE);
             s->ioaddr = s->io + SECTOR_OFFSET(s->addr) + offset;
         } else {
-            if (blk_read(s->blk, PAGE_START(addr) >> 9,
-                         s->io, (PAGE_SECTORS + 2)) < 0) {
+            if (blk_pread(s->blk, PAGE_START(addr), s->io,
+                          (PAGE_SECTORS + 2) << BDRV_SECTOR_BITS) < 0) {
                 printf("%s: read error in sector %" PRIu64 "\n",
                                 __func__, PAGE_START(addr) >> 9);
             }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 04/14] onenand: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (2 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 03/14] nand: " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 05/14] pflash: " Eric Blake
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz

Sector-based blk_write() should die; switch to byte-based
blk_pwrite() instead.  Likewise for blk_read().

This particular device picks its size during onenand_initfn(),
and can be at most 0x80000000 bytes; therefore, shifting an
'int sec' request to get back to a byte offset should never
overflow 32 bits.  But adding assertions to document that point
should not hurt.

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

---
v5: compile-tested, add asserts that size never exceeds 0x80000000,
so that we can consistently use uint32_t
---
 hw/block/onenand.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/hw/block/onenand.c b/hw/block/onenand.c
index 883f4b1..8d84227 100644
--- a/hw/block/onenand.c
+++ b/hw/block/onenand.c
@@ -224,7 +224,8 @@ static void onenand_reset(OneNANDState *s, int cold)
         /* Lock the whole flash */
         memset(s->blockwp, ONEN_LOCK_LOCKED, s->blocks);

-        if (s->blk_cur && blk_read(s->blk_cur, 0, s->boot[0], 8) < 0) {
+        if (s->blk_cur && blk_pread(s->blk_cur, 0, s->boot[0],
+                                    8 << BDRV_SECTOR_BITS) < 0) {
             hw_error("%s: Loading the BootRAM failed.\n", __func__);
         }
     }
@@ -240,8 +241,11 @@ static void onenand_system_reset(DeviceState *dev)
 static inline int onenand_load_main(OneNANDState *s, int sec, int secn,
                 void *dest)
 {
+    assert(UINT32_MAX >> BDRV_SECTOR_BITS > sec);
+    assert(UINT32_MAX >> BDRV_SECTOR_BITS > secn);
     if (s->blk_cur) {
-        return blk_read(s->blk_cur, sec, dest, secn) < 0;
+        return blk_pread(s->blk_cur, sec << BDRV_SECTOR_BITS, dest,
+                         secn << BDRV_SECTOR_BITS) < 0;
     } else if (sec + secn > s->secs_cur) {
         return 1;
     }
@@ -257,19 +261,22 @@ static inline int onenand_prog_main(OneNANDState *s, int sec, int secn,
     int result = 0;

     if (secn > 0) {
-        uint32_t size = (uint32_t)secn * 512;
+        uint32_t size = secn << BDRV_SECTOR_BITS;
+        uint32_t offset = sec << BDRV_SECTOR_BITS;
+        assert(UINT32_MAX >> BDRV_SECTOR_BITS > sec);
+        assert(UINT32_MAX >> BDRV_SECTOR_BITS > secn);
         const uint8_t *sp = (const uint8_t *)src;
         uint8_t *dp = 0;
         if (s->blk_cur) {
             dp = g_malloc(size);
-            if (!dp || blk_read(s->blk_cur, sec, dp, secn) < 0) {
+            if (!dp || blk_pread(s->blk_cur, offset, dp, size) < 0) {
                 result = 1;
             }
         } else {
             if (sec + secn > s->secs_cur) {
                 result = 1;
             } else {
-                dp = (uint8_t *)s->current + (sec << 9);
+                dp = (uint8_t *)s->current + offset;
             }
         }
         if (!result) {
@@ -278,7 +285,7 @@ static inline int onenand_prog_main(OneNANDState *s, int sec, int secn,
                 dp[i] &= sp[i];
             }
             if (s->blk_cur) {
-                result = blk_write(s->blk_cur, sec, dp, secn) < 0;
+                result = blk_pwrite(s->blk_cur, offset, dp, size, 0) < 0;
             }
         }
         if (dp && s->blk_cur) {
@@ -295,7 +302,8 @@ static inline int onenand_load_spare(OneNANDState *s, int sec, int secn,
     uint8_t buf[512];

     if (s->blk_cur) {
-        if (blk_read(s->blk_cur, s->secs_cur + (sec >> 5), buf, 1) < 0) {
+        uint32_t offset = (s->secs_cur + (sec >> 5)) << BDRV_SECTOR_BITS;
+        if (blk_pread(s->blk_cur, offset, buf, BDRV_SECTOR_SIZE) < 0) {
             return 1;
         }
         memcpy(dest, buf + ((sec & 31) << 4), secn << 4);
@@ -304,7 +312,7 @@ static inline int onenand_load_spare(OneNANDState *s, int sec, int secn,
     } else {
         memcpy(dest, s->current + (s->secs_cur << 9) + (sec << 4), secn << 4);
     }
- 
+
     return 0;
 }

@@ -315,10 +323,12 @@ static inline int onenand_prog_spare(OneNANDState *s, int sec, int secn,
     if (secn > 0) {
         const uint8_t *sp = (const uint8_t *)src;
         uint8_t *dp = 0, *dpp = 0;
+        uint32_t offset = (s->secs_cur + (sec >> 5)) << BDRV_SECTOR_BITS;
+        assert(UINT32_MAX >> BDRV_SECTOR_BITS > s->secs_cur + (sec >> 5));
         if (s->blk_cur) {
             dp = g_malloc(512);
             if (!dp
-                || blk_read(s->blk_cur, s->secs_cur + (sec >> 5), dp, 1) < 0) {
+                || blk_pread(s->blk_cur, offset, dp, BDRV_SECTOR_SIZE) < 0) {
                 result = 1;
             } else {
                 dpp = dp + ((sec & 31) << 4);
@@ -336,8 +346,8 @@ static inline int onenand_prog_spare(OneNANDState *s, int sec, int secn,
                 dpp[i] &= sp[i];
             }
             if (s->blk_cur) {
-                result = blk_write(s->blk_cur, s->secs_cur + (sec >> 5),
-                                   dp, 1) < 0;
+                result = blk_pwrite(s->blk_cur, offset, dp,
+                                    BDRV_SECTOR_SIZE, 0) < 0;
             }
         }
         g_free(dp);
@@ -355,14 +365,17 @@ static inline int onenand_erase(OneNANDState *s, int sec, int num)
     for (; num > 0; num--, sec++) {
         if (s->blk_cur) {
             int erasesec = s->secs_cur + (sec >> 5);
-            if (blk_write(s->blk_cur, sec, blankbuf, 1) < 0) {
+            if (blk_pwrite(s->blk_cur, sec << BDRV_SECTOR_BITS, blankbuf,
+                           BDRV_SECTOR_SIZE, 0) < 0) {
                 goto fail;
             }
-            if (blk_read(s->blk_cur, erasesec, tmpbuf, 1) < 0) {
+            if (blk_pread(s->blk_cur, erasesec << BDRV_SECTOR_BITS, tmpbuf,
+                          BDRV_SECTOR_SIZE) < 0) {
                 goto fail;
             }
             memcpy(tmpbuf + ((sec & 31) << 4), blankbuf, 1 << 4);
-            if (blk_write(s->blk_cur, erasesec, tmpbuf, 1) < 0) {
+            if (blk_pwrite(s->blk_cur, erasesec << BDRV_SECTOR_BITS, tmpbuf,
+                           BDRV_SECTOR_SIZE, 0) < 0) {
                 goto fail;
             }
         } else {
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 05/14] pflash: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (3 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 04/14] onenand: " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 06/14] sd: " Eric Blake
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz

Sector-based blk_write() should die; switch to byte-based
blk_pwrite() instead.  Likewise for blk_read().

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 hw/block/pflash_cfi01.c | 12 ++++++------
 hw/block/pflash_cfi02.c | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 106a775..3a1f85d 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -413,11 +413,11 @@ static void pflash_update(pflash_t *pfl, int offset,
     int offset_end;
     if (pfl->blk) {
         offset_end = offset + size;
-        /* round to sectors */
-        offset = offset >> 9;
-        offset_end = (offset_end + 511) >> 9;
-        blk_write(pfl->blk, offset, pfl->storage + (offset << 9),
-                  offset_end - offset);
+        /* widen to sector boundaries */
+        offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE);
+        offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE);
+        blk_pwrite(pfl->blk, offset, pfl->storage + offset,
+                   offset_end - offset, 0);
     }
 }

@@ -739,7 +739,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)

     if (pfl->blk) {
         /* read the initial flash content */
-        ret = blk_read(pfl->blk, 0, pfl->storage, total_len >> 9);
+        ret = blk_pread(pfl->blk, 0, pfl->storage, total_len);

         if (ret < 0) {
             vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index b13172c..5f10610 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -253,11 +253,11 @@ static void pflash_update(pflash_t *pfl, int offset,
     int offset_end;
     if (pfl->blk) {
         offset_end = offset + size;
-        /* round to sectors */
-        offset = offset >> 9;
-        offset_end = (offset_end + 511) >> 9;
-        blk_write(pfl->blk, offset, pfl->storage + (offset << 9),
-                  offset_end - offset);
+        /* widen to sector boundaries */
+        offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE);
+        offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE);
+        blk_pwrite(pfl->blk, offset, pfl->storage + offset,
+                   offset_end - offset, 0);
     }
 }

@@ -622,7 +622,7 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
     pfl->chip_len = chip_len;
     if (pfl->blk) {
         /* read the initial flash content */
-        ret = blk_read(pfl->blk, 0, pfl->storage, chip_len >> 9);
+        ret = blk_pread(pfl->blk, 0, pfl->storage, chip_len);
         if (ret < 0) {
             vmstate_unregister_ram(&pfl->orig_mem, DEVICE(pfl));
             error_setg(errp, "failed to read the initial flash content");
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 06/14] sd: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (4 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 05/14] pflash: " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 07/14] m25p80: " Eric Blake
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf

Sector-based blk_write() should die; switch to byte-based
blk_pwrite() instead.  Likewise for blk_read().

Greatly simplifies the code, now that we let the block layer
take care of alignment and read-modify-write on our behalf :)
In fact, we no longer need to include 'buf' in the migration
stream (although we do have to ensure that the stream remains
compatible).

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

---
v5: fix bug in sd_blk_write, drop sd->buf
---
 hw/sd/sd.c | 51 ++++-----------------------------------------------
 1 file changed, 4 insertions(+), 47 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b66e5d2..87e3d23 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -123,7 +123,6 @@ struct SDState {
     qemu_irq readonly_cb;
     qemu_irq inserted_cb;
     BlockBackend *blk;
-    uint8_t *buf;

     bool enable;
 };
@@ -551,7 +550,7 @@ static const VMStateDescription sd_vmstate = {
         VMSTATE_UINT64(data_start, SDState),
         VMSTATE_UINT32(data_offset, SDState),
         VMSTATE_UINT8_ARRAY(data, SDState, 512),
-        VMSTATE_BUFFER_POINTER_UNSAFE(buf, SDState, 1, 512),
+        VMSTATE_UNUSED_V(1, 512),
         VMSTATE_BOOL(enable, SDState),
         VMSTATE_END_OF_LIST()
     },
@@ -1577,57 +1576,17 @@ send_response:

 static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 {
-    uint64_t end = addr + len;
-
     DPRINTF("sd_blk_read: addr = 0x%08llx, len = %d\n",
             (unsigned long long) addr, len);
-    if (!sd->blk || blk_read(sd->blk, addr >> 9, sd->buf, 1) < 0) {
+    if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) {
         fprintf(stderr, "sd_blk_read: read error on host side\n");
-        return;
     }
-
-    if (end > (addr & ~511) + 512) {
-        memcpy(sd->data, sd->buf + (addr & 511), 512 - (addr & 511));
-
-        if (blk_read(sd->blk, end >> 9, sd->buf, 1) < 0) {
-            fprintf(stderr, "sd_blk_read: read error on host side\n");
-            return;
-        }
-        memcpy(sd->data + 512 - (addr & 511), sd->buf, end & 511);
-    } else
-        memcpy(sd->data, sd->buf + (addr & 511), len);
 }

 static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 {
-    uint64_t end = addr + len;
-
-    if ((addr & 511) || len < 512)
-        if (!sd->blk || blk_read(sd->blk, addr >> 9, sd->buf, 1) < 0) {
-            fprintf(stderr, "sd_blk_write: read error on host side\n");
-            return;
-        }
-
-    if (end > (addr & ~511) + 512) {
-        memcpy(sd->buf + (addr & 511), sd->data, 512 - (addr & 511));
-        if (blk_write(sd->blk, addr >> 9, sd->buf, 1) < 0) {
-            fprintf(stderr, "sd_blk_write: write error on host side\n");
-            return;
-        }
-
-        if (blk_read(sd->blk, end >> 9, sd->buf, 1) < 0) {
-            fprintf(stderr, "sd_blk_write: read error on host side\n");
-            return;
-        }
-        memcpy(sd->buf, sd->data + 512 - (addr & 511), end & 511);
-        if (blk_write(sd->blk, end >> 9, sd->buf, 1) < 0) {
-            fprintf(stderr, "sd_blk_write: write error on host side\n");
-        }
-    } else {
-        memcpy(sd->buf + (addr & 511), sd->data, len);
-        if (!sd->blk || blk_write(sd->blk, addr >> 9, sd->buf, 1) < 0) {
-            fprintf(stderr, "sd_blk_write: write error on host side\n");
-        }
+    if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) {
+        fprintf(stderr, "sd_blk_write: write error on host side\n");
     }
 }

@@ -1925,8 +1884,6 @@ static void sd_realize(DeviceState *dev, Error **errp)
         return;
     }

-    sd->buf = blk_blockalign(sd->blk, 512);
-
     if (sd->blk) {
         blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
     }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 07/14] m25p80: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (5 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 06/14] sd: " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 08/14] atapi: " Eric Blake
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Peter Crosthwaite, Max Reitz

Sector-based blk_read() should die; switch to byte-based
blk_pread() instead.

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

---
v5: compile tested this time
---
 hw/block/m25p80.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 906b712..01c51a2 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -907,8 +907,7 @@ static int m25p80_init(SSISlave *ss)
         s->storage = blk_blockalign(s->blk, s->size);

         /* FIXME: Move to late init */
-        if (blk_read(s->blk, 0, s->storage,
-                     DIV_ROUND_UP(s->size, BDRV_SECTOR_SIZE))) {
+        if (blk_pread(s->blk, 0, s->storage, s->size)) {
             fprintf(stderr, "Failed to initialize SPI flash!\n");
             return 1;
         }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 08/14] atapi: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (6 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 07/14] m25p80: " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 09/14] nbd: " Eric Blake
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, John Snow

Sector-based blk_read() should die; switch to byte-based
blk_pread() instead.

Add new defines ATAPI_SECTOR_BITS and ATAPI_SECTOR_SIZE to
use anywhere we were previously scaling BDRV_SECTOR_* by 4,
for better legibility.

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

---
v4: add new defines for use in more places [jsnow]
---
 hw/ide/atapi.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 2bb606c..95056d9 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -28,6 +28,9 @@
 #include "hw/scsi/scsi.h"
 #include "sysemu/block-backend.h"

+#define ATAPI_SECTOR_BITS (2 + BDRV_SECTOR_BITS)
+#define ATAPI_SECTOR_SIZE (1 << ATAPI_SECTOR_BITS)
+
 static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret);

 static void padstr8(uint8_t *buf, int buf_size, const char *src)
@@ -111,7 +114,7 @@ cd_read_sector_sync(IDEState *s)
 {
     int ret;
     block_acct_start(blk_get_stats(s->blk), &s->acct,
-                     4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
+                     ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);

 #ifdef DEBUG_IDE_ATAPI
     printf("cd_read_sector_sync: lba=%d\n", s->lba);
@@ -119,12 +122,12 @@ cd_read_sector_sync(IDEState *s)

     switch (s->cd_sector_size) {
     case 2048:
-        ret = blk_read(s->blk, (int64_t)s->lba << 2,
-                       s->io_buffer, 4);
+        ret = blk_pread(s->blk, (int64_t)s->lba << ATAPI_SECTOR_BITS,
+                        s->io_buffer, ATAPI_SECTOR_SIZE);
         break;
     case 2352:
-        ret = blk_read(s->blk, (int64_t)s->lba << 2,
-                       s->io_buffer + 16, 4);
+        ret = blk_pread(s->blk, (int64_t)s->lba << ATAPI_SECTOR_BITS,
+                        s->io_buffer + 16, ATAPI_SECTOR_SIZE);
         if (ret >= 0) {
             cd_data_to_raw(s->io_buffer, s->lba);
         }
@@ -182,7 +185,7 @@ static int cd_read_sector(IDEState *s)
     s->iov.iov_base = (s->cd_sector_size == 2352) ?
                       s->io_buffer + 16 : s->io_buffer;

-    s->iov.iov_len = 4 * BDRV_SECTOR_SIZE;
+    s->iov.iov_len = ATAPI_SECTOR_SIZE;
     qemu_iovec_init_external(&s->qiov, &s->iov, 1);

 #ifdef DEBUG_IDE_ATAPI
@@ -190,7 +193,7 @@ static int cd_read_sector(IDEState *s)
 #endif

     block_acct_start(blk_get_stats(s->blk), &s->acct,
-                     4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
+                     ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);

     ide_buffered_readv(s, (int64_t)s->lba << 2, &s->qiov, 4,
                        cd_read_sector_cb, s);
@@ -435,7 +438,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
 #endif

     s->bus->dma->iov.iov_base = (void *)(s->io_buffer + data_offset);
-    s->bus->dma->iov.iov_len = n * 4 * 512;
+    s->bus->dma->iov.iov_len = n * ATAPI_SECTOR_SIZE;
     qemu_iovec_init_external(&s->bus->dma->qiov, &s->bus->dma->iov, 1);

     s->bus->dma->aiocb = ide_buffered_readv(s, (int64_t)s->lba << 2,
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 09/14] nbd: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (7 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 08/14] atapi: " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 10/14] qemu-img: " Eric Blake
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Paolo Bonzini

Sector-based blk_read() should die; switch to byte-based
blk_pread() instead.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-nbd.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index c55b40f..c07ceef 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -159,12 +159,13 @@ static int find_partition(BlockBackend *blk, int partition,
                           off_t *offset, off_t *size)
 {
     struct partition_record mbr[4];
-    uint8_t data[512];
+    uint8_t data[BDRV_SECTOR_SIZE];
     int i;
     int ext_partnum = 4;
     int ret;

-    if ((ret = blk_read(blk, 0, data, 1)) < 0) {
+    ret = blk_pread(blk, 0, data, sizeof(data));
+    if (ret < 0) {
         error_report("error while reading: %s", strerror(-ret));
         exit(EXIT_FAILURE);
     }
@@ -182,10 +183,12 @@ static int find_partition(BlockBackend *blk, int partition,

         if (mbr[i].system == 0xF || mbr[i].system == 0x5) {
             struct partition_record ext[4];
-            uint8_t data1[512];
+            uint8_t data1[BDRV_SECTOR_SIZE];
             int j;

-            if ((ret = blk_read(blk, mbr[i].start_sector_abs, data1, 1)) < 0) {
+            ret = blk_pread(blk, mbr[i].start_sector_abs << BDRV_SECTOR_BITS,
+                            data1, sizeof(data1));
+            if (ret < 0) {
                 error_report("error while reading: %s", strerror(-ret));
                 exit(EXIT_FAILURE);
             }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 10/14] qemu-img: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (8 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 09/14] nbd: " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 11/14] qemu-io: " Eric Blake
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz

Sector-based blk_write() should die; switch to byte-based
blk_pwrite() instead.  Likewise for blk_read().

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-img.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 46f2a6d..c19f9d4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1088,7 +1088,8 @@ static int check_empty_sectors(BlockBackend *blk, int64_t sect_num,
                                uint8_t *buffer, bool quiet)
 {
     int pnum, ret = 0;
-    ret = blk_read(blk, sect_num, buffer, sect_count);
+    ret = blk_pread(blk, sect_num << BDRV_SECTOR_BITS, buffer,
+                    sect_count << BDRV_SECTOR_BITS);
     if (ret < 0) {
         error_report("Error while reading offset %" PRId64 " of %s: %s",
                      sectors_to_bytes(sect_num), filename, strerror(-ret));
@@ -1301,7 +1302,8 @@ static int img_compare(int argc, char **argv)
             nb_sectors = MIN(pnum1, pnum2);
         } else if (allocated1 == allocated2) {
             if (allocated1) {
-                ret = blk_read(blk1, sector_num, buf1, nb_sectors);
+                ret = blk_pread(blk1, sector_num << BDRV_SECTOR_BITS, buf1,
+                                nb_sectors << BDRV_SECTOR_BITS);
                 if (ret < 0) {
                     error_report("Error while reading offset %" PRId64 " of %s:"
                                  " %s", sectors_to_bytes(sector_num), filename1,
@@ -1309,7 +1311,8 @@ static int img_compare(int argc, char **argv)
                     ret = 4;
                     goto out;
                 }
-                ret = blk_read(blk2, sector_num, buf2, nb_sectors);
+                ret = blk_pread(blk2, sector_num << BDRV_SECTOR_BITS, buf2,
+                                nb_sectors << BDRV_SECTOR_BITS);
                 if (ret < 0) {
                     error_report("Error while reading offset %" PRId64
                                  " of %s: %s", sectors_to_bytes(sector_num),
@@ -1522,7 +1525,9 @@ static int convert_read(ImgConvertState *s, int64_t sector_num, int nb_sectors,
         bs_sectors = s->src_sectors[s->src_cur];

         n = MIN(nb_sectors, bs_sectors - (sector_num - s->src_cur_offset));
-        ret = blk_read(blk, sector_num - s->src_cur_offset, buf, n);
+        ret = blk_pread(blk,
+                        (sector_num - s->src_cur_offset) << BDRV_SECTOR_BITS,
+                        buf, n << BDRV_SECTOR_BITS);
         if (ret < 0) {
             return ret;
         }
@@ -1577,7 +1582,8 @@ static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors,
             if (!s->min_sparse ||
                 is_allocated_sectors_min(buf, n, &n, s->min_sparse))
             {
-                ret = blk_write(s->target, sector_num, buf, n);
+                ret = blk_pwrite(s->target, sector_num << BDRV_SECTOR_BITS,
+                                 buf, n << BDRV_SECTOR_BITS, 0);
                 if (ret < 0) {
                     return ret;
                 }
@@ -3023,7 +3029,8 @@ static int img_rebase(int argc, char **argv)
                     n = old_backing_num_sectors - sector;
                 }

-                ret = blk_read(blk_old_backing, sector, buf_old, n);
+                ret = blk_pread(blk_old_backing, sector << BDRV_SECTOR_BITS,
+                                buf_old, n << BDRV_SECTOR_BITS);
                 if (ret < 0) {
                     error_report("error while reading from old backing file");
                     goto out;
@@ -3037,7 +3044,8 @@ static int img_rebase(int argc, char **argv)
                     n = new_backing_num_sectors - sector;
                 }

-                ret = blk_read(blk_new_backing, sector, buf_new, n);
+                ret = blk_pread(blk_new_backing, sector << BDRV_SECTOR_BITS,
+                                buf_new, n << BDRV_SECTOR_BITS);
                 if (ret < 0) {
                     error_report("error while reading from new backing file");
                     goto out;
@@ -3053,8 +3061,10 @@ static int img_rebase(int argc, char **argv)
                 if (compare_sectors(buf_old + written * 512,
                     buf_new + written * 512, n - written, &pnum))
                 {
-                    ret = blk_write(blk, sector + written,
-                                    buf_old + written * 512, pnum);
+                    ret = blk_pwrite(blk,
+                                     (sector + written) << BDRV_SECTOR_BITS,
+                                     buf_old + written * 512,
+                                     pnum << BDRV_SECTOR_BITS, 0);
                     if (ret < 0) {
                         error_report("Error while writing to COW image: %s",
                             strerror(-ret));
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 11/14] qemu-io: Switch to byte-based block access
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (9 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 10/14] qemu-img: " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-04 13:10   ` Kevin Wolf
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 12/14] block: Switch blk_read_unthrottled() to byte interface Eric Blake
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz

blk_write() and blk_read() are now very simple wrappers around
blk_pwrite() and blk_pread().  There's no reason to require
the user to pass in aligned numbers.  Keep 'read -p' and
'write -p' so that I don't have to hunt down and update all
users of qemu-io, but make the default 'read' and 'write' now
do the same behavior that used to require -p.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-io-cmds.c | 75 +++++++++++++---------------------------------------------
 1 file changed, 16 insertions(+), 59 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index e26e543..4184fb8 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -419,40 +419,6 @@ fail:
     return buf;
 }

-static int do_read(BlockBackend *blk, char *buf, int64_t offset, int64_t count,
-                   int64_t *total)
-{
-    int ret;
-
-    if (count >> 9 > INT_MAX) {
-        return -ERANGE;
-    }
-
-    ret = blk_read(blk, offset >> 9, (uint8_t *)buf, count >> 9);
-    if (ret < 0) {
-        return ret;
-    }
-    *total = count;
-    return 1;
-}
-
-static int do_write(BlockBackend *blk, char *buf, int64_t offset, int64_t count,
-                    int64_t *total)
-{
-    int ret;
-
-    if (count >> 9 > INT_MAX) {
-        return -ERANGE;
-    }
-
-    ret = blk_write(blk, offset >> 9, (uint8_t *)buf, count >> 9);
-    if (ret < 0) {
-        return ret;
-    }
-    *total = count;
-    return 1;
-}
-
 static int do_pread(BlockBackend *blk, char *buf, int64_t offset,
                     int64_t count, int64_t *total)
 {
@@ -671,7 +637,7 @@ static void read_help(void)
 " -b, -- read from the VM state rather than the virtual disk\n"
 " -C, -- report statistics in a machine parsable format\n"
 " -l, -- length for pattern verification (only with -P)\n"
-" -p, -- use blk_pread to read the file\n"
+" -p, -- ignored for back-compat\n"
 " -P, -- use a pattern to verify read data\n"
 " -q, -- quiet mode, do not show I/O statistics\n"
 " -s, -- start offset for pattern verification (only with -P)\n"
@@ -687,7 +653,7 @@ static const cmdinfo_t read_cmd = {
     .cfunc      = read_f,
     .argmin     = 2,
     .argmax     = -1,
-    .args       = "[-abCpqv] [-P pattern [-s off] [-l len]] off len",
+    .args       = "[-abCqv] [-P pattern [-s off] [-l len]] off len",
     .oneline    = "reads a number of bytes at a specified offset",
     .help       = read_help,
 };
@@ -695,7 +661,7 @@ static const cmdinfo_t read_cmd = {
 static int read_f(BlockBackend *blk, int argc, char **argv)
 {
     struct timeval t1, t2;
-    int Cflag = 0, pflag = 0, qflag = 0, vflag = 0;
+    int Cflag = 0, qflag = 0, vflag = 0;
     int Pflag = 0, sflag = 0, lflag = 0, bflag = 0;
     int c, cnt;
     char *buf;
@@ -723,7 +689,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
             }
             break;
         case 'p':
-            pflag = 1;
+            /* Ignored for back-compat */
             break;
         case 'P':
             Pflag = 1;
@@ -755,11 +721,6 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
         return qemuio_command_usage(&read_cmd);
     }

-    if (bflag && pflag) {
-        printf("-b and -p cannot be specified at the same time\n");
-        return 0;
-    }
-
     offset = cvtnum(argv[optind]);
     if (offset < 0) {
         print_cvtnum_err(offset, argv[optind]);
@@ -790,7 +751,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
         return 0;
     }

-    if (!pflag) {
+    if (bflag) {
         if (offset & 0x1ff) {
             printf("offset %" PRId64 " is not sector aligned\n",
                    offset);
@@ -806,12 +767,10 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
     buf = qemu_io_alloc(blk, count, 0xab);

     gettimeofday(&t1, NULL);
-    if (pflag) {
-        cnt = do_pread(blk, buf, offset, count, &total);
-    } else if (bflag) {
+    if (bflag) {
         cnt = do_load_vmstate(blk, buf, offset, count, &total);
     } else {
-        cnt = do_read(blk, buf, offset, count, &total);
+        cnt = do_pread(blk, buf, offset, count, &total);
     }
     gettimeofday(&t2, NULL);

@@ -991,7 +950,7 @@ static void write_help(void)
 " filled with a set pattern (0xcdcdcdcd).\n"
 " -b, -- write to the VM state rather than the virtual disk\n"
 " -c, -- write compressed data with blk_write_compressed\n"
-" -p, -- use blk_pwrite to write the file\n"
+" -p, -- ignored for back-compat\n"
 " -P, -- use different pattern to fill file\n"
 " -C, -- report statistics in a machine parsable format\n"
 " -q, -- quiet mode, do not show I/O statistics\n"
@@ -1007,7 +966,7 @@ static const cmdinfo_t write_cmd = {
     .cfunc      = write_f,
     .argmin     = 2,
     .argmax     = -1,
-    .args       = "[-bcCpqz] [-P pattern ] off len",
+    .args       = "[-bcCqz] [-P pattern ] off len",
     .oneline    = "writes a number of bytes at a specified offset",
     .help       = write_help,
 };
@@ -1015,7 +974,7 @@ static const cmdinfo_t write_cmd = {
 static int write_f(BlockBackend *blk, int argc, char **argv)
 {
     struct timeval t1, t2;
-    int Cflag = 0, pflag = 0, qflag = 0, bflag = 0, Pflag = 0, zflag = 0;
+    int Cflag = 0, qflag = 0, bflag = 0, Pflag = 0, zflag = 0;
     int cflag = 0;
     int c, cnt;
     char *buf = NULL;
@@ -1037,7 +996,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
             Cflag = 1;
             break;
         case 'p':
-            pflag = 1;
+            /* Ignored for back-compat */
             break;
         case 'P':
             Pflag = 1;
@@ -1061,8 +1020,8 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
         return qemuio_command_usage(&write_cmd);
     }

-    if (bflag + pflag + zflag > 1) {
-        printf("-b, -p, or -z cannot be specified at the same time\n");
+    if (bflag + zflag > 1) {
+        printf("-b and -z cannot be specified at the same time\n");
         return 0;
     }

@@ -1088,7 +1047,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
         return 0;
     }

-    if (!pflag) {
+    if (bflag || cflag || zflag) {
         if (offset & 0x1ff) {
             printf("offset %" PRId64 " is not sector aligned\n",
                    offset);
@@ -1107,16 +1066,14 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
     }

     gettimeofday(&t1, NULL);
-    if (pflag) {
-        cnt = do_pwrite(blk, buf, offset, count, &total);
-    } else if (bflag) {
+    if (bflag) {
         cnt = do_save_vmstate(blk, buf, offset, count, &total);
     } else if (zflag) {
         cnt = do_co_write_zeroes(blk, offset, count, &total);
     } else if (cflag) {
         cnt = do_write_compressed(blk, buf, offset, count, &total);
     } else {
-        cnt = do_write(blk, buf, offset, count, &total);
+        cnt = do_pwrite(blk, buf, offset, count, &total);
     }
     gettimeofday(&t2, NULL);

-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 12/14] block: Switch blk_read_unthrottled() to byte interface
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (10 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 11/14] qemu-io: " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 13/14] block: Switch blk_write_zeroes() " Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 14/14] block: Kill blk_write(), blk_read() Eric Blake
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz, John Snow

Sector-based blk_read() should die; convert the one-off
variant blk_read_unthrottled().

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/sysemu/block-backend.h | 4 ++--
 block/block-backend.c          | 8 ++++----
 hw/block/hd-geometry.c         | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 6991b26..662a106 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -92,8 +92,8 @@ void *blk_get_attached_dev(BlockBackend *blk);
 void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void *opaque);
 int blk_read(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
              int nb_sectors);
-int blk_read_unthrottled(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
-                         int nb_sectors);
+int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
+                          int count);
 int blk_write(BlockBackend *blk, int64_t sector_num, const uint8_t *buf,
               int nb_sectors);
 int blk_write_zeroes(BlockBackend *blk, int64_t sector_num,
diff --git a/block/block-backend.c b/block/block-backend.c
index 96c1d7c..e5a8a07 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -790,19 +790,19 @@ int blk_read(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
     return blk_rw(blk, sector_num, buf, nb_sectors, blk_read_entry, 0);
 }

-int blk_read_unthrottled(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
-                         int nb_sectors)
+int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
+                          int count)
 {
     BlockDriverState *bs = blk_bs(blk);
     int ret;

-    ret = blk_check_request(blk, sector_num, nb_sectors);
+    ret = blk_check_byte_request(blk, offset, count);
     if (ret < 0) {
         return ret;
     }

     bdrv_no_throttling_begin(bs);
-    ret = blk_read(blk, sector_num, buf, nb_sectors);
+    ret = blk_pread(blk, offset, buf, count);
     bdrv_no_throttling_end(bs);
     return ret;
 }
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6d02192..d388f13 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -66,7 +66,7 @@ static int guess_disk_lchs(BlockBackend *blk,
      * but also in async I/O mode. So the I/O throttling function has to
      * be disabled temporarily here, not permanently.
      */
-    if (blk_read_unthrottled(blk, 0, buf, 1) < 0) {
+    if (blk_pread_unthrottled(blk, 0, buf, BDRV_SECTOR_SIZE) < 0) {
         return -1;
     }
     /* test msdos magic */
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 13/14] block: Switch blk_write_zeroes() to byte interface
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (11 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 12/14] block: Switch blk_read_unthrottled() to byte interface Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 14/14] block: Kill blk_write(), blk_read() Eric Blake
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz, Stefan Hajnoczi, Denis V. Lunev

Sector-based blk_write() should die; convert the one-off
variant blk_write_zeroes().

Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Denis V. Lunev <den@openvz.org>
---
 include/sysemu/block-backend.h | 4 ++--
 block/block-backend.c          | 8 ++++----
 block/parallels.c              | 3 ++-
 qemu-img.c                     | 3 ++-
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 662a106..681431b 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -96,8 +96,8 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
                           int count);
 int blk_write(BlockBackend *blk, int64_t sector_num, const uint8_t *buf,
               int nb_sectors);
-int blk_write_zeroes(BlockBackend *blk, int64_t sector_num,
-                     int nb_sectors, BdrvRequestFlags flags);
+int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+                      int count, BdrvRequestFlags flags);
 BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
                                  int nb_sectors, BdrvRequestFlags flags,
                                  BlockCompletionFunc *cb, void *opaque);
diff --git a/block/block-backend.c b/block/block-backend.c
index e5a8a07..71133b2 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -814,11 +814,11 @@ int blk_write(BlockBackend *blk, int64_t sector_num, const uint8_t *buf,
                   blk_write_entry, 0);
 }

-int blk_write_zeroes(BlockBackend *blk, int64_t sector_num,
-                     int nb_sectors, BdrvRequestFlags flags)
+int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+                      int count, BdrvRequestFlags flags)
 {
-    return blk_rw(blk, sector_num, NULL, nb_sectors, blk_write_entry,
-                  flags | BDRV_REQ_ZERO_WRITE);
+    return blk_prw(blk, offset, NULL, count, blk_write_entry,
+                   flags | BDRV_REQ_ZERO_WRITE);
 }

 static void error_callback_bh(void *opaque)
diff --git a/block/parallels.c b/block/parallels.c
index 2d8bc87..95bfc32 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -516,7 +516,8 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
     if (ret < 0) {
         goto exit;
     }
-    ret = blk_write_zeroes(file, 1, bat_sectors - 1, 0);
+    ret = blk_pwrite_zeroes(file, BDRV_SECTOR_SIZE,
+                            (bat_sectors - 1) << BDRV_SECTOR_BITS, 0);
     if (ret < 0) {
         goto exit;
     }
diff --git a/qemu-img.c b/qemu-img.c
index c19f9d4..41df87d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1595,7 +1595,8 @@ static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors,
             if (s->has_zero_init) {
                 break;
             }
-            ret = blk_write_zeroes(s->target, sector_num, n, 0);
+            ret = blk_pwrite_zeroes(s->target, sector_num << BDRV_SECTOR_BITS,
+                                    n << BDRV_SECTOR_BITS, 0);
             if (ret < 0) {
                 return ret;
             }
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH v5 14/14] block: Kill blk_write(), blk_read()
  2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
                   ` (12 preceding siblings ...)
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 13/14] block: Switch blk_write_zeroes() " Eric Blake
@ 2016-05-03 15:42 ` Eric Blake
  13 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-03 15:42 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz

Now that there are no remaining clients, we can drop these
functions, to ensure that all future users get the byte-based
interfaces.  Sadly, there are still remaining sector-based
interfaces, such as blk_aio_writev; those will have to wait
for another day.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/sysemu/block-backend.h |  4 ----
 block/block-backend.c          | 25 -------------------------
 2 files changed, 29 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 681431b..a63af97 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -90,12 +90,8 @@ void blk_attach_dev_nofail(BlockBackend *blk, void *dev);
 void blk_detach_dev(BlockBackend *blk, void *dev);
 void *blk_get_attached_dev(BlockBackend *blk);
 void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void *opaque);
-int blk_read(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
-             int nb_sectors);
 int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
                           int count);
-int blk_write(BlockBackend *blk, int64_t sector_num, const uint8_t *buf,
-              int nb_sectors);
 int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                       int count, BdrvRequestFlags flags);
 BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
diff --git a/block/block-backend.c b/block/block-backend.c
index 71133b2..e6ded54 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -772,24 +772,6 @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
     return rwco.ret;
 }

-static int blk_rw(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
-                  int nb_sectors, CoroutineEntry co_entry,
-                  BdrvRequestFlags flags)
-{
-    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
-        return -EINVAL;
-    }
-
-    return blk_prw(blk, sector_num << BDRV_SECTOR_BITS, buf,
-                   nb_sectors << BDRV_SECTOR_BITS, co_entry, flags);
-}
-
-int blk_read(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
-             int nb_sectors)
-{
-    return blk_rw(blk, sector_num, buf, nb_sectors, blk_read_entry, 0);
-}
-
 int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
                           int count)
 {
@@ -807,13 +789,6 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
     return ret;
 }

-int blk_write(BlockBackend *blk, int64_t sector_num, const uint8_t *buf,
-              int nb_sectors)
-{
-    return blk_rw(blk, sector_num, (uint8_t*) buf, nb_sectors,
-                  blk_write_entry, 0);
-}
-
 int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                       int count, BdrvRequestFlags flags)
 {
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v5 11/14] qemu-io: Switch to byte-based block access
  2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 11/14] qemu-io: " Eric Blake
@ 2016-05-04 13:10   ` Kevin Wolf
  2016-05-04 14:13     ` Eric Blake
  0 siblings, 1 reply; 17+ messages in thread
From: Kevin Wolf @ 2016-05-04 13:10 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-devel, qemu-block, Max Reitz

Am 03.05.2016 um 17:42 hat Eric Blake geschrieben:
> blk_write() and blk_read() are now very simple wrappers around
> blk_pwrite() and blk_pread().  There's no reason to require
> the user to pass in aligned numbers.  Keep 'read -p' and
> 'write -p' so that I don't have to hunt down and update all
> users of qemu-io, but make the default 'read' and 'write' now
> do the same behavior that used to require -p.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

This breaks at least qemu-iotests 023.

Kevin

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v5 11/14] qemu-io: Switch to byte-based block access
  2016-05-04 13:10   ` Kevin Wolf
@ 2016-05-04 14:13     ` Eric Blake
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-05-04 14:13 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel, qemu-block, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]

On 05/04/2016 07:10 AM, Kevin Wolf wrote:
> Am 03.05.2016 um 17:42 hat Eric Blake geschrieben:
>> blk_write() and blk_read() are now very simple wrappers around
>> blk_pwrite() and blk_pread().  There's no reason to require
>> the user to pass in aligned numbers.  Keep 'read -p' and
>> 'write -p' so that I don't have to hunt down and update all
>> users of qemu-io, but make the default 'read' and 'write' now
>> do the same behavior that used to require -p.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> This breaks at least qemu-iotests 023.

Whoops - shows how often I run qemu-iotests.  Will fix.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-05-04 14:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-03 15:42 [Qemu-devel] [PATCH v5 00/14] block: kill sector-based blk_write/read Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 01/14] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 02/14] fdc: Switch to byte-based block access Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 03/14] nand: " Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 04/14] onenand: " Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 05/14] pflash: " Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 06/14] sd: " Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 07/14] m25p80: " Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 08/14] atapi: " Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 09/14] nbd: " Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 10/14] qemu-img: " Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 11/14] qemu-io: " Eric Blake
2016-05-04 13:10   ` Kevin Wolf
2016-05-04 14:13     ` Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 12/14] block: Switch blk_read_unthrottled() to byte interface Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 13/14] block: Switch blk_write_zeroes() " Eric Blake
2016-05-03 15:42 ` [Qemu-devel] [PATCH v5 14/14] block: Kill blk_write(), blk_read() Eric Blake

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.