[PULL 0/7] Block layer patches

[PULL 0/7] Block layer patches

[Kevin Wolf]

The following changes since commit 60205b71421cbc529ca60b12c79e0eeace007319:

  Merge tag 'pull-aspeed-20220801' of https://github.com/legoater/qemu into staging (2022-08-01 13:55:11 -0700)

are available in the Git repository at:

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

for you to fetch changes up to 21b1d974595b3986c68fe80a1f7e9b87886d4bae:

  main loop: add missing documentation links to GS/IO macros (2022-08-02 12:02:17 +0200)

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

- libvduse: Coverity fixes
- hd-geometry: Fix ignored bios-chs-trans setting
- io_uring: Fix compiler warning (missing #include)
- main loop: add missing documentation links to GS/IO macros
- qemu-iotests: Discard stderr when probing devices

----------------------------------------------------------------
Cole Robinson (1):
      qemu-iotests: Discard stderr when probing devices

Emanuele Giuseppe Esposito (1):
      main loop: add missing documentation links to GS/IO macros

Jinhao Fan (1):
      block/io_uring: add missing include file

Lev Kujawski (1):
      hw/block/hd-geometry: Do not override specified bios-chs-trans

Xie Yongji (3):
      libvduse: Fix the incorrect function name
      libvduse: Replace strcpy() with strncpy()
      libvduse: Pass positive value to strerror()

 include/qemu/main-loop.h        | 18 +++++++++++++++---
 block/io_uring.c                |  1 +
 hw/block/hd-geometry.c          |  7 ++++++-
 subprojects/libvduse/libvduse.c | 13 +++++++------
 tests/qemu-iotests/common.rc    |  4 ++--
 5 files changed, 31 insertions(+), 12 deletions(-)

[PULL 1/7] block/io_uring: add missing include file

[Kevin Wolf]

From: Jinhao Fan <fanjinhao21s@ict.ac.cn>

The commit "Use io_uring_register_ring_fd() to skip fd operations" uses
warn_report but did not include the header file "qemu/error-report.h".
This causes "error: implicit declaration of function ‘warn_report’".
Include this header file.

Fixes: e2848bc574 ("Use io_uring_register_ring_fd() to skip fd operations")
Signed-off-by: Jinhao Fan <fanjinhao21s@ict.ac.cn>
Message-Id: <20220721065645.577404-1-fanjinhao21s@ict.ac.cn>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/io_uring.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/io_uring.c b/block/io_uring.c
index f8a19fd97f..a1760152e0 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -11,6 +11,7 @@
 #include "qemu/osdep.h"
 #include <liburing.h>
 #include "block/aio.h"
+#include "qemu/error-report.h"
 #include "qemu/queue.h"
 #include "block/block.h"
 #include "block/raw-aio.h"
-- 
2.35.3

[PULL 2/7] libvduse: Fix the incorrect function name

[Kevin Wolf]

From: Xie Yongji <xieyongji@bytedance.com>

In vduse_name_is_valid(), we actually check whether
the name is invalid or not. So let's change the
function name to vduse_name_is_invalid() to match
the behavior.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20220706095624.328-2-xieyongji@bytedance.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 subprojects/libvduse/libvduse.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index 9a2bcec282..6374933881 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -1193,7 +1193,7 @@ static int vduse_dev_init(VduseDev *dev, const char *name,
     return 0;
 }
 
-static inline bool vduse_name_is_valid(const char *name)
+static inline bool vduse_name_is_invalid(const char *name)
 {
     return strlen(name) >= VDUSE_NAME_MAX || strstr(name, "..");
 }
@@ -1242,7 +1242,7 @@ VduseDev *vduse_dev_create_by_name(const char *name, uint16_t num_queues,
     VduseDev *dev;
     int ret;
 
-    if (!name || vduse_name_is_valid(name) || !ops ||
+    if (!name || vduse_name_is_invalid(name) || !ops ||
         !ops->enable_queue || !ops->disable_queue) {
         fprintf(stderr, "Invalid parameter for vduse\n");
         return NULL;
@@ -1276,7 +1276,7 @@ VduseDev *vduse_dev_create(const char *name, uint32_t device_id,
     struct vduse_dev_config *dev_config;
     size_t size = offsetof(struct vduse_dev_config, config);
 
-    if (!name || vduse_name_is_valid(name) ||
+    if (!name || vduse_name_is_invalid(name) ||
         !has_feature(features,  VIRTIO_F_VERSION_1) || !config ||
         !config_size || !ops || !ops->enable_queue || !ops->disable_queue) {
         fprintf(stderr, "Invalid parameter for vduse\n");
-- 
2.35.3

[PULL 3/7] libvduse: Replace strcpy() with strncpy()

[Kevin Wolf]

From: Xie Yongji <xieyongji@bytedance.com>

Coverity reported a string overflow issue since we copied
"name" to "dev_config->name" without checking the length.
This should be a false positive since we already checked
the length of "name" in vduse_name_is_invalid(). But anyway,
let's replace strcpy() with strncpy() (as a general library,
we'd like to minimize dependencies on other libraries, so we
didn't use g_strlcpy() here) to fix the coverity complaint.

Fixes: Coverity CID 1490224
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20220706095624.328-3-xieyongji@bytedance.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 subprojects/libvduse/libvduse.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index 6374933881..1e36227388 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -1309,7 +1309,8 @@ VduseDev *vduse_dev_create(const char *name, uint32_t device_id,
         goto err_dev;
     }
 
-    strcpy(dev_config->name, name);
+    strncpy(dev_config->name, name, VDUSE_NAME_MAX);
+    dev_config->name[VDUSE_NAME_MAX - 1] = '\0';
     dev_config->device_id = device_id;
     dev_config->vendor_id = vendor_id;
     dev_config->features = features;
-- 
2.35.3

[PULL 4/7] libvduse: Pass positive value to strerror()

[Kevin Wolf]

From: Xie Yongji <xieyongji@bytedance.com>

The value passed to strerror() should be positive.
So let's fix it.

Fixes: Coverity CID 1490226, 1490223
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20220706095624.328-4-xieyongji@bytedance.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 subprojects/libvduse/libvduse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index 1e36227388..1a5981445c 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -1257,7 +1257,7 @@ VduseDev *vduse_dev_create_by_name(const char *name, uint16_t num_queues,
     ret = vduse_dev_init(dev, name, num_queues, ops, priv);
     if (ret < 0) {
         fprintf(stderr, "Failed to init vduse device %s: %s\n",
-                name, strerror(ret));
+                name, strerror(-ret));
         free(dev);
         return NULL;
     }
@@ -1331,7 +1331,7 @@ VduseDev *vduse_dev_create(const char *name, uint32_t device_id,
     ret = vduse_dev_init(dev, name, num_queues, ops, priv);
     if (ret < 0) {
         fprintf(stderr, "Failed to init vduse device %s: %s\n",
-                name, strerror(ret));
+                name, strerror(-ret));
         goto err;
     }
 
-- 
2.35.3

[PULL 5/7] hw/block/hd-geometry: Do not override specified bios-chs-trans

[Kevin Wolf]

From: Lev Kujawski <lkujaw@member.fsf.org>

For small disk images (<4 GiB), QEMU and SeaBIOS default to the
LARGE/ECHS disk translation method, but it is not uncommon for other
BIOS software to use LBA in these cases as well.  Some operating
system boot loaders (e.g., NT 4) do not handle LARGE translations
outside of fixed configurations.  See, e.g., Q154052:

"When starting an x86 based computer, Ntdetect.com retrieves and
stores Interrupt 13 information. . . If the disk controller is using a
32 sector/64 head translation scheme, this boundary will be 1 GB. If
the controller uses 63 sector/255 head translation [AUTHOR: i.e.,
LBA], the limit will be 4 GB."

To accommodate these situations, hd_geometry_guess() now follows the
disk translation specified by the user even when the ATA disk geometry
is guessed.

hd_geometry_guess():
* Only set the disk translation when translation is AUTO.
* Show the soon-to-be active translation (*ptrans) in the trace rather
  than what was guessed.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/56
Buglink: https://bugs.launchpad.net/qemu/+bug/1745312

Signed-off-by: Lev Kujawski <lkujaw@member.fsf.org>
Message-Id: <20220707204045.999544-1-lkujaw@member.fsf.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/hd-geometry.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 112094358e..dae13ab14d 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -150,7 +150,12 @@ void hd_geometry_guess(BlockBackend *blk,
         translation = BIOS_ATA_TRANSLATION_NONE;
     }
     if (ptrans) {
-        *ptrans = translation;
+        if (*ptrans == BIOS_ATA_TRANSLATION_AUTO) {
+            *ptrans = translation;
+        } else {
+            /* Defer to the translation specified by the user.  */
+            translation = *ptrans;
+        }
     }
     trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs, translation);
 }
-- 
2.35.3

[PULL 6/7] qemu-iotests: Discard stderr when probing devices

[Kevin Wolf]

From: Cole Robinson <crobinso@redhat.com>

qemu-iotests fails in the following setup:

  ./configure --enable-modules --enable-smartcard \
      --target-list=x86_64-softmmu,s390x-softmmu
  make
  cd build
  QEMU_PROG=`pwd`/s390x-softmmu/qemu-system-s390x \
      ../tests/check-block.sh qcow2
  ...
  --- /home/crobinso/src/qemu/tests/qemu-iotests/127.out
  +++ /home/crobinso/src/qemu/build/tests/qemu-iotests/scratch/127.out.bad
  @@ -1,4 +1,18 @@
   QA output created by 127
  +Failed to open module: /home/crobinso/src/qemu/build/hw-usb-smartcard.so: undefined symbol: ccid_card_ccid_attach
  ...
  --- /home/crobinso/src/qemu/tests/qemu-iotests/267.out
  +++ /home/crobinso/src/qemu/build/tests/qemu-iotests/scratch/267.out.bad
  @@ -1,4 +1,11 @@
   QA output created by 267
  +Failed to open module: /home/crobinso/src/qemu/build/hw-usb-smartcard.so: undefined symbol: ccid_card_ccid_attach

The stderr spew is its own known issue, but seems like iotests should
be discarding stderr in this case.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/common.rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 165b54a61e..db757025cb 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -982,7 +982,7 @@ _require_large_file()
 #
 _require_devices()
 {
-    available=$($QEMU -M none -device help | \
+    available=$($QEMU -M none -device help 2> /dev/null | \
                 grep ^name | sed -e 's/^name "//' -e 's/".*$//')
     for device
     do
@@ -994,7 +994,7 @@ _require_devices()
 
 _require_one_device_of()
 {
-    available=$($QEMU -M none -device help | \
+    available=$($QEMU -M none -device help 2> /dev/null | \
                 grep ^name | sed -e 's/^name "//' -e 's/".*$//')
     for device
     do
-- 
2.35.3

[PULL 7/7] main loop: add missing documentation links to GS/IO macros

[Kevin Wolf]

From: Emanuele Giuseppe Esposito <eesposit@redhat.com>

If we go directly to GLOBAL_STATE_CODE, IO_CODE or IO_OR_GS_CODE
definition, we just find that they "mark and check that the function
is part of the {category} API".
However, ther is no definition on what {category} API is, they are
in include/block/block-*.h
Therefore, add a comment that refers to such documentation.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20220609122206.1016936-1-eesposit@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/qemu/main-loop.h | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 5518845299..c50d1b7e3a 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -279,7 +279,11 @@ bool qemu_mutex_iothread_locked(void);
  */
 bool qemu_in_main_thread(void);
 
-/* Mark and check that the function is part of the global state API. */
+/*
+ * Mark and check that the function is part of the Global State API.
+ * Please refer to include/block/block-global-state.h for more
+ * information about GS API.
+ */
 #ifdef CONFIG_COCOA
 /*
  * When using the Cocoa UI, addRemovableDevicesMenuItems() is called from
@@ -298,13 +302,21 @@ bool qemu_in_main_thread(void);
     } while (0)
 #endif /* CONFIG_COCOA */
 
-/* Mark and check that the function is part of the I/O API. */
+/*
+ * Mark and check that the function is part of the I/O API.
+ * Please refer to include/block/block-io.h for more
+ * information about IO API.
+ */
 #define IO_CODE()                                                   \
     do {                                                            \
         /* nop */                                                   \
     } while (0)
 
-/* Mark and check that the function is part of the "I/O OR GS" API. */
+/*
+ * Mark and check that the function is part of the "I/O OR GS" API.
+ * Please refer to include/block/block-io.h for more
+ * information about "IO or GS" API.
+ */
 #define IO_OR_GS_CODE()                                             \
     do {                                                            \
         /* nop */                                                   \
-- 
2.35.3

Re: [PULL 0/7] Block layer patches

[Richard Henderson]

On 8/2/22 06:37, Kevin Wolf wrote:
> The following changes since commit 60205b71421cbc529ca60b12c79e0eeace007319:
> 
>    Merge tag 'pull-aspeed-20220801' of https://github.com/legoater/qemu into staging (2022-08-01 13:55:11 -0700)
> 
> are available in the Git repository at:
> 
>    git://repo.or.cz/qemu/kevin.git tags/for-upstream
> 
> for you to fetch changes up to 21b1d974595b3986c68fe80a1f7e9b87886d4bae:
> 
>    main loop: add missing documentation links to GS/IO macros (2022-08-02 12:02:17 +0200)
> 
> ----------------------------------------------------------------
> Block layer patches
> 
> - libvduse: Coverity fixes
> - hd-geometry: Fix ignored bios-chs-trans setting
> - io_uring: Fix compiler warning (missing #include)
> - main loop: add missing documentation links to GS/IO macros
> - qemu-iotests: Discard stderr when probing devices

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate.


r~


> 
> ----------------------------------------------------------------
> Cole Robinson (1):
>        qemu-iotests: Discard stderr when probing devices
> 
> Emanuele Giuseppe Esposito (1):
>        main loop: add missing documentation links to GS/IO macros
> 
> Jinhao Fan (1):
>        block/io_uring: add missing include file
> 
> Lev Kujawski (1):
>        hw/block/hd-geometry: Do not override specified bios-chs-trans
> 
> Xie Yongji (3):
>        libvduse: Fix the incorrect function name
>        libvduse: Replace strcpy() with strncpy()
>        libvduse: Pass positive value to strerror()
> 
>   include/qemu/main-loop.h        | 18 +++++++++++++++---
>   block/io_uring.c                |  1 +
>   hw/block/hd-geometry.c          |  7 ++++++-
>   subprojects/libvduse/libvduse.c | 13 +++++++------
>   tests/qemu-iotests/common.rc    |  4 ++--
>   5 files changed, 31 insertions(+), 12 deletions(-)
> 
>