On 5/23/24 14:25, Fabian Grünbichler wrote:
On May 23, 2024 12:40 pm, Dominik Csapak wrote:
On 5/22/24 12:08, Fabian Grünbichler wrote:
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
[snip]
+ $target_volname = "$vmid/" . $target_diskname;
this encodes a fact about volname semantics that might not be a given
for external, dir-based plugins (not sure if we want to worry about that
though, or how to avoid it ;)).
i mean we could call 'alloc' with a very small size instead
and simply "overwrite" it? then we'd also get around things like
mkpath and imagedir etc.
that might actually be nice(r) than the current approach since it avoids
the volname format issue entirely. the only downside is that we then
briefly have a "wrong" disk visible, but since the VM has to be locked
at that point there shouldn't be too much harm in that?
we also have a 'wrong' disk after extraction before the import step though
so yes I do think that approach should work fine
+ $target_path = $target_plugin->filesystem_path($target_scfg,
$target_volname);
this should be equivalent to PVE::Storage::path for DirPlugin based
storages?
+
+ print "renaming $source_path to $target_path\n";
+ my $imagedir = $target_plugin->get_subdir($target_scfg, 'images');
we already did this above, but see comment there ;)
true ;)
+ mkpath "$imagedir/$vmid";
+
+ rename($source_path, $target_path) or die "unable to move - $!\n";
+ };
+ if (my $err = $@) {
+ unlink $source_path;
+ unlink $target_path if defined($target_path);
isn't this pretty much impossible to happen? the last thing we do in the
eval block is the rename - if that failed, $target_path can't exist yet.
if it didn't fail, we can't end up here?
that probably depends on the underlying filesystem no? not
every fs has posix rename semantics i guess?
I think we can assume an intra-FS rename to either work and have an
effect, or not work and not have an effect on anything we want to
support as dir storage? :)
in that case we'd cleanup the file, and if it does not exists, it doesn't hurt
but
sure, but error handling tends to get more complicated over time, so not
having nops in there reduces the complexity somewhat IMHO.
fine with me :)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
