block: Make supported_write_flags a per-bds property
authorEric Blake <eblake@redhat.com>
Tue, 3 May 2016 22:39:06 +0000 (16:39 -0600)
committerKevin Wolf <kwolf@redhat.com>
Thu, 12 May 2016 13:22:09 +0000 (15:22 +0200)
Pre-patch, .supported_write_flags lives at the driver level, which
means we are blindly declaring that all block devices using a
given driver will either equally support FUA, or that we need a
fallback at the block layer.  But there are drivers where FUA
support is a per-block decision: the NBD block driver is dependent
on the remote server advertising NBD_FLAG_SEND_FUA (and has
fallback code to duplicate the flush that the block layer would do
if NBD had not set .supported_write_flags); and the iscsi block
driver is dependent on the mode sense bits advertised by the
underlying device (and is currently silently ignoring FUA requests
if the underlying device does not support FUA).

The fix is to make supported flags as a per-BDS option, set during
.bdrv_open().  This patch moves the variable and fixes NBD and iscsi
to set it only conditionally; later patches will then further
simplify the NBD driver to quit duplicating work done at the block
layer, as well as tackle the fact that SCSI does not support FUA
semantics on WRITESAME(10/16) but only on WRITE(10/16).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
block/io.c
block/iscsi.c
block/nbd-client.c
block/nbd.c
block/raw_bsd.c
include/block/block_int.h

index 0db1146159b5ff38ac79c72ddb6a60bcfd31ad7f..1fb7afeaf0c0f664f9be4b059a98d8bd8f49bbdf 100644 (file)
@@ -841,9 +841,10 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
 
     if (drv->bdrv_co_writev_flags) {
         ret = drv->bdrv_co_writev_flags(bs, sector_num, nb_sectors, qiov,
-                                        flags);
+                                        flags & bs->supported_write_flags);
+        flags &= ~bs->supported_write_flags;
     } else if (drv->bdrv_co_writev) {
-        assert(drv->supported_write_flags == 0);
+        assert(!bs->supported_write_flags);
         ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
     } else {
         BlockAIOCB *acb;
@@ -862,9 +863,7 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
     }
 
 emulate_flags:
-    if (ret == 0 && (flags & BDRV_REQ_FUA) &&
-        !(drv->supported_write_flags & BDRV_REQ_FUA))
-    {
+    if (ret == 0 && (flags & BDRV_REQ_FUA)) {
         ret = bdrv_co_flush(bs);
     }
 
index 4f7520463e45d7b520dad33c17cc7853bea0e6fd..6d5c1f6cebb6399fc2a833c3a07d7d6d29f7ae0f 100644 (file)
@@ -456,8 +456,11 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     struct IscsiTask iTask;
     uint64_t lba;
     uint32_t num_sectors;
-    bool fua;
+    bool fua = flags & BDRV_REQ_FUA;
 
+    if (fua) {
+        assert(iscsilun->dpofua);
+    }
     if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
         return -EINVAL;
     }
@@ -472,7 +475,6 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
     iscsi_co_init_iscsitask(iscsilun, &iTask);
 retry:
-    fua = iscsilun->dpofua && (flags & BDRV_REQ_FUA);
     if (iscsilun->use_16_for_rw) {
         iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba,
                                         NULL, num_sectors * iscsilun->block_size,
@@ -1548,6 +1550,9 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
     task = NULL;
 
     iscsi_modesense_sync(iscsilun);
+    if (iscsilun->dpofua) {
+        bs->supported_write_flags = BDRV_REQ_FUA;
+    }
 
     /* Check the write protect flag of the LUN if we want to write */
     if (iscsilun->type == TYPE_DISK && (flags & BDRV_O_RDWR) &&
@@ -1841,7 +1846,6 @@ static BlockDriver bdrv_iscsi = {
     .bdrv_co_write_zeroes = iscsi_co_write_zeroes,
     .bdrv_co_readv         = iscsi_co_readv,
     .bdrv_co_writev_flags  = iscsi_co_writev_flags,
-    .supported_write_flags = BDRV_REQ_FUA,
     .bdrv_co_flush_to_disk = iscsi_co_flush,
 
 #ifdef __linux__
index 878e879ace77f3d110c3f197b4ffda20ff788d41..5fc96e98565f5d25a0b06cc5b134b4fabba406d5 100644 (file)
@@ -414,6 +414,9 @@ int nbd_client_init(BlockDriverState *bs,
         logout("Failed to negotiate with the NBD server\n");
         return ret;
     }
+    if (client->nbdflags & NBD_FLAG_SEND_FUA) {
+        bs->supported_write_flags = BDRV_REQ_FUA;
+    }
 
     qemu_co_mutex_init(&client->send_mutex);
     qemu_co_mutex_init(&client->free_sema);
index fccbfef5851e61c112e554bddc9560bec1d9e134..a4fba911c36fcb99668d03257723ad3c4621b422 100644 (file)
@@ -471,7 +471,6 @@ static BlockDriver bdrv_nbd = {
     .bdrv_file_open             = nbd_open,
     .bdrv_co_readv              = nbd_co_readv,
     .bdrv_co_writev_flags       = nbd_co_writev_flags,
-    .supported_write_flags      = BDRV_REQ_FUA,
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
@@ -490,7 +489,6 @@ static BlockDriver bdrv_nbd_tcp = {
     .bdrv_file_open             = nbd_open,
     .bdrv_co_readv              = nbd_co_readv,
     .bdrv_co_writev_flags       = nbd_co_writev_flags,
-    .supported_write_flags      = BDRV_REQ_FUA,
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
@@ -509,7 +507,6 @@ static BlockDriver bdrv_nbd_unix = {
     .bdrv_file_open             = nbd_open,
     .bdrv_co_readv              = nbd_co_readv,
     .bdrv_co_writev_flags       = nbd_co_writev_flags,
-    .supported_write_flags      = BDRV_REQ_FUA,
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
index 5e65fb02d488e4b54000b99c60c6eceaf79e8700..1a1618ef073df8f38704fb74d4bac4e714153204 100644 (file)
@@ -204,6 +204,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
     bs->sg = bs->file->bs->sg;
+    bs->supported_write_flags = BDRV_REQ_FUA;
 
     if (bs->probed && !bdrv_is_read_only(bs)) {
         fprintf(stderr,
@@ -250,7 +251,6 @@ BlockDriver bdrv_raw = {
     .bdrv_create          = &raw_create,
     .bdrv_co_readv        = &raw_co_readv,
     .bdrv_co_writev_flags = &raw_co_writev_flags,
-    .supported_write_flags = BDRV_REQ_FUA,
     .bdrv_co_write_zeroes = &raw_co_write_zeroes,
     .bdrv_co_discard      = &raw_co_discard,
     .bdrv_co_get_block_status = &raw_co_get_block_status,
index 6fbe648231479f4d7d784309e8485debbca1a4ba..10f4962fe8b5ea021f9f3f438f502bcdb953c480 100644 (file)
@@ -158,8 +158,6 @@ struct BlockDriver {
     int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
 
-    int supported_write_flags;
-
     /*
      * Efficiently zero a region of the disk image.  Typically an image format
      * would use a compact metadata representation to implement this.  This
@@ -445,6 +443,8 @@ struct BlockDriverState {
 
     /* Alignment requirement for offset/length of I/O requests */
     unsigned int request_alignment;
+    /* Flags honored during pwrite (so far: BDRV_REQ_FUA) */
+    unsigned int supported_write_flags;
 
     /* the following member gives a name to every node on the bs graph. */
     char node_name[32];