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

…taging

Block layer patches:

- qemu-img: fix info --backing-chain --image-opts
- Error out on image creation with conflicting size options
- Fix external snapshot with VM state
- hmp: Allow using qdev ID for qemu-io command
- Misc code cleanup
- Many iotests improvements

# gpg: Signature made Thu 19 Dec 2019 17:23:11 GMT
# gpg:                using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* remotes/kevin/tags/for-upstream: (30 commits)
  iotests: Test external snapshot with VM state
  hmp: Allow using qdev ID for qemu-io command
  block: Activate recursively even for already active nodes
  iotests: 211: Remove duplication with VM.blockdev_create()
  iotests: 207: Remove duplication with VM.blockdev_create()
  iotests: 266: Convert to VM.blockdev_create()
  iotests: 237: Convert to VM.blockdev_create()
  iotests: 213: Convert to VM.blockdev_create()
  iotests: 212: Convert to VM.blockdev_create()
  iotests: 210: Convert to VM.blockdev_create()
  iotests: 206: Convert to VM.blockdev_create()
  iotests: 255: Drop blockdev_create()
  iotests: Create VM.blockdev_create()
  qcow2: Move error check of local_err near its assignment
  iotests: Fix IMGOPTSSYNTAX for nbd
  iotests/273: Filter format-specific information
  iotests: Add more "_require_drivers" checks to the shell-based tests
  MAINTAINERS: fix qcow2-bitmap.c under Dirty Bitmaps header
  qcow2: Use offset_into_cluster()
  iotests: Support job-complete in run_job()
  ...

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

MAINTAINERS

@@ -1872,12 +1872,12 @@ M: John Snow <jsnow@redhat.com>
 R: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 L: qemu-block@nongnu.org
 S: Supported
-F: util/hbitmap.c
-F: block/dirty-bitmap.c
 F: include/qemu/hbitmap.h
 F: include/block/dirty-bitmap.h
-F: qcow2-bitmap.c
+F: block/dirty-bitmap.c
+F: block/qcow2-bitmap.c
 F: migration/block-dirty-bitmap.c
+F: util/hbitmap.c
 F: tests/test-hbitmap.c
 F: docs/interop/bitmaps.rst
 T: git https://github.com/jnsnow/qemu.git bitmaps

block.c

@@ -5335,10 +5335,6 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs,
         return;
     }
 
-    if (!(bs->open_flags & BDRV_O_INACTIVE)) {
-        return;
-    }
-
     QLIST_FOREACH(child, &bs->children, next) {
         bdrv_co_invalidate_cache(child->bs, &local_err);
         if (local_err) {
@@ -5360,34 +5356,36 @@ static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs,
      * just keep the extended permissions for the next time that an activation
      * of the image is tried.
      */
-    bs->open_flags &= ~BDRV_O_INACTIVE;
-    bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
-    ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, &local_err);
-    if (ret < 0) {
-        bs->open_flags |= BDRV_O_INACTIVE;
-        error_propagate(errp, local_err);
-        return;
-    }
-    bdrv_set_perm(bs, perm, shared_perm);
-
-    if (bs->drv->bdrv_co_invalidate_cache) {
-        bs->drv->bdrv_co_invalidate_cache(bs, &local_err);
-        if (local_err) {
+    if (bs->open_flags & BDRV_O_INACTIVE) {
+        bs->open_flags &= ~BDRV_O_INACTIVE;
+        bdrv_get_cumulative_perm(bs, &perm, &shared_perm);
+        ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, &local_err);
+        if (ret < 0) {
             bs->open_flags |= BDRV_O_INACTIVE;
             error_propagate(errp, local_err);
             return;
         }
-    }
+        bdrv_set_perm(bs, perm, shared_perm);
 
-    FOR_EACH_DIRTY_BITMAP(bs, bm) {
-        bdrv_dirty_bitmap_skip_store(bm, false);
-    }
+        if (bs->drv->bdrv_co_invalidate_cache) {
+            bs->drv->bdrv_co_invalidate_cache(bs, &local_err);
+            if (local_err) {
+                bs->open_flags |= BDRV_O_INACTIVE;
+                error_propagate(errp, local_err);
+                return;
+            }
+        }
 
-    ret = refresh_total_sectors(bs, bs->total_sectors);
-    if (ret < 0) {
-        bs->open_flags |= BDRV_O_INACTIVE;
-        error_setg_errno(errp, -ret, "Could not refresh total sector count");
-        return;
+        FOR_EACH_DIRTY_BITMAP(bs, bm) {
+            bdrv_dirty_bitmap_skip_store(bm, false);
+        }
+
+        ret = refresh_total_sectors(bs, bs->total_sectors);
+        if (ret < 0) {
+            bs->open_flags |= BDRV_O_INACTIVE;
+            error_setg_errno(errp, -ret, "Could not refresh total sector count");
+            return;
+        }
     }
 
     QLIST_FOREACH(parent, &bs->parents, next_parent) {
@@ -5751,12 +5749,11 @@ void bdrv_img_create(const char *filename, const char *fmt,
         return;
     }
 
+    /* Create parameter list */
     create_opts = qemu_opts_append(create_opts, drv->create_opts);
     create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
 
-    /* Create parameter list with default values */
     opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
-    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort);
 
     /* Parse -o options */
     if (options) {
@@ -5766,6 +5763,13 @@ void bdrv_img_create(const char *filename, const char *fmt,
         }
     }
 
+    if (!qemu_opt_get(opts, BLOCK_OPT_SIZE)) {
+        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort);
+    } else if (img_size != UINT64_C(-1)) {
+        error_setg(errp, "The image size must be specified only once");
+        goto out;
+    }
+
     if (base_filename) {
         qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename, &local_err);
         if (local_err) {

block/qcow2.c

@@ -367,7 +367,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
                 return -EINVAL;
             }
 
-            if (bitmaps_ext.bitmap_directory_offset & (s->cluster_size - 1)) {
+            if (offset_into_cluster(s, bitmaps_ext.bitmap_directory_offset)) {
                 error_setg(errp, "bitmaps_ext: "
                                  "invalid bitmap directory offset");
                 return -EINVAL;
@@ -1705,14 +1705,14 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
     if (!(bdrv_get_flags(bs) & BDRV_O_INACTIVE)) {
         /* It's case 1, 2 or 3.2. Or 3.1 which is BUG in management layer. */
         bool header_updated = qcow2_load_dirty_bitmaps(bs, &local_err);
+        if (local_err != NULL) {
+            error_propagate(errp, local_err);
+            ret = -EINVAL;
+            goto fail;
+        }
 
         update_header = update_header && !header_updated;
     }
-    if (local_err != NULL) {
-        error_propagate(errp, local_err);
-        ret = -EINVAL;
-        goto fail;
-    }
 
     if (update_header) {
         ret = qcow2_update_header(bs);
@@ -1722,7 +1722,8 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
         }
     }
 
-    bs->supported_zero_flags = header.version >= 3 ? BDRV_REQ_MAY_UNMAP : 0;
+    bs->supported_zero_flags = header.version >= 3 ?
+                               BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK : 0;
 
     /* Repair image if dirty */
     if (!(flags & (BDRV_O_CHECK | BDRV_O_INACTIVE)) && !bs->read_only &&
@@ -1958,9 +1959,8 @@ static int coroutine_fn qcow2_co_block_status(BlockDriverState *bs,
 {
     BDRVQcow2State *s = bs->opaque;
     uint64_t cluster_offset;
-    int index_in_cluster, ret;
     unsigned int bytes;
-    int status = 0;
+    int ret, status = 0;
 
     qemu_co_mutex_lock(&s->lock);
 
@@ -1981,8 +1981,7 @@ static int coroutine_fn qcow2_co_block_status(BlockDriverState *bs,
 
     if ((ret == QCOW2_CLUSTER_NORMAL || ret == QCOW2_CLUSTER_ZERO_ALLOC) &&
         !s->crypto) {
-        index_in_cluster = offset & (s->cluster_size - 1);
-        *map = cluster_offset | index_in_cluster;
+        *map = cluster_offset | offset_into_cluster(s, offset);
         *file = s->data_file->bs;
         status |= BDRV_BLOCK_OFFSET_VALID;
     }

blockjob.c

@@ -261,7 +261,8 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
         return;
     }
     if (speed < 0) {
-        error_setg(errp, QERR_INVALID_PARAMETER, "speed");
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "speed",
+                   "a non-negative value");
         return;
     }
 

hmp-commands.hx

@@ -1875,9 +1875,11 @@ ETEXI
 
     {
         .name       = "qemu-io",
-        .args_type  = "device:B,command:s",
-        .params     = "[device] \"[command]\"",
-        .help       = "run a qemu-io command on a block device",
+        .args_type  = "qdev:-d,device:B,command:s",
+        .params     = "[-d] [device] \"[command]\"",
+        .help       = "run a qemu-io command on a block device\n\t\t\t"
+                      "-d: [device] is a device ID rather than a "
+                      "drive ID or node name",
         .cmd        = hmp_qemu_io,
     },
 

monitor/hmp-cmds.c

@@ -2467,23 +2467,31 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
 {
     BlockBackend *blk;
     BlockBackend *local_blk = NULL;
+    bool qdev = qdict_get_try_bool(qdict, "qdev", false);
     const char* device = qdict_get_str(qdict, "device");
     const char* command = qdict_get_str(qdict, "command");
     Error *err = NULL;
     int ret;
 
-    blk = blk_by_name(device);
-    if (!blk) {
-        BlockDriverState *bs = bdrv_lookup_bs(NULL, device, &err);
-        if (bs) {
-            blk = local_blk = blk_new(bdrv_get_aio_context(bs),
-                                      0, BLK_PERM_ALL);
-            ret = blk_insert_bs(blk, bs, &err);
-            if (ret < 0) {
+    if (qdev) {
+        blk = blk_by_qdev_id(device, &err);
+        if (!blk) {
+            goto fail;
+        }
+    } else {
+        blk = blk_by_name(device);
+        if (!blk) {
+            BlockDriverState *bs = bdrv_lookup_bs(NULL, device, &err);
+            if (bs) {
+                blk = local_blk = blk_new(bdrv_get_aio_context(bs),
+                                          0, BLK_PERM_ALL);
+                ret = blk_insert_bs(blk, bs, &err);
+                if (ret < 0) {
+                    goto fail;
+                }
+            } else {
                 goto fail;
             }
-        } else {
-            goto fail;
         }
     }
 

qapi/block-core.json

@@ -2963,9 +2963,13 @@
 #
 # Driver specific block device options for the NVMe backend.
 #
-# @device:    controller address of the NVMe device.
+# @device:    PCI controller address of the NVMe device in
+#             format hhhh:bb:ss.f (host:bus:slot.function)
 # @namespace: namespace number of the device, starting from 1.
 #
+# Note that the PCI @device must have been unbound from any host
+# kernel driver before instructing QEMU to add the blockdev.
+#
 # Since: 2.12
 ##
 { 'struct': 'BlockdevOptionsNVMe',

qemu-img.c

@@ -2680,7 +2680,10 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
 
         blk_unref(blk);
 
+        /* Clear parameters that only apply to the topmost image */
         filename = fmt = NULL;
+        image_opts = false;
+
         if (chain) {
             if (info->has_full_backing_filename) {
                 filename = info->full_backing_filename;

tests/qemu-iotests/030

@@ -943,7 +943,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('block-stream', device='drive0', speed=-1)
-        self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'")
+        self.assert_qmp(result, 'error/desc', "Parameter 'speed' expects a non-negative value")
 
         self.assert_no_active_block_jobs()
 
@@ -952,7 +952,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
-        self.assert_qmp(result, 'error/desc', "Invalid parameter 'speed'")
+        self.assert_qmp(result, 'error/desc', "Parameter 'speed' expects a non-negative value")
 
         self.cancel_and_wait(resume=True)
 

tests/qemu-iotests/049

@@ -78,6 +78,11 @@ for s in $sizes; do
     test_qemu_img create -f $IMGFMT -o size=$s "$TEST_IMG"
 done
 
+echo "== 4. Specify size twice (-o and traditional parameter) =="
+echo
+
+test_qemu_img create -f $IMGFMT -o size=10M "$TEST_IMG" 20M
+
 echo "== Check correct interpretation of suffixes for cluster size =="
 echo
 sizes="1024 1024b 1k 1K 1M "

tests/qemu-iotests/049.out

@@ -121,6 +121,11 @@ qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number below
 Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta-
 and exabytes, respectively.
 
+== 4. Specify size twice (-o and traditional parameter) ==
+
+qemu-img create -f qcow2 -o size=10M TEST_DIR/t.qcow2 20M
+qemu-img: TEST_DIR/t.qcow2: The image size must be specified only once
+
 == Check correct interpretation of suffixes for cluster size ==
 
 qemu-img create -f qcow2 -o cluster_size=1024 TEST_DIR/t.qcow2 64M

tests/qemu-iotests/051

@@ -41,6 +41,7 @@ _supported_proto file
 # A compat=0.10 image is created in this test which does not support anything
 # other than refcount_bits=16
 _unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
+_require_drivers nbd
 
 do_run_qemu()
 {