[libvirt] [PATCH] tests: redo test argv file line wrapping
Back in commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b Author: Juerg Haefliger <juerg.haefliger@hp.com> Date: Mon Jan 31 06:42:57 2011 -0500 tests: handle backspace-newline pairs in test input files all the test argv files were line wrapped so that the args were less than 80 characters. The way the line wrapping was done turns out to be quite undesirable, because it often leaves multiple parameters on the same line. If we later need to add or remove individual parameters, then it leaves us having to redo line wrapping. This commit changes the line wrapping so that every single "-param value" is one its own new line. If the "value" is still too long, then we break on ',' or ':' or ' ' as needed. This means that when we come to add / remove parameters from the test files line, the patch diffs will only ever show a single line added/removed which will greatly simplify review work. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 10 +- .../networkxml2firewalldata/nat-default-linux.args | 137 ++++++++++++++++----- .../qemuxml2argv-aarch64-aavmf-virtio-mmio.args | 33 +++-- tests/test-wrap-argv.pl | 106 ++++++++++++++++ 4 files changed, 247 insertions(+), 39 deletions(-) create mode 100644 tests/test-wrap-argv.pl To avoid sending a 10 MB email, I'v only include these 3 example file changes. To view *all* the changes see my staging branch https://gitlab.com/berrange/libvirt/commit/4fe45c39dae176ae4dbc7e126d2a2ff29... 'make check' and 'make syntax-check' of course still pass after applying the big change. diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args index 6b26964..e1afefd 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args @@ -1,3 +1,11 @@ -/usr/sbin/bhyve -c 1 -m 214 -A -I -u -H -P -s 0:0,hostbridge \ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-A \ +-I \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ -s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/networkxml2firewalldata/nat-default-linux.args b/tests/networkxml2firewalldata/nat-default-linux.args index b92a845..ffdafdf 100644 --- a/tests/networkxml2firewalldata/nat-default-linux.args +++ b/tests/networkxml2firewalldata/nat-default-linux.args @@ -1,30 +1,107 @@ -iptables --table filter --insert INPUT --in-interface virbr0 --protocol tcp \ ---destination-port 67 --jump ACCEPT -iptables --table filter --insert INPUT --in-interface virbr0 --protocol udp \ ---destination-port 67 --jump ACCEPT -iptables --table filter --insert OUTPUT --out-interface virbr0 --protocol udp \ ---destination-port 68 --jump ACCEPT -iptables --table filter --insert INPUT --in-interface virbr0 --protocol tcp \ ---destination-port 53 --jump ACCEPT -iptables --table filter --insert INPUT --in-interface virbr0 --protocol udp \ ---destination-port 53 --jump ACCEPT -iptables --table filter --insert FORWARD --in-interface virbr0 --jump REJECT -iptables --table filter --insert FORWARD --out-interface virbr0 --jump REJECT -iptables --table filter --insert FORWARD --in-interface virbr0 \ ---out-interface virbr0 --jump ACCEPT -iptables --table filter --insert FORWARD --source 192.168.122.0/24 \ ---in-interface virbr0 --jump ACCEPT -iptables --table filter --insert FORWARD --destination 192.168.122.0/24 \ ---out-interface virbr0 --match conntrack --ctstate ESTABLISHED,RELATED --jump ACCEPT -iptables --table nat --insert POSTROUTING --source 192.168.122.0/24 '!' \ ---destination 192.168.122.0/24 --jump MASQUERADE -iptables --table nat --insert POSTROUTING --source 192.168.122.0/24 \ --p udp '!' --destination 192.168.122.0/24 --jump MASQUERADE --to-ports 1024-65535 -iptables --table nat --insert POSTROUTING --source 192.168.122.0/24 \ --p tcp '!' --destination 192.168.122.0/24 --jump MASQUERADE --to-ports 1024-65535 -iptables --table nat --insert POSTROUTING --source 192.168.122.0/24 \ ---destination 255.255.255.255/32 --jump RETURN -iptables --table nat --insert POSTROUTING --source 192.168.122.0/24 \ ---destination 224.0.0.0/24 --jump RETURN -iptables --table mangle --insert POSTROUTING --out-interface virbr0 \ ---protocol udp --destination-port 68 --jump CHECKSUM --checksum-fill +iptables \ +--table filter \ +--insert INPUT \ +--in-interface virbr0 \ +--protocol tcp \ +--destination-port 67 \ +--jump ACCEPT +iptables \ +--table filter \ +--insert INPUT \ +--in-interface virbr0 \ +--protocol udp \ +--destination-port 67 \ +--jump ACCEPT +iptables \ +--table filter \ +--insert OUTPUT \ +--out-interface virbr0 \ +--protocol udp \ +--destination-port 68 \ +--jump ACCEPT +iptables \ +--table filter \ +--insert INPUT \ +--in-interface virbr0 \ +--protocol tcp \ +--destination-port 53 \ +--jump ACCEPT +iptables \ +--table filter \ +--insert INPUT \ +--in-interface virbr0 \ +--protocol udp \ +--destination-port 53 \ +--jump ACCEPT +iptables \ +--table filter \ +--insert FORWARD \ +--in-interface virbr0 \ +--jump REJECT +iptables \ +--table filter \ +--insert FORWARD \ +--out-interface virbr0 \ +--jump REJECT +iptables \ +--table filter \ +--insert FORWARD \ +--in-interface virbr0 \ +--out-interface virbr0 \ +--jump ACCEPT +iptables \ +--table filter \ +--insert FORWARD \ +--source 192.168.122.0/24 \ +--in-interface virbr0 \ +--jump ACCEPT +iptables \ +--table filter \ +--insert FORWARD \ +--destination 192.168.122.0/24 \ +--out-interface virbr0 \ +--match conntrack \ +--ctstate ESTABLISHED,RELATED \ +--jump ACCEPT +iptables \ +--table nat \ +--insert POSTROUTING \ +--source 192.168.122.0/24 '!' \ +--destination 192.168.122.0/24 \ +--jump MASQUERADE +iptables \ +--table nat \ +--insert POSTROUTING \ +--source 192.168.122.0/24 \ +-p udp '!' \ +--destination 192.168.122.0/24 \ +--jump MASQUERADE \ +--to-ports 1024-65535 +iptables \ +--table nat \ +--insert POSTROUTING \ +--source 192.168.122.0/24 \ +-p tcp '!' \ +--destination 192.168.122.0/24 \ +--jump MASQUERADE \ +--to-ports 1024-65535 +iptables \ +--table nat \ +--insert POSTROUTING \ +--source 192.168.122.0/24 \ +--destination 255.255.255.255/32 \ +--jump RETURN +iptables \ +--table nat \ +--insert POSTROUTING \ +--source 192.168.122.0/24 \ +--destination 224.0.0.0/24 \ +--jump RETURN +iptables \ +--table mangle \ +--insert POSTROUTING \ +--out-interface virbr0 \ +--protocol udp \ +--destination-port 68 \ +--jump CHECKSUM \ +--checksum-fill diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-aavmf-virtio-mmio.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-aavmf-virtio-mmio.args index d6d8aed..4cc4cd2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-aavmf-virtio-mmio.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-aavmf-virtio-mmio.args @@ -1,14 +1,31 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -/usr/bin/qemu-system-aarch64 -S -M virt -cpu cortex-a53 \ --m 1024 -smp 1 -nographic \ --nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait \ --boot c -kernel /aarch64.kernel -initrd /aarch64.initrd -append \ -'earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait' \ --dtb /aarch64.dtb -device virtio-serial-device,id=virtio-serial0 -usb \ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-aarch64 \ +-S \ +-M virt \ +-cpu cortex-a53 \ +-m 1024 \ +-smp 1 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-boot c \ +-kernel /aarch64.kernel \ +-initrd /aarch64.initrd \ +-append 'earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait' \ +-dtb /aarch64.dtb \ +-device virtio-serial-device,id=virtio-serial0 \ +-usb \ -drive file=/aarch64.raw,if=none,id=drive-virtio-disk0 \ -device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0 \ -device virtio-net-device,vlan=0,id=net0,mac=52:54:00:09:a4:37 \ --net user,vlan=0,name=hostnet0 -chardev pty,id=charconsole0 \ +-net user,vlan=0,name=hostnet0 \ +-chardev pty,id=charconsole0 \ -device virtconsole,chardev=charconsole0,id=console0 \ -device virtio-balloon-device,id=balloon0 \ -object rng-random,id=objrng0,filename=/dev/random \ diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl new file mode 100644 index 0000000..e8c782b --- /dev/null +++ b/tests/test-wrap-argv.pl @@ -0,0 +1,106 @@ +#!/usr/bin/perl + + + +foreach my $file (@ARGV) { + &rewrap($file); +} + +sub rewrap { + my $file = shift; + + # Read the original file + open FILE, "<", $file or die "cannot read $file: $!"; + my @lines; + while (<FILE>) { + # If there is a trailing '\' then kill the new line + if (/\\$/) { + chomp; + $_ =~ s/\\$//; + } + + push @lines, $_; + } + + # Skip empty files + return unless @lines; + + # Kill the last new line in the file + chomp @lines[$#lines]; + close FILE; + + # Reconstruct the master data by joining all lines + # and then split again based on the real desired + # newlines + @lines = split /\n/, join('', @lines); + + open FILE, ">", $file or die "cannot create $file: $!"; + + print "Process $file\n"; + # Now each @lines represents a single command, we + # can process them + foreach my $line (@lines) { + my @bits = split / /, join('', $line); + + # @bits contains env vars, then the command line + # and then the arguments + my @env; + my $cmd; + my @args; + + foreach my $bit (@bits) { + # If no command is defined yet, we must still + # have env vars + if (!defined $cmd) { + # Look for leading / or = to indicate command name + if ($bit =~ m,^/,) { + $cmd = $bit; + } else { + push @env, $bit; + } + } else { + # If there's a leading '-' then this is a new + # parameter, otherwise its a value for the prev + # parameter. + if ($bit =~ m,^-,) { + push @args, $bit; + } else { + $args[$#args] .= " " . $bit; + } + } + } + + # Print env + command first + print FILE join(" \\\n", @env, $cmd), " \\\n"; + # We might have to split line argument values... + for (my $i = 0; $i <= $#args; $i++) { + my $arg = $args[$i]; + while (length($arg) > 80) { + my $split = rindex $arg, ",", 80; + if ($split == -1) { + $split = rindex $arg, ":", 80; + } + if ($split == -1) { + $split = rindex $arg, " ", 80; + } + if ($split == -1) { + warn "$file: cannot find nice place to split '$arg' below 80 chars\n"; + $split = 79; + } + $split++; + + my $head = substr $arg, 0, $split; + $arg = substr $arg, $split; + + print FILE $head, "\\\n"; + } + print FILE $arg; + if ($i != $#args) { + print FILE " \\\n"; + } else { + print FILE "\n"; + } + } + } + close FILE; +} -- 2.5.0
On Fri, Nov 06, 2015 at 01:30:35PM +0000, Daniel P. Berrange wrote:
Back in
commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b Author: Juerg Haefliger <juerg.haefliger@hp.com> Date: Mon Jan 31 06:42:57 2011 -0500
tests: handle backspace-newline pairs in test input files
all the test argv files were line wrapped so that the args were less than 80 characters.
The way the line wrapping was done turns out to be quite undesirable, because it often leaves multiple parameters on the same line. If we later need to add or remove individual parameters, then it leaves us having to redo line wrapping.
This commit changes the line wrapping so that every single "-param value" is one its own new line. If the "value" is still too long, then we break on ',' or ':' or ' ' as needed.
What if we fix the syntax-check instead and allow longer than 80 character lines in case they have no space in it, or exactly one space (to allow --parameter option,option,option,...)? That would make even corner cases easier to review, e.g. when you remove or add a parameter into the long list of parameters.
This means that when we come to add / remove parameters from the test files line, the patch diffs will only ever show a single line added/removed which will greatly simplify review work.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 10 +- .../networkxml2firewalldata/nat-default-linux.args | 137 ++++++++++++++++----- .../qemuxml2argv-aarch64-aavmf-virtio-mmio.args | 33 +++-- tests/test-wrap-argv.pl | 106 ++++++++++++++++ 4 files changed, 247 insertions(+), 39 deletions(-) create mode 100644 tests/test-wrap-argv.pl
To avoid sending a 10 MB email, I'v only include these 3 example file changes. To view *all* the changes see my staging branch
https://gitlab.com/berrange/libvirt/commit/4fe45c39dae176ae4dbc7e126d2a2ff29...
Could you please remove the maintenance branches so I can add it for the future as a remote without having multiple sets of remote maintenance branches? Simple script, assuming "gitlab" is the name of your remote, is provided below for your convenience: git push gitlab $(git branch -a | \ sed -n '/^[^a-z]*remotes\/gitlab.*[0-9]-maint$/s_.*remotes/gitlab/_:_p')
'make check' and 'make syntax-check' of course still pass after applying the big change.
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl new file mode 100644 index 0000000..e8c782b --- /dev/null +++ b/tests/test-wrap-argv.pl @@ -0,0 +1,106 @@ +#!/usr/bin/perl + + + +foreach my $file (@ARGV) { + &rewrap($file); +} + +sub rewrap { + my $file = shift; + + # Read the original file + open FILE, "<", $file or die "cannot read $file: $!"; + my @lines; + while (<FILE>) { + # If there is a trailing '\' then kill the new line + if (/\\$/) { + chomp;
You could've removed newlines from all lines and rewrap everything so we're consistent. It looks like this works, But I would like to know your opinion on the suggestion above. If you disagree with that, then ACK to both patches, but let me know what you think, please. Have a nice weekend Martin
On Fri, Nov 06, 2015 at 04:00:04PM +0100, Martin Kletzander wrote:
On Fri, Nov 06, 2015 at 01:30:35PM +0000, Daniel P. Berrange wrote:
Back in
commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b Author: Juerg Haefliger <juerg.haefliger@hp.com> Date: Mon Jan 31 06:42:57 2011 -0500
tests: handle backspace-newline pairs in test input files
all the test argv files were line wrapped so that the args were less than 80 characters.
The way the line wrapping was done turns out to be quite undesirable, because it often leaves multiple parameters on the same line. If we later need to add or remove individual parameters, then it leaves us having to redo line wrapping.
This commit changes the line wrapping so that every single "-param value" is one its own new line. If the "value" is still too long, then we break on ',' or ':' or ' ' as needed.
What if we fix the syntax-check instead and allow longer than 80 character lines in case they have no space in it, or exactly one space (to allow --parameter option,option,option,...)? That would make even corner cases easier to review, e.g. when you remove or add a parameter into the long list of parameters.
The alternative that I'm actually thinking is that we make syntax-check skip line length on all these .args files. Instead re-run the 'test-wrap-argv.pl' script during syntax-check, and validate that no changeas are made. This will guarantee that all our .args files are always using our ideal formatting / line wrapping rules. Then I could remove these two special cases and just let us have "-arg value" lines exceed 80 characters when they need to
'make check' and 'make syntax-check' of course still pass after applying the big change.
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl new file mode 100644 index 0000000..e8c782b --- /dev/null +++ b/tests/test-wrap-argv.pl @@ -0,0 +1,106 @@ +#!/usr/bin/perl + + + +foreach my $file (@ARGV) { + &rewrap($file); +} + +sub rewrap { + my $file = shift; + + # Read the original file + open FILE, "<", $file or die "cannot read $file: $!"; + my @lines; + while (<FILE>) { + # If there is a trailing '\' then kill the new line + if (/\\$/) { + chomp;
You could've removed newlines from all lines and rewrap everything so we're consistent.
NB, some files have multiple commands in them, so we can't blindly remove all new lines. We have to preserve boundaries between commands. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, Nov 06, 2015 at 03:20:31PM +0000, Daniel P. Berrange wrote:
On Fri, Nov 06, 2015 at 04:00:04PM +0100, Martin Kletzander wrote:
On Fri, Nov 06, 2015 at 01:30:35PM +0000, Daniel P. Berrange wrote:
Back in
commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b Author: Juerg Haefliger <juerg.haefliger@hp.com> Date: Mon Jan 31 06:42:57 2011 -0500
tests: handle backspace-newline pairs in test input files
all the test argv files were line wrapped so that the args were less than 80 characters.
The way the line wrapping was done turns out to be quite undesirable, because it often leaves multiple parameters on the same line. If we later need to add or remove individual parameters, then it leaves us having to redo line wrapping.
This commit changes the line wrapping so that every single "-param value" is one its own new line. If the "value" is still too long, then we break on ',' or ':' or ' ' as needed.
What if we fix the syntax-check instead and allow longer than 80 character lines in case they have no space in it, or exactly one space (to allow --parameter option,option,option,...)? That would make even corner cases easier to review, e.g. when you remove or add a parameter into the long list of parameters.
The alternative that I'm actually thinking is that we make syntax-check skip line length on all these .args files.
Instead re-run the 'test-wrap-argv.pl' script during syntax-check, and validate that no changeas are made.
This will guarantee that all our .args files are always using our ideal formatting / line wrapping rules.
Then I could remove these two special cases and just let us have "-arg value" lines exceed 80 characters when they need to
OK, if that doesn't take more time then then simple check, so that the build time isn't getting longer and longer.
'make check' and 'make syntax-check' of course still pass after applying the big change.
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl new file mode 100644 index 0000000..e8c782b --- /dev/null +++ b/tests/test-wrap-argv.pl @@ -0,0 +1,106 @@ +#!/usr/bin/perl + + + +foreach my $file (@ARGV) { + &rewrap($file); +} + +sub rewrap { + my $file = shift; + + # Read the original file + open FILE, "<", $file or die "cannot read $file: $!"; + my @lines; + while (<FILE>) { + # If there is a trailing '\' then kill the new line + if (/\\$/) { + chomp;
You could've removed newlines from all lines and rewrap everything so we're consistent.
NB, some files have multiple commands in them, so we can't blindly remove all new lines. We have to preserve boundaries between commands.
Oh, I missed that.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, Nov 06, 2015 at 04:32:52PM +0100, Martin Kletzander wrote:
On Fri, Nov 06, 2015 at 03:20:31PM +0000, Daniel P. Berrange wrote:
On Fri, Nov 06, 2015 at 04:00:04PM +0100, Martin Kletzander wrote:
On Fri, Nov 06, 2015 at 01:30:35PM +0000, Daniel P. Berrange wrote:
Back in
commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b Author: Juerg Haefliger <juerg.haefliger@hp.com> Date: Mon Jan 31 06:42:57 2011 -0500
tests: handle backspace-newline pairs in test input files
all the test argv files were line wrapped so that the args were less than 80 characters.
The way the line wrapping was done turns out to be quite undesirable, because it often leaves multiple parameters on the same line. If we later need to add or remove individual parameters, then it leaves us having to redo line wrapping.
This commit changes the line wrapping so that every single "-param value" is one its own new line. If the "value" is still too long, then we break on ',' or ':' or ' ' as needed.
What if we fix the syntax-check instead and allow longer than 80 character lines in case they have no space in it, or exactly one space (to allow --parameter option,option,option,...)? That would make even corner cases easier to review, e.g. when you remove or add a parameter into the long list of parameters.
The alternative that I'm actually thinking is that we make syntax-check skip line length on all these .args files.
Instead re-run the 'test-wrap-argv.pl' script during syntax-check, and validate that no changeas are made.
This will guarantee that all our .args files are always using our ideal formatting / line wrapping rules.
Then I could remove these two special cases and just let us have "-arg value" lines exceed 80 characters when they need to
OK, if that doesn't take more time then then simple check, so that the build time isn't getting longer and longer.
As an unscientific benchmark, it takes 0.08 seconds to check all the .args files on my machine with warm cache. the make syntax-check will warm the cache already, so we should be fine. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, Nov 06, 2015 at 03:20:31PM +0000, Daniel P. Berrange wrote:
On Fri, Nov 06, 2015 at 04:00:04PM +0100, Martin Kletzander wrote:
On Fri, Nov 06, 2015 at 01:30:35PM +0000, Daniel P. Berrange wrote:
Back in
commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b Author: Juerg Haefliger <juerg.haefliger@hp.com> Date: Mon Jan 31 06:42:57 2011 -0500
tests: handle backspace-newline pairs in test input files
all the test argv files were line wrapped so that the args were less than 80 characters.
The way the line wrapping was done turns out to be quite undesirable, because it often leaves multiple parameters on the same line. If we later need to add or remove individual parameters, then it leaves us having to redo line wrapping.
This commit changes the line wrapping so that every single "-param value" is one its own new line. If the "value" is still too long, then we break on ',' or ':' or ' ' as needed.
What if we fix the syntax-check instead and allow longer than 80 character lines in case they have no space in it, or exactly one space (to allow --parameter option,option,option,...)? That would make even corner cases easier to review, e.g. when you remove or add a parameter into the long list of parameters.
The alternative that I'm actually thinking is that we make syntax-check skip line length on all these .args files.
Instead re-run the 'test-wrap-argv.pl' script during syntax-check, and validate that no changeas are made.
This will guarantee that all our .args files are always using our ideal formatting / line wrapping rules.
Then I could remove these two special cases and just let us have "-arg value" lines exceed 80 characters when they need to
So if we *never* line wrap within a '-arg value' parameter then we would have a fair number of lines longer than 80 characters. 80 qemuxml2argv-arm-virt-virtio.args 80 qemuxml2argv-cpu-strict1.args 80 qemuxml2argv-disk-drive-cache-v2-none.args 80 qemuxml2argv-disk-drive-network-sheepdog.args 80 qemuxml2argv-disk-snapshot.args 81 qemuxml2argv-arm-vexpressa9-virtio.args 81 qemuxml2argv-boot-menu-disable-drive-bootindex.args 81 qemuxml2argv-disk-drive-no-boot.args 81 qemuxml2argv-disk-virtio-ccw.args 81 qemuxml2argv-disk-virtio-ccw-many.args 82 qemuxml2argv-bios-nvram.args 82 qemuxml2argv-disk-drive-cache-unsafe.args 83 qemuxml2argv-disk-drive-copy-on-read.args 83 qemuxml2argv-serial-spiceport.args 84 qemuxml2argv-video-device-pciaddr-default.args 84 qemuxml2argv-video-qxl-device.args 84 qemuxml2argv-video-qxl-sec-device.args 84 qemuxml2argv-video-vga-device.args 84 qemuxml2argv-video-vga-device-vgamem.args 85 qemuxml2argv-disk-drive-cache-v2-wb.args 85 qemuxml2argv-disk-drive-network-iscsi.args 85 qemuxml2argv-numatune-memnode-no-memory.args 86 qemuxml2argv-arm-vexpressa9-basic.args 86 qemuxml2argv-cpu-host-model.args 86 qemuxml2argv-cpu-minimum1.args 86 qemuxml2argv-disk-drive-cache-directsync.args 86 qemuxml2argv-net-virtio-device.args 87 qemuxml2argv-disk-drive-network-gluster.args 87 qemuxml2argv-virtio-rng-random.args 88 qemuxml2argv-disk-cdrom-network-ftp.args 88 qemuxml2argv-disk-drive-cache-v2-wt.args 88 qemuxml2argv-usb-ich9-companion.args 88 qemuxml2argv-usb-redir.args 88 qemuxml2argv-usb-redir-boot.args 89 qemuxml2argv-disk-cdrom-network-http.args 89 qemuxml2argv-disk-cdrom-network-tftp.args 89 qemuxml2argv-disk-ide-wwn.args 89 qemuxml2argv-net-vhostuser.args 89 qemuxml2argv-net-virtio-netdev.args 89 qemuxml2argv-pci-bridge-many-disks.args 89 qemuxml2argv-pci-many.args 89 qemuxml2argv-serial-udp-chardev.args 89 qemuxml2argv-usb1-usb2.args 89 qemuxml2argv-usb-ich9-ehci-addr.args 90 qemuxml2argv-disk-cdrom-network-ftps.args 91 qemuxml2argv-cpu-host-model-fallback.args 91 qemuxml2argv-disk-aio.args 91 qemuxml2argv-disk-cdrom-network-https.args 91 qemuxml2argv-disk-serial.args 91 qemuxml2argv-fs9p-ccw.args 92 qemuxml2argv-virtio-lun.args 93 qemuxml2argv-boot-complex.args 93 qemuxml2argv-console-virtio-ccw.args 93 qemuxml2argv-disk-drive-error-policy-enospace.args 93 qemuxml2argv-disk-order.args 93 qemuxml2argv-hostdev-scsi-readonly.args 93 qemuxml2argv-hostdev-scsi-virtio-scsi.args 93 qemuxml2argv-virtio-rng-ccw.args 94 qemuxml2argv-disk-geometry.args 94 qemuxml2argv-disk-scsi-disk-split.args 94 qemuxml2argv-fs9p.args 95 qemuxml2argv-tpm-passthrough.args 96 qemuxml2argv-aarch64-virtio-pci.args 96 qemuxml2argv-channel-spicevmc-old.args 96 qemuxml2argv-disk-scsi-megasas.args 96 qemuxml2argv-disk-scsi-virtio-scsi.args 96 qemuxml2argv-disk-scsi-vscsi.args 96 qemuxml2argv-disk-source-pool-mode.args 96 qemuxml2argv-disk-virtio-scsi-ccw.args 96 qemuxml2argv-disk-virtio-scsi-cmd_per_lun.args 96 qemuxml2argv-disk-virtio-scsi-ioeventfd.args 96 qemuxml2argv-disk-virtio-scsi-max_sectors.args 96 qemuxml2argv-disk-virtio-scsi-num_queues.args 96 qemuxml2argv-pseries-vio.args 96 qemuxml2argv-pseries-vio-user-assigned.args 96 qemuxml2argv-video-qxl-device-vgamem.args 96 qemuxml2argv-video-qxl-sec-device-vgamem.args 97 qemuxml2argv-disk-scsi-lun-passthrough.args 97 qemuxml2argv-encrypted-disk.args 98 qemuxml2argv-disk-cdrom-tray.args 98 qemuxml2argv-disk-copy_on_read.args 98 qemuxml2argv-graphics-spice-timeout.args 100 qemuxml2argv-iothreads-disk-virtio-ccw.args 101 qemuxml2argv-hostdev-scsi-lsi-iscsi.args 101 qemuxml2argv-hostdev-scsi-virtio-iscsi.args 102 qemuxml2argv-disk-drive-network-iscsi-lun.args 103 qemuxml2argv-cpu-minimum2.args 103 qemuxml2argv-disk-drive-error-policy-stop.args 103 qemuxml2argv-iothreads-disk.args 104 qemuxml2argv-graphics-spice-sasl.args 104 qemuxml2argv-net-vhostuser-multiq.args 105 qemuxml2argv-boot-complex-bootindex.args 105 qemuxml2argv-boot-order.args 105 qemuxml2argv-boot-strict.args 105 qemuxml2argv-hostdev-scsi-boot.args 106 qemuxml2argv-blkdeviotune.args 106 qemuxml2argv-disk-ioeventfd.args 106 qemuxml2argv-event_idx.args 107 qemuxml2argv-channel-spicevmc.args 107 qemuxml2argv-channel-virtio-default.args 107 qemuxml2argv-controller-order.args 107 qemuxml2argv-disk-drive-error-policy-wreport-rignore.args 107 qemuxml2argv-shmem.args 108 qemuxml2argv-cpu-host-model-vendor.args 108 qemuxml2argv-disk-drive-shared.args 109 qemuxml2argv-hugepages-pages2.args 109 qemuxml2argv-hugepages-pages3.args 110 qemuxml2argv-pci-rom.args 111 qemuxml2argv-channel-virtio.args 111 qemuxml2argv-channel-virtio-unix.args 111 qemuxml2argv-migrate-numa-unaligned.args 111 qemuxml2argv-numatune-memnode.args 112 qemuxml2argv-channel-virtio-autoadd.args 112 qemuxml2argv-channel-virtio-auto.args 112 qemuxml2argv-channel-virtio-autoassign.args 112 qemuxml2argv-channel-virtio-state.args 112 qemuxml2argv-graphics-spice-agentmouse.args 113 qemuxml2argv-disk-blockio.args 113 qemuxml2argv-disk-drive-discard.args 116 qemuxml2argv-usb-redir-filter.args 117 qemuxml2argv-disk-scsi-disk-wwn.args 121 qemuxml2argv-graphics-spice-qxl-vga.args 129 qemuxml2argv-disk-drive-network-iscsi-auth.args 129 qemuxml2argv-disk-scsi-disk-vpd.args 129 qemuxml2argv-smartcard-host-certificates.args 131 qemuxml2argv-disk-iscsi.args 135 qemuxml2argv-memory-hotplug-dimm-addr.args 135 qemuxml2argv-memory-hotplug-dimm.args 136 qemuxml2argv-hugepages-numa.args 137 qemuxml2argv-hugepages-pages.args 144 qemuxml2argv-usb-redir-filter-version.args 145 qemuxml2argv-graphics-spice-agent-file-xfer.args 147 qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args 147 qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args 147 qemuxml2argv-hugepages-shared.args 155 qemuxml2argv-disk-drive-network-rbd.args 186 qemuxml2argv-disk-drive-network-rbd-ipv6.args 196 qemuxml2argv-smbios.args 203 qemuxml2argv-blkdeviotune-max.args 209 qemuxml2argv-graphics-spice-compression.args 225 qemuxml2argv-disk-drive-network-rbd-auth.args 238 qemuxml2argv-net-virtio-disable-offloads.args 291 qemuxml2argv-graphics-spice-usb-redir.args 314 qemuxml2argv-graphics-spice.args I didn't realize it was quite so huge with these. With spice reaching 314 characters for a single parameter value, I feel we really have to artificially split the value strings. 314 is just too long for a single line. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 11/06/2015 09:16 AM, Daniel P. Berrange wrote:
What if we fix the syntax-check instead and allow longer than 80 character lines in case they have no space in it, or exactly one space (to allow --parameter option,option,option,...)? That would make even corner cases easier to review, e.g. when you remove or add a parameter into the long list of parameters.
So if we *never* line wrap within a '-arg value' parameter then we would have a fair number of lines longer than 80 characters.
314 qemuxml2argv-graphics-spice.args
I didn't realize it was quite so huge with these. With spice reaching 314 characters for a single parameter value, I feel we really have to artificially split the value strings. 314 is just too long for a single line.
Indeed - it makes hunting for the diff within that line difficult when reading email where the lines are wrapped anyways by the mail client display. 314 is not as bad as the 998 byte limit where git send-email was choking and not even sending patches, but it is still too long for comfort, so I'm okay with making the automatic wrapper pick strategic ',' points to wrap at on long lines. But we can also compromise, such as perhaps making the cutoff for mid-word wrapping be at 100 bytes instead of 80 to touch fewer files. At any rate, the idea of having 'syntax-check' validate that auto-wrapping doesn't modify any files seems like a good one. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Nov 06, 2015 at 09:23:20AM -0700, Eric Blake wrote:
On 11/06/2015 09:16 AM, Daniel P. Berrange wrote:
What if we fix the syntax-check instead and allow longer than 80 character lines in case they have no space in it, or exactly one space (to allow --parameter option,option,option,...)? That would make even corner cases easier to review, e.g. when you remove or add a parameter into the long list of parameters.
So if we *never* line wrap within a '-arg value' parameter then we would have a fair number of lines longer than 80 characters.
314 qemuxml2argv-graphics-spice.args
I didn't realize it was quite so huge with these. With spice reaching 314 characters for a single parameter value, I feel we really have to artificially split the value strings. 314 is just too long for a single line.
Indeed - it makes hunting for the diff within that line difficult when reading email where the lines are wrapped anyways by the mail client display.
314 is not as bad as the 998 byte limit where git send-email was choking and not even sending patches, but it is still too long for comfort, so I'm okay with making the automatic wrapper pick strategic ',' points to wrap at on long lines. But we can also compromise, such as perhaps making the cutoff for mid-word wrapping be at 100 bytes instead of 80 to touch fewer files.
At any rate, the idea of having 'syntax-check' validate that auto-wrapping doesn't modify any files seems like a good one.
I've sent a v2 that adds the syntax-check hook. I decided to just keep it at 80 characters for simplicity rather than allowing larger special cases. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Martin Kletzander