[libvirt] [PATCH 0/3] virt-*-validate cleanups
Since I complained about virt-login-shell not having --help or --version, I figured I'd better audit our other shipping executable files. In the process, I found that we have been unable to auto-validate <domainsnapshot> objects, among others. Eric Blake (3): virt-xml-validate: add --help/--version option virt-xml-validate: add missing schemas virt-pki-validate: add --help/--version option tools/Makefile.am | 10 ++++---- tools/virt-pki-validate.in | 45 +++++++++++++++++++++++++++++++++++- tools/virt-xml-validate.in | 57 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 102 insertions(+), 10 deletions(-) -- 1.8.3.1
All good tools should have --help and --version output :) * tools/virt-xml-validate.in: Add option parsing. Output errors to stderr. Update documentation to match. * tools/Makefile.am (virt-xml-validate): Substitute version. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/Makefile.am | 5 +++-- tools/virt-xml-validate.in | 44 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/tools/Makefile.am b/tools/Makefile.am index f85c35c..03c9fd0 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -77,8 +77,9 @@ dist_man8_MANS = virt-sanlock-cleanup.8 endif virt-xml-validate: virt-xml-validate.in Makefile - $(AM_V_GEN)sed -e 's|[@]schemadir@|$(pkgdatadir)/schemas|' < $< > $@ \ - || (rm $@ && exit 1) && chmod +x $@ + $(AM_V_GEN)sed -e 's|[@]schemadir@|$(pkgdatadir)/schemas|g' \ + -e 's|[@]VERSION@|$(VERSION)|g' \ + < $< > $@ || (rm $@ && exit 1) && chmod +x $@ virt-xml-validate.1: virt-xml-validate.in $(top_srcdir)/configure.ac $(AM_V_GEN)$(POD2MAN) --name VIRT-XML-VALIDATE $< $(srcdir)/$@ \ diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in index 9c584ed..9744612 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -17,16 +17,39 @@ set -e +case $1 in + -h | --h | --he | --hel | --help) + cat <<EOF +Usage: + $0 XML-FILE [SCHEMA-NAME] + $0 OPTION + +Options: + -h | --help Display program help + -V | --version Display program version +EOF + exit ;; + -V | --v | --ve | --ver | --vers | --versi | --versio | --version) + cat <<EOF +$0 (libvirt) @VERSION@ +EOF + exit ;; + --) shift ;; + -*) + echo "$0: unrecognized option '$1'" >&2 + exit 1 ;; +esac + XMLFILE="$1" TYPE="$2" if [ -z "$XMLFILE" ]; then - echo "syntax: $0 XMLFILE [TYPE]" + echo "syntax: $0 XMLFILE [TYPE]" >&2 exit 1 fi if [ ! -f "$XMLFILE" ]; then - echo "$0: document $XMLFILE does not exist" + echo "$0: document $XMLFILE does not exist" >&2 exit 2 fi @@ -78,6 +101,7 @@ exit 0 =head1 SYNOPSIS virt-xml-validate XML-FILE [SCHEMA-NAME] + virt-xml-validate OPTION =head1 DESCRIPTION @@ -117,6 +141,20 @@ The schema for the XML format used to declare driver capabilities =back +=head1 OPTIONS + +=over + +=item B<-h, --help> + +Display command line help usage then exit. + +=item B<-V, --version> + +Display version information then exit. + +=back + =head1 EXIT STATUS Upon successful validation, an exit status of 0 will be set. Upon @@ -134,7 +172,7 @@ Alternatively report bugs to your software distributor / vendor. =head1 COPYRIGHT -Copyright (C) 2009-2012 by Red Hat, Inc. +Copyright (C) 2009-2013 by Red Hat, Inc. Copyright (C) 2009 by Daniel P. Berrange =head1 LICENSE -- 1.8.3.1
On 08/19/2013 04:43 PM, Eric Blake wrote:
All good tools should have --help and --version output :)
* tools/virt-xml-validate.in: Add option parsing. Output errors to stderr. Update documentation to match. * tools/Makefile.am (virt-xml-validate): Substitute version.
+ --) shift ;; + -*) + echo "$0: unrecognized option '$1'" >&2 + exit 1 ;; +esac + XMLFILE="$1"
This treats '-' as an unrecognized option (which is a bit misleading, since in POSIX terminology a lone - is NOT an option but an argument). It also raises the question - should we allow for an XMLFILE of '-' to mean reading from stdin, the way POSIX recommends? That is, should we support: virsh dumpxml $dom | virt-xml-validate - as a valid mode of execution? Right now, that won't work (we consume stdin while probing the type, so that it no longer has contents for the actual validation pass), so it would need a separate patch to make the shell script smarter about reusing an input file, which may turn out to be a fair chunk of work before we get there (and it would probably be me stuck implementing it...). And if '-' works to specify stdin, why not also let this tool operate in filter mode (no arguments implies reading from stdin, just as an explicit - would). I guess the fact that I changed '-' to raise an error in this patch is a minor change for people used to using '-' as the name of a file in the current directory; but as that is not typical usage, and as they can work around that by using './-', I don't consider it a regression [but I _will_ update the commit message to mention that this is an intentional change in corner-case behavior] -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
----- Original Message ----- From: "Eric Blake" <eblake@redhat.com> Cc: libvir-list@redhat.com Sent: Monday, August 19, 2013 6:58:56 PM Subject: Re: [libvirt] [PATCH 1/3] virt-xml-validate: add --help/--version option On 08/19/2013 04:43 PM, Eric Blake wrote:
All good tools should have --help and --version output :)
* tools/virt-xml-validate.in: Add option parsing. Output errors to stderr. Update documentation to match. * tools/Makefile.am (virt-xml-validate): Substitute version.
+ --) shift ;; + -*) + echo "$0: unrecognized option '$1'" >&2 + exit 1 ;; +esac + XMLFILE="$1"
This treats '-' as an unrecognized option (which is a bit misleading, since in POSIX terminology a lone - is NOT an option but an argument). It also raises the question - should we allow for an XMLFILE of '-' to mean reading from stdin, the way POSIX recommends? That is, should we support: virsh dumpxml $dom | virt-xml-validate - grep use "grep -f -" to donate the standard input. As written in its manpage -f is specified by POSIX. If we use -f, the single hyphen can be used as a filename. But as you said in the follow, user can prefix the filename with a path - ./-, or /home/Tim/-, so it is still fine in this case. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 08/19/2013 07:51 PM, Guan Nan Ren wrote:
This treats '-' as an unrecognized option (which is a bit misleading, since in POSIX terminology a lone - is NOT an option but an argument). It also raises the question - should we allow for an XMLFILE of '-' to mean reading from stdin, the way POSIX recommends? That is, should we support:
virsh dumpxml $dom | virt-xml-validate -
grep use "grep -f -" to donate the standard input. As written in its manpage -f is specified by POSIX. If we use -f, the single hyphen can be used as a filename. But as you said in the follow, user can prefix the filename with a path - ./-, or /home/Tim/-, so it is still fine in this case.
Actually, 'grep -f - file' and 'grep pattern -' are two quite different uses of '-' meaning stdin (one treats stdin as the file containing the pattern, the other treats stdin as the file to search). But yes, the point is that since POSIX requires '-' to mean stdin when possible, people are used to that idiom, and used to using ./- when wanting to refer to a file literally named -. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
ACK ----- Original Message ----- From: "Eric Blake" <eblake@redhat.com> To: libvir-list@redhat.com Sent: Monday, August 19, 2013 6:43:13 PM Subject: [libvirt] [PATCH 1/3] virt-xml-validate: add --help/--version option All good tools should have --help and --version output :) * tools/virt-xml-validate.in: Add option parsing. Output errors to stderr. Update documentation to match. * tools/Makefile.am (virt-xml-validate): Substitute version. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/Makefile.am | 5 +++-- tools/virt-xml-validate.in | 44 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/tools/Makefile.am b/tools/Makefile.am index f85c35c..03c9fd0 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -77,8 +77,9 @@ dist_man8_MANS = virt-sanlock-cleanup.8 endif virt-xml-validate: virt-xml-validate.in Makefile - $(AM_V_GEN)sed -e 's|[@]schemadir@|$(pkgdatadir)/schemas|' < $< > $@ \ - || (rm $@ && exit 1) && chmod +x $@ + $(AM_V_GEN)sed -e 's|[@]schemadir@|$(pkgdatadir)/schemas|g' \ + -e 's|[@]VERSION@|$(VERSION)|g' \ + < $< > $@ || (rm $@ && exit 1) && chmod +x $@ virt-xml-validate.1: virt-xml-validate.in $(top_srcdir)/configure.ac $(AM_V_GEN)$(POD2MAN) --name VIRT-XML-VALIDATE $< $(srcdir)/$@ \ diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in index 9c584ed..9744612 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -17,16 +17,39 @@ set -e +case $1 in + -h | --h | --he | --hel | --help) + cat <<EOF +Usage: + $0 XML-FILE [SCHEMA-NAME] + $0 OPTION + +Options: + -h | --help Display program help + -V | --version Display program version +EOF + exit ;; + -V | --v | --ve | --ver | --vers | --versi | --versio | --version) + cat <<EOF +$0 (libvirt) @VERSION@ +EOF + exit ;; + --) shift ;; + -*) + echo "$0: unrecognized option '$1'" >&2 + exit 1 ;; +esac + XMLFILE="$1" TYPE="$2" if [ -z "$XMLFILE" ]; then - echo "syntax: $0 XMLFILE [TYPE]" + echo "syntax: $0 XMLFILE [TYPE]" >&2 exit 1 fi if [ ! -f "$XMLFILE" ]; then - echo "$0: document $XMLFILE does not exist" + echo "$0: document $XMLFILE does not exist" >&2 exit 2 fi @@ -78,6 +101,7 @@ exit 0 =head1 SYNOPSIS virt-xml-validate XML-FILE [SCHEMA-NAME] + virt-xml-validate OPTION =head1 DESCRIPTION @@ -117,6 +141,20 @@ The schema for the XML format used to declare driver capabilities =back +=head1 OPTIONS + +=over + +=item B<-h, --help> + +Display command line help usage then exit. + +=item B<-V, --version> + +Display version information then exit. + +=back + =head1 EXIT STATUS Upon successful validation, an exit status of 0 will be set. Upon @@ -134,7 +172,7 @@ Alternatively report bugs to your software distributor / vendor. =head1 COPYRIGHT -Copyright (C) 2009-2012 by Red Hat, Inc. +Copyright (C) 2009-2013 by Red Hat, Inc. Copyright (C) 2009 by Daniel P. Berrange =head1 LICENSE -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
We were failing to autoprobe which schema to use for several top-level XML elements. * tools/virt-xml-validate.in (TYPE): Recognize <domainsnapshot>, <filter>, and <secret>. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virt-xml-validate.in | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in index 9744612..82686f4 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -56,6 +56,9 @@ fi if [ -z "$TYPE" ]; then ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` case "$ROOT" in + *domainsnapshot*) # Must come first, since *domain* is a substring + TYPE="domainsnapshot" + ;; *domain*) TYPE="domain" ;; @@ -74,8 +77,14 @@ if [ -z "$TYPE" ]; then *device*) TYPE="nodedev" ;; + *filter*) + TYPE="nwfilter" + ;; + *secret*) + TYPE="secret" + ;; *) - echo "$0: cannot determine schema type for $XMLFILE" + echo "$0: cannot determine schema type for $XMLFILE" >&2 exit 3 esac fi @@ -83,7 +92,7 @@ fi SCHEMA="@schemadir@/${TYPE}.rng" if [ ! -f "$SCHEMA" ]; then - echo "$0: schema $SCHEMA does not exist" + echo "$0: schema $SCHEMA does not exist" >&2 exit 4 fi -- 1.8.3.1
ACK ----- Original Message ----- From: "Eric Blake" <eblake@redhat.com> To: libvir-list@redhat.com Sent: Monday, August 19, 2013 6:43:14 PM Subject: [libvirt] [PATCH 2/3] virt-xml-validate: add missing schemas We were failing to autoprobe which schema to use for several top-level XML elements. * tools/virt-xml-validate.in (TYPE): Recognize <domainsnapshot>, <filter>, and <secret>. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virt-xml-validate.in | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in index 9744612..82686f4 100644 --- a/tools/virt-xml-validate.in +++ b/tools/virt-xml-validate.in @@ -56,6 +56,9 @@ fi if [ -z "$TYPE" ]; then ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk '{ print $3 }'` case "$ROOT" in + *domainsnapshot*) # Must come first, since *domain* is a substring + TYPE="domainsnapshot" + ;; *domain*) TYPE="domain" ;; @@ -74,8 +77,14 @@ if [ -z "$TYPE" ]; then *device*) TYPE="nodedev" ;; + *filter*) + TYPE="nwfilter" + ;; + *secret*) + TYPE="secret" + ;; *) - echo "$0: cannot determine schema type for $XMLFILE" + echo "$0: cannot determine schema type for $XMLFILE" >&2 exit 3 esac fi @@ -83,7 +92,7 @@ fi SCHEMA="@schemadir@/${TYPE}.rng" if [ ! -f "$SCHEMA" ]; then - echo "$0: schema $SCHEMA does not exist" + echo "$0: schema $SCHEMA does not exist" >&2 exit 4 fi -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Another program gains --help/--version :) * tools/virt-pki-validate.in: Add option parsing. Update documentation to match. * tools/Makefile.am (virt-pki-validate): Substitute version. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/Makefile.am | 5 +++-- tools/virt-pki-validate.in | 45 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/tools/Makefile.am b/tools/Makefile.am index 03c9fd0..ef6e31b 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -87,8 +87,9 @@ virt-xml-validate.1: virt-xml-validate.in $(top_srcdir)/configure.ac rm $(srcdir)/$@; exit 1; fi virt-pki-validate: virt-pki-validate.in Makefile - $(AM_V_GEN)sed -e 's|[@]sysconfdir@|$(sysconfdir)|' < $< > $@ \ - || (rm $@ && exit 1) && chmod +x $@ + $(AM_V_GEN)sed -e 's|[@]sysconfdir@|$(sysconfdir)|g' \ + -e 's|[@]VERSION@|$(VERSION)|g' \ + < $< > $@ || (rm $@ && exit 1) && chmod +x $@ virt-pki-validate.1: virt-pki-validate.in $(top_srcdir)/configure.ac $(AM_V_GEN)$(POD2MAN) --name VIRT-PKI-VALIDATE $< $(srcdir)/$@ \ diff --git a/tools/virt-pki-validate.in b/tools/virt-pki-validate.in index 5d0453f..87fb230 100755 --- a/tools/virt-pki-validate.in +++ b/tools/virt-pki-validate.in @@ -22,6 +22,35 @@ # # Daniel Veillard <veillard@redhat.com> # + +case $1 in +case $1 in + -h | --h | --he | --hel | --help) + cat <<EOF +Usage: + $0 [OPTION] + +Options: + -h | --help Display program help + -V | --version Display program version +EOF + exit ;; + -V | --v | --ve | --ver | --vers | --versi | --versio | --version) + cat <<EOF +$0 (libvirt) @VERSION@ +EOF + exit ;; + --) shift ;; + -*) + echo "$0: unrecognized option '$1'" >&2 + exit 1 ;; +esac + +if test $# != 0; then + echo "$0: unrecognized argument '$1'" >&2 + exit 1 ;; +fi + USER=`who am i | awk '{ print $1 }'` SERVER=1 CLIENT=1 @@ -300,7 +329,7 @@ exit 0 =head1 SYNOPSIS - virt-pki-validate + virt-pki-validate [OPTION] =head1 DESCRIPTION @@ -309,6 +338,20 @@ a secure libvirt server or client using the TLS encryption protocol. It will report any missing certificate or key files on the host. It should be run as root to ensure it can read all the necessary files +=head1 OPTIONS + +=over + +=item B<-h, --help> + +Display command line help usage then exit. + +=item B<-V, --version> + +Display version information then exit. + +=back + =head1 EXIT STATUS Upon successful validation, an exit status of 0 will be set. Upon -- 1.8.3.1
----- Original Message ----- From: "Eric Blake" <eblake@redhat.com> To: libvir-list@redhat.com Sent: Monday, August 19, 2013 6:43:15 PM Subject: [libvirt] [PATCH 3/3] virt-pki-validate: add --help/--version option Another program gains --help/--version :) * tools/virt-pki-validate.in: Add option parsing. Update documentation to match. * tools/Makefile.am (virt-pki-validate): Substitute version. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/Makefile.am | 5 +++-- tools/virt-pki-validate.in | 45 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) ... diff --git a/tools/virt-pki-validate.in b/tools/virt-pki-validate.in index 5d0453f..87fb230 100755 --- a/tools/virt-pki-validate.in +++ b/tools/virt-pki-validate.in @@ -22,6 +22,35 @@ # # Daniel Veillard <veillard@redhat.com> # + +case $1 in +case $1 in copy-paste error, double case lines. + -h | --h | --he | --hel | --help) + cat <<EOF ... + +if test $# != 0; then + echo "$0: unrecognized argument '$1'" >&2 + exit 1 ;; remove ;; here. ACK with these small fixes. -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 08/19/2013 08:01 PM, Guan Nan Ren wrote:
----- Original Message ----- From: "Eric Blake" <eblake@redhat.com> To: libvir-list@redhat.com Sent: Monday, August 19, 2013 6:43:15 PM Subject: [libvirt] [PATCH 3/3] virt-pki-validate: add --help/--version option
Another program gains --help/--version :)
+ +case $1 in +case $1 in
copy-paste error, double case lines.
Oops - must have hit send before my final save. Thanks for catching that.
+ -h | --h | --he | --hel | --help) + cat <<EOF ...
+ +if test $# != 0; then + echo "$0: unrecognized argument '$1'" >&2 + exit 1 ;;
remove ;; here.
ACK with these small fixes.
After one more round of testing, I pushed the fixed patches. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Guan Nan Ren