[libvirt] [PATCH v3 20/48] remote: introduce virtproxyd daemon to handle IP connectivity

Andrea Bolognani abologna at redhat.com
Tue Jul 30 13:41:31 UTC 2019


On Mon, 2019-07-29 at 18:11 +0100, Daniel P. Berrangé wrote:
[...]
> +++ b/src/remote/Makefile.inc.am
> +remote/virtproxyd.conf: remote/libvirtd.conf.in
> +	$(AM_V_GEN)sed \
> +		-e '/[@]CUT_ENABLE_IP[@]/d' \
> +		-e '/[@]END[@]/d' \
> +		-e 's/[@]DAEMON_NAME[@]/virtproxyd/' \
> +		< $^ > $@

Unnecessary input redirection.

[...]
> +remote/virtproxyd.aug: remote/libvirtd.aug.in
> +	$(AM_V_GEN)$(SED) \
> +		-e '/[@]CUT_ENABLE_IP[@]/d' \
> +		-e '/[@]END[@]/d' \
> +		-e 's/[@]DAEMON_NAME[@]/virtproxyd/' \
> +		-e 's/[@]DAEMON_NAME_UC[@]/Virtproxyd/' \
> +		$< > $@

Same here.

[...]
> +remote/test_virtproxyd.aug: remote/test_libvirtd.aug.in \
> +		remote/virtproxyd.conf $(AUG_GENTEST)
> +	$(AM_V_GEN)$(AUG_GENTEST) remote/virtproxyd.conf \
> +		$(srcdir)/remote/test_libvirtd.aug.in | \
> +		$(SED) -e '/[@]CUT_ENABLE_IP[@]/d' \
> +		-e '/[@]END[@]/d' \
> +		-e 's/[@]DAEMON_NAME[@]/virtproxyd/' \
> +		-e 's/[@]DAEMON_NAME_UC[@]/Virtproxyd/' \
> +		> $@ || rm -f $@

Wrong formatting for first argument (should be on its own line).

[...]
>  libvirtd.service: remote/libvirtd.service.in $(top_builddir)/config.status
>  	$(AM_V_GEN)sed $(LIBVIRTD_UNIT_VARS) < $< > $@-t && mv $@-t $@
>  
>  libvirt%.socket: remote/libvirt%.socket.in $(top_builddir)/config.status
> -	$(AM_V_GEN)sed $(LIBVIRTD_UNIT_VARS) < $< > $@-t && mv $@-t $@
> +	$(AM_V_GEN)sed $(LIBVIRTD_UNIT_VARS) \
> +		< $< > $@-t && mv $@-t $@

Unnecessary input redirection and questionable formatting:
$(LIBVIRTD_UNIT_VARS) should be on a separate line.

Also, and I'm sorry I've just noticed this, you're using 'sed'
instead of $(SED).

All three issues trace back to previous patches and should be fixed
there.

> +virtproxyd.service: remote/virtproxyd.service.in $(top_builddir)/config.status
> +	$(AM_V_GEN)sed $(VIRTPROXYD_UNIT_VARS) < $< > $@-t && mv $@-t $@
> +
> +virtproxy%.socket: remote/libvirt%.socket.in $(top_builddir)/config.status
> +	$(AM_V_GEN)sed $(VIRTPROXYD_UNIT_VARS) \
> +		< $< > $@-t && mv $@-t $@

These rules also suffer from all the issues listed above, but they're
introduced by this patch.

[...]
> +++ b/src/remote/remote_daemon_config.c
> @@ -108,7 +108,11 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
> +# ifdef LIBVIRTD
> +    data->listen_tls = 1; /* Only honoured it --listen is set */

s/it --listen/if --listen/

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list