[Qemu-devel] [PULL 0/4] Block layer and multiboot test patches

[Qemu-devel] [PULL 0/4] Block layer and multiboot test patches

[Kevin Wolf]

The following changes since commit ee9545ed1543020fba52fa5fb8f2b71c63e5389f:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2019-07-30 09:43:32 +0100)

are available in the Git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 7cef3d1290c9d675deff95029ba78e51fb727125:

  scsi-cd: Fix inserting read-only media in empty drive (2019-07-30 13:21:38 +0200)

----------------------------------------------------------------
Block layer patches:

- scsi-cd: Fix inserting read-only media in empty drive
- block/copy-on-read: Fix permissions for inactive node
- Test case fixes

----------------------------------------------------------------
Andrey Shinkevich (1):
      Fixes: add read-zeroes to 051.out

Kevin Wolf (3):
      tests/multiboot: Fix load address of test kernels
      block/copy-on-read: Fix permissions for inactive node
      scsi-cd: Fix inserting read-only media in empty drive

 block/copy-on-read.c       | 16 +++++++---------
 hw/scsi/scsi-disk.c        | 10 ++++++++--
 tests/multiboot/link.ld    |  6 +++---
 tests/qemu-iotests/051.out | 10 +++++-----
 4 files changed, 23 insertions(+), 19 deletions(-)

[Qemu-devel] [PULL 1/4] tests/multiboot: Fix load address of test kernels

[Kevin Wolf]

While older toolchains produced binaries where the physical load address
of ELF segments was the same as the virtual address, newer versions seem
to choose a different physical address if it isn't specified explicitly.
The means that the test kernel doesn't use the right addresses to access
e.g. format strings any more and the whole output disappears, causing
all test cases to fail.

Fix this by specifying the physical load address of sections explicitly.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/multiboot/link.ld | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/multiboot/link.ld b/tests/multiboot/link.ld
index 3d49b58c60..2eafcffc4f 100644
--- a/tests/multiboot/link.ld
+++ b/tests/multiboot/link.ld
@@ -3,14 +3,14 @@ ENTRY(_start)
 SECTIONS
 {
     . = 0x100000;
-    .text : {
+    .text : AT(ADDR(.text)) {
         *(multiboot)
         *(.text)
     }
-    .data ALIGN(4096) : {
+    .data ALIGN(4096) : AT(ADDR(.data)) {
         *(.data)
     }
-    .rodata ALIGN(4096) : {
+    .rodata ALIGN(4096) : AT(ADDR(.rodata)) {
         *(.rodata)
     }
     .bss ALIGN(4096) : {
-- 
2.20.1

[Qemu-devel] [PULL 2/4] Fixes: add read-zeroes to 051.out

[Kevin Wolf]

From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

The patch "iotests: Set read-zeroes on in null block driver for Valgrind"
with the commit ID a6862418fec4072 needs the change in 051.out when
compared against on the s390 system.

Fixes: a6862418fec40727b392c86dc13d9ec980efcb15
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/051.out | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 8993835b94..554c5ca90a 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -149,23 +149,23 @@ QEMU X.Y.Z monitor - type 'help' for more information
 
 === Cache modes ===
 
-Testing: -drive driver=null-co,cache=none
+Testing: -drive driver=null-co,read-zeroes=on,cache=none
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
-Testing: -drive driver=null-co,cache=directsync
+Testing: -drive driver=null-co,read-zeroes=on,cache=directsync
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
-Testing: -drive driver=null-co,cache=writeback
+Testing: -drive driver=null-co,read-zeroes=on,cache=writeback
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
-Testing: -drive driver=null-co,cache=writethrough
+Testing: -drive driver=null-co,read-zeroes=on,cache=writethrough
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
-Testing: -drive driver=null-co,cache=unsafe
+Testing: -drive driver=null-co,read-zeroes=on,cache=unsafe
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
-- 
2.20.1

[Qemu-devel] [PULL 3/4] block/copy-on-read: Fix permissions for inactive node

[Kevin Wolf]

The copy-on-read drive must not request the WRITE_UNCHANGED permission
for its child if the node is inactive, otherwise starting a migration
destination with -incoming will fail because the child cannot provide
write access yet:

  qemu-system-x86_64: -blockdev copy-on-read,file=img,node-name=cor: Block node is read-only

Earlier QEMU versions additionally ran into an abort() on the migration
source side: bdrv_inactivate_recurse() failed to update permissions.
This is silently ignored today because it was only supposed to loosen
restrictions. This is the symptom that was originally reported here:

  https://bugzilla.redhat.com/show_bug.cgi?id=1733022

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/copy-on-read.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 22f24fd0db..6631f30205 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -56,16 +56,14 @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild *c,
                            uint64_t perm, uint64_t shared,
                            uint64_t *nperm, uint64_t *nshared)
 {
-    if (c == NULL) {
-        *nperm = (perm & PERM_PASSTHROUGH) | BLK_PERM_WRITE_UNCHANGED;
-        *nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
-        return;
-    }
+    *nperm = perm & PERM_PASSTHROUGH;
+    *nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
 
-    *nperm = (perm & PERM_PASSTHROUGH) |
-             (c->perm & PERM_UNCHANGED);
-    *nshared = (shared & PERM_PASSTHROUGH) |
-               (c->shared_perm & PERM_UNCHANGED);
+    /* We must not request write permissions for an inactive node, the child
+     * cannot provide it. */
+    if (!(bs->open_flags & BDRV_O_INACTIVE)) {
+        *nperm |= BLK_PERM_WRITE_UNCHANGED;
+    }
 }
 
 
-- 
2.20.1

[Qemu-devel] [PULL 4/4] scsi-cd: Fix inserting read-only media in empty drive

[Kevin Wolf]

scsi-disks decides whether it has a read-only device by looking at
whether the BlockBackend specified as drive=... is read-only. In the
case of an anonymous BlockBackend (with a node name specified in
drive=...), this is the read-only flag of the attached node. In the case
of an empty anonymous BlockBackend, it's always read-write because
nothing prevented it from being read-write.

This is a problem because scsi-cd would take write permissions on the
anonymous BlockBackend of an empty drive created without a drive=...
option. Using blockdev-insert-medium with a read-only node fails then
with the error message "Block node is read-only".

Fix scsi_realize() so that scsi-cd devices always take read-only
permissions on their BlockBackend instead.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1733920
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi/scsi-disk.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 8e95e3e38d..af3e622dc5 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2318,6 +2318,7 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
 static void scsi_realize(SCSIDevice *dev, Error **errp)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+    bool read_only;
 
     if (!s->qdev.conf.blk) {
         error_setg(errp, "drive property not set");
@@ -2351,8 +2352,13 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
             return;
         }
     }
-    if (!blkconf_apply_backend_options(&dev->conf,
-                                       blk_is_read_only(s->qdev.conf.blk),
+
+    read_only = blk_is_read_only(s->qdev.conf.blk);
+    if (dev->type == TYPE_ROM) {
+        read_only = true;
+    }
+
+    if (!blkconf_apply_backend_options(&dev->conf, read_only,
                                        dev->type == TYPE_DISK, errp)) {
         return;
     }
-- 
2.20.1

Re: [Qemu-devel] [PULL 0/4] Block layer and multiboot test patches

[Peter Maydell]

On Tue, 30 Jul 2019 at 12:24, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit ee9545ed1543020fba52fa5fb8f2b71c63e5389f:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2019-07-30 09:43:32 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 7cef3d1290c9d675deff95029ba78e51fb727125:
>
>   scsi-cd: Fix inserting read-only media in empty drive (2019-07-30 13:21:38 +0200)
>
> ----------------------------------------------------------------
> Block layer patches:
>
> - scsi-cd: Fix inserting read-only media in empty drive
> - block/copy-on-read: Fix permissions for inactive node
> - Test case fixes
>
> ----------------------------------------------------------------
> Andrey Shinkevich (1):
>       Fixes: add read-zeroes to 051.out
>
> Kevin Wolf (3):
>       tests/multiboot: Fix load address of test kernels
>       block/copy-on-read: Fix permissions for inactive node
>       scsi-cd: Fix inserting read-only media in empty drive



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM