[PATCH] tests: ignore $__CF_USER_TEXT_ENCODING in env during commandtest
This environment variable is supposedly set according to the contents
of ~/.CFUserTextEncoding, and certainly on MacOS 14 (Sonoma) it shows
up in the environment, causing commandtest to fail. However, the value
that is shown in $__CF_USER_TEXT_ENCODING during the test 1) is not in
the environment of the shell the test is run from, and 2) doesn't
match the contents of ~/.CFUserTextEncoding.
It is true, though, that filtering out this environment setting from
the test results permits commandtest to pass on MacOS 14.
Signed-off-by: Laine Stump
---
[*] There may be a better way to suppress this environment setting
(maybe something done to prevent it from ever being added to the
environment in the first place?), and that would be fine too. This
patch does work though.
[*] Andrea's patches to force rpcgen to generate ANSI C code are also
required for the test suite to pass on MacOS 14.
tests/commandhelper.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index 9b56feb120..08ee48a3a8 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -169,9 +169,12 @@ static int printEnvironment(FILE *log)
for (i = 0; i < length; i++) {
/* Ignore the variables used to instruct the loader into
- * behaving differently, as they could throw the tests off. */
-if (!STRPREFIX(newenv[i], "LD_"))
+ * behaving differently, as they could throw the tests off.
+ * Also ignore __CF_USER_TEXT_ENCODING, which is set by MacOS. */
+if (!STRPREFIX(newenv[i], "LD_") &&
+!STRPREFIX(newenv[i], "__CF_USER_TEXT_ENCODING=")) {
fprintf(log, "ENV:%s\n", newenv[i]);
+}
}
return 0;
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [libvirt PATCH 0/2] rpc: Make rpcgen produce ANSI C code
On 11/2/23 7:37 PM, Andrea Bolognani wrote: Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1059558331 Should make it possible to build libvirt on macOS 14, where it currently fails with src/rpc/virnetprotocol.c:13:1: error: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Werror,-Wdeprecated-non-prototype] xdr_virNetMessageType(xdrs, objp) Andrea Bolognani (2): rpc: Drop support for portable-rpcgen rpc: Make rpcgen produce ANSI C code meson.build| 12 +--- src/rpc/genprotocol.pl | 5 ++--- 2 files changed, 3 insertions(+), 14 deletions(-) Reviewed-by: Laine Stump although I had to apply the patches manually (git am -3 failed). ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 2/2] rpc: Make rpcgen produce ANSI C code
This is the default for the version of rpcgen shipped with Linux distributions, but the one in macOS and possibly others default to K C, which modern compilers don't appreciate. Luckily, all versions of rpcgen shipped with our target platforms seem to support the -C option. Signed-off-by: Andrea Bolognani --- src/rpc/genprotocol.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rpc/genprotocol.pl b/src/rpc/genprotocol.pl index 6c664f48e7..5eb654cb7c 100755 --- a/src/rpc/genprotocol.pl +++ b/src/rpc/genprotocol.pl @@ -39,6 +39,8 @@ my $target = shift; unlink $target; +$rpcgen = "$rpcgen -C"; + open RPCGEN, "-|", "$rpcgen $mode $xdrdef" or die "cannot run $rpcgen $mode $xdrdef: $!"; open TARGET, ">$target" -- 2.41.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 1/2] rpc: Drop support for portable-rpcgen
We use the native version of rpcgen everywhere.
Signed-off-by: Andrea Bolognani
---
meson.build| 12 +---
src/rpc/genprotocol.pl | 3 ---
2 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/meson.build b/meson.build
index dc0969abcc..158a7cae01 100644
--- a/meson.build
+++ b/meson.build
@@ -779,14 +779,11 @@ endif
required_programs = [
'perl',
'python3',
+ 'rpcgen',
'xmllint',
'xsltproc',
]
-required_programs_groups = [
- { 'name': 'rpcgen', 'prog': [ 'rpcgen', 'portable-rpcgen' ] },
-]
-
if host_machine.system() == 'freebsd'
required_programs += 'ifconfig'
endif
@@ -798,13 +795,6 @@ foreach name : required_programs
set_variable('@0@_prog'.format(varname), prog)
endforeach
-foreach item : required_programs_groups
- prog = find_program(item.get('prog'), dirs: libvirt_sbin_path)
- varname = item.get('name').underscorify()
- conf.set_quoted(varname.to_upper(), prog.full_path())
- set_variable('@0@_prog'.format(varname), prog)
-endforeach
-
# optional programs
optional_programs = [
diff --git a/src/rpc/genprotocol.pl b/src/rpc/genprotocol.pl
index adf3991d7a..6c664f48e7 100755
--- a/src/rpc/genprotocol.pl
+++ b/src/rpc/genprotocol.pl
@@ -39,9 +39,6 @@ my $target = shift;
unlink $target;
-if ($rpcgen =~ /portable-rpcgen/) {
-$rpcgen = "$rpcgen -o -";
-}
open RPCGEN, "-|", "$rpcgen $mode $xdrdef"
or die "cannot run $rpcgen $mode $xdrdef: $!";
open TARGET, ">$target"
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 0/2] rpc: Make rpcgen produce ANSI C code
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1059558331 Should make it possible to build libvirt on macOS 14, where it currently fails with src/rpc/virnetprotocol.c:13:1: error: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Werror,-Wdeprecated-non-prototype] xdr_virNetMessageType(xdrs, objp) Andrea Bolognani (2): rpc: Drop support for portable-rpcgen rpc: Make rpcgen produce ANSI C code meson.build| 12 +--- src/rpc/genprotocol.pl | 5 ++--- 2 files changed, 3 insertions(+), 14 deletions(-) -- 2.41.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: mingw rpm build busted? (was: Release of libvirt-9.9.0)
On Wed, Nov 01, 2023 at 05:21:18PM -0400, Cole Robinson wrote: > On 11/1/23 11:02 AM, Daniel P. Berrangé wrote: > > On Wed, Nov 01, 2023 at 10:54:25AM -0400, Cole Robinson wrote: > >> RPM build is busted in mingw meson step. rawhide log: > >> https://kojipkgs.fedoraproject.org//work/tasks/8354/108418354/build.log > >> > >> Error is: > >> > >> ../meson.build:2050:4: ERROR: Problem encountered: cannot enable > >> expensive tests when tests are disabled > >> > >> I'm getting it on up to date f38 as well. Anyone else seeing this? meson > >> 1.2.3 and 1.2.2 both affected > > > > That will be exposed by > > > > ommit 8ce0decc372051d616018f57ae268e2f03082eec > > Author: Andrea Bolognani > > Date: Tue Oct 3 14:53:08 2023 +0200 > > > > meson: Make -Dexpensive_tests depend on -Dtests > > > > It only makes sense to enable expensive tests when tests are > > enabled. Disallow invalid configurations. > > > > Signed-off-by: Andrea Bolognani > > Reviewed-by: Michal Privoznik > > Reviewed-by: Martin Kletzander > > > > the bug is that libvirt.spec.in is passing > > > > -Dexpensive_tests=enabled \ > > -Dtests=disabled \ > > > > to %mingw_meson which is a nonsensical combniation. Flip expensive tests > > to disabled for mingw. > > Thanks Daniel, that fixes things for me Apologies for failing to take into consideration the MinGW part of the spec file. Again. On the other hand, I think this highlights the fact that our coverage in the area is not adequate. Patches to fix the issue you've reported and hopefully ensure that similar mistakes will not sneak in going forward: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/Y2W2PMWBS2EGGFNEMEZM5L5XNQ2EJSUJ/ -- Andrea Bolognani / Red Hat / Virtualization ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 09/11] rpm: Introduce with_native
The new _without_native knob makes it possible to skip the native
build completely and build for MinGW only.
Best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 395 +---
1 file changed, 210 insertions(+), 185 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 75fd0a2771..9626de4662 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -203,6 +203,9 @@
%define qemu_moddir %{_libdir}/qemu
%define qemu_datadir %{_datadir}/qemu
+# Native / MinGW builds
+%define with_native 0%{!?_without_native:1}
+
%define with_mingw32 0
%define with_mingw64 0
@@ -218,6 +221,13 @@
%define mingw_build_win64 %{with_mingw64}
%endif
+%if !%{with_native}
+# Building the debugsource package apparently only works if the
+# native build is enabled. debuginfo packages don't have this
+# problem and setting this doesn't disable them
+%global debug_package %{nil}
+%endif
+
# RHEL releases provide stable tool chains and so it is safe to turn
# compiler warning into errors without being worried about frequent
# changes in reported warnings
@@ -305,10 +315,11 @@ BuildRequires: systemd-rpm-macros
BuildRequires: rpcgen
# Fedora build root suckage
BuildRequires: gawk
+%if %{with_native}
BuildRequires: gcc
-%if %{with_libxl}
+%if %{with_libxl}
BuildRequires: xen-devel
-%endif
+%endif
BuildRequires: glib2-devel >= 2.56
BuildRequires: libxml2-devel
BuildRequires: readline-devel
@@ -323,9 +334,9 @@ BuildRequires: augeas
BuildRequires: systemd-devel >= 185
BuildRequires: libpciaccess-devel >= 0.10.9
BuildRequires: yajl-devel
-%if %{with_sanlock}
+%if %{with_sanlock}
BuildRequires: sanlock-devel >= 2.4
-%endif
+%endif
BuildRequires: libpcap-devel >= 1.5.0
BuildRequires: libnl3-devel
BuildRequires: libselinux-devel
@@ -337,59 +348,59 @@ BuildRequires: cyrus-sasl-devel
BuildRequires: polkit >= 0.112
# For mount/umount in FS driver
BuildRequires: util-linux
-%if %{with_qemu}
+%if %{with_qemu}
# For managing ACLs
BuildRequires: libacl-devel
# From QEMU RPMs, used by virstoragetest
BuildRequires: /usr/bin/qemu-img
-%endif
+%endif
# nbdkit support requires libnbd
-%if %{with_nbdkit}
+%if %{with_nbdkit}
BuildRequires: libnbd-devel
-%endif
+%endif
# For LVM drivers
BuildRequires: lvm2
# For pool type=iscsi
BuildRequires: iscsi-initiator-utils
-%if %{with_storage_iscsi_direct}
+%if %{with_storage_iscsi_direct}
# For pool type=iscsi-direct
BuildRequires: libiscsi-devel
-%endif
+%endif
# For disk driver
BuildRequires: parted-devel
# For Multipath support
BuildRequires: device-mapper-devel
-%if %{with_storage_rbd}
+%if %{with_storage_rbd}
BuildRequires: librados-devel
BuildRequires: librbd-devel
-%endif
-%if %{with_storage_gluster}
+%endif
+%if %{with_storage_gluster}
BuildRequires: glusterfs-api-devel >= 3.4.1
BuildRequires: glusterfs-devel >= 3.4.1
-%endif
-%if %{with_numactl}
+%endif
+%if %{with_numactl}
# For QEMU/LXC numa info
BuildRequires: numactl-devel
-%endif
+%endif
BuildRequires: libcap-ng-devel >= 0.5.0
-%if %{with_fuse}
+%if %{with_fuse}
BuildRequires: fuse-devel >= 2.8.6
-%endif
-%if %{with_libssh2}
+%endif
+%if %{with_libssh2}
BuildRequires: libssh2-devel >= 1.3.0
-%endif
-%if %{with_netcf}
+%endif
+%if %{with_netcf}
BuildRequires: netcf-devel >= 0.2.2
-%endif
-%if 0%{?fedora} || 0%{?rhel} >= 9
+%endif
+%if 0%{?fedora} || 0%{?rhel} >= 9
BuildRequires: passt
-%endif
-%if %{with_esx}
+%endif
+%if %{with_esx}
BuildRequires: libcurl-devel
-%endif
-%if %{with_hyperv}
+%endif
+%if %{with_hyperv}
BuildRequires: libwsman-devel >= 2.6.3
-%endif
+%endif
BuildRequires: audit-libs-devel
# we need /usr/sbin/dtrace
BuildRequires: systemtap-sdt-devel
@@ -399,19 +410,20 @@ BuildRequires: util-linux
BuildRequires: nfs-utils
# For storage wiping with different algorithms
BuildRequires: scrub
-%if %{with_numad}
+%if %{with_numad}
BuildRequires: numad
-%endif
-%if %{with_wireshark}
+%endif
+%if %{with_wireshark}
BuildRequires: wireshark-devel
-%endif
-%if %{with_libssh}
+%endif
+%if %{with_libssh}
BuildRequires: libssh-devel >= 0.8.1
-%endif
+%endif
BuildRequires: libtirpc-devel
+%if %{with_firewalld_zone}
# Needed for the firewalld_reload macro
-%if %{with_firewalld_zone}
BuildRequires: firewalld-filesystem
+%endif
%endif
%if %{with_mingw32}
@@ -450,6 +462,7 @@ Libvirt is a C toolkit to interact with the virtualization
capabilities
of recent versions of Linux (and other OSes). The main package includes
the libvirtd server exporting the virtualization support.
+%if %{with_native}
%package docs
Summary: API reference and website documentation
@@ -489,10 +502,10 @@ Requires: iproute
# for /sbin/tc
Requires: iproute-tc
Requires: polkit >= 0.112
-%if %{with_dmidecode}
+%if %{with_dmidecode}
# For virConnectGetSysinfo
[libvirt PATCH 10/11] ci: Refresh generated files
The native version of gettext is now included in MinGW containers.
Signed-off-by: Andrea Bolognani
---
ci/buildenv/fedora-38-cross-mingw32.sh| 1 +
ci/buildenv/fedora-38-cross-mingw64.sh| 1 +
ci/buildenv/fedora-rawhide-cross-mingw32.sh | 1 +
ci/buildenv/fedora-rawhide-cross-mingw64.sh | 1 +
ci/containers/fedora-38-cross-mingw32.Dockerfile | 1 +
ci/containers/fedora-38-cross-mingw64.Dockerfile | 1 +
ci/containers/fedora-rawhide-cross-mingw32.Dockerfile | 1 +
ci/containers/fedora-rawhide-cross-mingw64.Dockerfile | 1 +
8 files changed, 8 insertions(+)
diff --git a/ci/buildenv/fedora-38-cross-mingw32.sh
b/ci/buildenv/fedora-38-cross-mingw32.sh
index babfa6450a..f909705ed9 100644
--- a/ci/buildenv/fedora-38-cross-mingw32.sh
+++ b/ci/buildenv/fedora-38-cross-mingw32.sh
@@ -18,6 +18,7 @@ function install_buildenv() {
dwarves \
ebtables \
firewalld-filesystem \
+gettext \
git \
glibc-langpack-en \
grep \
diff --git a/ci/buildenv/fedora-38-cross-mingw64.sh
b/ci/buildenv/fedora-38-cross-mingw64.sh
index 18ae4c3f3d..d5173a68c5 100644
--- a/ci/buildenv/fedora-38-cross-mingw64.sh
+++ b/ci/buildenv/fedora-38-cross-mingw64.sh
@@ -18,6 +18,7 @@ function install_buildenv() {
dwarves \
ebtables \
firewalld-filesystem \
+gettext \
git \
glibc-langpack-en \
grep \
diff --git a/ci/buildenv/fedora-rawhide-cross-mingw32.sh
b/ci/buildenv/fedora-rawhide-cross-mingw32.sh
index 218159f17d..8d20d750e6 100644
--- a/ci/buildenv/fedora-rawhide-cross-mingw32.sh
+++ b/ci/buildenv/fedora-rawhide-cross-mingw32.sh
@@ -19,6 +19,7 @@ function install_buildenv() {
dwarves \
ebtables \
firewalld-filesystem \
+gettext \
git \
glibc-langpack-en \
grep \
diff --git a/ci/buildenv/fedora-rawhide-cross-mingw64.sh
b/ci/buildenv/fedora-rawhide-cross-mingw64.sh
index 42a3df41f5..107f618409 100644
--- a/ci/buildenv/fedora-rawhide-cross-mingw64.sh
+++ b/ci/buildenv/fedora-rawhide-cross-mingw64.sh
@@ -19,6 +19,7 @@ function install_buildenv() {
dwarves \
ebtables \
firewalld-filesystem \
+gettext \
git \
glibc-langpack-en \
grep \
diff --git a/ci/containers/fedora-38-cross-mingw32.Dockerfile
b/ci/containers/fedora-38-cross-mingw32.Dockerfile
index 7d3267b0f8..f7b659ac52 100644
--- a/ci/containers/fedora-38-cross-mingw32.Dockerfile
+++ b/ci/containers/fedora-38-cross-mingw32.Dockerfile
@@ -29,6 +29,7 @@ exec "$@"\n' > /usr/bin/nosync && \
dwarves \
ebtables \
firewalld-filesystem \
+ gettext \
git \
glibc-langpack-en \
grep \
diff --git a/ci/containers/fedora-38-cross-mingw64.Dockerfile
b/ci/containers/fedora-38-cross-mingw64.Dockerfile
index 286b22c434..bd346b9d49 100644
--- a/ci/containers/fedora-38-cross-mingw64.Dockerfile
+++ b/ci/containers/fedora-38-cross-mingw64.Dockerfile
@@ -29,6 +29,7 @@ exec "$@"\n' > /usr/bin/nosync && \
dwarves \
ebtables \
firewalld-filesystem \
+ gettext \
git \
glibc-langpack-en \
grep \
diff --git a/ci/containers/fedora-rawhide-cross-mingw32.Dockerfile
b/ci/containers/fedora-rawhide-cross-mingw32.Dockerfile
index 113e18159b..be730cf956 100644
--- a/ci/containers/fedora-rawhide-cross-mingw32.Dockerfile
+++ b/ci/containers/fedora-rawhide-cross-mingw32.Dockerfile
@@ -30,6 +30,7 @@ exec "$@"\n' > /usr/bin/nosync && \
dwarves \
ebtables \
firewalld-filesystem \
+ gettext \
git \
glibc-langpack-en \
grep \
diff --git a/ci/containers/fedora-rawhide-cross-mingw64.Dockerfile
b/ci/containers/fedora-rawhide-cross-mingw64.Dockerfile
index 19408fd177..ef5952dc30 100644
--- a/ci/containers/fedora-rawhide-cross-mingw64.Dockerfile
+++ b/ci/containers/fedora-rawhide-cross-mingw64.Dockerfile
@@ -30,6 +30,7 @@ exec "$@"\n' > /usr/bin/nosync && \
dwarves \
ebtables \
firewalld-filesystem \
+ gettext \
git \
glibc-langpack-en \
grep \
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 11/11] ci: Build RPMs on MinGW
Now that the spec file supports selectively disabling the native,
mingw32 and mingw64 parts, we can add coverage for the MinGW RPM
builds.
Signed-off-by: Andrea Bolognani
---
.gitlab-ci.yml | 11 ---
ci/jobs.sh | 19 ++-
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 1cdabed941..42c43a556e 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -57,10 +57,15 @@ include:
key: "$CI_JOB_NAME"
script:
- source ci/jobs.sh
-- run_build
-- if test "$CROSS" = "i686" ;
+- if test -x /usr/bin/rpmbuild && test "$RPM" != "skip";
then
-run_test;
+run_rpmbuild;
+ else
+run_build;
+if test "$CROSS" = "i686";
+then
+ run_test;
+fi;
fi
.cross_build_job_prebuilt_env:
diff --git a/ci/jobs.sh b/ci/jobs.sh
index ba522258a1..8f7748aab8 100644
--- a/ci/jobs.sh
+++ b/ci/jobs.sh
@@ -72,10 +72,27 @@ run_potfile() {
run_rpmbuild() {
run_dist
+
+case "$CROSS" in
+mingw32)
+without_feature1="_without_native 1"
+without_feature2="_without_mingw64 1"
+;;
+mingw64)
+without_feature1="_without_native 1"
+without_feature2="_without_mingw32 1"
+;;
+*)
+without_feature1="_without_mingw32 1"
+without_feature2="_without_mingw64 1"
+;;
+esac
+
run_cmd rpmbuild \
--clean \
--nodeps \
---define "_without_mingw 1" \
+--define "$without_feature1" \
+--define "$without_feature2" \
-ta build/meson-dist/libvirt-*.tar.xz
}
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 08/11] rpm: Introduce with_mingw32/with_mingw64
These replace the existing with_mingw but offer additional
granularity.
The existing _without_mingw knob retains its behavior of
disabling all MinGW builds at once for convenience, while the
newly introduced _without_mingw32/_without_mingw64 knobs make
it possible to disable only one of them.
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 49 -
1 file changed, 36 insertions(+), 13 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 6a999860f5..75fd0a2771 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -203,9 +203,19 @@
%define qemu_moddir %{_libdir}/qemu
%define qemu_datadir %{_datadir}/qemu
-%define with_mingw 0
+%define with_mingw32 0
+%define with_mingw64 0
+
%if 0%{?fedora}
-%define with_mingw 0%{!?_without_mingw:1}
+%if 0%{!?_without_mingw:1}
+%define with_mingw32 0%{!?_without_mingw32:1}
+%define with_mingw64 0%{!?_without_mingw64:1}
+%endif
+
+# These tell the other mingw macros whether to perform or
+# skip the 32-bit and 64-bit specific steps respectively
+%define mingw_build_win32 %{with_mingw32}
+%define mingw_build_win64 %{with_mingw64}
%endif
# RHEL releases provide stable tool chains and so it is safe to turn
@@ -404,7 +414,7 @@ BuildRequires: libtirpc-devel
BuildRequires: firewalld-filesystem
%endif
-%if %{with_mingw}
+%if %{with_mingw32}
BuildRequires: mingw32-filesystem
BuildRequires: mingw32-gcc
BuildRequires: mingw32-binutils
@@ -418,6 +428,8 @@ BuildRequires: mingw32-portablexdr
BuildRequires: mingw32-dlfcn
BuildRequires: mingw32-libssh2
BuildRequires: mingw32-curl
+%endif
+%if %{with_mingw64}
BuildRequires: mingw64-filesystem
BuildRequires: mingw64-gcc
BuildRequires: mingw64-binutils
@@ -1063,7 +1075,7 @@ Requires: libvirt-daemon-driver-network =
%{version}-%{release}
%description nss
Libvirt plugin for NSS for translating domain names into IP addresses.
-%if %{with_mingw}
+%if %{with_mingw32}
%package -n mingw32-libvirt
Summary: %{summary}
Obsoletes: mingw32-libvirt-static < 7.0.0
@@ -1072,6 +1084,10 @@ BuildArch: noarch
%description -n mingw32-libvirt
MinGW Windows libvirt virtualization library.
+%{?mingw32_debug_package}
+%endif
+
+%if %{with_mingw64}
%package -n mingw64-libvirt
Summary: %{summary}
Obsoletes: mingw64-libvirt-static < 7.0.0
@@ -1080,7 +1096,6 @@ BuildArch: noarch
%description -n mingw64-libvirt
MinGW Windows libvirt virtualization library.
-%{?mingw32_debug_package}
%{?mingw64_debug_package}
%endif
@@ -1326,7 +1341,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
%meson_build
-%if %{with_mingw}
+%if %{with_mingw32} || %{with_mingw64}
%mingw_meson \
--auto-features=enabled \
-Ddriver_remote=enabled \
@@ -1472,21 +1487,27 @@ mv
$RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
%endif
%endif
-%if %{with_mingw}
+%if %{with_mingw32} || %{with_mingw64}
%mingw_ninja_install
+%endif
+%if %{with_mingw32}
rm -rf $RPM_BUILD_ROOT%{mingw32_sysconfdir}/libvirt/nwfilter
-rm -rf $RPM_BUILD_ROOT%{mingw64_sysconfdir}/libvirt/nwfilter
rm -rf $RPM_BUILD_ROOT%{mingw32_datadir}/doc/*
-rm -rf $RPM_BUILD_ROOT%{mingw64_datadir}/doc/*
rm -rf $RPM_BUILD_ROOT%{mingw32_datadir}/gtk-doc/*
-rm -rf $RPM_BUILD_ROOT%{mingw64_datadir}/gtk-doc/*
-
rm -rf $RPM_BUILD_ROOT%{mingw32_libexecdir}/libvirt_iohelper.exe
-rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt_iohelper.exe
rm -rf $RPM_BUILD_ROOT%{mingw32_libexecdir}/libvirt-guests.sh
+%endif
+
+%if %{with_mingw64}
+rm -rf $RPM_BUILD_ROOT%{mingw64_sysconfdir}/libvirt/nwfilter
+rm -rf $RPM_BUILD_ROOT%{mingw64_datadir}/doc/*
+rm -rf $RPM_BUILD_ROOT%{mingw64_datadir}/gtk-doc/*
+rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt_iohelper.exe
rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
+%endif
+%if %{with_mingw32} || %{with_mingw64}
%mingw_debug_install_post
%mingw_find_lang %{name}
@@ -2388,7 +2409,7 @@ exit 0
%{_datadir}/libvirt/api/libvirt-qemu-api.xml
%{_datadir}/libvirt/api/libvirt-lxc-api.xml
-%if %{with_mingw}
+%if %{with_mingw32}
%files -n mingw32-libvirt -f mingw32-libvirt.lang
%dir %{mingw32_sysconfdir}/libvirt/
%config(noreplace) %{mingw32_sysconfdir}/libvirt/libvirt.conf
@@ -2445,7 +2466,9 @@ exit 0
%{mingw32_mandir}/man1/virt-pki-query-dn.1*
%{mingw32_mandir}/man1/virt-pki-validate.1*
%{mingw32_mandir}/man7/virkey*.7*
+%endif
+%if %{with_mingw64}
%files -n mingw64-libvirt -f mingw64-libvirt.lang
%dir %{mingw64_sysconfdir}/libvirt/
%config(noreplace) %{mingw64_sysconfdir}/libvirt/libvirt.conf
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 07/11] rpm: Split call to mingw_debug_package
This is functionally equivalent and will make future patches
nicer.
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index dc67ffabcc..6a999860f5 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1080,7 +1080,8 @@ BuildArch: noarch
%description -n mingw64-libvirt
MinGW Windows libvirt virtualization library.
-%{?mingw_debug_package}
+%{?mingw32_debug_package}
+%{?mingw64_debug_package}
%endif
%prep
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 06/11] rpm: Shuffle BuildRequires around
Move all dependencies that are needed both for native builds and
for MinGW ones near the top of the list. This will make future
patches nicer.
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 0b33a9a5e4..dc67ffabcc 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -281,7 +281,6 @@ Requires: libvirt-libs = %{version}-%{release}
# All build-time requirements. Run-time requirements are
# listed against each sub-RPM
BuildRequires: python3-docutils
-BuildRequires: gcc
BuildRequires: meson >= 0.56.0
BuildRequires: ninja-build
BuildRequires: git
@@ -289,16 +288,21 @@ BuildRequires: perl-interpreter
BuildRequires: python3
# For xmllint
BuildRequires: libxml2
+# For xsltproc
+BuildRequires: libxslt
+BuildRequires: gettext
+BuildRequires: systemd-rpm-macros
+BuildRequires: rpcgen
+# Fedora build root suckage
+BuildRequires: gawk
+BuildRequires: gcc
%if %{with_libxl}
BuildRequires: xen-devel
%endif
BuildRequires: glib2-devel >= 2.56
BuildRequires: libxml2-devel
-# For xsltproc
-BuildRequires: libxslt
BuildRequires: readline-devel
BuildRequires: bash-completion >= 2.0
-BuildRequires: gettext
BuildRequires: libtasn1-devel
BuildRequires: gnutls-devel
BuildRequires: libattr-devel
@@ -307,7 +311,6 @@ BuildRequires: libblkid-devel >= 2.17
# for augparse, optionally used in testing
BuildRequires: augeas
BuildRequires: systemd-devel >= 185
-BuildRequires: systemd-rpm-macros
BuildRequires: libpciaccess-devel >= 0.10.9
BuildRequires: yajl-devel
%if %{with_sanlock}
@@ -384,8 +387,6 @@ BuildRequires: systemtap-sdt-devel
BuildRequires: util-linux
# For showmount in FS driver (netfs discovery)
BuildRequires: nfs-utils
-# Fedora build root suckage
-BuildRequires: gawk
# For storage wiping with different algorithms
BuildRequires: scrub
%if %{with_numad}
@@ -397,7 +398,6 @@ BuildRequires: wireshark-devel
%if %{with_libssh}
BuildRequires: libssh-devel >= 0.8.1
%endif
-BuildRequires: rpcgen
BuildRequires: libtirpc-devel
# Needed for the firewalld_reload macro
%if %{with_firewalld_zone}
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 05/11] rpm: Add libxml2 BuildRequires for xmllint
It's already been dragged in by the -devel package, but since
we use the command line tool directly as part of our build
process it's more correct to explicitly depend on the runtime
package.
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8248e2ca80..0b33a9a5e4 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -287,6 +287,8 @@ BuildRequires: ninja-build
BuildRequires: git
BuildRequires: perl-interpreter
BuildRequires: python3
+# For xmllint
+BuildRequires: libxml2
%if %{with_libxl}
BuildRequires: xen-devel
%endif
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 04/11] rpm: Explain a couple of BuildRequires
Signed-off-by: Andrea Bolognani --- libvirt.spec.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index d16d622153..8248e2ca80 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -292,6 +292,7 @@ BuildRequires: xen-devel %endif BuildRequires: glib2-devel >= 2.56 BuildRequires: libxml2-devel +# For xsltproc BuildRequires: libxslt BuildRequires: readline-devel BuildRequires: bash-completion >= 2.0 @@ -315,6 +316,7 @@ BuildRequires: libnl3-devel BuildRequires: libselinux-devel BuildRequires: iptables BuildRequires: ebtables +# For modprobe BuildRequires: kmod BuildRequires: cyrus-sasl-devel BuildRequires: polkit >= 0.112 -- 2.41.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 03/11] rpm: Rename module-init-tools -> kmod
The old package name is only kept around for compatibility reasons. Signed-off-by: Andrea Bolognani --- libvirt.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index f21f76b256..d16d622153 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -315,7 +315,7 @@ BuildRequires: libnl3-devel BuildRequires: libselinux-devel BuildRequires: iptables BuildRequires: ebtables -BuildRequires: module-init-tools +BuildRequires: kmod BuildRequires: cyrus-sasl-devel BuildRequires: polkit >= 0.112 # For mount/umount in FS driver -- 2.41.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 02/11] rpm: Explicitly enable NLS support
We want it both for native builds and MinGW ones.
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 342a88024a..f21f76b256 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1283,6 +1283,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dcapng=enabled \
%{?arg_fuse} \
%{?arg_netcf} \
+ -Dnls=enabled \
-Dselinux=enabled \
%{?arg_selinux_mount} \
-Dapparmor=disabled \
@@ -1363,7 +1364,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dlibssh=disabled \
-Dlogin_shell=disabled \
-Dnetcf=disabled \
- -Dnls=disabled \
+ -Dnls=enabled \
-Dnss=disabled \
-Dnumactl=disabled \
-Dnumad=disabled \
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 00/11] ci: Build RPMs on MinGW
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1059447352 The openSUSE Leap 15 job failed, but that's caused by an unrelated packaging issue: $ pkg-config --cflags libtirpc -I/usr/include/tirpc $ rpm -ql libtirpc-devel | grep usr/include /usr/include/netconfig.h /usr/include/rpc /usr/include/rpc/auth.h /usr/include/rpc/auth_des.h ... /usr/include/rpc/types.h /usr/include/rpc/xdr.h /usr/include/rpcsvc /usr/include/rpcsvc/crypt.h /usr/include/rpcsvc/crypt.x The CI updates towards the end depend on the following unmerged libvirt-ci changes: https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/440 Andrea Bolognani (11): rpm: Disable expensive tests for MinGW builds rpm: Explicitly enable NLS support rpm: Rename module-init-tools -> kmod rpm: Explain a couple of BuildRequires rpm: Add libxml2 BuildRequires for xmllint rpm: Shuffle BuildRequires around rpm: Split call to mingw_debug_package rpm: Introduce with_mingw32/with_mingw64 rpm: Introduce with_native ci: Refresh generated files ci: Build RPMs on MinGW .gitlab-ci.yml| 11 +- ci/buildenv/fedora-38-cross-mingw32.sh| 1 + ci/buildenv/fedora-38-cross-mingw64.sh| 1 + ci/buildenv/fedora-rawhide-cross-mingw32.sh | 1 + ci/buildenv/fedora-rawhide-cross-mingw64.sh | 1 + .../fedora-38-cross-mingw32.Dockerfile| 1 + .../fedora-38-cross-mingw64.Dockerfile| 1 + .../fedora-rawhide-cross-mingw32.Dockerfile | 1 + .../fedora-rawhide-cross-mingw64.Dockerfile | 1 + ci/jobs.sh| 19 +- libvirt.spec.in | 470 ++ 11 files changed, 296 insertions(+), 212 deletions(-) -- 2.41.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 01/11] rpm: Disable expensive tests for MinGW builds
Tests are disabled so this combination never made any sense,
but with recent changes it has turned into a build failure.
Fixes: 8ce0decc372051d616018f57ae268e2f03082eec
Reported-by: Cole Robinson
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 262c59eb5b..342a88024a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1349,7 +1349,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Ddriver_vmware=disabled \
-Ddriver_vz=disabled \
-Ddtrace=disabled \
- -Dexpensive_tests=enabled \
+ -Dexpensive_tests=disabled \
-Dfirewalld=disabled \
-Dfirewalld_zone=disabled \
-Dfuse=disabled \
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] virDomainDeviceInfoCheckABIStability: Implement proper check for CCW addresses
CCW addresses need to be also checked for ABI stability.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 80f467ae7a..5ce1793c2d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19719,10 +19719,21 @@
virDomainDeviceInfoCheckABIStability(virDomainDeviceInfo *src,
}
break;
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
+if (src->addr.ccw.cssid != dst->addr.ccw.cssid ||
+src->addr.ccw.ssid != dst->addr.ccw.ssid ||
+src->addr.ccw.devno != dst->addr.ccw.devno) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target device CCW address %1$x.%2$x.%3$04x does
not match source %4$x.%5$x.%6$04x"),
+ dst->addr.ccw.cssid, dst->addr.ccw.ssid,
dst->addr.ccw.devno,
+ src->addr.ccw.cssid, src->addr.ccw.ssid,
src->addr.ccw.devno);
+return false;
+}
+break;
+
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
-case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v8 0/4] pci hotplug tracking
On 02.11.23 16:32, Michael S. Tsirkin wrote:
On Thu, Nov 02, 2023 at 04:28:43PM +0300, Vladimir Sementsov-Ogievskiy wrote:
On 02.11.23 15:12, Michael S. Tsirkin wrote:
On Thu, Nov 02, 2023 at 03:00:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
On 02.11.23 14:31, Michael S. Tsirkin wrote:
On Thu, Oct 05, 2023 at 12:29:22PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Hi all!
Main thing this series does is DEVICE_ON event - a counter-part to
DEVICE_DELETED. A guest-driven event that device is powered-on.
Details are in patch 2. The new event is paried with corresponding
command query-hotplug.
Several things questionable here:
1. depending on guest activity you can get as many
DEVICE_ON events as you like
No, I've made it so it may be sent only once per device
Maybe document that?
Right, my fault
2. it's just for shpc and native pcie - things are
confusing enough for management, we should make sure
it can work for all devices
Agree, I'm thinking about it
3. what about non hotpluggable devices? do we want the event for them?
I think, yes, especially if we make async=true|false flag for device_add, so
that successful device_add must be always followed by DEVICE_ON - like
device_del is followed by DEVICE_DELETED.
Maybe, to generalize, it should be called not DEVICE_ON (which mostly relate to
hotplug controller statuses) but DEVICE_ADDED - a full counterpart for
DEVICE_DELETED.
I feel this needs actual motivation so we can judge what's the
right way to do it.
My first motivation for this series was the fact that successful device_add doesn't
guarantee that hard disk successfully hotplugged to the guest. It relates to some
problems with shpc/pcie hotplug we had in the past, and they are mostly fixed. But still,
for management tool it's good to understand that all actions related to hotplug
controller are done and we have "green light".
what does "successfully" mean though? E.g. a bunch of guests will not
properly show you the device if the disk is not formatted properly.
Yes, I understand, that we may say only about "some degree of success".
But here is some physical sense still: DEVICE_ON indicates, that it's now safe
to call device_del. And calling device_del before DEVICE_ON is a kind of
unexpected behavior.
Is that really true? I really don't think we should introduce new types
of undefined behavior.
Good question. At least, it was true before some recent fixes to SHPC..
And even if it is so now, we could instead fix it by some "internal DEVICE_ON",
and postpone device deletion until we have it...
OK, I need a round to rethink this all. Thanks!
Recently new motivation come, as I described in my "ping" letter
<6bd19a07-5224-464d-b54d-1d738f5ba...@yandex-team.ru>, that we have a performance
degradation because of 7bed89958bfbf40df, which introduces drain_call_rcu() in device_add, to
make it more synchronous. So, my suggestion is make it instead more asynchronous (probably with
special flag) and rely on DEVICE_ON event.
This one?
commit 7bed89958bfbf40df9ca681cefbdca63abdde39d
Author: Maxim Levitsky
Date: Tue Oct 6 14:38:58 2020 +0200
device_core: use drain_call_rcu in in qmp_device_add
Soon, a device removal might only happen on RCU callback execution.
This is okay for device-del which provides a DEVICE_DELETED event,
but not for the failure case of device-add. To avoid changing
monitor semantics, just drain all pending RCU callbacks on error.
Signed-off-by: Maxim Levitsky
Suggested-by: Stefan Hajnoczi
Reviewed-by: Stefan Hajnoczi
Message-Id: <20200913160259.32145-4-mlevi...@redhat.com>
[Don't use it in qmp_device_del. - Paolo]
Signed-off-by: Paolo Bonzini
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index e9b7228480..bcfb90a08f 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -803,6 +803,18 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
Error **errp)
return;
}
dev = qdev_device_add(opts, errp);
+
+/*
+ * Drain all pending RCU callbacks. This is done because
+ * some bus related operations can delay a device removal
+ * (in this case this can happen if device is added and then
+ * removed due to a configuration error)
+ * to a RCU callback, but user might expect that this interface
+ * will finish its job completely once qmp command returns result
+ * to the user
+ */
+drain_call_rcu();
+
if (!dev) {
qemu_opts_del(opts);
return;
So maybe just move drain_call_rcu under if (!dev) then and be done with
it?
Hmm, I read the commit message thinking that it saying about device removal by
mistake and actually want to say both about device_add and device_del.. But I
was wrong?
Hmm, it directly say "just drain all pending RCU callbacks on error", but does
that on success path as well.
Yes, moving drain_call_rcu
Re: [PATCH v8 0/4] pci hotplug tracking
On Thu, Nov 02, 2023 at 04:28:43PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 02.11.23 15:12, Michael S. Tsirkin wrote:
> > On Thu, Nov 02, 2023 at 03:00:01PM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> > > On 02.11.23 14:31, Michael S. Tsirkin wrote:
> > > > On Thu, Oct 05, 2023 at 12:29:22PM +0300, Vladimir Sementsov-Ogievskiy
> > > > wrote:
> > > > > Hi all!
> > > > >
> > > > > Main thing this series does is DEVICE_ON event - a counter-part to
> > > > > DEVICE_DELETED. A guest-driven event that device is powered-on.
> > > > > Details are in patch 2. The new event is paried with corresponding
> > > > > command query-hotplug.
> > > >
> > > > Several things questionable here:
> > > > 1. depending on guest activity you can get as many
> > > > DEVICE_ON events as you like
> > >
> > > No, I've made it so it may be sent only once per device
> >
> > Maybe document that?
>
> Right, my fault
>
> >
> > > > 2. it's just for shpc and native pcie - things are
> > > > confusing enough for management, we should make sure
> > > > it can work for all devices
> > >
> > > Agree, I'm thinking about it
> > >
> > > > 3. what about non hotpluggable devices? do we want the event for them?
> > > >
> > >
> > > I think, yes, especially if we make async=true|false flag for device_add,
> > > so that successful device_add must be always followed by DEVICE_ON - like
> > > device_del is followed by DEVICE_DELETED.
> > >
> > > Maybe, to generalize, it should be called not DEVICE_ON (which mostly
> > > relate to hotplug controller statuses) but DEVICE_ADDED - a full
> > > counterpart for DEVICE_DELETED.
> > >
> > > >
> > > > I feel this needs actual motivation so we can judge what's the
> > > > right way to do it.
> > >
> > > My first motivation for this series was the fact that successful
> > > device_add doesn't guarantee that hard disk successfully hotplugged to
> > > the guest. It relates to some problems with shpc/pcie hotplug we had in
> > > the past, and they are mostly fixed. But still, for management tool it's
> > > good to understand that all actions related to hotplug controller are
> > > done and we have "green light".
> >
> > what does "successfully" mean though? E.g. a bunch of guests will not
> > properly show you the device if the disk is not formatted properly.
>
> Yes, I understand, that we may say only about "some degree of success".
>
> But here is some physical sense still: DEVICE_ON indicates, that it's now
> safe to call device_del. And calling device_del before DEVICE_ON is a kind of
> unexpected behavior.
>
Is that really true? I really don't think we should introduce new types
of undefined behavior.
> >
> > >
> > > Recently new motivation come, as I described in my "ping" letter
> > > <6bd19a07-5224-464d-b54d-1d738f5ba...@yandex-team.ru>, that we have a
> > > performance degradation because of 7bed89958bfbf40df, which introduces
> > > drain_call_rcu() in device_add, to make it more synchronous. So, my
> > > suggestion is make it instead more asynchronous (probably with special
> > > flag) and rely on DEVICE_ON event.
> >
> > This one?
> >
> > commit 7bed89958bfbf40df9ca681cefbdca63abdde39d
> > Author: Maxim Levitsky
> > Date: Tue Oct 6 14:38:58 2020 +0200
> >
> > device_core: use drain_call_rcu in in qmp_device_add
> > Soon, a device removal might only happen on RCU callback execution.
> > This is okay for device-del which provides a DEVICE_DELETED event,
> > but not for the failure case of device-add. To avoid changing
> > monitor semantics, just drain all pending RCU callbacks on error.
> > Signed-off-by: Maxim Levitsky
> > Suggested-by: Stefan Hajnoczi
> > Reviewed-by: Stefan Hajnoczi
> > Message-Id: <20200913160259.32145-4-mlevi...@redhat.com>
> > [Don't use it in qmp_device_del. - Paolo]
> > Signed-off-by: Paolo Bonzini
> >
> > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> > index e9b7228480..bcfb90a08f 100644
> > --- a/softmmu/qdev-monitor.c
> > +++ b/softmmu/qdev-monitor.c
> > @@ -803,6 +803,18 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
> > Error **errp)
> > return;
> > }
> > dev = qdev_device_add(opts, errp);
> > +
> > +/*
> > + * Drain all pending RCU callbacks. This is done because
> > + * some bus related operations can delay a device removal
> > + * (in this case this can happen if device is added and then
> > + * removed due to a configuration error)
> > + * to a RCU callback, but user might expect that this interface
> > + * will finish its job completely once qmp command returns result
> > + * to the user
> > + */
> > +drain_call_rcu();
> > +
> > if (!dev) {
> > qemu_opts_del(opts);
> > return;
> >
> >
> >
> > So maybe just move drain_call_rcu under if (!dev) then and be done with
> > it?
> >
>
> Hmm, I read the commit message thinking that
Re: [PATCH v8 0/4] pci hotplug tracking
On 02.11.23 15:12, Michael S. Tsirkin wrote:
On Thu, Nov 02, 2023 at 03:00:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
On 02.11.23 14:31, Michael S. Tsirkin wrote:
On Thu, Oct 05, 2023 at 12:29:22PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Hi all!
Main thing this series does is DEVICE_ON event - a counter-part to
DEVICE_DELETED. A guest-driven event that device is powered-on.
Details are in patch 2. The new event is paried with corresponding
command query-hotplug.
Several things questionable here:
1. depending on guest activity you can get as many
DEVICE_ON events as you like
No, I've made it so it may be sent only once per device
Maybe document that?
Right, my fault
2. it's just for shpc and native pcie - things are
confusing enough for management, we should make sure
it can work for all devices
Agree, I'm thinking about it
3. what about non hotpluggable devices? do we want the event for them?
I think, yes, especially if we make async=true|false flag for device_add, so
that successful device_add must be always followed by DEVICE_ON - like
device_del is followed by DEVICE_DELETED.
Maybe, to generalize, it should be called not DEVICE_ON (which mostly relate to
hotplug controller statuses) but DEVICE_ADDED - a full counterpart for
DEVICE_DELETED.
I feel this needs actual motivation so we can judge what's the
right way to do it.
My first motivation for this series was the fact that successful device_add doesn't
guarantee that hard disk successfully hotplugged to the guest. It relates to some
problems with shpc/pcie hotplug we had in the past, and they are mostly fixed. But still,
for management tool it's good to understand that all actions related to hotplug
controller are done and we have "green light".
what does "successfully" mean though? E.g. a bunch of guests will not
properly show you the device if the disk is not formatted properly.
Yes, I understand, that we may say only about "some degree of success".
But here is some physical sense still: DEVICE_ON indicates, that it's now safe
to call device_del. And calling device_del before DEVICE_ON is a kind of
unexpected behavior.
Recently new motivation come, as I described in my "ping" letter
<6bd19a07-5224-464d-b54d-1d738f5ba...@yandex-team.ru>, that we have a performance
degradation because of 7bed89958bfbf40df, which introduces drain_call_rcu() in device_add, to
make it more synchronous. So, my suggestion is make it instead more asynchronous (probably with
special flag) and rely on DEVICE_ON event.
This one?
commit 7bed89958bfbf40df9ca681cefbdca63abdde39d
Author: Maxim Levitsky
Date: Tue Oct 6 14:38:58 2020 +0200
device_core: use drain_call_rcu in in qmp_device_add
Soon, a device removal might only happen on RCU callback execution.
This is okay for device-del which provides a DEVICE_DELETED event,
but not for the failure case of device-add. To avoid changing
monitor semantics, just drain all pending RCU callbacks on error.
Signed-off-by: Maxim Levitsky
Suggested-by: Stefan Hajnoczi
Reviewed-by: Stefan Hajnoczi
Message-Id: <20200913160259.32145-4-mlevi...@redhat.com>
[Don't use it in qmp_device_del. - Paolo]
Signed-off-by: Paolo Bonzini
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index e9b7228480..bcfb90a08f 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -803,6 +803,18 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
Error **errp)
return;
}
dev = qdev_device_add(opts, errp);
+
+/*
+ * Drain all pending RCU callbacks. This is done because
+ * some bus related operations can delay a device removal
+ * (in this case this can happen if device is added and then
+ * removed due to a configuration error)
+ * to a RCU callback, but user might expect that this interface
+ * will finish its job completely once qmp command returns result
+ * to the user
+ */
+drain_call_rcu();
+
if (!dev) {
qemu_opts_del(opts);
return;
So maybe just move drain_call_rcu under if (!dev) then and be done with
it?
Hmm, I read the commit message thinking that it saying about device removal by
mistake and actually want to say both about device_add and device_del.. But I
was wrong?
Hmm, it directly say "just drain all pending RCU callbacks on error", but does
that on success path as well.
Yes, moving drain_call_rcu makes sense for me, and will close the second
"motivation". I can make a patch.
--
Best regards,
Vladimir
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v8 0/4] pci hotplug tracking
On Thu, Nov 02, 2023 at 03:00:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 02.11.23 14:31, Michael S. Tsirkin wrote:
> > On Thu, Oct 05, 2023 at 12:29:22PM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> > > Hi all!
> > >
> > > Main thing this series does is DEVICE_ON event - a counter-part to
> > > DEVICE_DELETED. A guest-driven event that device is powered-on.
> > > Details are in patch 2. The new event is paried with corresponding
> > > command query-hotplug.
> >
> > Several things questionable here:
> > 1. depending on guest activity you can get as many
> > DEVICE_ON events as you like
>
> No, I've made it so it may be sent only once per device
Maybe document that?
> > 2. it's just for shpc and native pcie - things are
> > confusing enough for management, we should make sure
> > it can work for all devices
>
> Agree, I'm thinking about it
>
> > 3. what about non hotpluggable devices? do we want the event for them?
> >
>
> I think, yes, especially if we make async=true|false flag for device_add, so
> that successful device_add must be always followed by DEVICE_ON - like
> device_del is followed by DEVICE_DELETED.
>
> Maybe, to generalize, it should be called not DEVICE_ON (which mostly relate
> to hotplug controller statuses) but DEVICE_ADDED - a full counterpart for
> DEVICE_DELETED.
>
> >
> > I feel this needs actual motivation so we can judge what's the
> > right way to do it.
>
> My first motivation for this series was the fact that successful device_add
> doesn't guarantee that hard disk successfully hotplugged to the guest. It
> relates to some problems with shpc/pcie hotplug we had in the past, and they
> are mostly fixed. But still, for management tool it's good to understand that
> all actions related to hotplug controller are done and we have "green light".
what does "successfully" mean though? E.g. a bunch of guests will not
properly show you the device if the disk is not formatted properly.
>
> Recently new motivation come, as I described in my "ping" letter
> <6bd19a07-5224-464d-b54d-1d738f5ba...@yandex-team.ru>, that we have a
> performance degradation because of 7bed89958bfbf40df, which introduces
> drain_call_rcu() in device_add, to make it more synchronous. So, my
> suggestion is make it instead more asynchronous (probably with special flag)
> and rely on DEVICE_ON event.
This one?
commit 7bed89958bfbf40df9ca681cefbdca63abdde39d
Author: Maxim Levitsky
Date: Tue Oct 6 14:38:58 2020 +0200
device_core: use drain_call_rcu in in qmp_device_add
Soon, a device removal might only happen on RCU callback execution.
This is okay for device-del which provides a DEVICE_DELETED event,
but not for the failure case of device-add. To avoid changing
monitor semantics, just drain all pending RCU callbacks on error.
Signed-off-by: Maxim Levitsky
Suggested-by: Stefan Hajnoczi
Reviewed-by: Stefan Hajnoczi
Message-Id: <20200913160259.32145-4-mlevi...@redhat.com>
[Don't use it in qmp_device_del. - Paolo]
Signed-off-by: Paolo Bonzini
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index e9b7228480..bcfb90a08f 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -803,6 +803,18 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
Error **errp)
return;
}
dev = qdev_device_add(opts, errp);
+
+/*
+ * Drain all pending RCU callbacks. This is done because
+ * some bus related operations can delay a device removal
+ * (in this case this can happen if device is added and then
+ * removed due to a configuration error)
+ * to a RCU callback, but user might expect that this interface
+ * will finish its job completely once qmp command returns result
+ * to the user
+ */
+drain_call_rcu();
+
if (!dev) {
qemu_opts_del(opts);
return;
So maybe just move drain_call_rcu under if (!dev) then and be done with
it?
--
MST
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v8 0/4] pci hotplug tracking
On 02.11.23 14:31, Michael S. Tsirkin wrote: On Thu, Oct 05, 2023 at 12:29:22PM +0300, Vladimir Sementsov-Ogievskiy wrote: Hi all! Main thing this series does is DEVICE_ON event - a counter-part to DEVICE_DELETED. A guest-driven event that device is powered-on. Details are in patch 2. The new event is paried with corresponding command query-hotplug. Several things questionable here: 1. depending on guest activity you can get as many DEVICE_ON events as you like No, I've made it so it may be sent only once per device 2. it's just for shpc and native pcie - things are confusing enough for management, we should make sure it can work for all devices Agree, I'm thinking about it 3. what about non hotpluggable devices? do we want the event for them? I think, yes, especially if we make async=true|false flag for device_add, so that successful device_add must be always followed by DEVICE_ON - like device_del is followed by DEVICE_DELETED. Maybe, to generalize, it should be called not DEVICE_ON (which mostly relate to hotplug controller statuses) but DEVICE_ADDED - a full counterpart for DEVICE_DELETED. I feel this needs actual motivation so we can judge what's the right way to do it. My first motivation for this series was the fact that successful device_add doesn't guarantee that hard disk successfully hotplugged to the guest. It relates to some problems with shpc/pcie hotplug we had in the past, and they are mostly fixed. But still, for management tool it's good to understand that all actions related to hotplug controller are done and we have "green light". Recently new motivation come, as I described in my "ping" letter <6bd19a07-5224-464d-b54d-1d738f5ba...@yandex-team.ru>, that we have a performance degradation because of 7bed89958bfbf40df, which introduces drain_call_rcu() in device_add, to make it more synchronous. So, my suggestion is make it instead more asynchronous (probably with special flag) and rely on DEVICE_ON event. v8: - improve naming, wording and style - make new QMP interface experimental Vladimir Sementsov-Ogievskiy (4): qapi/qdev.json: unite DEVICE_* event data into single structure qapi: add DEVICE_ON and query-hotplug infrastructure shpc: implement DEVICE_ON event and query-hotplug pcie: implement DEVICE_ON event and query-hotplug hw/core/hotplug.c | 12 +++ hw/pci-bridge/pci_bridge_dev.c | 14 +++ hw/pci-bridge/pcie_pci_bridge.c | 1 + hw/pci/pcie.c | 83 +++ hw/pci/pcie_port.c | 1 + hw/pci/shpc.c | 86 +++ include/hw/hotplug.h| 11 ++ include/hw/pci/pci_bridge.h | 2 + include/hw/pci/pcie.h | 2 + include/hw/pci/shpc.h | 2 + include/hw/qdev-core.h | 7 ++ include/monitor/qdev.h | 6 ++ qapi/qdev.json | 178 +--- softmmu/qdev-monitor.c | 58 +++ 14 files changed, 451 insertions(+), 12 deletions(-) -- 2.34.1 -- Best regards, Vladimir ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH v8 0/4] pci hotplug tracking
[cc Peter, Nikolay and libvirt list] On 02.11.23 11:06, Vladimir Sementsov-Ogievskiy wrote: Ping. And some addition. We have the case, when the commit commit 7bed89958bfbf40df9ca681cefbdca63abdde39d Author: Maxim Levitsky Date: Tue Oct 6 14:38:58 2020 +0200 device_core: use drain_call_rcu in in qmp_device_add Soon, a device removal might only happen on RCU callback execution. This is okay for device-del which provides a DEVICE_DELETED event, but not for the failure case of device-add. To avoid changing monitor semantics, just drain all pending RCU callbacks on error. sensibly slows down vm initialization (several calls to device_add of pc-dimm). And looking at commit message, I see that what I do in the series is exactly a suggestion to change monitor semantics. What do you think? Maybe we need a boolean "async" parameter for device_add, which will turn off drain_call_rcu() call and rely on user to handle DEVICE_ON? On 05.10.23 12:29, Vladimir Sementsov-Ogievskiy wrote: Hi all! Main thing this series does is DEVICE_ON event - a counter-part to DEVICE_DELETED. A guest-driven event that device is powered-on. Details are in patch 2. The new event is paried with corresponding command query-hotplug. v8: - improve naming, wording and style - make new QMP interface experimental Vladimir Sementsov-Ogievskiy (4): qapi/qdev.json: unite DEVICE_* event data into single structure qapi: add DEVICE_ON and query-hotplug infrastructure shpc: implement DEVICE_ON event and query-hotplug pcie: implement DEVICE_ON event and query-hotplug hw/core/hotplug.c | 12 +++ hw/pci-bridge/pci_bridge_dev.c | 14 +++ hw/pci-bridge/pcie_pci_bridge.c | 1 + hw/pci/pcie.c | 83 +++ hw/pci/pcie_port.c | 1 + hw/pci/shpc.c | 86 +++ include/hw/hotplug.h | 11 ++ include/hw/pci/pci_bridge.h | 2 + include/hw/pci/pcie.h | 2 + include/hw/pci/shpc.h | 2 + include/hw/qdev-core.h | 7 ++ include/monitor/qdev.h | 6 ++ qapi/qdev.json | 178 +--- softmmu/qdev-monitor.c | 58 +++ 14 files changed, 451 insertions(+), 12 deletions(-) -- Best regards, Vladimir ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
