[pve-devel] applied: [PATCH storage 2/5] import/export: new formats: raw, tar, qcow2, vmdk

[Dietmar Maurer]

comments inline

> On June 21, 2017 at 2:59 PM Wolfgang Bumiller <w.bumiller at proxmox.com> wrote:
> 
> 
> All of them have a `+size` prefix to show that they're not
> "pure raw" or "pure tar" streams, because some storage may
> need to know in advance how much storage to allocate.
> The formats are explained in comments.
> 
> PVE::Storage::Plugin now has default implementations for
> these for non-incremental streams exporting the current
> (rather than a snapshot state).
> To use qcow2 or vmdk formats $with_snapshots must be true,
> otherwise raw/tar will be used where $with_snapshots must
> be false.
> ---
>  PVE/CLI/pvesm.pm      |   7 ++-
>  PVE/Storage/Plugin.pm | 148
> ++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 148 insertions(+), 7 deletions(-)
> 
> diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
> index 8af4e7f..9adc8ba 100755
> --- a/PVE/CLI/pvesm.pm
> +++ b/PVE/CLI/pvesm.pm
> @@ -3,6 +3,7 @@ package PVE::CLI::pvesm;
>  use strict;
>  use warnings;
>  
> +use POSIX qw(O_RDONLY O_WRONLY O_CREAT O_TRUNC);
>  use Fcntl ':flock';
>  use File::Path;
>  
> @@ -21,7 +22,7 @@ use PVE::CLIHandler;
>  
>  use base qw(PVE::CLIHandler);
>  
> -my $KNOWN_EXPORT_FORMATS = ['zfs'];
> +my $KNOWN_EXPORT_FORMATS = ['raw+size', 'tar+size', 'qcow2+size',
> 'vmdk+size', 'zfs'];
>  
>  my $nodename = PVE::INotify::nodename();
>  
> @@ -202,7 +203,7 @@ __PACKAGE__->register_method ({
>  	if ($filename eq '-') {
>  	    $outfh = \*STDOUT;
>  	} else {
> -	    open($outfh, '>', $filename)
> +	    sysopen($outfh, $filename, O_CREAT|O_WRONLY|O_TRUNC)
>  		or die "open($filename): $!\n";


why do we need sysopen here?

>  	}
>  
> @@ -277,7 +278,7 @@ __PACKAGE__->register_method ({
>  	if ($filename eq '-') {
>  	    $infh = \*STDIN;
>  	} else {
> -	    open($infh, '<', $filename)
> +	    sysopen($infh, $filename, O_RDONLY)
>  		or die "open($filename): $!\n";
>  	}
>  
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index cc4c7e7..7bde330 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -888,26 +888,166 @@ sub check_connection {
>      return 1;
>  }
>  
> +# Import/Export interface:
> +#   Any path based storage is assumed to support 'raw' and 'tar' streams, so
> +#   the default implementations will return this if $scfg->{path} is set,
> +#   mimicking the old PVE::Storage::storage_migrate() function.
> +#
> +# Plugins may fall back to PVE::Storage::Plugin::volume_{export,import}...
> +#   functions in case the format doesn't match their specialized
> +#   implementations to reuse the raw/tar code.
> +#
> +# Format specification:
> +#   The following formats are all prefixed with image information in the form
> +#   of a 64 bit little endian unsigned integer (pack('Q<')) in order to be
> able
> +#   to preallocate the image on storages which require it.
> +#
> +#   raw+size: (image files only)
> +#     A raw binary data stream such as produced via `dd if=TheImageFile`.
> +#   qcow2+size, vmdk: (image files only)
> +#     A raw qcow2/vmdk/... file such as produced via `dd if=some.qcow2` for
> +#     files which are already in qcow2 format, or via `qemu-img convert`.
> +#     Note that these formats are only valid with $with_snapshots being true.
> +#   tar+size: (subvolumes only)
> +#     A GNU tar stream with the inner contents of the subvolume put into the
> +#     'subvol/' directory.
> +
> +# Plugins may reuse these helpers. Changes to the header format should be
> +# reflected by changes to the function prototypes.
> +sub write_common_header($$) {
> +    my ($fh, $image_size_in_bytes) = @_;
> +    syswrite($fh, pack("Q<", $image_size_in_bytes), 8);
> +}
> +
> +sub read_common_header($) {
> +    my ($fh) = @_;
> +    sysread($fh, my $size, 8);
> +    $size = unpack('Q<', $size);
> +    die "got a bad size (not a multiple of 1K)\n" if ($size&1023);
> +    # Size is in bytes!
> +    return $size;
> +}
> +
>  # Export a volume into a file handle as a stream of desired format.
>  sub volume_export {
>      my ($class, $scfg, $storeid, $fh, $volname, $format, $snapshot,
> $base_snapshot, $with_snapshots) = @_;
> -    die "volume export not implemented for $class";
> +    if ($scfg->{path} && !defined($snapshot) && !defined($base_snapshot)) {
> +	my $file = $class->path($scfg, $volname, $storeid)
> +	    or goto unsupported;
> +	my ($size, $file_format) = file_size_info($file);
> +
> +	if ($format eq 'raw+size') {
> +	    goto unsupported if $with_snapshots || $file_format eq 'subvol';
> +	    write_common_header($fh, $size);
> +	    if ($file_format eq 'raw') {
> +		run_command(['dd', "if=$file", "bs=4k"], output => '>&'.fileno($fh));
> +	    } else {
> +		run_command(['qemu-img', 'convert', '-f', $file_format, '-O', 'raw', $file,
> '/dev/stdout'],
> +		            output => '>&'.fileno($fh));

is it worth to use two different command here? I guess quemu-img can handle both
cases?

> +	    }
> +	    return;
> +	} elsif ($format =~ /^(qcow2|vmdk)\+size$/) {
> +	    my $data_format = $1;
> +	    goto unsupported if !$with_snapshots || $file_format ne $data_format;
> +	    write_common_header($fh, $size);
> +	    run_command(['dd', "if=$file", "bs=4k"], output => '>&'.fileno($fh));
> +	    return;
> +	} elsif ($format eq 'tar+size') {
> +	    goto unsupported if $file_format ne 'subvol';
> +	    write_common_header($fh, $size);
> +	    run_command(['tar', '--xform=s,^\./,subvol/,S', '-cf', '-', '-C', $file,
> '.'],
> +	                output => '>&'.fileno($fh));

why do we need --xform here?

Also, is '-cf' good enough? We use the following flags for vzdump backups:
 
-cpf 
--one-file-system 
--warning=no-file-ignored
--sparse
--numeric-owner
--acls
--xattrs'
--xattrs-include=user.*
--xattrs-include=security.capability
--warning=no-xattr-write

?