Re: [pve-devel] [PATCH storage v2] fix-4272: btrfs: add rename feature

[Aaron Lauterer <a.lauterer@proxmox.com>]

did you test if it works if a manually placed qcow2 or vmdk disk image 
is used as discussed in my previous responses and in person?

There currently are no checks in place, as there are for all the other 
functions in the BTRFS plugin that deal with creating/changing disk images.

One more thing inline.

On  2024-07-03  14:41, Maximiliano Sandoval wrote:
> Adds the ability to change the owner of a guest image.
> 
> Btrfs does not need special commands to rename a subvolume and this can
> be achieved the same as in Storage/plugin.pm's rename_volume taking
> special care of how the directory structure used by Btrfs.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>   src/PVE/Storage/BTRFSPlugin.pm | 40 ++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)
> 
> diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
> index 42815cb..9f71d78 100644
> --- a/src/PVE/Storage/BTRFSPlugin.pm
> +++ b/src/PVE/Storage/BTRFSPlugin.pm
> @@ -7,6 +7,7 @@ use base qw(PVE::Storage::Plugin);
>   
>   use Fcntl qw(S_ISDIR O_WRONLY O_CREAT O_EXCL);
>   use File::Basename qw(basename dirname);
> +use File::Copy 'move';
>   use File::Path qw(mkpath);
>   use IO::Dir;
>   use POSIX qw(EEXIST);
> @@ -618,6 +619,9 @@ sub volume_has_feature {
>   	    base => { qcow2 => 1, raw => 1, vmdk => 1 },
>   	    current => { qcow2 => 1, raw => 1, vmdk => 1 },
>   	},
> +	rename => {
> +	    current => { raw => 1 },
> +	},
>       };
>   
>       my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) = $class->parse_volname($volname);
> @@ -930,4 +934,40 @@ sub volume_import {
>       return "$storeid:$volname";
>   }
>   
> +sub rename_volume {
> +    my ($class, $scfg, $storeid, $source_volname, $target_vmid, $target_volname) = @_;
> +    die "no path found\n" if !$scfg->{path};
> +
> +    my (
> +	undef,
> +	undef,
> +	undef,
> +	undef,
> +	undef
> +	undef,
> +	$format
> +    ) = $class->parse_volname($source_volname);
> +
> +    my $ppath = $class->filesystem_path($scfg, $source_volname);
> +
> +    $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format, 1)
> +	if !$target_volname;
> +    $target_volname = "$target_vmid/$target_volname";
> +
> +    my $basedir = $class->get_subdir($scfg, 'images');
> +
> +    mkpath "${basedir}/${target_vmid}";
> +    my $source_dir = raw_name_to_dir($source_volname);
> +    my $target_dir = raw_name_to_dir($target_volname);
> +
> +    my $old_path = "${basedir}/${source_dir}";
> +    my $new_path = "${basedir}/${target_dir}";
> +
> +    die "target volume '${target_volname}' already exists\n" if -e $new_path;
> +    move $old_path, $new_path ||
did you try if it would just work with 'rename' like in the main 
implementation used in `Plugin.pm`?
Then we could drop the whole import of File::Copy::move

> +	die "rename '$old_path' to '$new_path' failed - $!\n";
> +
> +    return "${storeid}:$target_volname";
> +}
> +
>   1


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel