[pve-devel] [RFC pve-container 6/6] vzdump: use the new run_command variant

Wolfgang Bumiller w.bumiller at proxmox.com
Tue Sep 15 10:07:22 CEST 2015


Use the array-of-array version of run_command to build the
pipe, this should deal with most quoting issues.
Note that tar handles glob patterns in --exclude itself, so
quoting patterns instead of letting the shell resolve them
is also actually more correct.
---
 src/PVE/VZDump/LXC.pm | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
index 83e2ab7..8dfc7cc 100644
--- a/src/PVE/VZDump/LXC.pm
+++ b/src/PVE/VZDump/LXC.pm
@@ -267,42 +267,41 @@ sub archive {
     my $snapdir = $task->{snapdir};
     my $tmpdir = $task->{tmpdir};
 
-    my $taropts = "--totals --sparse --numeric-owner --xattrs --one-file-system";
+    my $tar = ['tar', 'cpf', '-',
+               '--totals', '--sparse', '--numeric-owner', '--xattrs',
+               '--one-file-system'];
 
     # note: --remove-files does not work because we do not 
     # backup all files (filters). tar complains:
     # Cannot rmdir: Directory not empty
     # we we disable this optimization for now
     #if ($snapdir eq $task->{tmpdir} && $snapdir =~ m|^$opts->{dumpdir}/|) {
-    #       $taropts .= " --remove-files"; # try to save space
+    #       push @$tar, "--remove-files"; # try to save space
     #}
 
-    my $cmd = "tar cpf - $taropts ";
     # The directory parameter can give a alternative directory as source.
     # the second parameter gives the structure in the tar.
-    $cmd .= "--directory=$tmpdir ./etc/vzdump/pct.conf ";
-    $cmd .= "--directory=$snapdir";
-
-    foreach my $exclude (@{$self->{vzdump}->{findexcl}}) {
-	$cmd .= " --exclude=.$exclude";
-    }
+    push @$tar, "--directory=$tmpdir", './etc/vzdump/pct.conf';
+    push @$tar, "--directory=$snapdir";
+    push @$tar, map { "--exclude=.$_" } @{$self->{vzdump}->{findexcl}};
 
     # add every enabled mountpoint (since we use --one-file-system)
     my $disks = $task->{disks};
     # mp already starts with a / so we only need to add the dot
-    foreach my $disk (@$disks) {
-	$cmd .= " .$disk->{mp}";
-    }
+    push @$tar, map { '.' . $_->{mp} } @$disks;
+
+    my $cmd = [ $tar ];
 
     my $bwl = $opts->{bwlimit}*1024; # bandwidth limit for cstream
-    $cmd .= "|cstream -t $bwl" if $opts->{bwlimit};
-    $cmd .= "|$comp" if $comp;
+    push @$cmd, [ 'cstream', '-t', $bwl ] if $opts->{bwlimit};
+    push @$cmd, [ $comp ] if $comp;
 
     if ($opts->{stdout}) {
-	$self->cmd ($cmd, output => ">&" . fileno($opts->{stdout}));
+	push @{$cmd->[-1]}, \(">&" . fileno($opts->{stdout}));
     } else {
-	$self->cmd ("$cmd >" . PVE::Tools::shellquote($filename));
+	push @{$cmd->[-1]}, \(">" . PVE::Tools::shellquote($filename));
     }
+    $self->cmd($cmd);
 }
 
 sub cleanup {
-- 
2.1.4





More information about the pve-devel mailing list