[pve-devel] [PATCH pve-zsync] fix #1301 Do not allow file path as mp.

[Fabian Grünbichler]

comments inline

On Wed, Mar 15, 2017 at 08:58:52AM +0100, Wolfgang Link wrote:
> If a file path is used as mp in lxc, you get a error message that it is not include in the sync.
> ---
>  pve-zsync | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/pve-zsync b/pve-zsync
> index 4993bed..3ea4b47 100644
> --- a/pve-zsync
> +++ b/pve-zsync
> @@ -769,20 +769,20 @@ sub parse_disks {
>      my $num = 0;
>      while ($text && $text =~ s/^(.*?)(\n|$)//) {
>  	my $line = $1;
> -	my $error = $vm_type eq 'qemu' ? 1 : 0 ;
> +	my $expose_error = $vm_type eq 'qemu' ? 1 : 0 ;
>  
>  	next if $line =~ /cdrom|none/;
>  	next if $line !~ m/^(?:((?:virtio|ide|scsi|sata|mp)\d+)|rootfs): /;
>  
>  	#QEMU if backup is not set include in  sync
> -	next if $vm_type eq 'qemu && ($line =~ m/backup=(?i:0|no|off|false)/)';
> +	next if $vm_type eq 'qemu' && ($line =~ m/backup=(?i:0|no|off|false)/);

that change is not mentioned at all in the commit message? should
probably be a separate commit..

>  
>  	#LXC if backup is not set do no in sync
> -	$error = ($line =~ m/backup=(?i:1|yes|on|true)/) if $vm_type eq 'lxc';
> +	$expose_error = ($line =~ m/backup=(?i:1|yes|on|true)/) if $vm_type eq 'lxc';

doesn't this line already handle the problem? if I don't want a bind or
device mount point to be backed up, I set backup=0 and pve-zsync will
not complain. if I want to back it up (i.e., I set backup=1) then
pve-zsync should complain, because otherwise unsuspecting users might
think that the mount point is somehow magically backed up, even though
that is technically impossible to do via zfs send/receive..

>  
>  	my $disk = undef;
>  	my $stor = undef;
> -	if($line =~ m/^(?:(?:(?:virtio|ide|scsi|sata|mp)\d+)|rootfs): (.*)$/) {
> +	if($line =~ m@^(?:(?:(?:virtio|ide|scsi|sata|mp)\d+)|rootfs): ([^/].*)$@) {

see above, also this would now fail for mount points that have the
optional "volume=" key for the volume.

>  	    my @parameter = split(/,/,$1);
>  
>  	    foreach my $opt (@parameter) {
> @@ -794,7 +794,7 @@ sub parse_disks {
>  	    }
>  
>  	} else {
> -	    print "Disk: \"$line\" will not include in pve-sync\n" if $get_err || $error;
> +	    print "Disk: \"$line\" will not include in pve-sync\n" if $get_err || $expose_error;
>  	    next;
>  	}
>  
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel