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

Andrea Bolognani posted 33 patches 2 years, 4 months ago
[libvirt PATCH v2 06/33] systemd: Introduce common templates
Posted by Andrea Bolognani 2 years, 4 months ago
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 <abologna@redhat.com>
---
 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 0000000000..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': unit.get('sockprefix', unit['service']),
         'deps': unit.get('deps', ''),
         'sockmode': sockmode,
@@ -825,6 +828,15 @@ if conf.has('WITH_LIBVIRTD')
       service_in = unit.get('service_in', service_in_default)
       service_out = '@0@.service'.format(unit['service'])
 
+      if 'service_extra_in' in unit
+        service_in = configure_file(
+          input: [ service_in, unit['service_extra_in'] ],
+          output: '@0@.in'.format(service_out),
+          command: [ merge_systemd_units_prog, '@INPUT0@', '@INPUT1@' ],
+          capture: true,
+        )
+      endif
+
       configure_file(
         input: service_in,
         output: service_out,
@@ -843,6 +855,16 @@ if conf.has('WITH_LIBVIRTD')
           socket_in = unit.get('socket_@0@_in'.format(socket), socket_in_default)
           socket_out = '@0@-@1@.socket'.format(unit['service'], socket)
         endif
+
+        if 'socket_extra_in' in unit
+          socket_in = configure_file(
+            input: [ socket_in, unit['socket_extra_in'] ],
+            output: '@0@.in'.format(socket_out),
+            command: [ merge_systemd_units_prog, '@INPUT0@', '@INPUT1@' ],
+            capture: true,
+          )
+        endif
+
         configure_file(
           input: socket_in,
           output: socket_out,
diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in
new file mode 100644
index 0000000000..39bb0badea
--- /dev/null
+++ b/src/virtd-admin.socket.in
@@ -0,0 +1,13 @@
+[Unit]
+Description=@name@ admin socket
+Before=@service@.service
+BindsTo=@service@.socket
+After=@service@.socket
+
+[Socket]
+ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock
+Service=@service@.service
+SocketMode=0600
+
+[Install]
+WantedBy=sockets.target
diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in
new file mode 100644
index 0000000000..b7b7ae0dd8
--- /dev/null
+++ b/src/virtd-ro.socket.in
@@ -0,0 +1,13 @@
+[Unit]
+Description=@name@ local read-only socket
+Before=@service@.service
+BindsTo=@service@.socket
+After=@service@.socket
+
+[Socket]
+ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro
+Service=@service@.service
+SocketMode=0666
+
+[Install]
+WantedBy=sockets.target
diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in
new file mode 100644
index 0000000000..7c8bcdb525
--- /dev/null
+++ b/src/virtd-tcp.socket.in
@@ -0,0 +1,12 @@
+[Unit]
+Description=@name@ non-TLS IP socket
+Before=@service@.service
+BindsTo=@service@.socket
+After=@service@.socket
+
+[Socket]
+ListenStream=16509
+Service=@service@.service
+
+[Install]
+WantedBy=sockets.target
diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in
new file mode 100644
index 0000000000..c6dceb2d4e
--- /dev/null
+++ b/src/virtd-tls.socket.in
@@ -0,0 +1,12 @@
+[Unit]
+Description=@name@ TLS IP socket
+Before=@service@.service
+BindsTo=@service@.socket
+After=@service@.socket
+
+[Socket]
+ListenStream=16514
+Service=@service@.service
+
+[Install]
+WantedBy=sockets.target
diff --git a/src/virtd.service.in b/src/virtd.service.in
new file mode 100644
index 0000000000..76f9c60351
--- /dev/null
+++ b/src/virtd.service.in
@@ -0,0 +1,25 @@
+[Unit]
+Description=@name@ daemon
+Conflicts=libvirtd.service
+Requires=@service@.socket
+Requires=@service@-ro.socket
+Requires=@service@-admin.socket
+After=network.target
+After=dbus.service
+After=apparmor.service
+Documentation=man:@service@(8)
+Documentation=https://libvirt.org
+
+[Service]
+Type=notify
+Environment=@SERVICE@_ARGS="--timeout 120"
+EnvironmentFile=-@initconfdir@/@service@
+ExecStart=@sbindir@/@service@ $@SERVICE@_ARGS
+ExecReload=/bin/kill -HUP $MAINPID
+Restart=on-failure
+
+[Install]
+WantedBy=multi-user.target
+Also=@service@.socket
+Also=@service@-ro.socket
+Also=@service@-admin.socket
diff --git a/src/virtd.socket.in b/src/virtd.socket.in
new file mode 100644
index 0000000000..aec0708fd4
--- /dev/null
+++ b/src/virtd.socket.in
@@ -0,0 +1,12 @@
+[Unit]
+Description=@name@ local socket
+Before=@service@.service
+
+[Socket]
+ListenStream=@runstatedir@/libvirt/@sockprefix@-sock
+Service=@service@.service
+SocketMode=@sockmode@
+RemoveOnStop=yes
+
+[Install]
+WantedBy=sockets.target
-- 
2.41.0
Re: [libvirt PATCH v2 06/33] systemd: Introduce common templates
Posted by Daniel P. Berrangé 2 years, 4 months ago
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 <abologna@redhat.com>
> ---
>  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 0000000000..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 (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': unit.get('sockprefix', unit['service']),
>          'deps': unit.get('deps', ''),
>          'sockmode': sockmode,
> @@ -825,6 +828,15 @@ if conf.has('WITH_LIBVIRTD')
>        service_in = unit.get('service_in', service_in_default)
>        service_out = '@0@.service'.format(unit['service'])
>  
> +      if 'service_extra_in' in unit
> +        service_in = configure_file(
> +          input: [ service_in, unit['service_extra_in'] ],
> +          output: '@0@.in'.format(service_out),
> +          command: [ merge_systemd_units_prog, '@INPUT0@', '@INPUT1@' ],
> +          capture: true,
> +        )
> +      endif
> +
>        configure_file(
>          input: service_in,
>          output: service_out,
> @@ -843,6 +855,16 @@ if conf.has('WITH_LIBVIRTD')
>            socket_in = unit.get('socket_@0@_in'.format(socket), socket_in_default)
>            socket_out = '@0@-@1@.socket'.format(unit['service'], socket)
>          endif
> +
> +        if 'socket_extra_in' in unit
> +          socket_in = configure_file(
> +            input: [ socket_in, unit['socket_extra_in'] ],
> +            output: '@0@.in'.format(socket_out),
> +            command: [ merge_systemd_units_prog, '@INPUT0@', '@INPUT1@' ],
> +            capture: true,
> +          )
> +        endif
> +
>          configure_file(
>            input: socket_in,
>            output: socket_out,
> diff --git a/src/virtd-admin.socket.in b/src/virtd-admin.socket.in
> new file mode 100644
> index 0000000000..39bb0badea
> --- /dev/null
> +++ b/src/virtd-admin.socket.in
> @@ -0,0 +1,13 @@
> +[Unit]
> +Description=@name@ admin socket
> +Before=@service@.service
> +BindsTo=@service@.socket
> +After=@service@.socket
> +
> +[Socket]
> +ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock
> +Service=@service@.service
> +SocketMode=0600
> +
> +[Install]
> +WantedBy=sockets.target
> diff --git a/src/virtd-ro.socket.in b/src/virtd-ro.socket.in
> new file mode 100644
> index 0000000000..b7b7ae0dd8
> --- /dev/null
> +++ b/src/virtd-ro.socket.in
> @@ -0,0 +1,13 @@
> +[Unit]
> +Description=@name@ local read-only socket
> +Before=@service@.service
> +BindsTo=@service@.socket
> +After=@service@.socket
> +
> +[Socket]
> +ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro
> +Service=@service@.service
> +SocketMode=0666
> +
> +[Install]
> +WantedBy=sockets.target
> diff --git a/src/virtd-tcp.socket.in b/src/virtd-tcp.socket.in
> new file mode 100644
> index 0000000000..7c8bcdb525
> --- /dev/null
> +++ b/src/virtd-tcp.socket.in
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=@name@ non-TLS IP socket
> +Before=@service@.service
> +BindsTo=@service@.socket
> +After=@service@.socket
> +
> +[Socket]
> +ListenStream=16509
> +Service=@service@.service
> +
> +[Install]
> +WantedBy=sockets.target
> diff --git a/src/virtd-tls.socket.in b/src/virtd-tls.socket.in
> new file mode 100644
> index 0000000000..c6dceb2d4e
> --- /dev/null
> +++ b/src/virtd-tls.socket.in
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=@name@ TLS IP socket
> +Before=@service@.service
> +BindsTo=@service@.socket
> +After=@service@.socket
> +
> +[Socket]
> +ListenStream=16514
> +Service=@service@.service
> +
> +[Install]
> +WantedBy=sockets.target
> diff --git a/src/virtd.service.in b/src/virtd.service.in
> new file mode 100644
> index 0000000000..76f9c60351
> --- /dev/null
> +++ b/src/virtd.service.in
> @@ -0,0 +1,25 @@
> +[Unit]
> +Description=@name@ daemon
> +Conflicts=libvirtd.service
> +Requires=@service@.socket
> +Requires=@service@-ro.socket
> +Requires=@service@-admin.socket
> +After=network.target
> +After=dbus.service
> +After=apparmor.service
> +Documentation=man:@service@(8)
> +Documentation=https://libvirt.org
> +
> +[Service]
> +Type=notify
> +Environment=@SERVICE@_ARGS="--timeout 120"
> +EnvironmentFile=-@initconfdir@/@service@
> +ExecStart=@sbindir@/@service@ $@SERVICE@_ARGS
> +ExecReload=/bin/kill -HUP $MAINPID
> +Restart=on-failure
> +
> +[Install]
> +WantedBy=multi-user.target
> +Also=@service@.socket
> +Also=@service@-ro.socket
> +Also=@service@-admin.socket
> diff --git a/src/virtd.socket.in b/src/virtd.socket.in
> new file mode 100644
> index 0000000000..aec0708fd4
> --- /dev/null
> +++ b/src/virtd.socket.in
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=@name@ local socket
> +Before=@service@.service
> +
> +[Socket]
> +ListenStream=@runstatedir@/libvirt/@sockprefix@-sock
> +Service=@service@.service
> +SocketMode=@sockmode@
> +RemoveOnStop=yes
> +
> +[Install]
> +WantedBy=sockets.target
> -- 
> 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 06/33] systemd: Introduce common templates
Posted by Andrea Bolognani 2 years, 4 months ago
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 06/33] systemd: Introduce common templates
Posted by Andrea Bolognani 2 years, 4 months ago
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 06/33] systemd: Introduce common templates
Posted by Daniel P. Berrangé 2 years, 4 months ago
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é <berrange@redhat.com>


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 :|