From d8336c6b286d9715bfe29a0e0b415dfd7891f671 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Tue, 20 Nov 2018 18:09:49 +0100
Subject: iotests: Replace time.clock() with Timeout
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

time.clock() is deprecated since Python 3.3. Current Python versions
warn that the function will be removed in Python 3.8, and those warnings
make the test case 118 fail.

Replace it with the Timeout mechanism that is compatible with both
Python 2 and 3, and makes the code even a little nicer.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qemu-iotests/118 | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
index ff3b2ae3e7..c4f4c213ca 100755
--- a/tests/qemu-iotests/118
+++ b/tests/qemu-iotests/118
@@ -53,21 +53,17 @@ class ChangeBaseClass(iotests.QMPTestCase):
         if not self.has_real_tray:
             return
 
-        timeout = time.clock() + 3
-        while not self.has_opened and time.clock() < timeout:
-            self.process_events()
-        if not self.has_opened:
-            self.fail('Timeout while waiting for the tray to open')
+        with iotests.Timeout(3, 'Timeout while waiting for the tray to open'):
+            while not self.has_opened:
+                self.process_events()
 
     def wait_for_close(self):
         if not self.has_real_tray:
             return
 
-        timeout = time.clock() + 3
-        while not self.has_closed and time.clock() < timeout:
-            self.process_events()
-        if not self.has_opened:
-            self.fail('Timeout while waiting for the tray to close')
+        with iotests.Timeout(3, 'Timeout while waiting for the tray to close'):
+            while not self.has_closed:
+                self.process_events()
 
 class GeneralChangeTestsBaseClass(ChangeBaseClass):
 
-- 
cgit v1.2.3


From fa1cfb40262b8a60e0f93b70491660f242638f81 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Tue, 20 Nov 2018 18:12:21 +0100
Subject: iotests: Replace assertEquals() with assertEqual()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

TestCase.assertEquals() is deprecated since Python 2.7. Recent Python
versions print a warning when the function is called, which makes test
cases fail.

Replace it with the preferred spelling assertEqual().

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qemu-iotests/041        | 6 +++---
 tests/qemu-iotests/118        | 4 ++--
 tests/qemu-iotests/iotests.py | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 3615011d98..26bf1701eb 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -469,7 +469,7 @@ new_state = "1"
             self.assert_qmp(event, 'data/id', 'drive0')
             event = self.vm.get_qmp_event(wait=True)
 
-        self.assertEquals(event['event'], 'BLOCK_JOB_ERROR')
+        self.assertEqual(event['event'], 'BLOCK_JOB_ERROR')
         self.assert_qmp(event, 'data/device', 'drive0')
         self.assert_qmp(event, 'data/operation', 'read')
         result = self.vm.qmp('query-block-jobs')
@@ -494,7 +494,7 @@ new_state = "1"
             self.assert_qmp(event, 'data/id', 'drive0')
             event = self.vm.get_qmp_event(wait=True)
 
-        self.assertEquals(event['event'], 'BLOCK_JOB_ERROR')
+        self.assertEqual(event['event'], 'BLOCK_JOB_ERROR')
         self.assert_qmp(event, 'data/device', 'drive0')
         self.assert_qmp(event, 'data/operation', 'read')
         result = self.vm.qmp('query-block-jobs')
@@ -625,7 +625,7 @@ new_state = "1"
         self.assert_qmp(result, 'return', {})
 
         event = self.vm.event_wait(name='BLOCK_JOB_ERROR')
-        self.assertEquals(event['event'], 'BLOCK_JOB_ERROR')
+        self.assertEqual(event['event'], 'BLOCK_JOB_ERROR')
         self.assert_qmp(event, 'data/device', 'drive0')
         self.assert_qmp(event, 'data/operation', 'write')
         result = self.vm.qmp('query-block-jobs')
diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
index c4f4c213ca..603e10e8a2 100755
--- a/tests/qemu-iotests/118
+++ b/tests/qemu-iotests/118
@@ -261,7 +261,7 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
         result = self.vm.qmp('blockdev-close-tray', id=self.device_name)
         # Should be a no-op
         self.assert_qmp(result, 'return', {})
-        self.assertEquals(self.vm.get_qmp_events(wait=False), [])
+        self.assertEqual(self.vm.get_qmp_events(wait=False), [])
 
     def test_remove_on_closed(self):
         if not self.has_real_tray:
@@ -448,7 +448,7 @@ class TestChangeReadOnly(ChangeBaseClass):
                                                        read_only_mode='retain')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        self.assertEquals(self.vm.get_qmp_events(wait=False), [])
+        self.assertEqual(self.vm.get_qmp_events(wait=False), [])
 
         result = self.vm.qmp('query-block')
         self.assert_qmp(result, 'return[0]/inserted/ro', False)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 27bb2b600c..d537538ba0 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -581,7 +581,7 @@ class QMPTestCase(unittest.TestCase):
     def wait_ready_and_cancel(self, drive='drive0'):
         self.wait_ready(drive=drive)
         event = self.cancel_and_wait(drive=drive)
-        self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
+        self.assertEqual(event['event'], 'BLOCK_JOB_COMPLETED')
         self.assert_qmp(event, 'data/type', 'mirror')
         self.assert_qmp(event, 'data/offset', event['data']['len'])
 
-- 
cgit v1.2.3


From 155af09d44f584a790118f78448f50f140d0f788 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Tue, 20 Nov 2018 16:52:41 -0600
Subject: iotests: Skip 233 if certtool not installed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The use of TLS while building qemu is optional. While the
'certtool' binary should be available on every platform that
supports building against TLS, that does not imply that the
developer has installed it.  Make the test gracefully skip
in that case.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/common.tls | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/common.tls b/tests/qemu-iotests/common.tls
index 39f17c1b99..eae81789bb 100644
--- a/tests/qemu-iotests/common.tls
+++ b/tests/qemu-iotests/common.tls
@@ -31,6 +31,9 @@ tls_x509_cleanup()
 
 tls_x509_init()
 {
+    (certtool --help) >/dev/null 2>&1 || \
+	_notrun "certtool utility not found, skipping test"
+
     mkdir -p "${tls_dir}"
 
     # use a fixed key so we don't waste system entropy on
-- 
cgit v1.2.3


From f0998879e049dad19beed881a1c56643ce536384 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Mon, 19 Nov 2018 11:19:20 +0100
Subject: qemu-img: Fix typo

Fixes: d402b6a21a825a5c07aac9251990860723d49f5d
Reported-by: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 13a6ca31b4..a9a2470e1a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -261,7 +261,7 @@ static int print_block_option_help(const char *filename, const char *fmt)
             return 1;
         }
         if (!proto_drv->create_opts) {
-            error_report("Protocal driver '%s' does not support image creation",
+            error_report("Protocol driver '%s' does not support image creation",
                          proto_drv->format_name);
             return 1;
         }
-- 
cgit v1.2.3


From 3ecd5a4f19fd9a497490a91aaa96e76a5edadd2c Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Mon, 19 Nov 2018 11:19:21 +0100
Subject: qemu-img: Fix leak

create_opts was leaked here.  This is not too bad since the process is
about to exit anyway, but relying on that does not make the code nicer
to read.

Fixes: d402b6a21a825a5c07aac9251990860723d49f5d
Reported-by: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qemu-img.c b/qemu-img.c
index a9a2470e1a..ad04f59565 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -263,6 +263,7 @@ static int print_block_option_help(const char *filename, const char *fmt)
         if (!proto_drv->create_opts) {
             error_report("Protocol driver '%s' does not support image creation",
                          proto_drv->format_name);
+            qemu_opts_free(create_opts);
             return 1;
         }
         create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
-- 
cgit v1.2.3


From 1c7f618f689b0b5b6bbed23a7c159e7dad7b996f Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 21 Nov 2018 12:47:47 +0000
Subject: scsi-disk: Fix crash if underlying host file or disk returns error

Commit 40dce4ee6 "scsi-disk: fix rerror/werror=ignore" introduced a
bug which causes qemu to crash with the assertion error below if the
host file or disk returns an error:

  qemu-system-x86_64: hw/scsi/scsi-bus.c:1374: scsi_req_complete:
  Assertion `req->status == -1' failed.

Kevin Wolf suggested this fix:

  < kwolf> Hm, should the final return false; in that patch
           actually be a return true?
  < kwolf> Because I think he didn't intend to change anything
           except BLOCK_ERROR_ACTION_IGNORE

Buglink: https://bugs.launchpad.net/qemu/+bug/1804323
Fixes: 40dce4ee61c68395f6d463fae792f61b7c003bce
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/scsi/scsi-disk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 6eb258d3f3..0e9027c8f3 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -482,7 +482,7 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int error, bool acct_failed)
     if (action == BLOCK_ERROR_ACTION_STOP) {
         scsi_req_retry(&r->req);
     }
-    return false;
+    return true;
 }
 
 static void scsi_write_complete_noio(SCSIDiskReq *r, int ret)
-- 
cgit v1.2.3


From 2a3d4331fa2d40708188b8000f98ff1f7dcd33bc Mon Sep 17 00:00:00 2001
From: Alberto Garcia <berto@igalia.com>
Date: Mon, 12 Nov 2018 16:00:48 +0200
Subject: block: Fix update of BDRV_O_AUTO_RDONLY in
 update_flags_from_options()

Commit e35bdc123a4ace9f4d3fcca added the auto-read-only option and the
code to update its corresponding flag in update_flags_from_options(),
but forgot to clear the flag if auto-read-only is false.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 3feac08535..9cd6f4a50d 100644
--- a/block.c
+++ b/block.c
@@ -1137,7 +1137,7 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
 
 static void update_flags_from_options(int *flags, QemuOpts *opts)
 {
-    *flags &= ~BDRV_O_CACHE_MASK;
+    *flags &= ~(BDRV_O_CACHE_MASK | BDRV_O_RDWR | BDRV_O_AUTO_RDONLY);
 
     assert(qemu_opt_find(opts, BDRV_OPT_CACHE_NO_FLUSH));
     if (qemu_opt_get_bool_del(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) {
@@ -1149,8 +1149,6 @@ static void update_flags_from_options(int *flags, QemuOpts *opts)
         *flags |= BDRV_O_NOCACHE;
     }
 
-    *flags &= ~BDRV_O_RDWR;
-
     assert(qemu_opt_find(opts, BDRV_OPT_READ_ONLY));
     if (!qemu_opt_get_bool_del(opts, BDRV_OPT_READ_ONLY, false)) {
         *flags |= BDRV_O_RDWR;
-- 
cgit v1.2.3


From e4c8f2925d22584b2008aadea5c70e1e05c2a522 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Tue, 20 Nov 2018 17:56:46 +0000
Subject: iotests: fix nbd test 233 to work correctly with raw images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The first qemu-io command must honour the $IMGFMT that is set rather
than hardcoding qcow2. The qemu-nbd commands should also set $IMGFMT
to avoid the insecure format probe warning.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/233 | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/233 b/tests/qemu-iotests/233
index a4da60d0ad..1814efe333 100755
--- a/tests/qemu-iotests/233
+++ b/tests/qemu-iotests/233
@@ -66,7 +66,7 @@ $QEMU_IO -c 'w -P 0x11 1m 1m' "$TEST_IMG" | _filter_qemu_io
 
 echo
 echo "== check TLS client to plain server fails =="
-nbd_server_start_tcp_socket "$TEST_IMG"
+nbd_server_start_tcp_socket -f $IMGFMT "$TEST_IMG"
 
 $QEMU_IMG info --image-opts \
     --object tls-creds-x509,dir=${tls_dir}/client1,endpoint=client,id=tls0 \
@@ -78,7 +78,10 @@ nbd_server_stop
 echo
 echo "== check plain client to TLS server fails =="
 
-nbd_server_start_tcp_socket --object tls-creds-x509,dir=${tls_dir}/server1,endpoint=server,id=tls0,verify-peer=yes --tls-creds tls0 "$TEST_IMG"
+nbd_server_start_tcp_socket \
+    --object tls-creds-x509,dir=${tls_dir}/server1,endpoint=server,id=tls0,verify-peer=yes \
+    --tls-creds tls0 \
+    -f $IMGFMT "$TEST_IMG"
 
 $QEMU_IMG info nbd://localhost:$nbd_tcp_port 2>&1 | sed "s/$nbd_tcp_port/PORT/g"
 
@@ -104,7 +107,7 @@ $QEMU_IO -c 'r -P 0x11 1m 1m' -c 'w -P 0x22 1m 1m' --image-opts \
     driver=nbd,host=$nbd_tcp_addr,port=$nbd_tcp_port,tls-creds=tls0 \
     2>&1 | _filter_qemu_io
 
-$QEMU_IO -f qcow2 -r -U -c 'r -P 0x22 1m 1m' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -f $IMGFMT -r -U -c 'r -P 0x22 1m 1m' "$TEST_IMG" | _filter_qemu_io
 
 # success, all done
 echo "*** done"
-- 
cgit v1.2.3


From 6bf7463615752934d7221e5be9820d9da45ab2de Mon Sep 17 00:00:00 2001
From: Igor Druzhinin <igor.druzhinin@citrix.com>
Date: Tue, 6 Nov 2018 12:16:55 +0000
Subject: nvme: call blk_drain in NVMe reset code to avoid lockups

When blk_flush called in NVMe reset path S/C queues are already freed
which means that re-entering AIO handling loop having some IO requests
unfinished will lockup or crash as their SG structures being potentially
reused. Call blk_drain before freeing the queues to avoid this nasty
scenario.

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/nvme.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d0226e7fdc..28d284346d 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -797,6 +797,8 @@ static void nvme_clear_ctrl(NvmeCtrl *n)
 {
     int i;
 
+    blk_drain(n->conf.blk);
+
     for (i = 0; i < n->num_queues; i++) {
         if (n->sq[i] != NULL) {
             nvme_free_sq(n->sq[i], n);
-- 
cgit v1.2.3


From 87ad860c622cc8f8916b5232bd8728c08f938fce Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 20 Nov 2018 19:41:48 +0100
Subject: nvme: fix out-of-bounds access to the CMB
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Because the CMB BAR has a min_access_size of 2, if you read the last
byte it will try to memcpy *2* bytes from n->cmbuf, causing an off-by-one
error.  This is CVE-2018-16847.

Another way to fix this might be to register the CMB as a RAM memory
region, which would also be more efficient.  However, that might be a
change for big-endian machines; I didn't think this through and I don't
know how real hardware works.  Add a basic testcase for the CMB in case
somebody does this change later on.

Cc: Keith Busch <keith.busch@intel.com>
Cc: qemu-block@nongnu.org
Reported-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Tested-by: Li Qiang <liq3ea@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/nvme.c        |  2 +-
 tests/Makefile.include |  2 +-
 tests/nvme-test.c      | 68 ++++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 28d284346d..8c35cab2b4 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1201,7 +1201,7 @@ static const MemoryRegionOps nvme_cmb_ops = {
     .write = nvme_cmb_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .impl = {
-        .min_access_size = 2,
+        .min_access_size = 1,
         .max_access_size = 8,
     },
 };
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 613242bc6e..fb0b449c02 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -730,7 +730,7 @@ tests/test-hmp$(EXESUF): tests/test-hmp.o
 tests/machine-none-test$(EXESUF): tests/machine-none-test.o
 tests/drive_del-test$(EXESUF): tests/drive_del-test.o $(libqos-virtio-obj-y)
 tests/qdev-monitor-test$(EXESUF): tests/qdev-monitor-test.o $(libqos-pc-obj-y)
-tests/nvme-test$(EXESUF): tests/nvme-test.o
+tests/nvme-test$(EXESUF): tests/nvme-test.o $(libqos-pc-obj-y)
 tests/pvpanic-test$(EXESUF): tests/pvpanic-test.o
 tests/i82801b11-test$(EXESUF): tests/i82801b11-test.o
 tests/ac97-test$(EXESUF): tests/ac97-test.o
diff --git a/tests/nvme-test.c b/tests/nvme-test.c
index 7674a446e4..2700ba838a 100644
--- a/tests/nvme-test.c
+++ b/tests/nvme-test.c
@@ -8,25 +8,73 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "libqtest.h"
+#include "libqos/libqos-pc.h"
+
+static QOSState *qnvme_start(const char *extra_opts)
+{
+    QOSState *qs;
+    const char *arch = qtest_get_arch();
+    const char *cmd = "-drive id=drv0,if=none,file=null-co://,format=raw "
+                      "-device nvme,addr=0x4.0,serial=foo,drive=drv0 %s";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qs = qtest_pc_boot(cmd, extra_opts ? : "");
+        global_qtest = qs->qts;
+        return qs;
+    }
+
+    g_printerr("nvme tests are only available on x86\n");
+    exit(EXIT_FAILURE);
+}
+
+static void qnvme_stop(QOSState *qs)
+{
+    qtest_shutdown(qs);
+}
 
-/* Tests only initialization so far. TODO: Replace with functional tests */
 static void nop(void)
 {
+    QOSState *qs;
+
+    qs = qnvme_start(NULL);
+    qnvme_stop(qs);
 }
 
-int main(int argc, char **argv)
+static void nvmetest_cmb_test(void)
 {
-    int ret;
+    const int cmb_bar_size = 2 * MiB;
+    QOSState *qs;
+    QPCIDevice *pdev;
+    QPCIBar bar;
 
-    g_test_init(&argc, &argv, NULL);
-    qtest_add_func("/nvme/nop", nop);
+    qs = qnvme_start("-global nvme.cmb_size_mb=2");
+    pdev = qpci_device_find(qs->pcibus, QPCI_DEVFN(4,0));
+    g_assert(pdev != NULL);
+
+    qpci_device_enable(pdev);
+    bar = qpci_iomap(pdev, 2, NULL);
+
+    qpci_io_writel(pdev, bar, 0, 0xccbbaa99);
+    g_assert_cmpint(qpci_io_readb(pdev, bar, 0), ==, 0x99);
+    g_assert_cmpint(qpci_io_readw(pdev, bar, 0), ==, 0xaa99);
+
+    /* Test partially out-of-bounds accesses.  */
+    qpci_io_writel(pdev, bar, cmb_bar_size - 1, 0x44332211);
+    g_assert_cmpint(qpci_io_readb(pdev, bar, cmb_bar_size - 1), ==, 0x11);
+    g_assert_cmpint(qpci_io_readw(pdev, bar, cmb_bar_size - 1), !=, 0x2211);
+    g_assert_cmpint(qpci_io_readl(pdev, bar, cmb_bar_size - 1), !=, 0x44332211);
+    g_free(pdev);
 
-    qtest_start("-drive id=drv0,if=none,file=null-co://,format=raw "
-                "-device nvme,drive=drv0,serial=foo");
-    ret = g_test_run();
+    qnvme_stop(qs);
+}
 
-    qtest_end();
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("/nvme/nop", nop);
+    qtest_add_func("/nvme/cmb_test", nvmetest_cmb_test);
 
-    return ret;
+    return g_test_run();
 }
-- 
cgit v1.2.3


From 2067d39e5e53b053bece6fa15711640123c119ed Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Thu, 22 Nov 2018 15:52:20 +0100
Subject: Revert "nvme: fix oob access issue(CVE-2018-16847)"

This reverts commit 5e3c0220d7e4f0361c4d36c697a8842f2b583402.
We have a better fix commited for this now.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/nvme.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 8c35cab2b4..84062d388f 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1177,10 +1177,6 @@ static void nvme_cmb_write(void *opaque, hwaddr addr, uint64_t data,
     unsigned size)
 {
     NvmeCtrl *n = (NvmeCtrl *)opaque;
-
-    if (addr + size > NVME_CMBSZ_GETSIZE(n->bar.cmbsz)) {
-        return;
-    }
     memcpy(&n->cmbuf[addr], &data, size);
 }
 
@@ -1189,9 +1185,6 @@ static uint64_t nvme_cmb_read(void *opaque, hwaddr addr, unsigned size)
     uint64_t val;
     NvmeCtrl *n = (NvmeCtrl *)opaque;
 
-    if (addr + size > NVME_CMBSZ_GETSIZE(n->bar.cmbsz)) {
-        return 0;
-    }
     memcpy(&val, &n->cmbuf[addr], size);
     return val;
 }
-- 
cgit v1.2.3


From 71a86ddece548860f040d565204cf1bf59d74663 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Thu, 22 Nov 2018 19:23:35 +0100
Subject: nvme: fix CMB endianness confusion

The CMB is marked as DEVICE_LITTLE_ENDIAN, so the data must be
read/written as if it was little-endian output (in the case of
big endian, we get two swaps, one in the memory core and one
in nvme.c).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/nvme.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 84062d388f..01f3e853cf 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1177,16 +1177,13 @@ static void nvme_cmb_write(void *opaque, hwaddr addr, uint64_t data,
     unsigned size)
 {
     NvmeCtrl *n = (NvmeCtrl *)opaque;
-    memcpy(&n->cmbuf[addr], &data, size);
+    stn_le_p(&n->cmbuf[addr], size, data);
 }
 
 static uint64_t nvme_cmb_read(void *opaque, hwaddr addr, unsigned size)
 {
-    uint64_t val;
     NvmeCtrl *n = (NvmeCtrl *)opaque;
-
-    memcpy(&val, &n->cmbuf[addr], size);
-    return val;
+    return ldn_le_p(&n->cmbuf[addr], size);
 }
 
 static const MemoryRegionOps nvme_cmb_ops = {
-- 
cgit v1.2.3


From ad3a7e4555bc50036a5257a6c1ed652ab0d1b650 Mon Sep 17 00:00:00 2001
From: Logan Gunthorpe <logang@deltatee.com>
Date: Wed, 21 Nov 2018 11:10:13 -0700
Subject: nvme: fix bug with PCI IRQ pins on teardown

When the submission and completion queues are being torn down
the IRQ will be asserted for the completion queue when the
submsission queue is deleted. Then when the completion queue
is deleted it stays asserted. Thus, on systems that do
not use MSI, no further interrupts can be triggered on the host.

Linux sees this as a long delay when unbinding the nvme device.
Eventually the interrupt timeout occurs and it continues.

To fix this we ensure we deassert the IRQ for a CQ when it is
deleted.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/nvme.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 01f3e853cf..9fbe5673cb 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -554,6 +554,7 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeCmd *cmd)
         trace_nvme_err_invalid_del_cq_notempty(qid);
         return NVME_INVALID_QUEUE_DEL;
     }
+    nvme_irq_deassert(n, cq);
     trace_nvme_del_cq(qid);
     nvme_free_cq(cq, n);
     return NVME_SUCCESS;
-- 
cgit v1.2.3


From a237dea330a2be9a2cbe95056b9a8d67627d95c6 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 19 Nov 2018 11:29:24 -0600
Subject: iotests: Enhance 223 to cover multiple bitmap granularities

Testing granularity at the same size as the cluster isn't quite
as fun as what happens when it is larger or smaller.  This
enhancement also shows that qemu's nbd server can serve the
same disk over multiple exports simultaneously.

Signed-off-by: Eric Blake <eblake@redhat.com>
Tested-by: John Snow <jsnow@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/223     | 43 +++++++++++++++++++++++++++++++++++--------
 tests/qemu-iotests/223.out | 32 +++++++++++++++++++++++++-------
 2 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
index 72419e0338..397b865d34 100755
--- a/tests/qemu-iotests/223
+++ b/tests/qemu-iotests/223
@@ -57,10 +57,11 @@ run_qemu()
 }
 
 echo
-echo "=== Create partially sparse image, then add dirty bitmap ==="
+echo "=== Create partially sparse image, then add dirty bitmaps ==="
 echo
 
-_make_test_img 4M
+# Two bitmaps, to contrast granularity issues
+_make_test_img -o cluster_size=4k 4M
 $QEMU_IO -c 'w -P 0x11 1M 2M' "$TEST_IMG" | _filter_qemu_io
 run_qemu <<EOF
 { "execute": "qmp_capabilities" }
@@ -78,7 +79,16 @@ run_qemu <<EOF
   "arguments": {
     "node": "n",
     "name": "b",
-    "persistent": true
+    "persistent": true,
+    "granularity": 65536
+  }
+}
+{ "execute": "block-dirty-bitmap-add",
+  "arguments": {
+    "node": "n",
+    "name": "b2",
+    "persistent": true,
+    "granularity": 512
   }
 }
 { "execute": "quit" }
@@ -88,10 +98,11 @@ echo
 echo "=== Write part of the file under active bitmap ==="
 echo
 
-$QEMU_IO -c 'w -P 0x22 2M 2M' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'w -P 0x22 512 512' -c 'w -P 0x33 2M 2M' "$TEST_IMG" \
+    | _filter_qemu_io
 
 echo
-echo "=== End dirty bitmap, and start serving image over NBD ==="
+echo "=== End dirty bitmaps, and start serving image over NBD ==="
 echo
 
 _launch_qemu 2> >(_filter_nbd)
@@ -103,6 +114,8 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"blockdev-add",
     "file":{"driver":"file", "filename":"'"$TEST_IMG"'"}}}' "return"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"x-block-dirty-bitmap-disable",
   "arguments":{"node":"n", "name":"b"}}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-block-dirty-bitmap-disable",
+  "arguments":{"node":"n", "name":"b2"}}' "return"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-start",
   "arguments":{"addr":{"type":"unix",
     "data":{"path":"'"$TEST_DIR/nbd"'"}}}}' "return"
@@ -110,26 +123,40 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
   "arguments":{"device":"n"}}' "return"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"x-nbd-server-add-bitmap",
   "arguments":{"name":"n", "bitmap":"b"}}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
+  "arguments":{"device":"n", "name":"n2"}}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-nbd-server-add-bitmap",
+  "arguments":{"name":"n2", "bitmap":"b2"}}' "return"
 
 echo
-echo "=== Contrast normal status with dirty-bitmap status ==="
+echo "=== Contrast normal status to large granularity dirty-bitmap ==="
 echo
 
 QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
 IMG="driver=nbd,export=n,server.type=unix,server.path=$TEST_DIR/nbd"
-$QEMU_IO -r -c 'r -P 0 0 1m' -c 'r -P 0x11 1m 1m' \
-  -c 'r -P 0x22 2m 2m' --image-opts "$IMG" | _filter_qemu_io
+$QEMU_IO -r -c 'r -P 0x22 512 512' -c 'r -P 0 512k 512k' -c 'r -P 0x11 1m 1m' \
+  -c 'r -P 0x33 2m 2m' --image-opts "$IMG" | _filter_qemu_io
 $QEMU_IMG map --output=json --image-opts \
   "$IMG" | _filter_qemu_img_map
 $QEMU_IMG map --output=json --image-opts \
   "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b" | _filter_qemu_img_map
 
+echo
+echo "=== Contrast to small granularity dirty-bitmap ==="
+echo
+
+IMG="driver=nbd,export=n2,server.type=unix,server.path=$TEST_DIR/nbd"
+$QEMU_IMG map --output=json --image-opts \
+  "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b2" | _filter_qemu_img_map
+
 echo
 echo "=== End NBD server ==="
 echo
 
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-remove",
   "arguments":{"name":"n"}}' "return"
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-remove",
+  "arguments":{"name":"n2"}}' "return"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-stop"}' "return"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"quit"}' "return"
 
diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
index 33021c8e6a..de417477de 100644
--- a/tests/qemu-iotests/223.out
+++ b/tests/qemu-iotests/223.out
@@ -1,6 +1,6 @@
 QA output created by 223
 
-=== Create partially sparse image, then add dirty bitmap ===
+=== Create partially sparse image, then add dirty bitmaps ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4194304
 wrote 2097152/2097152 bytes at offset 1048576
@@ -11,15 +11,18 @@ QMP_VERSION
 {"return": {}}
 {"return": {}}
 {"return": {}}
+{"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
 
 
 === Write part of the file under active bitmap ===
 
+wrote 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 2097152/2097152 bytes at offset 2097152
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
-=== End dirty bitmap, and start serving image over NBD ===
+=== End dirty bitmaps, and start serving image over NBD ===
 
 {"return": {}}
 {"return": {}}
@@ -27,18 +30,32 @@ wrote 2097152/2097152 bytes at offset 2097152
 {"return": {}}
 {"return": {}}
 {"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
 
-=== Contrast normal status with dirty-bitmap status ===
+=== Contrast normal status to large granularity dirty-bitmap ===
 
-read 1048576/1048576 bytes at offset 0
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 524288/524288 bytes at offset 524288
+512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 2097152/2097152 bytes at offset 2097152
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false},
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true},
+{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false},
 { "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true}]
-[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true},
+[{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": false},
+{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true},
+{ "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}]
+
+=== Contrast to small granularity dirty-bitmap ===
+
+[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true},
+{ "start": 512, "length": 512, "depth": 0, "zero": false, "data": false},
+{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true},
 { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}]
 
 === End NBD server ===
@@ -46,4 +63,5 @@ read 2097152/2097152 bytes at offset 2097152
 {"return": {}}
 {"return": {}}
 {"return": {}}
+{"return": {}}
 *** done
-- 
cgit v1.2.3


From 0065c455f9f03ef5e830ede804a7404a8892fbc7 Mon Sep 17 00:00:00 2001
From: Alberto Garcia <berto@igalia.com>
Date: Wed, 31 Oct 2018 18:16:37 +0200
Subject: block: Update BlockDriverState.inherits_from on bdrv_set_backing_hd()

When a BlockDriverState's child is opened (be it a backing file, the
protocol layer, or any other) inherits_from is set to point to the
parent node. Children opened separately and then attached to a parent
don't have this pointer set.

bdrv_reopen_queue_child() uses this to determine whether a node's
children must also be reopened inheriting the options from the parent
or not. If inherits_from points to the parent then the child is
reopened and its options can be changed, like in this example:

   $ qemu-img create -f qcow2 hd0.qcow2 1M
   $ qemu-img create -f qcow2 hd1.qcow2 1M
   $ $QEMU -drive if=none,node-name=hd0,file=hd0.qcow2,\
                  backing.driver=qcow2,backing.file.filename=hd1.qcow2
   (qemu) qemu-io hd0 "reopen -o backing.l2-cache-size=2M"

If the child does not inherit from the parent then it does not get
reopened and its options cannot be changed:

   $ $QEMU -drive if=none,node-name=hd1,file=hd1.qcow2
           -drive if=none,node-name=hd0,file=hd0.qcow2,backing=hd1
   (qemu) qemu-io hd0 "reopen -o backing.l2-cache-size=2M"
   Cannot change the option 'backing.l2-cache-size'

If a disk image has a chain of backing files then all of them are also
connected through their inherits_from pointers (i.e. it's possible to
walk the chain in reverse order from base to top).

However this is broken if the intermediate nodes are removed using
e.g. block-stream because the inherits_from pointer from the base node
becomes NULL:

   $ qemu-img create -f qcow2 hd0.qcow2 1M
   $ qemu-img create -f qcow2 -b hd0.qcow2 hd1.qcow2
   $ qemu-img create -f qcow2 -b hd1.qcow2 hd2.qcow2
   $ $QEMU -drive if=none,file=hd2.qcow2
   (qemu) qemu-io none0 "reopen -o backing.l2-cache-size=2M"
   (qemu) block_stream none0 0 hd0.qcow2
   (qemu) qemu-io none0 "reopen -o backing.l2-cache-size=2M"
   Cannot change the option 'backing.l2-cache-size'

This patch updates the inherits_from pointer if the intermediate nodes
of a backing chain are removed using bdrv_set_backing_hd(), and adds a
test case for this scenario.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                    |  21 +++++++++
 tests/qemu-iotests/161     | 104 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/161.out |  23 ++++++++++
 tests/qemu-iotests/group   |   1 +
 4 files changed, 149 insertions(+)
 create mode 100755 tests/qemu-iotests/161
 create mode 100644 tests/qemu-iotests/161.out

diff --git a/block.c b/block.c
index 9cd6f4a50d..fe0cdc7d99 100644
--- a/block.c
+++ b/block.c
@@ -2260,6 +2260,18 @@ static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load)
     }
 }
 
+/* Return true if you can reach parent going through child->inherits_from
+ * recursively. If parent or child are NULL, return false */
+static bool bdrv_inherits_from_recursive(BlockDriverState *child,
+                                         BlockDriverState *parent)
+{
+    while (child && child != parent) {
+        child = child->inherits_from;
+    }
+
+    return child != NULL;
+}
+
 /*
  * Sets the backing file link of a BDS. A new reference is created; callers
  * which don't need their own reference any more must call bdrv_unref().
@@ -2267,6 +2279,9 @@ static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load)
 void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
                          Error **errp)
 {
+    bool update_inherits_from = bdrv_chain_contains(bs, backing_hd) &&
+        bdrv_inherits_from_recursive(backing_hd, bs);
+
     if (backing_hd) {
         bdrv_ref(backing_hd);
     }
@@ -2282,6 +2297,12 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
 
     bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_backing,
                                     errp);
+    /* If backing_hd was already part of bs's backing chain, and
+     * inherits_from pointed recursively to bs then let's update it to
+     * point directly to bs (else it will become NULL). */
+    if (update_inherits_from) {
+        backing_hd->inherits_from = bs;
+    }
     if (!bs->backing) {
         bdrv_unref(backing_hd);
     }
diff --git a/tests/qemu-iotests/161 b/tests/qemu-iotests/161
new file mode 100755
index 0000000000..8d0c6afb47
--- /dev/null
+++ b/tests/qemu-iotests/161
@@ -0,0 +1,104 @@
+#!/bin/bash
+#
+# Test reopening a backing image after block-stream
+#
+# Copyright (C) 2018 Igalia, S.L.
+#
+# 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=berto@igalia.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+    rm -f "$TEST_IMG.base"
+    rm -f "$TEST_IMG.int"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+# Any format implementing BlockDriver.bdrv_change_backing_file
+_supported_fmt qcow2 qed
+_supported_proto file
+_supported_os Linux
+
+IMG_SIZE=1M
+
+# Create the images
+TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE | _filter_imgfmt
+TEST_IMG="$TEST_IMG.int" _make_test_img -b "$TEST_IMG.base" | _filter_imgfmt
+_make_test_img -b "$TEST_IMG.int" | _filter_imgfmt
+
+# First test: reopen $TEST.IMG changing the detect-zeroes option on
+# its backing file ($TEST_IMG.int).
+echo
+echo "*** Change an option on the backing file"
+echo
+_launch_qemu -drive if=none,file="${TEST_IMG}"
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'qmp_capabilities' }" \
+    'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'human-monitor-command',
+       'arguments': { 'command-line':
+                      'qemu-io none0 \"reopen -o backing.detect-zeroes=on\"' } }" \
+    "return"
+
+_cleanup_qemu
+
+# Second test: stream $TEST_IMG.base into $TEST_IMG.int and then
+# reopen $TEST.IMG changing the detect-zeroes option on its new
+# backing file ($TEST_IMG.base).
+echo
+echo "*** Stream and then change an option on the backing file"
+echo
+_launch_qemu -drive if=none,file="${TEST_IMG}"
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'qmp_capabilities' }" \
+    'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'block-stream', \
+       'arguments': { 'device': 'none0',
+                      'base': '${TEST_IMG}.base' } }" \
+    'return'
+
+# Wait for block-stream to finish
+sleep 0.5
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'human-monitor-command',
+       'arguments': { 'command-line':
+                      'qemu-io none0 \"reopen -o backing.detect-zeroes=on\"' } }" \
+    "return"
+
+_cleanup_qemu
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/161.out b/tests/qemu-iotests/161.out
new file mode 100644
index 0000000000..a3474717a2
--- /dev/null
+++ b/tests/qemu-iotests/161.out
@@ -0,0 +1,23 @@
+QA output created by 161
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+Formatting 'TEST_DIR/t.IMGFMT.int', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.int
+
+*** Change an option on the backing file
+
+{"return": {}}
+{"return": ""}
+
+*** Stream and then change an option on the backing file
+
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "none0"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "none0"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "none0"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "none0"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "none0", "len": 1048576, "offset": 1048576, "speed": 0, "type": "stream"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "none0"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "none0"}}
+{"return": ""}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 2722103381..ddf1a5b549 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -167,6 +167,7 @@
 158 rw auto quick
 159 rw auto quick
 160 rw auto quick
+161 rw auto quick
 162 auto quick
 163 rw auto
 165 rw auto quick
-- 
cgit v1.2.3


From 6bd858b3117a5aab066f3cf02ca72000eaa10ddb Mon Sep 17 00:00:00 2001
From: Alberto Garcia <berto@igalia.com>
Date: Wed, 31 Oct 2018 18:16:38 +0200
Subject: block: Update BlockDriverState.inherits_from on
 bdrv_drop_intermediate()

The previous patch fixed the inherits_from pointer after block-stream,
and this one does the same for block-commit.

When block-commit finishes and the 'top' node is not the topmost one
from the backing chain then all nodes above 'base' up to and including
'top' are removed from the chain.

The bdrv_drop_intermediate() call converts a chain like this one:

    base <- intermediate <- top <- active

into this one:

    base <- active

In a simple scenario each backing file from the first chain has the
inherits_from attribute pointing to its parent. This means that
reopening 'active' will recursively reopen all its children, whose
options can be changed in the process.

However after the 'block-commit' call base.inherits_from is NULL and
the chain is broken, so 'base' does not inherit from 'active' and will
not be reopened automatically:

   $ qemu-img create -f qcow2 hd0.qcow2 1M
   $ qemu-img create -f qcow2 -b hd0.qcow2 hd1.qcow2
   $ qemu-img create -f qcow2 -b hd1.qcow2 hd2.qcow2
   $ $QEMU -drive if=none,file=hd2.qcow2

   { 'execute': 'block-commit',
     'arguments': {
       'device': 'none0',
       'top': 'hd1.qcow2' } }

   { 'execute': 'human-monitor-command',
     'arguments': {
        'command-line':
          'qemu-io none0 "reopen -o backing.l2-cache-size=2M"' } }

   { "return": "Cannot change the option 'backing.l2-cache-size'\r\n"}

This patch updates base.inherits_from in this scenario, and adds a
test case.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                    | 16 ++++++++++++++++
 tests/qemu-iotests/161     | 35 ++++++++++++++++++++++++++++++++++-
 tests/qemu-iotests/161.out | 16 ++++++++++++++++
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index fe0cdc7d99..5ba3435f8f 100644
--- a/block.c
+++ b/block.c
@@ -3855,6 +3855,8 @@ BlockDriverState *bdrv_find_base(BlockDriverState *bs)
 int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
                            const char *backing_file_str)
 {
+    BlockDriverState *explicit_top = top;
+    bool update_inherits_from;
     BdrvChild *c, *next;
     Error *local_err = NULL;
     int ret = -EIO;
@@ -3870,6 +3872,16 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
         goto exit;
     }
 
+    /* If 'base' recursively inherits from 'top' then we should set
+     * base->inherits_from to top->inherits_from after 'top' and all
+     * other intermediate nodes have been dropped.
+     * If 'top' is an implicit node (e.g. "commit_top") we should skip
+     * it because no one inherits from it. We use explicit_top for that. */
+    while (explicit_top && explicit_top->implicit) {
+        explicit_top = backing_bs(explicit_top);
+    }
+    update_inherits_from = bdrv_inherits_from_recursive(base, explicit_top);
+
     /* success - we can delete the intermediate states, and link top->base */
     /* TODO Check graph modification op blockers (BLK_PERM_GRAPH_MOD) once
      * we've figured out how they should work. */
@@ -3905,6 +3917,10 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
         bdrv_unref(top);
     }
 
+    if (update_inherits_from) {
+        base->inherits_from = explicit_top->inherits_from;
+    }
+
     ret = 0;
 exit:
     bdrv_unref(top);
diff --git a/tests/qemu-iotests/161 b/tests/qemu-iotests/161
index 8d0c6afb47..180df17ad6 100755
--- a/tests/qemu-iotests/161
+++ b/tests/qemu-iotests/161
@@ -1,6 +1,6 @@
 #!/bin/bash
 #
-# Test reopening a backing image after block-stream
+# Test reopening a backing image after block-stream and block-commit
 #
 # Copyright (C) 2018 Igalia, S.L.
 #
@@ -98,6 +98,39 @@ _send_qemu_cmd $QEMU_HANDLE \
 
 _cleanup_qemu
 
+# Third test: commit $TEST_IMG.int into $TEST_IMG.base and then reopen
+# $TEST.IMG changing the detect-zeroes option on its new backing file
+# ($TEST_IMG.base).
+echo
+echo "*** Commit and then change an option on the backing file"
+echo
+# Create the images again
+TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE | _filter_imgfmt
+TEST_IMG="$TEST_IMG.int" _make_test_img -b "$TEST_IMG.base" | _filter_imgfmt
+_make_test_img -b "$TEST_IMG.int" | _filter_imgfmt
+
+_launch_qemu -drive if=none,file="${TEST_IMG}"
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'qmp_capabilities' }" \
+    'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'block-commit', \
+       'arguments': { 'device': 'none0',
+                      'top': '${TEST_IMG}.int' } }" \
+    'return'
+
+# Wait for block-commit to finish
+sleep 0.5
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'human-monitor-command',
+       'arguments': { 'command-line':
+                      'qemu-io none0 \"reopen -o backing.detect-zeroes=on\"' } }" \
+    "return"
+
+_cleanup_qemu
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/161.out b/tests/qemu-iotests/161.out
index a3474717a2..39951993ee 100644
--- a/tests/qemu-iotests/161.out
+++ b/tests/qemu-iotests/161.out
@@ -20,4 +20,20 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "none0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "none0"}}
 {"return": ""}
+
+*** Commit and then change an option on the backing file
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1048576
+Formatting 'TEST_DIR/t.IMGFMT.int', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.int
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "none0"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "none0"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "none0"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "none0"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "none0", "len": 1048576, "offset": 1048576, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "none0"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "none0"}}
+{"return": ""}
 *** done
-- 
cgit v1.2.3