From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id CFD161FF2AD for ; Thu, 4 Jul 2024 11:53:00 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1C50C1FB9B; Thu, 4 Jul 2024 11:53:17 +0200 (CEST) Message-ID: Date: Thu, 4 Jul 2024 11:52:42 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion , Maximiliano Sandoval References: <20240703124147.441210-1-m.sandoval@proxmox.com> Content-Language: en-US From: Aaron Lauterer In-Reply-To: <20240703124147.441210-1-m.sandoval@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.291 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URI_NOVOWEL 0.5 URI hostname has long non-vowel sequence Subject: Re: [pve-devel] [PATCH storage v2] fix-4272: btrfs: add rename feature X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 > --- > 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