* [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read
@ 2016-05-04 23:55 Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 01/20] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
` (20 more replies)
0 siblings, 21 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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 v5 [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-05/msg00235.html
Also available as a tag at this location:
git fetch git://repo.or.cz/qemu/ericb.git nbd-block-v6
Changes since then:
- patch 2: new patch for something that confused me
- patch 3 (was 12/14): moved earlier
- patch 4 (was 13/14): moved earlier, retitled, and touches more
interfaces
- patches 5-9: new, to cover blk_aio_{read,write}v
- patch 15: also cover blk_aio_{read,write}v
- patch 19: also cover blk_aio_{read,write}v, don't break
qemu-iotests 23 [kwolf]
- patch 20 (was 14/14): retitle, kill more dead code
I still plan to make qemu-io 'read' accept unaligned access
without requiring -p, but that will now come in a later series
that does other qemu-io UI improvements (besides, 20 patches is
big enough to review for now). For qemu-iotests 23 to work
with unaligned 'read', I also have to fix 'readv' to be
unaligned; which is why this series grew to include blk_aio_*.
Note that there are still a few sector-aligned blk_* interfaces
left even after this series, but we are progressively getting
closer to using byte interfaces throughout the code base.
001/20:[----] [--] 'block: Allow BDRV_REQ_FUA through blk_pwrite()'
002/20:[down] 'block: Drop private ioctl-only members of BlockRequest'
003/20:[----] [--] 'block: Switch blk_read_unthrottled() to byte interface'
004/20:[down] 'block: Switch blk_*write_zeroes() to byte interface'
005/20:[down] 'block: Introduce byte-based aio read/write'
006/20:[down] 'ide: Switch to byte-based aio block access'
007/20:[down] 'scsi-disk: Switch to byte-based aio block access'
008/20:[down] 'virtio: Switch to byte-based aio block access'
009/20:[down] 'xen_disk: Switch to byte-based aio block access'
010/20:[----] [--] 'fdc: Switch to byte-based block access'
011/20:[----] [--] 'nand: Switch to byte-based block access'
012/20:[----] [--] 'onenand: Switch to byte-based block access'
013/20:[----] [--] 'pflash: Switch to byte-based block access'
014/20:[----] [--] 'sd: Switch to byte-based block access'
015/20:[0020] [FC] 'm25p80: Switch to byte-based block access'
016/20:[----] [--] 'atapi: Switch to byte-based block access'
017/20:[----] [--] 'nbd: Switch to byte-based block access'
018/20:[----] [--] 'qemu-img: Switch to byte-based block access'
019/20:[0041] [FC] 'qemu-io: Switch to byte-based block access'
020/20:[down] 'block: Kill unused sector-based blk_* functions'
Eric Blake (20):
block: Allow BDRV_REQ_FUA through blk_pwrite()
block: Drop private ioctl-only members of BlockRequest
block: Switch blk_read_unthrottled() to byte interface
block: Switch blk_*write_zeroes() to byte interface
block: Introduce byte-based aio read/write
ide: Switch to byte-based aio block access
scsi-disk: Switch to byte-based aio block access
virtio: Switch to byte-based aio block access
xen_disk: Switch to byte-based aio block access
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: Kill unused sector-based blk_* functions
hw/ide/internal.h | 2 +-
include/block/block.h | 16 ++-----
include/sysemu/block-backend.h | 33 ++++++-------
include/sysemu/dma.h | 4 +-
block/block-backend.c | 104 ++++++++++++-----------------------------
block/crypto.c | 2 +-
block/io.c | 8 ++--
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 ++--
dma-helpers.c | 14 +++---
hw/block/fdc.c | 25 ++++++----
hw/block/hd-geometry.c | 2 +-
hw/block/m25p80.c | 23 +++------
hw/block/nand.c | 36 ++++++++------
hw/block/onenand.c | 41 ++++++++++------
hw/block/pflash_cfi01.c | 12 ++---
hw/block/pflash_cfi02.c | 12 ++---
hw/block/virtio-blk.c | 18 ++++---
hw/block/xen_disk.c | 10 ++--
hw/ide/atapi.c | 19 ++++----
hw/ide/core.c | 10 ++--
hw/ide/macio.c | 9 ++--
hw/nvram/spapr_nvram.c | 4 +-
hw/scsi/scsi-disk.c | 35 +++++++-------
hw/sd/sd.c | 51 ++------------------
nbd/server.c | 2 +-
qemu-img.c | 31 ++++++++----
qemu-io-cmds.c | 70 ++++++---------------------
qemu-nbd.c | 11 +++--
trace-events | 2 +-
37 files changed, 277 insertions(+), 383 deletions(-)
--
2.5.5
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v6 01/20] block: Allow BDRV_REQ_FUA through blk_pwrite()
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 02/20] block: Drop private ioctl-only members of BlockRequest Eric Blake
` (19 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 02/20] block: Drop private ioctl-only members of BlockRequest
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 01/20] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-06 10:37 ` Kevin Wolf
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 03/20] block: Switch blk_read_unthrottled() to byte interface Eric Blake
` (18 subsequent siblings)
20 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, Stefan Hajnoczi, Fam Zheng, Max Reitz
I was thrown by the fact that the public type BlockRequest had
an anonymous union, but no obvious discriminator. Turns out
that the only client of the second branch of the union was code
internal to io.c, and that with a slight abuse of QEMUIOVector*
to pass a void* pointer, we can make the public interface less
confusing.
(Yes, I know that strict C doesn't guarantee that you can cast
void* to the wrong type and then back to void* - it only
guarantees the reverse direction of the original pointer to
void* and back to the original type - but we already have other
assumptions throughout the qemu code base that assume that all
pointers are interchangeable in representation).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/block.h | 16 ++++------------
block/io.c | 8 +++++---
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 0e8b4d1..754aae3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -334,18 +334,10 @@ void bdrv_aio_cancel_async(BlockAIOCB *acb);
typedef struct BlockRequest {
/* Fields to be filled by multiwrite caller */
- union {
- struct {
- int64_t sector;
- int nb_sectors;
- int flags;
- QEMUIOVector *qiov;
- };
- struct {
- int req;
- void *buf;
- };
- };
+ int64_t sector;
+ int nb_sectors;
+ int flags;
+ QEMUIOVector *qiov;
BlockCompletionFunc *cb;
void *opaque;
diff --git a/block/io.c b/block/io.c
index 0db1146..f15c0f4 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2614,7 +2614,7 @@ static void coroutine_fn bdrv_co_aio_ioctl_entry(void *opaque)
{
BlockAIOCBCoroutine *acb = opaque;
acb->req.error = bdrv_co_do_ioctl(acb->common.bs,
- acb->req.req, acb->req.buf);
+ acb->req.flags, acb->req.qiov);
bdrv_co_complete(acb);
}
@@ -2628,8 +2628,10 @@ BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
acb->need_bh = true;
acb->req.error = -EINPROGRESS;
- acb->req.req = req;
- acb->req.buf = buf;
+ /* Slight type abuse here, so we don't have to expose extra fields
+ * in the public struct BlockRequest */
+ acb->req.flags = req;
+ acb->req.qiov = buf;
co = qemu_coroutine_create(bdrv_co_aio_ioctl_entry);
qemu_coroutine_enter(co, acb);
--
2.5.5
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v6 03/20] block: Switch blk_read_unthrottled() to byte interface
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 01/20] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 02/20] block: Drop private ioctl-only members of BlockRequest Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 04/20] block: Switch blk_*write_zeroes() " Eric Blake
` (17 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 04/20] block: Switch blk_*write_zeroes() to byte interface
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (2 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 03/20] block: Switch blk_read_unthrottled() to byte interface Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 05/20] block: Introduce byte-based aio read/write Eric Blake
` (16 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, Max Reitz, Stefan Hajnoczi, Denis V. Lunev,
Paolo Bonzini
Sector-based blk_write() should die; convert the one-off
variant blk_write_zeroes() to use an offset/count interface
instead. Likewise for blk_co_write_zeroes() and
blk_aio_write_zeroes().
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/sysemu/block-backend.h | 12 ++++++------
block/block-backend.c | 33 +++++++++++----------------------
block/parallels.c | 3 ++-
hw/scsi/scsi-disk.c | 4 ++--
qemu-img.c | 3 ++-
qemu-io-cmds.c | 6 ++----
6 files changed, 25 insertions(+), 36 deletions(-)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 662a106..851376b 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -96,10 +96,10 @@ 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);
-BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
- int nb_sectors, BdrvRequestFlags flags,
+int blk_write_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags);
+BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t offset,
+ int count, 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,
@@ -179,8 +179,8 @@ int blk_get_open_flags_from_root_state(BlockBackend *blk);
void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
BlockCompletionFunc *cb, void *opaque);
-int coroutine_fn blk_co_write_zeroes(BlockBackend *blk, int64_t sector_num,
- int nb_sectors, BdrvRequestFlags flags);
+int coroutine_fn blk_co_write_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags);
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);
diff --git a/block/block-backend.c b/block/block-backend.c
index e5a8a07..f8f88a6 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_write_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)
@@ -930,18 +930,12 @@ static void blk_aio_write_entry(void *opaque)
blk_aio_complete(acb);
}
-BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
- int nb_sectors, BdrvRequestFlags flags,
+BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque)
{
- if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
- return blk_abort_aio_request(blk, cb, opaque, -EINVAL);
- }
-
- return blk_aio_prwv(blk, sector_num << BDRV_SECTOR_BITS,
- nb_sectors << BDRV_SECTOR_BITS, NULL,
- blk_aio_write_entry, flags | BDRV_REQ_ZERO_WRITE,
- cb, opaque);
+ return blk_aio_prwv(blk, offset, count, NULL, blk_aio_write_entry,
+ flags | BDRV_REQ_ZERO_WRITE, cb, opaque);
}
int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int count)
@@ -1444,15 +1438,10 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
}
-int coroutine_fn blk_co_write_zeroes(BlockBackend *blk, int64_t sector_num,
- int nb_sectors, BdrvRequestFlags flags)
+int coroutine_fn blk_co_write_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags)
{
- if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
- return -EINVAL;
- }
-
- return blk_co_pwritev(blk, sector_num << BDRV_SECTOR_BITS,
- nb_sectors << BDRV_SECTOR_BITS, NULL,
+ return blk_co_pwritev(blk, offset, count, NULL,
flags | BDRV_REQ_ZERO_WRITE);
}
diff --git a/block/parallels.c b/block/parallels.c
index 2d8bc87..cddbfc4 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_write_zeroes(file, BDRV_SECTOR_SIZE,
+ (bat_sectors - 1) << BDRV_SECTOR_BITS, 0);
if (ret < 0) {
goto exit;
}
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index c3ce54a..1335392 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1781,8 +1781,8 @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r, uint8_t *inbuf)
nb_sectors * s->qdev.blocksize,
BLOCK_ACCT_WRITE);
r->req.aiocb = blk_aio_write_zeroes(s->qdev.conf.blk,
- r->req.cmd.lba * (s->qdev.blocksize / 512),
- nb_sectors * (s->qdev.blocksize / 512),
+ r->req.cmd.lba * s->qdev.blocksize,
+ nb_sectors * s->qdev.blocksize,
flags, scsi_aio_complete, r);
return;
}
diff --git a/qemu-img.c b/qemu-img.c
index 46f2a6d..76430a8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1589,7 +1589,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_write_zeroes(s->target, sector_num << BDRV_SECTOR_BITS,
+ n << BDRV_SECTOR_BITS, 0);
if (ret < 0) {
return ret;
}
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index e26e543..a3e3982 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -494,8 +494,7 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque)
{
CoWriteZeroes *data = opaque;
- data->ret = blk_co_write_zeroes(data->blk, data->offset / BDRV_SECTOR_SIZE,
- data->count / BDRV_SECTOR_SIZE, 0);
+ data->ret = blk_co_write_zeroes(data->blk, data->offset, data->count, 0);
data->done = true;
if (data->ret < 0) {
*data->total = data->ret;
@@ -1703,8 +1702,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
}
ctx->qiov.size = count;
- blk_aio_write_zeroes(blk, ctx->offset >> 9, count >> 9, 0,
- aio_write_done, ctx);
+ blk_aio_write_zeroes(blk, ctx->offset, count, 0, aio_write_done, ctx);
} else {
nr_iov = argc - optind;
ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov,
--
2.5.5
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v6 05/20] block: Introduce byte-based aio read/write
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (3 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 04/20] block: Switch blk_*write_zeroes() " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-06 12:31 ` Kevin Wolf
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 06/20] ide: Switch to byte-based aio block access Eric Blake
` (15 subsequent siblings)
20 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz
blk_aio_readv() and blk_aio_writev() are annoying in that they
can't access sub-sector granularity, and cannot pass flags.
Also, they require the caller to pass redundant information
about the size of the I/O.
Add new blk_aio_preadv() and blk_aio_pwritev() functions to fix
the flaws. The next few patches will upgrade callers, then
finally delete the old interfaces.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/sysemu/block-backend.h | 6 ++++++
block/block-backend.c | 16 ++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 851376b..8d7839c 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -110,9 +110,15 @@ int64_t blk_nb_sectors(BlockBackend *blk);
BlockAIOCB *blk_aio_readv(BlockBackend *blk, int64_t sector_num,
QEMUIOVector *iov, int nb_sectors,
BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
+ QEMUIOVector *iov, BdrvRequestFlags flags,
+ BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *blk_aio_writev(BlockBackend *blk, int64_t sector_num,
QEMUIOVector *iov, int nb_sectors,
BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset,
+ QEMUIOVector *iov, BdrvRequestFlags flags,
+ BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *blk_aio_flush(BlockBackend *blk,
BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *blk_aio_discard(BlockBackend *blk,
diff --git a/block/block-backend.c b/block/block-backend.c
index f8f88a6..104852d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -998,6 +998,14 @@ BlockAIOCB *blk_aio_readv(BlockBackend *blk, int64_t sector_num,
blk_aio_read_entry, 0, cb, opaque);
}
+BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
+ QEMUIOVector *iov, BdrvRequestFlags flags,
+ BlockCompletionFunc *cb, void *opaque)
+{
+ return blk_aio_prwv(blk, offset, iov->size, iov,
+ blk_aio_read_entry, flags, cb, opaque);
+}
+
BlockAIOCB *blk_aio_writev(BlockBackend *blk, int64_t sector_num,
QEMUIOVector *iov, int nb_sectors,
BlockCompletionFunc *cb, void *opaque)
@@ -1011,6 +1019,14 @@ BlockAIOCB *blk_aio_writev(BlockBackend *blk, int64_t sector_num,
blk_aio_write_entry, 0, cb, opaque);
}
+BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset,
+ QEMUIOVector *iov, BdrvRequestFlags flags,
+ BlockCompletionFunc *cb, void *opaque)
+{
+ return blk_aio_prwv(blk, offset, iov->size, iov,
+ blk_aio_write_entry, flags, cb, opaque);
+}
+
BlockAIOCB *blk_aio_flush(BlockBackend *blk,
BlockCompletionFunc *cb, void *opaque)
{
--
2.5.5
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v6 06/20] ide: Switch to byte-based aio block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (4 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 05/20] block: Introduce byte-based aio read/write Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 07/20] scsi-disk: " Eric Blake
` (14 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, John Snow
Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
The patch had to touch multiple files at once, because dma_blk_io()
takes pointers to the functions, and ide_issue_trim() piggybacks on
the same interface (while ignoring offset under the hood).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hw/ide/internal.h | 2 +-
include/sysemu/dma.h | 4 ++--
dma-helpers.c | 14 +++++++-------
hw/ide/core.c | 10 +++++-----
hw/ide/macio.c | 9 +++------
5 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index d2c458f..ceb9e59 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -614,7 +614,7 @@ void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
void ide_transfer_stop(IDEState *s);
void ide_set_inactive(IDEState *s, bool more);
BlockAIOCB *ide_issue_trim(BlockBackend *blk,
- int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+ int64_t offset, QEMUIOVector *qiov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num,
QEMUIOVector *iov, int nb_sectors,
diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h
index b0fbb9b..0f7cd4d 100644
--- a/include/sysemu/dma.h
+++ b/include/sysemu/dma.h
@@ -197,8 +197,8 @@ void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len);
void qemu_sglist_destroy(QEMUSGList *qsg);
#endif
-typedef BlockAIOCB *DMAIOFunc(BlockBackend *blk, int64_t sector_num,
- QEMUIOVector *iov, int nb_sectors,
+typedef BlockAIOCB *DMAIOFunc(BlockBackend *blk, int64_t offset,
+ QEMUIOVector *iov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *dma_blk_io(BlockBackend *blk,
diff --git a/dma-helpers.c b/dma-helpers.c
index 4ad0bca..a6cc15f 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -73,7 +73,7 @@ typedef struct {
BlockBackend *blk;
BlockAIOCB *acb;
QEMUSGList *sg;
- uint64_t sector_num;
+ uint64_t offset;
DMADirection dir;
int sg_cur_index;
dma_addr_t sg_cur_byte;
@@ -130,7 +130,7 @@ static void dma_blk_cb(void *opaque, int ret)
trace_dma_blk_cb(dbs, ret);
dbs->acb = NULL;
- dbs->sector_num += dbs->iov.size / 512;
+ dbs->offset += dbs->iov.size;
if (dbs->sg_cur_index == dbs->sg->nsg || ret < 0) {
dma_complete(dbs, ret);
@@ -164,8 +164,8 @@ static void dma_blk_cb(void *opaque, int ret)
qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK);
}
- dbs->acb = dbs->io_func(dbs->blk, dbs->sector_num, &dbs->iov,
- dbs->iov.size / 512, dma_blk_cb, dbs);
+ dbs->acb = dbs->io_func(dbs->blk, dbs->offset, &dbs->iov, 0,
+ dma_blk_cb, dbs);
assert(dbs->acb);
}
@@ -203,7 +203,7 @@ BlockAIOCB *dma_blk_io(
dbs->acb = NULL;
dbs->blk = blk;
dbs->sg = sg;
- dbs->sector_num = sector_num;
+ dbs->offset = sector_num << BDRV_SECTOR_BITS;
dbs->sg_cur_index = 0;
dbs->sg_cur_byte = 0;
dbs->dir = dir;
@@ -219,7 +219,7 @@ BlockAIOCB *dma_blk_read(BlockBackend *blk,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void *opaque)
{
- return dma_blk_io(blk, sg, sector, blk_aio_readv, cb, opaque,
+ return dma_blk_io(blk, sg, sector, blk_aio_preadv, cb, opaque,
DMA_DIRECTION_FROM_DEVICE);
}
@@ -227,7 +227,7 @@ BlockAIOCB *dma_blk_write(BlockBackend *blk,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void *opaque)
{
- return dma_blk_io(blk, sg, sector, blk_aio_writev, cb, opaque,
+ return dma_blk_io(blk, sg, sector, blk_aio_pwritev, cb, opaque,
DMA_DIRECTION_TO_DEVICE);
}
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 41e6a2d..fe2bfba 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -442,7 +442,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
}
BlockAIOCB *ide_issue_trim(BlockBackend *blk,
- int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+ int64_t offset, QEMUIOVector *qiov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque)
{
TrimAIOCB *iocb;
@@ -616,8 +616,8 @@ BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num,
req->iov.iov_len = iov->size;
qemu_iovec_init_external(&req->qiov, &req->iov, 1);
- aioreq = blk_aio_readv(s->blk, sector_num, &req->qiov, nb_sectors,
- ide_buffered_readv_cb, req);
+ aioreq = blk_aio_preadv(s->blk, sector_num << BDRV_SECTOR_BITS,
+ &req->qiov, 0, ide_buffered_readv_cb, req);
QLIST_INSERT_HEAD(&s->buffered_requests, req, list);
return aioreq;
@@ -1006,8 +1006,8 @@ static void ide_sector_write(IDEState *s)
block_acct_start(blk_get_stats(s->blk), &s->acct,
n * BDRV_SECTOR_SIZE, BLOCK_ACCT_WRITE);
- s->pio_aiocb = blk_aio_writev(s->blk, sector_num, &s->qiov, n,
- ide_sector_write_cb, s);
+ s->pio_aiocb = blk_aio_pwritev(s->blk, sector_num << BDRV_SECTOR_BITS,
+ &s->qiov, 0, ide_sector_write_cb, s);
}
static void ide_flush_cb(void *opaque, int ret)
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index ae29b6f..d7d9c0f 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -120,8 +120,7 @@ static void pmac_dma_read(BlockBackend *blk,
MACIO_DPRINTF("--- Block read transfer - sector_num: %" PRIx64 " "
"nsector: %x\n", (offset >> 9), (bytes >> 9));
- s->bus->dma->aiocb = blk_aio_readv(blk, (offset >> 9), &io->iov,
- (bytes >> 9), cb, io);
+ s->bus->dma->aiocb = blk_aio_preadv(blk, offset, &io->iov, 0, cb, io);
}
static void pmac_dma_write(BlockBackend *blk,
@@ -205,8 +204,7 @@ static void pmac_dma_write(BlockBackend *blk,
MACIO_DPRINTF("--- Block write transfer - sector_num: %" PRIx64 " "
"nsector: %x\n", (offset >> 9), (bytes >> 9));
- s->bus->dma->aiocb = blk_aio_writev(blk, (offset >> 9), &io->iov,
- (bytes >> 9), cb, io);
+ s->bus->dma->aiocb = blk_aio_pwritev(blk, offset, &io->iov, 0, cb, io);
}
static void pmac_dma_trim(BlockBackend *blk,
@@ -232,8 +230,7 @@ static void pmac_dma_trim(BlockBackend *blk,
s->io_buffer_index += io->len;
io->len = 0;
- s->bus->dma->aiocb = ide_issue_trim(blk, (offset >> 9), &io->iov,
- (bytes >> 9), cb, io);
+ s->bus->dma->aiocb = ide_issue_trim(blk, offset, &io->iov, 0, cb, io);
}
static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
--
2.5.5
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v6 07/20] scsi-disk: Switch to byte-based aio block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (5 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 06/20] ide: Switch to byte-based aio block access Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-06 12:50 ` Kevin Wolf
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 08/20] virtio: " Eric Blake
` (13 subsequent siblings)
20 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, Paolo Bonzini
Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hw/scsi/scsi-disk.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 1335392..5d98f7b 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -343,8 +343,9 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
n * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
- r->req.aiocb = blk_aio_readv(s->qdev.conf.blk, r->sector, &r->qiov, n,
- scsi_read_complete, r);
+ r->req.aiocb = blk_aio_preadv(s->qdev.conf.blk,
+ r->sector << BDRV_SECTOR_BITS, &r->qiov,
+ 0, scsi_read_complete, r);
}
done:
@@ -504,7 +505,6 @@ static void scsi_write_data(SCSIRequest *req)
{
SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
- uint32_t n;
/* No data transfer may already be in progress */
assert(r->req.aiocb == NULL);
@@ -544,11 +544,11 @@ static void scsi_write_data(SCSIRequest *req)
r->req.aiocb = dma_blk_write(s->qdev.conf.blk, r->req.sg, r->sector,
scsi_dma_complete, r);
} else {
- n = r->qiov.size / 512;
block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
- n * BDRV_SECTOR_SIZE, BLOCK_ACCT_WRITE);
- r->req.aiocb = blk_aio_writev(s->qdev.conf.blk, r->sector, &r->qiov, n,
- scsi_write_complete, r);
+ r->qiov.size, BLOCK_ACCT_WRITE);
+ r->req.aiocb = blk_aio_pwritev(s->qdev.conf.blk,
+ r->sector << BDRV_SECTOR_BITS, &r->qiov,
+ 0, scsi_write_complete, r);
}
}
@@ -1730,13 +1730,11 @@ static void scsi_write_same_complete(void *opaque, int ret)
if (data->iov.iov_len) {
block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
data->iov.iov_len, BLOCK_ACCT_WRITE);
- /* blk_aio_write doesn't like the qiov size being different from
- * nb_sectors, make sure they match.
- */
qemu_iovec_init_external(&data->qiov, &data->iov, 1);
- r->req.aiocb = blk_aio_writev(s->qdev.conf.blk, data->sector,
- &data->qiov, data->iov.iov_len / 512,
- scsi_write_same_complete, data);
+ r->req.aiocb = blk_aio_pwritev(s->qdev.conf.blk,
+ data->sector << BDRV_SECTOR_BITS,
+ &data->qiov, 0,
+ scsi_write_same_complete, data);
return;
}
@@ -1803,9 +1801,10 @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r, uint8_t *inbuf)
scsi_req_ref(&r->req);
block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
data->iov.iov_len, BLOCK_ACCT_WRITE);
- r->req.aiocb = blk_aio_writev(s->qdev.conf.blk, data->sector,
- &data->qiov, data->iov.iov_len / 512,
- scsi_write_same_complete, data);
+ r->req.aiocb = blk_aio_pwritev(s->qdev.conf.blk,
+ data->sector << BDRV_SECTOR_BITS,
+ &data->qiov, 0,
+ scsi_write_same_complete, data);
}
static void scsi_disk_emulate_write_data(SCSIRequest *req)
--
2.5.5
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v6 08/20] virtio: Switch to byte-based aio block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (6 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 07/20] scsi-disk: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` Eric Blake
` (12 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, Michael S. Tsirkin, Stefan Hajnoczi, Max Reitz
Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
The trace is modified at the same time, and nb_sectors is now
unused. Fix a comment typo while in the vicinity.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hw/block/virtio-blk.c | 18 ++++++++----------
trace-events | 2 +-
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 3f88f8c..284e646 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -322,7 +322,6 @@ static inline void submit_requests(BlockBackend *blk, MultiReqBuffer *mrb,
{
QEMUIOVector *qiov = &mrb->reqs[start]->qiov;
int64_t sector_num = mrb->reqs[start]->sector_num;
- int nb_sectors = mrb->reqs[start]->qiov.size / BDRV_SECTOR_SIZE;
bool is_write = mrb->is_write;
if (num_reqs > 1) {
@@ -331,7 +330,7 @@ static inline void submit_requests(BlockBackend *blk, MultiReqBuffer *mrb,
int tmp_niov = qiov->niov;
/* mrb->reqs[start]->qiov was initialized from external so we can't
- * modifiy it here. We need to initialize it locally and then add the
+ * modify it here. We need to initialize it locally and then add the
* external iovecs. */
qemu_iovec_init(qiov, niov);
@@ -343,23 +342,22 @@ static inline void submit_requests(BlockBackend *blk, MultiReqBuffer *mrb,
qemu_iovec_concat(qiov, &mrb->reqs[i]->qiov, 0,
mrb->reqs[i]->qiov.size);
mrb->reqs[i - 1]->mr_next = mrb->reqs[i];
- nb_sectors += mrb->reqs[i]->qiov.size / BDRV_SECTOR_SIZE;
}
- assert(nb_sectors == qiov->size / BDRV_SECTOR_SIZE);
- trace_virtio_blk_submit_multireq(mrb, start, num_reqs, sector_num,
- nb_sectors, is_write);
+ trace_virtio_blk_submit_multireq(mrb, start, num_reqs,
+ sector_num << BDRV_SECTOR_BITS,
+ qiov->size, is_write);
block_acct_merge_done(blk_get_stats(blk),
is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ,
num_reqs - 1);
}
if (is_write) {
- blk_aio_writev(blk, sector_num, qiov, nb_sectors,
+ blk_aio_pwritev(blk, sector_num << BDRV_SECTOR_BITS, qiov, 0,
+ virtio_blk_rw_complete, mrb->reqs[start]);
+ } else {
+ blk_aio_preadv(blk, sector_num << BDRV_SECTOR_BITS, qiov, 0,
virtio_blk_rw_complete, mrb->reqs[start]);
- } else {
- blk_aio_readv(blk, sector_num, qiov, nb_sectors,
- virtio_blk_rw_complete, mrb->reqs[start]);
}
}
diff --git a/trace-events b/trace-events
index b4acd2a..b588091 100644
--- a/trace-events
+++ b/trace-events
@@ -118,7 +118,7 @@ virtio_blk_req_complete(void *req, int status) "req %p status %d"
virtio_blk_rw_complete(void *req, int ret) "req %p ret %d"
virtio_blk_handle_write(void *req, uint64_t sector, size_t nsectors) "req %p sector %"PRIu64" nsectors %zu"
virtio_blk_handle_read(void *req, uint64_t sector, size_t nsectors) "req %p sector %"PRIu64" nsectors %zu"
-virtio_blk_submit_multireq(void *mrb, int start, int num_reqs, uint64_t sector, size_t nsectors, bool is_write) "mrb %p start %d num_reqs %d sector %"PRIu64" nsectors %zu is_write %d"
+virtio_blk_submit_multireq(void *mrb, int start, int num_reqs, uint64_t offset, size_t size, bool is_write) "mrb %p start %d num_reqs %d offset %"PRIu64" size %zu is_write %d"
# hw/block/dataplane/virtio-blk.c
virtio_blk_data_plane_start(void *s) "dataplane %p"
--
2.5.5
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v6 09/20] xen_disk: Switch to byte-based aio block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 02/20] block: Drop private ioctl-only members of BlockRequest Eric Blake
` (19 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, Stefano Stabellini, Anthony Perard, Max Reitz,
open list:X86
Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hw/block/xen_disk.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index d4ce380..064c116 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -554,9 +554,8 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
block_acct_start(blk_get_stats(blkdev->blk), &ioreq->acct,
ioreq->v.size, BLOCK_ACCT_READ);
ioreq->aio_inflight++;
- blk_aio_readv(blkdev->blk, ioreq->start / BLOCK_SIZE,
- &ioreq->v, ioreq->v.size / BLOCK_SIZE,
- qemu_aio_complete, ioreq);
+ blk_aio_preadv(blkdev->blk, ioreq->start, &ioreq->v, 0,
+ qemu_aio_complete, ioreq);
break;
case BLKIF_OP_WRITE:
case BLKIF_OP_FLUSH_DISKCACHE:
@@ -569,9 +568,8 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
ioreq->req.operation == BLKIF_OP_WRITE ?
BLOCK_ACCT_WRITE : BLOCK_ACCT_FLUSH);
ioreq->aio_inflight++;
- blk_aio_writev(blkdev->blk, ioreq->start / BLOCK_SIZE,
- &ioreq->v, ioreq->v.size / BLOCK_SIZE,
- qemu_aio_complete, ioreq);
+ blk_aio_pwritev(blkdev->blk, ioreq->start, &ioreq->v, 0,
+ qemu_aio_complete, ioreq);
break;
case BLKIF_OP_DISCARD:
{
--
2.5.5
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v6 09/20] xen_disk: Switch to byte-based aio block access
@ 2016-05-04 23:55 ` Eric Blake
0 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Stefano Stabellini, qemu-block, Max Reitz, open list:X86,
Anthony Perard
Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hw/block/xen_disk.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index d4ce380..064c116 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -554,9 +554,8 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
block_acct_start(blk_get_stats(blkdev->blk), &ioreq->acct,
ioreq->v.size, BLOCK_ACCT_READ);
ioreq->aio_inflight++;
- blk_aio_readv(blkdev->blk, ioreq->start / BLOCK_SIZE,
- &ioreq->v, ioreq->v.size / BLOCK_SIZE,
- qemu_aio_complete, ioreq);
+ blk_aio_preadv(blkdev->blk, ioreq->start, &ioreq->v, 0,
+ qemu_aio_complete, ioreq);
break;
case BLKIF_OP_WRITE:
case BLKIF_OP_FLUSH_DISKCACHE:
@@ -569,9 +568,8 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
ioreq->req.operation == BLKIF_OP_WRITE ?
BLOCK_ACCT_WRITE : BLOCK_ACCT_FLUSH);
ioreq->aio_inflight++;
- blk_aio_writev(blkdev->blk, ioreq->start / BLOCK_SIZE,
- &ioreq->v, ioreq->v.size / BLOCK_SIZE,
- qemu_aio_complete, ioreq);
+ blk_aio_pwritev(blkdev->blk, ioreq->start, &ioreq->v, 0,
+ qemu_aio_complete, ioreq);
break;
case BLKIF_OP_DISCARD:
{
--
2.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v6 10/20] fdc: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (8 preceding siblings ...)
2016-05-04 23:55 ` Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 11/20] nand: " Eric Blake
` (10 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 11/20] nand: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (9 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 10/20] fdc: Switch to byte-based " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 12/20] onenand: " Eric Blake
` (9 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 12/20] onenand: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (10 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 11/20] nand: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 13/20] pflash: " Eric Blake
` (8 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 13/20] pflash: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (11 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 12/20] onenand: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 14/20] sd: " Eric Blake
` (7 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 14/20] sd: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (12 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 13/20] pflash: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 15/20] m25p80: " Eric Blake
` (6 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 15/20] m25p80: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (13 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 14/20] sd: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 16/20] atapi: " Eric Blake
` (5 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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.
Likewise for blk_aio_readv() and blk_aio_writev().
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hw/block/m25p80.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 906b712..5d30863 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -358,25 +358,21 @@ static void blk_sync_complete(void *opaque, int ret)
static void flash_sync_page(Flash *s, int page)
{
- int blk_sector, nb_sectors;
QEMUIOVector iov;
if (!s->blk || blk_is_read_only(s->blk)) {
return;
}
- blk_sector = (page * s->pi->page_size) / BDRV_SECTOR_SIZE;
- nb_sectors = DIV_ROUND_UP(s->pi->page_size, BDRV_SECTOR_SIZE);
qemu_iovec_init(&iov, 1);
- qemu_iovec_add(&iov, s->storage + blk_sector * BDRV_SECTOR_SIZE,
- nb_sectors * BDRV_SECTOR_SIZE);
- blk_aio_writev(s->blk, blk_sector, &iov, nb_sectors, blk_sync_complete,
- NULL);
+ qemu_iovec_add(&iov, s->storage + page * s->pi->page_size,
+ s->pi->page_size);
+ blk_aio_pwritev(s->blk, page * s->pi->page_size, &iov, 0,
+ blk_sync_complete, NULL);
}
static inline void flash_sync_area(Flash *s, int64_t off, int64_t len)
{
- int64_t start, end, nb_sectors;
QEMUIOVector iov;
if (!s->blk || blk_is_read_only(s->blk)) {
@@ -384,13 +380,9 @@ static inline void flash_sync_area(Flash *s, int64_t off, int64_t len)
}
assert(!(len % BDRV_SECTOR_SIZE));
- start = off / BDRV_SECTOR_SIZE;
- end = (off + len) / BDRV_SECTOR_SIZE;
- nb_sectors = end - start;
qemu_iovec_init(&iov, 1);
- qemu_iovec_add(&iov, s->storage + (start * BDRV_SECTOR_SIZE),
- nb_sectors * BDRV_SECTOR_SIZE);
- blk_aio_writev(s->blk, start, &iov, nb_sectors, blk_sync_complete, NULL);
+ qemu_iovec_add(&iov, s->storage + off, len);
+ blk_aio_pwritev(s->blk, off, &iov, 0, blk_sync_complete, NULL);
}
static void flash_erase(Flash *s, int offset, FlashCMD cmd)
@@ -907,8 +899,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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 16/20] atapi: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (14 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 15/20] m25p80: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 17/20] nbd: " Eric Blake
` (4 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 17/20] nbd: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (15 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 16/20] atapi: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-06 13:08 ` Kevin Wolf
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 18/20] qemu-img: " Eric Blake
` (3 subsequent siblings)
20 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 18/20] qemu-img: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (16 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 17/20] nbd: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 19/20] qemu-io: " Eric Blake
` (2 subsequent siblings)
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 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 76430a8..491a460 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;
}
@@ -3024,7 +3030,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;
@@ -3038,7 +3045,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;
@@ -3054,8 +3062,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] 33+ messages in thread
* [Qemu-devel] [PATCH v6 19/20] qemu-io: Switch to byte-based block access
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (17 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 18/20] qemu-img: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 20/20] block: Kill unused sector-based blk_* functions Eric Blake
2016-05-06 13:11 ` [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Kevin Wolf
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz
qemu-io is the last user of several sector-based interfaces.
This patch upgrades to the new interfaces under the hood,
then deletes the resulting dead code. Note that for maximum
back-compat, while the -p option is no longer required to get
blk_pread(), it is still needed to allow for unaligned access;
this is because qemu-iotest 23 relies on qemu-io rejecting
unaligned accesses without -p. A later patch may clean up the
interface to be more user-friendly, but it's better to separate
what's done under the hood from what the user sees.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qemu-io-cmds.c | 62 ++++++++++------------------------------------------------
1 file changed, 10 insertions(+), 52 deletions(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index a3e3982..0bbbc72 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)
{
@@ -588,8 +554,7 @@ static int do_aio_readv(BlockBackend *blk, QEMUIOVector *qiov,
{
int async_ret = NOT_DONE;
- blk_aio_readv(blk, offset >> 9, qiov, qiov->size >> 9,
- aio_rw_done, &async_ret);
+ blk_aio_preadv(blk, offset, qiov, 0, aio_rw_done, &async_ret);
while (async_ret == NOT_DONE) {
main_loop_wait(false);
}
@@ -603,8 +568,7 @@ static int do_aio_writev(BlockBackend *blk, QEMUIOVector *qiov,
{
int async_ret = NOT_DONE;
- blk_aio_writev(blk, offset >> 9, qiov, qiov->size >> 9,
- aio_rw_done, &async_ret);
+ blk_aio_pwritev(blk, offset, qiov, 0, aio_rw_done, &async_ret);
while (async_ret == NOT_DONE) {
main_loop_wait(false);
}
@@ -670,7 +634,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, -- allow unaligned access\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"
@@ -805,12 +769,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);
@@ -990,7 +952,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, -- allow unaligned access\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"
@@ -1106,16 +1068,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);
@@ -1592,8 +1552,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
gettimeofday(&ctx->t1, NULL);
block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,
BLOCK_ACCT_READ);
- blk_aio_readv(blk, ctx->offset >> 9, &ctx->qiov,
- ctx->qiov.size >> 9, aio_read_done, ctx);
+ blk_aio_preadv(blk, ctx->offset, &ctx->qiov, 0, aio_read_done, ctx);
return 0;
}
@@ -1717,8 +1676,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,
BLOCK_ACCT_WRITE);
- blk_aio_writev(blk, ctx->offset >> 9, &ctx->qiov,
- ctx->qiov.size >> 9, aio_write_done, ctx);
+ blk_aio_pwritev(blk, ctx->offset, &ctx->qiov, 0, aio_write_done, ctx);
}
return 0;
}
--
2.5.5
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v6 20/20] block: Kill unused sector-based blk_* functions
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (18 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 19/20] qemu-io: " Eric Blake
@ 2016-05-04 23:55 ` Eric Blake
2016-05-06 13:11 ` [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Kevin Wolf
20 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-04 23:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, Max Reitz
Now that there are no remaining clients, we can drop the
sector-based blk_read(), blk_write(), blk_aio_readv(), and
blk_aio_writev(). Sadly, there are still remaining
sector-based interfaces, such as blk_*discard(), or
blk_write_compressed(); those will have to wait for another
day.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/sysemu/block-backend.h | 10 ---------
block/block-backend.c | 51 ------------------------------------------
2 files changed, 61 deletions(-)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8d7839c..457efd9 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_write_zeroes(BlockBackend *blk, int64_t offset,
int count, BdrvRequestFlags flags);
BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t offset,
@@ -107,15 +103,9 @@ int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count,
int64_t blk_getlength(BlockBackend *blk);
void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr);
int64_t blk_nb_sectors(BlockBackend *blk);
-BlockAIOCB *blk_aio_readv(BlockBackend *blk, int64_t sector_num,
- QEMUIOVector *iov, int nb_sectors,
- BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
QEMUIOVector *iov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *blk_aio_writev(BlockBackend *blk, int64_t sector_num,
- QEMUIOVector *iov, int nb_sectors,
- BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset,
QEMUIOVector *iov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque);
diff --git a/block/block-backend.c b/block/block-backend.c
index 104852d..0551be4 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_write_zeroes(BlockBackend *blk, int64_t offset,
int count, BdrvRequestFlags flags)
{
@@ -985,19 +960,6 @@ int64_t blk_nb_sectors(BlockBackend *blk)
return bdrv_nb_sectors(blk_bs(blk));
}
-BlockAIOCB *blk_aio_readv(BlockBackend *blk, int64_t sector_num,
- QEMUIOVector *iov, int nb_sectors,
- BlockCompletionFunc *cb, void *opaque)
-{
- if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
- return blk_abort_aio_request(blk, cb, opaque, -EINVAL);
- }
-
- assert(nb_sectors << BDRV_SECTOR_BITS == iov->size);
- return blk_aio_prwv(blk, sector_num << BDRV_SECTOR_BITS, iov->size, iov,
- blk_aio_read_entry, 0, cb, opaque);
-}
-
BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
QEMUIOVector *iov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque)
@@ -1006,19 +968,6 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
blk_aio_read_entry, flags, cb, opaque);
}
-BlockAIOCB *blk_aio_writev(BlockBackend *blk, int64_t sector_num,
- QEMUIOVector *iov, int nb_sectors,
- BlockCompletionFunc *cb, void *opaque)
-{
- if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
- return blk_abort_aio_request(blk, cb, opaque, -EINVAL);
- }
-
- assert(nb_sectors << BDRV_SECTOR_BITS == iov->size);
- return blk_aio_prwv(blk, sector_num << BDRV_SECTOR_BITS, iov->size, iov,
- blk_aio_write_entry, 0, cb, opaque);
-}
-
BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset,
QEMUIOVector *iov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque)
--
2.5.5
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/20] block: Drop private ioctl-only members of BlockRequest
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 02/20] block: Drop private ioctl-only members of BlockRequest Eric Blake
@ 2016-05-06 10:37 ` Kevin Wolf
2016-05-06 12:23 ` Eric Blake
0 siblings, 1 reply; 33+ messages in thread
From: Kevin Wolf @ 2016-05-06 10:37 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Fam Zheng, Max Reitz
Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
> I was thrown by the fact that the public type BlockRequest had
> an anonymous union, but no obvious discriminator. Turns out
> that the only client of the second branch of the union was code
> internal to io.c, and that with a slight abuse of QEMUIOVector*
> to pass a void* pointer, we can make the public interface less
> confusing.
>
> (Yes, I know that strict C doesn't guarantee that you can cast
> void* to the wrong type and then back to void* - it only
> guarantees the reverse direction of the original pointer to
> void* and back to the original type - but we already have other
> assumptions throughout the qemu code base that assume that all
> pointers are interchangeable in representation).
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
Do you really think abusing fields makes things clearer than using a
union? We could just add comments instead that tell which branch is used
for what. And after my patch "block: Remove bdrv_aio_multiwrite()", I
think the struct isn't part of a public interface any more anyway.
Kevin
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/20] block: Drop private ioctl-only members of BlockRequest
2016-05-06 10:37 ` Kevin Wolf
@ 2016-05-06 12:23 ` Eric Blake
0 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-06 12:23 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Fam Zheng, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]
On 05/06/2016 04:37 AM, Kevin Wolf wrote:
> Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
>> I was thrown by the fact that the public type BlockRequest had
>> an anonymous union, but no obvious discriminator. Turns out
>> that the only client of the second branch of the union was code
>> internal to io.c, and that with a slight abuse of QEMUIOVector*
>> to pass a void* pointer, we can make the public interface less
>> confusing.
>>
>> (Yes, I know that strict C doesn't guarantee that you can cast
>> void* to the wrong type and then back to void* - it only
>> guarantees the reverse direction of the original pointer to
>> void* and back to the original type - but we already have other
>> assumptions throughout the qemu code base that assume that all
>> pointers are interchangeable in representation).
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> Do you really think abusing fields makes things clearer than using a
> union? We could just add comments instead that tell which branch is used
> for what. And after my patch "block: Remove bdrv_aio_multiwrite()", I
> think the struct isn't part of a public interface any more anyway.
Oh, you're right! Consider this patch severed from the series (I may
still send a documentation patch, and/or move the struct out of the
public header into block_int.h, but it is sufficiently unrelated to the
rest of the series).
--
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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/20] block: Introduce byte-based aio read/write
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 05/20] block: Introduce byte-based aio read/write Eric Blake
@ 2016-05-06 12:31 ` Kevin Wolf
2016-05-06 14:12 ` Eric Blake
0 siblings, 1 reply; 33+ messages in thread
From: Kevin Wolf @ 2016-05-06 12:31 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block, Max Reitz
Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
> blk_aio_readv() and blk_aio_writev() are annoying in that they
> can't access sub-sector granularity, and cannot pass flags.
> Also, they require the caller to pass redundant information
> about the size of the I/O.
>
> Add new blk_aio_preadv() and blk_aio_pwritev() functions to fix
> the flaws. The next few patches will upgrade callers, then
> finally delete the old interfaces.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> include/sysemu/block-backend.h | 6 ++++++
> block/block-backend.c | 16 ++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index 851376b..8d7839c 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -110,9 +110,15 @@ int64_t blk_nb_sectors(BlockBackend *blk);
> BlockAIOCB *blk_aio_readv(BlockBackend *blk, int64_t sector_num,
> QEMUIOVector *iov, int nb_sectors,
> BlockCompletionFunc *cb, void *opaque);
> +BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
> + QEMUIOVector *iov, BdrvRequestFlags flags,
If you have to respin for some reason, I think QEMUIOVectors are
commonly called qiov, whereas iov is usually the name for struct iovec.
Kevin
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 07/20] scsi-disk: Switch to byte-based aio block access
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 07/20] scsi-disk: " Eric Blake
@ 2016-05-06 12:50 ` Kevin Wolf
2016-05-06 14:18 ` Eric Blake
0 siblings, 1 reply; 33+ messages in thread
From: Kevin Wolf @ 2016-05-06 12:50 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block, Paolo Bonzini
Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
> Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
> to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> hw/scsi/scsi-disk.c | 31 +++++++++++++++----------------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 1335392..5d98f7b 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -343,8 +343,9 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
> n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
> block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
> n * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
If you replace n * BDRV_SECTOR_SIZE here as well, n is unused and can go
away (you actually did that already for writes).
> - r->req.aiocb = blk_aio_readv(s->qdev.conf.blk, r->sector, &r->qiov, n,
> - scsi_read_complete, r);
> + r->req.aiocb = blk_aio_preadv(s->qdev.conf.blk,
> + r->sector << BDRV_SECTOR_BITS, &r->qiov,
> + 0, scsi_read_complete, r);
> }
>
> done:
> @@ -504,7 +505,6 @@ static void scsi_write_data(SCSIRequest *req)
> {
> SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
> - uint32_t n;
>
> /* No data transfer may already be in progress */
> assert(r->req.aiocb == NULL);
> @@ -544,11 +544,11 @@ static void scsi_write_data(SCSIRequest *req)
> r->req.aiocb = dma_blk_write(s->qdev.conf.blk, r->req.sg, r->sector,
> scsi_dma_complete, r);
> } else {
> - n = r->qiov.size / 512;
> block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
> - n * BDRV_SECTOR_SIZE, BLOCK_ACCT_WRITE);
> - r->req.aiocb = blk_aio_writev(s->qdev.conf.blk, r->sector, &r->qiov, n,
> - scsi_write_complete, r);
> + r->qiov.size, BLOCK_ACCT_WRITE);
> + r->req.aiocb = blk_aio_pwritev(s->qdev.conf.blk,
> + r->sector << BDRV_SECTOR_BITS, &r->qiov,
> + 0, scsi_write_complete, r);
> }
> }
>
> @@ -1730,13 +1730,11 @@ static void scsi_write_same_complete(void *opaque, int ret)
> if (data->iov.iov_len) {
> block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
> data->iov.iov_len, BLOCK_ACCT_WRITE);
> - /* blk_aio_write doesn't like the qiov size being different from
> - * nb_sectors, make sure they match.
> - */
Wouldn't it be better to update the comment instead of deleting it? If I
understand correctly, this additional qemu_iovec_init_external() is for
the last part of an unaligned WRITE SAME request, where the qiov can
become shorter than in the previous iterations.
> qemu_iovec_init_external(&data->qiov, &data->iov, 1);
> - r->req.aiocb = blk_aio_writev(s->qdev.conf.blk, data->sector,
> - &data->qiov, data->iov.iov_len / 512,
> - scsi_write_same_complete, data);
> + r->req.aiocb = blk_aio_pwritev(s->qdev.conf.blk,
> + data->sector << BDRV_SECTOR_BITS,
> + &data->qiov, 0,
> + scsi_write_same_complete, data);
> return;
> }
Kevin
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 17/20] nbd: Switch to byte-based block access
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 17/20] nbd: " Eric Blake
@ 2016-05-06 13:08 ` Kevin Wolf
2016-05-06 14:19 ` Eric Blake
0 siblings, 1 reply; 33+ messages in thread
From: Kevin Wolf @ 2016-05-06 13:08 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block, Paolo Bonzini
Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
> 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];
I like 512 better, actually. This is not the size of the arbitrary unit
that some block layer functions use, but the size of an MBR. If you
don't like the magic number, a new #define would probably be best.
> 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];
Same here.
> 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);
> }
Kevin
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
` (19 preceding siblings ...)
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 20/20] block: Kill unused sector-based blk_* functions Eric Blake
@ 2016-05-06 13:11 ` Kevin Wolf
2016-05-06 14:20 ` Eric Blake
20 siblings, 1 reply; 33+ messages in thread
From: Kevin Wolf @ 2016-05-06 13:11 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block
Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
> 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 v5 [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-05/msg00235.html
I had some comments on patch 2, 5, 7 and 17. All of them are minor, so
I'll leave it to you whether you want to do another version or not. Just
let me know.
Kevin
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/20] block: Introduce byte-based aio read/write
2016-05-06 12:31 ` Kevin Wolf
@ 2016-05-06 14:12 ` Eric Blake
0 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-06 14:12 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 806 bytes --]
On 05/06/2016 06:31 AM, Kevin Wolf wrote:
>> +++ b/include/sysemu/block-backend.h
>> @@ -110,9 +110,15 @@ int64_t blk_nb_sectors(BlockBackend *blk);
>> BlockAIOCB *blk_aio_readv(BlockBackend *blk, int64_t sector_num,
>> QEMUIOVector *iov, int nb_sectors,
>> BlockCompletionFunc *cb, void *opaque);
>> +BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
>> + QEMUIOVector *iov, BdrvRequestFlags flags,
>
> If you have to respin for some reason, I think QEMUIOVectors are
> commonly called qiov, whereas iov is usually the name for struct iovec.
Copy-and-paste, but also a trivial cleanup.
--
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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 07/20] scsi-disk: Switch to byte-based aio block access
2016-05-06 12:50 ` Kevin Wolf
@ 2016-05-06 14:18 ` Eric Blake
2016-05-06 14:49 ` Kevin Wolf
0 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2016-05-06 14:18 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 2761 bytes --]
On 05/06/2016 06:50 AM, Kevin Wolf wrote:
> Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
>> Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
>> to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
>>
>> @@ -343,8 +343,9 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
>> n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
>> block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
>> n * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
>
> If you replace n * BDRV_SECTOR_SIZE here as well, n is unused and can go
> away (you actually did that already for writes).
Sure, I can add that in.
>> @@ -1730,13 +1730,11 @@ static void scsi_write_same_complete(void *opaque, int ret)
>> if (data->iov.iov_len) {
>> block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
>> data->iov.iov_len, BLOCK_ACCT_WRITE);
>> - /* blk_aio_write doesn't like the qiov size being different from
>> - * nb_sectors, make sure they match.
>> - */
>
> Wouldn't it be better to update the comment instead of deleting it? If I
> understand correctly, this additional qemu_iovec_init_external() is for
> the last part of an unaligned WRITE SAME request, where the qiov can
> become shorter than in the previous iterations.
The comment mattered when you were passing two sizes to blk_aio_writev
(one embedded in data->qiov, and one in terms of sectors as a direct
argument); the block layer asserted the two were equal. But now that we
are handling bytes, we pass in a single size (that of data->qiov), so
the comment no longer makes sense.
>
>> qemu_iovec_init_external(&data->qiov, &data->iov, 1);
>> - r->req.aiocb = blk_aio_writev(s->qdev.conf.blk, data->sector,
>> - &data->qiov, data->iov.iov_len / 512,
>> - scsi_write_same_complete, data);
>> + r->req.aiocb = blk_aio_pwritev(s->qdev.conf.blk,
>> + data->sector << BDRV_SECTOR_BITS,
>> + &data->qiov, 0,
It caught me more than once while writing the patch that my new function
replaced nb_sectors (the duplicated size) by flags, rather than adding a
parameter (as it was harder to get the compiler to gripe about
incomplete conversions, such as forgetting to s/write/pwrite/). But by
the end of the series, when the old interface is gone, everything still
compiles under the new name, so at least we're assured that the series
worked on that front.
--
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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 17/20] nbd: Switch to byte-based block access
2016-05-06 13:08 ` Kevin Wolf
@ 2016-05-06 14:19 ` Eric Blake
0 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-06 14:19 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]
On 05/06/2016 07:08 AM, Kevin Wolf wrote:
> Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
>> 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];
>
> I like 512 better, actually. This is not the size of the arbitrary unit
> that some block layer functions use, but the size of an MBR. If you
> don't like the magic number, a new #define would probably be best.
MBR_SIZE it will be :)
--
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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read
2016-05-06 13:11 ` [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Kevin Wolf
@ 2016-05-06 14:20 ` Eric Blake
0 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2016-05-06 14:20 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]
On 05/06/2016 07:11 AM, Kevin Wolf wrote:
> Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
>> 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 v5 [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-05/msg00235.html
>
> I had some comments on patch 2, 5, 7 and 17. All of them are minor, so
> I'll leave it to you whether you want to do another version or not. Just
> let me know.
I don't mind doing another spin; your comments will be enough churn to
be worth a good review on those patches (that, and 2 disappearing makes
it easier if you have a clean series to 'git am').
--
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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v6 07/20] scsi-disk: Switch to byte-based aio block access
2016-05-06 14:18 ` Eric Blake
@ 2016-05-06 14:49 ` Kevin Wolf
0 siblings, 0 replies; 33+ messages in thread
From: Kevin Wolf @ 2016-05-06 14:49 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 2652 bytes --]
Am 06.05.2016 um 16:18 hat Eric Blake geschrieben:
> On 05/06/2016 06:50 AM, Kevin Wolf wrote:
> > Am 05.05.2016 um 01:55 hat Eric Blake geschrieben:
> >> @@ -1730,13 +1730,11 @@ static void scsi_write_same_complete(void *opaque, int ret)
> >> if (data->iov.iov_len) {
> >> block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
> >> data->iov.iov_len, BLOCK_ACCT_WRITE);
> >> - /* blk_aio_write doesn't like the qiov size being different from
> >> - * nb_sectors, make sure they match.
> >> - */
> >
> > Wouldn't it be better to update the comment instead of deleting it? If I
> > understand correctly, this additional qemu_iovec_init_external() is for
> > the last part of an unaligned WRITE SAME request, where the qiov can
> > become shorter than in the previous iterations.
>
> The comment mattered when you were passing two sizes to blk_aio_writev
> (one embedded in data->qiov, and one in terms of sectors as a direct
> argument); the block layer asserted the two were equal. But now that we
> are handling bytes, we pass in a single size (that of data->qiov), so
> the comment no longer makes sense.
Yes, with the current text it doesn't make sense any more. But it
explains why we have another qemu_iovec_init_external() here, and as
that call remains, it would still be good to have an explanation.
Or maybe it's obvious, don't know...
> >
> >> qemu_iovec_init_external(&data->qiov, &data->iov, 1);
> >> - r->req.aiocb = blk_aio_writev(s->qdev.conf.blk, data->sector,
> >> - &data->qiov, data->iov.iov_len / 512,
> >> - scsi_write_same_complete, data);
> >> + r->req.aiocb = blk_aio_pwritev(s->qdev.conf.blk,
> >> + data->sector << BDRV_SECTOR_BITS,
> >> + &data->qiov, 0,
>
> It caught me more than once while writing the patch that my new function
> replaced nb_sectors (the duplicated size) by flags, rather than adding a
> parameter (as it was harder to get the compiler to gripe about
> incomplete conversions, such as forgetting to s/write/pwrite/). But by
> the end of the series, when the old interface is gone, everything still
> compiles under the new name, so at least we're assured that the series
> worked on that front.
Yes, I was a bit worried that I might miss possible cases where you
still pass bytes instead of flags when the compiler can't complain about
them. I didn't see any, so hopefully that means it's good.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2016-05-06 14:50 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-04 23:55 [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 01/20] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 02/20] block: Drop private ioctl-only members of BlockRequest Eric Blake
2016-05-06 10:37 ` Kevin Wolf
2016-05-06 12:23 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 03/20] block: Switch blk_read_unthrottled() to byte interface Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 04/20] block: Switch blk_*write_zeroes() " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 05/20] block: Introduce byte-based aio read/write Eric Blake
2016-05-06 12:31 ` Kevin Wolf
2016-05-06 14:12 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 06/20] ide: Switch to byte-based aio block access Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 07/20] scsi-disk: " Eric Blake
2016-05-06 12:50 ` Kevin Wolf
2016-05-06 14:18 ` Eric Blake
2016-05-06 14:49 ` Kevin Wolf
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 08/20] virtio: " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 09/20] xen_disk: " Eric Blake
2016-05-04 23:55 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 10/20] fdc: Switch to byte-based " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 11/20] nand: " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 12/20] onenand: " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 13/20] pflash: " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 14/20] sd: " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 15/20] m25p80: " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 16/20] atapi: " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 17/20] nbd: " Eric Blake
2016-05-06 13:08 ` Kevin Wolf
2016-05-06 14:19 ` Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 18/20] qemu-img: " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 19/20] qemu-io: " Eric Blake
2016-05-04 23:55 ` [Qemu-devel] [PATCH v6 20/20] block: Kill unused sector-based blk_* functions Eric Blake
2016-05-06 13:11 ` [Qemu-devel] [PATCH v6 00/20] block: kill sector-based blk_write/read Kevin Wolf
2016-05-06 14:20 ` 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.