Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
Hello, Am 21.05.19 um 09:35 schrieb Johannes Holmberg: > On Tue, 2019-05-21 at 07:36 +0200, Philipp Hahn wrote: >> Am 20.05.19 um 13:57 schrieb Johannes Holmberg: >>> diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in ... >>> +if [ "$XMLFILE" = "-" ]; then >>> +TMPFILE=`mktemp --tmpdir virt-xml.` ... >> Quoting > > Good point(s). I can only imagine I skipped the quoting because TMPFILE > is known to contain a safe string, No, it's not guaranteed, as it honors TMPDIR: > $ LANG=C TMPDIR='/tmp/un safe' mktemp --tmpdir virt-xml. > mktemp: failed to create file via template '/tmp/un safe/virt-xml.': ... Philipp -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
On Tue, 2019-05-21 at 07:36 +0200, Philipp Hahn wrote:
> Hello,
>
> Some nits:
>
> Am 20.05.19 um 13:57 schrieb Johannes Holmberg:
> > diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-
> > validate.in
> > index 64ae33..2d2afb74ec 100644
> > --- a/tools/virt-xml-validate.in
> > +++ b/tools/virt-xml-validate.in
> > @@ -16,6 +16,16 @@
> >
> > set -e
> >
> > +TMPFILE=
> > +
> > +cleanup() {
> > + if [ $TMPFILE ]; then
>
> Missing Quoting.
> Better also give "-n"
>
> > +rm -f $TMPFILE
>
> Quoting
>
> > + fi
> > +}
> > +
> > +trap cleanup EXIT
> > +
> > case $1 in
>
> Not your fault, but also missing quoting.
>
> >-h | --h | --he | --hel | --help)
> > cat < > @@ -34,7 +44,7 @@ $0 (libvirt) @VERSION@
> > EOF
> > exit ;;
> >--) shift ;;
> > - -*)
> > + -?*)
> > echo "$0: unrecognized option '$1'" >&2
> > exit 1 ;;
> > esac
> > @@ -42,18 +52,27 @@ esac
> > XMLFILE="$1"
> > TYPE="$2"
> >
> > -if [ -z "$XMLFILE" ]; then
> > - echo "syntax: $0 XMLFILE [TYPE]" >&2
> > - exit 1
> > -fi
> > +if [ "$XMLFILE" = "-" ]; then
> > +TMPFILE=`mktemp --tmpdir virt-xml.`> +cat > $TMPFILE
>
> Quoting
>
> > +else
> > + if [ -z "$XMLFILE" ]; then
> > +echo "syntax: $0 XMLFILE [TYPE]" >&2
> > +exit 1
> > + fi
> >
> > -if [ ! -f "$XMLFILE" ]; then
> > - echo "$0: document $XMLFILE does not exist" >&2
> > - exit 2
> > + if [ ! -f "$XMLFILE" ]; then
> > +echo "$0: document $XMLFILE does not exist" >&2
> > +exit 2
> > + fi
> > fi
> >
> > if [ -z "$TYPE" ]; then
> > - ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0
> > 1 " | awk '{ print $3 }'`
> > + if [ $TMPFILE ]; then
> > +ROOT=`xmllint --stream --debug - < "$TMPFILE" 2>/dev/null |
> > grep "^0 1 " | awk '{ print $3 }'`
> > + else
> > +ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep
> > "^0 1 " | awk '{ print $3 }'`
> > + fi
> >case "$ROOT" in
> > *domainsnapshot*) # Must come first, since *domain* is a
> > substring
> > TYPE="domainsnapshot"
> > @@ -101,6 +120,9 @@ if [ ! -f "$SCHEMA" ]; then
> >exit 4
> > fi
> >
> > -xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
> > -
> > +if [ $TMPFILE ]; then
>
> Quoting
> "-n"
>
> > + xmllint --noout --relaxng "$SCHEMA" - < "$TMPFILE"
> > +else
> > + xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
> > +fi
> > exit
Good point(s). I can only imagine I skipped the quoting because TMPFILE
is known to contain a safe string, but that is a poor excuse really.
I'll post a v2 shortly.
/Johannes
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
Hello,
Some nits:
Am 20.05.19 um 13:57 schrieb Johannes Holmberg:
> diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in
> index 64ae33..2d2afb74ec 100644
> --- a/tools/virt-xml-validate.in
> +++ b/tools/virt-xml-validate.in
> @@ -16,6 +16,16 @@
>
> set -e
>
> +TMPFILE=
> +
> +cleanup() {
> + if [ $TMPFILE ]; then
Missing Quoting.
Better also give "-n"
> +rm -f $TMPFILE
Quoting
> + fi
> +}
> +
> +trap cleanup EXIT
> +
> case $1 in
Not your fault, but also missing quoting.
>-h | --h | --he | --hel | --help)
> cat < @@ -34,7 +44,7 @@ $0 (libvirt) @VERSION@
> EOF
> exit ;;
>--) shift ;;
> - -*)
> + -?*)
> echo "$0: unrecognized option '$1'" >&2
> exit 1 ;;
> esac
> @@ -42,18 +52,27 @@ esac
> XMLFILE="$1"
> TYPE="$2"
>
> -if [ -z "$XMLFILE" ]; then
> - echo "syntax: $0 XMLFILE [TYPE]" >&2
> - exit 1
> -fi
> +if [ "$XMLFILE" = "-" ]; then
> +TMPFILE=`mktemp --tmpdir virt-xml.`> +cat > $TMPFILE
Quoting
> +else
> + if [ -z "$XMLFILE" ]; then
> +echo "syntax: $0 XMLFILE [TYPE]" >&2
> +exit 1
> + fi
>
> -if [ ! -f "$XMLFILE" ]; then
> - echo "$0: document $XMLFILE does not exist" >&2
> - exit 2
> + if [ ! -f "$XMLFILE" ]; then
> +echo "$0: document $XMLFILE does not exist" >&2
> +exit 2
> + fi
> fi
>
> if [ -z "$TYPE" ]; then
> - ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk
> '{ print $3 }'`
> + if [ $TMPFILE ]; then
> +ROOT=`xmllint --stream --debug - < "$TMPFILE" 2>/dev/null | grep "^0 1 "
> | awk '{ print $3 }'`
> + else
> +ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " |
> awk '{ print $3 }'`
> + fi
>case "$ROOT" in
> *domainsnapshot*) # Must come first, since *domain* is a substring
> TYPE="domainsnapshot"
> @@ -101,6 +120,9 @@ if [ ! -f "$SCHEMA" ]; then
>exit 4
> fi
>
> -xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
> -
> +if [ $TMPFILE ]; then
Quoting
"-n"
> + xmllint --noout --relaxng "$SCHEMA" - < "$TMPFILE"
> +else
> + xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
> +fi
> exit
Philipp
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
On Mon, 2019-05-20 at 14:18 +0200, Martin Kletzander wrote: > On Mon, May 20, 2019 at 11:57:13AM +, Johannes Holmberg wrote: > > --- > > > > Hello, > > > > This is an updated version of a patch I submitted on 2015-06-10. I > > got > > some feedback on it but then moved on to a different project and > > forgot about it. Anyway, I've updated the patch according to the > > feedback so if you are still interested, here it is! :) > > > > Hi, thanks for the patch. Wouldn't it be easier and cleaner if we > just did: > > if [ "$XMLFILE" = "-" ]; then > XMLFILE=/dev/stdin > fi > > ?? I tried it but the problem is that if the SCHEMA-NAME argument is not supplied, the validation needs two passes, one to read the root tag, and another one to do the actual xml validation. If I use /dev/stdin the second pass will (sometimes) fail because the input is already exhausted. It works if the input is redirected from a file: $ ./virt-xml-validate - < asdf.xml /dev/stdin validates But if I pipe it from another program it fails: $ cat asdf.xml | ./virt-xml-validate - /dev/stdin:1: parser error : Document is empty /Johannes -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
On Mon, May 20, 2019 at 11:57:13AM +, Johannes Holmberg wrote: --- Hello, This is an updated version of a patch I submitted on 2015-06-10. I got some feedback on it but then moved on to a different project and forgot about it. Anyway, I've updated the patch according to the feedback so if you are still interested, here it is! :) Hi, thanks for the patch. Wouldn't it be easier and cleaner if we just did: if [ "$XMLFILE" = "-" ]; then XMLFILE=/dev/stdin fi ?? signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
---
Hello,
This is an updated version of a patch I submitted on 2015-06-10. I got
some feedback on it but then moved on to a different project and
forgot about it. Anyway, I've updated the patch according to the
feedback so if you are still interested, here it is! :)
/Johannes
tools/virt-xml-validate.in | 44 --
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in
index 64ae33..2d2afb74ec 100644
--- a/tools/virt-xml-validate.in
+++ b/tools/virt-xml-validate.in
@@ -16,6 +16,16 @@
set -e
+TMPFILE=
+
+cleanup() {
+ if [ $TMPFILE ]; then
+rm -f $TMPFILE
+ fi
+}
+
+trap cleanup EXIT
+
case $1 in
-h | --h | --he | --hel | --help)
cat <&2
exit 1 ;;
esac
@@ -42,18 +52,27 @@ esac
XMLFILE="$1"
TYPE="$2"
-if [ -z "$XMLFILE" ]; then
- echo "syntax: $0 XMLFILE [TYPE]" >&2
- exit 1
-fi
+if [ "$XMLFILE" = "-" ]; then
+TMPFILE=`mktemp --tmpdir virt-xml.`
+cat > $TMPFILE
+else
+ if [ -z "$XMLFILE" ]; then
+echo "syntax: $0 XMLFILE [TYPE]" >&2
+exit 1
+ fi
-if [ ! -f "$XMLFILE" ]; then
- echo "$0: document $XMLFILE does not exist" >&2
- exit 2
+ if [ ! -f "$XMLFILE" ]; then
+echo "$0: document $XMLFILE does not exist" >&2
+exit 2
+ fi
fi
if [ -z "$TYPE" ]; then
- ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk
'{ print $3 }'`
+ if [ $TMPFILE ]; then
+ROOT=`xmllint --stream --debug - < "$TMPFILE" 2>/dev/null | grep "^0 1 " |
awk '{ print $3 }'`
+ else
+ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk
'{ print $3 }'`
+ fi
case "$ROOT" in
*domainsnapshot*) # Must come first, since *domain* is a substring
TYPE="domainsnapshot"
@@ -101,6 +120,9 @@ if [ ! -f "$SCHEMA" ]; then
exit 4
fi
-xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
-
+if [ $TMPFILE ]; then
+ xmllint --noout --relaxng "$SCHEMA" - < "$TMPFILE"
+else
+ xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
+fi
exit
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
On Wed, Jun 24, 2015 at 06:17:35PM +0200, Johannes Holmberg wrote: - On 24 Jun, 2015, at 09:14, Martin Kletzander mklet...@redhat.com wrote: Why don't you just set XMLFILE=$TMPFILE ? That would get rid of lot of the code changes below and would be more readable. Other than that it looks good. Good question, with a somewhat long answer. I initially did exactly what you suggest, and yes, it makes for simpler code. The only problem with it is that when xmllint finds a problem with the xml, it will output a string on the format file name:line number: error description. If the file was provided on stdin, the file name will be something like /tmp/virt-xml.asdf which will not make much sense to the user. So instead I tried to be consistent: if virt-xml-validate received a filename, xmllint will receive a filename. If virt-xml-validate received data on stdin, xmllint will receive data on stdin. This way, when the xml is provided on stdin and xmllint finds a problem, the file name in the error message will just show up as -, which is probably more in line with what the user was expecting. Anyway, it's not something I feel super-strongly about, so if you prefer to have it changed, just let me know and I'll have an updated patch for you. I'm on the edge here. You've got perfectly good point here. If there was at least a way of simplifying each condition somehow. Each such approach pollutes the code with 'eval' or new function that makes it even more unreadable :( Anyway, I'm going into too much unnecessary details. Thanks to your explanation I'm OK with your approach, I would suggest just one teeny tiny change, and that's using 'xmllint - $file' instead of 'cat $file | xmllint -'. If that's ok with you, just Cc me on the v2 and I'll have a look at it. Thanks, Martin signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
On Wed, Jun 10, 2015 at 12:03:44PM +0200, Johannes Holmberg wrote:
---
Hello,
I often find myself wanting to validate a domain xml without having
to save it to a file, so I've updated virt-xml-validate to support
the standard practice of reading from stdin when - is given as the
input file.
Of course, the validation is multipass so there still needs to be a
temporary file but I think it makes sense to hide this complexity in
virt-xml-validate and not force it onto the user.
/Johannes
It looks like this could work, just a couple things...
tools/virt-xml-validate.in | 53 +-
1 file changed, 38 insertions(+), 15 deletions(-)
diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in
index a04fa06..8807f8d 100644
--- a/tools/virt-xml-validate.in
+++ b/tools/virt-xml-validate.in
@@ -17,6 +17,16 @@
set -e
+TMPFILE=
+
+cleanup() {
+ if [ $TMPFILE ]; then
+rm -f $TMPFILE
+ fi
+}
+
+trap cleanup EXIT
+
case $1 in
-h | --h | --he | --hel | --help)
cat EOF
@@ -35,7 +45,7 @@ $0 (libvirt) @VERSION@
EOF
exit ;;
--) shift ;;
- -*)
+ -?*)
echo $0: unrecognized option '$1' 2
exit 1 ;;
esac
@@ -43,18 +53,27 @@ esac
XMLFILE=$1
TYPE=$2
-if [ -z $XMLFILE ]; then
- echo syntax: $0 XMLFILE [TYPE] 2
- exit 1
-fi
-
-if [ ! -f $XMLFILE ]; then
- echo $0: document $XMLFILE does not exist 2
- exit 2
+if [ $XMLFILE = - ]; then
+TMPFILE=`mktemp --tmpdir virt-xml.`
+cat $TMPFILE
Why don't you just set XMLFILE=$TMPFILE ? That would get rid of lot
of the code changes below and would be more readable.
Other than that it looks good.
+else
+ if [ -z $XMLFILE ]; then
+echo syntax: $0 XMLFILE [TYPE] 2
+exit 1
+ fi
+
+ if [ ! -f $XMLFILE ]; then
+echo $0: document $XMLFILE does not exist 2
+exit 2
+ fi
fi
if [ -z $TYPE ]; then
- ROOT=`xmllint --stream --debug $XMLFILE 2/dev/null | grep ^0 1 | awk
'{ print $3 }'`
+ if [ $TMPFILE ]; then
+ROOT=`cat $TMPFILE | xmllint --stream --debug - 2/dev/null | grep ^0 1
| awk '{ print $3 }'`
+ else
+ROOT=`xmllint --stream --debug $XMLFILE 2/dev/null | grep ^0 1 | awk
'{ print $3 }'`
+ fi
case $ROOT in
*domainsnapshot*) # Must come first, since *domain* is a substring
TYPE=domainsnapshot
@@ -99,8 +118,11 @@ if [ ! -f $SCHEMA ]; then
exit 4
fi
-xmllint --noout --relaxng $SCHEMA $XMLFILE
-
+if [ $TMPFILE ]; then
+ cat $TMPFILE | xmllint --noout --relaxng $SCHEMA -
+else
+ xmllint --noout --relaxng $SCHEMA $XMLFILE
+fi
exit
: =cut
@@ -119,9 +141,10 @@ exit
Validates a libvirt XML for compliance with the published schema.
The first compulsory argument is the path to the XML file to be
-validated. The optional second argument is the name of the schema
-to validate against. If omitted, the schema name will be inferred
-from the name of the root element in the XML document.
+validated (or - to read the XML from standard input). The optional
+second argument is the name of the schema to validate against. If
+omitted, the schema name will be inferred from the name of the root
+element in the XML document.
Valid schema names currently include
--
1.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
- On 24 Jun, 2015, at 09:14, Martin Kletzander mklet...@redhat.com wrote: Why don't you just set XMLFILE=$TMPFILE ? That would get rid of lot of the code changes below and would be more readable. Other than that it looks good. Good question, with a somewhat long answer. I initially did exactly what you suggest, and yes, it makes for simpler code. The only problem with it is that when xmllint finds a problem with the xml, it will output a string on the format file name:line number: error description. If the file was provided on stdin, the file name will be something like /tmp/virt-xml.asdf which will not make much sense to the user. So instead I tried to be consistent: if virt-xml-validate received a filename, xmllint will receive a filename. If virt-xml-validate received data on stdin, xmllint will receive data on stdin. This way, when the xml is provided on stdin and xmllint finds a problem, the file name in the error message will just show up as -, which is probably more in line with what the user was expecting. Anyway, it's not something I feel super-strongly about, so if you prefer to have it changed, just let me know and I'll have an updated patch for you. /Johannes -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin
---
Hello,
I often find myself wanting to validate a domain xml without having
to save it to a file, so I've updated virt-xml-validate to support
the standard practice of reading from stdin when - is given as the
input file.
Of course, the validation is multipass so there still needs to be a
temporary file but I think it makes sense to hide this complexity in
virt-xml-validate and not force it onto the user.
/Johannes
tools/virt-xml-validate.in | 53 +-
1 file changed, 38 insertions(+), 15 deletions(-)
diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in
index a04fa06..8807f8d 100644
--- a/tools/virt-xml-validate.in
+++ b/tools/virt-xml-validate.in
@@ -17,6 +17,16 @@
set -e
+TMPFILE=
+
+cleanup() {
+ if [ $TMPFILE ]; then
+rm -f $TMPFILE
+ fi
+}
+
+trap cleanup EXIT
+
case $1 in
-h | --h | --he | --hel | --help)
cat EOF
@@ -35,7 +45,7 @@ $0 (libvirt) @VERSION@
EOF
exit ;;
--) shift ;;
- -*)
+ -?*)
echo $0: unrecognized option '$1' 2
exit 1 ;;
esac
@@ -43,18 +53,27 @@ esac
XMLFILE=$1
TYPE=$2
-if [ -z $XMLFILE ]; then
- echo syntax: $0 XMLFILE [TYPE] 2
- exit 1
-fi
-
-if [ ! -f $XMLFILE ]; then
- echo $0: document $XMLFILE does not exist 2
- exit 2
+if [ $XMLFILE = - ]; then
+TMPFILE=`mktemp --tmpdir virt-xml.`
+cat $TMPFILE
+else
+ if [ -z $XMLFILE ]; then
+echo syntax: $0 XMLFILE [TYPE] 2
+exit 1
+ fi
+
+ if [ ! -f $XMLFILE ]; then
+echo $0: document $XMLFILE does not exist 2
+exit 2
+ fi
fi
if [ -z $TYPE ]; then
- ROOT=`xmllint --stream --debug $XMLFILE 2/dev/null | grep ^0 1 | awk
'{ print $3 }'`
+ if [ $TMPFILE ]; then
+ROOT=`cat $TMPFILE | xmllint --stream --debug - 2/dev/null | grep ^0 1
| awk '{ print $3 }'`
+ else
+ROOT=`xmllint --stream --debug $XMLFILE 2/dev/null | grep ^0 1 | awk
'{ print $3 }'`
+ fi
case $ROOT in
*domainsnapshot*) # Must come first, since *domain* is a substring
TYPE=domainsnapshot
@@ -99,8 +118,11 @@ if [ ! -f $SCHEMA ]; then
exit 4
fi
-xmllint --noout --relaxng $SCHEMA $XMLFILE
-
+if [ $TMPFILE ]; then
+ cat $TMPFILE | xmllint --noout --relaxng $SCHEMA -
+else
+ xmllint --noout --relaxng $SCHEMA $XMLFILE
+fi
exit
: =cut
@@ -119,9 +141,10 @@ exit
Validates a libvirt XML for compliance with the published schema.
The first compulsory argument is the path to the XML file to be
-validated. The optional second argument is the name of the schema
-to validate against. If omitted, the schema name will be inferred
-from the name of the root element in the XML document.
+validated (or - to read the XML from standard input). The optional
+second argument is the name of the schema to validate against. If
+omitted, the schema name will be inferred from the name of the root
+element in the XML document.
Valid schema names currently include
--
1.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
