Alas, the shell is not a real programming language. Patch generated by manual confirmation of vim's s/[^"]\@<=\$\S\+\s\@=/"&"/gc and s/\(echo \)\(a)&lt;=[^&quot;].*\$.*$/&quot;&amp;&quot;/c matches. This patch generate a lot of noise and carries little benefits, as I do not really expect $PKI to contain spaces or backticks. I'm just fuming, and would not really mind if this patch is ignored --- tools/virt-pki-validate.in | 64 ++++++++++++++++++++++---------------------- tools/virt-xml-validate.in | 10 +++--- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/tools/virt-pki-validate.in b/tools/virt-pki-validate.in index 96659cf..8a4249d 100755 --- a/tools/virt-pki-validate.in +++ b/tools/virt-pki-validate.in @@ -20,27 +20,27 @@ then echo "make sure the gnutls-utils (or gnutls-bin) package is installed" exit 1 fi -echo Found $CERTOOL +echo Found "$CERTOOL" # # Check the directory structure # SYSCONFDIR="@SYSCONFDIR@" PKI="$SYSCONFDIR/pki" -if [ ! -d $PKI ] +if [ ! -d "$PKI" ] then echo the $PKI directory is missing, it is usually echo installed as part of the filesystem or openssl packages exit 1 fi -if [ ! -r $PKI ] +if [ ! -r "$PKI" ] then echo the $PKI directory is not readable by $USER echo "as root do: chmod a+rx $PKI" exit 1 fi -if [ ! -x $PKI ] +if [ ! -x "$PKI" ] then echo the $PKI directory is not listable by $USER echo "as root do: chmod a+rx $PKI" @@ -48,20 +48,20 @@ then fi CA="$PKI/CA" -if [ ! -d $CA ] +if [ ! -d "$CA" ] then echo the $CA directory is missing, it is usually echo installed as part of the or openssl package exit 1 fi -if [ ! -r $CA ] +if [ ! -r "$CA" ] then echo the $CA directory is not readable by $USER echo "as root do: chmod a+rx $CA" exit 1 fi -if [ ! -x $CA ] +if [ ! -x "$CA" ] then echo the $CA directory is not listable by $USER echo "as root do: chmod a+rx $CA" @@ -69,7 +69,7 @@ then fi LIBVIRT="$PKI/libvirt" -if [ ! -d $LIBVIRT ] +if [ ! -d "$LIBVIRT" ] then echo the $LIBVIRT directory is missing, it is usually echo installed by the libvirt package @@ -77,13 +77,13 @@ then exit 1 fi -if [ ! -r $LIBVIRT ] +if [ ! -r "$LIBVIRT" ] then echo the $LIBVIRT directory is not readable by $USER echo "as root do: chown root:root $LIBVIRT ; chmod 755 $LIBVIRT" exit 1 fi -if [ ! -x $LIBVIRT ] +if [ ! -x "$LIBVIRT" ] then echo the $LIBVIRT directory is not listable by $USER echo "as root do: chown root:root $LIBVIRT ; chmod 755 $LIBVIRT" @@ -91,7 +91,7 @@ then fi LIBVIRTP="$LIBVIRT/private" -if [ ! -d $LIBVIRTP ] +if [ ! -d "$LIBVIRTP" ] then echo the $LIBVIRTP directory is missing, it is usually echo installed by the libvirt package @@ -99,13 +99,13 @@ then exit 1 fi -if [ ! -r $LIBVIRTP ] +if [ ! -r "$LIBVIRTP" ] then echo the $LIBVIRTP directory is not readable by $USER echo "as root do: chown root:root $LIBVIRTP ; chmod 755 $LIBVIRTP" exit 1 fi -if [ ! -x $LIBVIRTP ] +if [ ! -x "$LIBVIRTP" ] then echo the $LIBVIRTP directory is not listable by $USER echo "as root do: chown root:root $LIBVIRTP ; chmod 755 $LIBVIRTP" @@ -116,7 +116,7 @@ fi # Now check the certificates # First the CA certificate # -if [ ! -f $CA/cacert.pem ] +if [ ! -f "$CA/cacert.pem" ] then echo the CA certificate $CA/cacert.pem is missing while it echo should be installed on both client and servers @@ -124,7 +124,7 @@ then echo on how to install it exit 1 fi -if [ ! -r $CA/cacert.pem ] +if [ ! -r "$CA/cacert.pem" ] then echo the CA certificate $CA/cacert.pem is not readable by $USER echo "as root do: chmod 644 $CA/cacert.pem" @@ -135,7 +135,7 @@ sed_get_org='/Issuer:/ { s/,.*// p }' -ORG=`$CERTOOL -i --infile $CA/cacert.pem | sed -n "$sed_get_org"` +ORG=`"$CERTOOL" -i --infile "$CA/cacert.pem" | sed -n "$sed_get_org"` if [ "$ORG" = "" ] then echo the CA certificate $CA/cacert.pem does not define the organization @@ -148,29 +148,29 @@ echo Found CA certificate $CA/cacert.pem for $ORG # Second the client certificates -if [ -f $LIBVIRT/clientcert.pem ] +if [ -f "$LIBVIRT/clientcert.pem" ] then - if [ ! -r $LIBVIRT/clientcert.pem ] + if [ ! -r "$LIBVIRT/clientcert.pem" ] then echo Client certificate $LIBVIRT/clientcert.pem should be world readable echo "as root do: chown root:root $LIBVIRT/clientcert.pem ; chmod 644 $LIBVIRT/clientcert.pem" else - S_ORG=`$CERTOOL -i --infile $LIBVIRT/clientcert.pem | grep Subject: | sed 's+.*O=\([a-zA-Z \._-]*\).*+\1+'` + S_ORG=`"$CERTOOL" -i --infile "$LIBVIRT/clientcert.pem" | grep Subject: | sed 's+.*O=\([a-zA-Z \._-]*\).*+\1+'` if [ "$ORG" != "$S_ORG" ] then echo The CA certificate and the client certificate do not match echo CA organization: $ORG echo Client organization: $S_ORG fi - CLIENT=`$CERTOOL -i --infile $LIBVIRT/clientcert.pem | grep Subject: | sed 's+.*CN=\(.[a-zA-Z \._-]*\).*+\1+'` + CLIENT=`"$CERTOOL" -i --infile "$LIBVIRT/clientcert.pem" | grep Subject: | sed 's+.*CN=\(.[a-zA-Z \._-]*\).*+\1+'` echo Found client certificate $LIBVIRT/clientcert.pem for $CLIENT - if [ ! -e $LIBVIRTP/clientkey.pem ] + if [ ! -e "$LIBVIRTP/clientkey.pem" ] then echo Missing client private key $LIBVIRTP/clientkey.pem else echo Found client private key $LIBVIRTP/clientkey.pem - OWN=`ls -l $LIBVIRTP/clientkey.pem | awk '{ print $3 }'` - MOD=`ls -l $LIBVIRTP/clientkey.pem | awk '{ print $1 }'` + OWN=`ls -l "$LIBVIRTP/clientkey.pem" | awk '{ print $3 }'` + MOD=`ls -l "$LIBVIRTP/clientkey.pem" | awk '{ print $1 }'` if [ "$OWN" != "root" ] then echo The client private key should be owned by root @@ -185,7 +185,7 @@ then fi else - echo Did not found $LIBVIRT/clientcert.pem client certificate + echo Did not found "$LIBVIRT/clientcert.pem" client certificate echo The machine cannot act as a client echo "see http://libvirt.org/remote.html#Remote_TLS_client_certificates" echo on how to regenerate it @@ -194,21 +194,21 @@ fi # Third the server certificates -if [ -f $LIBVIRT/servercert.pem ] +if [ -f "$LIBVIRT/servercert.pem" ] then - if [ ! -r $LIBVIRT/servercert.pem ] + if [ ! -r "$LIBVIRT/servercert.pem" ] then echo Server certificate $LIBVIRT/servercert.pem should be world readable echo "as root do: chown root:root $LIBVIRT/servercert.pem ; chmod 644 $LIBVIRT/servercert.pem" else - S_ORG=`$CERTOOL -i --infile $LIBVIRT/servercert.pem | grep Subject: | sed 's+.*O=\([a-zA-Z\. _-]*\).*+\1+'` + S_ORG=`"$CERTOOL" -i --infile "$LIBVIRT/servercert.pem" | grep Subject: | sed 's+.*O=\([a-zA-Z\. _-]*\).*+\1+'` if [ "$ORG" != "$S_ORG" ] then echo The CA certificate and the server certificate do not match echo CA organization: $ORG echo Server organization: $S_ORG fi - S_HOST=`$CERTOOL -i --infile $LIBVIRT/servercert.pem | grep Subject: | sed 's+.*CN=\([a-zA-Z\. _-]*\)+\1+'` + S_HOST=`"$CERTOOL" -i --infile "$LIBVIRT/servercert.pem" | grep Subject: | sed 's+.*CN=\([a-zA-Z\. _-]*\)+\1+'` if test "$S_HOST" != "`hostname -s`" && test "$S_HOST" != "`hostname`" then echo The server certificate does not seem to match the host name @@ -216,13 +216,13 @@ then echo Server certificate CN: '"'$S_HOST'"' fi echo Found server certificate $LIBVIRT/servercert.pem for $S_HOST - if [ ! -e $LIBVIRTP/serverkey.pem ] + if [ ! -e "$LIBVIRTP/serverkey.pem" ] then echo Missing server private key $LIBVIRTP/serverkey.pem else echo Found server private key $LIBVIRTP/serverkey.pem - OWN=`ls -l $LIBVIRTP/serverkey.pem | awk '{ print $3 }'` - MOD=`ls -l $LIBVIRTP/serverkey.pem | awk '{ print $1 }'` + OWN=`ls -l "$LIBVIRTP/serverkey.pem" | awk '{ print $3 }'` + MOD=`ls -l "$LIBVIRTP/serverkey.pem" | awk '{ print $1 }'` if [ "$OWN" != "root" ] then echo The server private key should be owned by root @@ -259,7 +259,7 @@ then fi if [ -r "$SYSCONFDIR"/sysconfig/iptables ] then - if grep $PORT "$SYSCONFDIR"/sysconfig/iptables >/dev/null 2>&1 + if grep "$PORT" "$SYSCONFDIR"/sysconfig/iptables >/dev/null 2>&1 then : else diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in index 54d045c..1959261 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -17,8 +17,8 @@ set -e -XMLFILE=$1 -TYPE=$2 +XMLFILE="$1" +TYPE="$2" if [ -z "$XMLFILE" ]; then echo "syntax: $0 XMLFILE [TYPE]" @@ -31,8 +31,8 @@ if [ ! -f "$XMLFILE" ]; then fi if [ -z "$TYPE" ]; then - ROOT=`xmllint --stream --debug $XMLFILE 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` - case $ROOT in + ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` + case "$ROOT" in *domain*) TYPE="domain" ;; @@ -64,7 +64,7 @@ if [ ! -f "$SCHEMA" ]; then exit 4 fi -xmllint --noout --relaxng $SCHEMA $XMLFILE +xmllint --noout --relaxng "$SCHEMA" "$XMLFILE" exit 0 -- 1.7.4

On 02/20/2011 01:29 PM, Dan Kenigsberg wrote: $PKI containing backticks wouldn't matter; really the issue is $PKI containing spaces subject to IFS splitting. But since was assume SYSCONFDIR might contain spaces (which may very well be the case on some mingw installation)... then yes, $PKI may contain spaces... and changes like this are important. Yuck - let's fix this grammar in the process. Everything else looks good, and I agree with danpb's ack. I've pushed this with the following squashed in: diff --git i/tools/virt-pki-validate.in w/tools/virt-pki-validate.in index 8a4249d..01825d1 100755 --- i/tools/virt-pki-validate.in +++ w/tools/virt-pki-validate.in @@ -185,7 +185,7 @@ then fi else - echo Did not found "$LIBVIRT/clientcert.pem" client certificate + echo Did not find "$LIBVIRT/clientcert.pem" client certificate echo The machine cannot act as a client echo "see http://libvirt.org/remote.html#Remote_TLS_client_certificates" echo on how to regenerate it @@ -237,7 +237,7 @@ then fi else - echo Did not found $LIBVIRT/servercert.pem server certificate + echo Did not find $LIBVIRT/servercert.pem server certificate echo The machine cannot act as a server echo "see http://libvirt.org/remote.html#Remote_TLS_server_certificates" echo on how to regenerate it -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

* libvirt.spec.in (Requires): Add gnutls-utils, for virt-pki-validate. Suggested by Daniel P. Berrange. --- Like so? libvirt.spec.in | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index a29693a..b5c9fd9 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -418,6 +418,8 @@ Requires: ncurses Requires: nc # Needed by libvirt-guests init script. Requires: gettext +# Needed by virt-pki-validate script. +Requires: gnutls-utils %if %{with_sasl} Requires: cyrus-sasl # Not technically required, but makes 'out-of-box' config -- 1.7.4

On 02/21/2011 10:57 AM, Daniel P. Berrange wrote: Thanks; pushed. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org