On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
[...]
>  remote/test_libvirtd.aug: remote/test_libvirtd.aug.in \
>               remote/libvirtd.conf $(AUG_GENTEST)
> -     $(AM_V_GEN)$(AUG_GENTEST) remote/libvirtd.conf $< > $@
> +     $(AM_V_GEN)$(AUG_GENTEST) remote/libvirtd.conf \
> +             $(srcdir)/remote/test_libvirtd.aug.in | \
> +             $(SED) -e '/:: CUT ENABLE_IP ::/d' \
> +             -e '/:: END ::/d' \
> +             -e 's/:: DAEMON_NAME ::/libvirtd/' \
> +             -e 's/:: DAEMON_NAME_UC ::/Libvirtd/' \
> +             > $@ || rm -f $@

The indentation for sed arguments, especially the first one, is
quite awkward here.

[...]
> +++ b/src/remote/libvirtd.aug.in
> @@ -1,6 +1,6 @@
> -(* /etc/libvirt/libvirtd.conf *)
> +(* /etc/libvirt/:: DAEMON_NAME ::.conf *)

This is a pretty obvious example of ":: VARIABLE ::" being inferior
than the existing convention: compare it with the much more readable

  (* /etc/libvirt/@DAEMON_NAME@.conf *)

[...]
> +++ b/src/remote/test_libvirtd.aug.in
> @@ -48,7 +54,7 @@ module Test_libvirtd =
>          { "admin_max_client_requests" = "5" }
>          { "log_level" = "3" }
>          { "log_filters" = "1:qemu 1:libvirt 4:object 4:json 4:event 1:util" }
> -        { "log_outputs" = "3:syslog:libvirtd" }
> +        { "log_outputs" = "3:syslog::: DAEMON_NAME ::" }

And another example right here:

  { "log_outputs" = "3:syslog:@DAEMON_NAME@" }

would be much better.


With the markers used for variable substitution changed,

  Reviewed-by: Andrea Bolognani <abolo...@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to