qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23
@ 2021-06-25 14:17 Paolo Bonzini
  2021-06-25 14:17 ` [PULL 01/28] target/i386: kvm: add support for TSC scaling Paolo Bonzini
                   ` (28 more replies)
  0 siblings, 29 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:17 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit ecba223da6215d6f6ce2d343b70b2e9a19bfb90b:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210624' into staging (2021-06-24 15:00:34 +0100)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 0aebebb561c9c23b9c6d3d58040f83547f059b5c:

  machine: reject -smp dies!=1 for non-PC machines (2021-06-25 16:16:11 +0200)

Starting to flush my own queue before 6.1 soft freeze.  The
block/file-posix.c patches have been reviewed/acked by Max.

----------------------------------------------------------------
* Some Meson test conversions
* KVM dirty page ring buffer fix
* KVM TSC scaling support
* Fixes for SG_IO with /dev/sdX devices
* (Non)support for host devices on iOS
* -smp cleanups

----------------------------------------------------------------
Joelle van Dyne (3):
      block: feature detection for host block support
      block: check for sys/disk.h
      block: detect DKIOCGETBLOCKCOUNT/SIZE before use

Paolo Bonzini (24):
      target/i386: kvm: add support for TSC scaling
      meson: drop unused CONFIG_GCRYPT_HMAC
      configure: drop unused variables for xts
      meson: remove preadv from summary
      tests: remove QCRYPTO_HAVE_TLS_TEST_SUPPORT
      configure, meson: convert crypto detection to meson
      configure, meson: convert libtasn1 detection to meson
      configure, meson: convert pam detection to meson
      configure, meson: convert libusb detection to meson
      configure, meson: convert libcacard detection to meson
      configure, meson: convert libusbredir detection to meson
      file-posix: fix max_iov for /dev/sg devices
      scsi-generic: pass max_segments via max_iov field in BlockLimits
      osdep: provide ROUND_DOWN macro
      block-backend: align max_transfer to request alignment
      block: add max_hw_transfer to BlockLimits
      file-posix: try BLKSECTGET on block devices too, do not round to power of 2
      block: try BSD disk size ioctls one after another
      file-posix: handle EINTR during ioctl
      machine: move dies from X86MachineState to CpuTopology
      machine: move common smp_parse code to caller
      machine: add error propagation to mc->smp_parse
      machine: pass QAPI struct to mc->smp_parse
      machine: reject -smp dies!=1 for non-PC machines

Peter Xu (1):
      KVM: Fix dirty ring mmap incorrect size due to renaming accident

 accel/kvm/kvm-all.c                   |   4 +-
 authz/meson.build                     |   2 +-
 block.c                               |   2 +-
 block/block-backend.c                 |  19 +-
 block/file-posix.c                    | 144 +++++++++------
 block/io.c                            |   2 +
 configure                             | 326 +++-------------------------------
 crypto/meson.build                    |  41 ++---
 hw/core/machine.c                     | 140 +++++++++------
 hw/i386/pc.c                          | 110 +++++-------
 hw/i386/x86.c                         |  15 +-
 hw/scsi/scsi-generic.c                |   6 +-
 hw/usb/meson.build                    |   6 +-
 include/block/block_int.h             |   7 +
 include/hw/boards.h                   |   3 +-
 include/hw/i386/pc.h                  |   3 -
 include/hw/i386/x86.h                 |   1 -
 include/qemu/osdep.h                  |  28 ++-
 include/sysemu/block-backend.h        |   1 +
 meson.build                           | 159 ++++++++++++-----
 meson_options.txt                     |  14 ++
 qapi/block-core.json                  |  14 +-
 qapi/machine.json                     |  28 +++
 target/i386/cpu.c                     |   2 +-
 target/i386/cpu.h                     |   1 +
 target/i386/kvm/kvm.c                 |  12 +-
 tests/unit/crypto-tls-psk-helpers.c   |   6 -
 tests/unit/crypto-tls-psk-helpers.h   |   4 -
 tests/unit/crypto-tls-x509-helpers.c  |   4 -
 tests/unit/crypto-tls-x509-helpers.h  |  11 +-
 tests/unit/meson.build                |  10 +-
 tests/unit/pkix_asn1_tab.c            |   3 -
 tests/unit/test-crypto-tlscredsx509.c |  12 --
 tests/unit/test-crypto-tlssession.c   |  12 --
 tests/unit/test-io-channel-tls.c      |  12 --
 35 files changed, 501 insertions(+), 663 deletions(-)
-- 
2.31.1



^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PULL 01/28] target/i386: kvm: add support for TSC scaling
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
@ 2021-06-25 14:17 ` Paolo Bonzini
  2021-06-25 14:17 ` [PULL 02/28] meson: drop unused CONFIG_GCRYPT_HMAC Paolo Bonzini
                   ` (27 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:17 UTC (permalink / raw)
  To: qemu-devel

Linux 5.14 will add support for nested TSC scaling.  Add the
corresponding feature in QEMU; to keep support for existing kernels,
do not add it to any processor yet.

The handling of the VMCS enumeration MSR is ugly; once we have more than
one case, we may want to add a table to check VMX features against.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c     |  2 +-
 target/i386/cpu.h     |  1 +
 target/i386/kvm/kvm.c | 12 ++++++++----
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a9fe1662d3..d8f3ab3192 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1031,7 +1031,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
             "vmx-invpcid-exit", "vmx-vmfunc", "vmx-shadow-vmcs", "vmx-encls-exit",
             "vmx-rdseed-exit", "vmx-pml", NULL, NULL,
             "vmx-xsaves", NULL, NULL, NULL,
-            NULL, NULL, NULL, NULL,
+            NULL, "vmx-tsc-scaling", NULL, NULL,
             NULL, NULL, NULL, NULL,
         },
         .msr = {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1e11071d81..f7fa5870b1 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -972,6 +972,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
 #define VMX_SECONDARY_EXEC_RDSEED_EXITING           0x00010000
 #define VMX_SECONDARY_EXEC_ENABLE_PML               0x00020000
 #define VMX_SECONDARY_EXEC_XSAVES                   0x00100000
+#define VMX_SECONDARY_EXEC_TSC_SCALING              0x02000000
 
 #define VMX_PIN_BASED_EXT_INTR_MASK                 0x00000001
 #define VMX_PIN_BASED_NMI_EXITING                   0x00000008
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index ad950c3c27..04e4ec063f 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2700,8 +2700,6 @@ static uint64_t make_vmx_msr_value(uint32_t index, uint32_t features)
     return must_be_one | (((uint64_t)can_be_one) << 32);
 }
 
-#define VMCS12_MAX_FIELD_INDEX (0x17)
-
 static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
 {
     uint64_t kvm_vmx_basic =
@@ -2791,8 +2789,14 @@ static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
                       CR0_PE_MASK | CR0_PG_MASK | CR0_NE_MASK);
     kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR4_FIXED0,
                       CR4_VMXE_MASK);
-    kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMCS_ENUM,
-                      VMCS12_MAX_FIELD_INDEX << 1);
+
+    if (f[FEAT_VMX_SECONDARY_CTLS] & VMX_SECONDARY_EXEC_TSC_SCALING) {
+        /* TSC multiplier (0x2032).  */
+        kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMCS_ENUM, 0x32);
+    } else {
+        /* Preemption timer (0x482E).  */
+        kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMCS_ENUM, 0x2E);
+    }
 }
 
 static void kvm_msr_entry_add_perf(X86CPU *cpu, FeatureWordArray f)
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 02/28] meson: drop unused CONFIG_GCRYPT_HMAC
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
  2021-06-25 14:17 ` [PULL 01/28] target/i386: kvm: add support for TSC scaling Paolo Bonzini
@ 2021-06-25 14:17 ` Paolo Bonzini
  2021-06-25 14:17 ` [PULL 03/28] configure: drop unused variables for xts Paolo Bonzini
                   ` (26 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé, Richard Henderson

CONFIG_GCRYPT_HMAC has been removed now that all supported distros have it.

Reviewed-by: Richard Henderson <richard.henderson@liaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/meson.build b/meson.build
index d8a92666fb..87147a5f3f 100644
--- a/meson.build
+++ b/meson.build
@@ -2664,7 +2664,6 @@ summary_info += {'GNUTLS support':    config_host.has_key('CONFIG_GNUTLS')}
 # TODO: add back version
 summary_info += {'libgcrypt':         config_host.has_key('CONFIG_GCRYPT')}
 if config_host.has_key('CONFIG_GCRYPT')
-   summary_info += {'  hmac':            config_host.has_key('CONFIG_GCRYPT_HMAC')}
    summary_info += {'  XTS':             not config_host.has_key('CONFIG_QEMU_PRIVATE_XTS')}
 endif
 # TODO: add back version
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 03/28] configure: drop unused variables for xts
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
  2021-06-25 14:17 ` [PULL 01/28] target/i386: kvm: add support for TSC scaling Paolo Bonzini
  2021-06-25 14:17 ` [PULL 02/28] meson: drop unused CONFIG_GCRYPT_HMAC Paolo Bonzini
@ 2021-06-25 14:17 ` Paolo Bonzini
  2021-06-25 14:17 ` [PULL 04/28] meson: remove preadv from summary Paolo Bonzini
                   ` (25 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P . Berrangé, Philippe Mathieu-Daudé, Richard Henderson

All XTS configuration uses qemu_private_xts.  Drop other variables as
they have only ever been used to generate the summary (which has since
been moved to meson.build).

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@liaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/configure b/configure
index 38704b4e11..00e7dd749a 100755
--- a/configure
+++ b/configure
@@ -406,9 +406,7 @@ gtk="auto"
 tls_priority="NORMAL"
 gnutls="$default_feature"
 nettle="$default_feature"
-nettle_xts="no"
 gcrypt="$default_feature"
-gcrypt_xts="no"
 qemu_private_xts="yes"
 auth_pam="$default_feature"
 vte="$default_feature"
@@ -2897,7 +2895,6 @@ int main(void) {
 }
 EOF
         if compile_prog "$nettle_cflags" "$nettle_libs" ; then
-            nettle_xts=yes
             qemu_private_xts=no
         fi
     fi
@@ -2938,7 +2935,6 @@ int main(void) {
 }
 EOF
         if compile_prog "$gcrypt_cflags" "$gcrypt_libs" ; then
-            gcrypt_xts=yes
             qemu_private_xts=no
         fi
     elif test "$gcrypt" = "yes"; then
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 04/28] meson: remove preadv from summary
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2021-06-25 14:17 ` [PULL 03/28] configure: drop unused variables for xts Paolo Bonzini
@ 2021-06-25 14:17 ` Paolo Bonzini
  2021-06-25 14:17 ` [PULL 05/28] tests: remove QCRYPTO_HAVE_TLS_TEST_SUPPORT Paolo Bonzini
                   ` (24 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

Meson is more verbose than the configure script; the outcome of the preadv test
can be found in its output and it is not worth including it again in the summary.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/meson.build b/meson.build
index 87147a5f3f..3809f51f7f 100644
--- a/meson.build
+++ b/meson.build
@@ -2565,7 +2565,6 @@ summary_info += {'PIE':               get_option('b_pie')}
 summary_info += {'static build':      config_host.has_key('CONFIG_STATIC')}
 summary_info += {'malloc trim support': has_malloc_trim}
 summary_info += {'membarrier':        config_host.has_key('CONFIG_MEMBARRIER')}
-summary_info += {'preadv support':    config_host_data.get('CONFIG_PREADV')}
 summary_info += {'fdatasync':         config_host.has_key('CONFIG_FDATASYNC')}
 summary_info += {'madvise':           config_host.has_key('CONFIG_MADVISE')}
 summary_info += {'posix_madvise':     config_host.has_key('CONFIG_POSIX_MADVISE')}
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 05/28] tests: remove QCRYPTO_HAVE_TLS_TEST_SUPPORT
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2021-06-25 14:17 ` [PULL 04/28] meson: remove preadv from summary Paolo Bonzini
@ 2021-06-25 14:17 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 06/28] configure, meson: convert crypto detection to meson Paolo Bonzini
                   ` (23 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

meson.build already decides whether it is possible to build the TLS
test suite.  There is no need to include that in the source as well.
The dummy tests in fact are broken because they do not produce valid
TAP output (empty output is rejected by scripts/tap-driver.pl).

Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/unit/crypto-tls-psk-helpers.c   |  6 ------
 tests/unit/crypto-tls-psk-helpers.h   |  4 ----
 tests/unit/crypto-tls-x509-helpers.c  |  4 ----
 tests/unit/crypto-tls-x509-helpers.h  | 11 +----------
 tests/unit/pkix_asn1_tab.c            |  3 ---
 tests/unit/test-crypto-tlscredsx509.c | 12 ------------
 tests/unit/test-crypto-tlssession.c   | 12 ------------
 tests/unit/test-io-channel-tls.c      | 12 ------------
 8 files changed, 1 insertion(+), 63 deletions(-)

diff --git a/tests/unit/crypto-tls-psk-helpers.c b/tests/unit/crypto-tls-psk-helpers.c
index a8395477c3..7f8a488961 100644
--- a/tests/unit/crypto-tls-psk-helpers.c
+++ b/tests/unit/crypto-tls-psk-helpers.c
@@ -20,14 +20,10 @@
 
 #include "qemu/osdep.h"
 
-/* Include this first because it defines QCRYPTO_HAVE_TLS_TEST_SUPPORT */
 #include "crypto-tls-x509-helpers.h"
-
 #include "crypto-tls-psk-helpers.h"
 #include "qemu/sockets.h"
 
-#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
-
 void test_tls_psk_init(const char *pskfile)
 {
     FILE *fp;
@@ -46,5 +42,3 @@ void test_tls_psk_cleanup(const char *pskfile)
 {
     unlink(pskfile);
 }
-
-#endif /* QCRYPTO_HAVE_TLS_TEST_SUPPORT */
diff --git a/tests/unit/crypto-tls-psk-helpers.h b/tests/unit/crypto-tls-psk-helpers.h
index 5aa9951cb6..faa645c629 100644
--- a/tests/unit/crypto-tls-psk-helpers.h
+++ b/tests/unit/crypto-tls-psk-helpers.h
@@ -23,11 +23,7 @@
 
 #include <gnutls/gnutls.h>
 
-#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
-
 void test_tls_psk_init(const char *keyfile);
 void test_tls_psk_cleanup(const char *keyfile);
 
-#endif /* QCRYPTO_HAVE_TLS_TEST_SUPPORT */
-
 #endif
diff --git a/tests/unit/crypto-tls-x509-helpers.c b/tests/unit/crypto-tls-x509-helpers.c
index 97658592a2..fc609b3fd4 100644
--- a/tests/unit/crypto-tls-x509-helpers.c
+++ b/tests/unit/crypto-tls-x509-helpers.c
@@ -24,8 +24,6 @@
 #include "crypto/init.h"
 #include "qemu/sockets.h"
 
-#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
-
 /*
  * This stores some static data that is needed when
  * encoding extensions in the x509 certs
@@ -504,5 +502,3 @@ void test_tls_discard_cert(QCryptoTLSTestCertReq *req)
         unlink(req->filename);
     }
 }
-
-#endif /* QCRYPTO_HAVE_TLS_TEST_SUPPORT */
diff --git a/tests/unit/crypto-tls-x509-helpers.h b/tests/unit/crypto-tls-x509-helpers.h
index 8fcd7785ab..cf6329e653 100644
--- a/tests/unit/crypto-tls-x509-helpers.h
+++ b/tests/unit/crypto-tls-x509-helpers.h
@@ -23,14 +23,7 @@
 
 #include <gnutls/gnutls.h>
 #include <gnutls/x509.h>
-
-#if !(defined WIN32) && \
-    defined(CONFIG_TASN1)
-# define QCRYPTO_HAVE_TLS_TEST_SUPPORT
-#endif
-
-#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
-# include <libtasn1.h>
+#include <libtasn1.h>
 
 
 /*
@@ -127,6 +120,4 @@ void test_tls_cleanup(const char *keyfile);
 
 extern const asn1_static_node pkix_asn1_tab[];
 
-#endif /* QCRYPTO_HAVE_TLS_TEST_SUPPORT */
-
 #endif
diff --git a/tests/unit/pkix_asn1_tab.c b/tests/unit/pkix_asn1_tab.c
index 15397cf77a..89521408a1 100644
--- a/tests/unit/pkix_asn1_tab.c
+++ b/tests/unit/pkix_asn1_tab.c
@@ -6,8 +6,6 @@
 #include "qemu/osdep.h"
 #include "crypto-tls-x509-helpers.h"
 
-#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
-
 const asn1_static_node pkix_asn1_tab[] = {
   {"PKIX1", 536875024, 0},
   {0, 1073741836, 0},
@@ -1105,4 +1103,3 @@ const asn1_static_node pkix_asn1_tab[] = {
   {0, 1048586, "2"},
   {0, 0, 0}
 };
-#endif /* QCRYPTO_HAVE_TLS_TEST_SUPPORT */
diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
index f487349c32..aab4149b56 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -25,8 +25,6 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 
-#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
-
 #define WORKDIR "tests/test-crypto-tlscredsx509-work/"
 #define KEYFILE WORKDIR "key-ctx.pem"
 
@@ -706,13 +704,3 @@ int main(int argc, char **argv)
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
-
-#else /* ! QCRYPTO_HAVE_TLS_TEST_SUPPORT */
-
-int
-main(void)
-{
-    return EXIT_SUCCESS;
-}
-
-#endif /* ! QCRYPTO_HAVE_TLS_TEST_SUPPORT */
diff --git a/tests/unit/test-crypto-tlssession.c b/tests/unit/test-crypto-tlssession.c
index 8b2453fa79..5f0da9192c 100644
--- a/tests/unit/test-crypto-tlssession.c
+++ b/tests/unit/test-crypto-tlssession.c
@@ -31,8 +31,6 @@
 #include "qemu/sockets.h"
 #include "authz/list.h"
 
-#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
-
 #define WORKDIR "tests/test-crypto-tlssession-work/"
 #define PSKFILE WORKDIR "keys.psk"
 #define KEYFILE WORKDIR "key-ctx.pem"
@@ -648,13 +646,3 @@ int main(int argc, char **argv)
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
-
-#else /* ! QCRYPTO_HAVE_TLS_TEST_SUPPORT */
-
-int
-main(void)
-{
-    return EXIT_SUCCESS;
-}
-
-#endif /* ! QCRYPTO_HAVE_TLS_TEST_SUPPORT */
diff --git a/tests/unit/test-io-channel-tls.c b/tests/unit/test-io-channel-tls.c
index ad7554c534..f6fb988c01 100644
--- a/tests/unit/test-io-channel-tls.c
+++ b/tests/unit/test-io-channel-tls.c
@@ -34,8 +34,6 @@
 #include "authz/list.h"
 #include "qom/object_interfaces.h"
 
-#ifdef QCRYPTO_HAVE_TLS_TEST_SUPPORT
-
 #define WORKDIR "tests/test-io-channel-tls-work/"
 #define KEYFILE WORKDIR "key-ctx.pem"
 
@@ -334,13 +332,3 @@ int main(int argc, char **argv)
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
-
-#else /* ! QCRYPTO_HAVE_TLS_TEST_SUPPORT */
-
-int
-main(void)
-{
-    return EXIT_SUCCESS;
-}
-
-#endif /* ! QCRYPTO_HAVE_TLS_TEST_SUPPORT */
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 06/28] configure, meson: convert crypto detection to meson
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2021-06-25 14:17 ` [PULL 05/28] tests: remove QCRYPTO_HAVE_TLS_TEST_SUPPORT Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 07/28] configure, meson: convert libtasn1 " Paolo Bonzini
                   ` (22 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé, Richard Henderson

Reviewed-by: Richard Henderson <richard.henderson@liaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure              | 188 +++--------------------------------------
 crypto/meson.build     |  41 +++------
 meson.build            |  81 +++++++++++++-----
 meson_options.txt      |   6 ++
 tests/unit/meson.build |   6 +-
 5 files changed, 90 insertions(+), 232 deletions(-)

diff --git a/configure b/configure
index 00e7dd749a..897e968e02 100755
--- a/configure
+++ b/configure
@@ -404,10 +404,9 @@ seccomp="auto"
 glusterfs="auto"
 gtk="auto"
 tls_priority="NORMAL"
-gnutls="$default_feature"
-nettle="$default_feature"
-gcrypt="$default_feature"
-qemu_private_xts="yes"
+gnutls="auto"
+nettle="auto"
+gcrypt="auto"
 auth_pam="$default_feature"
 vte="$default_feature"
 virglrenderer="$default_feature"
@@ -1372,17 +1371,17 @@ for opt do
   ;;
   --tls-priority=*) tls_priority="$optarg"
   ;;
-  --disable-gnutls) gnutls="no"
+  --disable-gnutls) gnutls="disabled"
   ;;
-  --enable-gnutls) gnutls="yes"
+  --enable-gnutls) gnutls="enabled"
   ;;
-  --disable-nettle) nettle="no"
+  --disable-nettle) nettle="disabled"
   ;;
-  --enable-nettle) nettle="yes"
+  --enable-nettle) nettle="enabled"
   ;;
-  --disable-gcrypt) gcrypt="no"
+  --disable-gcrypt) gcrypt="disabled"
   ;;
-  --enable-gcrypt) gcrypt="yes"
+  --enable-gcrypt) gcrypt="enabled"
   ;;
   --disable-auth-pam) auth_pam="no"
   ;;
@@ -2800,156 +2799,6 @@ EOF
   fi
 fi
 
-##########################################
-# GNUTLS probe
-
-if test "$gnutls" != "no"; then
-    pass="no"
-    if $pkg_config --exists "gnutls >= 3.5.18"; then
-        gnutls_cflags=$($pkg_config --cflags gnutls)
-        gnutls_libs=$($pkg_config --libs gnutls)
-        # Packaging for the static libraries is not always correct.
-        # At least ubuntu 18.04 ships only shared libraries.
-        write_c_skeleton
-        if compile_prog "" "$gnutls_libs" ; then
-            pass="yes"
-        fi
-    fi
-    if test "$pass" = "no" && test "$gnutls" = "yes"; then
-	feature_not_found "gnutls" "Install gnutls devel >= 3.1.18"
-    else
-        gnutls="$pass"
-    fi
-fi
-
-
-# If user didn't give a --disable/enable-gcrypt flag,
-# then mark as disabled if user requested nettle
-# explicitly
-if test -z "$gcrypt"
-then
-    if test "$nettle" = "yes"
-    then
-        gcrypt="no"
-    fi
-fi
-
-# If user didn't give a --disable/enable-nettle flag,
-# then mark as disabled if user requested gcrypt
-# explicitly
-if test -z "$nettle"
-then
-    if test "$gcrypt" = "yes"
-    then
-        nettle="no"
-    fi
-fi
-
-has_libgcrypt() {
-    if ! has "libgcrypt-config"
-    then
-	return 1
-    fi
-
-    if test -n "$cross_prefix"
-    then
-	host=$(libgcrypt-config --host)
-	if test "$host-" != $cross_prefix
-	then
-	    return 1
-	fi
-    fi
-
-    maj=`libgcrypt-config --version | awk -F . '{print $1}'`
-    min=`libgcrypt-config --version | awk -F . '{print $2}'`
-
-    if test $maj != 1 || test $min -lt 8
-    then
-       return 1
-    fi
-
-    return 0
-}
-
-
-if test "$nettle" != "no"; then
-    pass="no"
-    if $pkg_config --exists "nettle >= 3.4"; then
-        nettle_cflags=$($pkg_config --cflags nettle)
-        nettle_libs=$($pkg_config --libs nettle)
-        # Link test to make sure the given libraries work (e.g for static).
-        write_c_skeleton
-        if compile_prog "" "$nettle_libs" ; then
-            if test -z "$gcrypt"; then
-               gcrypt="no"
-            fi
-            pass="yes"
-        fi
-    fi
-    if test "$pass" = "yes"
-    then
-        cat > $TMPC << EOF
-#include <nettle/xts.h>
-int main(void) {
-  return 0;
-}
-EOF
-        if compile_prog "$nettle_cflags" "$nettle_libs" ; then
-            qemu_private_xts=no
-        fi
-    fi
-    if test "$pass" = "no" && test "$nettle" = "yes"; then
-        feature_not_found "nettle" "Install nettle devel >= 2.7.1"
-    else
-        nettle="$pass"
-    fi
-fi
-
-if test "$gcrypt" != "no"; then
-    pass="no"
-    if has_libgcrypt; then
-        gcrypt_cflags=$(libgcrypt-config --cflags)
-        gcrypt_libs=$(libgcrypt-config --libs)
-        # Debian has removed -lgpg-error from libgcrypt-config
-        # as it "spreads unnecessary dependencies" which in
-        # turn breaks static builds...
-        if test "$static" = "yes"
-        then
-            gcrypt_libs="$gcrypt_libs -lgpg-error"
-        fi
-
-        # Link test to make sure the given libraries work (e.g for static).
-        write_c_skeleton
-        if compile_prog "" "$gcrypt_libs" ; then
-            pass="yes"
-        fi
-    fi
-    if test "$pass" = "yes"; then
-        gcrypt="yes"
-        cat > $TMPC << EOF
-#include <gcrypt.h>
-int main(void) {
-  gcry_cipher_hd_t handle;
-  gcry_cipher_open(&handle, GCRY_CIPHER_AES, GCRY_CIPHER_MODE_XTS, 0);
-  return 0;
-}
-EOF
-        if compile_prog "$gcrypt_cflags" "$gcrypt_libs" ; then
-            qemu_private_xts=no
-        fi
-    elif test "$gcrypt" = "yes"; then
-        feature_not_found "gcrypt" "Install gcrypt devel >= 1.5.0"
-    else
-        gcrypt="no"
-    fi
-fi
-
-
-if test "$gcrypt" = "yes" && test "$nettle" = "yes"
-then
-    error_exit "Only one of gcrypt & nettle can be enabled"
-fi
-
 ##########################################
 # libtasn1 - only for the TLS creds/session test suite
 
@@ -5705,24 +5554,6 @@ if test "$gdbus_codegen" != "" ; then
     echo "GDBUS_CODEGEN=$gdbus_codegen" >> $config_host_mak
 fi
 echo "CONFIG_TLS_PRIORITY=\"$tls_priority\"" >> $config_host_mak
-if test "$gnutls" = "yes" ; then
-  echo "CONFIG_GNUTLS=y" >> $config_host_mak
-  echo "GNUTLS_CFLAGS=$gnutls_cflags" >> $config_host_mak
-  echo "GNUTLS_LIBS=$gnutls_libs" >> $config_host_mak
-fi
-if test "$gcrypt" = "yes" ; then
-  echo "CONFIG_GCRYPT=y" >> $config_host_mak
-  echo "GCRYPT_CFLAGS=$gcrypt_cflags" >> $config_host_mak
-  echo "GCRYPT_LIBS=$gcrypt_libs" >> $config_host_mak
-fi
-if test "$nettle" = "yes" ; then
-  echo "CONFIG_NETTLE=y" >> $config_host_mak
-  echo "NETTLE_CFLAGS=$nettle_cflags" >> $config_host_mak
-  echo "NETTLE_LIBS=$nettle_libs" >> $config_host_mak
-fi
-if test "$qemu_private_xts" = "yes" ; then
-  echo "CONFIG_QEMU_PRIVATE_XTS=y" >> $config_host_mak
-fi
 if test "$tasn1" = "yes" ; then
   echo "CONFIG_TASN1=y" >> $config_host_mak
 fi
@@ -6439,6 +6270,7 @@ if test "$skip_meson" = no; then
         -Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
         -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
         -Drbd=$rbd -Dlzo=$lzo -Dsnappy=$snappy -Dlzfse=$lzfse \
+        -Dgnutls=$gnutls -Dnettle=$nettle -Dgcrypt=$gcrypt \
         -Dzstd=$zstd -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
         -Dattr=$attr -Ddefault_devices=$default_devices \
         -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
diff --git a/crypto/meson.build b/crypto/meson.build
index af7e80c6f6..7cbf1a6ba7 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -22,48 +22,31 @@ crypto_ss.add(files(
   'tlssession.c',
 ))
 
-if 'CONFIG_NETTLE' in config_host
-  crypto_ss.add(files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'))
-elif 'CONFIG_GCRYPT' in config_host
-  crypto_ss.add(files('hash-gcrypt.c', 'pbkdf-gcrypt.c'))
-  crypto_ss.add(files('hmac-gcrypt.c'))
+if nettle.found()
+  crypto_ss.add(nettle, files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'))
+elif gcrypt.found()
+  crypto_ss.add(gcrypt, files('hash-gcrypt.c', 'hmac-gcrypt.c', 'pbkdf-gcrypt.c'))
 else
   crypto_ss.add(files('hash-glib.c', 'hmac-glib.c', 'pbkdf-stub.c'))
 endif
+if xts == 'private'
+  crypto_ss.add(files('xts.c'))
+endif
 
 crypto_ss.add(when: 'CONFIG_SECRET_KEYRING', if_true: files('secret_keyring.c'))
-crypto_ss.add(when: 'CONFIG_QEMU_PRIVATE_XTS', if_true: files('xts.c'))
 crypto_ss.add(when: 'CONFIG_AF_ALG', if_true: files('afalg.c', 'cipher-afalg.c', 'hash-afalg.c'))
-crypto_ss.add(when: 'CONFIG_GNUTLS', if_true: files('tls-cipher-suites.c'))
-
-if 'CONFIG_NETTLE' in config_host
-  crypto_ss.add(nettle)
-elif 'CONFIG_GCRYPT' in config_host
-  crypto_ss.add(gcrypt)
-endif
-
-if 'CONFIG_GNUTLS' in config_host
-  crypto_ss.add(gnutls)
-endif
-
+crypto_ss.add(when: gnutls, if_true: files('tls-cipher-suites.c'))
 
 util_ss.add(files('aes.c'))
 util_ss.add(files('init.c'))
 
-if 'CONFIG_GCRYPT' in config_host
-  util_ss.add(files('random-gcrypt.c'))
-elif 'CONFIG_GNUTLS' in config_host
-  util_ss.add(files('random-gnutls.c'))
+if gcrypt.found()
+  util_ss.add(gcrypt, files('random-gcrypt.c'))
+elif gnutls.found()
+  util_ss.add(gnutls, files('random-gnutls.c'))
 elif 'CONFIG_RNG_NONE' in config_host
   util_ss.add(files('random-none.c'))
 else
   util_ss.add(files('random-platform.c'))
 endif
 
-if 'CONFIG_GCRYPT' in config_host
-  util_ss.add(gcrypt)
-endif
-
-if 'CONFIG_GNUTLS' in config_host
-  util_ss.add(gnutls)
-endif
diff --git a/meson.build b/meson.build
index 3809f51f7f..286b37aecb 100644
--- a/meson.build
+++ b/meson.build
@@ -320,21 +320,6 @@ urcubp = not_found
 if 'CONFIG_TRACE_UST' in config_host
   urcubp = declare_dependency(link_args: config_host['URCU_BP_LIBS'].split())
 endif
-gcrypt = not_found
-if 'CONFIG_GCRYPT' in config_host
-  gcrypt = declare_dependency(compile_args: config_host['GCRYPT_CFLAGS'].split(),
-                              link_args: config_host['GCRYPT_LIBS'].split())
-endif
-nettle = not_found
-if 'CONFIG_NETTLE' in config_host
-  nettle = declare_dependency(compile_args: config_host['NETTLE_CFLAGS'].split(),
-                              link_args: config_host['NETTLE_LIBS'].split())
-endif
-gnutls = not_found
-if 'CONFIG_GNUTLS' in config_host
-  gnutls = declare_dependency(compile_args: config_host['GNUTLS_CFLAGS'].split(),
-                              link_args: config_host['GNUTLS_LIBS'].split())
-endif
 pixman = not_found
 if have_system or have_tools
   pixman = dependency('pixman-1', required: have_system, version:'>=0.21.8',
@@ -829,6 +814,54 @@ if 'CONFIG_OPENGL' in config_host
                               link_args: config_host['OPENGL_LIBS'].split())
 endif
 
+gnutls = not_found
+if not get_option('gnutls').auto() or have_system
+  gnutls = dependency('gnutls', version: '>=3.5.18',
+                      method: 'pkg-config',
+                      required: get_option('gnutls'),
+                      kwargs: static_kwargs)
+endif
+
+# Nettle has priority over gcrypt
+gcrypt = not_found
+nettle = not_found
+xts = 'private'
+if get_option('nettle').enabled() and get_option('gcrypt').enabled()
+  error('Only one of gcrypt & nettle can be enabled')
+elif (not get_option('nettle').auto() or have_system) and not get_option('gcrypt').enabled()
+  nettle = dependency('nettle', version: '>=3.4',
+                      method: 'pkg-config',
+                      required: get_option('nettle'),
+                      kwargs: static_kwargs)
+  if nettle.found() and cc.has_header('nettle/xts.h', dependencies: nettle)
+    xts = 'nettle'
+  endif
+endif
+if (not get_option('gcrypt').auto() or have_system) and not nettle.found()
+  gcrypt = dependency('libgcrypt', version: '>=1.5',
+                         method: 'config-tool',
+                         required: get_option('gcrypt'),
+                         kwargs: static_kwargs)
+  if gcrypt.found() and cc.compiles('''
+    #include <gcrypt.h>
+    int main(void) {
+      gcry_cipher_hd_t handle;
+      gcry_cipher_open(&handle, GCRY_CIPHER_AES, GCRY_CIPHER_MODE_XTS, 0);
+      return 0;
+    }
+    ''', dependencies: gcrypt)
+    xts = 'gcrypt'
+  endif
+  # Debian has removed -lgpg-error from libgcrypt-config
+  # as it "spreads unnecessary dependencies" which in
+  # turn breaks static builds...
+  if gcrypt.found() and enable_static
+    gcrypt = declare_dependency(dependencies: [
+      gcrypt,
+      cc.find_library('gpg-error', required: true, kwargs: static_kwargs)])
+  endif
+endif
+
 gtk = not_found
 gtkx11 = not_found
 if not get_option('gtk').auto() or (have_system and not cocoa.found())
@@ -1165,6 +1198,10 @@ config_host_data.set('CONFIG_VIRTFS', have_virtfs)
 config_host_data.set('CONFIG_XKBCOMMON', xkbcommon.found())
 config_host_data.set('CONFIG_KEYUTILS', keyutils.found())
 config_host_data.set('CONFIG_GETTID', has_gettid)
+config_host_data.set('CONFIG_GNUTLS', gnutls.found())
+config_host_data.set('CONFIG_GCRYPT', gcrypt.found())
+config_host_data.set('CONFIG_NETTLE', nettle.found())
+config_host_data.set('CONFIG_QEMU_PRIVATE_XTS', xts == 'private')
 config_host_data.set('CONFIG_MALLOC_TRIM', has_malloc_trim)
 config_host_data.set('CONFIG_STATX', has_statx)
 config_host_data.set('CONFIG_ZSTD', zstd.found())
@@ -2659,16 +2696,16 @@ summary(summary_info, bool_yn: true, section: 'Block layer support')
 # Crypto
 summary_info = {}
 summary_info += {'TLS priority':      config_host['CONFIG_TLS_PRIORITY']}
-summary_info += {'GNUTLS support':    config_host.has_key('CONFIG_GNUTLS')}
+summary_info += {'GNUTLS support':    gnutls.found()}
 # TODO: add back version
-summary_info += {'libgcrypt':         config_host.has_key('CONFIG_GCRYPT')}
-if config_host.has_key('CONFIG_GCRYPT')
-   summary_info += {'  XTS':             not config_host.has_key('CONFIG_QEMU_PRIVATE_XTS')}
+summary_info += {'libgcrypt':         gcrypt.found()}
+if gcrypt.found()
+   summary_info += {'  XTS':             xts != 'private'}
 endif
 # TODO: add back version
-summary_info += {'nettle':            config_host.has_key('CONFIG_NETTLE')}
-if config_host.has_key('CONFIG_NETTLE')
-   summary_info += {'  XTS':             not config_host.has_key('CONFIG_QEMU_PRIVATE_XTS')}
+summary_info += {'nettle':            nettle.found()}
+if nettle.found()
+   summary_info += {'  XTS':             xts != 'private'}
 endif
 summary_info += {'crypto afalg':      config_host.has_key('CONFIG_AF_ALG')}
 summary_info += {'rng-none':          config_host.has_key('CONFIG_RNG_NONE')}
diff --git a/meson_options.txt b/meson_options.txt
index 3d304cac96..343ffffb7c 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -76,6 +76,12 @@ option('iconv', type : 'feature', value : 'auto',
        description: 'Font glyph conversion support')
 option('curses', type : 'feature', value : 'auto',
        description: 'curses UI')
+option('gnutls', type : 'feature', value : 'auto',
+       description: 'GNUTLS cryptography support')
+option('nettle', type : 'feature', value : 'auto',
+       description: 'nettle cryptography support')
+option('gcrypt', type : 'feature', value : 'auto',
+       description: 'libgcrypt cryptography support')
 option('libudev', type : 'feature', value : 'auto',
        description: 'Use libudev to enumerate host devices')
 option('lzfse', type : 'feature', value : 'auto',
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index b3bc2109da..fcf6ed2ef5 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -83,7 +83,7 @@ if have_block
     'test-crypto-afsplit': [io],
     'test-crypto-block': [io],
   }
-  if 'CONFIG_GNUTLS' in config_host and \
+  if gnutls.found() and \
      'CONFIG_TASN1' in config_host and \
      'CONFIG_POSIX' in config_host
     tests += {
@@ -97,7 +97,7 @@ if have_block
   if 'CONFIG_AUTH_PAM' in config_host
     tests += {'test-authz-pam': [authz]}
   endif
-  if 'CONFIG_QEMU_PRIVATE_XTS' in config_host
+  if xts == 'private'
     tests += {'test-crypto-xts': [crypto, io]}
   endif
   if 'CONFIG_POSIX' in config_host
@@ -106,7 +106,7 @@ if have_block
   if 'CONFIG_REPLICATION' in config_host
     tests += {'test-replication': [testblock]}
   endif
-  if 'CONFIG_NETTLE' in config_host or 'CONFIG_GCRYPT' in config_host
+  if nettle.found() or gcrypt.found()
     tests += {'test-crypto-pbkdf': [io]}
   endif
   if 'CONFIG_EPOLL_CREATE1' in config_host
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 07/28] configure, meson: convert libtasn1 detection to meson
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 06/28] configure, meson: convert crypto detection to meson Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 08/28] configure, meson: convert pam " Paolo Bonzini
                   ` (21 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé, Richard Henderson

Make it depend on gnutls too, since it is only used as part of gnutls
tests.

Reviewed-by: Richard Henderson <richard.henderson@liaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure              | 19 -------------------
 meson.build            |  9 +++++----
 tests/unit/meson.build |  2 +-
 3 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/configure b/configure
index 897e968e02..3d36eea55f 100755
--- a/configure
+++ b/configure
@@ -2799,20 +2799,6 @@ EOF
   fi
 fi
 
-##########################################
-# libtasn1 - only for the TLS creds/session test suite
-
-tasn1=yes
-tasn1_cflags=""
-tasn1_libs=""
-if $pkg_config --exists "libtasn1"; then
-    tasn1_cflags=$($pkg_config --cflags libtasn1)
-    tasn1_libs=$($pkg_config --libs libtasn1)
-else
-    tasn1=no
-fi
-
-
 ##########################################
 # PAM probe
 
@@ -5554,9 +5540,6 @@ if test "$gdbus_codegen" != "" ; then
     echo "GDBUS_CODEGEN=$gdbus_codegen" >> $config_host_mak
 fi
 echo "CONFIG_TLS_PRIORITY=\"$tls_priority\"" >> $config_host_mak
-if test "$tasn1" = "yes" ; then
-  echo "CONFIG_TASN1=y" >> $config_host_mak
-fi
 if test "$auth_pam" = "yes" ; then
     echo "CONFIG_AUTH_PAM=y" >> $config_host_mak
 fi
@@ -6017,8 +6000,6 @@ echo "LD_I386_EMULATION=$ld_i386_emulation" >> $config_host_mak
 echo "EXESUF=$EXESUF" >> $config_host_mak
 echo "HOST_DSOSUF=$HOST_DSOSUF" >> $config_host_mak
 echo "LIBS_QGA=$libs_qga" >> $config_host_mak
-echo "TASN1_LIBS=$tasn1_libs" >> $config_host_mak
-echo "TASN1_CFLAGS=$tasn1_cflags" >> $config_host_mak
 if test "$gcov" = "yes" ; then
   echo "CONFIG_GCOV=y" >> $config_host_mak
 fi
diff --git a/meson.build b/meson.build
index 286b37aecb..d4ce2ca57b 100644
--- a/meson.build
+++ b/meson.build
@@ -985,9 +985,10 @@ if 'CONFIG_LIBDAXCTL' in config_host
   libdaxctl = declare_dependency(link_args: config_host['LIBDAXCTL_LIBS'].split())
 endif
 tasn1 = not_found
-if 'CONFIG_TASN1' in config_host
-  tasn1 = declare_dependency(compile_args: config_host['TASN1_CFLAGS'].split(),
-                             link_args: config_host['TASN1_LIBS'].split())
+if gnutls.found()
+  tasn1 = dependency('libtasn1',
+                     method: 'pkg-config',
+                     kwargs: static_kwargs)
 endif
 keyutils = dependency('libkeyutils', required: false,
                       method: 'pkg-config', kwargs: static_kwargs)
@@ -2727,7 +2728,7 @@ summary_info += {'pixman':            pixman.found()}
 summary_info += {'VTE support':       config_host.has_key('CONFIG_VTE')}
 # TODO: add back version
 summary_info += {'slirp support':     slirp_opt == 'disabled' ? false : slirp_opt}
-summary_info += {'libtasn1':          config_host.has_key('CONFIG_TASN1')}
+summary_info += {'libtasn1':          tasn1.found()}
 summary_info += {'PAM':               config_host.has_key('CONFIG_AUTH_PAM')}
 summary_info += {'iconv support':     iconv.found()}
 summary_info += {'curses support':    curses.found()}
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index fcf6ed2ef5..4c1ebc06ac 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -84,7 +84,7 @@ if have_block
     'test-crypto-block': [io],
   }
   if gnutls.found() and \
-     'CONFIG_TASN1' in config_host and \
+     tasn1.found() and \
      'CONFIG_POSIX' in config_host
     tests += {
       'test-crypto-tlscredsx509': ['crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c',
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 08/28] configure, meson: convert pam detection to meson
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 07/28] configure, meson: convert libtasn1 " Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 09/28] configure, meson: convert libusb " Paolo Bonzini
                   ` (20 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 authz/meson.build      |  2 +-
 configure              | 38 ++++----------------------------------
 meson.build            | 31 ++++++++++++++++++++++++++-----
 meson_options.txt      |  2 ++
 tests/unit/meson.build |  2 +-
 5 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/authz/meson.build b/authz/meson.build
index 88fa7769cb..42a1ec0ff6 100644
--- a/authz/meson.build
+++ b/authz/meson.build
@@ -6,4 +6,4 @@ authz_ss.add(files(
   'simple.c',
 ))
 
-authz_ss.add(when: ['CONFIG_AUTH_PAM', pam], if_true: files('pamacct.c'))
+authz_ss.add(when: pam, if_true: files('pamacct.c'))
diff --git a/configure b/configure
index 3d36eea55f..237e99c3d0 100755
--- a/configure
+++ b/configure
@@ -407,7 +407,7 @@ tls_priority="NORMAL"
 gnutls="auto"
 nettle="auto"
 gcrypt="auto"
-auth_pam="$default_feature"
+auth_pam="auto"
 vte="$default_feature"
 virglrenderer="$default_feature"
 tpm="$default_feature"
@@ -1383,9 +1383,9 @@ for opt do
   ;;
   --enable-gcrypt) gcrypt="enabled"
   ;;
-  --disable-auth-pam) auth_pam="no"
+  --disable-auth-pam) auth_pam="disabled"
   ;;
-  --enable-auth-pam) auth_pam="yes"
+  --enable-auth-pam) auth_pam="enabled"
   ;;
   --enable-rdma) rdma="yes"
   ;;
@@ -2799,33 +2799,6 @@ EOF
   fi
 fi
 
-##########################################
-# PAM probe
-
-if test "$auth_pam" != "no"; then
-    cat > $TMPC <<EOF
-#include <security/pam_appl.h>
-#include <stdio.h>
-int main(void) {
-   const char *service_name = "qemu";
-   const char *user = "frank";
-   const struct pam_conv pam_conv = { 0 };
-   pam_handle_t *pamh = NULL;
-   pam_start(service_name, user, &pam_conv, &pamh);
-   return 0;
-}
-EOF
-    if compile_prog "" "-lpam" ; then
-        auth_pam=yes
-    else
-        if test "$auth_pam" = "yes"; then
-            feature_not_found "PAM" "Install PAM development package"
-        else
-            auth_pam=no
-        fi
-    fi
-fi
-
 ##########################################
 # VTE probe
 
@@ -5540,9 +5513,6 @@ if test "$gdbus_codegen" != "" ; then
     echo "GDBUS_CODEGEN=$gdbus_codegen" >> $config_host_mak
 fi
 echo "CONFIG_TLS_PRIORITY=\"$tls_priority\"" >> $config_host_mak
-if test "$auth_pam" = "yes" ; then
-    echo "CONFIG_AUTH_PAM=y" >> $config_host_mak
-fi
 if test "$have_broken_size_max" = "yes" ; then
     echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
 fi
@@ -6251,7 +6221,7 @@ if test "$skip_meson" = no; then
         -Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
         -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
         -Drbd=$rbd -Dlzo=$lzo -Dsnappy=$snappy -Dlzfse=$lzfse \
-        -Dgnutls=$gnutls -Dnettle=$nettle -Dgcrypt=$gcrypt \
+        -Dgnutls=$gnutls -Dnettle=$nettle -Dgcrypt=$gcrypt -Dauth_pam=$auth_pam \
         -Dzstd=$zstd -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
         -Dattr=$attr -Ddefault_devices=$default_devices \
         -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
diff --git a/meson.build b/meson.build
index d4ce2ca57b..d3025e05fc 100644
--- a/meson.build
+++ b/meson.build
@@ -325,10 +325,6 @@ if have_system or have_tools
   pixman = dependency('pixman-1', required: have_system, version:'>=0.21.8',
                       method: 'pkg-config', kwargs: static_kwargs)
 endif
-pam = not_found
-if 'CONFIG_AUTH_PAM' in config_host
-  pam = cc.find_library('pam')
-endif
 libaio = cc.find_library('aio', required: false)
 zlib = dependency('zlib', required: true, kwargs: static_kwargs)
 linux_io_uring = not_found
@@ -907,6 +903,31 @@ if get_option('vnc').enabled()
   endif
 endif
 
+pam = not_found
+if not get_option('auth_pam').auto() or have_system
+  pam = cc.find_library('pam', has_headers: ['security/pam_appl.h'],
+                        required: get_option('auth_pam'),
+                        kwargs: static_kwargs)
+endif
+if pam.found() and not cc.links('''
+   #include <stddef.h>
+   #include <security/pam_appl.h>
+   int main(void) {
+     const char *service_name = "qemu";
+     const char *user = "frank";
+     const struct pam_conv pam_conv = { 0 };
+     pam_handle_t *pamh = NULL;
+     pam_start(service_name, user, &pam_conv, &pamh);
+     return 0;
+   }''', dependencies: pam)
+  pam = not_found
+  if get_option('auth_pam').enabled()
+    error('could not link libpam')
+  else
+    warning('could not link libpam, disabling')
+  endif
+endif
+
 snappy = not_found
 if not get_option('snappy').auto() or have_system
   snappy = cc.find_library('snappy', has_headers: ['snappy-c.h'],
@@ -2729,7 +2750,7 @@ summary_info += {'VTE support':       config_host.has_key('CONFIG_VTE')}
 # TODO: add back version
 summary_info += {'slirp support':     slirp_opt == 'disabled' ? false : slirp_opt}
 summary_info += {'libtasn1':          tasn1.found()}
-summary_info += {'PAM':               config_host.has_key('CONFIG_AUTH_PAM')}
+summary_info += {'PAM':               pam.found()}
 summary_info += {'iconv support':     iconv.found()}
 summary_info += {'curses support':    curses.found()}
 # TODO: add back version
diff --git a/meson_options.txt b/meson_options.txt
index 343ffffb7c..ac6e90da07 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -52,6 +52,8 @@ option('multiprocess', type: 'feature', value: 'auto',
 
 option('attr', type : 'feature', value : 'auto',
        description: 'attr/xattr support')
+option('auth_pam', type : 'feature', value : 'auto',
+       description: 'PAM access control')
 option('brlapi', type : 'feature', value : 'auto',
        description: 'brlapi character device driver')
 option('bzip2', type : 'feature', value : 'auto',
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 4c1ebc06ac..3e0504dd21 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -94,7 +94,7 @@ if have_block
       'test-io-channel-tls': ['io-channel-helpers.c', 'crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c',
                               tasn1, io, crypto, gnutls]}
   endif
-  if 'CONFIG_AUTH_PAM' in config_host
+  if pam.found()
     tests += {'test-authz-pam': [authz]}
   endif
   if xts == 'private'
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 09/28] configure, meson: convert libusb detection to meson
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 08/28] configure, meson: convert pam " Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 10/28] configure, meson: convert libcacard " Paolo Bonzini
                   ` (19 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure          | 27 ++++-----------------------
 hw/usb/meson.build |  2 +-
 meson.build        | 11 +++++++----
 meson_options.txt  |  2 ++
 4 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/configure b/configure
index 237e99c3d0..e54d06b99e 100755
--- a/configure
+++ b/configure
@@ -374,7 +374,7 @@ spice_protocol="auto"
 rbd="auto"
 smartcard="$default_feature"
 u2f="auto"
-libusb="$default_feature"
+libusb="auto"
 usb_redir="$default_feature"
 opengl="$default_feature"
 cpuid_h="no"
@@ -1285,9 +1285,9 @@ for opt do
   ;;
   --enable-u2f) u2f="enabled"
   ;;
-  --disable-libusb) libusb="no"
+  --disable-libusb) libusb="disabled"
   ;;
-  --enable-libusb) libusb="yes"
+  --enable-libusb) libusb="enabled"
   ;;
   --disable-usb-redir) usb_redir="no"
   ;;
@@ -3994,20 +3994,6 @@ if test "$smartcard" != "no"; then
     fi
 fi
 
-# check for libusb
-if test "$libusb" != "no" ; then
-    if $pkg_config --atleast-version=1.0.13 libusb-1.0; then
-        libusb="yes"
-        libusb_cflags=$($pkg_config --cflags libusb-1.0)
-        libusb_libs=$($pkg_config --libs libusb-1.0)
-    else
-        if test "$libusb" = "yes"; then
-            feature_not_found "libusb" "Install libusb devel >= 1.0.13"
-        fi
-        libusb="no"
-    fi
-fi
-
 # check for usbredirparser for usb network redirection support
 if test "$usb_redir" != "no" ; then
     if $pkg_config --atleast-version=0.6 libusbredirparser-0.5; then
@@ -5631,12 +5617,6 @@ if test "$smartcard" = "yes" ; then
   echo "SMARTCARD_LIBS=$libcacard_libs" >> $config_host_mak
 fi
 
-if test "$libusb" = "yes" ; then
-  echo "CONFIG_USB_LIBUSB=y" >> $config_host_mak
-  echo "LIBUSB_CFLAGS=$libusb_cflags" >> $config_host_mak
-  echo "LIBUSB_LIBS=$libusb_libs" >> $config_host_mak
-fi
-
 if test "$usb_redir" = "yes" ; then
   echo "CONFIG_USB_REDIR=y" >> $config_host_mak
   echo "USB_REDIR_CFLAGS=$usb_redir_cflags" >> $config_host_mak
@@ -6215,6 +6195,7 @@ if test "$skip_meson" = no; then
         -Dkvm=$kvm -Dhax=$hax -Dwhpx=$whpx -Dhvf=$hvf -Dnvmm=$nvmm \
         -Dxen=$xen -Dxen_pci_passthrough=$xen_pci_passthrough -Dtcg=$tcg \
         -Dcocoa=$cocoa -Dgtk=$gtk -Dmpath=$mpath -Dsdl=$sdl -Dsdl_image=$sdl_image \
+        -Dlibusb=$libusb \
         -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
         -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f -Dvirtiofsd=$virtiofsd \
         -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index f357270d0b..bd3f8735b9 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -72,7 +72,7 @@ if config_host.has_key('CONFIG_USB_REDIR')
 endif
 
 # usb pass-through
-softmmu_ss.add(when: ['CONFIG_USB', 'CONFIG_USB_LIBUSB', libusb],
+softmmu_ss.add(when: ['CONFIG_USB', libusb],
                if_true: files('host-libusb.c'),
                if_false: files('host-stub.c'))
 softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('host-stub.c'))
diff --git a/meson.build b/meson.build
index d3025e05fc..0b4b55b9da 100644
--- a/meson.build
+++ b/meson.build
@@ -992,10 +992,12 @@ if 'CONFIG_USB_REDIR' in config_host
                                 link_args: config_host['USB_REDIR_LIBS'].split())
 endif
 libusb = not_found
-if 'CONFIG_USB_LIBUSB' in config_host
-  libusb = declare_dependency(compile_args: config_host['LIBUSB_CFLAGS'].split(),
-                              link_args: config_host['LIBUSB_LIBS'].split())
+if not get_option('libusb').auto() or have_system
+  libusb = dependency('libusb-1.0', required: get_option('libusb'),
+                      version: '>=1.0.13', method: 'pkg-config',
+                      kwargs: static_kwargs)
 endif
+
 libpmem = not_found
 if 'CONFIG_LIBPMEM' in config_host
   libpmem = declare_dependency(compile_args: config_host['LIBPMEM_CFLAGS'].split(),
@@ -1211,6 +1213,7 @@ config_host_data.set('CONFIG_SDL', sdl.found())
 config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
 config_host_data.set('CONFIG_SECCOMP', seccomp.found())
 config_host_data.set('CONFIG_SNAPPY', snappy.found())
+config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
 config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
 config_host_data.set('CONFIG_VNC', vnc.found())
 config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
@@ -2780,7 +2783,7 @@ summary_info += {'rbd support':       rbd.found()}
 summary_info += {'xfsctl support':    config_host.has_key('CONFIG_XFS')}
 summary_info += {'smartcard support': config_host.has_key('CONFIG_SMARTCARD')}
 summary_info += {'U2F support':       u2f.found()}
-summary_info += {'libusb':            config_host.has_key('CONFIG_USB_LIBUSB')}
+summary_info += {'libusb':            libusb.found()}
 summary_info += {'usb net redir':     config_host.has_key('CONFIG_USB_REDIR')}
 summary_info += {'OpenGL support':    config_host.has_key('CONFIG_OPENGL')}
 summary_info += {'GBM':               config_host.has_key('CONFIG_GBM')}
diff --git a/meson_options.txt b/meson_options.txt
index ac6e90da07..02c14d4751 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -86,6 +86,8 @@ option('gcrypt', type : 'feature', value : 'auto',
        description: 'libgcrypt cryptography support')
 option('libudev', type : 'feature', value : 'auto',
        description: 'Use libudev to enumerate host devices')
+option('libusb', type : 'feature', value : 'auto',
+       description: 'libusb support for USB passthrough')
 option('lzfse', type : 'feature', value : 'auto',
        description: 'lzfse support for DMG images')
 option('lzo', type : 'feature', value : 'auto',
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 10/28] configure, meson: convert libcacard detection to meson
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (8 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 09/28] configure, meson: convert libusb " Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 11/28] configure, meson: convert libusbredir " Paolo Bonzini
                   ` (18 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure          | 28 ++++------------------------
 hw/usb/meson.build |  2 +-
 meson.build        |  9 +++++----
 meson_options.txt  |  2 ++
 4 files changed, 12 insertions(+), 29 deletions(-)

diff --git a/configure b/configure
index e54d06b99e..02b0acc1f5 100755
--- a/configure
+++ b/configure
@@ -372,7 +372,7 @@ trace_file="trace"
 spice="$default_feature"
 spice_protocol="auto"
 rbd="auto"
-smartcard="$default_feature"
+smartcard="auto"
 u2f="auto"
 libusb="auto"
 usb_redir="$default_feature"
@@ -1277,9 +1277,9 @@ for opt do
   ;;
   --enable-xfsctl) xfs="yes"
   ;;
-  --disable-smartcard) smartcard="no"
+  --disable-smartcard) smartcard="disabled"
   ;;
-  --enable-smartcard) smartcard="yes"
+  --enable-smartcard) smartcard="enabled"
   ;;
   --disable-u2f) u2f="disabled"
   ;;
@@ -3980,20 +3980,6 @@ EOF
   fi
 fi
 
-# check for smartcard support
-if test "$smartcard" != "no"; then
-    if $pkg_config --atleast-version=2.5.1 libcacard; then
-        libcacard_cflags=$($pkg_config --cflags libcacard)
-        libcacard_libs=$($pkg_config --libs libcacard)
-        smartcard="yes"
-    else
-        if test "$smartcard" = "yes"; then
-            feature_not_found "smartcard" "Install libcacard devel"
-        fi
-        smartcard="no"
-    fi
-fi
-
 # check for usbredirparser for usb network redirection support
 if test "$usb_redir" != "no" ; then
     if $pkg_config --atleast-version=0.6 libusbredirparser-0.5; then
@@ -5611,12 +5597,6 @@ if test "$spice" = "yes" ; then
   echo "SPICE_LIBS=$spice_libs" >> $config_host_mak
 fi
 
-if test "$smartcard" = "yes" ; then
-  echo "CONFIG_SMARTCARD=y" >> $config_host_mak
-  echo "SMARTCARD_CFLAGS=$libcacard_cflags" >> $config_host_mak
-  echo "SMARTCARD_LIBS=$libcacard_libs" >> $config_host_mak
-fi
-
 if test "$usb_redir" = "yes" ; then
   echo "CONFIG_USB_REDIR=y" >> $config_host_mak
   echo "USB_REDIR_CFLAGS=$usb_redir_cflags" >> $config_host_mak
@@ -6195,7 +6175,7 @@ if test "$skip_meson" = no; then
         -Dkvm=$kvm -Dhax=$hax -Dwhpx=$whpx -Dhvf=$hvf -Dnvmm=$nvmm \
         -Dxen=$xen -Dxen_pci_passthrough=$xen_pci_passthrough -Dtcg=$tcg \
         -Dcocoa=$cocoa -Dgtk=$gtk -Dmpath=$mpath -Dsdl=$sdl -Dsdl_image=$sdl_image \
-        -Dlibusb=$libusb \
+        -Dlibusb=$libusb -Dsmartcard=$smartcard \
         -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
         -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f -Dvirtiofsd=$virtiofsd \
         -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index bd3f8735b9..df9effbb10 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -49,7 +49,7 @@ softmmu_ss.add(when: ['CONFIG_POSIX', 'CONFIG_USB_STORAGE_MTP'], if_true: files(
 # smartcard
 softmmu_ss.add(when: 'CONFIG_USB_SMARTCARD', if_true: files('dev-smartcard-reader.c'))
 
-if config_host.has_key('CONFIG_SMARTCARD')
+if cacard.found()
   usbsmartcard_ss = ss.source_set()
   usbsmartcard_ss.add(when: 'CONFIG_USB_SMARTCARD',
                       if_true: [cacard, files('ccid-card-emulated.c', 'ccid-card-passthru.c')])
diff --git a/meson.build b/meson.build
index 0b4b55b9da..afcfcb8c57 100644
--- a/meson.build
+++ b/meson.build
@@ -976,9 +976,10 @@ if 'CONFIG_XEN_BACKEND' in config_host
                            link_args: config_host['XEN_LIBS'].split())
 endif
 cacard = not_found
-if 'CONFIG_SMARTCARD' in config_host
-  cacard = declare_dependency(compile_args: config_host['SMARTCARD_CFLAGS'].split(),
-                              link_args: config_host['SMARTCARD_LIBS'].split())
+if not get_option('smartcard').auto() or have_system
+  cacard = dependency('libcacard', required: get_option('smartcard'),
+                      version: '>=2.5.1', method: 'pkg-config',
+                      kwargs: static_kwargs)
 endif
 u2f = not_found
 if have_system
@@ -2781,7 +2782,7 @@ summary_info += {'bpf support': libbpf.found()}
 summary_info += {'spice support':     config_host.has_key('CONFIG_SPICE')}
 summary_info += {'rbd support':       rbd.found()}
 summary_info += {'xfsctl support':    config_host.has_key('CONFIG_XFS')}
-summary_info += {'smartcard support': config_host.has_key('CONFIG_SMARTCARD')}
+summary_info += {'smartcard support': cacard.found()}
 summary_info += {'U2F support':       u2f.found()}
 summary_info += {'libusb':            libusb.found()}
 summary_info += {'usb net redir':     config_host.has_key('CONFIG_USB_REDIR')}
diff --git a/meson_options.txt b/meson_options.txt
index 02c14d4751..cd9374384e 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -102,6 +102,8 @@ option('sdl_image', type : 'feature', value : 'auto',
        description: 'SDL Image support for icons')
 option('seccomp', type : 'feature', value : 'auto',
        description: 'seccomp support')
+option('smartcard', type : 'feature', value : 'auto',
+       description: 'CA smartcard emulation support')
 option('snappy', type : 'feature', value : 'auto',
        description: 'snappy compression support')
 option('u2f', type : 'feature', value : 'auto',
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 11/28] configure, meson: convert libusbredir detection to meson
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (9 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 10/28] configure, meson: convert libcacard " Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 12/28] KVM: Fix dirty ring mmap incorrect size due to renaming accident Paolo Bonzini
                   ` (17 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure          | 28 ++++------------------------
 hw/usb/meson.build |  2 +-
 meson.build        |  9 +++++----
 meson_options.txt  |  2 ++
 4 files changed, 12 insertions(+), 29 deletions(-)

diff --git a/configure b/configure
index 02b0acc1f5..e799d908a3 100755
--- a/configure
+++ b/configure
@@ -375,7 +375,7 @@ rbd="auto"
 smartcard="auto"
 u2f="auto"
 libusb="auto"
-usb_redir="$default_feature"
+usb_redir="auto"
 opengl="$default_feature"
 cpuid_h="no"
 avx2_opt="$default_feature"
@@ -1289,9 +1289,9 @@ for opt do
   ;;
   --enable-libusb) libusb="enabled"
   ;;
-  --disable-usb-redir) usb_redir="no"
+  --disable-usb-redir) usb_redir="disabled"
   ;;
-  --enable-usb-redir) usb_redir="yes"
+  --enable-usb-redir) usb_redir="enabled"
   ;;
   --disable-zlib-test)
   ;;
@@ -3980,20 +3980,6 @@ EOF
   fi
 fi
 
-# check for usbredirparser for usb network redirection support
-if test "$usb_redir" != "no" ; then
-    if $pkg_config --atleast-version=0.6 libusbredirparser-0.5; then
-        usb_redir="yes"
-        usb_redir_cflags=$($pkg_config --cflags libusbredirparser-0.5)
-        usb_redir_libs=$($pkg_config --libs libusbredirparser-0.5)
-    else
-        if test "$usb_redir" = "yes"; then
-            feature_not_found "usb-redir" "Install usbredir devel"
-        fi
-        usb_redir="no"
-    fi
-fi
-
 ##########################################
 # check if we have VSS SDK headers for win
 
@@ -5597,12 +5583,6 @@ if test "$spice" = "yes" ; then
   echo "SPICE_LIBS=$spice_libs" >> $config_host_mak
 fi
 
-if test "$usb_redir" = "yes" ; then
-  echo "CONFIG_USB_REDIR=y" >> $config_host_mak
-  echo "USB_REDIR_CFLAGS=$usb_redir_cflags" >> $config_host_mak
-  echo "USB_REDIR_LIBS=$usb_redir_libs" >> $config_host_mak
-fi
-
 if test "$opengl" = "yes" ; then
   echo "CONFIG_OPENGL=y" >> $config_host_mak
   echo "OPENGL_CFLAGS=$opengl_cflags" >> $config_host_mak
@@ -6175,7 +6155,7 @@ if test "$skip_meson" = no; then
         -Dkvm=$kvm -Dhax=$hax -Dwhpx=$whpx -Dhvf=$hvf -Dnvmm=$nvmm \
         -Dxen=$xen -Dxen_pci_passthrough=$xen_pci_passthrough -Dtcg=$tcg \
         -Dcocoa=$cocoa -Dgtk=$gtk -Dmpath=$mpath -Dsdl=$sdl -Dsdl_image=$sdl_image \
-        -Dlibusb=$libusb -Dsmartcard=$smartcard \
+        -Dlibusb=$libusb -Dsmartcard=$smartcard -Dusb_redir=$usb_redir \
         -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
         -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f -Dvirtiofsd=$virtiofsd \
         -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index df9effbb10..4f24b5274d 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -64,7 +64,7 @@ if u2f.found()
 endif
 
 # usb redirect
-if config_host.has_key('CONFIG_USB_REDIR')
+if usbredir.found()
   usbredir_ss = ss.source_set()
   usbredir_ss.add(when: 'CONFIG_USB',
                   if_true: [usbredir, files('redirect.c', 'quirks.c')])
diff --git a/meson.build b/meson.build
index afcfcb8c57..64e23175ab 100644
--- a/meson.build
+++ b/meson.build
@@ -988,9 +988,10 @@ if have_system
                    kwargs: static_kwargs)
 endif
 usbredir = not_found
-if 'CONFIG_USB_REDIR' in config_host
-  usbredir = declare_dependency(compile_args: config_host['USB_REDIR_CFLAGS'].split(),
-                                link_args: config_host['USB_REDIR_LIBS'].split())
+if not get_option('usb_redir').auto() or have_system
+  usbredir = dependency('libusbredirparser-0.5', required: get_option('usb_redir'),
+                        version: '>=0.6', method: 'pkg-config',
+                        kwargs: static_kwargs)
 endif
 libusb = not_found
 if not get_option('libusb').auto() or have_system
@@ -2785,7 +2786,7 @@ summary_info += {'xfsctl support':    config_host.has_key('CONFIG_XFS')}
 summary_info += {'smartcard support': cacard.found()}
 summary_info += {'U2F support':       u2f.found()}
 summary_info += {'libusb':            libusb.found()}
-summary_info += {'usb net redir':     config_host.has_key('CONFIG_USB_REDIR')}
+summary_info += {'usb net redir':     usbredir.found()}
 summary_info += {'OpenGL support':    config_host.has_key('CONFIG_OPENGL')}
 summary_info += {'GBM':               config_host.has_key('CONFIG_GBM')}
 summary_info += {'libiscsi support':  libiscsi.found()}
diff --git a/meson_options.txt b/meson_options.txt
index cd9374384e..f7ec9bee27 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -108,6 +108,8 @@ option('snappy', type : 'feature', value : 'auto',
        description: 'snappy compression support')
 option('u2f', type : 'feature', value : 'auto',
        description: 'U2F emulation support')
+option('usb_redir', type : 'feature', value : 'auto',
+       description: 'libusbredir support')
 option('vnc', type : 'feature', value : 'enabled',
        description: 'VNC server')
 option('vnc_jpeg', type : 'feature', value : 'auto',
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 12/28] KVM: Fix dirty ring mmap incorrect size due to renaming accident
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (10 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 11/28] configure, meson: convert libusbredir " Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 13/28] file-posix: fix max_iov for /dev/sg devices Paolo Bonzini
                   ` (16 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hyman Huang, Dr . David Alan Gilbert, Peter Xu

From: Peter Xu <peterx@redhat.com>

Found this when I wanted to try the per-vcpu dirty rate series out, then I
found that it's not really working and it can quickly hang death a guest.  I
found strange errors (e.g. guest crash after migration) happens even without
the per-vcpu dirty rate series.

When merging dirty ring, probably no one notice that the trivial renaming diff
[1] missed two existing references of kvm_dirty_ring_sizes; they do matter
since otherwise we'll mmap() a shorter range of memory after the renaming.

I think it didn't SIGBUS for me easily simply because some other stuff within
qemu mmap()ed right after the dirty rings (e.g. when testing 4096 slots, it
aligned with one small page on x86), so when we access the rings we've been
reading/writting to random memory elsewhere of qemu.

Fix the two sizes when map/unmap the shared dirty gfn memory.

[1] https://lore.kernel.org/qemu-devel/dac5f0c6-1bca-3daf-e5d2-6451dbbaca93@redhat.com/

Cc: Hyman Huang <huangy81@chinatelecom.cn>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20210609014355.217110-1-peterx@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/kvm/kvm-all.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index c7ec538850..e5b10dd129 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -411,7 +411,7 @@ static int do_kvm_destroy_vcpu(CPUState *cpu)
     }
 
     if (cpu->kvm_dirty_gfns) {
-        ret = munmap(cpu->kvm_dirty_gfns, s->kvm_dirty_ring_size);
+        ret = munmap(cpu->kvm_dirty_gfns, s->kvm_dirty_ring_bytes);
         if (ret < 0) {
             goto err;
         }
@@ -495,7 +495,7 @@ int kvm_init_vcpu(CPUState *cpu, Error **errp)
 
     if (s->kvm_dirty_ring_size) {
         /* Use MAP_SHARED to share pages with the kernel */
-        cpu->kvm_dirty_gfns = mmap(NULL, s->kvm_dirty_ring_size,
+        cpu->kvm_dirty_gfns = mmap(NULL, s->kvm_dirty_ring_bytes,
                                    PROT_READ | PROT_WRITE, MAP_SHARED,
                                    cpu->kvm_fd,
                                    PAGE_SIZE * KVM_DIRTY_LOG_PAGE_OFFSET);
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 13/28] file-posix: fix max_iov for /dev/sg devices
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (11 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 12/28] KVM: Fix dirty ring mmap incorrect size due to renaming accident Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 14/28] scsi-generic: pass max_segments via max_iov field in BlockLimits Paolo Bonzini
                   ` (15 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Max Reitz

Even though it was only called for devices that have bs->sg set (which
must be character devices), sg_get_max_segments looked at /sys/dev/block
which only works for block devices.

On Linux the sg driver has its own way to provide the maximum number of
iovecs in a scatter/gather list, so add support for it.  The block device
path is kept because it will be reinstated in the next patches.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/file-posix.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index b3fbb9bd63..b8dc19ce1a 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1178,6 +1178,17 @@ static int sg_get_max_segments(int fd)
         goto out;
     }
 
+    if (S_ISCHR(st.st_mode)) {
+        if (ioctl(fd, SG_GET_SG_TABLESIZE, &ret) == 0) {
+            return ret;
+        }
+        return -ENOTSUP;
+    }
+
+    if (!S_ISBLK(st.st_mode)) {
+        return -ENOTSUP;
+    }
+
     sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_segments",
                                 major(st.st_rdev), minor(st.st_rdev));
     sysfd = open(sysfspath, O_RDONLY);
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 14/28] scsi-generic: pass max_segments via max_iov field in BlockLimits
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (12 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 13/28] file-posix: fix max_iov for /dev/sg devices Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 15/28] osdep: provide ROUND_DOWN macro Paolo Bonzini
                   ` (14 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Max Reitz

I/O to a disk via read/write is not limited by the number of segments allowed
by the host adapter; the kernel can split requests if needed, and the limit
imposed by the host adapter can be very low (256k or so) to avoid that SG_IO
returns EINVAL if memory is heavily fragmented.

Since this value is only interesting for SG_IO-based I/O, do not include
it in the max_transfer and only take it into account when patching the
block limits VPD page in the scsi-generic device.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/file-posix.c     | 3 +--
 hw/scsi/scsi-generic.c | 6 ++++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index b8dc19ce1a..6db690baf2 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1237,8 +1237,7 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 
         ret = sg_get_max_segments(s->fd);
         if (ret > 0) {
-            bs->bl.max_transfer = MIN(bs->bl.max_transfer,
-                                      ret * qemu_real_host_page_size);
+            bs->bl.max_iov = ret;
         }
     }
 
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 40e039864f..b6c4143dc7 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -179,10 +179,12 @@ static int scsi_handle_inquiry_reply(SCSIGenericReq *r, SCSIDevice *s, int len)
         (r->req.cmd.buf[1] & 0x01)) {
         page = r->req.cmd.buf[2];
         if (page == 0xb0) {
-            uint32_t max_transfer =
-                blk_get_max_transfer(s->conf.blk) / s->blocksize;
+            uint32_t max_transfer = blk_get_max_transfer(s->conf.blk);
+            uint32_t max_iov = blk_get_max_iov(s->conf.blk);
 
             assert(max_transfer);
+            max_transfer = MIN_NON_ZERO(max_transfer, max_iov * qemu_real_host_page_size)
+                / s->blocksize;
             stl_be_p(&r->buf[8], max_transfer);
             /* Also take care of the opt xfer len. */
             stl_be_p(&r->buf[12],
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 15/28] osdep: provide ROUND_DOWN macro
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (13 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 14/28] scsi-generic: pass max_segments via max_iov field in BlockLimits Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-29  4:12   ` Philippe Mathieu-Daudé
  2021-06-25 14:18 ` [PULL 16/28] block-backend: align max_transfer to request alignment Paolo Bonzini
                   ` (13 subsequent siblings)
  28 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel

osdep.h provides a ROUND_UP macro to hide bitwise operations for the
purpose of rounding a number up to a power of two; add a ROUND_DOWN
macro that does the same with truncation towards zero.

While at it, change the formatting of some comments.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/osdep.h | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 0a54bf7be8..c3656b755a 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -319,11 +319,16 @@ extern "C" {
     })
 #endif
 
-/* Round number down to multiple */
+/*
+ * Round number down to multiple. Safe when m is not a power of 2 (see
+ * ROUND_DOWN for a faster version when a power of 2 is guaranteed).
+ */
 #define QEMU_ALIGN_DOWN(n, m) ((n) / (m) * (m))
 
-/* Round number up to multiple. Safe when m is not a power of 2 (see
- * ROUND_UP for a faster version when a power of 2 is guaranteed) */
+/*
+ * Round number up to multiple. Safe when m is not a power of 2 (see
+ * ROUND_UP for a faster version when a power of 2 is guaranteed).
+ */
 #define QEMU_ALIGN_UP(n, m) QEMU_ALIGN_DOWN((n) + (m) - 1, (m))
 
 /* Check if n is a multiple of m */
@@ -340,11 +345,22 @@ extern "C" {
 /* Check if pointer p is n-bytes aligned */
 #define QEMU_PTR_IS_ALIGNED(p, n) QEMU_IS_ALIGNED((uintptr_t)(p), (n))
 
-/* Round number up to multiple. Requires that d be a power of 2 (see
+/*
+ * Round number down to multiple. Requires that d be a power of 2 (see
  * QEMU_ALIGN_UP for a safer but slower version on arbitrary
- * numbers); works even if d is a smaller type than n.  */
+ * numbers); works even if d is a smaller type than n.
+ */
+#ifndef ROUND_DOWN
+#define ROUND_DOWN(n, d) ((n) & -(0 ? (n) : (d)))
+#endif
+
+/*
+ * Round number up to multiple. Requires that d be a power of 2 (see
+ * QEMU_ALIGN_UP for a safer but slower version on arbitrary
+ * numbers); works even if d is a smaller type than n.
+ */
 #ifndef ROUND_UP
-#define ROUND_UP(n, d) (((n) + (d) - 1) & -(0 ? (n) : (d)))
+#define ROUND_UP(n, d) ROUND_DOWN((n) + (d) - 1, (d))
 #endif
 
 #ifndef DIV_ROUND_UP
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 16/28] block-backend: align max_transfer to request alignment
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (14 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 15/28] osdep: provide ROUND_DOWN macro Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 17/28] block: add max_hw_transfer to BlockLimits Paolo Bonzini
                   ` (12 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel

Block device requests must be aligned to bs->bl.request_alignment.
It makes sense for drivers to align bs->bl.max_transfer the same
way; however when there is no specified limit, blk_get_max_transfer
just returns INT_MAX.  Since the contract of the function does not
specify that INT_MAX means "no maximum", just align the outcome
of the function (whether INT_MAX or bs->bl.max_transfer) before
returning it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/block-backend.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 15f1ea4288..6e37582740 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1957,12 +1957,12 @@ uint32_t blk_get_request_alignment(BlockBackend *blk)
 uint32_t blk_get_max_transfer(BlockBackend *blk)
 {
     BlockDriverState *bs = blk_bs(blk);
-    uint32_t max = 0;
+    uint32_t max = INT_MAX;
 
     if (bs) {
-        max = bs->bl.max_transfer;
+        max = MIN_NON_ZERO(max, bs->bl.max_transfer);
     }
-    return MIN_NON_ZERO(max, INT_MAX);
+    return ROUND_DOWN(max, blk_get_request_alignment(blk));
 }
 
 int blk_get_max_iov(BlockBackend *blk)
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 17/28] block: add max_hw_transfer to BlockLimits
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (15 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 16/28] block-backend: align max_transfer to request alignment Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2 Paolo Bonzini
                   ` (11 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel

For block host devices, I/O can happen through either the kernel file
descriptor I/O system calls (preadv/pwritev, io_submit, io_uring)
or the SCSI passthrough ioctl SG_IO.

In the latter case, the size of each transfer can be limited by the
HBA, while for file descriptor I/O the kernel is able to split and
merge I/O in smaller pieces as needed.  Applying the HBA limits to
file descriptor I/O results in more system calls and suboptimal
performance, so this patch splits the max_transfer limit in two:
max_transfer remains valid and is used in general, while max_hw_transfer
is limited to the maximum hardware size.  max_hw_transfer can then be
included by the scsi-generic driver in the block limits page, to ensure
that the stricter hardware limit is used.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/block-backend.c          | 13 +++++++++++++
 block/file-posix.c             |  2 +-
 block/io.c                     |  2 ++
 hw/scsi/scsi-generic.c         |  2 +-
 include/block/block_int.h      |  7 +++++++
 include/sysemu/block-backend.h |  1 +
 6 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 6e37582740..deb55c272e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1953,6 +1953,19 @@ uint32_t blk_get_request_alignment(BlockBackend *blk)
     return bs ? bs->bl.request_alignment : BDRV_SECTOR_SIZE;
 }
 
+/* Returns the maximum hardware transfer length, in bytes; guaranteed nonzero */
+uint64_t blk_get_max_hw_transfer(BlockBackend *blk)
+{
+    BlockDriverState *bs = blk_bs(blk);
+    uint64_t max = INT_MAX;
+
+    if (bs) {
+        max = MIN_NON_ZERO(max, bs->bl.max_hw_transfer);
+        max = MIN_NON_ZERO(max, bs->bl.max_transfer);
+    }
+    return ROUND_DOWN(max, blk_get_request_alignment(blk));
+}
+
 /* Returns the maximum transfer length, in bytes; guaranteed nonzero */
 uint32_t blk_get_max_transfer(BlockBackend *blk)
 {
diff --git a/block/file-posix.c b/block/file-posix.c
index 6db690baf2..88e58d2863 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1232,7 +1232,7 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
         int ret = sg_get_max_transfer_length(s->fd);
 
         if (ret > 0 && ret <= BDRV_REQUEST_MAX_BYTES) {
-            bs->bl.max_transfer = pow2floor(ret);
+            bs->bl.max_hw_transfer = pow2floor(ret);
         }
 
         ret = sg_get_max_segments(s->fd);
diff --git a/block/io.c b/block/io.c
index 323854d063..dd93364258 100644
--- a/block/io.c
+++ b/block/io.c
@@ -127,6 +127,8 @@ static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src)
 {
     dst->opt_transfer = MAX(dst->opt_transfer, src->opt_transfer);
     dst->max_transfer = MIN_NON_ZERO(dst->max_transfer, src->max_transfer);
+    dst->max_hw_transfer = MIN_NON_ZERO(dst->max_hw_transfer,
+                                        src->max_hw_transfer);
     dst->opt_mem_alignment = MAX(dst->opt_mem_alignment,
                                  src->opt_mem_alignment);
     dst->min_mem_alignment = MAX(dst->min_mem_alignment,
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index b6c4143dc7..665baf900e 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -179,7 +179,7 @@ static int scsi_handle_inquiry_reply(SCSIGenericReq *r, SCSIDevice *s, int len)
         (r->req.cmd.buf[1] & 0x01)) {
         page = r->req.cmd.buf[2];
         if (page == 0xb0) {
-            uint32_t max_transfer = blk_get_max_transfer(s->conf.blk);
+            uint64_t max_transfer = blk_get_max_hw_transfer(s->conf.blk);
             uint32_t max_iov = blk_get_max_iov(s->conf.blk);
 
             assert(max_transfer);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 057d88b1fc..f1a54db0f8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -695,6 +695,13 @@ typedef struct BlockLimits {
      * clamped down. */
     uint32_t max_transfer;
 
+    /* Maximal hardware transfer length in bytes.  Applies whenever
+     * transfers to the device bypass the kernel I/O scheduler, for
+     * example with SG_IO.  If larger than max_transfer or if zero,
+     * blk_get_max_hw_transfer will fall back to max_transfer.
+     */
+    uint64_t max_hw_transfer;
+
     /* memory alignment, in bytes so that no bounce buffer is needed */
     size_t min_mem_alignment;
 
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 5423e3d9c6..9ac5f7bbd3 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -208,6 +208,7 @@ void blk_eject(BlockBackend *blk, bool eject_flag);
 int blk_get_flags(BlockBackend *blk);
 uint32_t blk_get_request_alignment(BlockBackend *blk);
 uint32_t blk_get_max_transfer(BlockBackend *blk);
+uint64_t blk_get_max_hw_transfer(BlockBackend *blk);
 int blk_get_max_iov(BlockBackend *blk);
 void blk_set_guest_block_size(BlockBackend *blk, int align);
 void *blk_try_blockalign(BlockBackend *blk, size_t size);
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (16 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 17/28] block: add max_hw_transfer to BlockLimits Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-09-06 14:24   ` Halil Pasic
  2021-06-25 14:18 ` [PULL 19/28] block: feature detection for host block support Paolo Bonzini
                   ` (10 subsequent siblings)
  28 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel

bs->sg is only true for character devices, but block devices can also
be used with scsi-block and scsi-generic.  Unfortunately BLKSECTGET
returns bytes in an int for /dev/sgN devices, and sectors in a short
for block devices, so account for that in the code.

The maximum transfer also need not be a power of 2 (for example I have
seen disks with 1280 KiB maximum transfer) so there's no need to pass
the result through pow2floor.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/file-posix.c | 67 ++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 88e58d2863..ea102483b0 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1147,22 +1147,27 @@ static void raw_reopen_abort(BDRVReopenState *state)
     s->reopen_state = NULL;
 }
 
-static int sg_get_max_transfer_length(int fd)
+static int hdev_get_max_hw_transfer(int fd, struct stat *st)
 {
 #ifdef BLKSECTGET
-    int max_bytes = 0;
-
-    if (ioctl(fd, BLKSECTGET, &max_bytes) == 0) {
-        return max_bytes;
+    if (S_ISBLK(st->st_mode)) {
+        unsigned short max_sectors = 0;
+        if (ioctl(fd, BLKSECTGET, &max_sectors) == 0) {
+            return max_sectors * 512;
+        }
     } else {
-        return -errno;
+        int max_bytes = 0;
+        if (ioctl(fd, BLKSECTGET, &max_bytes) == 0) {
+            return max_bytes;
+        }
     }
+    return -errno;
 #else
     return -ENOSYS;
 #endif
 }
 
-static int sg_get_max_segments(int fd)
+static int hdev_get_max_segments(int fd, struct stat *st)
 {
 #ifdef CONFIG_LINUX
     char buf[32];
@@ -1171,26 +1176,20 @@ static int sg_get_max_segments(int fd)
     int ret;
     int sysfd = -1;
     long max_segments;
-    struct stat st;
 
-    if (fstat(fd, &st)) {
-        ret = -errno;
-        goto out;
-    }
-
-    if (S_ISCHR(st.st_mode)) {
+    if (S_ISCHR(st->st_mode)) {
         if (ioctl(fd, SG_GET_SG_TABLESIZE, &ret) == 0) {
             return ret;
         }
         return -ENOTSUP;
     }
 
-    if (!S_ISBLK(st.st_mode)) {
+    if (!S_ISBLK(st->st_mode)) {
         return -ENOTSUP;
     }
 
     sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_segments",
-                                major(st.st_rdev), minor(st.st_rdev));
+                                major(st->st_rdev), minor(st->st_rdev));
     sysfd = open(sysfspath, O_RDONLY);
     if (sysfd == -1) {
         ret = -errno;
@@ -1227,23 +1226,33 @@ out:
 static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
-
-    if (bs->sg) {
-        int ret = sg_get_max_transfer_length(s->fd);
-
-        if (ret > 0 && ret <= BDRV_REQUEST_MAX_BYTES) {
-            bs->bl.max_hw_transfer = pow2floor(ret);
-        }
-
-        ret = sg_get_max_segments(s->fd);
-        if (ret > 0) {
-            bs->bl.max_iov = ret;
-        }
-    }
+    struct stat st;
 
     raw_probe_alignment(bs, s->fd, errp);
     bs->bl.min_mem_alignment = s->buf_align;
     bs->bl.opt_mem_alignment = MAX(s->buf_align, qemu_real_host_page_size);
+
+    /*
+     * Maximum transfers are best effort, so it is okay to ignore any
+     * errors.  That said, based on the man page errors in fstat would be
+     * very much unexpected; the only possible case seems to be ENOMEM.
+     */
+    if (fstat(s->fd, &st)) {
+        return;
+    }
+
+    if (bs->sg || S_ISBLK(st.st_mode)) {
+        int ret = hdev_get_max_hw_transfer(s->fd, &st);
+
+        if (ret > 0 && ret <= BDRV_REQUEST_MAX_BYTES) {
+            bs->bl.max_hw_transfer = ret;
+        }
+
+        ret = hdev_get_max_segments(s->fd, &st);
+        if (ret > 0) {
+            bs->bl.max_iov = ret;
+        }
+    }
 }
 
 static int check_for_dasd(int fd)
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 19/28] block: feature detection for host block support
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (17 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2 Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 20/28] block: check for sys/disk.h Paolo Bonzini
                   ` (9 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joelle van Dyne

From: Joelle van Dyne <j@getutm.app>

On Darwin (iOS), there are no system level APIs for directly accessing
host block devices. We detect this at configure time.

Signed-off-by: Joelle van Dyne <j@getutm.app>
Message-Id: <20210315180341.31638-2-j@getutm.app>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/file-posix.c   | 33 ++++++++++++++++++++++-----------
 meson.build          |  6 +++++-
 qapi/block-core.json | 14 ++++++++++----
 3 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index ea102483b0..e56bb491a1 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -42,6 +42,8 @@
 #include "scsi/constants.h"
 
 #if defined(__APPLE__) && (__MACH__)
+#include <sys/ioctl.h>
+#if defined(HAVE_HOST_BLOCK_DEVICE)
 #include <paths.h>
 #include <sys/param.h>
 #include <IOKit/IOKitLib.h>
@@ -52,6 +54,7 @@
 //#include <IOKit/storage/IOCDTypes.h>
 #include <IOKit/storage/IODVDMedia.h>
 #include <CoreFoundation/CoreFoundation.h>
+#endif /* defined(HAVE_HOST_BLOCK_DEVICE) */
 #endif
 
 #ifdef __sun__
@@ -178,7 +181,17 @@ typedef struct BDRVRawReopenState {
     bool check_cache_dropped;
 } BDRVRawReopenState;
 
-static int fd_open(BlockDriverState *bs);
+static int fd_open(BlockDriverState *bs)
+{
+    BDRVRawState *s = bs->opaque;
+
+    /* this is just to ensure s->fd is sane (its called by io ops) */
+    if (s->fd >= 0) {
+        return 0;
+    }
+    return -EIO;
+}
+
 static int64_t raw_getlength(BlockDriverState *bs);
 
 typedef struct RawPosixAIOData {
@@ -3033,6 +3046,7 @@ static BlockStatsSpecific *raw_get_specific_stats(BlockDriverState *bs)
     return stats;
 }
 
+#if defined(HAVE_HOST_BLOCK_DEVICE)
 static BlockStatsSpecific *hdev_get_specific_stats(BlockDriverState *bs)
 {
     BlockStatsSpecific *stats = g_new(BlockStatsSpecific, 1);
@@ -3042,6 +3056,7 @@ static BlockStatsSpecific *hdev_get_specific_stats(BlockDriverState *bs)
 
     return stats;
 }
+#endif /* HAVE_HOST_BLOCK_DEVICE */
 
 static QemuOptsList raw_create_opts = {
     .name = "raw-create-opts",
@@ -3257,6 +3272,8 @@ BlockDriver bdrv_file = {
 /***********************************************/
 /* host device */
 
+#if defined(HAVE_HOST_BLOCK_DEVICE)
+
 #if defined(__APPLE__) && defined(__MACH__)
 static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
                                 CFIndex maxPathSize, int flags);
@@ -3549,16 +3566,6 @@ hdev_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
 }
 #endif /* linux */
 
-static int fd_open(BlockDriverState *bs)
-{
-    BDRVRawState *s = bs->opaque;
-
-    /* this is just to ensure s->fd is sane (its called by io ops) */
-    if (s->fd >= 0)
-        return 0;
-    return -EIO;
-}
-
 static coroutine_fn int
 hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
 {
@@ -3882,6 +3889,8 @@ static BlockDriver bdrv_host_cdrom = {
 };
 #endif /* __FreeBSD__ */
 
+#endif /* HAVE_HOST_BLOCK_DEVICE */
+
 static void bdrv_file_init(void)
 {
     /*
@@ -3889,6 +3898,7 @@ static void bdrv_file_init(void)
      * registered last will get probed first.
      */
     bdrv_register(&bdrv_file);
+#if defined(HAVE_HOST_BLOCK_DEVICE)
     bdrv_register(&bdrv_host_device);
 #ifdef __linux__
     bdrv_register(&bdrv_host_cdrom);
@@ -3896,6 +3906,7 @@ static void bdrv_file_init(void)
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
     bdrv_register(&bdrv_host_cdrom);
 #endif
+#endif /* HAVE_HOST_BLOCK_DEVICE */
 }
 
 block_init(bdrv_file_init);
diff --git a/meson.build b/meson.build
index 64e23175ab..6419d4ee41 100644
--- a/meson.build
+++ b/meson.build
@@ -183,7 +183,7 @@ if targetos == 'windows'
                                       include_directories: include_directories('.'))
 elif targetos == 'darwin'
   coref = dependency('appleframeworks', modules: 'CoreFoundation')
-  iokit = dependency('appleframeworks', modules: 'IOKit')
+  iokit = dependency('appleframeworks', modules: 'IOKit', required: false)
 elif targetos == 'sunos'
   socket = [cc.find_library('socket'),
             cc.find_library('nsl'),
@@ -1148,6 +1148,9 @@ if get_option('cfi')
   add_global_link_arguments(cfi_flags, native: false, language: ['c', 'cpp', 'objc'])
 endif
 
+have_host_block_device = (targetos != 'darwin' or
+    cc.has_header('IOKit/storage/IOMedia.h'))
+
 #################
 # config-host.h #
 #################
@@ -1247,6 +1250,7 @@ config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h'))
 config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
 config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
 config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
+config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
 
 config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>'))
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2ea294129e..a54f37dbef 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -897,7 +897,8 @@
   'discriminator': 'driver',
   'data': {
       'file': 'BlockStatsSpecificFile',
-      'host_device': 'BlockStatsSpecificFile',
+      'host_device': { 'type': 'BlockStatsSpecificFile',
+                       'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
       'nvme': 'BlockStatsSpecificNvme' } }
 
 ##
@@ -2814,7 +2815,10 @@
 { 'enum': 'BlockdevDriver',
   'data': [ 'blkdebug', 'blklogwrites', 'blkreplay', 'blkverify', 'bochs',
             'cloop', 'compress', 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps',
-            'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'iscsi',
+            'gluster',
+            {'name': 'host_cdrom', 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
+            {'name': 'host_device', 'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
+            'http', 'https', 'iscsi',
             'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels',
             'preallocate', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
             { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },
@@ -3995,8 +3999,10 @@
       'ftp':        'BlockdevOptionsCurlFtp',
       'ftps':       'BlockdevOptionsCurlFtps',
       'gluster':    'BlockdevOptionsGluster',
-      'host_cdrom': 'BlockdevOptionsFile',
-      'host_device':'BlockdevOptionsFile',
+      'host_cdrom':  { 'type': 'BlockdevOptionsFile',
+                       'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
+      'host_device': { 'type': 'BlockdevOptionsFile',
+                       'if': 'defined(HAVE_HOST_BLOCK_DEVICE)' },
       'http':       'BlockdevOptionsCurlHttp',
       'https':      'BlockdevOptionsCurlHttps',
       'iscsi':      'BlockdevOptionsIscsi',
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 20/28] block: check for sys/disk.h
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (18 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 19/28] block: feature detection for host block support Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 21/28] block: try BSD disk size ioctls one after another Paolo Bonzini
                   ` (8 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé, Joelle van Dyne, Max Reitz

From: Joelle van Dyne <j@getutm.app>

Some BSD platforms do not have this header.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Joelle van Dyne <j@getutm.app>
Message-Id: <20210315180341.31638-3-j@getutm.app>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c     | 2 +-
 meson.build | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 3f456892d0..1d37f133a8 100644
--- a/block.c
+++ b/block.c
@@ -54,7 +54,7 @@
 #ifdef CONFIG_BSD
 #include <sys/ioctl.h>
 #include <sys/queue.h>
-#ifndef __DragonFly__
+#if defined(HAVE_SYS_DISK_H)
 #include <sys/disk.h>
 #endif
 #endif
diff --git a/meson.build b/meson.build
index 6419d4ee41..144456426c 100644
--- a/meson.build
+++ b/meson.build
@@ -1251,6 +1251,7 @@ config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
 config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
 config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
 config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
+config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
 
 config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>'))
 
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 21/28] block: try BSD disk size ioctls one after another
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (19 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 20/28] block: check for sys/disk.h Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 22/28] block: detect DKIOCGETBLOCKCOUNT/SIZE before use Paolo Bonzini
                   ` (7 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel

Try all the possible ioctls for disk size as long as they are
supported, to keep the #if ladder simple.

Extracted and cleaned up from a patch by Joelle van Dyne and
Warner Losh.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/file-posix.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index e56bb491a1..f16d987c07 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2327,39 +2327,37 @@ static int64_t raw_getlength(BlockDriverState *bs)
 again:
 #endif
     if (!fstat(fd, &sb) && (S_IFCHR & sb.st_mode)) {
+        size = 0;
 #ifdef DIOCGMEDIASIZE
-        if (ioctl(fd, DIOCGMEDIASIZE, (off_t *)&size))
-#elif defined(DIOCGPART)
-        {
-                struct partinfo pi;
-                if (ioctl(fd, DIOCGPART, &pi) == 0)
-                        size = pi.media_size;
-                else
-                        size = 0;
+        if (ioctl(fd, DIOCGMEDIASIZE, (off_t *)&size)) {
+            size = 0;
+        }
+#endif
+#ifdef DIOCGPART
+        if (size == 0) {
+            struct partinfo pi;
+            if (ioctl(fd, DIOCGPART, &pi) == 0) {
+                size = pi.media_size;
+            }
         }
-        if (size == 0)
 #endif
 #if defined(__APPLE__) && defined(__MACH__)
-        {
+        if (size == 0) {
             uint64_t sectors = 0;
             uint32_t sector_size = 0;
 
             if (ioctl(fd, DKIOCGETBLOCKCOUNT, &sectors) == 0
                && ioctl(fd, DKIOCGETBLOCKSIZE, &sector_size) == 0) {
                 size = sectors * sector_size;
-            } else {
-                size = lseek(fd, 0LL, SEEK_END);
-                if (size < 0) {
-                    return -errno;
-                }
             }
         }
-#else
-        size = lseek(fd, 0LL, SEEK_END);
+#endif
+        if (size == 0) {
+            size = lseek(fd, 0LL, SEEK_END);
+        }
         if (size < 0) {
             return -errno;
         }
-#endif
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
         switch(s->type) {
         case FTYPE_CD:
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 22/28] block: detect DKIOCGETBLOCKCOUNT/SIZE before use
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (20 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 21/28] block: try BSD disk size ioctls one after another Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 23/28] file-posix: handle EINTR during ioctl Paolo Bonzini
                   ` (6 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Joelle van Dyne, Warner Losh

From: Joelle van Dyne <j@getutm.app>

iOS hosts do not have these defined so we fallback to the
default behaviour.

Co-authored-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Joelle van Dyne <j@getutm.app>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index f16d987c07..74b8216077 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2341,7 +2341,7 @@ again:
             }
         }
 #endif
-#if defined(__APPLE__) && defined(__MACH__)
+#if defined(DKIOCGETBLOCKCOUNT) && defined(DKIOCGETBLOCKSIZE)
         if (size == 0) {
             uint64_t sectors = 0;
             uint32_t sector_size = 0;
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 23/28] file-posix: handle EINTR during ioctl
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (21 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 22/28] block: detect DKIOCGETBLOCKCOUNT/SIZE before use Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 24/28] machine: move dies from X86MachineState to CpuTopology Paolo Bonzini
                   ` (5 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gordon Watson

Similar to other handle_aiocb_* functions, handle_aiocb_ioctl needs to cater
for the possibility that ioctl is interrupted by a signal.  Otherwise, the
I/O is incorrectly reported as a failure to the guest.

Reported-by: Gordon Watson <gwatson@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/file-posix.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 74b8216077..a26eab0ac3 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1347,7 +1347,9 @@ static int handle_aiocb_ioctl(void *opaque)
     RawPosixAIOData *aiocb = opaque;
     int ret;
 
-    ret = ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd, aiocb->ioctl.buf);
+    do {
+        ret = ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd, aiocb->ioctl.buf);
+    } while (ret == -1 && errno == EINTR);
     if (ret == -1) {
         return -errno;
     }
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 24/28] machine: move dies from X86MachineState to CpuTopology
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (22 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 23/28] file-posix: handle EINTR during ioctl Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 25/28] machine: move common smp_parse code to caller Paolo Bonzini
                   ` (4 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

In order to make SMP configuration a Machine property, we need a getter as
well as a setter.  To simplify the implementation put everything that the
getter needs in the CpuTopology struct.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210617155308.928754-7-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c     |  1 +
 hw/i386/pc.c          |  4 +---
 hw/i386/x86.c         | 15 +++++++--------
 include/hw/boards.h   |  1 +
 include/hw/i386/pc.h  |  1 -
 include/hw/i386/x86.h |  1 -
 6 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 55b9bc7817..d776c8cf20 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -970,6 +970,7 @@ static void machine_initfn(Object *obj)
     ms->smp.cpus = mc->default_cpus;
     ms->smp.max_cpus = mc->default_cpus;
     ms->smp.cores = 1;
+    ms->smp.dies = 1;
     ms->smp.threads = 1;
     ms->smp.sockets = 1;
 }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c6d8d0d84d..92958e9ad7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -712,8 +712,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
  */
 void pc_smp_parse(MachineState *ms, QemuOpts *opts)
 {
-    X86MachineState *x86ms = X86_MACHINE(ms);
-
     if (opts) {
         unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
         unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
@@ -769,7 +767,7 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
         ms->smp.cores = cores;
         ms->smp.threads = threads;
         ms->smp.sockets = sockets;
-        x86ms->smp_dies = dies;
+        ms->smp.dies = dies;
     }
 
     if (ms->smp.cpus > 1) {
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index d30cf27e29..00448ed55a 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -64,7 +64,7 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
 {
     MachineState *ms = MACHINE(x86ms);
 
-    topo_info->dies_per_pkg = x86ms->smp_dies;
+    topo_info->dies_per_pkg = ms->smp.dies;
     topo_info->cores_per_die = ms->smp.cores;
     topo_info->threads_per_core = ms->smp.threads;
 }
@@ -293,7 +293,7 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
 
     init_topo_info(&topo_info, x86ms);
 
-    env->nr_dies = x86ms->smp_dies;
+    env->nr_dies = ms->smp.dies;
 
     /*
      * If APIC ID is not set,
@@ -301,13 +301,13 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
      */
     if (cpu->apic_id == UNASSIGNED_APIC_ID) {
         int max_socket = (ms->smp.max_cpus - 1) /
-                                smp_threads / smp_cores / x86ms->smp_dies;
+                                smp_threads / smp_cores / ms->smp.dies;
 
         /*
          * die-id was optional in QEMU 4.0 and older, so keep it optional
          * if there's only one die per socket.
          */
-        if (cpu->die_id < 0 && x86ms->smp_dies == 1) {
+        if (cpu->die_id < 0 && ms->smp.dies == 1) {
             cpu->die_id = 0;
         }
 
@@ -322,9 +322,9 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
         if (cpu->die_id < 0) {
             error_setg(errp, "CPU die-id is not set");
             return;
-        } else if (cpu->die_id > x86ms->smp_dies - 1) {
+        } else if (cpu->die_id > ms->smp.dies - 1) {
             error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u",
-                       cpu->die_id, x86ms->smp_dies - 1);
+                       cpu->die_id, ms->smp.dies - 1);
             return;
         }
         if (cpu->core_id < 0) {
@@ -477,7 +477,7 @@ const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms)
                                  &topo_info, &topo_ids);
         ms->possible_cpus->cpus[i].props.has_socket_id = true;
         ms->possible_cpus->cpus[i].props.socket_id = topo_ids.pkg_id;
-        if (x86ms->smp_dies > 1) {
+        if (ms->smp.dies > 1) {
             ms->possible_cpus->cpus[i].props.has_die_id = true;
             ms->possible_cpus->cpus[i].props.die_id = topo_ids.die_id;
         }
@@ -1269,7 +1269,6 @@ static void x86_machine_initfn(Object *obj)
 
     x86ms->smm = ON_OFF_AUTO_AUTO;
     x86ms->acpi = ON_OFF_AUTO_AUTO;
-    x86ms->smp_dies = 1;
     x86ms->pci_irq_mask = ACPI_BUILD_PCI_IRQS;
     x86ms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
     x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3d55d2bd62..87ae5cc300 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -282,6 +282,7 @@ typedef struct DeviceMemoryState {
  */
 typedef struct CpuTopology {
     unsigned int cpus;
+    unsigned int dies;
     unsigned int cores;
     unsigned int threads;
     unsigned int sockets;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1522a3359a..4c2ca6d36a 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -19,7 +19,6 @@
  * PCMachineState:
  * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
  * @boot_cpus: number of present VCPUs
- * @smp_dies: number of dies per one package
  */
 typedef struct PCMachineState {
     /*< private >*/
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 25a1f16f01..6e9244a82c 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -62,7 +62,6 @@ struct X86MachineState {
     unsigned pci_irq_mask;
     unsigned apic_id_limit;
     uint16_t boot_cpus;
-    unsigned smp_dies;
 
     OnOffAuto smm;
     OnOffAuto acpi;
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 25/28] machine: move common smp_parse code to caller
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (23 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 24/28] machine: move dies from X86MachineState to CpuTopology Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 26/28] machine: add error propagation to mc->smp_parse Paolo Bonzini
                   ` (3 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel

Most of smp_parse and pc_smp_parse is guarded by an "if (opts)"
conditional, and the rest is common to both function.  Move the
conditional and the common code to the caller, machine_smp_parse.

Move the replay_add_blocker call after all errors are checked for.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210617155308.928754-8-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c | 114 +++++++++++++++++++++++-----------------------
 hw/i386/pc.c      | 108 ++++++++++++++++++++-----------------------
 2 files changed, 107 insertions(+), 115 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index d776c8cf20..1016ec9e1c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -741,67 +741,59 @@ void machine_set_cpu_numa_node(MachineState *machine,
 
 static void smp_parse(MachineState *ms, QemuOpts *opts)
 {
-    if (opts) {
-        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
-        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
-        unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
-        unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+    unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
+    unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
+    unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
+    unsigned threads = qemu_opt_get_number(opts, "threads", 0);
 
-        /* compute missing values, prefer sockets over cores over threads */
-        if (cpus == 0 || sockets == 0) {
-            cores = cores > 0 ? cores : 1;
-            threads = threads > 0 ? threads : 1;
-            if (cpus == 0) {
-                sockets = sockets > 0 ? sockets : 1;
-                cpus = cores * threads * sockets;
-            } else {
-                ms->smp.max_cpus =
-                        qemu_opt_get_number(opts, "maxcpus", cpus);
-                sockets = ms->smp.max_cpus / (cores * threads);
-            }
-        } else if (cores == 0) {
-            threads = threads > 0 ? threads : 1;
-            cores = cpus / (sockets * threads);
-            cores = cores > 0 ? cores : 1;
-        } else if (threads == 0) {
-            threads = cpus / (cores * sockets);
-            threads = threads > 0 ? threads : 1;
-        } else if (sockets * cores * threads < cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) < "
-                         "smp_cpus (%u)",
-                         sockets, cores, threads, cpus);
-            exit(1);
+    /* compute missing values, prefer sockets over cores over threads */
+    if (cpus == 0 || sockets == 0) {
+        cores = cores > 0 ? cores : 1;
+        threads = threads > 0 ? threads : 1;
+        if (cpus == 0) {
+            sockets = sockets > 0 ? sockets : 1;
+            cpus = cores * threads * sockets;
+        } else {
+            ms->smp.max_cpus =
+                    qemu_opt_get_number(opts, "maxcpus", cpus);
+            sockets = ms->smp.max_cpus / (cores * threads);
         }
-
-        ms->smp.max_cpus =
-                qemu_opt_get_number(opts, "maxcpus", cpus);
-
-        if (ms->smp.max_cpus < cpus) {
-            error_report("maxcpus must be equal to or greater than smp");
-            exit(1);
-        }
-
-        if (sockets * cores * threads != ms->smp.max_cpus) {
-            error_report("Invalid CPU topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) "
-                         "!= maxcpus (%u)",
-                         sockets, cores, threads,
-                         ms->smp.max_cpus);
-            exit(1);
-        }
-
-        ms->smp.cpus = cpus;
-        ms->smp.cores = cores;
-        ms->smp.threads = threads;
-        ms->smp.sockets = sockets;
+    } else if (cores == 0) {
+        threads = threads > 0 ? threads : 1;
+        cores = cpus / (sockets * threads);
+        cores = cores > 0 ? cores : 1;
+    } else if (threads == 0) {
+        threads = cpus / (cores * sockets);
+        threads = threads > 0 ? threads : 1;
+    } else if (sockets * cores * threads < cpus) {
+        error_report("cpu topology: "
+                        "sockets (%u) * cores (%u) * threads (%u) < "
+                        "smp_cpus (%u)",
+                        sockets, cores, threads, cpus);
+        exit(1);
     }
 
-    if (ms->smp.cpus > 1) {
-        Error *blocker = NULL;
-        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
-        replay_add_blocker(blocker);
+    ms->smp.max_cpus =
+            qemu_opt_get_number(opts, "maxcpus", cpus);
+
+    if (ms->smp.max_cpus < cpus) {
+        error_report("maxcpus must be equal to or greater than smp");
+        exit(1);
     }
+
+    if (sockets * cores * threads != ms->smp.max_cpus) {
+        error_report("Invalid CPU topology: "
+                        "sockets (%u) * cores (%u) * threads (%u) "
+                        "!= maxcpus (%u)",
+                        sockets, cores, threads,
+                        ms->smp.max_cpus);
+        exit(1);
+    }
+
+    ms->smp.cpus = cpus;
+    ms->smp.cores = cores;
+    ms->smp.threads = threads;
+    ms->smp.sockets = sockets;
 }
 
 static void machine_class_init(ObjectClass *oc, void *data)
@@ -1135,7 +1127,9 @@ bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
 {
     MachineClass *mc = MACHINE_GET_CLASS(ms);
 
-    mc->smp_parse(ms, opts);
+    if (opts) {
+        mc->smp_parse(ms, opts);
+    }
 
     /* sanity-check smp_cpus and max_cpus against mc */
     if (ms->smp.cpus < mc->min_cpus) {
@@ -1151,6 +1145,12 @@ bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
                    mc->name, mc->max_cpus);
         return false;
     }
+
+    if (ms->smp.cpus > 1) {
+        Error *blocker = NULL;
+        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
+        replay_add_blocker(blocker);
+    }
     return true;
 }
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 92958e9ad7..e206ac85f3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -712,69 +712,61 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
  */
 void pc_smp_parse(MachineState *ms, QemuOpts *opts)
 {
-    if (opts) {
-        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
-        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
-        unsigned dies = qemu_opt_get_number(opts, "dies", 1);
-        unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
-        unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+    unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
+    unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
+    unsigned dies = qemu_opt_get_number(opts, "dies", 1);
+    unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
+    unsigned threads = qemu_opt_get_number(opts, "threads", 0);
 
-        /* compute missing values, prefer sockets over cores over threads */
-        if (cpus == 0 || sockets == 0) {
-            cores = cores > 0 ? cores : 1;
-            threads = threads > 0 ? threads : 1;
-            if (cpus == 0) {
-                sockets = sockets > 0 ? sockets : 1;
-                cpus = cores * threads * dies * sockets;
-            } else {
-                ms->smp.max_cpus =
-                        qemu_opt_get_number(opts, "maxcpus", cpus);
-                sockets = ms->smp.max_cpus / (cores * threads * dies);
-            }
-        } else if (cores == 0) {
-            threads = threads > 0 ? threads : 1;
-            cores = cpus / (sockets * dies * threads);
-            cores = cores > 0 ? cores : 1;
-        } else if (threads == 0) {
-            threads = cpus / (cores * dies * sockets);
-            threads = threads > 0 ? threads : 1;
-        } else if (sockets * dies * cores * threads < cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
-                         "smp_cpus (%u)",
-                         sockets, dies, cores, threads, cpus);
-            exit(1);
+    /* compute missing values, prefer sockets over cores over threads */
+    if (cpus == 0 || sockets == 0) {
+        cores = cores > 0 ? cores : 1;
+        threads = threads > 0 ? threads : 1;
+        if (cpus == 0) {
+            sockets = sockets > 0 ? sockets : 1;
+            cpus = cores * threads * dies * sockets;
+        } else {
+            ms->smp.max_cpus =
+                    qemu_opt_get_number(opts, "maxcpus", cpus);
+            sockets = ms->smp.max_cpus / (cores * threads * dies);
         }
-
-        ms->smp.max_cpus =
-                qemu_opt_get_number(opts, "maxcpus", cpus);
-
-        if (ms->smp.max_cpus < cpus) {
-            error_report("maxcpus must be equal to or greater than smp");
-            exit(1);
-        }
-
-        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
-            error_report("Invalid CPU topology deprecated: "
-                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
-                         "!= maxcpus (%u)",
-                         sockets, dies, cores, threads,
-                         ms->smp.max_cpus);
-            exit(1);
-        }
-
-        ms->smp.cpus = cpus;
-        ms->smp.cores = cores;
-        ms->smp.threads = threads;
-        ms->smp.sockets = sockets;
-        ms->smp.dies = dies;
+    } else if (cores == 0) {
+        threads = threads > 0 ? threads : 1;
+        cores = cpus / (sockets * dies * threads);
+        cores = cores > 0 ? cores : 1;
+    } else if (threads == 0) {
+        threads = cpus / (cores * dies * sockets);
+        threads = threads > 0 ? threads : 1;
+    } else if (sockets * dies * cores * threads < cpus) {
+        error_report("cpu topology: "
+                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
+                        "smp_cpus (%u)",
+                        sockets, dies, cores, threads, cpus);
+        exit(1);
     }
 
-    if (ms->smp.cpus > 1) {
-        Error *blocker = NULL;
-        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
-        replay_add_blocker(blocker);
+    ms->smp.max_cpus =
+            qemu_opt_get_number(opts, "maxcpus", cpus);
+
+    if (ms->smp.max_cpus < cpus) {
+        error_report("maxcpus must be equal to or greater than smp");
+        exit(1);
     }
+
+    if (sockets * dies * cores * threads != ms->smp.max_cpus) {
+        error_report("Invalid CPU topology deprecated: "
+                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+                        "!= maxcpus (%u)",
+                        sockets, dies, cores, threads,
+                        ms->smp.max_cpus);
+        exit(1);
+    }
+
+    ms->smp.cpus = cpus;
+    ms->smp.cores = cores;
+    ms->smp.threads = threads;
+    ms->smp.sockets = sockets;
+    ms->smp.dies = dies;
 }
 
 static
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 26/28] machine: add error propagation to mc->smp_parse
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (24 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 25/28] machine: move common smp_parse code to caller Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 27/28] machine: pass QAPI struct " Paolo Bonzini
                   ` (2 subsequent siblings)
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

Clean up the smp_parse functions to use Error** instead of exiting.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210617155308.928754-9-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c    | 34 +++++++++++++++++++---------------
 hw/i386/pc.c         | 28 ++++++++++++++--------------
 include/hw/boards.h  |  2 +-
 include/hw/i386/pc.h |  2 --
 4 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1016ec9e1c..5a9c97ccc5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -739,7 +739,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
     }
 }
 
-static void smp_parse(MachineState *ms, QemuOpts *opts)
+static void smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
 {
     unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
     unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
@@ -766,28 +766,28 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
         threads = cpus / (cores * sockets);
         threads = threads > 0 ? threads : 1;
     } else if (sockets * cores * threads < cpus) {
-        error_report("cpu topology: "
-                        "sockets (%u) * cores (%u) * threads (%u) < "
-                        "smp_cpus (%u)",
-                        sockets, cores, threads, cpus);
-        exit(1);
+        error_setg(errp, "cpu topology: "
+                   "sockets (%u) * cores (%u) * threads (%u) < "
+                   "smp_cpus (%u)",
+                   sockets, cores, threads, cpus);
+        return;
     }
 
     ms->smp.max_cpus =
             qemu_opt_get_number(opts, "maxcpus", cpus);
 
     if (ms->smp.max_cpus < cpus) {
-        error_report("maxcpus must be equal to or greater than smp");
-        exit(1);
+        error_setg(errp, "maxcpus must be equal to or greater than smp");
+        return;
     }
 
     if (sockets * cores * threads != ms->smp.max_cpus) {
-        error_report("Invalid CPU topology: "
-                        "sockets (%u) * cores (%u) * threads (%u) "
-                        "!= maxcpus (%u)",
-                        sockets, cores, threads,
-                        ms->smp.max_cpus);
-        exit(1);
+        error_setg(errp, "Invalid CPU topology: "
+                   "sockets (%u) * cores (%u) * threads (%u) "
+                   "!= maxcpus (%u)",
+                   sockets, cores, threads,
+                   ms->smp.max_cpus);
+        return;
     }
 
     ms->smp.cpus = cpus;
@@ -1126,9 +1126,13 @@ MemoryRegion *machine_consume_memdev(MachineState *machine,
 bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
 {
     MachineClass *mc = MACHINE_GET_CLASS(ms);
+    ERRP_GUARD();
 
     if (opts) {
-        mc->smp_parse(ms, opts);
+        mc->smp_parse(ms, opts, errp);
+        if (*errp) {
+            return false;
+        }
     }
 
     /* sanity-check smp_cpus and max_cpus against mc */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e206ac85f3..cce275dcb1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -710,7 +710,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
  * This function is very similar to smp_parse()
  * in hw/core/machine.c but includes CPU die support.
  */
-void pc_smp_parse(MachineState *ms, QemuOpts *opts)
+static void pc_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
 {
     unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
     unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
@@ -738,28 +738,28 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
         threads = cpus / (cores * dies * sockets);
         threads = threads > 0 ? threads : 1;
     } else if (sockets * dies * cores * threads < cpus) {
-        error_report("cpu topology: "
-                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
-                        "smp_cpus (%u)",
-                        sockets, dies, cores, threads, cpus);
-        exit(1);
+        error_setg(errp, "cpu topology: "
+                   "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
+                   "smp_cpus (%u)",
+                   sockets, dies, cores, threads, cpus);
+        return;
     }
 
     ms->smp.max_cpus =
             qemu_opt_get_number(opts, "maxcpus", cpus);
 
     if (ms->smp.max_cpus < cpus) {
-        error_report("maxcpus must be equal to or greater than smp");
-        exit(1);
+        error_setg(errp, "maxcpus must be equal to or greater than smp");
+        return;
     }
 
     if (sockets * dies * cores * threads != ms->smp.max_cpus) {
-        error_report("Invalid CPU topology deprecated: "
-                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
-                        "!= maxcpus (%u)",
-                        sockets, dies, cores, threads,
-                        ms->smp.max_cpus);
-        exit(1);
+        error_setg(errp, "Invalid CPU topology deprecated: "
+                   "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+                   "!= maxcpus (%u)",
+                   sockets, dies, cores, threads,
+                   ms->smp.max_cpus);
+        return;
     }
 
     ms->smp.cpus = cpus;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 87ae5cc300..0483d6af86 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -210,7 +210,7 @@ struct MachineClass {
     void (*reset)(MachineState *state);
     void (*wakeup)(MachineState *state);
     int (*kvm_type)(MachineState *machine, const char *arg);
-    void (*smp_parse)(MachineState *ms, QemuOpts *opts);
+    void (*smp_parse)(MachineState *ms, QemuOpts *opts, Error **errp);
 
     BlockInterfaceType block_default_type;
     int units_per_default_bus;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 4c2ca6d36a..87294f2632 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -138,8 +138,6 @@ extern int fd_bootchk;
 
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
-void pc_smp_parse(MachineState *ms, QemuOpts *opts);
-
 void pc_guest_info_init(PCMachineState *pcms);
 
 #define PCI_HOST_PROP_PCI_HOLE_START   "pci-hole-start"
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 27/28] machine: pass QAPI struct to mc->smp_parse
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (25 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 26/28] machine: add error propagation to mc->smp_parse Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-25 14:18 ` [PULL 28/28] machine: reject -smp dies!=1 for non-PC machines Paolo Bonzini
  2021-06-29  8:37 ` [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Peter Maydell
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

As part of converting -smp to a property with a QAPI type, define
the struct and use it to do the actual parsing.  machine_smp_parse
takes care of doing the QemuOpts->QAPI conversion by hand, for now.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210617155308.928754-10-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c   | 33 +++++++++++++++++++++++----------
 hw/i386/pc.c        | 18 ++++++++----------
 include/hw/boards.h |  2 +-
 qapi/machine.json   | 28 ++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 5a9c97ccc5..9ad8341a31 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -739,12 +739,12 @@ void machine_set_cpu_numa_node(MachineState *machine,
     }
 }
 
-static void smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
+static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
 {
-    unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
-    unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
-    unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
-    unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+    unsigned cpus    = config->has_cpus ? config->cpus : 0;
+    unsigned sockets = config->has_sockets ? config->sockets : 0;
+    unsigned cores   = config->has_cores ? config->cores : 0;
+    unsigned threads = config->has_threads ? config->threads : 0;
 
     /* compute missing values, prefer sockets over cores over threads */
     if (cpus == 0 || sockets == 0) {
@@ -754,8 +754,7 @@ static void smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
             sockets = sockets > 0 ? sockets : 1;
             cpus = cores * threads * sockets;
         } else {
-            ms->smp.max_cpus =
-                    qemu_opt_get_number(opts, "maxcpus", cpus);
+            ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
             sockets = ms->smp.max_cpus / (cores * threads);
         }
     } else if (cores == 0) {
@@ -773,8 +772,7 @@ static void smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
         return;
     }
 
-    ms->smp.max_cpus =
-            qemu_opt_get_number(opts, "maxcpus", cpus);
+    ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
 
     if (ms->smp.max_cpus < cpus) {
         error_setg(errp, "maxcpus must be equal to or greater than smp");
@@ -1129,7 +1127,22 @@ bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
     ERRP_GUARD();
 
     if (opts) {
-        mc->smp_parse(ms, opts, errp);
+        SMPConfiguration config = {
+            .has_cpus = !!qemu_opt_get(opts, "cpus"),
+            .cpus = qemu_opt_get_number(opts, "cpus", 0),
+            .has_sockets = !!qemu_opt_get(opts, "sockets"),
+            .sockets = qemu_opt_get_number(opts, "sockets", 0),
+            .has_dies = !!qemu_opt_get(opts, "dies"),
+            .dies = qemu_opt_get_number(opts, "dies", 0),
+            .has_cores = !!qemu_opt_get(opts, "cores"),
+            .cores = qemu_opt_get_number(opts, "cores", 0),
+            .has_threads = !!qemu_opt_get(opts, "threads"),
+            .threads = qemu_opt_get_number(opts, "threads", 0),
+            .has_maxcpus = !!qemu_opt_get(opts, "maxcpus"),
+            .maxcpus = qemu_opt_get_number(opts, "maxcpus", 0),
+        };
+
+        mc->smp_parse(ms, &config, errp);
         if (*errp) {
             return false;
         }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index cce275dcb1..8e1220db72 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -710,13 +710,13 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
  * This function is very similar to smp_parse()
  * in hw/core/machine.c but includes CPU die support.
  */
-static void pc_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
+static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
 {
-    unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
-    unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
-    unsigned dies = qemu_opt_get_number(opts, "dies", 1);
-    unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
-    unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+    unsigned cpus    = config->has_cpus ? config->cpus : 0;
+    unsigned sockets = config->has_sockets ? config->sockets : 0;
+    unsigned dies    = config->has_dies ? config->dies : 1;
+    unsigned cores   = config->has_cores ? config->cores : 0;
+    unsigned threads = config->has_threads ? config->threads : 0;
 
     /* compute missing values, prefer sockets over cores over threads */
     if (cpus == 0 || sockets == 0) {
@@ -726,8 +726,7 @@ static void pc_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
             sockets = sockets > 0 ? sockets : 1;
             cpus = cores * threads * dies * sockets;
         } else {
-            ms->smp.max_cpus =
-                    qemu_opt_get_number(opts, "maxcpus", cpus);
+            ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
             sockets = ms->smp.max_cpus / (cores * threads * dies);
         }
     } else if (cores == 0) {
@@ -745,8 +744,7 @@ static void pc_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
         return;
     }
 
-    ms->smp.max_cpus =
-            qemu_opt_get_number(opts, "maxcpus", cpus);
+    ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
 
     if (ms->smp.max_cpus < cpus) {
         error_setg(errp, "maxcpus must be equal to or greater than smp");
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 0483d6af86..1eae4427e8 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -210,7 +210,7 @@ struct MachineClass {
     void (*reset)(MachineState *state);
     void (*wakeup)(MachineState *state);
     int (*kvm_type)(MachineState *machine, const char *arg);
-    void (*smp_parse)(MachineState *ms, QemuOpts *opts, Error **errp);
+    void (*smp_parse)(MachineState *ms, SMPConfiguration *config, Error **errp);
 
     BlockInterfaceType block_default_type;
     int units_per_default_bus;
diff --git a/qapi/machine.json b/qapi/machine.json
index e4d0f9b24f..c3210ee1fb 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1284,3 +1284,31 @@
 ##
 { 'event': 'MEM_UNPLUG_ERROR',
   'data': { 'device': 'str', 'msg': 'str' } }
+
+##
+# @SMPConfiguration:
+#
+# Schema for CPU topology configuration.  "0" or a missing value lets
+# QEMU figure out a suitable value based on the ones that are provided.
+#
+# @cpus: number of virtual CPUs in the virtual machine
+#
+# @sockets: number of sockets in the CPU topology
+#
+# @dies: number of dies per socket in the CPU topology
+#
+# @cores: number of cores per thread in the CPU topology
+#
+# @threads: number of threads per core in the CPU topology
+#
+# @maxcpus: maximum number of hotpluggable virtual CPUs in the virtual machine
+#
+# Since: 6.1
+##
+{ 'struct': 'SMPConfiguration', 'data': {
+     '*cpus': 'int',
+     '*sockets': 'int',
+     '*dies': 'int',
+     '*cores': 'int',
+     '*threads': 'int',
+     '*maxcpus': 'int' } }
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PULL 28/28] machine: reject -smp dies!=1 for non-PC machines
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (26 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 27/28] machine: pass QAPI struct " Paolo Bonzini
@ 2021-06-25 14:18 ` Paolo Bonzini
  2021-06-29  8:37 ` [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Peter Maydell
  28 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-06-25 14:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P . Berrangé

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210617155308.928754-11-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/machine.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 9ad8341a31..ffc076ae84 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -746,6 +746,10 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
     unsigned cores   = config->has_cores ? config->cores : 0;
     unsigned threads = config->has_threads ? config->threads : 0;
 
+    if (config->has_dies && config->dies != 0 && config->dies != 1) {
+        error_setg(errp, "dies not supported by this machine's CPU topology");
+    }
+
     /* compute missing values, prefer sockets over cores over threads */
     if (cpus == 0 || sockets == 0) {
         cores = cores > 0 ? cores : 1;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PULL 15/28] osdep: provide ROUND_DOWN macro
  2021-06-25 14:18 ` [PULL 15/28] osdep: provide ROUND_DOWN macro Paolo Bonzini
@ 2021-06-29  4:12   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-29  4:12 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 6/25/21 4:18 PM, Paolo Bonzini wrote:
> osdep.h provides a ROUND_UP macro to hide bitwise operations for the
> purpose of rounding a number up to a power of two; add a ROUND_DOWN
> macro that does the same with truncation towards zero.
> 
> While at it, change the formatting of some comments.
> 

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/osdep.h | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23
  2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
                   ` (27 preceding siblings ...)
  2021-06-25 14:18 ` [PULL 28/28] machine: reject -smp dies!=1 for non-PC machines Paolo Bonzini
@ 2021-06-29  8:37 ` Peter Maydell
  28 siblings, 0 replies; 37+ messages in thread
From: Peter Maydell @ 2021-06-29  8:37 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Fri, 25 Jun 2021 at 15:20, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit ecba223da6215d6f6ce2d343b70b2e9a19bfb90b:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210624' into staging (2021-06-24 15:00:34 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 0aebebb561c9c23b9c6d3d58040f83547f059b5c:
>
>   machine: reject -smp dies!=1 for non-PC machines (2021-06-25 16:16:11 +0200)
>
> Starting to flush my own queue before 6.1 soft freeze.  The
> block/file-posix.c patches have been reviewed/acked by Max.
>
> ----------------------------------------------------------------
> * Some Meson test conversions
> * KVM dirty page ring buffer fix
> * KVM TSC scaling support
> * Fixes for SG_IO with /dev/sdX devices
> * (Non)support for host devices on iOS
> * -smp cleanups


Applied, thanks.

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

-- PMM


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2
  2021-06-25 14:18 ` [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2 Paolo Bonzini
@ 2021-09-06 14:24   ` Halil Pasic
  2021-09-22 19:51     ` Halil Pasic
  0 siblings, 1 reply; 37+ messages in thread
From: Halil Pasic @ 2021-09-06 14:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Halil Pasic, Christian Borntraeger, qemu-devel

On Fri, 25 Jun 2021 16:18:12 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> bs->sg is only true for character devices, but block devices can also
> be used with scsi-block and scsi-generic.  Unfortunately BLKSECTGET
> returns bytes in an int for /dev/sgN devices, and sectors in a short
> for block devices, so account for that in the code.
> 
> The maximum transfer also need not be a power of 2 (for example I have
> seen disks with 1280 KiB maximum transfer) so there's no need to pass
> the result through pow2floor.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

We have found that this patch leads to in guest I/O errors when DASD
is used as a source device. I.e. libvirt domain xml wise something like:

    <disk type='block' device='disk'>
      <driver name='qemu' type='raw' cache='none' io='native' iothread='1'/>
      <source dev='/dev/disk/by-id/ccw-XXXXXXX'/>
      <backingStore/>
      <target dev='vdb' bus='virtio'/>
      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0008'/>
    </disk>

I don't think it is the fault of this patch: it LGTM. But it correlates
100%, furthermore the problem seems to be related to the value of
bl.max_iov which now comes from sysfs. 

We are still investigating what is actually wrong. Just wanted to give
everybody a heads-up that this does seem to cause a nasty regression on
s390x, even if the code itself is perfect.

Regards,
Halil


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2
  2021-09-06 14:24   ` Halil Pasic
@ 2021-09-22 19:51     ` Halil Pasic
  2021-09-23  9:18       ` Recent qemu patch results in aio failures with host DASD disks resulting in guest I/O errors Christian Borntraeger
  2021-09-23 10:57       ` [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2 Paolo Bonzini
  0 siblings, 2 replies; 37+ messages in thread
From: Halil Pasic @ 2021-09-22 19:51 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, open list:AIO, qemu-block, qemu-devel, Max Reitz,
	Halil Pasic, Christian Borntraeger, Benjamin LaHaise,
	Alexander Viro, Jan Hoeppner, Stefan Haberland

On Mon, 6 Sep 2021 16:24:20 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:

> On Fri, 25 Jun 2021 16:18:12 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > bs->sg is only true for character devices, but block devices can also
> > be used with scsi-block and scsi-generic.  Unfortunately BLKSECTGET
> > returns bytes in an int for /dev/sgN devices, and sectors in a short
> > for block devices, so account for that in the code.
> > 
> > The maximum transfer also need not be a power of 2 (for example I have
> > seen disks with 1280 KiB maximum transfer) so there's no need to pass
> > the result through pow2floor.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>  
> 
> We have found that this patch leads to in guest I/O errors when DASD
> is used as a source device. I.e. libvirt domain xml wise something like:
> 
>     <disk type='block' device='disk'>
>       <driver name='qemu' type='raw' cache='none' io='native' iothread='1'/>
>       <source dev='/dev/disk/by-id/ccw-XXXXXXX'/>
>       <backingStore/>
>       <target dev='vdb' bus='virtio'/>
>       <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0008'/>
>     </disk>
> 
> I don't think it is the fault of this patch: it LGTM. But it correlates
> 100%, furthermore the problem seems to be related to the value of
> bl.max_iov which now comes from sysfs. 
> 
> We are still investigating what is actually wrong. Just wanted to give
> everybody a heads-up that this does seem to cause a nasty regression on
> s390x, even if the code itself is perfect.
> 

We have figured out what is going on here. The problem seems to be
specific to linux aio, which seems to limit the size of the iovec to
1024 (UIO_MAXIOV).

Because of this requests get rejected with -EINVAL by the io_submit()
syscall. Here comes a real world example:

(gdb) p *laiocb
$5 = {co = 0x3ff700072c0, ctx = 0x3fe60002650, iocb = {data = 0x0, aio_rw_flags = 0, key = 0, 
    aio_lio_opcode = 8, aio_reqprio = 0, aio_fildes = 38, u = {c = {buf = 0x3ff70055bc0, 
        nbytes = 1274, offset = 19977953280, __pad3 = 0, flags = 1, resfd = 43}, v = {
        vec = 0x3ff70055bc0, nr = 0, offset = 19977953280}, poll = {__pad1 = 1023, 
        events = 1879399360}, saddr = {addr = 0x3ff70055bc0, len = 0}}}, ret = -22, 
  nbytes = 20586496, qiov = 0x3ff700382f8, is_read = false, next = {sqe_next = 0x0}}

On the host kernel side, the -EINVAL comes from this line:
https://elixir.bootlin.com/linux/v5.15-rc2/source/lib/iov_iter.c#L1863
in iovec_from_user() roughly via: __do_sys_io_submit()->
io_submit_one() -> aio_write() -> aio_setup_rw() -> __import_iovec().

Based on the offline discussion with the DASD maintainers, and on the
linux in source tree documentation (Documentation/block/queue-sysfs.rst
und Documentation/block/biodoc.rst), I believe that the DASD driver is
not wrong when advertising the value 65535 for queue/max_segments.

I believe QEMU jumps to the wrong conclusion in blk_get_max_iov() or
in virtio_blk_submit_multireq(), I can't really tell because I'm not
sure what the semantic of blk_get_max_iov() is. But if, I had to, I would
bet that blk_get_max_iov() returns the wrong value, when linux aio is
used. I'm not sure what is the exact relationship between max_segments
and max_iov.

One idea on how to fix this would be, to cap max_iov to UIO_MAXIOV
(unconditionally, or when linux aio is used). But I have to admit, I
don't have clarity. I couldn't even find documentation that states
this limitation of linux aio (I didn't look for it too hard though), so
I can not even be sure this is a QEMU problem.

That is why I decided to write this up first, and start a discussion on
who is playing foul with the most relevant people posted. I intend to try
myself fixing the problem once my understanding of it reaches a
sufficient level.

Thanks in advance for your contribution!

Regards,
Halil


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Recent qemu patch results in aio failures with host DASD disks resulting in guest I/O errors
  2021-09-22 19:51     ` Halil Pasic
@ 2021-09-23  9:18       ` Christian Borntraeger
  2021-09-23 10:57       ` [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2 Paolo Bonzini
  1 sibling, 0 replies; 37+ messages in thread
From: Christian Borntraeger @ 2021-09-23  9:18 UTC (permalink / raw)
  To: Halil Pasic, Paolo Bonzini
  Cc: Kevin Wolf, open list:AIO, qemu-block, qemu-devel, Max Reitz,
	Benjamin LaHaise, Alexander Viro, Jan Hoeppner, Stefan Haberland

Am 22.09.21 um 21:51 schrieb Halil Pasic:
> On Mon, 6 Sep 2021 16:24:20 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
>> On Fri, 25 Jun 2021 16:18:12 +0200
>> Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>> bs->sg is only true for character devices, but block devices can also
>>> be used with scsi-block and scsi-generic.  Unfortunately BLKSECTGET
>>> returns bytes in an int for /dev/sgN devices, and sectors in a short
>>> for block devices, so account for that in the code.
>>>
>>> The maximum transfer also need not be a power of 2 (for example I have
>>> seen disks with 1280 KiB maximum transfer) so there's no need to pass
>>> the result through pow2floor.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> We have found that this patch leads to in guest I/O errors when DASD
>> is used as a source device. I.e. libvirt domain xml wise something like:
>>
>>      <disk type='block' device='disk'>
>>        <driver name='qemu' type='raw' cache='none' io='native' iothread='1'/>
>>        <source dev='/dev/disk/by-id/ccw-XXXXXXX'/>
>>        <backingStore/>
>>        <target dev='vdb' bus='virtio'/>
>>        <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0008'/>
>>      </disk>
>>
>> I don't think it is the fault of this patch: it LGTM. But it correlates
>> 100%, furthermore the problem seems to be related to the value of
>> bl.max_iov which now comes from sysfs.
>>
>> We are still investigating what is actually wrong. Just wanted to give
>> everybody a heads-up that this does seem to cause a nasty regression on
>> s390x, even if the code itself is perfect.
>>
> 
> We have figured out what is going on here. The problem seems to be
> specific to linux aio, which seems to limit the size of the iovec to
> 1024 (UIO_MAXIOV).
> 
> Because of this requests get rejected with -EINVAL by the io_submit()
> syscall. Here comes a real world example:
> 
> (gdb) p *laiocb
> $5 = {co = 0x3ff700072c0, ctx = 0x3fe60002650, iocb = {data = 0x0, aio_rw_flags = 0, key = 0,
>      aio_lio_opcode = 8, aio_reqprio = 0, aio_fildes = 38, u = {c = {buf = 0x3ff70055bc0,
>          nbytes = 1274, offset = 19977953280, __pad3 = 0, flags = 1, resfd = 43}, v = {
>          vec = 0x3ff70055bc0, nr = 0, offset = 19977953280}, poll = {__pad1 = 1023,
>          events = 1879399360}, saddr = {addr = 0x3ff70055bc0, len = 0}}}, ret = -22,
>    nbytes = 20586496, qiov = 0x3ff700382f8, is_read = false, next = {sqe_next = 0x0}}
> 
> On the host kernel side, the -EINVAL comes from this line:
> https://elixir.bootlin.com/linux/v5.15-rc2/source/lib/iov_iter.c#L1863
> in iovec_from_user() roughly via: __do_sys_io_submit()->
> io_submit_one() -> aio_write() -> aio_setup_rw() -> __import_iovec().
> 
> Based on the offline discussion with the DASD maintainers, and on the
> linux in source tree documentation (Documentation/block/queue-sysfs.rst
> und Documentation/block/biodoc.rst), I believe that the DASD driver is
> not wrong when advertising the value 65535 for queue/max_segments.
> 
> I believe QEMU jumps to the wrong conclusion in blk_get_max_iov() or
> in virtio_blk_submit_multireq(), I can't really tell because I'm not
> sure what the semantic of blk_get_max_iov() is. But if, I had to, I would
> bet that blk_get_max_iov() returns the wrong value, when linux aio is
> used. I'm not sure what is the exact relationship between max_segments
> and max_iov.
> 
> One idea on how to fix this would be, to cap max_iov to UIO_MAXIOV
> (unconditionally, or when linux aio is used). But I have to admit, I
> don't have clarity. I couldn't even find documentation that states
> this limitation of linux aio (I didn't look for it too hard though), so
> I can not even be sure this is a QEMU problem.
> 
> That is why I decided to write this up first, and start a discussion on
> who is playing foul with the most relevant people posted. I intend to try
> myself fixing the problem once my understanding of it reaches a
> sufficient level.
> 
> Thanks in advance for your contribution!

Changing subject to reflect what we see.
For reference the QEMU patch is

https://git.qemu.org/?p=qemu.git;a=commitdiff;h=18473467d55a20d643b6c9b3a52de42f705b4d35;hp=24b36e9813ec15da7db62e3b3621730710c5f020


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2
  2021-09-22 19:51     ` Halil Pasic
  2021-09-23  9:18       ` Recent qemu patch results in aio failures with host DASD disks resulting in guest I/O errors Christian Borntraeger
@ 2021-09-23 10:57       ` Paolo Bonzini
  2021-09-23 12:13         ` Halil Pasic
  1 sibling, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2021-09-23 10:57 UTC (permalink / raw)
  To: Halil Pasic
  Cc: Kevin Wolf, open list:AIO, qemu-block, qemu-devel, Max Reitz,
	Christian Borntraeger, Benjamin LaHaise, Alexander Viro,
	Jan Hoeppner, Stefan Haberland

On 22/09/21 21:51, Halil Pasic wrote:
> We have figured out what is going on here. The problem seems to be
> specific to linux aio, which seems to limit the size of the iovec to
> 1024 (UIO_MAXIOV).

Hi Halil,

I'll send a patch shortly to fix this issue.  Sorry about the delay as I 
was busy with KVM Forum and all that. :(

Paolo


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2
  2021-09-23 10:57       ` [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2 Paolo Bonzini
@ 2021-09-23 12:13         ` Halil Pasic
  2021-09-23 13:02           ` Paolo Bonzini
  0 siblings, 1 reply; 37+ messages in thread
From: Halil Pasic @ 2021-09-23 12:13 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, open list:AIO, qemu-block, qemu-devel, Max Reitz,
	Halil Pasic, Christian Borntraeger, Benjamin LaHaise,
	Alexander Viro, Jan Hoeppner, Stefan Haberland

On Thu, 23 Sep 2021 12:57:38 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 22/09/21 21:51, Halil Pasic wrote:
> > We have figured out what is going on here. The problem seems to be
> > specific to linux aio, which seems to limit the size of the iovec to
> > 1024 (UIO_MAXIOV).  
> 
> Hi Halil,
> 
> I'll send a patch shortly to fix this issue.  Sorry about the delay as I 
> was busy with KVM Forum and all that. :(

Hi Paolo!

With some guidance I could cook up a patch myself. But if you prefer to
do it yourself, I will be glad to test the fix (please put me on cc).
Thanks!

Regards,
Halil



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2
  2021-09-23 12:13         ` Halil Pasic
@ 2021-09-23 13:02           ` Paolo Bonzini
  0 siblings, 0 replies; 37+ messages in thread
From: Paolo Bonzini @ 2021-09-23 13:02 UTC (permalink / raw)
  To: Halil Pasic
  Cc: Kevin Wolf, open list:AIO, qemu-block, qemu-devel, Max Reitz,
	Christian Borntraeger, Benjamin LaHaise, Alexander Viro,
	Jan Hoeppner, Stefan Haberland

On 23/09/21 14:13, Halil Pasic wrote:
> On Thu, 23 Sep 2021 12:57:38 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> On 22/09/21 21:51, Halil Pasic wrote:
>>> We have figured out what is going on here. The problem seems to be
>>> specific to linux aio, which seems to limit the size of the iovec to
>>> 1024 (UIO_MAXIOV).
>>
>> Hi Halil,
>>
>> I'll send a patch shortly to fix this issue.  Sorry about the delay as I
>> was busy with KVM Forum and all that. :(
> 
> Hi Paolo!
> 
> With some guidance I could cook up a patch myself. But if you prefer to
> do it yourself, I will be glad to test the fix (please put me on cc).
> Thanks!

NP, the patch already existed but I was busy and didn't send it out.

Paolo



^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2021-09-23 13:05 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-25 14:17 [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Paolo Bonzini
2021-06-25 14:17 ` [PULL 01/28] target/i386: kvm: add support for TSC scaling Paolo Bonzini
2021-06-25 14:17 ` [PULL 02/28] meson: drop unused CONFIG_GCRYPT_HMAC Paolo Bonzini
2021-06-25 14:17 ` [PULL 03/28] configure: drop unused variables for xts Paolo Bonzini
2021-06-25 14:17 ` [PULL 04/28] meson: remove preadv from summary Paolo Bonzini
2021-06-25 14:17 ` [PULL 05/28] tests: remove QCRYPTO_HAVE_TLS_TEST_SUPPORT Paolo Bonzini
2021-06-25 14:18 ` [PULL 06/28] configure, meson: convert crypto detection to meson Paolo Bonzini
2021-06-25 14:18 ` [PULL 07/28] configure, meson: convert libtasn1 " Paolo Bonzini
2021-06-25 14:18 ` [PULL 08/28] configure, meson: convert pam " Paolo Bonzini
2021-06-25 14:18 ` [PULL 09/28] configure, meson: convert libusb " Paolo Bonzini
2021-06-25 14:18 ` [PULL 10/28] configure, meson: convert libcacard " Paolo Bonzini
2021-06-25 14:18 ` [PULL 11/28] configure, meson: convert libusbredir " Paolo Bonzini
2021-06-25 14:18 ` [PULL 12/28] KVM: Fix dirty ring mmap incorrect size due to renaming accident Paolo Bonzini
2021-06-25 14:18 ` [PULL 13/28] file-posix: fix max_iov for /dev/sg devices Paolo Bonzini
2021-06-25 14:18 ` [PULL 14/28] scsi-generic: pass max_segments via max_iov field in BlockLimits Paolo Bonzini
2021-06-25 14:18 ` [PULL 15/28] osdep: provide ROUND_DOWN macro Paolo Bonzini
2021-06-29  4:12   ` Philippe Mathieu-Daudé
2021-06-25 14:18 ` [PULL 16/28] block-backend: align max_transfer to request alignment Paolo Bonzini
2021-06-25 14:18 ` [PULL 17/28] block: add max_hw_transfer to BlockLimits Paolo Bonzini
2021-06-25 14:18 ` [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2 Paolo Bonzini
2021-09-06 14:24   ` Halil Pasic
2021-09-22 19:51     ` Halil Pasic
2021-09-23  9:18       ` Recent qemu patch results in aio failures with host DASD disks resulting in guest I/O errors Christian Borntraeger
2021-09-23 10:57       ` [PULL 18/28] file-posix: try BLKSECTGET on block devices too, do not round to power of 2 Paolo Bonzini
2021-09-23 12:13         ` Halil Pasic
2021-09-23 13:02           ` Paolo Bonzini
2021-06-25 14:18 ` [PULL 19/28] block: feature detection for host block support Paolo Bonzini
2021-06-25 14:18 ` [PULL 20/28] block: check for sys/disk.h Paolo Bonzini
2021-06-25 14:18 ` [PULL 21/28] block: try BSD disk size ioctls one after another Paolo Bonzini
2021-06-25 14:18 ` [PULL 22/28] block: detect DKIOCGETBLOCKCOUNT/SIZE before use Paolo Bonzini
2021-06-25 14:18 ` [PULL 23/28] file-posix: handle EINTR during ioctl Paolo Bonzini
2021-06-25 14:18 ` [PULL 24/28] machine: move dies from X86MachineState to CpuTopology Paolo Bonzini
2021-06-25 14:18 ` [PULL 25/28] machine: move common smp_parse code to caller Paolo Bonzini
2021-06-25 14:18 ` [PULL 26/28] machine: add error propagation to mc->smp_parse Paolo Bonzini
2021-06-25 14:18 ` [PULL 27/28] machine: pass QAPI struct " Paolo Bonzini
2021-06-25 14:18 ` [PULL 28/28] machine: reject -smp dies!=1 for non-PC machines Paolo Bonzini
2021-06-29  8:37 ` [PULL v2 00/28] Misc (including block file-posix) for 2021-06-23 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).