* [Qemu-devel] [PATCH v7 01/19] block: Allow BDRV_REQ_FUA through blk_pwrite()
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 02/19] block: Switch blk_read_unthrottled() to byte interface Eric Blake
` (19 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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 9023686..23fbace 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1681,7 +1681,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 61e20af..e6c97c2 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;
@@ -2120,7 +2120,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] 30+ messages in thread* [Qemu-devel] [PATCH v7 02/19] block: Switch blk_read_unthrottled() to byte interface
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 01/19] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 03/19] block: Switch blk_*write_zeroes() " Eric Blake
` (18 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 03/19] block: Switch blk_*write_zeroes() to byte interface
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 01/19] block: Allow BDRV_REQ_FUA through blk_pwrite() Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 02/19] block: Switch blk_read_unthrottled() to byte interface Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-23 15:42 ` Kevin Wolf
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 04/19] block: Introduce byte-based aio read/write Eric Blake
` (17 subsequent siblings)
20 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* Re: [Qemu-devel] [PATCH v7 03/19] block: Switch blk_*write_zeroes() to byte interface
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 03/19] block: Switch blk_*write_zeroes() " Eric Blake
@ 2016-05-23 15:42 ` Kevin Wolf
2016-05-23 16:03 ` Eric Blake
0 siblings, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2016-05-23 15:42 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, qemu-block, Max Reitz, Stefan Hajnoczi,
Denis V. Lunev, Paolo Bonzini
Am 06.05.2016 um 18:26 hat Eric Blake geschrieben:
> 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>
You built a nice trap here: bdrv_*write_zeroes() is still sector based
whereas blk_*write_zeroes() changed to be byte based without a rename.
That broke my block jobs series which converts from bdrv_* to blk_*
after the next (seemingly clean) rebase.
I fixed my series now, but can we please convert bdrv_* as well
before we get more victims in the same trap? Also (and perhaps more
importantly) we could rename the functions to something like
blk_*pwrite_zeroes(), indicating the switch to byte based.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v7 03/19] block: Switch blk_*write_zeroes() to byte interface
2016-05-23 15:42 ` Kevin Wolf
@ 2016-05-23 16:03 ` Eric Blake
0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-23 16:03 UTC (permalink / raw)
To: Kevin Wolf
Cc: qemu-devel, qemu-block, Max Reitz, Stefan Hajnoczi,
Denis V. Lunev, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]
On 05/23/2016 09:42 AM, Kevin Wolf wrote:
> Am 06.05.2016 um 18:26 hat Eric Blake geschrieben:
>> 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>
>
> You built a nice trap here: bdrv_*write_zeroes() is still sector based
> whereas blk_*write_zeroes() changed to be byte based without a rename.
> That broke my block jobs series which converts from bdrv_* to blk_*
> after the next (seemingly clean) rebase.
>
> I fixed my series now, but can we please convert bdrv_* as well
> before we get more victims in the same trap? Also (and perhaps more
> importantly) we could rename the functions to something like
> blk_*pwrite_zeroes(), indicating the switch to byte based.
Sure, the rename idea makes sense. I should have something posted later
today.
--
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] 30+ messages in thread
* [Qemu-devel] [PATCH v7 04/19] block: Introduce byte-based aio read/write
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (2 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 03/19] block: Switch blk_*write_zeroes() " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 05/19] ide: Switch to byte-based aio block access Eric Blake
` (16 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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 (qiov->size in bytes must match
nb_sectors in sectors).
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>
---
v7: Prefer 'qiov' name for QEMUIOVector
---
include/sysemu/block-backend.h | 8 +++++++-
block/block-backend.c | 18 +++++++++++++++++-
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 851376b..73df1a6 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -1,7 +1,7 @@
/*
* QEMU Block backends
*
- * Copyright (C) 2014 Red Hat, Inc.
+ * Copyright (C) 2014-2016 Red Hat, Inc.
*
* Authors:
* Markus Armbruster <armbru@redhat.com>,
@@ -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 *qiov, 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 *qiov, 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..6ac76d0 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1,7 +1,7 @@
/*
* QEMU Block backends
*
- * Copyright (C) 2014 Red Hat, Inc.
+ * Copyright (C) 2014-2016 Red Hat, Inc.
*
* Authors:
* Markus Armbruster <armbru@redhat.com>,
@@ -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 *qiov, BdrvRequestFlags flags,
+ BlockCompletionFunc *cb, void *opaque)
+{
+ return blk_aio_prwv(blk, offset, qiov->size, qiov,
+ 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 *qiov, BdrvRequestFlags flags,
+ BlockCompletionFunc *cb, void *opaque)
+{
+ return blk_aio_prwv(blk, offset, qiov->size, qiov,
+ 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 05/19] ide: Switch to byte-based aio block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (3 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 04/19] block: Introduce byte-based aio read/write Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 06/19] scsi-disk: " Eric Blake
` (15 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 06/19] scsi-disk: Switch to byte-based aio block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (4 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 05/19] ide: Switch to byte-based aio block access Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-10 8:55 ` Kevin Wolf
2016-05-12 11:25 ` Paolo Bonzini
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 07/19] virtio: " Eric Blake
` (14 subsequent siblings)
20 siblings, 2 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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.
As part of the cleanup, scsi_init_iovec() no longer needs to return
a value, and reword a comment.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v7: remove more unused 'n', fix comment wording
---
hw/scsi/scsi-disk.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 1335392..d6bb9ad 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -108,7 +108,7 @@ static void scsi_check_condition(SCSIDiskReq *r, SCSISense sense)
scsi_req_complete(&r->req, CHECK_CONDITION);
}
-static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
+static void scsi_init_iovec(SCSIDiskReq *r, size_t size)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
@@ -118,7 +118,6 @@ static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
}
r->iov.iov_len = MIN(r->sector_count * 512, r->buflen);
qemu_iovec_init_external(&r->qiov, &r->iov, 1);
- return r->qiov.size / 512;
}
static void scsi_disk_save_request(QEMUFile *f, SCSIRequest *req)
@@ -316,7 +315,6 @@ done:
static void scsi_do_read(SCSIDiskReq *r, int ret)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
- uint32_t n;
assert (r->req.aiocb == NULL);
@@ -340,11 +338,12 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
r->req.aiocb = dma_blk_read(s->qdev.conf.blk, r->req.sg, r->sector,
scsi_dma_complete, r);
} else {
- n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
+ 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);
+ SCSI_DMA_BUF_SIZE, BLOCK_ACCT_READ);
+ 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 +503,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 +542,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 +1728,13 @@ 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.
- */
+ /* Reinitialize qiov, to handle unaligned WRITE SAME request
+ * where final qiov may need smaller size */
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] 30+ messages in thread* Re: [Qemu-devel] [PATCH v7 06/19] scsi-disk: Switch to byte-based aio block access
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 06/19] scsi-disk: " Eric Blake
@ 2016-05-10 8:55 ` Kevin Wolf
2016-05-10 12:56 ` Eric Blake
2016-05-12 11:25 ` Paolo Bonzini
1 sibling, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2016-05-10 8:55 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block, Paolo Bonzini
Am 06.05.2016 um 18:26 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.
>
> As part of the cleanup, scsi_init_iovec() no longer needs to return
> a value, and reword a comment.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v7: remove more unused 'n', fix comment wording
> ---
> hw/scsi/scsi-disk.c | 41 ++++++++++++++++++++---------------------
> 1 file changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 1335392..d6bb9ad 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -108,7 +108,7 @@ static void scsi_check_condition(SCSIDiskReq *r, SCSISense sense)
> scsi_req_complete(&r->req, CHECK_CONDITION);
> }
>
> -static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
> +static void scsi_init_iovec(SCSIDiskReq *r, size_t size)
> {
> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
>
> @@ -118,7 +118,6 @@ static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
> }
> r->iov.iov_len = MIN(r->sector_count * 512, r->buflen);
> qemu_iovec_init_external(&r->qiov, &r->iov, 1);
> - return r->qiov.size / 512;
The return value was MIN(r->sector_count, SCSI_DMA_BUF_SIZE / 512).
> }
>
> static void scsi_disk_save_request(QEMUFile *f, SCSIRequest *req)
> @@ -316,7 +315,6 @@ done:
> static void scsi_do_read(SCSIDiskReq *r, int ret)
> {
> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
> - uint32_t n;
>
> assert (r->req.aiocb == NULL);
>
> @@ -340,11 +338,12 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
> r->req.aiocb = dma_blk_read(s->qdev.conf.blk, r->req.sg, r->sector,
> scsi_dma_complete, r);
> } else {
> - n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
> + 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);
> + SCSI_DMA_BUF_SIZE, BLOCK_ACCT_READ);
But here you use SCSI_DMA_BUF_SIZE without considering r->sector_count.
Is this correct or are requests that are smaller than SCSI_DMA_BUF_SIZE
now accounted for more data than they actually read?
Would r->qiov.size be more obviously correct? You did that for writes.
> + r->req.aiocb = blk_aio_preadv(s->qdev.conf.blk,
> + r->sector << BDRV_SECTOR_BITS, &r->qiov,
> + 0, scsi_read_complete, r);
> }
>
> done:
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v7 06/19] scsi-disk: Switch to byte-based aio block access
2016-05-10 8:55 ` Kevin Wolf
@ 2016-05-10 12:56 ` Eric Blake
2016-05-10 14:49 ` Kevin Wolf
0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2016-05-10 12:56 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]
On 05/10/2016 02:55 AM, Kevin Wolf wrote:
> Am 06.05.2016 um 18:26 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.
>>
>> As part of the cleanup, scsi_init_iovec() no longer needs to return
>> a value, and reword a comment.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>
>> @@ -118,7 +118,6 @@ static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
>> }
>> r->iov.iov_len = MIN(r->sector_count * 512, r->buflen);
>> qemu_iovec_init_external(&r->qiov, &r->iov, 1);
>> - return r->qiov.size / 512;
>
> The return value was MIN(r->sector_count, SCSI_DMA_BUF_SIZE / 512).
>
>> }
>>
>> static void scsi_disk_save_request(QEMUFile *f, SCSIRequest *req)
>> - n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
>> + 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);
>> + SCSI_DMA_BUF_SIZE, BLOCK_ACCT_READ);
>
> But here you use SCSI_DMA_BUF_SIZE without considering r->sector_count.
> Is this correct or are requests that are smaller than SCSI_DMA_BUF_SIZE
> now accounted for more data than they actually read?
>
> Would r->qiov.size be more obviously correct? You did that for writes.
Yes. Is that something you are able to fix on commit, or should I submit
a fixup?
--
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] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v7 06/19] scsi-disk: Switch to byte-based aio block access
2016-05-10 12:56 ` Eric Blake
@ 2016-05-10 14:49 ` Kevin Wolf
0 siblings, 0 replies; 30+ messages in thread
From: Kevin Wolf @ 2016-05-10 14:49 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1846 bytes --]
Am 10.05.2016 um 14:56 hat Eric Blake geschrieben:
> On 05/10/2016 02:55 AM, Kevin Wolf wrote:
> > Am 06.05.2016 um 18:26 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.
> >>
> >> As part of the cleanup, scsi_init_iovec() no longer needs to return
> >> a value, and reword a comment.
> >>
> >> Signed-off-by: Eric Blake <eblake@redhat.com>
> >>
>
> >> @@ -118,7 +118,6 @@ static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
> >> }
> >> r->iov.iov_len = MIN(r->sector_count * 512, r->buflen);
> >> qemu_iovec_init_external(&r->qiov, &r->iov, 1);
> >> - return r->qiov.size / 512;
> >
> > The return value was MIN(r->sector_count, SCSI_DMA_BUF_SIZE / 512).
> >
> >> }
> >>
> >> static void scsi_disk_save_request(QEMUFile *f, SCSIRequest *req)
>
> >> - n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
> >> + 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);
> >> + SCSI_DMA_BUF_SIZE, BLOCK_ACCT_READ);
> >
> > But here you use SCSI_DMA_BUF_SIZE without considering r->sector_count.
> > Is this correct or are requests that are smaller than SCSI_DMA_BUF_SIZE
> > now accounted for more data than they actually read?
> >
> > Would r->qiov.size be more obviously correct? You did that for writes.
>
> Yes. Is that something you are able to fix on commit, or should I submit
> a fixup?
Okay, I can fix it up while applying.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v7 06/19] scsi-disk: Switch to byte-based aio block access
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 06/19] scsi-disk: " Eric Blake
2016-05-10 8:55 ` Kevin Wolf
@ 2016-05-12 11:25 ` Paolo Bonzini
2016-05-12 11:26 ` Paolo Bonzini
2016-05-12 16:58 ` Eric Blake
1 sibling, 2 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-05-12 11:25 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: kwolf, qemu-block
On 06/05/2016 18:26, Eric Blake wrote:
> @@ -340,11 +338,12 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
> r->req.aiocb = dma_blk_read(s->qdev.conf.blk, r->req.sg, r->sector,
> scsi_dma_complete, r);
This is broken, it should be changed to an offset in the previous patch.
Please rename the function too, so that it is obvious that you have
changed all callers.
How was this patch tested?
Paolo
> } else {
> - n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE);
> + 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);
> + SCSI_DMA_BUF_SIZE, BLOCK_ACCT_READ);
> + r->req.aiocb = blk_aio_preadv(s->qdev.conf.blk,
> + r->sector << BDRV_SECTOR_BITS, &r->qiov,
> + 0, scsi_read_complete, r);
> }
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v7 06/19] scsi-disk: Switch to byte-based aio block access
2016-05-12 11:25 ` Paolo Bonzini
@ 2016-05-12 11:26 ` Paolo Bonzini
2016-05-12 16:58 ` Eric Blake
1 sibling, 0 replies; 30+ messages in thread
From: Paolo Bonzini @ 2016-05-12 11:26 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: kwolf, qemu-block
On 12/05/2016 13:25, Paolo Bonzini wrote:
>> > r->req.aiocb = dma_blk_read(s->qdev.conf.blk, r->req.sg, r->sector,
>> > scsi_dma_complete, r);
> This is broken, it should be changed to an offset in the previous patch.
>
> Please rename the function too, so that it is obvious that you have
> changed all callers.
Oh, dma_blk_read is still sector-based...
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v7 06/19] scsi-disk: Switch to byte-based aio block access
2016-05-12 11:25 ` Paolo Bonzini
2016-05-12 11:26 ` Paolo Bonzini
@ 2016-05-12 16:58 ` Eric Blake
1 sibling, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-12 16:58 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: kwolf, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]
On 05/12/2016 05:25 AM, Paolo Bonzini wrote:
>
>
> On 06/05/2016 18:26, Eric Blake wrote:
>> @@ -340,11 +338,12 @@ static void scsi_do_read(SCSIDiskReq *r, int ret)
>> r->req.aiocb = dma_blk_read(s->qdev.conf.blk, r->req.sg, r->sector,
>> scsi_dma_complete, r);
>
> This is broken, it should be changed to an offset in the previous patch.
>
> Please rename the function too, so that it is obvious that you have
> changed all callers.
>
> How was this patch tested?
Sadly, my testing was limited to NBD use of the blk_ functions, then
compile testing and close audit of all other drivers, to limit the
changes to ONLY the blk_ calling conventions. That's why I kept sector
alignment in other calls (as you later noticed, that means
dma_blk_read() was unchanged in semantics).
Yes, there are more cleanups possible, such as altering things to a new
dma_blk_pread() with byte semantics, but I'm hoping that it can be done
on a per-driver basis by someone more familiar with how to test the
changes. I also like the fact that my patches were done at the very
beginning of the 2.7 cycle to maximize testing.
--
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] 30+ messages in thread
* [Qemu-devel] [PATCH v7 07/19] virtio: Switch to byte-based aio block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (5 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 06/19] scsi-disk: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 08/19] xen_disk: " Eric Blake
` (13 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 08/19] xen_disk: Switch to byte-based aio block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (6 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 07/19] virtio: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 09/19] fdc: Switch to byte-based " Eric Blake
` (12 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 09/19] fdc: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (7 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 08/19] xen_disk: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 10/19] nand: " Eric Blake
` (11 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 10/19] nand: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (8 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 09/19] fdc: Switch to byte-based " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 11/19] onenand: " Eric Blake
` (10 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 11/19] onenand: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (9 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 10/19] nand: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 12/19] pflash: " Eric Blake
` (9 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 12/19] pflash: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (10 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 11/19] onenand: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 13/19] sd: " Eric Blake
` (8 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 13/19] sd: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (11 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 12/19] pflash: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 14/19] m25p80: " Eric Blake
` (7 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 14/19] m25p80: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (12 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 13/19] sd: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 15/19] atapi: " Eric Blake
` (6 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 15/19] atapi: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (13 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 14/19] m25p80: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 16/19] nbd: " Eric Blake
` (5 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 16/19] nbd: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (14 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 15/19] atapi: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 17/19] qemu-img: " Eric Blake
` (4 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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.
Add a constant for our magic number 512, to make it obvious
that this size will NOT change even if BDRV_SECTOR_SIZE does,
even though the two happen to be the same for now. Split
assignments from conditionals to keep checkpatch.pl happy.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v7: new magic number
---
qemu-nbd.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c55b40f..3e54113 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -46,6 +46,8 @@
#define QEMU_NBD_OPT_TLSCREDS 261
#define QEMU_NBD_OPT_IMAGE_OPTS 262
+#define MBR_SIZE 512
+
static NBDExport *exp;
static bool newproto;
static int verbose;
@@ -159,12 +161,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[MBR_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 +185,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[MBR_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 * MBR_SIZE,
+ 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 17/19] qemu-img: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (15 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 16/19] nbd: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 18/19] qemu-io: " Eric Blake
` (3 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 18/19] qemu-io: Switch to byte-based block access
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (16 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 17/19] qemu-img: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 19/19] block: Kill unused sector-based blk_* functions Eric Blake
` (2 subsequent siblings)
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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] 30+ messages in thread* [Qemu-devel] [PATCH v7 19/19] block: Kill unused sector-based blk_* functions
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (17 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 18/19] qemu-io: " Eric Blake
@ 2016-05-06 16:26 ` Eric Blake
2016-05-10 8:56 ` [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Kevin Wolf
2016-05-10 15:06 ` Kevin Wolf
20 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2016-05-06 16:26 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 73df1a6..26736ed 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 *qiov, 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 *qiov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque);
diff --git a/block/block-backend.c b/block/block-backend.c
index 6ac76d0..a1e2c7f 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 *qiov, 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 *qiov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque)
--
2.5.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (18 preceding siblings ...)
2016-05-06 16:26 ` [Qemu-devel] [PATCH v7 19/19] block: Kill unused sector-based blk_* functions Eric Blake
@ 2016-05-10 8:56 ` Kevin Wolf
2016-05-10 15:06 ` Kevin Wolf
20 siblings, 0 replies; 30+ messages in thread
From: Kevin Wolf @ 2016-05-10 8:56 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block
Am 06.05.2016 um 18:26 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 v6 [3].
> 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/msg00557.html
>
> Also available as a tag at this location:
> git fetch git://repo.or.cz/qemu/ericb.git nbd-block-v7
I commented on patch 6. The rest looks good to me.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read
2016-05-06 16:26 [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Eric Blake
` (19 preceding siblings ...)
2016-05-10 8:56 ` [Qemu-devel] [PATCH v7 00/19] block: kill sector-based blk_write/read Kevin Wolf
@ 2016-05-10 15:06 ` Kevin Wolf
20 siblings, 0 replies; 30+ messages in thread
From: Kevin Wolf @ 2016-05-10 15:06 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block
Am 06.05.2016 um 18:26 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 v6 [3].
> 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/msg00557.html
>
> Also available as a tag at this location:
> git fetch git://repo.or.cz/qemu/ericb.git nbd-block-v7
Thanks, fixed up scsi-disk and applied to block-next.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread