diff mbox

[2/2] raw_bsd: Convert to byte-based interface

Message ID 1466740739-7240-3-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake June 24, 2016, 3:58 a.m. UTC 
Since the raw format driver is just passing things through, we can
do byte-based read and write if the underlying protocol does
likewise.

There's one tricky part - if we probed the image format, we document
that we restrict operations on the initial sector.  Rather than
trying to handle a read-modify-write on the first sector, it's
easiest to just include in our restrictions that partial writes to
the first sector are not permitted.

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

Comments

Paolo Bonzini June 27, 2016, 12:19 p.m. UTC | #1 
On 24/06/2016 05:58, Eric Blake wrote:
> Since the raw format driver is just passing things through, we can
> do byte-based read and write if the underlying protocol does
> likewise.
> 
> There's one tricky part - if we probed the image format, we document
> that we restrict operations on the initial sector.  Rather than
> trying to handle a read-modify-write on the first sector, it's
> easiest to just include in our restrictions that partial writes to
> the first sector are not permitted.

Can we just set the alignment to at least 512 if probed?  It's the
practical guest alignment anyway.

Paolo
Eric Blake June 27, 2016, 12:51 p.m. UTC | #2 
On 06/27/2016 06:19 AM, Paolo Bonzini wrote:
> 
> 
> On 24/06/2016 05:58, Eric Blake wrote:
>> Since the raw format driver is just passing things through, we can
>> do byte-based read and write if the underlying protocol does
>> likewise.
>>
>> There's one tricky part - if we probed the image format, we document
>> that we restrict operations on the initial sector.  Rather than
>> trying to handle a read-modify-write on the first sector, it's
>> easiest to just include in our restrictions that partial writes to
>> the first sector are not permitted.
> 
> Can we just set the alignment to at least 512 if probed?  It's the
> practical guest alignment anyway.

Interesting idea; should be fairly easy to attempt.
diff mbox

Patch

diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 365c38a..a417d9a 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -50,32 +50,32 @@  static int raw_reopen_prepare(BDRVReopenState *reopen_state,
     return 0;
 }

-static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
-                                     int nb_sectors, QEMUIOVector *qiov)
+static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
+                                      uint64_t bytes, QEMUIOVector *qiov,
+                                      int flags)
 {
     BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-    return bdrv_co_readv(bs->file->bs, sector_num, nb_sectors, qiov);
+    return bdrv_co_preadv(bs->file->bs, offset, bytes, qiov, flags);
 }

-static int coroutine_fn
-raw_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
-                    QEMUIOVector *qiov, int flags)
+static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
+                                       uint64_t bytes, QEMUIOVector *qiov,
+                                       int flags)
 {
     void *buf = NULL;
     BlockDriver *drv;
     QEMUIOVector local_qiov;
     int ret;

-    if (bs->probed && sector_num == 0) {
-        /* As long as these conditions are true, we can't get partial writes to
-         * the probe buffer and can just directly check the request. */
+    if (bs->probed && offset < BLOCK_PROBE_BUF_SIZE && bytes) {
+        /* Handling partial writes would be a pain - so we just
+         * require that the guest cannot write to the first sector
+         * without writing the entire sector */
         QEMU_BUILD_BUG_ON(BLOCK_PROBE_BUF_SIZE != 512);
         QEMU_BUILD_BUG_ON(BDRV_SECTOR_SIZE != 512);
-
-        if (nb_sectors == 0) {
-            /* qemu_iovec_to_buf() would fail, but we want to return success
-             * instead of -EINVAL in this case. */
-            return 0;
+        if (offset != 0 || bytes < BLOCK_PROBE_BUF_SIZE) {
+            ret = -EINVAL;
+            goto fail;
         }

         buf = qemu_try_blockalign(bs->file->bs, 512);
@@ -105,8 +105,7 @@  raw_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     }

     BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
-    ret = bdrv_co_pwritev(bs->file->bs, sector_num * BDRV_SECTOR_SIZE,
-                          nb_sectors * BDRV_SECTOR_SIZE, qiov, flags);
+    ret = bdrv_co_pwritev(bs->file->bs, offset, bytes, qiov, flags);

 fail:
     if (qiov == &local_qiov) {
@@ -240,8 +239,8 @@  BlockDriver bdrv_raw = {
     .bdrv_open            = &raw_open,
     .bdrv_close           = &raw_close,
     .bdrv_create          = &raw_create,
-    .bdrv_co_readv        = &raw_co_readv,
-    .bdrv_co_writev_flags = &raw_co_writev_flags,
+    .bdrv_co_preadv       = &raw_co_preadv,
+    .bdrv_co_pwritev      = &raw_co_pwritev,
     .bdrv_co_pwrite_zeroes = &raw_co_pwrite_zeroes,
     .bdrv_co_pdiscard     = &raw_co_pdiscard,
     .bdrv_co_get_block_status = &raw_co_get_block_status,