[Qemu-devel] [PATCH] block: Catch simultaneous usage of options and their aliases

[Kevin Wolf <kwolf@redhat.com>]

While thinking about precedence of conflicting block device options from
different sources, I noticed that you can specify both an option and its
legacy alias at the same time (e.g. readonly=on,read-only=off). Rather
than specifying the order of precedence, we should simply forbid such
combinations.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c                 | 46 +++++++++++++++++++++++++++++++---------------
 tests/qemu-iotests/051     | 23 +++++++++++++++++++++++
 tests/qemu-iotests/051.out | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 15 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index a194d04..dddd7a9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -532,12 +532,22 @@ err_no_opts:
     return NULL;
 }
 
-static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to)
+static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to,
+                            Error **errp)
 {
     const char *value;
 
+    if (*errp) {
+        return;
+    }
+
     value = qemu_opt_get(opts, from);
     if (value) {
+        if (qemu_opt_find(opts, to)) {
+            error_setg(errp, "'%s' and its alias '%s' can't be used at the "
+                       "same time", to, from);
+            return;
+        }
         qemu_opt_set(opts, to, value);
         qemu_opt_unset(opts, from);
     }
@@ -643,26 +653,32 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     Error *local_err = NULL;
 
     /* Change legacy command line options into QMP ones */
-    qemu_opt_rename(all_opts, "iops", "throttling.iops-total");
-    qemu_opt_rename(all_opts, "iops_rd", "throttling.iops-read");
-    qemu_opt_rename(all_opts, "iops_wr", "throttling.iops-write");
+    qemu_opt_rename(all_opts, "iops", "throttling.iops-total", &local_err);
+    qemu_opt_rename(all_opts, "iops_rd", "throttling.iops-read", &local_err);
+    qemu_opt_rename(all_opts, "iops_wr", "throttling.iops-write", &local_err);
 
-    qemu_opt_rename(all_opts, "bps", "throttling.bps-total");
-    qemu_opt_rename(all_opts, "bps_rd", "throttling.bps-read");
-    qemu_opt_rename(all_opts, "bps_wr", "throttling.bps-write");
+    qemu_opt_rename(all_opts, "bps", "throttling.bps-total", &local_err);
+    qemu_opt_rename(all_opts, "bps_rd", "throttling.bps-read", &local_err);
+    qemu_opt_rename(all_opts, "bps_wr", "throttling.bps-write", &local_err);
 
-    qemu_opt_rename(all_opts, "iops_max", "throttling.iops-total-max");
-    qemu_opt_rename(all_opts, "iops_rd_max", "throttling.iops-read-max");
-    qemu_opt_rename(all_opts, "iops_wr_max", "throttling.iops-write-max");
+    qemu_opt_rename(all_opts, "iops_max", "throttling.iops-total-max", &local_err);
+    qemu_opt_rename(all_opts, "iops_rd_max", "throttling.iops-read-max", &local_err);
+    qemu_opt_rename(all_opts, "iops_wr_max", "throttling.iops-write-max", &local_err);
 
-    qemu_opt_rename(all_opts, "bps_max", "throttling.bps-total-max");
-    qemu_opt_rename(all_opts, "bps_rd_max", "throttling.bps-read-max");
-    qemu_opt_rename(all_opts, "bps_wr_max", "throttling.bps-write-max");
+    qemu_opt_rename(all_opts, "bps_max", "throttling.bps-total-max", &local_err);
+    qemu_opt_rename(all_opts, "bps_rd_max", "throttling.bps-read-max", &local_err);
+    qemu_opt_rename(all_opts, "bps_wr_max", "throttling.bps-write-max", &local_err);
 
     qemu_opt_rename(all_opts,
-                    "iops_size", "throttling.iops-size");
+                    "iops_size", "throttling.iops-size", &local_err);
+
+    qemu_opt_rename(all_opts, "readonly", "read-only", &local_err);
 
-    qemu_opt_rename(all_opts, "readonly", "read-only");
+    if (local_err) {
+        error_report("%s", error_get_pretty(local_err));
+        error_free(local_err);
+        return NULL;
+    }
 
     value = qemu_opt_get(all_opts, "cache");
     if (value) {
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index a41334e..11c858f 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -199,6 +199,29 @@ run_qemu -drive file.driver=raw
 run_qemu -drive foo=bar
 
 echo
+echo === Specifying both an option and its legacy alias ===
+echo
+
+run_qemu -drive file="$TEST_IMG",iops=1234,throttling.iops-total=5678
+run_qemu -drive file="$TEST_IMG",iops_rd=1234,throttling.iops-read=5678
+run_qemu -drive file="$TEST_IMG",iops_wr=1234,throttling.iops-write=5678
+
+run_qemu -drive file="$TEST_IMG",bps=1234,throttling.bps-total=5678
+run_qemu -drive file="$TEST_IMG",bps_rd=1234,throttling.bps-read=5678
+run_qemu -drive file="$TEST_IMG",bps_wr=1234,throttling.bps-write=5678
+
+run_qemu -drive file="$TEST_IMG",iops_max=1234,throttling.iops-total-max=5678
+run_qemu -drive file="$TEST_IMG",iops_rd_max=1234,throttling.iops-read-max=5678
+run_qemu -drive file="$TEST_IMG",iops_wr_max=1234,throttling.iops-write-max=5678
+
+run_qemu -drive file="$TEST_IMG",bps_max=1234,throttling.bps-total-max=5678
+run_qemu -drive file="$TEST_IMG",bps_rd_max=1234,throttling.bps-read-max=5678
+run_qemu -drive file="$TEST_IMG",bps_wr_max=1234,throttling.bps-write-max=5678
+
+run_qemu -drive file="$TEST_IMG",iops_size=1234,throttling.iops-size=5678
+run_qemu -drive file="$TEST_IMG",readonly=on,read-only=off
+
+echo
 echo === Parsing protocol from file name ===
 echo
 
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index fee597e..7f16134 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -275,6 +275,51 @@ Testing: -drive foo=bar
 QEMU_PROG: -drive foo=bar: could not open disk image ide0-hd0: Must specify either driver or file
 
 
+=== Specifying both an option and its legacy alias ===
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops=1234,throttling.iops-total=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops=1234,throttling.iops-total=5678: 'throttling.iops-total' and its alias 'iops' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_rd=1234,throttling.iops-read=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_rd=1234,throttling.iops-read=5678: 'throttling.iops-read' and its alias 'iops_rd' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_wr=1234,throttling.iops-write=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_wr=1234,throttling.iops-write=5678: 'throttling.iops-write' and its alias 'iops_wr' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps=1234,throttling.bps-total=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps=1234,throttling.bps-total=5678: 'throttling.bps-total' and its alias 'bps' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_rd=1234,throttling.bps-read=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd=1234,throttling.bps-read=5678: 'throttling.bps-read' and its alias 'bps_rd' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_wr=1234,throttling.bps-write=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_wr=1234,throttling.bps-write=5678: 'throttling.bps-write' and its alias 'bps_wr' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_max=1234,throttling.iops-total-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_max=1234,throttling.iops-total-max=5678: 'throttling.iops-total-max' and its alias 'iops_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_rd_max=1234,throttling.iops-read-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_rd_max=1234,throttling.iops-read-max=5678: 'throttling.iops-read-max' and its alias 'iops_rd_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_wr_max=1234,throttling.iops-write-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_wr_max=1234,throttling.iops-write-max=5678: 'throttling.iops-write-max' and its alias 'iops_wr_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_max=1234,throttling.bps-total-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_max=1234,throttling.bps-total-max=5678: 'throttling.bps-total-max' and its alias 'bps_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_rd_max=1234,throttling.bps-read-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_rd_max=1234,throttling.bps-read-max=5678: 'throttling.bps-read-max' and its alias 'bps_rd_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,bps_wr_max=1234,throttling.bps-write-max=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,bps_wr_max=1234,throttling.bps-write-max=5678: 'throttling.bps-write-max' and its alias 'bps_wr_max' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,iops_size=1234,throttling.iops-size=5678
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,iops_size=1234,throttling.iops-size=5678: 'throttling.iops-size' and its alias 'iops_size' can't be used at the same time
+
+Testing: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' and its alias 'readonly' can't be used at the same time
+
+
 === Parsing protocol from file name ===
 
 Testing: -hda foo:bar
-- 
1.8.3.1