Re: [libvirt PATCH v2 0/8] meson: Improve handling of tests

2023-10-26 Thread Martin Kletzander

On Wed, Oct 25, 2023 at 07:06:53PM +0200, Andrea Bolognani wrote:

Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1049326523

Changes from [v1]

 * fix test suite on macOS and ensure it is run as part of the
   pipeline (with the previous version it would just always fail);

 * disable a couple of tests (check-html, check-html-references)
   that I had missed the first time around.

[v1] https://listman.redhat.com/archives/libvir-list/2023-October/242491.html

Andrea Bolognani (8):
 tests: Fix some test cases on macOS
 ci: Disable optimizations on macOS
 meson: Do less when not building from git
 meson: Move all handling of test options together
 meson: Handle -Dtests=enabled with Clang
 meson: Make -Dexpensive_tests depend on -Dtests
 meson: Disable all tests when tests are disabled
 meson: Rename build_tests -> tests_enabled


Reviewed-by: Martin Kletzander 



build-aux/meson.build |  99 +
ci/cirrus/build.yml   |   2 +-
docs/html/meson.build |  20 +-
docs/meson.build  |  24 ++-
meson.build   |  75 ---
meson_options.txt |   2 +-
src/access/meson.build|  16 +-
src/meson.build   | 204 +-
.../disk-source-fd.x86_64-latest.args |  10 +-
.../disk-vhostvdpa.x86_64-latest.args |   2 +-
tests/qemuxml2argvtest.c  |   8 +-
11 files changed, 242 insertions(+), 220 deletions(-)

--
2.41.0



signature.asc
Description: PGP signature


[libvirt PATCH v2 5/8] meson: Handle -Dtests=enabled with Clang

2023-10-25 Thread Andrea Bolognani
There are some cases in which we automatically disable tests when
using Clang as the compiler. If the user has explicitly asked for
tests to be enabled, however, we should error out instead of
silently disabling things.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Michal Privoznik 
---
 meson.build | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index a372f99c21..ea8ee84ba0 100644
--- a/meson.build
+++ b/meson.build
@@ -2035,7 +2035,11 @@ if build_tests[0] and \
# If CLang doesn't support -fsemantic-interposition then our
# mocking doesn't work. The best we can do is to not run the
# test suite.
-   build_tests = [ false, '!!! Forcibly disabling tests because CLang lacks 
-fsemantic-interposition. Update CLang or disable optimization !!!' ]
+   msg = 'Forcibly disabling tests because CLang lacks 
-fsemantic-interposition. Update CLang or disable optimization'
+   if get_option('tests').enabled()
+ error(msg)
+   endif
+   build_tests = [ false, '!!! @0@ !!!'.format(msg) ]
 endif
 
 if get_option('expensive_tests').auto()
-- 
2.41.0



[libvirt PATCH v2 7/8] meson: Disable all tests when tests are disabled

2023-10-25 Thread Andrea Bolognani
Currently, passing -Dtests=disabled only disables a subset of
tests: those that are written in C and thus require compilation.
Other tests, such as the syntax-check ones and those that are
implemented as scripts, are always enabled.

There's a potentially dangerous consequence of this behavior:
when tests are disabled, 'meson test' will succeed as if they
had been enabled. No indication of this will be shown, so the
user will likely make the reasonable assumption that everything
is fine when in fact the significantly reduced coverage might
be hiding failures.

To solve this issues, disable *all* tests when asked to do so,
and inject an intentionally failing test to ensure that 'meson
test' doesn't succeed.

Best viewed with 'git show -w'.

Signed-off-by: Andrea Bolognani 
---
 build-aux/meson.build  |   2 +-
 docs/html/meson.build  |  20 ++--
 docs/meson.build   |  24 ++---
 meson.build|   7 ++
 src/access/meson.build |  16 ++--
 src/meson.build| 204 +
 6 files changed, 144 insertions(+), 129 deletions(-)

diff --git a/build-aux/meson.build b/build-aux/meson.build
index b5d88a4c44..84405c5ec8 100644
--- a/build-aux/meson.build
+++ b/build-aux/meson.build
@@ -1,6 +1,6 @@
 # Skip syntax-check if not building from git because we get the list of files
 # to check using git commands and it fails if we are not in git repository.
-if git
+if git and build_tests[0]
   flake8_path = ''
   if flake8_prog.found()
 flake8_path = flake8_prog.full_path()
diff --git a/docs/html/meson.build b/docs/html/meson.build
index c0a666f4e1..b4e81f8501 100644
--- a/docs/html/meson.build
+++ b/docs/html/meson.build
@@ -119,12 +119,14 @@ html_xslt_gen = []
 
 # --- end of XSLT processing ---
 
-test(
-  'check-html',
-  xmllint_prog,
-  args: [
-'--nonet', '--noout', docs_html_paths,
-  ],
-  depends: docs_html_dep,
-  suite: 'script'
-)
+if build_tests[0]
+  test(
+'check-html',
+xmllint_prog,
+args: [
+  '--nonet', '--noout', docs_html_paths,
+],
+depends: docs_html_dep,
+suite: 'script'
+  )
+endif
diff --git a/docs/meson.build b/docs/meson.build
index b20ef1c926..52763a8597 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -351,14 +351,16 @@ run_target(
   depends: install_web_deps,
 )
 
-test(
-  'check-html-references',
-  python3_prog,
-  args: [
-check_html_references_prog.full_path(),
-'--webroot',
-meson.project_build_root() / 'docs'
-  ],
-  env: runutf8,
-  suite: 'script'
-)
+if build_tests[0]
+  test(
+'check-html-references',
+python3_prog,
+args: [
+  check_html_references_prog.full_path(),
+  '--webroot',
+  meson.project_build_root() / 'docs'
+],
+env: runutf8,
+suite: 'script'
+  )
+endif
diff --git a/meson.build b/meson.build
index 33027404f6..b30150d605 100644
--- a/meson.build
+++ b/meson.build
@@ -2085,6 +2085,13 @@ subdir('tools')
 
 if build_tests[0]
   subdir('tests')
+else
+  # Ensure that 'meson test' fails when tests are disabled, as opposed to
+  # misleadingly succeeding at doing absolutely nothing
+  test(
+'tests-are-disabled',
+python3_prog, args: [ '-c', 'raise Exception("tests are disabled")' ],
+  )
 endif
 
 subdir('examples')
diff --git a/src/access/meson.build b/src/access/meson.build
index e65f17c0a2..6ca953c932 100644
--- a/src/access/meson.build
+++ b/src/access/meson.build
@@ -105,10 +105,12 @@ access_dep = declare_dependency(
 
 generated_sym_files += access_gen_sym
 
-test(
-  'check-aclperms',
-  python3_prog,
-  args: [ check_aclperms_prog.full_path(), access_perm_h, 
files('viraccessperm.c') ],
-  env: runutf8,
-  suite: 'script'
-)
+if build_tests[0]
+  test(
+'check-aclperms',
+python3_prog,
+args: [ check_aclperms_prog.full_path(), access_perm_h, 
files('viraccessperm.c') ],
+env: runutf8,
+suite: 'script'
+  )
+endif
diff --git a/src/meson.build b/src/meson.build
index 43146fe3c3..e25b3e5980 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -946,121 +946,123 @@ meson.add_install_script(
 
 # Check driver files
 
-if host_machine.system() == 'linux'
-  test(
-'check-symfile',
-python3_prog,
-args: [ check_symfile_prog.full_path(), libvirt_syms, libvirt_lib ],
-env: runutf8,
-suite: 'script'
-  )
-
-  if conf.has('WITH_REMOTE')
+if build_tests[0]
+  if host_machine.system() == 'linux'
 test(
-  'check-admin-symfile',
+  'check-symfile',
   python3_prog,
-  args: [ check_symfile_prog.full_path(), libvirt_admin_syms, 
libvirt_admin_lib ],
+  args: [ check_symfile_prog.full_path(), libvirt_syms, libvirt_lib ],
   env: runutf8,
   suite: 'script'
 )
+
+if conf.has('WITH_REMOTE')
+  test(
+'check-admin-symfile',
+python3_prog,
+args: [ check_symfile_prog.full_path(), libvirt_admin_syms, 
libvirt_admin_lib ],
+env: runutf8,
+suite: 'script'
+  )
+endif
   endif
-endif
 
-test(
-  

[libvirt PATCH v2 1/8] tests: Fix some test cases on macOS

2023-10-25 Thread Andrea Bolognani
Test cases that depend on duplicating fds are using fairly big
values as targets.

This works fine on Linux, where RLIMIT_NOFILE is 1024 by
default, but fails on macOS which uses 256 as the default.

Decrease the values so that they're valid across all platforms.

Signed-off-by: Andrea Bolognani 
---
 .../qemuxml2argvdata/disk-source-fd.x86_64-latest.args | 10 +-
 .../qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args |  2 +-
 tests/qemuxml2argvtest.c   |  8 
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qemuxml2argvdata/disk-source-fd.x86_64-latest.args 
b/tests/qemuxml2argvdata/disk-source-fd.x86_64-latest.args
index 9d8109a8f4..1341b7d032 100644
--- a/tests/qemuxml2argvdata/disk-source-fd.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-source-fd.x86_64-latest.args
@@ -27,18 +27,18 @@ 
XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -no-shutdown \
 -boot strict=on \
 -device 
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
--add-fd set=2,fd=700,opaque=libvirt-4-storage0 \
--add-fd set=2,fd=705,opaque=libvirt-4-storage1 \
+-add-fd set=2,fd=200,opaque=libvirt-4-storage0 \
+-add-fd set=2,fd=205,opaque=libvirt-4-storage1 \
 -blockdev 
'{"driver":"file","filename":"/dev/fdset/2","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}'
 \
 -blockdev 
'{"node-name":"libvirt-4-format","read-only":false,"driver":"qcow2","file":"libvirt-4-storage"}'
 \
 -device 
'{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-4-format","id":"virtio-disk4","bootindex":1}'
 \
 -blockdev 
'{"driver":"file","filename":"/var/lib/libvirt/images/rhel7.1484071876","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}'
 \
 -blockdev 
'{"node-name":"libvirt-3-format","read-only":true,"driver":"qcow2","file":"libvirt-3-storage","backing":null}'
 \
--add-fd set=1,fd=777,opaque=libvirt-2-storage0 \
--add-fd set=1,fd=778,opaque=libvirt-2-storage1 \
+-add-fd set=1,fd=247,opaque=libvirt-2-storage0 \
+-add-fd set=1,fd=248,opaque=libvirt-2-storage1 \
 -blockdev 
'{"driver":"file","filename":"/dev/fdset/1","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}'
 \
 -blockdev 
'{"node-name":"libvirt-2-format","read-only":true,"driver":"qcow2","file":"libvirt-2-storage","backing":"libvirt-3-format"}'
 \
--add-fd set=0,fd=704,opaque=libvirt-1-storage0 \
+-add-fd set=0,fd=204,opaque=libvirt-1-storage0 \
 -blockdev 
'{"driver":"file","filename":"/dev/fdset/0","node-name":"libvirt-1-storage","read-only":false,"discard":"unmap"}'
 \
 -blockdev 
'{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2","file":"libvirt-1-storage","backing":"libvirt-2-format"}'
 \
 -device 
'{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-1-format","id":"virtio-disk5"}'
 \
diff --git a/tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args 
b/tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args
index b987455ee4..27035184ad 100644
--- a/tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args
@@ -27,7 +27,7 @@ 
XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -no-shutdown \
 -boot strict=on \
 -device 
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
--add-fd set=0,fd=801,opaque=libvirt-1-storage-vdpa \
+-add-fd set=0,fd=201,opaque=libvirt-1-storage-vdpa \
 -blockdev 
'{"driver":"virtio-blk-vhost-vdpa","path":"/dev/fdset/0","node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}'
 \
 -blockdev 
'{"node-name":"libvirt-1-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"raw","file":"libvirt-1-storage"}'
 \
 -device 
'{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-1-format","id":"virtio-disk0","bootindex":1,"write-cache":"on"}'
 \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 48058cb924..4fda68a4ce 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1186,7 +1186,7 @@ mymain(void)
 DO_TEST_CAPS_LATEST("disk-vhostuser-numa");
 DO_TEST_CAPS_LATEST("disk-vhostuser");
 DO_TEST_CAPS_ARCH_LATEST_FULL("disk-vhostvdpa", "x86_64",
-  ARG_VDPA_FD, "/dev/vhost-vdpa-0", 801);
+  ARG_VDPA_FD, "/dev/vhost-vdpa-0", 201);
 DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-device-lun-type-invalid");
 DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-attaching-partition-nosupport");
 DO_TEST_CAPS_LATEST("disk-usb-device");
@@ -1226,9 +1226,9 @@ mymain(void)
 DO_TEST_CAPS_LATEST("disk-backing-chains-index");
 DO_TEST_CAPS_LATEST("disk-backing-chains-noindex");
 DO_TEST_CAPS_ARCH_LATEST_FULL("disk-source-fd", "x86_64",
-  ARG_FD_GROUP, "testgroup2", 2, 700, 705,
-  

[libvirt PATCH v2 8/8] meson: Rename build_tests -> tests_enabled

2023-10-25 Thread Andrea Bolognani
Given that this variable now controls not just whether C tests
are built, but also whether any test at all is executed, the new
name is more appropriate.

Update the description for the corresponding meson option
accordingly.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Michal Privoznik 
---
 build-aux/meson.build  |  2 +-
 docs/html/meson.build  |  2 +-
 docs/meson.build   |  2 +-
 meson.build| 14 +++---
 meson_options.txt  |  2 +-
 src/access/meson.build |  2 +-
 src/meson.build|  2 +-
 7 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/build-aux/meson.build b/build-aux/meson.build
index 84405c5ec8..f96d46c445 100644
--- a/build-aux/meson.build
+++ b/build-aux/meson.build
@@ -1,6 +1,6 @@
 # Skip syntax-check if not building from git because we get the list of files
 # to check using git commands and it fails if we are not in git repository.
-if git and build_tests[0]
+if git and tests_enabled[0]
   flake8_path = ''
   if flake8_prog.found()
 flake8_path = flake8_prog.full_path()
diff --git a/docs/html/meson.build b/docs/html/meson.build
index b4e81f8501..e2758ed177 100644
--- a/docs/html/meson.build
+++ b/docs/html/meson.build
@@ -119,7 +119,7 @@ html_xslt_gen = []
 
 # --- end of XSLT processing ---
 
-if build_tests[0]
+if tests_enabled[0]
   test(
 'check-html',
 xmllint_prog,
diff --git a/docs/meson.build b/docs/meson.build
index 52763a8597..87d728213c 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -351,7 +351,7 @@ run_target(
   depends: install_web_deps,
 )
 
-if build_tests[0]
+if tests_enabled[0]
   test(
 'check-html-references',
 python3_prog,
diff --git a/meson.build b/meson.build
index b30150d605..e51ba9231e 100644
--- a/meson.build
+++ b/meson.build
@@ -2027,8 +2027,8 @@ conf.set_quoted('TLS_PRIORITY', 
get_option('tls_priority'))
 
 # test options
 
-build_tests = [ not get_option('tests').disabled() ]
-if build_tests[0] and \
+tests_enabled = [ not get_option('tests').disabled() ]
+if tests_enabled[0] and \
cc.get_id() == 'clang' and \
not supported_cc_flags.contains('-fsemantic-interposition') \
and get_option('optimization') != '0'
@@ -2039,14 +2039,14 @@ if build_tests[0] and \
if get_option('tests').enabled()
  error(msg)
endif
-   build_tests = [ false, '!!! @0@ !!!'.format(msg) ]
+   tests_enabled = [ false, '!!! @0@ !!!'.format(msg) ]
 endif
 
 if get_option('expensive_tests').auto()
-  use_expensive_tests = not git and build_tests[0]
+  use_expensive_tests = not git and tests_enabled[0]
 else
   use_expensive_tests = get_option('expensive_tests').enabled()
-  if use_expensive_tests and not build_tests[0]
+  if use_expensive_tests and not tests_enabled[0]
 error('cannot enable expensive tests when tests are disabled')
   endif
 endif
@@ -2083,7 +2083,7 @@ subdir('src')
 
 subdir('tools')
 
-if build_tests[0]
+if tests_enabled[0]
   subdir('tests')
 else
   # Ensure that 'meson test' fails when tests are disabled, as opposed to
@@ -2307,7 +2307,7 @@ endif
 misc_summary = {
   'Warning Flags': supported_cc_flags,
   'docs': gen_docs,
-  'tests': build_tests,
+  'tests': tests_enabled,
   'DTrace': conf.has('WITH_DTRACE_PROBES'),
   'firewalld': conf.has('WITH_FIREWALLD'),
   'firewalld-zone': conf.has('WITH_FIREWALLD_ZONE'),
diff --git a/meson_options.txt b/meson_options.txt
index 7c428a9eb0..a0928102bf 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -10,7 +10,7 @@ option('git_werror', type: 'feature', value: 'auto', 
description: 'use -Werror i
 option('rpath', type: 'feature', value: 'auto', description: 'whether to 
include rpath information in installed binaries and libraries')
 option('docdir', type: 'string', value: '', description: 'documentation 
installation directory')
 option('docs', type: 'feature', value: 'auto', description: 'whether to 
generate documentation')
-option('tests', type: 'feature', value: 'auto', description: 'whether to build 
tests')
+option('tests', type: 'feature', value: 'auto', description: 'whether to build 
and run tests')
 
 
 # build dependencies options
diff --git a/src/access/meson.build b/src/access/meson.build
index 6ca953c932..fc5ba5b342 100644
--- a/src/access/meson.build
+++ b/src/access/meson.build
@@ -105,7 +105,7 @@ access_dep = declare_dependency(
 
 generated_sym_files += access_gen_sym
 
-if build_tests[0]
+if tests_enabled[0]
   test(
 'check-aclperms',
 python3_prog,
diff --git a/src/meson.build b/src/meson.build
index e25b3e5980..5fc4d03b4a 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -946,7 +946,7 @@ meson.add_install_script(
 
 # Check driver files
 
-if build_tests[0]
+if tests_enabled[0]
   if host_machine.system() == 'linux'
 test(
   'check-symfile',
-- 
2.41.0



[libvirt PATCH v2 6/8] meson: Make -Dexpensive_tests depend on -Dtests

2023-10-25 Thread Andrea Bolognani
It only makes sense to enable expensive tests when tests are
enabled. Disallow invalid configurations.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Michal Privoznik 
---
 meson.build | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index ea8ee84ba0..33027404f6 100644
--- a/meson.build
+++ b/meson.build
@@ -2043,9 +2043,12 @@ if build_tests[0] and \
 endif
 
 if get_option('expensive_tests').auto()
-  use_expensive_tests = not git
+  use_expensive_tests = not git and build_tests[0]
 else
   use_expensive_tests = get_option('expensive_tests').enabled()
+  if use_expensive_tests and not build_tests[0]
+error('cannot enable expensive tests when tests are disabled')
+  endif
 endif
 
 coverage_flags = []
-- 
2.41.0



[libvirt PATCH v2 3/8] meson: Do less when not building from git

2023-10-25 Thread Andrea Bolognani
As explained in the comment, the syntax-check machinery uses git
to figure out the list of files it should operate on, so we can
only enable it when building from git.

Despite only registering the various tests with meson in that
case, however, we unconditionally perform a bunch of preparation
that is only useful for the purpose of registering and running
the tests. If we're not going to do that, we can skip a few steps
and save a bit of time.

Best viewed with 'git show -w'.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Michal Privoznik 
---
 build-aux/meson.build | 99 +--
 1 file changed, 49 insertions(+), 50 deletions(-)

diff --git a/build-aux/meson.build b/build-aux/meson.build
index 16d085505d..b5d88a4c44 100644
--- a/build-aux/meson.build
+++ b/build-aux/meson.build
@@ -1,63 +1,62 @@
-flake8_path = ''
-if flake8_prog.found()
-  flake8_path = flake8_prog.full_path()
-endif
+# Skip syntax-check if not building from git because we get the list of files
+# to check using git commands and it fails if we are not in git repository.
+if git
+  flake8_path = ''
+  if flake8_prog.found()
+flake8_path = flake8_prog.full_path()
+  endif
 
-if host_machine.system() == 'freebsd' or host_machine.system() == 'darwin'
-  make_prog = find_program('gmake')
-  sed_prog = find_program('gsed')
-else
-  make_prog = find_program('make')
-  sed_prog = find_program('sed')
-endif
+  if host_machine.system() == 'freebsd' or host_machine.system() == 'darwin'
+make_prog = find_program('gmake')
+sed_prog = find_program('gsed')
+  else
+make_prog = find_program('make')
+sed_prog = find_program('sed')
+  endif
 
-if host_machine.system() == 'freebsd'
-  grep_prog = find_program('grep')
-  grep_cmd = run_command(grep_prog, '--version', check: true)
-  if grep_cmd.stdout().startswith('grep (BSD grep')
-grep_prog = find_program('/usr/local/bin/grep', required: false)
-if not grep_prog.found()
-  error('GNU grep not found')
+  if host_machine.system() == 'freebsd'
+grep_prog = find_program('grep')
+grep_cmd = run_command(grep_prog, '--version', check: true)
+if grep_cmd.stdout().startswith('grep (BSD grep')
+  grep_prog = find_program('/usr/local/bin/grep', required: false)
+  if not grep_prog.found()
+error('GNU grep not found')
+  endif
 endif
+  elif host_machine.system() == 'darwin'
+grep_prog = find_program('ggrep')
+  else
+grep_prog = find_program('grep')
   endif
-elif host_machine.system() == 'darwin'
-  grep_prog = find_program('ggrep')
-else
-  grep_prog = find_program('grep')
-endif
-
-awk_prog = find_program('awk')
 
-syntax_check_conf = configuration_data({
-  'top_srcdir': meson.project_source_root(),
-  'top_builddir': meson.project_build_root(),
-  'flake8_path': flake8_path,
-  'runutf8': ' '.join(runutf8),
-  'PYTHON3': python3_prog.full_path(),
-  'GREP': grep_prog.full_path(),
-  'SED': sed_prog.full_path(),
-  'AWK': awk_prog.full_path(),
-})
+  awk_prog = find_program('awk')
 
-configure_file(
-  input: 'Makefile.in',
-  output: '@BASENAME@',
-  configuration: syntax_check_conf,
-)
+  syntax_check_conf = configuration_data({
+'top_srcdir': meson.project_source_root(),
+'top_builddir': meson.project_build_root(),
+'flake8_path': flake8_path,
+'runutf8': ' '.join(runutf8),
+'PYTHON3': python3_prog.full_path(),
+'GREP': grep_prog.full_path(),
+'SED': sed_prog.full_path(),
+'AWK': awk_prog.full_path(),
+  })
 
-rc = run_command(
-  'sed', '-n',
-  's/^sc_\\([a-zA-Z0-9_-]*\\):.*/\\1/p',
-  meson.current_source_dir() / 'syntax-check.mk',
-  check: true,
-)
+  configure_file(
+input: 'Makefile.in',
+output: '@BASENAME@',
+configuration: syntax_check_conf,
+  )
 
-sc_tests = rc.stdout().strip().split()
+  rc = run_command(
+'sed', '-n',
+'s/^sc_\\([a-zA-Z0-9_-]*\\):.*/\\1/p',
+meson.current_source_dir() / 'syntax-check.mk',
+check: true,
+  )
 
+  sc_tests = rc.stdout().strip().split()
 
-# Skip syntax-check if not building from git because we get the list of files
-# to check using git commands and it fails if we are not in git repository.
-if git
   foreach target : sc_tests
 test(
   target,
-- 
2.41.0



[libvirt PATCH v2 4/8] meson: Move all handling of test options together

2023-10-25 Thread Andrea Bolognani
This will make future patches nicer.

Note that we need to handle these somewhat late because of the
dependency on information about the compiler and the flags it
supports.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Michal Privoznik 
---
 meson.build | 57 +++--
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/meson.build b/meson.build
index 47748febb8..a372f99c21 100644
--- a/meson.build
+++ b/meson.build
@@ -151,23 +151,6 @@ if packager_version != ''
 endif
 
 
-# test options
-
-if get_option('expensive_tests').auto()
-  use_expensive_tests = not git
-else
-  use_expensive_tests = get_option('expensive_tests').enabled()
-endif
-
-coverage_flags = []
-if get_option('test_coverage')
-  coverage_flags = [
-'-fprofile-arcs',
-'-ftest-coverage',
-  ]
-endif
-
-
 # Add RPATH information when building for a non-standard prefix, or
 # when explicitly requested to do so
 
@@ -2041,6 +2024,35 @@ endif
 
 conf.set_quoted('TLS_PRIORITY', get_option('tls_priority'))
 
+
+# test options
+
+build_tests = [ not get_option('tests').disabled() ]
+if build_tests[0] and \
+   cc.get_id() == 'clang' and \
+   not supported_cc_flags.contains('-fsemantic-interposition') \
+   and get_option('optimization') != '0'
+   # If CLang doesn't support -fsemantic-interposition then our
+   # mocking doesn't work. The best we can do is to not run the
+   # test suite.
+   build_tests = [ false, '!!! Forcibly disabling tests because CLang lacks 
-fsemantic-interposition. Update CLang or disable optimization !!!' ]
+endif
+
+if get_option('expensive_tests').auto()
+  use_expensive_tests = not git
+else
+  use_expensive_tests = get_option('expensive_tests').enabled()
+endif
+
+coverage_flags = []
+if get_option('test_coverage')
+  coverage_flags = [
+'-fprofile-arcs',
+'-ftest-coverage',
+  ]
+endif
+
+
 # Various definitions
 
 # Python3 < 3.7 treats the C locale as 7-bit only. We must force env vars so
@@ -2064,17 +2076,6 @@ subdir('src')
 
 subdir('tools')
 
-build_tests = [ not get_option('tests').disabled() ]
-if build_tests[0] and \
-   cc.get_id() == 'clang' and \
-   not supported_cc_flags.contains('-fsemantic-interposition') \
-   and get_option('optimization') != '0'
-   # If CLang doesn't support -fsemantic-interposition then our
-   # mocking doesn't work. The best we can do is to not run the
-   # test suite.
-   build_tests = [ false, '!!! Forcibly disabling tests because CLang lacks 
-fsemantic-interposition. Update CLang or disable optimization !!!' ]
-endif
-
 if build_tests[0]
   subdir('tests')
 endif
-- 
2.41.0



[libvirt PATCH v2 2/8] ci: Disable optimizations on macOS

2023-10-25 Thread Andrea Bolognani
Clang can be too aggressive at optimizations, which can end up
breaking our test suite. See f9f5ab57189b for details.

As a result of this, since 7944700b4037 we are automatically
disabling tests when Clang is used unless it supports the
-fsemantic-interposition compiler flag.

Since the version of Clang included in macOS doesn't support that
compiler flag, we end up always disabling the test suite on that
platform.

This is already far from ideal, considering that it was just last
year when we finally managed to get the test suite to successfully
pass on macOS, and it would be a real shame if the situation
regressed again.

With the upcoming changes, which will turn running 'meson test'
into a hard failure if tests are disabled, this behavior will
result in every single pipeline failing.

Work around the problem the only way we can: disabling
optimizations entirely for the macOS CI jobs.

Signed-off-by: Andrea Bolognani 
---
 ci/cirrus/build.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ci/cirrus/build.yml b/ci/cirrus/build.yml
index 519e5ae144..60ac90eee0 100644
--- a/ci/cirrus/build.yml
+++ b/ci/cirrus/build.yml
@@ -24,7 +24,7 @@ build_task:
 - git fetch origin "${CI_MERGE_REQUEST_REF_PATH:-$CI_COMMIT_REF_NAME}"
 - git reset --hard "$CI_COMMIT_SHA"
   build_script:
-- meson setup build
+- if test "$(uname)" = "Darwin"; then meson setup build --optimization 0; 
else meson setup build; fi
 - meson dist -C build --no-tests
 - meson compile -C build
 - meson test -C build --no-suite syntax-check --print-errorlogs || (cat 
~/Library/Logs/DiagnosticReports/*.crash && exit 1)
-- 
2.41.0



[libvirt PATCH v2 0/8] meson: Improve handling of tests

2023-10-25 Thread Andrea Bolognani
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1049326523

Changes from [v1]

  * fix test suite on macOS and ensure it is run as part of the
pipeline (with the previous version it would just always fail);

  * disable a couple of tests (check-html, check-html-references)
that I had missed the first time around.

[v1] https://listman.redhat.com/archives/libvir-list/2023-October/242491.html

Andrea Bolognani (8):
  tests: Fix some test cases on macOS
  ci: Disable optimizations on macOS
  meson: Do less when not building from git
  meson: Move all handling of test options together
  meson: Handle -Dtests=enabled with Clang
  meson: Make -Dexpensive_tests depend on -Dtests
  meson: Disable all tests when tests are disabled
  meson: Rename build_tests -> tests_enabled

 build-aux/meson.build |  99 +
 ci/cirrus/build.yml   |   2 +-
 docs/html/meson.build |  20 +-
 docs/meson.build  |  24 ++-
 meson.build   |  75 ---
 meson_options.txt |   2 +-
 src/access/meson.build|  16 +-
 src/meson.build   | 204 +-
 .../disk-source-fd.x86_64-latest.args |  10 +-
 .../disk-vhostvdpa.x86_64-latest.args |   2 +-
 tests/qemuxml2argvtest.c  |   8 +-
 11 files changed, 242 insertions(+), 220 deletions(-)

-- 
2.41.0



Re: [libvirt PATCH v2] hypervisor: Move interface mgmt methods to hypervisor

2023-10-19 Thread Praveen Paladugu
On Wed, Oct 18, 2023 at 03:36:47PM -0400, Laine Stump wrote:
> On 10/16/23 3:34 PM, Praveen K Paladugu wrote:
> >Move guest interface management methods from qemu to hypervisor. These
> >methods will be shared by networking support in ch driver.
> >
> >Signed-off-by: Praveen K Paladugu 
> >---
> >  po/POTFILES   |   1 +
> >  src/hypervisor/domain_interface.c | 280 ++
> >  src/hypervisor/domain_interface.h |  39 +
> >  src/hypervisor/meson.build|   1 +
> >  src/libvirt_private.syms  |   6 +
> >  src/qemu/qemu_command.c   |   7 +-
> >  src/qemu/qemu_hotplug.c   |   3 +-
> >  src/qemu/qemu_interface.c | 246 +-
> >  src/qemu/qemu_interface.h |   6 -
> >  src/qemu/qemu_process.c   |   3 +-
> >  10 files changed, 343 insertions(+), 249 deletions(-)
> >  create mode 100644 src/hypervisor/domain_interface.c
> >  create mode 100644 src/hypervisor/domain_interface.h
> >
> >diff --git a/po/POTFILES b/po/POTFILES
> >index 3a51aea5cb..023c041f61 100644
> >--- a/po/POTFILES
> >+++ b/po/POTFILES
> >@@ -92,6 +92,7 @@ src/hyperv/hyperv_util.c
> >  src/hyperv/hyperv_wmi.c
> >  src/hypervisor/domain_cgroup.c
> >  src/hypervisor/domain_driver.c
> >+src/hypervisor/domain_interface.c
> >  src/hypervisor/virhostdev.c
> >  src/interface/interface_backend_netcf.c
> >  src/interface/interface_backend_udev.c
> >diff --git a/src/hypervisor/domain_interface.c 
> >b/src/hypervisor/domain_interface.c
> >new file mode 100644
> >index 00..592c4253df
> >--- /dev/null
> >+++ b/src/hypervisor/domain_interface.c
> >@@ -0,0 +1,280 @@
> >+/*
> >+ * Copyright (C) 2015-2016 Red Hat, Inc.
> >+ * Copyright IBM Corp. 2014
> >+ *
> >+ * domain_interface.c: methods to manage guest/domain interfaces
> >+ *
> >+ * This library is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU Lesser General Public
> >+ * License as published by the Free Software Foundation; either
> >+ * version 2.1 of the License, or (at your option) any later version.
> >+ *
> >+ * This library is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >+ * Lesser General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU Lesser General Public
> >+ * License along with this library.  If not, see
> >+ * .
> >+ */
> >+
> >+#include 
> >+
> >+#include "virconftypes.h"
> >+#include "virmacaddr.h"
> >+#include "virnetdevtap.h"
> >+#include "domain_audit.h"
> >+#include "domain_conf.h"
> >+#include "domain_interface.h"
> >+#include "domain_nwfilter.h"
> >+#include "viralloc.h"
> >+#include "virebtables.h"
> >+#include "virfile.h"
> >+#include "virlog.h"
> >+#include "virnetdevbridge.h"
> >+#include "network_conf.h"
> >+
> >+#define VIR_FROM_THIS VIR_FROM_INTERFACE
> >+
> >+VIR_LOG_INIT("interface.interface_connect");
> >+
> >+bool
> >+virDomainInterfaceIsVnetCompatModel(const virDomainNetDef *net)
> >+{
> >+return (virDomainNetIsVirtioModel(net) ||
> >+net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
> >+net->model == VIR_DOMAIN_NET_MODEL_IGB ||
> >+net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
> >+}
> >+
> >+/* virDomainInterfaceEthernetConnect:
> >+ * @def: the definition of the VM
> >+ * @driver: qemu driver data
> >+ * @net: pointer to the VM's interface description
> >+ * @tapfd: array of file descriptor return value for the new device
> >+ * @tapfdsize: number of file descriptors in @tapfd
> >+ *
> >+ * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_ETHERNET
> >+ * (i.e. if the connection is made with a tap device)
> >+ */
> >+int
> >+virDomainInterfaceEthernetConnect(virDomainDef *def,
> >+virDomainNetDef *net,
> >+ebtablesContext *ebtables,
> >+bool macFilter,
> >+bool privileged,
> >+int *tapfd,
> >+size_t tapfdSize)
> 
> This is all fine as far as it goes, but I was expecting that all of
> the functions at the same level (qemuInterface*Connect()) would be
> genericized and moved to the new file. I guess you're only planning
> to use the plain "ethernet" type of interface in the ch driver, but
> for QEMU to have some of the *Connect functions in one place and
> some in another makes the code more confusing; this is especially so
> since virDomainInterfaceStartDevice() has cases for these other
> interface types.

I started with enabling the "ethernet" mode, so I moved all the related
methods. I am planning to also enable "Bridge" and "NAT" modes too in follow
up patches. I did this to test the methods properly before I push.

> 
> I'm 

Re: [libvirt PATCH v2] hypervisor: Move interface mgmt methods to hypervisor

2023-10-18 Thread Laine Stump

On 10/16/23 3:34 PM, Praveen K Paladugu wrote:

Move guest interface management methods from qemu to hypervisor. These
methods will be shared by networking support in ch driver.

Signed-off-by: Praveen K Paladugu 
---
  po/POTFILES   |   1 +
  src/hypervisor/domain_interface.c | 280 ++
  src/hypervisor/domain_interface.h |  39 +
  src/hypervisor/meson.build|   1 +
  src/libvirt_private.syms  |   6 +
  src/qemu/qemu_command.c   |   7 +-
  src/qemu/qemu_hotplug.c   |   3 +-
  src/qemu/qemu_interface.c | 246 +-
  src/qemu/qemu_interface.h |   6 -
  src/qemu/qemu_process.c   |   3 +-
  10 files changed, 343 insertions(+), 249 deletions(-)
  create mode 100644 src/hypervisor/domain_interface.c
  create mode 100644 src/hypervisor/domain_interface.h

diff --git a/po/POTFILES b/po/POTFILES
index 3a51aea5cb..023c041f61 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -92,6 +92,7 @@ src/hyperv/hyperv_util.c
  src/hyperv/hyperv_wmi.c
  src/hypervisor/domain_cgroup.c
  src/hypervisor/domain_driver.c
+src/hypervisor/domain_interface.c
  src/hypervisor/virhostdev.c
  src/interface/interface_backend_netcf.c
  src/interface/interface_backend_udev.c
diff --git a/src/hypervisor/domain_interface.c 
b/src/hypervisor/domain_interface.c
new file mode 100644
index 00..592c4253df
--- /dev/null
+++ b/src/hypervisor/domain_interface.c
@@ -0,0 +1,280 @@
+/*
+ * Copyright (C) 2015-2016 Red Hat, Inc.
+ * Copyright IBM Corp. 2014
+ *
+ * domain_interface.c: methods to manage guest/domain interfaces
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "virconftypes.h"
+#include "virmacaddr.h"
+#include "virnetdevtap.h"
+#include "domain_audit.h"
+#include "domain_conf.h"
+#include "domain_interface.h"
+#include "domain_nwfilter.h"
+#include "viralloc.h"
+#include "virebtables.h"
+#include "virfile.h"
+#include "virlog.h"
+#include "virnetdevbridge.h"
+#include "network_conf.h"
+
+#define VIR_FROM_THIS VIR_FROM_INTERFACE
+
+VIR_LOG_INIT("interface.interface_connect");
+
+bool
+virDomainInterfaceIsVnetCompatModel(const virDomainNetDef *net)
+{
+return (virDomainNetIsVirtioModel(net) ||
+net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
+net->model == VIR_DOMAIN_NET_MODEL_IGB ||
+net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
+}
+
+/* virDomainInterfaceEthernetConnect:
+ * @def: the definition of the VM
+ * @driver: qemu driver data
+ * @net: pointer to the VM's interface description
+ * @tapfd: array of file descriptor return value for the new device
+ * @tapfdsize: number of file descriptors in @tapfd
+ *
+ * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_ETHERNET
+ * (i.e. if the connection is made with a tap device)
+ */
+int
+virDomainInterfaceEthernetConnect(virDomainDef *def,
+virDomainNetDef *net,
+ebtablesContext *ebtables,
+bool macFilter,
+bool privileged,
+int *tapfd,
+size_t tapfdSize)


This is all fine as far as it goes, but I was expecting that all of the 
functions at the same level (qemuInterface*Connect()) would be 
genericized and moved to the new file. I guess you're only planning to 
use the plain "ethernet" type of interface in the ch driver, but for 
QEMU to have some of the *Connect functions in one place and some in 
another makes the code more confusing; this is especially so since 
virDomainInterfaceStartDevice() has cases for these other interface types.


I'm guessing possibly one of the reasons for not moving the Connect() 
functions for the other types over was because they are using 
QEMU-specific datatypes and functions that can't be called from the 
hypervisor directory (because files there can only call to functions in 
src/util, src/conf, and src/hypervisor). But that can be solved fairly 
easily since the QEMU-specific stuff is just some strings in the driver 
config - you can just modify the QEMU caller of that code to retrieve 
the string from the QEMU config and pass it to the Connect() function as 
a string.


Along with this, it would be more cohesive 

[libvirt PATCH v2] hypervisor: Move interface mgmt methods to hypervisor

2023-10-16 Thread Praveen K Paladugu
Move guest interface management methods from qemu to hypervisor. These
methods will be shared by networking support in ch driver.

Signed-off-by: Praveen K Paladugu 
---
 po/POTFILES   |   1 +
 src/hypervisor/domain_interface.c | 280 ++
 src/hypervisor/domain_interface.h |  39 +
 src/hypervisor/meson.build|   1 +
 src/libvirt_private.syms  |   6 +
 src/qemu/qemu_command.c   |   7 +-
 src/qemu/qemu_hotplug.c   |   3 +-
 src/qemu/qemu_interface.c | 246 +-
 src/qemu/qemu_interface.h |   6 -
 src/qemu/qemu_process.c   |   3 +-
 10 files changed, 343 insertions(+), 249 deletions(-)
 create mode 100644 src/hypervisor/domain_interface.c
 create mode 100644 src/hypervisor/domain_interface.h

diff --git a/po/POTFILES b/po/POTFILES
index 3a51aea5cb..023c041f61 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -92,6 +92,7 @@ src/hyperv/hyperv_util.c
 src/hyperv/hyperv_wmi.c
 src/hypervisor/domain_cgroup.c
 src/hypervisor/domain_driver.c
+src/hypervisor/domain_interface.c
 src/hypervisor/virhostdev.c
 src/interface/interface_backend_netcf.c
 src/interface/interface_backend_udev.c
diff --git a/src/hypervisor/domain_interface.c 
b/src/hypervisor/domain_interface.c
new file mode 100644
index 00..592c4253df
--- /dev/null
+++ b/src/hypervisor/domain_interface.c
@@ -0,0 +1,280 @@
+/*
+ * Copyright (C) 2015-2016 Red Hat, Inc.
+ * Copyright IBM Corp. 2014
+ *
+ * domain_interface.c: methods to manage guest/domain interfaces
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "virconftypes.h"
+#include "virmacaddr.h"
+#include "virnetdevtap.h"
+#include "domain_audit.h"
+#include "domain_conf.h"
+#include "domain_interface.h"
+#include "domain_nwfilter.h"
+#include "viralloc.h"
+#include "virebtables.h"
+#include "virfile.h"
+#include "virlog.h"
+#include "virnetdevbridge.h"
+#include "network_conf.h"
+
+#define VIR_FROM_THIS VIR_FROM_INTERFACE
+
+VIR_LOG_INIT("interface.interface_connect");
+
+bool
+virDomainInterfaceIsVnetCompatModel(const virDomainNetDef *net)
+{
+return (virDomainNetIsVirtioModel(net) ||
+net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
+net->model == VIR_DOMAIN_NET_MODEL_IGB ||
+net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
+}
+
+/* virDomainInterfaceEthernetConnect:
+ * @def: the definition of the VM
+ * @driver: qemu driver data
+ * @net: pointer to the VM's interface description
+ * @tapfd: array of file descriptor return value for the new device
+ * @tapfdsize: number of file descriptors in @tapfd
+ *
+ * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_ETHERNET
+ * (i.e. if the connection is made with a tap device)
+ */
+int
+virDomainInterfaceEthernetConnect(virDomainDef *def,
+virDomainNetDef *net,
+ebtablesContext *ebtables,
+bool macFilter,
+bool privileged,
+int *tapfd,
+size_t tapfdSize)
+{
+virMacAddr tapmac;
+int ret = -1;
+unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
+bool template_ifname = false;
+const char *tunpath = "/dev/net/tun";
+const char *auditdev = tunpath;
+
+if (net->backend.tap) {
+tunpath = net->backend.tap;
+if (!privileged) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("cannot use custom tap device in session mode"));
+goto cleanup;
+}
+}
+VIR_WARN("PPK: interfaceEthernetConnect Called\n");
+if (virDomainInterfaceIsVnetCompatModel(net))
+tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
+
+if (net->managed_tap == VIR_TRISTATE_BOOL_NO) {
+if (!net->ifname) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("target dev must be supplied when managed='no'"));
+goto cleanup;
+}
+if (virNetDevExists(net->ifname) != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("target managed='no' but specified dev doesn't 
exist"));
+goto cleanup;
+}
+
+

Re: [libvirt PATCH v2 0/1] meson: Improve nbdkit configurability

2023-10-05 Thread Jonathon Jongsma

On 10/5/23 4:22 AM, Andrea Bolognani wrote:

Changes from [v1]:

   * disable nbdkit on anything older than Fedora 40 in the RPM.

[v1] https://listman.redhat.com/archives/libvir-list/2023-October/242498.html

Andrea Bolognani (1):
   meson: Improve nbdkit configurability

  libvirt.spec.in| 28 +---
  meson.build| 29 +
  meson_options.txt  |  2 +-
  src/qemu/qemu_nbdkit.c |  6 +++---
  4 files changed, 50 insertions(+), 15 deletions(-)




Reviewed-by: Jonathon Jongsma 



[libvirt PATCH v2 1/1] meson: Improve nbdkit configurability

2023-10-05 Thread Andrea Bolognani
Currently, nbdkit support will automatically be enabled as long as
the pidfd_open(2) syscall is available. Optionally, libnbd is used
to generate more user-friendly error messages.

In theory this is all good, since use of nbdkit is supposed to be
transparent to the user. In practice, however, there is a problem:
if support for it is enabled at build time and the necessary
runtime components are installed, nbdkit will always be preferred,
with no way for the user to opt out.

This will arguably be fine in the long run, but right now none of
the platforms that we target ships with a SELinux policy that
allows libvirt to launch nbdkit, and the AppArmor policy that we
maintain ourselves hasn't been updated either.

So, in practice, as of today having nbdkit installed on the host
makes network disks completely unusable unless you're willing to
compromise the overall security of the system by disabling
SELinux/AppArmor.

In order to make the transition smoother, provide a convenient
way for users and distro packagers to disable nbdkit support at
compile time until SELinux and AppArmor are ready.

In the process, detection is completely overhauled. libnbd is
made mandatory when nbdkit support is enabled, since availability
across operating systems is comparable and offering users the
option to make error messages worse doesn't make a lot of sense;
we also make sure that an explicit request from the user to
enable/disable nbdkit support is either complied with, or results
in a build failure when that's not possible. Last but not least,
we avoid linking against libnbd when nbdkit support is disabled.

At the RPM level, we disable the feature when building against
anything older than Fedora 40, which still doesn't have the
necessary SELinux bits but will hopefully gain them by the time
it's released. We also allow nbdkit support to be disabled at
build time the same way as other optional features, that is, by
passing "--define '_without_nbdkit 1'" to rpmbuild. Finally, if
nbdkit support has been disabled, installing libvirt will no
longer drag it in as a (weak) dependency.

Signed-off-by: Andrea Bolognani 
---
 libvirt.spec.in| 28 +---
 meson.build| 29 +
 meson_options.txt  |  2 +-
 src/qemu/qemu_nbdkit.c |  6 +++---
 4 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index f3d21ccc8f..fe54c45c5c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -95,6 +95,7 @@
 %define with_fuse 0
 %define with_sanlock  0
 %define with_numad0
+%define with_nbdkit   0
 %define with_firewalld_zone   0
 %define with_netcf0
 %define with_libssh2  0
@@ -173,6 +174,18 @@
 %endif
 %endif
 
+# We should only enable nbdkit support if the OS ships a SELinux policy that
+# allows libvirt to launch it. Right now that's not the case anywhere, but
+# things should be fine by the time Fedora 40 is released.
+#
+# TODO: add RHEL 9 once a minor release that contains the necessary SELinux
+#   bits exists (we only support the most recent minor release)
+%if %{with_qemu}
+%if 0%{?fedora} >= 40
+%define with_nbdkit 0%{!?_without_nbdkit:1}
+%endif
+%endif
+
 %ifarch %{arches_dmidecode}
 %define with_dmidecode 0%{!?_without_dmidecode:1}
 %endif
@@ -312,6 +325,9 @@ BuildRequires: util-linux
 BuildRequires: libacl-devel
 # From QEMU RPMs, used by virstoragetest
 BuildRequires: /usr/bin/qemu-img
+%endif
+# nbdkit support requires libnbd
+%if %{with_nbdkit}
 BuildRequires: libnbd-devel
 %endif
 # For LVM drivers
@@ -769,9 +785,11 @@ Requires: numad
 Recommends: passt
 Recommends: passt-selinux
 %endif
+%if %{with_nbdkit}
 Recommends: nbdkit
 Recommends: nbdkit-curl-plugin
 Recommends: nbdkit-ssh-plugin
+%endif
 
 %description daemon-driver-qemu
 The qemu driver plugin for the libvirtd daemon, providing
@@ -1078,10 +1096,8 @@ exit 1
 
 %if %{with_qemu}
 %define arg_qemu -Ddriver_qemu=enabled
-%define arg_libnbd -Dlibnbd=enabled
 %else
 %define arg_qemu -Ddriver_qemu=disabled
-%define arg_libnbd -Dlibnbd=disabled
 %endif
 
 %if %{with_openvz}
@@ -1158,6 +1174,12 @@ exit 1
 %define arg_numad -Dnumad=disabled
 %endif
 
+%if %{with_nbdkit}
+%define arg_nbdkit -Dnbdkit=enabled
+%else
+%define arg_nbdkit -Dnbdkit=disabled
+%endif
+
 %if %{with_fuse}
 %define arg_fuse -Dfuse=enabled
 %else
@@ -1270,7 +1292,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' 
%{_specdir}/libvirt.spec)
-Dyajl=enabled \
%{?arg_sanlock} \
-Dlibpcap=enabled \
-   %{?arg_libnbd} \
+   %{?arg_nbdkit} \
-Dlibnl=enabled \
-Daudit=enabled \
-Ddtrace=enabled \
diff --git a/meson.build b/meson.build
index 6fa1f74670..de23fbda1e 100644
--- a/meson.build
+++ b/meson.build
@@ -1011,10 +1011,27 @@ endif
 libiscsi_version = '1.18.0'
 libiscsi_dep = 

[libvirt PATCH v2 0/1] meson: Improve nbdkit configurability

2023-10-05 Thread Andrea Bolognani
Changes from [v1]:

  * disable nbdkit on anything older than Fedora 40 in the RPM.

[v1] https://listman.redhat.com/archives/libvir-list/2023-October/242498.html

Andrea Bolognani (1):
  meson: Improve nbdkit configurability

 libvirt.spec.in| 28 +---
 meson.build| 29 +
 meson_options.txt  |  2 +-
 src/qemu/qemu_nbdkit.c |  6 +++---
 4 files changed, 50 insertions(+), 15 deletions(-)

-- 
2.41.0



Re: [libvirt PATCH v2 31/33] systemd: Add RemoveOnStop=yes to all sockets

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:32PM +0200, Andrea Bolognani wrote:
> Currently we only set this for the main sockets, which means
> that
> 
>   $ systemctl stop virtqemud.socket
> 
> will make the socket disappear from the filesystem while
> 
>   $ systemctl stop virtqemud-ro.socket
> 
> won't. Get rid of this inconsistency.

systemd recommands against using RemoveOnStop, on the basis that
it is valid to keep the service running but stop the socket.
We've used deps to ensure thats not possible though, so adding
RemoveOnStop isn't creating problems we don't already have.

> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/locking/virtlockd-admin.socket.in | 1 +
>  src/locking/virtlockd.socket.in   | 1 +
>  src/logging/virtlogd-admin.socket.in  | 1 +
>  src/logging/virtlogd.socket.in| 1 +
>  src/remote/libvirtd-admin.socket.in   | 1 +
>  src/remote/libvirtd-ro.socket.in  | 1 +
>  src/virtd-admin.socket.in | 1 +
>  src/virtd-ro.socket.in| 1 +
>  8 files changed, 8 insertions(+)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 06/33] systemd: Introduce common templates

2023-09-28 Thread Daniel P . Berrangé
On Thu, Sep 28, 2023 at 06:52:35AM -0500, Andrea Bolognani wrote:
> On Thu, Sep 28, 2023 at 04:30:03AM -0500, Andrea Bolognani wrote:
> > On Thu, Sep 28, 2023 at 09:24:11AM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Sep 27, 2023 at 06:19:07PM +0200, Andrea Bolognani wrote:
> > > > +++ b/scripts/merge-systemd-units.py
> > > > @@ -0,0 +1,91 @@
> > > > +#!/usr/bin/env python3
> > >
> > > Stick a license header of SPDX tag on this.
> >
> > Done (patch below).
> >
> > > Also if you didn't already do it, run the file through 'black'
> > > and let it do whatever it wants todo to formatting.
> >
> > It just changed all single quotes into double quotes :)
> >
> >
> > - 8< - 8< - 8< - 8< - 8< - 8< - 8< - 8< 
> > -
> > diff --git a/scripts/merge-systemd-units.py b/scripts/merge-systemd-units.py
> > index f54c9556c9..bc3321230d 100755
> > --- a/scripts/merge-systemd-units.py
> > +++ b/scripts/merge-systemd-units.py
> > @@ -1,5 +1,8 @@
> >  #!/usr/bin/env python3
> >
> > +# Copyright (C) 2023 Red Hat, Inc.
> > +# SPDX-License-Identifier: LGPL-2.1-or-later
> > +
> >  import sys
> >
> >  SECTIONS = [
> > - >8 - >8 - >8 - >8 - >8 - >8 - >8 - >8 
> > -
> 
> Can I consider the patch Reviewed-by: you with the above (and the
> trivial changes to quotess applied by black) squashed in, or do you
> want me to send a v3 for that? Everything else is ACKed at this
> point, but I'm not going to push until 9.9.0 is open for business
> anyway.

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 06/33] systemd: Introduce common templates

2023-09-28 Thread Andrea Bolognani
On Thu, Sep 28, 2023 at 04:30:03AM -0500, Andrea Bolognani wrote:
> On Thu, Sep 28, 2023 at 09:24:11AM +0100, Daniel P. Berrangé wrote:
> > On Wed, Sep 27, 2023 at 06:19:07PM +0200, Andrea Bolognani wrote:
> > > +++ b/scripts/merge-systemd-units.py
> > > @@ -0,0 +1,91 @@
> > > +#!/usr/bin/env python3
> >
> > Stick a license header of SPDX tag on this.
>
> Done (patch below).
>
> > Also if you didn't already do it, run the file through 'black'
> > and let it do whatever it wants todo to formatting.
>
> It just changed all single quotes into double quotes :)
>
>
> - 8< - 8< - 8< - 8< - 8< - 8< - 8< - 8< -
> diff --git a/scripts/merge-systemd-units.py b/scripts/merge-systemd-units.py
> index f54c9556c9..bc3321230d 100755
> --- a/scripts/merge-systemd-units.py
> +++ b/scripts/merge-systemd-units.py
> @@ -1,5 +1,8 @@
>  #!/usr/bin/env python3
>
> +# Copyright (C) 2023 Red Hat, Inc.
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +
>  import sys
>
>  SECTIONS = [
> - >8 - >8 - >8 - >8 - >8 - >8 - >8 - >8 -

Can I consider the patch Reviewed-by: you with the above (and the
trivial changes to quotess applied by black) squashed in, or do you
want me to send a v3 for that? Everything else is ACKed at this
point, but I'm not going to push until 9.9.0 is open for business
anyway.

By the way, thank you for the review! And thanks to both you and
Pavel for pushing me in the direction of having most of the
processing performed by an external Python script instead of directly
by meson! It ended up looking *a lot* nicer than what I had :)

-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [libvirt PATCH v2 10/33] systemd: Switch virtnwfilterd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:11PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/nwfilter/meson.build  |  4 
>  src/nwfilter/virtnwfilterd.service.in | 25 -
>  2 files changed, 29 deletions(-)
>  delete mode 100644 src/nwfilter/virtnwfilterd.service.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 07/33] systemd: Use common templates by default

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:08PM +0200, Andrea Bolognani wrote:
> All services are still listing their input files explicitly, so
> no changes to the output files will occur yet.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/meson.build | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 26/33] systemd: Downgrade read-only/admin sockets to Wants

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:27PM +0200, Andrea Bolognani wrote:
> Only the main socket is actually necessary for the service to be
> usable.
> 
> In the past, we've had security issues that could be exploited via
> access to the read-only socket, so a security-minded administrator
> might consider disabling all optional sockets. This change makes
> such a setup possible.
> 
> Note that the services will still try to activate all their
> sockets on startup, even if they have been disabled. To make sure
> that the optional sockets are never started, they will have to be
> masked.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/locking/virtlockd.service.in | 2 +-
>  src/logging/virtlogd.service.in  | 2 +-
>  src/virtd.service.in | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 30/33] systemd: Add Also between sockets

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:31PM +0200, Andrea Bolognani wrote:
> This results in all sockets for a service being enabled when a
> single one of them is.
> 
> The -tcp and -tls sockets are intentionally excluded, because
> enabling them should require explicit action on the
> administrator's part; moreover, disabling them should not result
> in the local sockets being disabled too.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/locking/virtlockd-admin.socket.in | 1 +
>  src/locking/virtlockd.socket.in   | 1 +
>  src/logging/virtlogd-admin.socket.in  | 1 +
>  src/logging/virtlogd.socket.in| 1 +
>  src/remote/libvirtd-admin.socket.in   | 2 ++
>  src/remote/libvirtd-ro.socket.in  | 2 ++
>  src/remote/libvirtd.socket.in | 2 ++
>  src/virtd-admin.socket.in | 2 ++
>  src/virtd-ro.socket.in| 2 ++
>  src/virtd.socket.in   | 2 ++
>  10 files changed, 16 insertions(+)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 32/33] systemd: Improve and unify unit descriptions

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:33PM +0200, Andrea Bolognani wrote:
> Hypervisors are referred to by their user-facing name rather
> than the name of their libvirt driver, the monolithic daemon is
> explicitly referred to as legacy, and a consistent format is
> used throughout.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/ch/meson.build| 2 +-
>  src/interface/meson.build | 2 +-
>  src/libxl/meson.build | 2 +-
>  src/locking/meson.build   | 2 +-
>  src/locking/virtlockd-admin.socket.in | 2 +-
>  src/locking/virtlockd.service.in  | 2 +-
>  src/locking/virtlockd.socket.in   | 2 +-
>  src/logging/meson.build   | 2 +-
>  src/logging/virtlogd-admin.socket.in  | 2 +-
>  src/logging/virtlogd.service.in   | 2 +-
>  src/logging/virtlogd.socket.in| 2 +-
>  src/lxc/meson.build   | 2 +-
>  src/network/meson.build   | 2 +-
>  src/node_device/meson.build   | 2 +-
>  src/nwfilter/meson.build  | 2 +-
>  src/qemu/meson.build  | 2 +-
>  src/remote/libvirtd-admin.socket.in   | 2 +-
>  src/remote/libvirtd-ro.socket.in  | 2 +-
>  src/remote/libvirtd-tcp.socket.in | 2 +-
>  src/remote/libvirtd-tls.socket.in | 2 +-
>  src/remote/libvirtd.service.in| 2 +-
>  src/remote/libvirtd.socket.in | 2 +-
>  src/remote/meson.build| 4 ++--
>  src/secret/meson.build| 2 +-
>  src/storage/meson.build   | 2 +-
>  src/vbox/meson.build  | 2 +-
>  src/virtd-admin.socket.in | 2 +-
>  src/virtd-ro.socket.in| 2 +-
>  src/virtd-tcp.socket.in   | 2 +-
>  src/virtd-tls.socket.in   | 2 +-
>  src/virtd.service.in  | 2 +-
>  src/virtd.socket.in   | 2 +-
>  src/vz/meson.build| 2 +-
>  33 files changed, 34 insertions(+), 34 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 14/33] systemd: Switch virtvboxd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:15PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/vbox/meson.build|  5 +
>  src/vbox/virtvboxd.service.extra.in |  2 ++
>  src/vbox/virtvboxd.service.in   | 26 --
>  3 files changed, 3 insertions(+), 30 deletions(-)
>  create mode 100644 src/vbox/virtvboxd.service.extra.in
>  delete mode 100644 src/vbox/virtvboxd.service.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 08/33] systemd: Switch virtnodedevd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Thu, Sep 28, 2023 at 05:38:45AM -0500, Andrea Bolognani wrote:
> On Thu, Sep 28, 2023 at 11:16:53AM +0100, Daniel P. Berrangé wrote:
> > On Wed, Sep 27, 2023 at 06:19:09PM +0200, Andrea Bolognani wrote:
> > > Signed-off-by: Andrea Bolognani 
> > > ---
> > >  src/node_device/meson.build |  4 
> > >  src/node_device/virtnodedevd.service.in | 25 -
> >
> > >  2 files changed, 29 deletions(-)
> > >  delete mode 100644 src/node_device/virtnodedevd.service.in
> >
> > Reviewed-by: Daniel P. Berrangé 
> >
> > Though I wonder if its worth just keeping an empty stub here, with
> > the section headings. It'd be a little confusing to see the stubs
> > present for some daemons but not others.
> 
> We'd have to do the same for sockets then, on account of virtxend
> using an override for them.
> 
> Maybe we could change the merge script so that contents before the
> start of the first section are simply ignored, and then have
> 
>   $ cat src/node_device/virtnodedevd.service.in
>   # Merged into src/virtd.service.in
>   $ cat src/node_device/virtnodedevd.socket.in
>   # Merged into src/virtd*.socket.in
> 
> for services that don't need any overrides, and
> 
>   $ cat src/libxl/virtxend.service.extra.in
>   # Merged into src/virtd.service.in
> 
>   [Unit]
>   Wants=virtlockd.socket
>   After=virtlockd.socket
>   ...
>   $ cat src/libxl/virtxend.socket.extra.in
>   # Merged into src/virtd*.socket.in
> 
>   [Unit]
>   ConditionPathExists=/proc/xen/capabilities
> 
> for services that do. It would mean introducing quite a number of
> additional files, but maybe the advantages in terms of
> discoverability make up for that downside?

Yeah, I think that's a nice idea.

> If we allow empty overrides, we might be even able to simplify the
> way the various services are defined in their meson.build files, by
> somehow deriving the path of the file instead of requiring it to be
> provided explicitly. That part could be tricky though.
> 
> Overall I'm not opposed to the idea, but let's consider it for a
> follow-up instead of stalling this further, okay?

Sure

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 16/33] systemd: Switch virtchd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:17PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/ch/meson.build  |  5 +---
>  src/ch/virtchd.service.extra.in | 22 +
>  src/ch/virtchd.service.in   | 44 -
>  3 files changed, 23 insertions(+), 48 deletions(-)
>  create mode 100644 src/ch/virtchd.service.extra.in
>  delete mode 100644 src/ch/virtchd.service.in

Reviewed-by: Daniel P. Berrangé 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 13/33] systemd: Switch virtstoraged to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:14PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/storage/meson.build   |  5 +
>  src/storage/virtstoraged.service.extra.in |  3 +++
>  src/storage/virtstoraged.service.in   | 27 ---
>  3 files changed, 4 insertions(+), 31 deletions(-)
>  create mode 100644 src/storage/virtstoraged.service.extra.in
>  delete mode 100644 src/storage/virtstoraged.service.in

Reviewed-by: Daniel P. Berrangé 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 09/33] systemd: Switch virtinterfaced to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:10PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/interface/meson.build   |  4 
>  src/interface/virtinterfaced.service.in | 25 -
>  2 files changed, 29 deletions(-)
>  delete mode 100644 src/interface/virtinterfaced.service.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 15/33] systemd: Switch virtvzd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:16PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/vz/meson.build  |  5 +
>  src/vz/virtvzd.service.extra.in |  2 ++
>  src/vz/virtvzd.service.in   | 26 --
>  3 files changed, 3 insertions(+), 30 deletions(-)
>  create mode 100644 src/vz/virtvzd.service.extra.in
>  delete mode 100644 src/vz/virtvzd.service.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 19/33] systemd: Switch virtqemud to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:20PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/qemu/meson.build|  5 +--
>  src/qemu/virtqemud.service.extra.in | 28 +
>  src/qemu/virtqemud.service.in   | 48 -
>  3 files changed, 29 insertions(+), 52 deletions(-)
>  create mode 100644 src/qemu/virtqemud.service.extra.in
>  delete mode 100644 src/qemu/virtqemud.service.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 08/33] systemd: Switch virtnodedevd to common templates

2023-09-28 Thread Andrea Bolognani
On Thu, Sep 28, 2023 at 11:16:53AM +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 27, 2023 at 06:19:09PM +0200, Andrea Bolognani wrote:
> > Signed-off-by: Andrea Bolognani 
> > ---
> >  src/node_device/meson.build |  4 
> >  src/node_device/virtnodedevd.service.in | 25 -
>
> >  2 files changed, 29 deletions(-)
> >  delete mode 100644 src/node_device/virtnodedevd.service.in
>
> Reviewed-by: Daniel P. Berrangé 
>
> Though I wonder if its worth just keeping an empty stub here, with
> the section headings. It'd be a little confusing to see the stubs
> present for some daemons but not others.

We'd have to do the same for sockets then, on account of virtxend
using an override for them.

Maybe we could change the merge script so that contents before the
start of the first section are simply ignored, and then have

  $ cat src/node_device/virtnodedevd.service.in
  # Merged into src/virtd.service.in
  $ cat src/node_device/virtnodedevd.socket.in
  # Merged into src/virtd*.socket.in

for services that don't need any overrides, and

  $ cat src/libxl/virtxend.service.extra.in
  # Merged into src/virtd.service.in

  [Unit]
  Wants=virtlockd.socket
  After=virtlockd.socket
  ...
  $ cat src/libxl/virtxend.socket.extra.in
  # Merged into src/virtd*.socket.in

  [Unit]
  ConditionPathExists=/proc/xen/capabilities

for services that do. It would mean introducing quite a number of
additional files, but maybe the advantages in terms of
discoverability make up for that downside?

If we allow empty overrides, we might be even able to simplify the
way the various services are defined in their meson.build files, by
somehow deriving the path of the file instead of requiring it to be
provided explicitly. That part could be tricky though.

Overall I'm not opposed to the idea, but let's consider it for a
follow-up instead of stalling this further, okay?

-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [libvirt PATCH v2 21/33] systemd: Drop libvirtd_socket*_in values

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:22PM +0200, Andrea Bolognani wrote:
> Now that the migration to common templates has been completed,
> we no longer need these.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/meson.build | 4 
>  1 file changed, 4 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 11/33] systemd: Switch virtsecretd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:12PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/secret/meson.build|  4 
>  src/secret/virtsecretd.service.in | 25 -
>  2 files changed, 29 deletions(-)
>  delete mode 100644 src/secret/virtsecretd.service.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 08/33] systemd: Switch virtnodedevd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:09PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/node_device/meson.build |  4 
>  src/node_device/virtnodedevd.service.in | 25 -

>  2 files changed, 29 deletions(-)
>  delete mode 100644 src/node_device/virtnodedevd.service.in

Reviewed-by: Daniel P. Berrangé 

Though I wonder if its worth just keeping an empty stub here, with
the section headings. It'd be a little confusing to see the stubs
present for some daemons but not others.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 02/33] systemd: Introduce service_in/service_out variables

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:03PM +0200, Andrea Bolognani wrote:
> They're similar to the existing socket_in/socket_out variables
> and will make future changes nicer.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/meson.build | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 18/33] systemd: Switch virtlxcd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:19PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/lxc/meson.build   |  5 +---
>  src/lxc/virtlxcd.service.extra.in | 22 
>  src/lxc/virtlxcd.service.in   | 44 ---
>  3 files changed, 23 insertions(+), 48 deletions(-)
>  create mode 100644 src/lxc/virtlxcd.service.extra.in
>  delete mode 100644 src/lxc/virtlxcd.service.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 20/33] systemd: Switch virtproxyd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:21PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/remote/meson.build   |  6 --
>  src/remote/virtproxyd.service.in | 25 -
>  2 files changed, 31 deletions(-)
>  delete mode 100644 src/remote/virtproxyd.service.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 12/33] systemd: Switch virtnetworkd to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:13PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/network/meson.build   |  5 +
>  src/network/virtnetworkd.service.extra.in |  2 ++
>  src/network/virtnetworkd.service.in   | 26 ---
>  3 files changed, 3 insertions(+), 30 deletions(-)
>  create mode 100644 src/network/virtnetworkd.service.extra.in
>  delete mode 100644 src/network/virtnetworkd.service.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 17/33] systemd: Switch virtxend to common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:18PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/libxl/meson.build   |  7 ++-
>  src/libxl/virtxend.service.extra.in | 12 +++
>  src/libxl/virtxend.service.in   | 32 -
>  src/libxl/virtxend.socket.extra.in  |  2 ++
>  4 files changed, 16 insertions(+), 37 deletions(-)
>  create mode 100644 src/libxl/virtxend.service.extra.in
>  delete mode 100644 src/libxl/virtxend.service.in
>  create mode 100644 src/libxl/virtxend.socket.extra.in

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 04/33] systemd: Introduce temporary libvirtd_socket*_in values

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:05PM +0200, Andrea Bolognani wrote:
> These will be useful during the upcoming migration to common
> templates for systemd units and will be dropped as soon as all
> services have been converted.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/meson.build | 4 
>  1 file changed, 4 insertions(+)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 03/33] systemd: Make @service_in@ optional

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:04PM +0200, Andrea Bolognani wrote:
> It is currently considered required, but we're soon going to
> provide a default that will be suitable for most services.
> 
> Since all services currently provide a value explicitly, we
> can implement a default without breaking anything.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/meson.build | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 06/33] systemd: Introduce common templates

2023-09-28 Thread Andrea Bolognani
On Thu, Sep 28, 2023 at 09:24:11AM +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 27, 2023 at 06:19:07PM +0200, Andrea Bolognani wrote:
> > +++ b/scripts/merge-systemd-units.py
> > @@ -0,0 +1,91 @@
> > +#!/usr/bin/env python3
>
> Stick a license header of SPDX tag on this.

Done (patch below).

> Also if you didn't already do it, run the file through 'black'
> and let it do whatever it wants todo to formatting.

It just changed all single quotes into double quotes :)


- 8< - 8< - 8< - 8< - 8< - 8< - 8< - 8< -
diff --git a/scripts/merge-systemd-units.py b/scripts/merge-systemd-units.py
index f54c9556c9..bc3321230d 100755
--- a/scripts/merge-systemd-units.py
+++ b/scripts/merge-systemd-units.py
@@ -1,5 +1,8 @@
 #!/usr/bin/env python3

+# Copyright (C) 2023 Red Hat, Inc.
+# SPDX-License-Identifier: LGPL-2.1-or-later
+
 import sys

 SECTIONS = [
- >8 - >8 - >8 - >8 - >8 - >8 - >8 - >8 -
-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [libvirt PATCH v2 05/33] systemd: Provide all input files explicitly

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:06PM +0200, Andrea Bolognani wrote:
> We're about to change the defaults and start migrating to common
> templates: in order to be able to switch units over one at a
> time, make the input files that are currently used explicit
> rather than implicit.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/ch/meson.build  |  3 +++
>  src/interface/meson.build   |  3 +++
>  src/libxl/meson.build   |  3 +++
>  src/lxc/meson.build |  3 +++
>  src/network/meson.build |  3 +++
>  src/node_device/meson.build |  3 +++
>  src/nwfilter/meson.build|  3 +++
>  src/qemu/meson.build|  3 +++
>  src/remote/meson.build  | 10 ++
>  src/secret/meson.build  |  3 +++
>  src/storage/meson.build |  3 +++
>  src/vbox/meson.build|  3 +++
>  src/vz/meson.build  |  3 +++
>  13 files changed, 46 insertions(+)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 06/33] systemd: Introduce common templates

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:07PM +0200, Andrea Bolognani wrote:
> We already use templating to generate sockets, which are all
> based off libvirtd's. Push the idea further, and extend it to
> cover services as well.
> 
> This is more challenging, as the various modular daemons each have
> their own needs in terms of what system services needs to be
> available before they can be started, which other components of
> libvirt they depend on, and so on.
> 
> In order to make this sort of per-service tweaks possible, we
> introduce a Python script that can merge two systemd units
> together. The script is aware of the semantics of systemd's unit
> definition format, so it can intelligently merge sections
> together.
> 
> This generic systemd unit merging mechanism will also supersede
> the extremely ad-hoc @deps@ variable, which is currently used in
> a single scenario.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  scripts/merge-systemd-units.py | 91 ++
>  scripts/meson.build|  1 +
>  src/meson.build| 22 
>  src/virtd-admin.socket.in  | 13 +
>  src/virtd-ro.socket.in | 13 +
>  src/virtd-tcp.socket.in| 12 +
>  src/virtd-tls.socket.in| 12 +
>  src/virtd.service.in   | 25 ++
>  src/virtd.socket.in| 12 +
>  9 files changed, 201 insertions(+)
>  create mode 100755 scripts/merge-systemd-units.py
>  create mode 100644 src/virtd-admin.socket.in
>  create mode 100644 src/virtd-ro.socket.in
>  create mode 100644 src/virtd-tcp.socket.in
>  create mode 100644 src/virtd-tls.socket.in
>  create mode 100644 src/virtd.service.in
>  create mode 100644 src/virtd.socket.in
> 
> diff --git a/scripts/merge-systemd-units.py b/scripts/merge-systemd-units.py
> new file mode 100755
> index 00..136bc8d416
> --- /dev/null
> +++ b/scripts/merge-systemd-units.py
> @@ -0,0 +1,91 @@
> +#!/usr/bin/env python3

Stick a license header of SPDX tag on this.

Also if you didn't already do it, run the file through 'black'
and let it do whatever it wants todo to formatting.

...reminds me we really ought to get around to running 'black'
on the rest of our existing python.

> +
> +import sys
> +
> +SECTIONS = [
> +'[Unit]',
> +'[Service]',
> +'[Socket]',
> +'[Install]',
> +]
> +
> +
> +def parse_unit(unit_path):
> +unit = {}
> +current_section = '[Invalid]'
> +
> +with open(unit_path) as f:
> +for line in f:
> +line = line.strip()
> +
> +if line == '':
> +continue
> +
> +if line[0] == '[' and line[-1] == ']':
> +if line not in SECTIONS:
> +print('Unknown section {}'.format(line))
> +sys.exit(1)
> +
> +current_section = line
> +continue
> +
> +if current_section not in unit:
> +unit[current_section] = []
> +
> +unit[current_section].append(line)
> +
> +if '[Invalid]' in unit:
> +print('Contents found outside of any section')
> +sys.exit(1)
> +
> +return unit
> +
> +
> +def format_unit(unit):
> +lines = []
> +
> +for section in SECTIONS:
> +if section not in unit:
> +continue
> +
> +lines.append(section)
> +
> +for line in unit[section]:
> +lines.append(line)
> +
> +lines.append('')
> +
> +return '\n'.join(lines)
> +
> +
> +def merge_units(base, extra):
> +merged = {}
> +
> +for section in SECTIONS:
> +if section in extra and section not in base:
> +print('Section {} in extra but not in base'.format(section))
> +sys.exit(1)
> +
> +if section not in base:
> +continue
> +
> +merged[section] = base[section]
> +
> +if section not in extra:
> +continue
> +
> +merged[section].extend(extra[section])
> +
> +return merged
> +
> +
> +if len(sys.argv) < 2:
> +print('usage: {} BASE EXTRA'.format(sys.argv[0]))
> +sys.exit(1)
> +
> +base = parse_unit(sys.argv[1])
> +extra = parse_unit(sys.argv[2])
> +
> +merged = merge_units(base, extra)
> +
> +sys.stdout.write(format_unit(merged))
> diff --git a/scripts/meson.build b/scripts/meson.build
> index 05b71184f1..65fd1e21c5 100644
> --- a/scripts/meson.build
> +++ b/scripts/meson.build
> @@ -19,6 +19,7 @@ scripts = [
>'header-ifdef.py',
>'hvsupport.py',
>'hyperv_wmi_generator.py',
> +  'merge-systemd-units.py',
>'meson-dist.py',
>'meson-gen-authors.py',
>'meson-gen-def.py',
> diff --git a/src/meson.build b/src/meson.build
> index 2fbf98b9fe..02c92621ba 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -203,6 +203,8 @@ libvirtd_socket_admin_in = files('remote' / 
> 'libvirtd-admin.socket.in')
>  #   * sockets - array of additional sockets (optional, default [ 'main', 
> 'ro', 'admin' ])
>  #   * service_in - service source file 

Re: [libvirt PATCH v2 22/33] systemd: Drop @deps@

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:23PM +0200, Andrea Bolognani wrote:
> It's no longer used anywhere.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/meson.build | 2 --
>  src/remote/libvirtd-admin.socket.in | 1 -
>  src/remote/libvirtd-ro.socket.in| 1 -
>  src/remote/libvirtd-tcp.socket.in   | 1 -
>  src/remote/libvirtd-tls.socket.in   | 1 -
>  src/remote/libvirtd.socket.in   | 1 -
>  6 files changed, 7 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 25/33] systemd: Replace Requires with BindTo+After for main socket

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:26PM +0200, Andrea Bolognani wrote:
> This is the strongest relationship that can be declared between
> two units, and causes the service to be terminated immediately
> if its main socket disappears. This is the behavior we want.
> 
> Note that we don't do the same for the read-only/admin sockets,
> because those are not as critical for the core functionality of
> services as the main socket it.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/locking/virtlockd.service.in | 3 ++-
>  src/logging/virtlogd.service.in  | 3 ++-
>  src/virtd.service.in | 3 ++-
>  3 files changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 23/33] systemd: Drop parametrization from libvirtd sockets

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:24PM +0200, Andrea Bolognani wrote:
> Up until now the files have been used as template for most
> services, but now that those have been converted to common
> templates we can drop parametrization and make it clear that
> these files are for libvirtd only.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/remote/libvirtd-admin.socket.in | 10 +-
>  src/remote/libvirtd-ro.socket.in| 10 +-
>  src/remote/libvirtd-tcp.socket.in   |  8 
>  src/remote/libvirtd-tls.socket.in   |  8 
>  src/remote/libvirtd.socket.in   |  6 +++---
>  5 files changed, 21 insertions(+), 21 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 01/33] systemd: Drop Conflicts from virtproxyd sockets

2023-09-28 Thread Daniel P . Berrangé
On Wed, Sep 27, 2023 at 06:19:02PM +0200, Andrea Bolognani wrote:
> The idea behind these is to prevent running both modular daemons
> and monolithic daemon at the same time. We will implement a more
> effective solution for that shortly.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/remote/meson.build | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[libvirt PATCH v2 22/33] systemd: Drop @deps@

2023-09-27 Thread Andrea Bolognani
It's no longer used anywhere.

Signed-off-by: Andrea Bolognani 
---
 src/meson.build | 2 --
 src/remote/libvirtd-admin.socket.in | 1 -
 src/remote/libvirtd-ro.socket.in| 1 -
 src/remote/libvirtd-tcp.socket.in   | 1 -
 src/remote/libvirtd-tls.socket.in   | 1 -
 src/remote/libvirtd.socket.in   | 1 -
 6 files changed, 7 deletions(-)

diff --git a/src/meson.build b/src/meson.build
index 541ca61101..144f24e526 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -201,7 +201,6 @@ guest_unit_files = []
 #   * socket_$name_in - additional socket source files (optional, default 
virtd.socket.in or virtd-$name.socket.in)
 #   * service_extra_in - unit to merge with service_in (optional, default None)
 #   * socket_extra_in - unit to merge with socket_$name_in (optional, default 
None)
-#   * deps - socket dependencies (optional, default '')
 virt_daemon_units = []
 
 # openrc_init_files
@@ -817,7 +816,6 @@ if conf.has('WITH_LIBVIRTD')
 'service': unit['service'],
 'SERVICE': unit['service'].to_upper(),
 'sockprefix': unit.get('sockprefix', unit['service']),
-'deps': unit.get('deps', ''),
 'sockmode': sockmode,
   })
 
diff --git a/src/remote/libvirtd-admin.socket.in 
b/src/remote/libvirtd-admin.socket.in
index 01e1a08939..39bb0badea 100644
--- a/src/remote/libvirtd-admin.socket.in
+++ b/src/remote/libvirtd-admin.socket.in
@@ -3,7 +3,6 @@ Description=@name@ admin socket
 Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
-@deps@
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock
diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in
index 58ae1beb95..b7b7ae0dd8 100644
--- a/src/remote/libvirtd-ro.socket.in
+++ b/src/remote/libvirtd-ro.socket.in
@@ -3,7 +3,6 @@ Description=@name@ local read-only socket
 Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
-@deps@
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro
diff --git a/src/remote/libvirtd-tcp.socket.in 
b/src/remote/libvirtd-tcp.socket.in
index 6949df315e..7c8bcdb525 100644
--- a/src/remote/libvirtd-tcp.socket.in
+++ b/src/remote/libvirtd-tcp.socket.in
@@ -3,7 +3,6 @@ Description=@name@ non-TLS IP socket
 Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
-@deps@
 
 [Socket]
 ListenStream=16509
diff --git a/src/remote/libvirtd-tls.socket.in 
b/src/remote/libvirtd-tls.socket.in
index ada2b871f0..c6dceb2d4e 100644
--- a/src/remote/libvirtd-tls.socket.in
+++ b/src/remote/libvirtd-tls.socket.in
@@ -3,7 +3,6 @@ Description=@name@ TLS IP socket
 Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
-@deps@
 
 [Socket]
 ListenStream=16514
diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in
index e6e903a8ce..aec0708fd4 100644
--- a/src/remote/libvirtd.socket.in
+++ b/src/remote/libvirtd.socket.in
@@ -1,7 +1,6 @@
 [Unit]
 Description=@name@ local socket
 Before=@service@.service
-@deps@
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/@sockprefix@-sock
-- 
2.41.0



[libvirt PATCH v2 31/33] systemd: Add RemoveOnStop=yes to all sockets

2023-09-27 Thread Andrea Bolognani
Currently we only set this for the main sockets, which means
that

  $ systemctl stop virtqemud.socket

will make the socket disappear from the filesystem while

  $ systemctl stop virtqemud-ro.socket

won't. Get rid of this inconsistency.

Signed-off-by: Andrea Bolognani 
---
 src/locking/virtlockd-admin.socket.in | 1 +
 src/locking/virtlockd.socket.in   | 1 +
 src/logging/virtlogd-admin.socket.in  | 1 +
 src/logging/virtlogd.socket.in| 1 +
 src/remote/libvirtd-admin.socket.in   | 1 +
 src/remote/libvirtd-ro.socket.in  | 1 +
 src/virtd-admin.socket.in | 1 +
 src/virtd-ro.socket.in| 1 +
 8 files changed, 8 insertions(+)

diff --git a/src/locking/virtlockd-admin.socket.in 
b/src/locking/virtlockd-admin.socket.in
index d05ba982d9..0452a0cfdb 100644
--- a/src/locking/virtlockd-admin.socket.in
+++ b/src/locking/virtlockd-admin.socket.in
@@ -7,6 +7,7 @@ After=virtlockd.socket
 ListenStream=@runstatedir@/libvirt/virtlockd-admin-sock
 Service=virtlockd.service
 SocketMode=0600
+RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
diff --git a/src/locking/virtlockd.socket.in b/src/locking/virtlockd.socket.in
index 98aabb2511..31a576aa16 100644
--- a/src/locking/virtlockd.socket.in
+++ b/src/locking/virtlockd.socket.in
@@ -5,6 +5,7 @@ Description=Virtual machine lock manager socket
 ListenStream=@runstatedir@/libvirt/virtlockd-sock
 Service=virtlockd.service
 SocketMode=0600
+RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
diff --git a/src/logging/virtlogd-admin.socket.in 
b/src/logging/virtlogd-admin.socket.in
index 75ec7bd5fa..ddb9a1393b 100644
--- a/src/logging/virtlogd-admin.socket.in
+++ b/src/logging/virtlogd-admin.socket.in
@@ -7,6 +7,7 @@ After=virtlogd.socket
 ListenStream=@runstatedir@/libvirt/virtlogd-admin-sock
 Service=virtlogd.service
 SocketMode=0600
+RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
diff --git a/src/logging/virtlogd.socket.in b/src/logging/virtlogd.socket.in
index b044d62e7c..084cbe179d 100644
--- a/src/logging/virtlogd.socket.in
+++ b/src/logging/virtlogd.socket.in
@@ -5,6 +5,7 @@ Description=Virtual machine log manager socket
 ListenStream=@runstatedir@/libvirt/virtlogd-sock
 Service=virtlogd.service
 SocketMode=0600
+RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
diff --git a/src/remote/libvirtd-admin.socket.in 
b/src/remote/libvirtd-admin.socket.in
index 6df038d95a..e0bbf9b1ac 100644
--- a/src/remote/libvirtd-admin.socket.in
+++ b/src/remote/libvirtd-admin.socket.in
@@ -7,6 +7,7 @@ After=libvirtd.socket
 ListenStream=@runstatedir@/libvirt/libvirt-admin-sock
 Service=libvirtd.service
 SocketMode=0600
+RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in
index 6797517c50..c8adc8109b 100644
--- a/src/remote/libvirtd-ro.socket.in
+++ b/src/remote/libvirtd-ro.socket.in
@@ -7,6 +7,7 @@ After=libvirtd.socket
 ListenStream=@runstatedir@/libvirt/libvirt-sock-ro
 Service=libvirtd.service
 SocketMode=0666
+RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in
index 5a5f577041..818d4ab84f 100644
--- a/src/virtd-admin.socket.in
+++ b/src/virtd-admin.socket.in
@@ -9,6 +9,7 @@ After=libvirtd-admin.socket
 ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock
 Service=@service@.service
 SocketMode=0600
+RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in
index 692279665d..57b313e016 100644
--- a/src/virtd-ro.socket.in
+++ b/src/virtd-ro.socket.in
@@ -9,6 +9,7 @@ After=libvirtd-ro.socket
 ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro
 Service=@service@.service
 SocketMode=0666
+RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
-- 
2.41.0



[libvirt PATCH v2 28/33] systemd: Drop Before=libvirtd from virtlogd/virtlockd

2023-09-27 Thread Andrea Bolognani
We have already declared the mirror relationship, so this one
is now redundant.

Moreover, this version was incomplete: it only ever worked for
the monolithic daemon, but the modular daemons for QEMU and Xen
also want the sockets to be active.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Daniel P. Berrangé 
---
 src/locking/virtlockd-admin.socket.in | 1 -
 src/locking/virtlockd.service.in  | 1 -
 src/locking/virtlockd.socket.in   | 1 -
 src/logging/virtlogd-admin.socket.in  | 1 -
 src/logging/virtlogd.service.in   | 1 -
 src/logging/virtlogd.socket.in| 1 -
 6 files changed, 6 deletions(-)

diff --git a/src/locking/virtlockd-admin.socket.in 
b/src/locking/virtlockd-admin.socket.in
index c66e0f9693..d5ebd7f60b 100644
--- a/src/locking/virtlockd-admin.socket.in
+++ b/src/locking/virtlockd-admin.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=Virtual machine lock manager admin socket
-Before=libvirtd.service
 BindsTo=virtlockd.socket
 After=virtlockd.socket
 
diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in
index e0a7040ad3..20b4b26f35 100644
--- a/src/locking/virtlockd.service.in
+++ b/src/locking/virtlockd.service.in
@@ -4,7 +4,6 @@ BindsTo=virtlockd.socket
 Wants=virtlockd-admin.socket
 After=virtlockd.socket
 After=virtlockd-admin.socket
-Before=libvirtd.service
 Documentation=man:virtlockd(8)
 Documentation=https://libvirt.org
 
diff --git a/src/locking/virtlockd.socket.in b/src/locking/virtlockd.socket.in
index 4ce75391ae..d2cc2a06a3 100644
--- a/src/locking/virtlockd.socket.in
+++ b/src/locking/virtlockd.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=Virtual machine lock manager socket
-Before=libvirtd.service
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/virtlockd-sock
diff --git a/src/logging/virtlogd-admin.socket.in 
b/src/logging/virtlogd-admin.socket.in
index 5c0fb1880e..67259803ca 100644
--- a/src/logging/virtlogd-admin.socket.in
+++ b/src/logging/virtlogd-admin.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=Virtual machine log manager socket
-Before=libvirtd.service
 BindsTo=virtlogd.socket
 After=virtlogd.socket
 
diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in
index eab0d2c27c..776d753e9a 100644
--- a/src/logging/virtlogd.service.in
+++ b/src/logging/virtlogd.service.in
@@ -4,7 +4,6 @@ BindsTo=virtlogd.socket
 Wants=virtlogd-admin.socket
 After=virtlogd.socket
 After=virtlogd-admin.socket
-Before=libvirtd.service
 Documentation=man:virtlogd(8)
 Documentation=https://libvirt.org
 
diff --git a/src/logging/virtlogd.socket.in b/src/logging/virtlogd.socket.in
index ff3e66e09b..7b3fc73773 100644
--- a/src/logging/virtlogd.socket.in
+++ b/src/logging/virtlogd.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=Virtual machine log manager socket
-Before=libvirtd.service
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/virtlogd-sock
-- 
2.41.0



[libvirt PATCH v2 33/33] systemd: Move Documentation lines

2023-09-27 Thread Andrea Bolognani
Like the Description, these are intended to be displayed to the
user, so it makes sense to have them towards the top of the file
before all the information that systemd will parse to calculate
dependencies.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Daniel P. Berrangé 
---
 src/locking/virtlockd.service.in | 4 ++--
 src/logging/virtlogd.service.in  | 4 ++--
 src/remote/libvirtd.service.in   | 4 ++--
 src/virtd.service.in | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in
index 290a2887a5..ce00b6def9 100644
--- a/src/locking/virtlockd.service.in
+++ b/src/locking/virtlockd.service.in
@@ -1,11 +1,11 @@
 [Unit]
 Description=libvirt locking daemon
+Documentation=man:virtlockd(8)
+Documentation=https://libvirt.org/
 BindsTo=virtlockd.socket
 Wants=virtlockd-admin.socket
 After=virtlockd.socket
 After=virtlockd-admin.socket
-Documentation=man:virtlockd(8)
-Documentation=https://libvirt.org
 
 [Service]
 Type=notify
diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in
index 4289ef1cb4..52c9e5bb9e 100644
--- a/src/logging/virtlogd.service.in
+++ b/src/logging/virtlogd.service.in
@@ -1,11 +1,11 @@
 [Unit]
 Description=libvirt logging daemon
+Documentation=man:virtlogd(8)
+Documentation=https://libvirt.org/
 BindsTo=virtlogd.socket
 Wants=virtlogd-admin.socket
 After=virtlogd.socket
 After=virtlogd-admin.socket
-Documentation=man:virtlogd(8)
-Documentation=https://libvirt.org
 
 [Service]
 Type=notify
diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index 9e303f29c8..24a6712b75 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -1,5 +1,7 @@
 [Unit]
 Description=libvirt legacy monolithic daemon
+Documentation=man:libvirtd(8)
+Documentation=https://libvirt.org/
 # Use Wants instead of Requires so that users
 # can disable these three .socket units to revert
 # to a traditional non-activation deployment setup
@@ -22,8 +24,6 @@ After=remote-fs.target
 After=systemd-machined.service
 After=xencommons.service
 Conflicts=xendomains.service
-Documentation=man:libvirtd(8)
-Documentation=https://libvirt.org
 
 [Service]
 Type=notify
diff --git a/src/virtd.service.in b/src/virtd.service.in
index 91ac4478bd..651a8d82d7 100644
--- a/src/virtd.service.in
+++ b/src/virtd.service.in
@@ -1,5 +1,7 @@
 [Unit]
 Description=libvirt @name@ daemon
+Documentation=man:@service@(8)
+Documentation=https://libvirt.org/
 BindsTo=@service@.socket
 Wants=@service@-ro.socket
 Wants=@service@-admin.socket
@@ -11,8 +13,6 @@ After=libvirtd.service
 After=network.target
 After=dbus.service
 After=apparmor.service
-Documentation=man:@service@(8)
-Documentation=https://libvirt.org
 
 [Service]
 Type=notify
-- 
2.41.0



[libvirt PATCH v2 03/33] systemd: Make @service_in@ optional

2023-09-27 Thread Andrea Bolognani
It is currently considered required, but we're soon going to
provide a default that will be suitable for most services.

Since all services currently provide a value explicitly, we
can implement a default without breaking anything.

Signed-off-by: Andrea Bolognani 
---
 src/meson.build | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/meson.build b/src/meson.build
index c6728cc8f8..b7c2076c04 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -194,10 +194,10 @@ guest_unit_files = []
 # virt_daemon_units:
 #   generate libvirt daemon systemd unit files
 #   * service - name of the service (required)
-#   * service_in - service source file (required)
 #   * name - socket description (required)
 #   * sockprefix - socket prefix name (optional, default unit['service'])
 #   * sockets - array of additional sockets (optional, default [ 'main', 'ro', 
'admin' ])
+#   * service_in - service source file (optional, default 
remote/libvirtd.service.in)
 #   * socket_$name_in - additional socket source files (optional, default 
remote/libvirtd.socket.in )
 #   * deps - socket dependencies (optional, default '')
 virt_daemon_units = []
@@ -803,6 +803,8 @@ if conf.has('WITH_LIBVIRTD')
   sockmode = '0600'
 endif
 
+service_in_default = 'remote' / 'libvirtd.service.in'
+
 foreach unit : virt_daemon_units
   unit_conf = configuration_data({
 'runstatedir': runstatedir,
@@ -816,7 +818,7 @@ if conf.has('WITH_LIBVIRTD')
 'sockmode': sockmode,
   })
 
-  service_in = unit['service_in']
+  service_in = unit.get('service_in', service_in_default)
   service_out = '@0@.service'.format(unit['service'])
 
   configure_file(
-- 
2.41.0



[libvirt PATCH v2 11/33] systemd: Switch virtsecretd to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/secret/meson.build|  4 
 src/secret/virtsecretd.service.in | 25 -
 2 files changed, 29 deletions(-)
 delete mode 100644 src/secret/virtsecretd.service.in

diff --git a/src/secret/meson.build b/src/secret/meson.build
index 58e47c22e8..e05b46abea 100644
--- a/src/secret/meson.build
+++ b/src/secret/meson.build
@@ -33,11 +33,7 @@ if conf.has('WITH_SECRETS')
 
   virt_daemon_units += {
 'service': 'virtsecretd',
-'service_in': files('virtsecretd.service.in'),
 'name': 'Libvirt secret',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/secret/virtsecretd.service.in 
b/src/secret/virtsecretd.service.in
deleted file mode 100644
index 3804fe553b..00
--- a/src/secret/virtsecretd.service.in
+++ /dev/null
@@ -1,25 +0,0 @@
-[Unit]
-Description=Virtualization secret daemon
-Conflicts=libvirtd.service
-Requires=virtsecretd.socket
-Requires=virtsecretd-ro.socket
-Requires=virtsecretd-admin.socket
-After=network.target
-After=dbus.service
-After=apparmor.service
-Documentation=man:virtsecretd(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTSECRETD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtsecretd
-ExecStart=@sbindir@/virtsecretd $VIRTSECRETD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-
-[Install]
-WantedBy=multi-user.target
-Also=virtsecretd.socket
-Also=virtsecretd-ro.socket
-Also=virtsecretd-admin.socket
-- 
2.41.0



[libvirt PATCH v2 02/33] systemd: Introduce service_in/service_out variables

2023-09-27 Thread Andrea Bolognani
They're similar to the existing socket_in/socket_out variables
and will make future changes nicer.

Signed-off-by: Andrea Bolognani 
---
 src/meson.build | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/meson.build b/src/meson.build
index 6c85cc9b9b..c6728cc8f8 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -816,9 +816,12 @@ if conf.has('WITH_LIBVIRTD')
 'sockmode': sockmode,
   })
 
+  service_in = unit['service_in']
+  service_out = '@0@.service'.format(unit['service'])
+
   configure_file(
-input: unit['service_in'],
-output: '@0@.service'.format(unit['service']),
+input: service_in,
+output: service_out,
 configuration: unit_conf,
 install: true,
 install_dir: systemd_unit_dir,
-- 
2.41.0



[libvirt PATCH v2 05/33] systemd: Provide all input files explicitly

2023-09-27 Thread Andrea Bolognani
We're about to change the defaults and start migrating to common
templates: in order to be able to switch units over one at a
time, make the input files that are currently used explicit
rather than implicit.

Signed-off-by: Andrea Bolognani 
---
 src/ch/meson.build  |  3 +++
 src/interface/meson.build   |  3 +++
 src/libxl/meson.build   |  3 +++
 src/lxc/meson.build |  3 +++
 src/network/meson.build |  3 +++
 src/node_device/meson.build |  3 +++
 src/nwfilter/meson.build|  3 +++
 src/qemu/meson.build|  3 +++
 src/remote/meson.build  | 10 ++
 src/secret/meson.build  |  3 +++
 src/storage/meson.build |  3 +++
 src/vbox/meson.build|  3 +++
 src/vz/meson.build  |  3 +++
 13 files changed, 46 insertions(+)

diff --git a/src/ch/meson.build b/src/ch/meson.build
index 936b9bc95a..dc08069dcd 100644
--- a/src/ch/meson.build
+++ b/src/ch/meson.build
@@ -59,6 +59,9 @@ if conf.has('WITH_CH')
 'service': 'virtchd',
 'service_in': files('virtchd.service.in'),
 'name': 'Libvirt ch',
+'socket_in': libvirtd_socket_in,
+'socket_ro_in': libvirtd_socket_ro_in,
+'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   virt_install_dirs += [
diff --git a/src/interface/meson.build b/src/interface/meson.build
index 06c5241fa3..6fa65117c3 100644
--- a/src/interface/meson.build
+++ b/src/interface/meson.build
@@ -46,6 +46,9 @@ if conf.has('WITH_INTERFACE')
 'service': 'virtinterfaced',
 'service_in': files('virtinterfaced.service.in'),
 'name': 'Libvirt interface',
+'socket_in': libvirtd_socket_in,
+'socket_ro_in': libvirtd_socket_ro_in,
+'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/libxl/meson.build b/src/libxl/meson.build
index db8ccde38e..a1553dbe27 100644
--- a/src/libxl/meson.build
+++ b/src/libxl/meson.build
@@ -68,6 +68,9 @@ if conf.has('WITH_LIBXL')
 'service': 'virtxend',
 'service_in': files('virtxend.service.in'),
 'name': 'Libvirt libxl',
+'socket_in': libvirtd_socket_in,
+'socket_ro_in': libvirtd_socket_ro_in,
+'socket_admin_in': libvirtd_socket_admin_in,
 'deps': 'ConditionPathExists=/proc/xen/capabilities',
   }
 
diff --git a/src/lxc/meson.build b/src/lxc/meson.build
index a8773f64a5..531078448c 100644
--- a/src/lxc/meson.build
+++ b/src/lxc/meson.build
@@ -166,6 +166,9 @@ if conf.has('WITH_LXC')
 'service': 'virtlxcd',
 'service_in': files('virtlxcd.service.in'),
 'name': 'Libvirt lxc',
+'socket_in': libvirtd_socket_in,
+'socket_ro_in': libvirtd_socket_ro_in,
+'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/network/meson.build b/src/network/meson.build
index 40abfaef7e..2e51d5d47b 100644
--- a/src/network/meson.build
+++ b/src/network/meson.build
@@ -64,6 +64,9 @@ if conf.has('WITH_NETWORK')
 'service': 'virtnetworkd',
 'service_in': files('virtnetworkd.service.in'),
 'name': 'Libvirt network',
+'socket_in': libvirtd_socket_in,
+'socket_ro_in': libvirtd_socket_ro_in,
+'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/node_device/meson.build b/src/node_device/meson.build
index 47d9f63600..dd60b1f819 100644
--- a/src/node_device/meson.build
+++ b/src/node_device/meson.build
@@ -54,6 +54,9 @@ if conf.has('WITH_NODE_DEVICES')
 'service': 'virtnodedevd',
 'service_in': files('virtnodedevd.service.in'),
 'name': 'Libvirt nodedev',
+'socket_in': libvirtd_socket_in,
+'socket_ro_in': libvirtd_socket_ro_in,
+'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/nwfilter/meson.build b/src/nwfilter/meson.build
index 5efdee7189..de672bb827 100644
--- a/src/nwfilter/meson.build
+++ b/src/nwfilter/meson.build
@@ -52,6 +52,9 @@ if conf.has('WITH_NWFILTER')
 'service': 'virtnwfilterd',
 'service_in': files('virtnwfilterd.service.in'),
 'name': 'Libvirt nwfilter',
+'socket_in': libvirtd_socket_in,
+'socket_ro_in': libvirtd_socket_ro_in,
+'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index afa9139d9a..b52497bdf0 100644
--- a/src/qemu/meson.build
+++ b/src/qemu/meson.build
@@ -185,6 +185,9 @@ if conf.has('WITH_QEMU')
 'service': 'virtqemud',
 'service_in': files('virtqemud.service.in'),
 'name': 'Libvirt qemu',
+'socket_in': libvirtd_socket_in,
+'socket_ro_in': libvirtd_socket_ro_in,
+'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/remote/meson.build b/src/remote/meson.build
index dc2f528d0b..78c08bf5ad 100644
--- a/src/remote/meson.build
+++ b/src/remote/meson.build
@@ -194,6 +194,11 @@ if conf.has('WITH_REMOTE')
   'name': 'Libvirt',
   'sockprefix': 'libvirt',
   'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
+  'socket_in': 

[libvirt PATCH v2 12/33] systemd: Switch virtnetworkd to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/network/meson.build   |  5 +
 src/network/virtnetworkd.service.extra.in |  2 ++
 src/network/virtnetworkd.service.in   | 26 ---
 3 files changed, 3 insertions(+), 30 deletions(-)
 create mode 100644 src/network/virtnetworkd.service.extra.in
 delete mode 100644 src/network/virtnetworkd.service.in

diff --git a/src/network/meson.build b/src/network/meson.build
index 2e51d5d47b..ed7707c714 100644
--- a/src/network/meson.build
+++ b/src/network/meson.build
@@ -62,11 +62,8 @@ if conf.has('WITH_NETWORK')
 
   virt_daemon_units += {
 'service': 'virtnetworkd',
-'service_in': files('virtnetworkd.service.in'),
 'name': 'Libvirt network',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
+'service_extra_in': files('virtnetworkd.service.extra.in'),
   }
 
   openrc_init_files += {
diff --git a/src/network/virtnetworkd.service.extra.in 
b/src/network/virtnetworkd.service.extra.in
new file mode 100644
index 00..9fcabf652d
--- /dev/null
+++ b/src/network/virtnetworkd.service.extra.in
@@ -0,0 +1,2 @@
+[Service]
+KillMode=process
diff --git a/src/network/virtnetworkd.service.in 
b/src/network/virtnetworkd.service.in
deleted file mode 100644
index 3d7374715d..00
--- a/src/network/virtnetworkd.service.in
+++ /dev/null
@@ -1,26 +0,0 @@
-[Unit]
-Description=Virtualization network daemon
-Conflicts=libvirtd.service
-Requires=virtnetworkd.socket
-Requires=virtnetworkd-ro.socket
-Requires=virtnetworkd-admin.socket
-After=network.target
-After=dbus.service
-After=apparmor.service
-Documentation=man:virtnetworkd(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTNETWORKD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtnetworkd
-ExecStart=@sbindir@/virtnetworkd $VIRTNETWORKD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-KillMode=process
-
-[Install]
-WantedBy=multi-user.target
-Also=virtnetworkd.socket
-Also=virtnetworkd-ro.socket
-Also=virtnetworkd-admin.socket
-- 
2.41.0



[libvirt PATCH v2 13/33] systemd: Switch virtstoraged to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/storage/meson.build   |  5 +
 src/storage/virtstoraged.service.extra.in |  3 +++
 src/storage/virtstoraged.service.in   | 27 ---
 3 files changed, 4 insertions(+), 31 deletions(-)
 create mode 100644 src/storage/virtstoraged.service.extra.in
 delete mode 100644 src/storage/virtstoraged.service.in

diff --git a/src/storage/meson.build b/src/storage/meson.build
index e0a1e9f4de..fb7feea81d 100644
--- a/src/storage/meson.build
+++ b/src/storage/meson.build
@@ -111,11 +111,8 @@ if conf.has('WITH_STORAGE')
 
   virt_daemon_units += {
 'service': 'virtstoraged',
-'service_in': files('virtstoraged.service.in'),
 'name': 'Libvirt storage',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
+'service_extra_in': files('virtstoraged.service.extra.in'),
   }
 
   openrc_init_files += {
diff --git a/src/storage/virtstoraged.service.extra.in 
b/src/storage/virtstoraged.service.extra.in
new file mode 100644
index 00..d134ae18da
--- /dev/null
+++ b/src/storage/virtstoraged.service.extra.in
@@ -0,0 +1,3 @@
+[Unit]
+After=iscsid.service
+After=remote-fs.target
diff --git a/src/storage/virtstoraged.service.in 
b/src/storage/virtstoraged.service.in
deleted file mode 100644
index 235fbc6798..00
--- a/src/storage/virtstoraged.service.in
+++ /dev/null
@@ -1,27 +0,0 @@
-[Unit]
-Description=Virtualization storage daemon
-Conflicts=libvirtd.service
-Requires=virtstoraged.socket
-Requires=virtstoraged-ro.socket
-Requires=virtstoraged-admin.socket
-After=network.target
-After=dbus.service
-After=iscsid.service
-After=apparmor.service
-After=remote-fs.target
-Documentation=man:virtstoraged(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTSTORAGED_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtstoraged
-ExecStart=@sbindir@/virtstoraged $VIRTSTORAGED_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-
-[Install]
-WantedBy=multi-user.target
-Also=virtstoraged.socket
-Also=virtstoraged-ro.socket
-Also=virtstoraged-admin.socket
-- 
2.41.0



[libvirt PATCH v2 04/33] systemd: Introduce temporary libvirtd_socket*_in values

2023-09-27 Thread Andrea Bolognani
These will be useful during the upcoming migration to common
templates for systemd units and will be dropped as soon as all
services have been converted.

Signed-off-by: Andrea Bolognani 
---
 src/meson.build | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/meson.build b/src/meson.build
index b7c2076c04..2fbf98b9fe 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -191,6 +191,10 @@ virt_test_aug_dir = datadir / 'augeas' / 'lenses' / 'tests'
 #   guest unit files to install
 guest_unit_files = []
 
+libvirtd_socket_in = files('remote' / 'libvirtd.socket.in')
+libvirtd_socket_ro_in = files('remote' / 'libvirtd-ro.socket.in')
+libvirtd_socket_admin_in = files('remote' / 'libvirtd-admin.socket.in')
+
 # virt_daemon_units:
 #   generate libvirt daemon systemd unit files
 #   * service - name of the service (required)
-- 
2.41.0



[libvirt PATCH v2 06/33] systemd: Introduce common templates

2023-09-27 Thread Andrea Bolognani
We already use templating to generate sockets, which are all
based off libvirtd's. Push the idea further, and extend it to
cover services as well.

This is more challenging, as the various modular daemons each have
their own needs in terms of what system services needs to be
available before they can be started, which other components of
libvirt they depend on, and so on.

In order to make this sort of per-service tweaks possible, we
introduce a Python script that can merge two systemd units
together. The script is aware of the semantics of systemd's unit
definition format, so it can intelligently merge sections
together.

This generic systemd unit merging mechanism will also supersede
the extremely ad-hoc @deps@ variable, which is currently used in
a single scenario.

Signed-off-by: Andrea Bolognani 
---
 scripts/merge-systemd-units.py | 91 ++
 scripts/meson.build|  1 +
 src/meson.build| 22 
 src/virtd-admin.socket.in  | 13 +
 src/virtd-ro.socket.in | 13 +
 src/virtd-tcp.socket.in| 12 +
 src/virtd-tls.socket.in| 12 +
 src/virtd.service.in   | 25 ++
 src/virtd.socket.in| 12 +
 9 files changed, 201 insertions(+)
 create mode 100755 scripts/merge-systemd-units.py
 create mode 100644 src/virtd-admin.socket.in
 create mode 100644 src/virtd-ro.socket.in
 create mode 100644 src/virtd-tcp.socket.in
 create mode 100644 src/virtd-tls.socket.in
 create mode 100644 src/virtd.service.in
 create mode 100644 src/virtd.socket.in

diff --git a/scripts/merge-systemd-units.py b/scripts/merge-systemd-units.py
new file mode 100755
index 00..136bc8d416
--- /dev/null
+++ b/scripts/merge-systemd-units.py
@@ -0,0 +1,91 @@
+#!/usr/bin/env python3
+
+import sys
+
+SECTIONS = [
+'[Unit]',
+'[Service]',
+'[Socket]',
+'[Install]',
+]
+
+
+def parse_unit(unit_path):
+unit = {}
+current_section = '[Invalid]'
+
+with open(unit_path) as f:
+for line in f:
+line = line.strip()
+
+if line == '':
+continue
+
+if line[0] == '[' and line[-1] == ']':
+if line not in SECTIONS:
+print('Unknown section {}'.format(line))
+sys.exit(1)
+
+current_section = line
+continue
+
+if current_section not in unit:
+unit[current_section] = []
+
+unit[current_section].append(line)
+
+if '[Invalid]' in unit:
+print('Contents found outside of any section')
+sys.exit(1)
+
+return unit
+
+
+def format_unit(unit):
+lines = []
+
+for section in SECTIONS:
+if section not in unit:
+continue
+
+lines.append(section)
+
+for line in unit[section]:
+lines.append(line)
+
+lines.append('')
+
+return '\n'.join(lines)
+
+
+def merge_units(base, extra):
+merged = {}
+
+for section in SECTIONS:
+if section in extra and section not in base:
+print('Section {} in extra but not in base'.format(section))
+sys.exit(1)
+
+if section not in base:
+continue
+
+merged[section] = base[section]
+
+if section not in extra:
+continue
+
+merged[section].extend(extra[section])
+
+return merged
+
+
+if len(sys.argv) < 2:
+print('usage: {} BASE EXTRA'.format(sys.argv[0]))
+sys.exit(1)
+
+base = parse_unit(sys.argv[1])
+extra = parse_unit(sys.argv[2])
+
+merged = merge_units(base, extra)
+
+sys.stdout.write(format_unit(merged))
diff --git a/scripts/meson.build b/scripts/meson.build
index 05b71184f1..65fd1e21c5 100644
--- a/scripts/meson.build
+++ b/scripts/meson.build
@@ -19,6 +19,7 @@ scripts = [
   'header-ifdef.py',
   'hvsupport.py',
   'hyperv_wmi_generator.py',
+  'merge-systemd-units.py',
   'meson-dist.py',
   'meson-gen-authors.py',
   'meson-gen-def.py',
diff --git a/src/meson.build b/src/meson.build
index 2fbf98b9fe..02c92621ba 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -203,6 +203,8 @@ libvirtd_socket_admin_in = files('remote' / 
'libvirtd-admin.socket.in')
 #   * sockets - array of additional sockets (optional, default [ 'main', 'ro', 
'admin' ])
 #   * service_in - service source file (optional, default 
remote/libvirtd.service.in)
 #   * socket_$name_in - additional socket source files (optional, default 
remote/libvirtd.socket.in )
+#   * service_extra_in - unit to merge with service_in (optional, default None)
+#   * socket_extra_in - unit to merge with socket_$name_in (optional, default 
None)
 #   * deps - socket dependencies (optional, default '')
 virt_daemon_units = []
 
@@ -817,6 +819,7 @@ if conf.has('WITH_LIBVIRTD')
 'initconfdir': initconfdir,
 'name': unit['name'],
 'service': unit['service'],
+'SERVICE': unit['service'].to_upper(),
 'sockprefix': 

[libvirt PATCH v2 14/33] systemd: Switch virtvboxd to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/vbox/meson.build|  5 +
 src/vbox/virtvboxd.service.extra.in |  2 ++
 src/vbox/virtvboxd.service.in   | 26 --
 3 files changed, 3 insertions(+), 30 deletions(-)
 create mode 100644 src/vbox/virtvboxd.service.extra.in
 delete mode 100644 src/vbox/virtvboxd.service.in

diff --git a/src/vbox/meson.build b/src/vbox/meson.build
index 2d6b71ab8f..ee6efbdb42 100644
--- a/src/vbox/meson.build
+++ b/src/vbox/meson.build
@@ -57,11 +57,8 @@ if conf.has('WITH_VBOX')
 
   virt_daemon_units += {
 'service': 'virtvboxd',
-'service_in': files('virtvboxd.service.in'),
 'name': 'Libvirt vbox',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
+'service_extra_in': files('virtvboxd.service.extra.in'),
   }
 
   openrc_init_files += {
diff --git a/src/vbox/virtvboxd.service.extra.in 
b/src/vbox/virtvboxd.service.extra.in
new file mode 100644
index 00..ba3ad13ace
--- /dev/null
+++ b/src/vbox/virtvboxd.service.extra.in
@@ -0,0 +1,2 @@
+[Unit]
+After=remote-fs.target
diff --git a/src/vbox/virtvboxd.service.in b/src/vbox/virtvboxd.service.in
deleted file mode 100644
index a567ed2443..00
--- a/src/vbox/virtvboxd.service.in
+++ /dev/null
@@ -1,26 +0,0 @@
-[Unit]
-Description=Virtualization vbox daemon
-Conflicts=libvirtd.service
-Requires=virtvboxd.socket
-Requires=virtvboxd-ro.socket
-Requires=virtvboxd-admin.socket
-After=network.target
-After=dbus.service
-After=apparmor.service
-After=remote-fs.target
-Documentation=man:virtvboxd(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTVBOXD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtvboxd
-ExecStart=@sbindir@/virtvboxd $VIRTVBOXD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-
-[Install]
-WantedBy=multi-user.target
-Also=virtvboxd.socket
-Also=virtvboxd-ro.socket
-Also=virtvboxd-admin.socket
-- 
2.41.0



[libvirt PATCH v2 07/33] systemd: Use common templates by default

2023-09-27 Thread Andrea Bolognani
All services are still listing their input files explicitly, so
no changes to the output files will occur yet.

Signed-off-by: Andrea Bolognani 
---
 src/meson.build | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/meson.build b/src/meson.build
index 02c92621ba..0fbefe37d5 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -201,8 +201,8 @@ libvirtd_socket_admin_in = files('remote' / 
'libvirtd-admin.socket.in')
 #   * name - socket description (required)
 #   * sockprefix - socket prefix name (optional, default unit['service'])
 #   * sockets - array of additional sockets (optional, default [ 'main', 'ro', 
'admin' ])
-#   * service_in - service source file (optional, default 
remote/libvirtd.service.in)
-#   * socket_$name_in - additional socket source files (optional, default 
remote/libvirtd.socket.in )
+#   * service_in - service source file (optional, default virtd.service.in)
+#   * socket_$name_in - additional socket source files (optional, default 
virtd.socket.in or virtd-$name.socket.in)
 #   * service_extra_in - unit to merge with service_in (optional, default None)
 #   * socket_extra_in - unit to merge with socket_$name_in (optional, default 
None)
 #   * deps - socket dependencies (optional, default '')
@@ -809,7 +809,7 @@ if conf.has('WITH_LIBVIRTD')
   sockmode = '0600'
 endif
 
-service_in_default = 'remote' / 'libvirtd.service.in'
+service_in_default = 'virtd.service.in'
 
 foreach unit : virt_daemon_units
   unit_conf = configuration_data({
@@ -847,11 +847,11 @@ if conf.has('WITH_LIBVIRTD')
 
   foreach socket : unit.get('sockets', [ 'main', 'ro', 'admin' ])
 if socket == 'main'
-  socket_in_default = 'remote' / 'libvirtd.socket.in'
+  socket_in_default = 'virtd.socket.in'
   socket_in = unit.get('socket_in', socket_in_default)
   socket_out = '@0@.socket'.format(unit['service'])
 else
-  socket_in_default = 'remote' / 
'libvirtd-@0...@.socket.in'.format(socket)
+  socket_in_default = 'virtd-@0...@.socket.in'.format(socket)
   socket_in = unit.get('socket_@0@_in'.format(socket), 
socket_in_default)
   socket_out = '@0@-@1@.socket'.format(unit['service'], socket)
 endif
-- 
2.41.0



[libvirt PATCH v2 27/33] systemd: Augment Requires/Wants with After

2023-09-27 Thread Andrea Bolognani
Requires/Wants only tells systemd that the corresponding unit
should be started when the current one is, but that could very
well happen in parallel. For virtlogd/virtlockd, we want the
socket to be already active when the hypervisor driver is
started.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Daniel P. Berrangé 
---
 src/libxl/virtxend.service.extra.in | 1 +
 src/locking/virtlockd.service.in| 1 +
 src/logging/virtlogd.service.in | 1 +
 src/qemu/virtqemud.service.extra.in | 2 ++
 src/remote/libvirtd.service.in  | 7 ++-
 src/virtd.service.in| 2 ++
 6 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/libxl/virtxend.service.extra.in 
b/src/libxl/virtxend.service.extra.in
index ba38ba9160..55783aa3d5 100644
--- a/src/libxl/virtxend.service.extra.in
+++ b/src/libxl/virtxend.service.extra.in
@@ -1,5 +1,6 @@
 [Unit]
 Wants=virtlockd.socket
+After=virtlockd.socket
 After=remote-fs.target
 After=xencommons.service
 Conflicts=xendomains.service
diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in
index fcf479c3c6..e0a7040ad3 100644
--- a/src/locking/virtlockd.service.in
+++ b/src/locking/virtlockd.service.in
@@ -3,6 +3,7 @@ Description=Virtual machine lock manager
 BindsTo=virtlockd.socket
 Wants=virtlockd-admin.socket
 After=virtlockd.socket
+After=virtlockd-admin.socket
 Before=libvirtd.service
 Documentation=man:virtlockd(8)
 Documentation=https://libvirt.org
diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in
index 3265ecd6af..eab0d2c27c 100644
--- a/src/logging/virtlogd.service.in
+++ b/src/logging/virtlogd.service.in
@@ -3,6 +3,7 @@ Description=Virtual machine log manager
 BindsTo=virtlogd.socket
 Wants=virtlogd-admin.socket
 After=virtlogd.socket
+After=virtlogd-admin.socket
 Before=libvirtd.service
 Documentation=man:virtlogd(8)
 Documentation=https://libvirt.org
diff --git a/src/qemu/virtqemud.service.extra.in 
b/src/qemu/virtqemud.service.extra.in
index eaf616f575..585e1e82eb 100644
--- a/src/qemu/virtqemud.service.extra.in
+++ b/src/qemu/virtqemud.service.extra.in
@@ -1,6 +1,8 @@
 [Unit]
 Requires=virtlogd.socket
 Wants=virtlockd.socket
+After=virtlogd.socket
+After=virtlockd.socket
 Wants=systemd-machined.service
 After=systemd-machined.service
 After=remote-fs.target
diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index 8839c00a15..a2c3c8f8fa 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -1,13 +1,18 @@
 [Unit]
 Description=Virtualization daemon
-Requires=virtlogd.socket
 # Use Wants instead of Requires so that users
 # can disable these three .socket units to revert
 # to a traditional non-activation deployment setup
 Wants=libvirtd.socket
 Wants=libvirtd-ro.socket
 Wants=libvirtd-admin.socket
+After=libvirtd.socket
+After=libvirtd-ro.socket
+After=libvirtd-admin.socket
+Requires=virtlogd.socket
 Wants=virtlockd.socket
+After=virtlogd.socket
+After=virtlockd.socket
 Wants=systemd-machined.service
 After=network.target
 After=dbus.service
diff --git a/src/virtd.service.in b/src/virtd.service.in
index f4f1bc217d..e1a5814b13 100644
--- a/src/virtd.service.in
+++ b/src/virtd.service.in
@@ -4,6 +4,8 @@ BindsTo=@service@.socket
 Wants=@service@-ro.socket
 Wants=@service@-admin.socket
 After=@service@.socket
+After=@service@-ro.socket
+After=@service@-admin.socket
 Conflicts=libvirtd.service
 After=libvirtd.service
 After=network.target
-- 
2.41.0



[libvirt PATCH v2 29/33] systemd: Drop Before=foo.service from sockets

2023-09-27 Thread Andrea Bolognani
systemd will automatically infer this dependency based on the
socket's Service=foo.service setting.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Daniel P. Berrangé 
---
 src/remote/libvirtd-admin.socket.in | 1 -
 src/remote/libvirtd-ro.socket.in| 1 -
 src/remote/libvirtd-tcp.socket.in   | 1 -
 src/remote/libvirtd-tls.socket.in   | 1 -
 src/remote/libvirtd.socket.in   | 1 -
 src/virtd-admin.socket.in   | 1 -
 src/virtd-ro.socket.in  | 1 -
 src/virtd-tcp.socket.in | 1 -
 src/virtd-tls.socket.in | 1 -
 src/virtd.socket.in | 1 -
 10 files changed, 10 deletions(-)

diff --git a/src/remote/libvirtd-admin.socket.in 
b/src/remote/libvirtd-admin.socket.in
index 8d927db63b..098e372971 100644
--- a/src/remote/libvirtd-admin.socket.in
+++ b/src/remote/libvirtd-admin.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ admin socket
-Before=libvirtd.service
 BindsTo=libvirtd.socket
 After=libvirtd.socket
 
diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in
index cc10190ab4..101555e8a0 100644
--- a/src/remote/libvirtd-ro.socket.in
+++ b/src/remote/libvirtd-ro.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ local read-only socket
-Before=libvirtd.service
 BindsTo=libvirtd.socket
 After=libvirtd.socket
 
diff --git a/src/remote/libvirtd-tcp.socket.in 
b/src/remote/libvirtd-tcp.socket.in
index bc35f19c06..8b8fbcd01a 100644
--- a/src/remote/libvirtd-tcp.socket.in
+++ b/src/remote/libvirtd-tcp.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ non-TLS IP socket
-Before=libvirtd.service
 BindsTo=libvirtd.socket
 After=libvirtd.socket
 
diff --git a/src/remote/libvirtd-tls.socket.in 
b/src/remote/libvirtd-tls.socket.in
index 868a0be318..fefda22c6b 100644
--- a/src/remote/libvirtd-tls.socket.in
+++ b/src/remote/libvirtd-tls.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ TLS IP socket
-Before=libvirtd.service
 BindsTo=libvirtd.socket
 After=libvirtd.socket
 
diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in
index ea0554546a..3019821df3 100644
--- a/src/remote/libvirtd.socket.in
+++ b/src/remote/libvirtd.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ local socket
-Before=libvirtd.service
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/libvirt-sock
diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in
index 42cc1f670f..63db2be5fe 100644
--- a/src/virtd-admin.socket.in
+++ b/src/virtd-admin.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ admin socket
-Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
 Conflicts=libvirtd-admin.socket
diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in
index 7b8cbdba20..32e4789b8b 100644
--- a/src/virtd-ro.socket.in
+++ b/src/virtd-ro.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ local read-only socket
-Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
 Conflicts=libvirtd-ro.socket
diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in
index 9fe90ed0a0..10480d64e3 100644
--- a/src/virtd-tcp.socket.in
+++ b/src/virtd-tcp.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ non-TLS IP socket
-Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
 Conflicts=libvirtd-tcp.socket
diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in
index bb89daddb5..83a1e343bc 100644
--- a/src/virtd-tls.socket.in
+++ b/src/virtd-tls.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ TLS IP socket
-Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
 Conflicts=libvirtd-tls.socket
diff --git a/src/virtd.socket.in b/src/virtd.socket.in
index 053dc1c782..d0a0bb3b1c 100644
--- a/src/virtd.socket.in
+++ b/src/virtd.socket.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=@name@ local socket
-Before=@service@.service
 Conflicts=libvirtd.socket
 After=libvirtd.socket
 
-- 
2.41.0



[libvirt PATCH v2 25/33] systemd: Replace Requires with BindTo+After for main socket

2023-09-27 Thread Andrea Bolognani
This is the strongest relationship that can be declared between
two units, and causes the service to be terminated immediately
if its main socket disappears. This is the behavior we want.

Note that we don't do the same for the read-only/admin sockets,
because those are not as critical for the core functionality of
services as the main socket it.

Signed-off-by: Andrea Bolognani 
---
 src/locking/virtlockd.service.in | 3 ++-
 src/logging/virtlogd.service.in  | 3 ++-
 src/virtd.service.in | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in
index 9e91fa3261..35924a2ad7 100644
--- a/src/locking/virtlockd.service.in
+++ b/src/locking/virtlockd.service.in
@@ -1,7 +1,8 @@
 [Unit]
 Description=Virtual machine lock manager
-Requires=virtlockd.socket
+BindsTo=virtlockd.socket
 Requires=virtlockd-admin.socket
+After=virtlockd.socket
 Before=libvirtd.service
 Documentation=man:virtlockd(8)
 Documentation=https://libvirt.org
diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in
index 97c942ffb0..79d34bc73e 100644
--- a/src/logging/virtlogd.service.in
+++ b/src/logging/virtlogd.service.in
@@ -1,7 +1,8 @@
 [Unit]
 Description=Virtual machine log manager
-Requires=virtlogd.socket
+BindsTo=virtlogd.socket
 Requires=virtlogd-admin.socket
+After=virtlogd.socket
 Before=libvirtd.service
 Documentation=man:virtlogd(8)
 Documentation=https://libvirt.org
diff --git a/src/virtd.service.in b/src/virtd.service.in
index 60ab122cbc..e7f08b4da9 100644
--- a/src/virtd.service.in
+++ b/src/virtd.service.in
@@ -1,8 +1,9 @@
 [Unit]
 Description=@name@ daemon
-Requires=@service@.socket
+BindsTo=@service@.socket
 Requires=@service@-ro.socket
 Requires=@service@-admin.socket
+After=@service@.socket
 Conflicts=libvirtd.service
 After=libvirtd.service
 After=network.target
-- 
2.41.0



[libvirt PATCH v2 18/33] systemd: Switch virtlxcd to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/lxc/meson.build   |  5 +---
 src/lxc/virtlxcd.service.extra.in | 22 
 src/lxc/virtlxcd.service.in   | 44 ---
 3 files changed, 23 insertions(+), 48 deletions(-)
 create mode 100644 src/lxc/virtlxcd.service.extra.in
 delete mode 100644 src/lxc/virtlxcd.service.in

diff --git a/src/lxc/meson.build b/src/lxc/meson.build
index 531078448c..84e6c313ea 100644
--- a/src/lxc/meson.build
+++ b/src/lxc/meson.build
@@ -164,11 +164,8 @@ if conf.has('WITH_LXC')
 
   virt_daemon_units += {
 'service': 'virtlxcd',
-'service_in': files('virtlxcd.service.in'),
 'name': 'Libvirt lxc',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
+'service_extra_in': files('virtlxcd.service.extra.in'),
   }
 
   openrc_init_files += {
diff --git a/src/lxc/virtlxcd.service.extra.in 
b/src/lxc/virtlxcd.service.extra.in
new file mode 100644
index 00..bc2fef57cc
--- /dev/null
+++ b/src/lxc/virtlxcd.service.extra.in
@@ -0,0 +1,22 @@
+[Unit]
+Wants=systemd-machined.service
+After=systemd-machined.service
+After=remote-fs.target
+
+[Service]
+KillMode=process
+# Raise hard limits to match behaviour of systemd >= 240.
+# During startup, daemon will set soft limit to match hard limit
+# per systemd recommendations
+LimitNOFILE=1024:524288
+# The cgroups pids controller can limit the number of tasks started by
+# the daemon, which can limit the number of domains for some hypervisors.
+# A conservative default of 8 tasks per guest results in a TasksMax of
+# 32k to support 4096 guests.
+TasksMax=32768
+# With cgroups v2 there is no devices controller anymore, we have to use
+# eBPF to control access to devices. In order to do that we create a eBPF
+# hash MAP which locks memory. The default map size for 64 devices together
+# with program takes 12k per guest. After rounding up we will get 64M to
+# support 4096 guests.
+LimitMEMLOCK=64M
diff --git a/src/lxc/virtlxcd.service.in b/src/lxc/virtlxcd.service.in
deleted file mode 100644
index ee3a7f1083..00
--- a/src/lxc/virtlxcd.service.in
+++ /dev/null
@@ -1,44 +0,0 @@
-[Unit]
-Description=Virtualization lxc daemon
-Conflicts=libvirtd.service
-Requires=virtlxcd.socket
-Requires=virtlxcd-ro.socket
-Requires=virtlxcd-admin.socket
-Wants=systemd-machined.service
-After=network.target
-After=dbus.service
-After=apparmor.service
-After=remote-fs.target
-After=systemd-machined.service
-Documentation=man:virtlxcd(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTLXCD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtlxcd
-ExecStart=@sbindir@/virtlxcd $VIRTLXCD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-KillMode=process
-Restart=on-failure
-# Raise hard limits to match behaviour of systemd >= 240.
-# During startup, daemon will set soft limit to match hard limit
-# per systemd recommendations
-LimitNOFILE=1024:524288
-# The cgroups pids controller can limit the number of tasks started by
-# the daemon, which can limit the number of domains for some hypervisors.
-# A conservative default of 8 tasks per guest results in a TasksMax of
-# 32k to support 4096 guests.
-TasksMax=32768
-# With cgroups v2 there is no devices controller anymore, we have to use
-# eBPF to control access to devices.  In order to do that we create a eBPF
-# hash MAP which locks memory.  The default map size for 64 devices together
-# with program takes 12k per guest.  After rounding up we will get 64M to
-# support 4096 guests.
-LimitMEMLOCK=64M
-
-[Install]
-WantedBy=multi-user.target
-Also=virtlxcd.socket
-Also=virtlxcd-ro.socket
-Also=virtlxcd-admin.socket
-- 
2.41.0



[libvirt PATCH v2 21/33] systemd: Drop libvirtd_socket*_in values

2023-09-27 Thread Andrea Bolognani
Now that the migration to common templates has been completed,
we no longer need these.

Signed-off-by: Andrea Bolognani 
---
 src/meson.build | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/meson.build b/src/meson.build
index 0fbefe37d5..541ca61101 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -191,10 +191,6 @@ virt_test_aug_dir = datadir / 'augeas' / 'lenses' / 'tests'
 #   guest unit files to install
 guest_unit_files = []
 
-libvirtd_socket_in = files('remote' / 'libvirtd.socket.in')
-libvirtd_socket_ro_in = files('remote' / 'libvirtd-ro.socket.in')
-libvirtd_socket_admin_in = files('remote' / 'libvirtd-admin.socket.in')
-
 # virt_daemon_units:
 #   generate libvirt daemon systemd unit files
 #   * service - name of the service (required)
-- 
2.41.0



[libvirt PATCH v2 23/33] systemd: Drop parametrization from libvirtd sockets

2023-09-27 Thread Andrea Bolognani
Up until now the files have been used as template for most
services, but now that those have been converted to common
templates we can drop parametrization and make it clear that
these files are for libvirtd only.

Signed-off-by: Andrea Bolognani 
---
 src/remote/libvirtd-admin.socket.in | 10 +-
 src/remote/libvirtd-ro.socket.in| 10 +-
 src/remote/libvirtd-tcp.socket.in   |  8 
 src/remote/libvirtd-tls.socket.in   |  8 
 src/remote/libvirtd.socket.in   |  6 +++---
 5 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/remote/libvirtd-admin.socket.in 
b/src/remote/libvirtd-admin.socket.in
index 39bb0badea..8d927db63b 100644
--- a/src/remote/libvirtd-admin.socket.in
+++ b/src/remote/libvirtd-admin.socket.in
@@ -1,12 +1,12 @@
 [Unit]
 Description=@name@ admin socket
-Before=@service@.service
-BindsTo=@service@.socket
-After=@service@.socket
+Before=libvirtd.service
+BindsTo=libvirtd.socket
+After=libvirtd.socket
 
 [Socket]
-ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock
-Service=@service@.service
+ListenStream=@runstatedir@/libvirt/libvirt-admin-sock
+Service=libvirtd.service
 SocketMode=0600
 
 [Install]
diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in
index b7b7ae0dd8..cc10190ab4 100644
--- a/src/remote/libvirtd-ro.socket.in
+++ b/src/remote/libvirtd-ro.socket.in
@@ -1,12 +1,12 @@
 [Unit]
 Description=@name@ local read-only socket
-Before=@service@.service
-BindsTo=@service@.socket
-After=@service@.socket
+Before=libvirtd.service
+BindsTo=libvirtd.socket
+After=libvirtd.socket
 
 [Socket]
-ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro
-Service=@service@.service
+ListenStream=@runstatedir@/libvirt/libvirt-sock-ro
+Service=libvirtd.service
 SocketMode=0666
 
 [Install]
diff --git a/src/remote/libvirtd-tcp.socket.in 
b/src/remote/libvirtd-tcp.socket.in
index 7c8bcdb525..bc35f19c06 100644
--- a/src/remote/libvirtd-tcp.socket.in
+++ b/src/remote/libvirtd-tcp.socket.in
@@ -1,12 +1,12 @@
 [Unit]
 Description=@name@ non-TLS IP socket
-Before=@service@.service
-BindsTo=@service@.socket
-After=@service@.socket
+Before=libvirtd.service
+BindsTo=libvirtd.socket
+After=libvirtd.socket
 
 [Socket]
 ListenStream=16509
-Service=@service@.service
+Service=libvirtd.service
 
 [Install]
 WantedBy=sockets.target
diff --git a/src/remote/libvirtd-tls.socket.in 
b/src/remote/libvirtd-tls.socket.in
index c6dceb2d4e..868a0be318 100644
--- a/src/remote/libvirtd-tls.socket.in
+++ b/src/remote/libvirtd-tls.socket.in
@@ -1,12 +1,12 @@
 [Unit]
 Description=@name@ TLS IP socket
-Before=@service@.service
-BindsTo=@service@.socket
-After=@service@.socket
+Before=libvirtd.service
+BindsTo=libvirtd.socket
+After=libvirtd.socket
 
 [Socket]
 ListenStream=16514
-Service=@service@.service
+Service=libvirtd.service
 
 [Install]
 WantedBy=sockets.target
diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in
index aec0708fd4..ea0554546a 100644
--- a/src/remote/libvirtd.socket.in
+++ b/src/remote/libvirtd.socket.in
@@ -1,10 +1,10 @@
 [Unit]
 Description=@name@ local socket
-Before=@service@.service
+Before=libvirtd.service
 
 [Socket]
-ListenStream=@runstatedir@/libvirt/@sockprefix@-sock
-Service=@service@.service
+ListenStream=@runstatedir@/libvirt/libvirt-sock
+Service=libvirtd.service
 SocketMode=@sockmode@
 RemoveOnStop=yes
 
-- 
2.41.0



[libvirt PATCH v2 17/33] systemd: Switch virtxend to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/libxl/meson.build   |  7 ++-
 src/libxl/virtxend.service.extra.in | 12 +++
 src/libxl/virtxend.service.in   | 32 -
 src/libxl/virtxend.socket.extra.in  |  2 ++
 4 files changed, 16 insertions(+), 37 deletions(-)
 create mode 100644 src/libxl/virtxend.service.extra.in
 delete mode 100644 src/libxl/virtxend.service.in
 create mode 100644 src/libxl/virtxend.socket.extra.in

diff --git a/src/libxl/meson.build b/src/libxl/meson.build
index a1553dbe27..171d6ca005 100644
--- a/src/libxl/meson.build
+++ b/src/libxl/meson.build
@@ -66,12 +66,9 @@ if conf.has('WITH_LIBXL')
 
   virt_daemon_units += {
 'service': 'virtxend',
-'service_in': files('virtxend.service.in'),
 'name': 'Libvirt libxl',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
-'deps': 'ConditionPathExists=/proc/xen/capabilities',
+'service_extra_in': files('virtxend.service.extra.in'),
+'socket_extra_in': files('virtxend.socket.extra.in'),
   }
 
   openrc_init_files += {
diff --git a/src/libxl/virtxend.service.extra.in 
b/src/libxl/virtxend.service.extra.in
new file mode 100644
index 00..ba38ba9160
--- /dev/null
+++ b/src/libxl/virtxend.service.extra.in
@@ -0,0 +1,12 @@
+[Unit]
+Wants=virtlockd.socket
+After=remote-fs.target
+After=xencommons.service
+Conflicts=xendomains.service
+ConditionPathExists=/proc/xen/capabilities
+
+[Service]
+KillMode=process
+
+[Install]
+Also=virtlockd.socket
diff --git a/src/libxl/virtxend.service.in b/src/libxl/virtxend.service.in
deleted file mode 100644
index c6a88f7fe9..00
--- a/src/libxl/virtxend.service.in
+++ /dev/null
@@ -1,32 +0,0 @@
-[Unit]
-Description=Virtualization xen daemon
-Conflicts=libvirtd.service
-Requires=virtxend.socket
-Requires=virtxend-ro.socket
-Requires=virtxend-admin.socket
-Wants=virtlockd.socket
-After=network.target
-After=dbus.service
-After=apparmor.service
-After=remote-fs.target
-After=xencommons.service
-Conflicts=xendomains.service
-Documentation=man:virtxend(8)
-Documentation=https://libvirt.org
-ConditionPathExists=/proc/xen/capabilities
-
-[Service]
-Type=notify
-Environment=VIRTXEND_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtxend
-ExecStart=@sbindir@/virtxend $VIRTXEND_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-KillMode=process
-
-[Install]
-WantedBy=multi-user.target
-Also=virtlockd.socket
-Also=virtxend.socket
-Also=virtxend-ro.socket
-Also=virtxend-admin.socket
diff --git a/src/libxl/virtxend.socket.extra.in 
b/src/libxl/virtxend.socket.extra.in
new file mode 100644
index 00..c8322efbbc
--- /dev/null
+++ b/src/libxl/virtxend.socket.extra.in
@@ -0,0 +1,2 @@
+[Unit]
+ConditionPathExists=/proc/xen/capabilities
-- 
2.41.0



[libvirt PATCH v2 26/33] systemd: Downgrade read-only/admin sockets to Wants

2023-09-27 Thread Andrea Bolognani
Only the main socket is actually necessary for the service to be
usable.

In the past, we've had security issues that could be exploited via
access to the read-only socket, so a security-minded administrator
might consider disabling all optional sockets. This change makes
such a setup possible.

Note that the services will still try to activate all their
sockets on startup, even if they have been disabled. To make sure
that the optional sockets are never started, they will have to be
masked.

Signed-off-by: Andrea Bolognani 
---
 src/locking/virtlockd.service.in | 2 +-
 src/logging/virtlogd.service.in  | 2 +-
 src/virtd.service.in | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in
index 35924a2ad7..fcf479c3c6 100644
--- a/src/locking/virtlockd.service.in
+++ b/src/locking/virtlockd.service.in
@@ -1,7 +1,7 @@
 [Unit]
 Description=Virtual machine lock manager
 BindsTo=virtlockd.socket
-Requires=virtlockd-admin.socket
+Wants=virtlockd-admin.socket
 After=virtlockd.socket
 Before=libvirtd.service
 Documentation=man:virtlockd(8)
diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in
index 79d34bc73e..3265ecd6af 100644
--- a/src/logging/virtlogd.service.in
+++ b/src/logging/virtlogd.service.in
@@ -1,7 +1,7 @@
 [Unit]
 Description=Virtual machine log manager
 BindsTo=virtlogd.socket
-Requires=virtlogd-admin.socket
+Wants=virtlogd-admin.socket
 After=virtlogd.socket
 Before=libvirtd.service
 Documentation=man:virtlogd(8)
diff --git a/src/virtd.service.in b/src/virtd.service.in
index e7f08b4da9..f4f1bc217d 100644
--- a/src/virtd.service.in
+++ b/src/virtd.service.in
@@ -1,8 +1,8 @@
 [Unit]
 Description=@name@ daemon
 BindsTo=@service@.socket
-Requires=@service@-ro.socket
-Requires=@service@-admin.socket
+Wants=@service@-ro.socket
+Wants=@service@-admin.socket
 After=@service@.socket
 Conflicts=libvirtd.service
 After=libvirtd.service
-- 
2.41.0



[libvirt PATCH v2 10/33] systemd: Switch virtnwfilterd to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/nwfilter/meson.build  |  4 
 src/nwfilter/virtnwfilterd.service.in | 25 -
 2 files changed, 29 deletions(-)
 delete mode 100644 src/nwfilter/virtnwfilterd.service.in

diff --git a/src/nwfilter/meson.build b/src/nwfilter/meson.build
index de672bb827..c091bc3f1b 100644
--- a/src/nwfilter/meson.build
+++ b/src/nwfilter/meson.build
@@ -50,11 +50,7 @@ if conf.has('WITH_NWFILTER')
 
   virt_daemon_units += {
 'service': 'virtnwfilterd',
-'service_in': files('virtnwfilterd.service.in'),
 'name': 'Libvirt nwfilter',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/nwfilter/virtnwfilterd.service.in 
b/src/nwfilter/virtnwfilterd.service.in
deleted file mode 100644
index d6e98240a8..00
--- a/src/nwfilter/virtnwfilterd.service.in
+++ /dev/null
@@ -1,25 +0,0 @@
-[Unit]
-Description=Virtualization nwfilter daemon
-Conflicts=libvirtd.service
-Requires=virtnwfilterd.socket
-Requires=virtnwfilterd-ro.socket
-Requires=virtnwfilterd-admin.socket
-After=network.target
-After=dbus.service
-After=apparmor.service
-Documentation=man:virtnwfilterd(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTNWFILTERD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtnwfilterd
-ExecStart=@sbindir@/virtnwfilterd $VIRTNWFILTERD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-
-[Install]
-WantedBy=multi-user.target
-Also=virtnwfilterd.socket
-Also=virtnwfilterd-ro.socket
-Also=virtnwfilterd-admin.socket
-- 
2.41.0



[libvirt PATCH v2 32/33] systemd: Improve and unify unit descriptions

2023-09-27 Thread Andrea Bolognani
Hypervisors are referred to by their user-facing name rather
than the name of their libvirt driver, the monolithic daemon is
explicitly referred to as legacy, and a consistent format is
used throughout.

Signed-off-by: Andrea Bolognani 
---
 src/ch/meson.build| 2 +-
 src/interface/meson.build | 2 +-
 src/libxl/meson.build | 2 +-
 src/locking/meson.build   | 2 +-
 src/locking/virtlockd-admin.socket.in | 2 +-
 src/locking/virtlockd.service.in  | 2 +-
 src/locking/virtlockd.socket.in   | 2 +-
 src/logging/meson.build   | 2 +-
 src/logging/virtlogd-admin.socket.in  | 2 +-
 src/logging/virtlogd.service.in   | 2 +-
 src/logging/virtlogd.socket.in| 2 +-
 src/lxc/meson.build   | 2 +-
 src/network/meson.build   | 2 +-
 src/node_device/meson.build   | 2 +-
 src/nwfilter/meson.build  | 2 +-
 src/qemu/meson.build  | 2 +-
 src/remote/libvirtd-admin.socket.in   | 2 +-
 src/remote/libvirtd-ro.socket.in  | 2 +-
 src/remote/libvirtd-tcp.socket.in | 2 +-
 src/remote/libvirtd-tls.socket.in | 2 +-
 src/remote/libvirtd.service.in| 2 +-
 src/remote/libvirtd.socket.in | 2 +-
 src/remote/meson.build| 4 ++--
 src/secret/meson.build| 2 +-
 src/storage/meson.build   | 2 +-
 src/vbox/meson.build  | 2 +-
 src/virtd-admin.socket.in | 2 +-
 src/virtd-ro.socket.in| 2 +-
 src/virtd-tcp.socket.in   | 2 +-
 src/virtd-tls.socket.in   | 2 +-
 src/virtd.service.in  | 2 +-
 src/virtd.socket.in   | 2 +-
 src/vz/meson.build| 2 +-
 33 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/src/ch/meson.build b/src/ch/meson.build
index 0ef7288257..df246ef9b0 100644
--- a/src/ch/meson.build
+++ b/src/ch/meson.build
@@ -57,7 +57,7 @@ if conf.has('WITH_CH')
 
   virt_daemon_units += {
 'service': 'virtchd',
-'name': 'Libvirt ch',
+'name': 'Cloud Hypervisor',
 'service_extra_in': files('virtchd.service.extra.in'),
   }
 
diff --git a/src/interface/meson.build b/src/interface/meson.build
index 54c0b1a935..b1617d83e6 100644
--- a/src/interface/meson.build
+++ b/src/interface/meson.build
@@ -44,7 +44,7 @@ if conf.has('WITH_INTERFACE')
 
   virt_daemon_units += {
 'service': 'virtinterfaced',
-'name': 'Libvirt interface',
+'name': 'interface',
   }
 
   openrc_init_files += {
diff --git a/src/libxl/meson.build b/src/libxl/meson.build
index 171d6ca005..8e6f455139 100644
--- a/src/libxl/meson.build
+++ b/src/libxl/meson.build
@@ -66,7 +66,7 @@ if conf.has('WITH_LIBXL')
 
   virt_daemon_units += {
 'service': 'virtxend',
-'name': 'Libvirt libxl',
+'name': 'Xen',
 'service_extra_in': files('virtxend.service.extra.in'),
 'socket_extra_in': files('virtxend.socket.extra.in'),
   }
diff --git a/src/locking/meson.build b/src/locking/meson.build
index 2ccc822ed3..6b3cd781d1 100644
--- a/src/locking/meson.build
+++ b/src/locking/meson.build
@@ -144,7 +144,7 @@ if conf.has('WITH_LIBVIRTD')
   virt_daemon_units += {
 'service': 'virtlockd',
 'service_in': files('virtlockd.service.in'),
-'name': 'Libvirt locking',
+'name': 'locking',
 'sockets': [ 'main', 'admin' ],
 'socket_in': files('virtlockd.socket.in'),
 'socket_admin_in': files('virtlockd-admin.socket.in'),
diff --git a/src/locking/virtlockd-admin.socket.in 
b/src/locking/virtlockd-admin.socket.in
index 0452a0cfdb..ed5b94edba 100644
--- a/src/locking/virtlockd-admin.socket.in
+++ b/src/locking/virtlockd-admin.socket.in
@@ -1,5 +1,5 @@
 [Unit]
-Description=Virtual machine lock manager admin socket
+Description=libvirt locking daemon admin socket
 BindsTo=virtlockd.socket
 After=virtlockd.socket
 
diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in
index 20b4b26f35..290a2887a5 100644
--- a/src/locking/virtlockd.service.in
+++ b/src/locking/virtlockd.service.in
@@ -1,5 +1,5 @@
 [Unit]
-Description=Virtual machine lock manager
+Description=libvirt locking daemon
 BindsTo=virtlockd.socket
 Wants=virtlockd-admin.socket
 After=virtlockd.socket
diff --git a/src/locking/virtlockd.socket.in b/src/locking/virtlockd.socket.in
index 31a576aa16..4eec90a95e 100644
--- a/src/locking/virtlockd.socket.in
+++ b/src/locking/virtlockd.socket.in
@@ -1,5 +1,5 @@
 [Unit]
-Description=Virtual machine lock manager socket
+Description=libvirt locking daemon socket
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/virtlockd-sock
diff --git a/src/logging/meson.build b/src/logging/meson.build
index 95d2ef2a3f..1527f91faf 100644
--- a/src/logging/meson.build
+++ b/src/logging/meson.build
@@ -91,7 +91,7 @@ if conf.has('WITH_LIBVIRTD')
   virt_daemon_units += {
 'service': 'virtlogd',
 'service_in': files('virtlogd.service.in'),
-'name': 'Libvirt logging',
+'name': 'logging',

[libvirt PATCH v2 16/33] systemd: Switch virtchd to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/ch/meson.build  |  5 +---
 src/ch/virtchd.service.extra.in | 22 +
 src/ch/virtchd.service.in   | 44 -
 3 files changed, 23 insertions(+), 48 deletions(-)
 create mode 100644 src/ch/virtchd.service.extra.in
 delete mode 100644 src/ch/virtchd.service.in

diff --git a/src/ch/meson.build b/src/ch/meson.build
index dc08069dcd..0ef7288257 100644
--- a/src/ch/meson.build
+++ b/src/ch/meson.build
@@ -57,11 +57,8 @@ if conf.has('WITH_CH')
 
   virt_daemon_units += {
 'service': 'virtchd',
-'service_in': files('virtchd.service.in'),
 'name': 'Libvirt ch',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
+'service_extra_in': files('virtchd.service.extra.in'),
   }
 
   virt_install_dirs += [
diff --git a/src/ch/virtchd.service.extra.in b/src/ch/virtchd.service.extra.in
new file mode 100644
index 00..bc2fef57cc
--- /dev/null
+++ b/src/ch/virtchd.service.extra.in
@@ -0,0 +1,22 @@
+[Unit]
+Wants=systemd-machined.service
+After=systemd-machined.service
+After=remote-fs.target
+
+[Service]
+KillMode=process
+# Raise hard limits to match behaviour of systemd >= 240.
+# During startup, daemon will set soft limit to match hard limit
+# per systemd recommendations
+LimitNOFILE=1024:524288
+# The cgroups pids controller can limit the number of tasks started by
+# the daemon, which can limit the number of domains for some hypervisors.
+# A conservative default of 8 tasks per guest results in a TasksMax of
+# 32k to support 4096 guests.
+TasksMax=32768
+# With cgroups v2 there is no devices controller anymore, we have to use
+# eBPF to control access to devices. In order to do that we create a eBPF
+# hash MAP which locks memory. The default map size for 64 devices together
+# with program takes 12k per guest. After rounding up we will get 64M to
+# support 4096 guests.
+LimitMEMLOCK=64M
diff --git a/src/ch/virtchd.service.in b/src/ch/virtchd.service.in
deleted file mode 100644
index 351eee312b..00
--- a/src/ch/virtchd.service.in
+++ /dev/null
@@ -1,44 +0,0 @@
-[Unit]
-Description=Virtualization Cloud-Hypervisor daemon
-Conflicts=libvirtd.service
-Requires=virtchd.socket
-Requires=virtchd-ro.socket
-Requires=virtchd-admin.socket
-Wants=systemd-machined.service
-After=network.target
-After=dbus.service
-After=apparmor.service
-After=remote-fs.target
-After=systemd-machined.service
-Documentation=man:virtchd(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTCHD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtchd
-ExecStart=@sbindir@/virtchd $VIRTCHD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-KillMode=process
-Restart=on-failure
-# Raise hard limits to match behaviour of systemd >= 240.
-# During startup, daemon will set soft limit to match hard limit
-# per systemd recommendations
-LimitNOFILE=1024:524288
-# The cgroups pids controller can limit the number of tasks started by
-# the daemon, which can limit the number of domains for some hypervisors.
-# A conservative default of 8 tasks per guest results in a TasksMax of
-# 32k to support 4096 guests.
-TasksMax=32768
-# With cgroups v2 there is no devices controller anymore, we have to use
-# eBPF to control access to devices.  In order to do that we create a eBPF
-# hash MAP which locks memory.  The default map size for 64 devices together
-# with program takes 12k per guest.  After rounding up we will get 64M to
-# support 4096 guests.
-LimitMEMLOCK=64M
-
-[Install]
-WantedBy=multi-user.target
-Also=virtchd.socket
-Also=virtchd-ro.socket
-Also=virtchd-admin.socket
-- 
2.41.0



[libvirt PATCH v2 20/33] systemd: Switch virtproxyd to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/remote/meson.build   |  6 --
 src/remote/virtproxyd.service.in | 25 -
 2 files changed, 31 deletions(-)
 delete mode 100644 src/remote/virtproxyd.service.in

diff --git a/src/remote/meson.build b/src/remote/meson.build
index 78c08bf5ad..5ee6d4e61c 100644
--- a/src/remote/meson.build
+++ b/src/remote/meson.build
@@ -224,15 +224,9 @@ if conf.has('WITH_REMOTE')
 
 virt_daemon_units += {
   'service': 'virtproxyd',
-  'service_in': files('virtproxyd.service.in'),
   'name': 'Libvirt proxy',
   'sockprefix': 'libvirt',
   'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
-  'socket_in': files('libvirtd.socket.in'),
-  'socket_ro_in': files('libvirtd-ro.socket.in'),
-  'socket_admin_in': files('libvirtd-admin.socket.in'),
-  'socket_tcp_in': files('libvirtd-tcp.socket.in'),
-  'socket_tls_in': files('libvirtd-tls.socket.in'),
 }
 
 openrc_init_files += {
diff --git a/src/remote/virtproxyd.service.in b/src/remote/virtproxyd.service.in
deleted file mode 100644
index 9b829641f7..00
--- a/src/remote/virtproxyd.service.in
+++ /dev/null
@@ -1,25 +0,0 @@
-[Unit]
-Description=Virtualization daemon
-Conflicts=libvirtd.service
-Requires=virtproxyd.socket
-Requires=virtproxyd-ro.socket
-Requires=virtproxyd-admin.socket
-After=network.target
-After=dbus.service
-After=apparmor.service
-Documentation=man:virtproxyd(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTPROXYD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtproxyd
-ExecStart=@sbindir@/virtproxyd $VIRTPROXYD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-
-[Install]
-WantedBy=multi-user.target
-Also=virtproxyd.socket
-Also=virtproxyd-ro.socket
-Also=virtproxyd-admin.socket
-- 
2.41.0



[libvirt PATCH v2 08/33] systemd: Switch virtnodedevd to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/node_device/meson.build |  4 
 src/node_device/virtnodedevd.service.in | 25 -
 2 files changed, 29 deletions(-)
 delete mode 100644 src/node_device/virtnodedevd.service.in

diff --git a/src/node_device/meson.build b/src/node_device/meson.build
index dd60b1f819..2614ff8b9c 100644
--- a/src/node_device/meson.build
+++ b/src/node_device/meson.build
@@ -52,11 +52,7 @@ if conf.has('WITH_NODE_DEVICES')
 
   virt_daemon_units += {
 'service': 'virtnodedevd',
-'service_in': files('virtnodedevd.service.in'),
 'name': 'Libvirt nodedev',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/node_device/virtnodedevd.service.in 
b/src/node_device/virtnodedevd.service.in
deleted file mode 100644
index 2ac41db32e..00
--- a/src/node_device/virtnodedevd.service.in
+++ /dev/null
@@ -1,25 +0,0 @@
-[Unit]
-Description=Virtualization nodedev daemon
-Conflicts=libvirtd.service
-Requires=virtnodedevd.socket
-Requires=virtnodedevd-ro.socket
-Requires=virtnodedevd-admin.socket
-After=network.target
-After=dbus.service
-After=apparmor.service
-Documentation=man:virtnodedevd(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTNODEDEVD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtnodedevd
-ExecStart=@sbindir@/virtnodedevd $VIRTNODEDEVD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-
-[Install]
-WantedBy=multi-user.target
-Also=virtnodedevd.socket
-Also=virtnodedevd-ro.socket
-Also=virtnodedevd-admin.socket
-- 
2.41.0



[libvirt PATCH v2 24/33] systemd: Make modular daemons conflict with libvirtd

2023-09-27 Thread Andrea Bolognani
We want to make sure that, at any given time, we have either the
modular daemons or the monolithic one running, never both. In
order to achieve that, make every single modular unit conflict
with the corresponding libvirtd unit.

We set both Conflicts=libvirtd.unit and After=libvirtd.unit: this
tells systemd that, whenever virtfood.unit and libvirtd.unit are
part of the same transaction, the former should win out.

Thanks to this, if both the modular daemons and the monolithic
one have been enabled because of outdated automation or a simple
mistake of the administrator, the request to start libvirtd at
boot will be ignored and the result will be a regular modular
deployment.

If the request to start libvirtd is made when the modular daemons
are already running, we have no way to prevent systemd from
complying with that request; however, thanks to the way the
conflict relationship has been declared, they will be shut down
cleanly before libvirtd is started. From the user's point of
view, the transition from modular to monolithic will be
completely transparent: it's basically the same scenario as a
regular package upgrade, just with an extra twist.

Note that, while switching from modular to monolithic at runtime
happens automatically, going back requires manual intervention,
i.e. starting all the necessary sockets one by one. That's okay:
the goal here is to prevent misconfiguration and force of habit
to accidentally disrupt a working setup, not to encourage the
scenario. In a correctly configured and managed host, it should
never occur.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Daniel P. Berrangé 
---
 src/virtd-admin.socket.in | 2 ++
 src/virtd-ro.socket.in| 2 ++
 src/virtd-tcp.socket.in   | 2 ++
 src/virtd-tls.socket.in   | 2 ++
 src/virtd.service.in  | 3 ++-
 src/virtd.socket.in   | 2 ++
 6 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in
index 39bb0badea..42cc1f670f 100644
--- a/src/virtd-admin.socket.in
+++ b/src/virtd-admin.socket.in
@@ -3,6 +3,8 @@ Description=@name@ admin socket
 Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
+Conflicts=libvirtd-admin.socket
+After=libvirtd-admin.socket
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock
diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in
index b7b7ae0dd8..7b8cbdba20 100644
--- a/src/virtd-ro.socket.in
+++ b/src/virtd-ro.socket.in
@@ -3,6 +3,8 @@ Description=@name@ local read-only socket
 Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
+Conflicts=libvirtd-ro.socket
+After=libvirtd-ro.socket
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro
diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in
index 7c8bcdb525..9fe90ed0a0 100644
--- a/src/virtd-tcp.socket.in
+++ b/src/virtd-tcp.socket.in
@@ -3,6 +3,8 @@ Description=@name@ non-TLS IP socket
 Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
+Conflicts=libvirtd-tcp.socket
+After=libvirtd-tcp.socket
 
 [Socket]
 ListenStream=16509
diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in
index c6dceb2d4e..bb89daddb5 100644
--- a/src/virtd-tls.socket.in
+++ b/src/virtd-tls.socket.in
@@ -3,6 +3,8 @@ Description=@name@ TLS IP socket
 Before=@service@.service
 BindsTo=@service@.socket
 After=@service@.socket
+Conflicts=libvirtd-tls.socket
+After=libvirtd-tls.socket
 
 [Socket]
 ListenStream=16514
diff --git a/src/virtd.service.in b/src/virtd.service.in
index 76f9c60351..60ab122cbc 100644
--- a/src/virtd.service.in
+++ b/src/virtd.service.in
@@ -1,9 +1,10 @@
 [Unit]
 Description=@name@ daemon
-Conflicts=libvirtd.service
 Requires=@service@.socket
 Requires=@service@-ro.socket
 Requires=@service@-admin.socket
+Conflicts=libvirtd.service
+After=libvirtd.service
 After=network.target
 After=dbus.service
 After=apparmor.service
diff --git a/src/virtd.socket.in b/src/virtd.socket.in
index aec0708fd4..053dc1c782 100644
--- a/src/virtd.socket.in
+++ b/src/virtd.socket.in
@@ -1,6 +1,8 @@
 [Unit]
 Description=@name@ local socket
 Before=@service@.service
+Conflicts=libvirtd.socket
+After=libvirtd.socket
 
 [Socket]
 ListenStream=@runstatedir@/libvirt/@sockprefix@-sock
-- 
2.41.0



[libvirt PATCH v2 01/33] systemd: Drop Conflicts from virtproxyd sockets

2023-09-27 Thread Andrea Bolognani
The idea behind these is to prevent running both modular daemons
and monolithic daemon at the same time. We will implement a more
effective solution for that shortly.

Signed-off-by: Andrea Bolognani 
---
 src/remote/meson.build | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/remote/meson.build b/src/remote/meson.build
index eb4f7a0068..dc2f528d0b 100644
--- a/src/remote/meson.build
+++ b/src/remote/meson.build
@@ -128,8 +128,6 @@ libvirtd_socket_unit_files = [
   'libvirtd-tls.socket',
 ]
 
-libvirtd_socket_conflicts = ' '.join(libvirtd_socket_unit_files)
-
 logrotate_files = [
   'libvirtd.qemu',
   'libvirtd.lxc',
@@ -225,7 +223,6 @@ if conf.has('WITH_REMOTE')
   'name': 'Libvirt proxy',
   'sockprefix': 'libvirt',
   'sockets': [ 'main', 'ro', 'admin', 'tcp', 'tls' ],
-  'deps': 'Conflicts=' + libvirtd_socket_conflicts,
 }
 
 openrc_init_files += {
-- 
2.41.0



[libvirt PATCH v2 00/33] systemd: Improve units for services and sockets

2023-09-27 Thread Andrea Bolognani
A grab bag of changes, ranging from very much functional ones
to purely aesthetical ones.

Changes from [v1]

  * patches 01-11 from the original series have been pushed;

  * patch 40 from the original series has been dropped;

  * patches 02 (cosmetic) and 31 (bug fix) have been added;

  * the templating mechanism has been completely overhauled, and
now uses a Python script for performing service-specific unit
customizations instead of meson's built-in templating
capabilities;

  * as a result of the above, service-specific customizations are now
stored in specific foo.{service,socket}.extra.in files instead of
meson.build;

  * various other tweaks in response to review feedback.

[v1] https://listman.redhat.com/archives/libvir-list/2023-September/242288.html

Andrea Bolognani (33):
  systemd: Drop Conflicts from virtproxyd sockets
  systemd: Introduce service_in/service_out variables
  systemd: Make @service_in@ optional
  systemd: Introduce temporary libvirtd_socket*_in values
  systemd: Provide all input files explicitly
  systemd: Introduce common templates
  systemd: Use common templates by default
  systemd: Switch virtnodedevd to common templates
  systemd: Switch virtinterfaced to common templates
  systemd: Switch virtnwfilterd to common templates
  systemd: Switch virtsecretd to common templates
  systemd: Switch virtnetworkd to common templates
  systemd: Switch virtstoraged to common templates
  systemd: Switch virtvboxd to common templates
  systemd: Switch virtvzd to common templates
  systemd: Switch virtchd to common templates
  systemd: Switch virtxend to common templates
  systemd: Switch virtlxcd to common templates
  systemd: Switch virtqemud to common templates
  systemd: Switch virtproxyd to common templates
  systemd: Drop libvirtd_socket*_in values
  systemd: Drop @deps@
  systemd: Drop parametrization from libvirtd sockets
  systemd: Make modular daemons conflict with libvirtd
  systemd: Replace Requires with BindTo+After for main socket
  systemd: Downgrade read-only/admin sockets to Wants
  systemd: Augment Requires/Wants with After
  systemd: Drop Before=libvirtd from virtlogd/virtlockd
  systemd: Drop Before=foo.service from sockets
  systemd: Add Also between sockets
  systemd: Add RemoveOnStop=yes to all sockets
  systemd: Improve and unify unit descriptions
  systemd: Move Documentation lines

 scripts/merge-systemd-units.py| 91 +++
 scripts/meson.build   |  1 +
 src/ch/meson.build|  4 +-
 src/ch/virtchd.service.extra.in   | 22 +
 src/ch/virtchd.service.in | 44 -
 src/interface/meson.build |  3 +-
 src/interface/virtinterfaced.service.in   | 25 -
 src/libxl/meson.build |  6 +-
 src/libxl/virtxend.service.extra.in   | 13 +++
 src/libxl/virtxend.service.in | 32 ---
 src/libxl/virtxend.socket.extra.in|  2 +
 src/locking/meson.build   |  2 +-
 src/locking/virtlockd-admin.socket.in |  5 +-
 src/locking/virtlockd.service.in  | 11 ++-
 src/locking/virtlockd.socket.in   |  5 +-
 src/logging/meson.build   |  2 +-
 src/logging/virtlogd-admin.socket.in  |  5 +-
 src/logging/virtlogd.service.in   | 11 ++-
 src/logging/virtlogd.socket.in|  5 +-
 src/lxc/meson.build   |  4 +-
 src/lxc/virtlxcd.service.extra.in | 22 +
 src/lxc/virtlxcd.service.in   | 44 -
 src/meson.build   | 41 +++--
 src/network/meson.build   |  4 +-
 src/network/virtnetworkd.service.extra.in |  2 +
 src/network/virtnetworkd.service.in   | 26 --
 src/node_device/meson.build   |  3 +-
 src/node_device/virtnodedevd.service.in   | 25 -
 src/nwfilter/meson.build  |  3 +-
 src/nwfilter/virtnwfilterd.service.in | 25 -
 src/qemu/meson.build  |  4 +-
 src/qemu/virtqemud.service.extra.in   | 30 ++
 src/qemu/virtqemud.service.in | 48 --
 src/remote/libvirtd-admin.socket.in   | 15 +--
 src/remote/libvirtd-ro.socket.in  | 15 +--
 src/remote/libvirtd-tcp.socket.in | 10 +-
 src/remote/libvirtd-tls.socket.in | 10 +-
 src/remote/libvirtd.service.in| 13 ++-
 src/remote/libvirtd.socket.in | 10 +-
 src/remote/meson.build| 13 +--
 src/remote/virtproxyd.service.in  | 25 -
 src/secret/meson.build|  3 +-
 src/secret/virtsecretd.service.in | 25 -
 src/storage/meson.build   |  4 +-
 src/storage/virtstoraged.service.extra.in |  3 +
 src/storage/virtstoraged.service.in   | 

[libvirt PATCH v2 19/33] systemd: Switch virtqemud to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/qemu/meson.build|  5 +--
 src/qemu/virtqemud.service.extra.in | 28 +
 src/qemu/virtqemud.service.in   | 48 -
 3 files changed, 29 insertions(+), 52 deletions(-)
 create mode 100644 src/qemu/virtqemud.service.extra.in
 delete mode 100644 src/qemu/virtqemud.service.in

diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index b52497bdf0..1afc301a6d 100644
--- a/src/qemu/meson.build
+++ b/src/qemu/meson.build
@@ -183,11 +183,8 @@ if conf.has('WITH_QEMU')
 
   virt_daemon_units += {
 'service': 'virtqemud',
-'service_in': files('virtqemud.service.in'),
 'name': 'Libvirt qemu',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
+'service_extra_in': files('virtqemud.service.extra.in'),
   }
 
   openrc_init_files += {
diff --git a/src/qemu/virtqemud.service.extra.in 
b/src/qemu/virtqemud.service.extra.in
new file mode 100644
index 00..eaf616f575
--- /dev/null
+++ b/src/qemu/virtqemud.service.extra.in
@@ -0,0 +1,28 @@
+[Unit]
+Requires=virtlogd.socket
+Wants=virtlockd.socket
+Wants=systemd-machined.service
+After=systemd-machined.service
+After=remote-fs.target
+
+[Service]
+KillMode=process
+# Raise hard limits to match behaviour of systemd >= 240.
+# During startup, daemon will set soft limit to match hard limit
+# per systemd recommendations
+LimitNOFILE=1024:524288
+# The cgroups pids controller can limit the number of tasks started by
+# the daemon, which can limit the number of domains for some hypervisors.
+# A conservative default of 8 tasks per guest results in a TasksMax of
+# 32k to support 4096 guests.
+TasksMax=32768
+# With cgroups v2 there is no devices controller anymore, we have to use
+# eBPF to control access to devices. In order to do that we create a eBPF
+# hash MAP which locks memory. The default map size for 64 devices together
+# with program takes 12k per guest. After rounding up we will get 64M to
+# support 4096 guests.
+LimitMEMLOCK=64M
+
+[Install]
+Also=virtlogd.socket
+Also=virtlockd.socket
diff --git a/src/qemu/virtqemud.service.in b/src/qemu/virtqemud.service.in
deleted file mode 100644
index e79670ca95..00
--- a/src/qemu/virtqemud.service.in
+++ /dev/null
@@ -1,48 +0,0 @@
-[Unit]
-Description=Virtualization qemu daemon
-Conflicts=libvirtd.service
-Requires=virtlogd.socket
-Requires=virtqemud.socket
-Requires=virtqemud-ro.socket
-Requires=virtqemud-admin.socket
-Wants=virtlockd.socket
-Wants=systemd-machined.service
-After=network.target
-After=dbus.service
-After=apparmor.service
-After=remote-fs.target
-After=systemd-machined.service
-Documentation=man:virtqemud(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTQEMUD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtqemud
-ExecStart=@sbindir@/virtqemud $VIRTQEMUD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-KillMode=process
-Restart=on-failure
-# Raise hard limits to match behaviour of systemd >= 240.
-# During startup, daemon will set soft limit to match hard limit
-# per systemd recommendations
-LimitNOFILE=1024:524288
-# The cgroups pids controller can limit the number of tasks started by
-# the daemon, which can limit the number of domains for some hypervisors.
-# A conservative default of 8 tasks per guest results in a TasksMax of
-# 32k to support 4096 guests.
-TasksMax=32768
-# With cgroups v2 there is no devices controller anymore, we have to use
-# eBPF to control access to devices.  In order to do that we create a eBPF
-# hash MAP which locks memory.  The default map size for 64 devices together
-# with program takes 12k per guest.  After rounding up we will get 64M to
-# support 4096 guests.
-LimitMEMLOCK=64M
-
-[Install]
-WantedBy=multi-user.target
-Also=virtlogd.socket
-Also=virtlockd.socket
-Also=virtqemud.socket
-Also=virtqemud-ro.socket
-Also=virtqemud-admin.socket
-- 
2.41.0



[libvirt PATCH v2 30/33] systemd: Add Also between sockets

2023-09-27 Thread Andrea Bolognani
This results in all sockets for a service being enabled when a
single one of them is.

The -tcp and -tls sockets are intentionally excluded, because
enabling them should require explicit action on the
administrator's part; moreover, disabling them should not result
in the local sockets being disabled too.

Signed-off-by: Andrea Bolognani 
---
 src/locking/virtlockd-admin.socket.in | 1 +
 src/locking/virtlockd.socket.in   | 1 +
 src/logging/virtlogd-admin.socket.in  | 1 +
 src/logging/virtlogd.socket.in| 1 +
 src/remote/libvirtd-admin.socket.in   | 2 ++
 src/remote/libvirtd-ro.socket.in  | 2 ++
 src/remote/libvirtd.socket.in | 2 ++
 src/virtd-admin.socket.in | 2 ++
 src/virtd-ro.socket.in| 2 ++
 src/virtd.socket.in   | 2 ++
 10 files changed, 16 insertions(+)

diff --git a/src/locking/virtlockd-admin.socket.in 
b/src/locking/virtlockd-admin.socket.in
index d5ebd7f60b..d05ba982d9 100644
--- a/src/locking/virtlockd-admin.socket.in
+++ b/src/locking/virtlockd-admin.socket.in
@@ -10,3 +10,4 @@ SocketMode=0600
 
 [Install]
 WantedBy=sockets.target
+Also=virtlockd.socket
diff --git a/src/locking/virtlockd.socket.in b/src/locking/virtlockd.socket.in
index d2cc2a06a3..98aabb2511 100644
--- a/src/locking/virtlockd.socket.in
+++ b/src/locking/virtlockd.socket.in
@@ -8,3 +8,4 @@ SocketMode=0600
 
 [Install]
 WantedBy=sockets.target
+Also=virtlockd-admin.socket
diff --git a/src/logging/virtlogd-admin.socket.in 
b/src/logging/virtlogd-admin.socket.in
index 67259803ca..75ec7bd5fa 100644
--- a/src/logging/virtlogd-admin.socket.in
+++ b/src/logging/virtlogd-admin.socket.in
@@ -10,3 +10,4 @@ SocketMode=0600
 
 [Install]
 WantedBy=sockets.target
+Also=virtlogd.socket
diff --git a/src/logging/virtlogd.socket.in b/src/logging/virtlogd.socket.in
index 7b3fc73773..b044d62e7c 100644
--- a/src/logging/virtlogd.socket.in
+++ b/src/logging/virtlogd.socket.in
@@ -8,3 +8,4 @@ SocketMode=0600
 
 [Install]
 WantedBy=sockets.target
+Also=virtlogd-admin.socket
diff --git a/src/remote/libvirtd-admin.socket.in 
b/src/remote/libvirtd-admin.socket.in
index 098e372971..6df038d95a 100644
--- a/src/remote/libvirtd-admin.socket.in
+++ b/src/remote/libvirtd-admin.socket.in
@@ -10,3 +10,5 @@ SocketMode=0600
 
 [Install]
 WantedBy=sockets.target
+Also=libvirtd.socket
+Also=libvirtd-ro.socket
diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in
index 101555e8a0..6797517c50 100644
--- a/src/remote/libvirtd-ro.socket.in
+++ b/src/remote/libvirtd-ro.socket.in
@@ -10,3 +10,5 @@ SocketMode=0666
 
 [Install]
 WantedBy=sockets.target
+Also=libvirtd.socket
+Also=libvirtd-admin.socket
diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in
index 3019821df3..f483facdf3 100644
--- a/src/remote/libvirtd.socket.in
+++ b/src/remote/libvirtd.socket.in
@@ -9,3 +9,5 @@ RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
+Also=libvirtd-ro.socket
+Also=libvirtd-admin.socket
diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in
index 63db2be5fe..5a5f577041 100644
--- a/src/virtd-admin.socket.in
+++ b/src/virtd-admin.socket.in
@@ -12,3 +12,5 @@ SocketMode=0600
 
 [Install]
 WantedBy=sockets.target
+Also=@service@.socket
+Also=@service@-ro.socket
diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in
index 32e4789b8b..692279665d 100644
--- a/src/virtd-ro.socket.in
+++ b/src/virtd-ro.socket.in
@@ -12,3 +12,5 @@ SocketMode=0666
 
 [Install]
 WantedBy=sockets.target
+Also=@service@.socket
+Also=@service@-admin.socket
diff --git a/src/virtd.socket.in b/src/virtd.socket.in
index d0a0bb3b1c..7a8c4bf0c2 100644
--- a/src/virtd.socket.in
+++ b/src/virtd.socket.in
@@ -11,3 +11,5 @@ RemoveOnStop=yes
 
 [Install]
 WantedBy=sockets.target
+Also=@service@-ro.socket
+Also=@service@-admin.socket
-- 
2.41.0



[libvirt PATCH v2 09/33] systemd: Switch virtinterfaced to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/interface/meson.build   |  4 
 src/interface/virtinterfaced.service.in | 25 -
 2 files changed, 29 deletions(-)
 delete mode 100644 src/interface/virtinterfaced.service.in

diff --git a/src/interface/meson.build b/src/interface/meson.build
index 6fa65117c3..54c0b1a935 100644
--- a/src/interface/meson.build
+++ b/src/interface/meson.build
@@ -44,11 +44,7 @@ if conf.has('WITH_INTERFACE')
 
   virt_daemon_units += {
 'service': 'virtinterfaced',
-'service_in': files('virtinterfaced.service.in'),
 'name': 'Libvirt interface',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
   }
 
   openrc_init_files += {
diff --git a/src/interface/virtinterfaced.service.in 
b/src/interface/virtinterfaced.service.in
deleted file mode 100644
index 5cb2cd19dc..00
--- a/src/interface/virtinterfaced.service.in
+++ /dev/null
@@ -1,25 +0,0 @@
-[Unit]
-Description=Virtualization interface daemon
-Conflicts=libvirtd.service
-Requires=virtinterfaced.socket
-Requires=virtinterfaced-ro.socket
-Requires=virtinterfaced-admin.socket
-After=network.target
-After=dbus.service
-After=apparmor.service
-Documentation=man:virtinterfaced(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTINTERFACED_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtinterfaced
-ExecStart=@sbindir@/virtinterfaced $VIRTINTERFACED_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-
-[Install]
-WantedBy=multi-user.target
-Also=virtinterfaced.socket
-Also=virtinterfaced-ro.socket
-Also=virtinterfaced-admin.socket
-- 
2.41.0



[libvirt PATCH v2 15/33] systemd: Switch virtvzd to common templates

2023-09-27 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/vz/meson.build  |  5 +
 src/vz/virtvzd.service.extra.in |  2 ++
 src/vz/virtvzd.service.in   | 26 --
 3 files changed, 3 insertions(+), 30 deletions(-)
 create mode 100644 src/vz/virtvzd.service.extra.in
 delete mode 100644 src/vz/virtvzd.service.in

diff --git a/src/vz/meson.build b/src/vz/meson.build
index 9c2eb90463..842cdb6136 100644
--- a/src/vz/meson.build
+++ b/src/vz/meson.build
@@ -48,11 +48,8 @@ if conf.has('WITH_VZ')
 
   virt_daemon_units += {
 'service': 'virtvzd',
-'service_in': files('virtvzd.service.in'),
 'name': 'Libvirt vz',
-'socket_in': libvirtd_socket_in,
-'socket_ro_in': libvirtd_socket_ro_in,
-'socket_admin_in': libvirtd_socket_admin_in,
+'service_extra_in': files('virtvzd.service.extra.in'),
   }
 
   openrc_init_files += {
diff --git a/src/vz/virtvzd.service.extra.in b/src/vz/virtvzd.service.extra.in
new file mode 100644
index 00..ba3ad13ace
--- /dev/null
+++ b/src/vz/virtvzd.service.extra.in
@@ -0,0 +1,2 @@
+[Unit]
+After=remote-fs.target
diff --git a/src/vz/virtvzd.service.in b/src/vz/virtvzd.service.in
deleted file mode 100644
index 5521e89e10..00
--- a/src/vz/virtvzd.service.in
+++ /dev/null
@@ -1,26 +0,0 @@
-[Unit]
-Description=Virtualization vz daemon
-Conflicts=libvirtd.service
-Requires=virtvzd.socket
-Requires=virtvzd-ro.socket
-Requires=virtvzd-admin.socket
-After=network.target
-After=dbus.service
-After=apparmor.service
-After=remote-fs.target
-Documentation=man:virtvzd(8)
-Documentation=https://libvirt.org
-
-[Service]
-Type=notify
-Environment=VIRTVZD_ARGS="--timeout 120"
-EnvironmentFile=-@initconfdir@/virtvzd
-ExecStart=@sbindir@/virtvzd $VIRTVZD_ARGS
-ExecReload=/bin/kill -HUP $MAINPID
-Restart=on-failure
-
-[Install]
-WantedBy=multi-user.target
-Also=virtvzd.socket
-Also=virtvzd-ro.socket
-Also=virtvzd-admin.socket
-- 
2.41.0



Re: [libvirt PATCH v2] util: fix success return for virProcessKillPainfullyDelay()

2023-09-26 Thread Ján Tomko

On a Monday in 2023, Jonathon Jongsma wrote:

virProcessKillPainfullyDelay() currently almost always returns 1 or -1,
even though the documentation indicates that it should return 0 if the
process was terminated gracefully. But the computation of the return
code is faulty and the only case that it currently returns 0 is when it
is called with the pid of a process that does not exist.

Since no callers ever even distinguish between the 0 and 1 response
codes, simply get rid of the distinction and return 0 for both cases.

Signed-off-by: Jonathon Jongsma 
---

Change in v2:
- just drop the distinction between 0 and 1 and always return 0.
  Suggested by Ján Tomko

src/util/virprocess.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


[libvirt PATCH v2] util: fix success return for virProcessKillPainfullyDelay()

2023-09-25 Thread Jonathon Jongsma
virProcessKillPainfullyDelay() currently almost always returns 1 or -1,
even though the documentation indicates that it should return 0 if the
process was terminated gracefully. But the computation of the return
code is faulty and the only case that it currently returns 0 is when it
is called with the pid of a process that does not exist.

Since no callers ever even distinguish between the 0 and 1 response
codes, simply get rid of the distinction and return 0 for both cases.

Signed-off-by: Jonathon Jongsma 
---

Change in v2:
 - just drop the distinction between 0 and 1 and always return 0.
   Suggested by Ján Tomko

 src/util/virprocess.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 6ce5ef99a9..b6fb17db83 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -363,9 +363,8 @@ pid_t virProcessGroupGet(pid_t pid)
 /*
  * Try to kill the process and verify it has exited
  *
- * Returns 0 if it was killed gracefully, 1 if it
- * was killed forcibly, -1 if it is still alive,
- * or another error occurred.
+ * Returns 0 if it was killed, -1 if it is still alive or another error
+ * occurred.
  *
  * Callers can provide an extra delay in seconds to
  * wait longer than the default.
@@ -426,7 +425,7 @@ virProcessKillPainfullyDelay(pid_t pid, bool force, 
unsigned int extradelay, boo
  (long long)pid, signame);
 return -1;
 }
-return signum == SIGTERM ? 0 : 1;
+return 0;
 }
 
 g_usleep(200 * 1000);
-- 
2.41.0



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-13 Thread Erik Skultety
On Wed, Sep 13, 2023 at 12:57:09PM +0200, Erik Skultety wrote:
> On Wed, Sep 13, 2023 at 08:56:35AM +0100, Daniel P. Berrangé wrote:
> > On Tue, Sep 12, 2023 at 04:11:01PM -0500, Jonathon Jongsma wrote:
> > > On 9/12/23 7:00 AM, Peter Krempa wrote:
> > > > On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> > > > > see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> > > > > 
> > > > > Changes in v2:
> > > > >   - Don't use virStorageSource->path for vdpa device path to avoid 
> > > > > clashing with
> > > > > existing path functionality
> > > > >   - Move vdpa device opening to the 
> > > > > qemuProcessPrepareHostStorageSource()
> > > > > function rather than the qemuDomainPrepareStorageSource() 
> > > > > function. This
> > > > > also required some additional support in the tests for setting up 
> > > > > the
> > > > > objects properly for testing.
> > > > >   - rebased to latest master branch
> > > > 
> > > > Reviewed-by: Peter Krempa 
> > > > 
> > > 
> > > I pushed this series, but for some reason only the ubuntu images are 
> > > failing
> > > CI with no useful output:
> > > https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836
> > 
> > This is a behavioural regression from the recent CI refactoring of Eriks'.
> > 
> > We have lost the "--no-suite syntax-check --print-errorlogs" arguments
> > when running tests.
> 
> Sigh. I'm already running a pipeline with a fix that adds those test options
> back.
> 
> Erik

How about this one? 
https://listman.redhat.com/archives/libvir-list/2023-September/242071.html

Pipeline: https://gitlab.com/eskultety/libvirt/-/pipelines/1002436800

Erik



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-13 Thread Erik Skultety
On Wed, Sep 13, 2023 at 08:56:35AM +0100, Daniel P. Berrangé wrote:
> On Tue, Sep 12, 2023 at 04:11:01PM -0500, Jonathon Jongsma wrote:
> > On 9/12/23 7:00 AM, Peter Krempa wrote:
> > > On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> > > > see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> > > > 
> > > > Changes in v2:
> > > >   - Don't use virStorageSource->path for vdpa device path to avoid 
> > > > clashing with
> > > > existing path functionality
> > > >   - Move vdpa device opening to the 
> > > > qemuProcessPrepareHostStorageSource()
> > > > function rather than the qemuDomainPrepareStorageSource() function. 
> > > > This
> > > > also required some additional support in the tests for setting up 
> > > > the
> > > > objects properly for testing.
> > > >   - rebased to latest master branch
> > > 
> > > Reviewed-by: Peter Krempa 
> > > 
> > 
> > I pushed this series, but for some reason only the ubuntu images are failing
> > CI with no useful output:
> > https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836
> 
> This is a behavioural regression from the recent CI refactoring of Eriks'.
> 
> We have lost the "--no-suite syntax-check --print-errorlogs" arguments
> when running tests.

Sigh. I'm already running a pipeline with a fix that adds those test options
back.

Erik



Re: [libvirt PATCH v2 18/35] .gitlab-ci.yml: Convert the native build job to the build.sh usage

2023-09-13 Thread Daniel P . Berrangé
On Mon, Sep 11, 2023 at 03:43:19PM +0200, Erik Skultety wrote:
> Individual shell command executions are replaced by respective
> functions in the ci/build.sh base script. This will make sure we use
> the same recipes in GitLab jobs as well as in local executions.
> 
> Signed-off-by: Erik Skultety 
> Reviewed-by: Daniel P. Berrangé 
> Erik Skultety :
> ---
>  .gitlab-ci.yml | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 1c6af8f8b3..c837812091 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -25,15 +25,13 @@ include:
>- ccache/
>  key: "$CI_JOB_NAME"
>script:
> -- *script_variables
> -- meson setup build --werror $MESON_ARGS || (cat 
> build/meson-logs/meson-log.txt && exit 1)
> -- meson dist -C build --no-tests
> +- source ci/jobs.sh
>  - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip";
>then
> -rpmbuild --clean --nodeps --define "_without_mingw 1" -ta 
> build/meson-dist/libvirt-*.tar.xz;
> +run_rpmbuild;
>else
> -meson compile -C build;
> -meson test -C build --no-suite syntax-check --print-errorlogs;
> +run_build;
> +run_test;

I missed a regression here - we're loosing the --no-suite and
--print-errorlogs args when running tests, so we can no longer
diagnose the failures.

>fi
>after_script:
>  - test "$CI_JOB_STATUS" != "success" && exit 1;
> -- 
> 2.41.0
> 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-13 Thread Daniel P . Berrangé
On Tue, Sep 12, 2023 at 04:11:01PM -0500, Jonathon Jongsma wrote:
> On 9/12/23 7:00 AM, Peter Krempa wrote:
> > On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> > > see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> > > 
> > > Changes in v2:
> > >   - Don't use virStorageSource->path for vdpa device path to avoid 
> > > clashing with
> > > existing path functionality
> > >   - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
> > > function rather than the qemuDomainPrepareStorageSource() function. 
> > > This
> > > also required some additional support in the tests for setting up the
> > > objects properly for testing.
> > >   - rebased to latest master branch
> > 
> > Reviewed-by: Peter Krempa 
> > 
> 
> I pushed this series, but for some reason only the ubuntu images are failing
> CI with no useful output:
> https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836

This is a behavioural regression from the recent CI refactoring of Eriks'.

We have lost the "--no-suite syntax-check --print-errorlogs" arguments
when running tests.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-13 Thread Peter Krempa
On Tue, Sep 12, 2023 at 16:11:01 -0500, Jonathon Jongsma wrote:
> On 9/12/23 7:00 AM, Peter Krempa wrote:
> > On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> > > see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> > > 
> > > Changes in v2:
> > >   - Don't use virStorageSource->path for vdpa device path to avoid 
> > > clashing with
> > > existing path functionality
> > >   - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
> > > function rather than the qemuDomainPrepareStorageSource() function. 
> > > This
> > > also required some additional support in the tests for setting up the
> > > objects properly for testing.
> > >   - rebased to latest master branch
> > 
> > Reviewed-by: Peter Krempa 
> > 
> 
> I pushed this series, but for some reason only the ubuntu images are failing
> CI with no useful output:
> https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836
> 
> I suspect it may be related to address sanitizer stuff, does anybody have
> tips on getting more information about this failure?

Usually valgrind does a good job of finding everyhting the sanitizer
complains about:

 $ valgrind --trace-children=yes --leak-check=full ./tests/qemuxml2argvtest

[...]

==352693== 18 bytes in 1 blocks are definitely lost in loss record 257 of 684
==352693==at 0x484182F: malloc (vg_replace_malloc.c:431)
==352693==by 0x51DA07F: xmlStrndup (in /usr/lib64/libxml2.so.2.10.4)
==352693==by 0x49D918E: virXMLPropStringRequired (virxml.c:321)
==352693==by 0x4A0D866: virDomainStorageSourceParse (domain_conf.c:7526)
==352693==by 0x4A0E9F0: virDomainDiskDefParseSourceXML (domain_conf.c:7926)
==352693==by 0x4A0ECCF: virDomainDiskDefParseXML (domain_conf.c:8000)
==352693==by 0x4A2DA93: virDomainDefParseXML (domain_conf.c:18767)
==352693==by 0x4A31678: virDomainDefParseNode (domain_conf.c:19557)
==352693==by 0x134000: testCompareXMLToArgv (qemuxml2argvtest.c:566)
==352693==by 0x1356B9: virTestRun (testutils.c:143)
==352693==by 0x135940: virTestRunLog (testutils.c:198)
==352693==by 0x11D422: mymain (qemuxml2argvtest.c:1164)


Looks like the new field is not freed. I'll post the patch soon



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-12 Thread Jonathon Jongsma

On 9/12/23 7:00 AM, Peter Krempa wrote:

On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:

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

Changes in v2:
  - Don't use virStorageSource->path for vdpa device path to avoid clashing with
existing path functionality
  - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
function rather than the qemuDomainPrepareStorageSource() function. This
also required some additional support in the tests for setting up the
objects properly for testing.
  - rebased to latest master branch


Reviewed-by: Peter Krempa 



I pushed this series, but for some reason only the ubuntu images are 
failing CI with no useful output: 
https://gitlab.com/libvirt/libvirt/-/pipelines/1001459836


I suspect it may be related to address sanitizer stuff, does anybody 
have tips on getting more information about this failure?


Jonathon



Re: [libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-12 Thread Peter Krempa
On Mon, Sep 11, 2023 at 16:53:42 -0500, Jonathon Jongsma wrote:
> see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.
> 
> Changes in v2:
>  - Don't use virStorageSource->path for vdpa device path to avoid clashing 
> with
>existing path functionality
>  - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
>function rather than the qemuDomainPrepareStorageSource() function. This
>also required some additional support in the tests for setting up the
>objects properly for testing.
>  - rebased to latest master branch

Reviewed-by: Peter Krempa 



[libvirt PATCH v2 2/5] qemu: add virtio-blk-vhost-vdpa capability

2023-09-11 Thread Jonathon Jongsma
Check whether the qemu binary supports the vdpa block driver. We can't
rely simply on the existence of the virtio-blk-vhost-vdpa block driver
since the first releases of qemu didn't support fd-passing for this
driver. So we have to check for the 'fdset' feature on the driver
object. This feature will be present in the qemu 8.1.0 release and was
merged to qemu in commit 98b126f5.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 87412dd4ec..3a1bfbf74d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -697,6 +697,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
 
   /* 450 */
   "run-with.async-teardown", /* QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN 
*/
+  "virtio-blk-vhost-vdpa", /* 
QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA */
 );
 
 
@@ -1531,6 +1532,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+rbd/encrypt/format/^luks-any", 
QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY },
 { "blockdev-add/arg-type/+nbd/tls-hostname", 
QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME },
 { "blockdev-add/arg-type/+qcow2/discard-no-unref", 
QEMU_CAPS_QCOW2_DISCARD_NO_UNREF },
+{ "blockdev-add/arg-type/+virtio-blk-vhost-vdpa/$fdset", 
QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA},
 { "blockdev-snapshot/$allow-write-only-overlay", 
QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY },
 { "chardev-add/arg-type/backend/+socket/data/reconnect", 
QEMU_CAPS_CHARDEV_RECONNECT },
 { "device_add/$json-cli-hotplug", QEMU_CAPS_DEVICE_JSON },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e51d3fffdc..3c4f7f625b 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -676,6 +676,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 
 /* 450 */
 QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN, /* asynchronous teardown -run-with 
async-teardown=on|off */
+QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA, /* virtio-blk-vhost-vdpa block 
driver */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml 
b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml
index 6f8c5a57b7..d266dd0f31 100644
--- a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml
@@ -197,6 +197,7 @@
   
   
   
+  
   8001000
   43100245
   v8.1.0
-- 
2.41.0



[libvirt PATCH v2 3/5] qemu: make vdpa connect function more generic

2023-09-11 Thread Jonathon Jongsma
qemuInterfaceVDPAConnect() was a helper function for connecting to the
vdpa device file. But in order to support other vdpa devices besides
network interfaces (e.g. vdpa block devices) make this function a bit
more generic.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_command.c   | 23 ++-
 src/qemu/qemu_command.h   |  1 +
 src/qemu/qemu_interface.c | 23 ---
 src/qemu/qemu_interface.h |  2 --
 tests/qemuhotplugmock.c   |  4 ++--
 tests/qemuxml2argvmock.c  |  2 +-
 6 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 778958700b..e84374b4cf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8533,7 +8533,7 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
 break;
 
 case VIR_DOMAIN_NET_TYPE_VDPA:
-if ((vdpafd = qemuInterfaceVDPAConnect(net)) < 0)
+if ((vdpafd = qemuVDPAConnect(net->data.vdpa.devicepath)) < 0)
 return -1;
 
 netpriv->vdpafd = qemuFDPassNew(net->info.alias, priv);
@@ -10993,3 +10993,24 @@ 
qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSource *top,
 
 return g_steal_pointer();
 }
+
+
+/* qemuVDPAConnect:
+ * @devicepath: the path to the vdpa device
+ *
+ * returns: file descriptor of the vdpa device
+ */
+int
+qemuVDPAConnect(const char *devicepath)
+{
+int fd;
+
+if ((fd = open(devicepath, O_RDWR)) < 0) {
+virReportSystemError(errno,
+ _("Unable to open '%1$s' for vdpa device"),
+ devicepath);
+return -1;
+}
+
+return fd;
+}
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 55efa45601..341ec43f9a 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -248,3 +248,4 @@ qemuBuildTPMOpenBackendFDs(const char *tpmdev,
 
 const char * qemuAudioDriverTypeToString(virDomainAudioType type);
 virDomainAudioType qemuAudioDriverTypeFromString(const char *str);
+int qemuVDPAConnect(const char *devicepath) G_NO_INLINE;
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index e875de48ee..8856bb95a8 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -648,29 +648,6 @@ qemuInterfaceBridgeConnect(virDomainDef *def,
 }
 
 
-/* qemuInterfaceVDPAConnect:
- * @net: pointer to the VM's interface description
- *
- * returns: file descriptor of the vdpa device
- *
- * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_VDPA
- */
-int
-qemuInterfaceVDPAConnect(virDomainNetDef *net)
-{
-int fd;
-
-if ((fd = open(net->data.vdpa.devicepath, O_RDWR)) < 0) {
-virReportSystemError(errno,
- _("Unable to open '%1$s' for vdpa device"),
- net->data.vdpa.devicepath);
-return -1;
-}
-
-return fd;
-}
-
-
 /*
  * Returns: -1 on error, 0 on success. Populates net->privateData->slirp if
  * the slirp helper is needed.
diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h
index d866beb184..6eed3e6bd7 100644
--- a/src/qemu/qemu_interface.h
+++ b/src/qemu/qemu_interface.h
@@ -55,5 +55,3 @@ int qemuInterfaceOpenVhostNet(virDomainObj *def,
 
 int qemuInterfacePrepareSlirp(virQEMUDriver *driver,
   virDomainNetDef *net);
-
-int qemuInterfaceVDPAConnect(virDomainNetDef *net) G_NO_INLINE;
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index 89d287945a..dd7e2c67e0 100644
--- a/tests/qemuhotplugmock.c
+++ b/tests/qemuhotplugmock.c
@@ -18,8 +18,8 @@
 
 #include 
 
+#include "qemu/qemu_command.h"
 #include "qemu/qemu_hotplug.h"
-#include "qemu/qemu_interface.h"
 #include "qemu/qemu_process.h"
 #include "testutilsqemu.h"
 #include "conf/domain_conf.h"
@@ -94,7 +94,7 @@ qemuProcessKillManagedPRDaemon(virDomainObj *vm G_GNUC_UNUSED)
 }
 
 int
-qemuInterfaceVDPAConnect(virDomainNetDef *net G_GNUC_UNUSED)
+qemuVDPAConnect(const char *devicepath G_GNUC_UNUSED)
 {
 /* need a valid fd or sendmsg won't work. Just open /dev/null */
 return open("/dev/null", O_RDONLY);
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 400dd5c020..52c44b2ed0 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -255,7 +255,7 @@ virNetDevBandwidthSetRootQDisc(const char *ifname 
G_GNUC_UNUSED,
 
 
 int
-qemuInterfaceVDPAConnect(virDomainNetDef *net G_GNUC_UNUSED)
+qemuVDPAConnect(const char *devicepath G_GNUC_UNUSED)
 {
 if (fcntl(1732, F_GETFD) != -1)
 abort();
-- 
2.41.0



[libvirt PATCH v2 4/5] qemu: consider vdpa block devices for memlock limits

2023-09-11 Thread Jonathon Jongsma
vDPA block devices will also need the same consideration for memlock
limits as other vdpa devices, so consider these devices when calculating
memlock limits.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c7d64e1b5c..52ea8f649d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9652,7 +9652,7 @@ qemuDomainGetNumNVMeDisks(const virDomainDef *def)
 
 
 static int
-qemuDomainGetNumVDPANetDevices(const virDomainDef *def)
+qemuDomainGetNumVDPADevices(const virDomainDef *def)
 {
 size_t i;
 int n = 0;
@@ -9662,6 +9662,14 @@ qemuDomainGetNumVDPANetDevices(const virDomainDef *def)
 n++;
 }
 
+for (i = 0; i < def->ndisks; i++) {
+virStorageSource *src;
+for (src = def->disks[i]->src; src; src = src->backingStore) {
+if (src->type == VIR_STORAGE_TYPE_VHOST_VDPA)
+n++;
+}
+}
+
 return n;
 }
 
@@ -9704,7 +9712,7 @@ qemuDomainGetMemLockLimitBytes(virDomainDef *def)
 
 nvfio = qemuDomainGetNumVFIOHostdevs(def);
 nnvme = qemuDomainGetNumNVMeDisks(def);
-nvdpa = qemuDomainGetNumVDPANetDevices(def);
+nvdpa = qemuDomainGetNumVDPADevices(def);
 /* For device passthrough using VFIO the guest memory and MMIO memory
  * regions need to be locked persistent in order to allow DMA.
  *
-- 
2.41.0



[libvirt PATCH v2 1/5] conf: add ability to configure a vdpa block disk device

2023-09-11 Thread Jonathon Jongsma
vDPA block devices can be configured as follows:


  


Signed-off-by: Jonathon Jongsma 
---
 docs/formatdomain.rst | 19 +--
 src/ch/ch_monitor.c   |  1 +
 src/conf/domain_conf.c|  8 
 src/conf/schemas/domaincommon.rng | 13 +
 src/conf/storage_source_conf.c|  7 ++-
 src/conf/storage_source_conf.h|  2 ++
 src/libxl/xen_xl.c|  1 +
 src/qemu/qemu_block.c |  6 ++
 src/qemu/qemu_command.c   |  1 +
 src/qemu/qemu_migration.c |  2 ++
 src/qemu/qemu_snapshot.c  |  4 
 src/qemu/qemu_validate.c  |  1 +
 src/storage_file/storage_source.c |  1 +
 13 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index bc469e5f9f..a65edc6703 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2678,6 +2678,11 @@ paravirtualized driver is specified via the ``disk`` 
element.


  
+ 
+   
+   
+   
+ 

...
 
@@ -2688,8 +2693,9 @@ paravirtualized driver is specified via the ``disk`` 
element.
``type``
   Valid values are "file", "block", "dir" ( :since:`since 0.7.5` ),
   "network" ( :since:`since 0.8.7` ), or "volume" ( :since:`since 1.0.5` ),
-  or "nvme" ( :since:`since 6.0.0` ), or "vhostuser" ( :since:`since 
7.1.0` )
-  and refer to the underlying source for the disk. :since:`Since 0.0.3`
+  or "nvme" ( :since:`since 6.0.0` ), or "vhostuser" ( :since:`since 
7.1.0` ),
+  or "vhostvdpa" ( :since:`since 9.8.0 (QEMU 8.1.0)`) and refer to the
+  underlying source for the disk. :since:`Since 0.0.3`
``device``
   Indicates how the disk is to be exposed to the guest OS. Possible values
   for this attribute are "floppy", "disk", "cdrom", and "lun", defaulting 
to
@@ -2879,6 +2885,15 @@ paravirtualized driver is specified via the ``disk`` 
element.
    XML for this disk type. Additionally features such as 
blockjobs,
   incremental backups and snapshots are not supported for this disk type.
 
+   ``vhostvdpa``
+  Enables the hypervisor to connect to a vDPA block device. Requires shared
+  memory configured for the VM, for more details see ``access`` mode for
+  ``memoryBacking`` element (See `Memory Backing`_).
+
+  The ``source`` element has a mandatory attribute ``dev`` that specifies
+  the fully-qualified path to the vhost-vdpa character device (e.g.
+  ``/dev/vhost-vdpa-0``).
+
With "file", "block", and "volume", one or more optional sub-elements
``seclabel`` (See `Security label`_) can be used to override the domain
security labeling policy for just that source file.
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 200ad6c77b..1691a4efb6 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -197,6 +197,7 @@ virCHMonitorBuildDiskJson(virJSONValue *disks, 
virDomainDiskDef *diskdef)
 case VIR_STORAGE_TYPE_VOLUME:
 case VIR_STORAGE_TYPE_NVME:
 case VIR_STORAGE_TYPE_VHOST_USER:
+case VIR_STORAGE_TYPE_VHOST_VDPA:
 case VIR_STORAGE_TYPE_LAST:
 default:
 virReportEnumRangeError(virStorageType, diskdef->src->type);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2c8727de54..1f14ef6f23 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7522,6 +7522,10 @@ virDomainStorageSourceParse(xmlNodePtr node,
 if (virDomainDiskSourceVHostUserParse(node, src, xmlopt, ctxt) < 0)
 return -1;
 break;
+case VIR_STORAGE_TYPE_VHOST_VDPA:
+if (!(src->vdpadev = virXMLPropStringRequired(node, "dev")))
+return -1;
+break;
 case VIR_STORAGE_TYPE_NONE:
 case VIR_STORAGE_TYPE_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -22386,6 +22390,10 @@ virDomainDiskSourceFormat(virBuffer *buf,
 virDomainDiskSourceVhostuserFormat(, , 
src->vhostuser);
 break;
 
+case VIR_STORAGE_TYPE_VHOST_VDPA:
+virBufferEscapeString(, " dev='%s'", src->vdpadev);
+break;
+
 case VIR_STORAGE_TYPE_NONE:
 case VIR_STORAGE_TYPE_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/conf/schemas/domaincommon.rng 
b/src/conf/schemas/domaincommon.rng
index 2f9ba31c0a..1fe9ccb70e 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -1811,6 +1811,7 @@
   
   
   
+  
 
   
 
@@ -2381,6 +2382,18 @@
 
   
 
+  
+
+  vhostvdpa
+
+
+  
+
+  
+  
+
+  
+
   
 
   (ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+
diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index dcac3a8ff6..f57cb3241d 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -47,7 +47,8 @@ VIR_ENUM_IMPL(virStorage,
   "network",
   "volume",
   "nvme",

[libvirt PATCH v2 5/5] qemu: Implement support for vDPA block devices

2023-09-11 Thread Jonathon Jongsma
Requires recent qemu with support for the virtio-blk-vhost-vdpa device
and the ability to pass a /dev/fdset/N path for the vdpa path (8.1.0)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1900770
Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_block.c | 20 --
 src/qemu/qemu_process.c   | 34 +
 src/qemu/qemu_validate.c  | 32 ++--
 .../disk-vhostvdpa.x86_64-latest.args | 37 +++
 tests/qemuxml2argvdata/disk-vhostvdpa.xml | 21 +++
 tests/qemuxml2argvtest.c  | 34 +
 tests/testutilsqemu.c | 11 ++
 tests/testutilsqemu.h |  2 +
 8 files changed, 185 insertions(+), 6 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.xml

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0b4c2dbcf4..d31bbde0f4 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -778,6 +778,20 @@ qemuBlockStorageSourceGetNVMeProps(virStorageSource *src)
 }
 
 
+static virJSONValue *
+qemuBlockStorageSourceGetVhostVdpaProps(virStorageSource *src)
+{
+virJSONValue *ret = NULL;
+qemuDomainStorageSourcePrivate *srcpriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+
+ignore_value(virJSONValueObjectAdd(,
+   "s:driver", "virtio-blk-vhost-vdpa",
+   "s:path", 
qemuFDPassGetPath(srcpriv->fdpass),
+   NULL));
+return ret;
+}
+
+
 static int
 qemuBlockStorageSourceGetBlockdevGetCacheProps(virStorageSource *src,
virJSONValue *props)
@@ -874,9 +888,9 @@ qemuBlockStorageSourceGetBackendProps(virStorageSource *src,
 break;
 
 case VIR_STORAGE_TYPE_VHOST_VDPA:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("vhostvdpa disk type not yet supported"));
-return NULL;
+if (!(fileprops = qemuBlockStorageSourceGetVhostVdpaProps(src)))
+return NULL;
+break;
 
 case VIR_STORAGE_TYPE_VHOST_USER:
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7a1cdb0302..42837c4a8a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6820,6 +6820,28 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj 
*vm)
 }
 
 
+static int
+qemuProcessPrepareHostStorageSourceVDPA(virStorageSource *src,
+qemuDomainObjPrivate *priv)
+{
+qemuDomainStorageSourcePrivate *srcpriv = NULL;
+virStorageType actualType = virStorageSourceGetActualType(src);
+int vdpafd = -1;
+
+if (actualType != VIR_STORAGE_TYPE_VHOST_VDPA)
+return 0;
+
+if ((vdpafd = qemuVDPAConnect(src->vdpadev)) < 0)
+return -1;
+
+srcpriv = qemuDomainStorageSourcePrivateFetch(src);
+
+srcpriv->fdpass = qemuFDPassNew(src->nodestorage, priv);
+qemuFDPassAddFD(srcpriv->fdpass, , "-vdpa");
+return 0;
+}
+
+
 static int
 qemuProcessPrepareHostStorage(virQEMUDriver *driver,
   virDomainObj *vm,
@@ -6856,6 +6878,18 @@ qemuProcessPrepareHostStorage(virQEMUDriver *driver,
 return -1;
 }
 
+/* connect to any necessary vdpa block devices */
+for (i = vm->def->ndisks; i > 0; i--) {
+size_t idx = i - 1;
+virDomainDiskDef *disk = vm->def->disks[idx];
+virStorageSource *src;
+
+for (src = disk->src; virStorageSourceIsBacking(src); src = 
src->backingStore) {
+if (qemuProcessPrepareHostStorageSourceVDPA(src, vm->privateData) 
< 0)
+return -1;
+}
+}
+
 return 0;
 }
 
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 165ab3a66a..5bae56b00f 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3175,13 +3175,39 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef 
*disk,
 }
 
 if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER) {
+const char *vhosttype = virStorageTypeToString(disk->src->type);
+
 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_BLK)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("vhostuser disk is not supported with this QEMU 
binary"));
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("%1$s disk is not supported with this QEMU 
binary"),
+   vhosttype);
+return -1;
+}
+
+if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, vhosttype) 
< 0)
+return -1;
+}
+
+if (disk->src->type == VIR_STORAGE_TYPE_VHOST_VDPA) {
+const char *vhosttype = virStorageTypeToString(disk->src->type);
+
+

[libvirt PATCH v2 0/5] Add support for vDPA block devices

2023-09-11 Thread Jonathon Jongsma
see https://bugzilla.redhat.com/show_bug.cgi?id=1900770.

Changes in v2:
 - Don't use virStorageSource->path for vdpa device path to avoid clashing with
   existing path functionality
 - Move vdpa device opening to the qemuProcessPrepareHostStorageSource()
   function rather than the qemuDomainPrepareStorageSource() function. This
   also required some additional support in the tests for setting up the
   objects properly for testing.
 - rebased to latest master branch

Jonathon Jongsma (5):
  conf: add ability to configure a vdpa block disk device
  qemu: add virtio-blk-vhost-vdpa capability
  qemu: make vdpa connect function more generic
  qemu: consider vdpa block devices for memlock limits
  qemu: Implement support for vDPA block devices

 docs/formatdomain.rst | 19 +-
 src/ch/ch_monitor.c   |  1 +
 src/conf/domain_conf.c|  8 
 src/conf/schemas/domaincommon.rng | 13 +++
 src/conf/storage_source_conf.c|  7 +++-
 src/conf/storage_source_conf.h|  2 +
 src/libxl/xen_xl.c|  1 +
 src/qemu/qemu_block.c | 20 ++
 src/qemu/qemu_capabilities.c  |  2 +
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_command.c   | 24 +++-
 src/qemu/qemu_command.h   |  1 +
 src/qemu/qemu_domain.c| 12 +-
 src/qemu/qemu_interface.c | 23 
 src/qemu/qemu_interface.h |  2 -
 src/qemu/qemu_migration.c |  2 +
 src/qemu/qemu_process.c   | 34 +
 src/qemu/qemu_snapshot.c  |  4 ++
 src/qemu/qemu_validate.c  | 33 +++--
 src/storage_file/storage_source.c |  1 +
 .../caps_8.1.0_x86_64.xml |  1 +
 tests/qemuhotplugmock.c   |  4 +-
 .../disk-vhostvdpa.x86_64-latest.args | 37 +++
 tests/qemuxml2argvdata/disk-vhostvdpa.xml | 21 +++
 tests/qemuxml2argvmock.c  |  2 +-
 tests/qemuxml2argvtest.c  | 34 +
 tests/testutilsqemu.c | 11 ++
 tests/testutilsqemu.h |  2 +
 28 files changed, 285 insertions(+), 37 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.xml

-- 
2.41.0



Re: [libvirt PATCH v2 10/35] ci: build.sh: Add a wrapper function over the 'potfile' job

2023-09-11 Thread Daniel P . Berrangé
On Mon, Sep 11, 2023 at 03:43:11PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart. There's one notable difference such that we pass '-j1' to
> the meson compile command otherwise we'd have to execute the 'run_build'
> function twice, passing 'libvirt-pot-dep' and 'libvirt-pot' targets
> in a serial manner.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 11 +++
>  1 file changed, 11 insertions(+)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH v2 13/35] ci: build.sh: Drop changing working directory to CI_CONT_DIR

2023-09-11 Thread Daniel P . Berrangé
On Mon, Sep 11, 2023 at 03:43:14PM +0200, Erik Skultety wrote:
> Firstly, this would mangle with "sourcing" this file in either
> execution environment later down the road. Secondly, we won't need this
> as future ci/helper patches will generate a throwaway script that will
> take care of a correct execution of a build job in a similar fashion as
> if the job ran in a GitLab environment.
> 
> Signed-off-by: Erik Skultety 
> Erik Skultety :
> ---
>  ci/build.sh | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



  1   2   3   4   5   6   7   8   9   10   >