Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging

Block layer patches:

- file-posix: Check correct file type (regular file for 'file',
  character or block device for 'host_device'/'host_cdrom')
- scsi-disk: Block Device Characteristics emulation fix
- qemu-img: Consider required alignment for sparse area detection
- Documentation and test improvements

# gpg: Signature made Thu 12 Jul 2018 17:29:17 BST
# gpg:                using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* remotes/kevin/tags/for-upstream:
  qemu-img: align result of is_allocated_sectors
  scsi-disk: Block Device Characteristics emulation fix
  iotests: add test 226 for file driver types
  file-posix: specify expected filetypes
  qemu-img: Document copy offloading implications with -S and -c
  iotests: nbd: Stop qemu-nbd before remaking image
  iotests: 153: Fix dead code

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

12 files changed, 226 insertions, 35 deletions

diff --git a/block/file-posix.c b/block/file-posix.c
index 28824aae65..60af4b3d51 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -438,7 +438,8 @@ static QemuOptsList raw_runtime_opts = {
 };
 
 static int raw_open_common(BlockDriverState *bs, QDict *options,
-                           int bdrv_flags, int open_flags, Error **errp)
+                           int bdrv_flags, int open_flags,
+                           bool device, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     QemuOpts *opts;
@@ -585,10 +586,32 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
         error_setg_errno(errp, errno, "Could not stat file");
         goto fail;
     }
-    if (S_ISREG(st.st_mode)) {
-        s->discard_zeroes = true;
-        s->has_fallocate = true;
+
+    if (!device) {
+        if (S_ISBLK(st.st_mode)) {
+            warn_report("Opening a block device as a file using the '%s' "
+                        "driver is deprecated", bs->drv->format_name);
+        } else if (S_ISCHR(st.st_mode)) {
+            warn_report("Opening a character device as a file using the '%s' "
+                        "driver is deprecated", bs->drv->format_name);
+        } else if (!S_ISREG(st.st_mode)) {
+            error_setg(errp, "A regular file was expected by the '%s' driver, "
+                       "but something else was given", bs->drv->format_name);
+            ret = -EINVAL;
+            goto fail;
+        } else {
+            s->discard_zeroes = true;
+            s->has_fallocate = true;
+        }
+    } else {
+        if (!(S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
+            error_setg(errp, "'%s' driver expects either "
+                       "a character or block device", bs->drv->format_name);
+            ret = -EINVAL;
+            goto fail;
+        }
     }
+
     if (S_ISBLK(st.st_mode)) {
 #ifdef BLKDISCARDZEROES
         unsigned int arg;
@@ -641,7 +664,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
     BDRVRawState *s = bs->opaque;
 
     s->type = FTYPE_FILE;
-    return raw_open_common(bs, options, flags, 0, errp);
+    return raw_open_common(bs, options, flags, 0, false, errp);
 }
 
 typedef enum {
@@ -2939,7 +2962,7 @@ hdev_open_Mac_error:
 
     s->type = FTYPE_FILE;
 
-    ret = raw_open_common(bs, options, flags, 0, &local_err);
+    ret = raw_open_common(bs, options, flags, 0, true, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
 #if defined(__APPLE__) && defined(__MACH__)
@@ -3170,7 +3193,7 @@ static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
     s->type = FTYPE_CD;
 
     /* open will not fail even if no CD is inserted, so add O_NONBLOCK */
-    return raw_open_common(bs, options, flags, O_NONBLOCK, errp);
+    return raw_open_common(bs, options, flags, O_NONBLOCK, true, errp);
 }
 
 static int cdrom_probe_device(const char *filename)
@@ -3284,7 +3307,7 @@ static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
 
     s->type = FTYPE_CD;
 
-    ret = raw_open_common(bs, options, flags, 0, &local_err);
+    ret = raw_open_common(bs, options, flags, 0, true, &local_err);
     if (ret) {
         error_propagate(errp, local_err);
         return ret;
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d7df357029..5bb390773b 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -775,11 +775,12 @@ int scsi_disk_emulate_vpd_page(SCSIRequest *req, uint8_t *outbuf)
     }
     case 0xb1: /* block device characteristics */
     {
-        buflen = 8;
+        buflen = 0x40;
         outbuf[4] = (s->rotation_rate >> 8) & 0xff;
         outbuf[5] = s->rotation_rate & 0xff;
-        outbuf[6] = 0;
-        outbuf[7] = 0;
+        outbuf[6] = 0; /* PRODUCT TYPE */
+        outbuf[7] = 0; /* WABEREQ | WACEREQ | NOMINAL FORM FACTOR */
+        outbuf[8] = 0; /* VBULS */
         break;
     }
     case 0xb2: /* thin provisioning */
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 0fbf8b108b..1047c407e7 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2969,6 +2969,12 @@ replacement since it is not needed anymore.
 The @option{-enable-hax} option has been replaced by @option{-accel hax}.
 Both options have been introduced in QEMU version 2.9.0.
 
+@subsection -drive file=json:@{...@{'driver':'file'@}@} (since 3.0)
+
+The 'file' driver for drives is no longer appropriate for character or host
+devices and will only accept regular files (S_IFREG). The correct driver
+for these file types is 'host_cdrom' or 'host_device' as appropriate.
+
 @section QEMU Machine Protocol (QMP) commands
 
 @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
diff --git a/qemu-img.c b/qemu-img.c
index f4074ebf75..4a7ce43dc9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1105,11 +1105,15 @@ static int64_t find_nonzero(const uint8_t *buf, int64_t n)
  *
  * 'pnum' is set to the number of sectors (including and immediately following
  * the first one) that are known to be in the same allocated/unallocated state.
+ * The function will try to align the end offset to alignment boundaries so
+ * that the request will at least end aligned and consequtive requests will
+ * also start at an aligned offset.
  */
-static int is_allocated_sectors(const uint8_t *buf, int n, int *pnum)
+static int is_allocated_sectors(const uint8_t *buf, int n, int *pnum,
+                                int64_t sector_num, int alignment)
 {
     bool is_zero;
-    int i;
+    int i, tail;
 
     if (n <= 0) {
         *pnum = 0;
@@ -1122,6 +1126,23 @@ static int is_allocated_sectors(const uint8_t *buf, int n, int *pnum)
             break;
         }
     }
+
+    tail = (sector_num + i) & (alignment - 1);
+    if (tail) {
+        if (is_zero && i <= tail) {
+            /* treat unallocated areas which only consist
+             * of a small tail as allocated. */
+            is_zero = false;
+        }
+        if (!is_zero) {
+            /* align up end offset of allocated areas. */
+            i += alignment - tail;
+            i = MIN(i, n);
+        } else {
+            /* align down end offset of zero areas. */
+            i -= tail;
+        }
+    }
     *pnum = i;
     return !is_zero;
 }
@@ -1132,7 +1153,7 @@ static int is_allocated_sectors(const uint8_t *buf, int n, int *pnum)
  * breaking up write requests for only small sparse areas.
  */
 static int is_allocated_sectors_min(const uint8_t *buf, int n, int *pnum,
-    int min)
+    int min, int64_t sector_num, int alignment)
 {
     int ret;
     int num_checked, num_used;
@@ -1141,7 +1162,7 @@ static int is_allocated_sectors_min(const uint8_t *buf, int n, int *pnum,
         min = n;
     }
 
-    ret = is_allocated_sectors(buf, n, pnum);
+    ret = is_allocated_sectors(buf, n, pnum, sector_num, alignment);
     if (!ret) {
         return ret;
     }
@@ -1149,13 +1170,15 @@ static int is_allocated_sectors_min(const uint8_t *buf, int n, int *pnum,
     num_used = *pnum;
     buf += BDRV_SECTOR_SIZE * *pnum;
     n -= *pnum;
+    sector_num += *pnum;
     num_checked = num_used;
 
     while (n > 0) {
-        ret = is_allocated_sectors(buf, n, pnum);
+        ret = is_allocated_sectors(buf, n, pnum, sector_num, alignment);
 
         buf += BDRV_SECTOR_SIZE * *pnum;
         n -= *pnum;
+        sector_num += *pnum;
         num_checked += *pnum;
         if (ret) {
             num_used = num_checked;
@@ -1560,6 +1583,7 @@ typedef struct ImgConvertState {
     bool wr_in_order;
     bool copy_range;
     int min_sparse;
+    int alignment;
     size_t cluster_sectors;
     size_t buf_sectors;
     long num_coroutines;
@@ -1724,7 +1748,8 @@ static int coroutine_fn convert_co_write(ImgConvertState *s, int64_t sector_num,
              * zeroed. */
             if (!s->min_sparse ||
                 (!s->compressed &&
-                 is_allocated_sectors_min(buf, n, &n, s->min_sparse)) ||
+                 is_allocated_sectors_min(buf, n, &n, s->min_sparse,
+                                          sector_num, s->alignment)) ||
                 (s->compressed &&
                  !buffer_is_zero(buf, n * BDRV_SECTOR_SIZE)))
             {
@@ -2368,6 +2393,13 @@ static int img_convert(int argc, char **argv)
                                 out_bs->bl.pdiscard_alignment >>
                                 BDRV_SECTOR_BITS)));
 
+    /* try to align the write requests to the destination to avoid unnecessary
+     * RMW cycles. */
+    s.alignment = MAX(pow2floor(s.min_sparse),
+                      DIV_ROUND_UP(out_bs->bl.request_alignment,
+                                   BDRV_SECTOR_SIZE));
+    assert(is_power_of_2(s.alignment));
+
     if (skip_create) {
         int64_t output_sectors = blk_nb_sectors(s.target);
         if (output_sectors < 0) {
diff --git a/qemu-img.texi b/qemu-img.texi
index aeb1b9e66c..5853cd18d1 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -96,7 +96,8 @@ will enumerate information about backing files in a disk image chain. Refer
 below for further description.
 
 @item -c
-indicates that target image must be compressed (qcow format only)
+indicates that target image must be compressed (qcow format only). If this
+option is used, copy offloading will not be attempted.
 
 @item -h
 with or without a command shows help and lists the supported formats
@@ -115,7 +116,8 @@ in case both @var{-q} and @var{-p} options are used.
 indicates the consecutive number of bytes that must contain only zeros
 for qemu-img to create a sparse image during conversion. This value is rounded
 down to the nearest 512 bytes. You may use the common size suffixes like
-@code{k} for kilobytes.
+@code{k} for kilobytes. If this option is used, copy offloading will not be
+attempted.
 
 @item -t @var{cache}
 specifies the cache mode that should be used with the (destination) file. See
diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
index 6c7ee1da6c..c576705284 100644
--- a/tests/qemu-iotests/122.out
+++ b/tests/qemu-iotests/122.out
@@ -194,12 +194,12 @@ wrote 1024/1024 bytes at offset 17408
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 convert -S 4k
-[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-{ "start": 1024, "length": 7168, "depth": 0, "zero": true, "data": false},
-{ "start": 8192, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-{ "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false},
-{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-{ "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 4096, "depth": 0, "zero": true, "data": false},
+{ "start": 8192, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 12288, "length": 4096, "depth": 0, "zero": true, "data": false},
+{ "start": 16384, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 20480, "length": 67088384, "depth": 0, "zero": true, "data": false}]
 
 convert -c -S 4k
 [{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true},
@@ -210,10 +210,8 @@ convert -c -S 4k
 { "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]
 
 convert -S 8k
-[{ "start": 0, "length": 9216, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-{ "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false},
-{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-{ "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]
+[{ "start": 0, "length": 24576, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 24576, "length": 67084288, "depth": 0, "zero": true, "data": false}]
 
 convert -c -S 8k
 [{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true},
diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
index 673813cd39..0daeb1b005 100755
--- a/tests/qemu-iotests/153
+++ b/tests/qemu-iotests/153
@@ -162,6 +162,7 @@ for opts1 in "" "read-only=on" "read-only=on,force-share=on"; do
     _cleanup_qemu
 done
 
+test_opts="read-only=off read-only=on read-only=on,force-share=on"
 for opt1 in $test_opts; do
     for opt2 in $test_opts; do
         echo
@@ -170,6 +171,7 @@ for opt1 in $test_opts; do
     done
 done
 
+echo
 echo "== Creating ${TEST_IMG}.[abc] ==" | _filter_testdir
 (
     $QEMU_IMG create -f qcow2 "${TEST_IMG}.a" -b "${TEST_IMG}"
diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out
index 3492ba7a29..93eaf10486 100644
--- a/tests/qemu-iotests/153.out
+++ b/tests/qemu-iotests/153.out
@@ -369,6 +369,31 @@ _qemu_img_wrapper bench -U -w -c 1 TEST_DIR/t.qcow2
 qemu-img: Could not open 'TEST_DIR/t.qcow2': force-share=on can only be used with read-only images
 
 Round done
+
+== Two devices with the same image (read-only=off - read-only=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,read-only=off: Failed to get "write" lock
+Is another process using the image?
+
+== Two devices with the same image (read-only=off - read-only=on) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,read-only=on: Failed to get shared "write" lock
+Is another process using the image?
+
+== Two devices with the same image (read-only=off - read-only=on,force-share=on) ==
+
+== Two devices with the same image (read-only=on - read-only=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,read-only=off: Failed to get "write" lock
+Is another process using the image?
+
+== Two devices with the same image (read-only=on - read-only=on) ==
+
+== Two devices with the same image (read-only=on - read-only=on,force-share=on) ==
+
+== Two devices with the same image (read-only=on,force-share=on - read-only=off) ==
+
+== Two devices with the same image (read-only=on,force-share=on - read-only=on) ==
+
+== Two devices with the same image (read-only=on,force-share=on - read-only=on,force-share=on) ==
+
 == Creating TEST_DIR/t.qcow2.[abc] ==
 Formatting 'TEST_DIR/t.IMGFMT.a', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT
 Formatting 'TEST_DIR/t.IMGFMT.b', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT
diff --git a/tests/qemu-iotests/226 b/tests/qemu-iotests/226
new file mode 100755
index 0000000000..460aea2fc9
--- /dev/null
+++ b/tests/qemu-iotests/226
@@ -0,0 +1,66 @@
+#!/bin/bash
+#
+# This test covers expected filetypes for the file, host_cdrom and
+# host_device drivers.
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=jsnow@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1    # failure is the default!
+
+_cleanup()
+{
+    rmdir "$TEST_IMG"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# Generic format, but tests file-protocol specific error handling
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+# Create something decidedly not a file, blockdev or chardev...
+mkdir "$TEST_IMG"
+
+for PROTO in "file" "host_device" "host_cdrom"; do
+    echo
+    echo "=== Testing with driver:$PROTO ==="
+    echo
+    echo "== Testing RO =="
+    $QEMU_IO -c "open -r -o driver=$PROTO,filename=$TEST_IMG" 2>&1 | _filter_imgfmt | _filter_testdir
+    $QEMU_IO -c "open -r -o driver=$PROTO,filename=/dev/null" 2>&1 | _filter_imgfmt
+    echo "== Testing RW =="
+    $QEMU_IO -c "open -o driver=$PROTO,filename=$TEST_IMG" 2>&1 | _filter_imgfmt | _filter_testdir
+    $QEMU_IO -c "open -o driver=$PROTO,filename=/dev/null" 2>&1 | _filter_imgfmt
+done
+
+# success, all done
+echo
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/226.out b/tests/qemu-iotests/226.out
new file mode 100644
index 0000000000..8c0d060ffc
--- /dev/null
+++ b/tests/qemu-iotests/226.out
@@ -0,0 +1,26 @@
+QA output created by 226
+
+=== Testing with driver:file ===
+
+== Testing RO ==
+can't open: A regular file was expected by the 'file' driver, but something else was given
+warning: Opening a character device as a file using the 'file' driver is deprecated
+== Testing RW ==
+can't open: Could not open 'TEST_DIR/t.IMGFMT': Is a directory
+warning: Opening a character device as a file using the 'file' driver is deprecated
+
+=== Testing with driver:host_device ===
+
+== Testing RO ==
+can't open: 'host_device' driver expects either a character or block device
+== Testing RW ==
+can't open: Could not open 'TEST_DIR/t.IMGFMT': Is a directory
+
+=== Testing with driver:host_cdrom ===
+
+== Testing RO ==
+can't open: 'host_cdrom' driver expects either a character or block device
+== Testing RW ==
+can't open: Could not open 'TEST_DIR/t.IMGFMT': Is a directory
+
+*** done
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index d054cb97fc..44bee16a5e 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -195,6 +195,16 @@ _use_sample_img()
     fi
 }
 
+_stop_nbd_server()
+{
+    if [ -f "${QEMU_TEST_DIR}/qemu-nbd.pid" ]; then
+        local QEMU_NBD_PID
+        read QEMU_NBD_PID < "${QEMU_TEST_DIR}/qemu-nbd.pid"
+        kill ${QEMU_NBD_PID}
+        rm -f "${QEMU_TEST_DIR}/qemu-nbd.pid"
+    fi
+}
+
 _make_test_img()
 {
     # extra qemu-img options can be added by tests
@@ -234,6 +244,10 @@ _make_test_img()
         extra_img_options="-o $optstr $extra_img_options"
     fi
 
+    if [ $IMGPROTO = "nbd" ]; then
+        _stop_nbd_server
+    fi
+
     # XXX(hch): have global image options?
     (
      if [ $use_backing = 1 ]; then
@@ -274,12 +288,7 @@ _cleanup_test_img()
     case "$IMGPROTO" in
 
         nbd)
-            if [ -f "${QEMU_TEST_DIR}/qemu-nbd.pid" ]; then
-                local QEMU_NBD_PID
-                read QEMU_NBD_PID < "${QEMU_TEST_DIR}/qemu-nbd.pid"
-                kill ${QEMU_NBD_PID}
-                rm -f "${QEMU_TEST_DIR}/qemu-nbd.pid"
-            fi
+            _stop_nbd_server
             rm -f "$TEST_IMG_FILE"
             ;;
         vxhs)
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1c9f679821..68f66259b2 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -223,3 +223,4 @@
 222 rw auto quick
 223 rw auto quick
 225 rw auto quick
+226 auto quick