[pve-devel] [PATCH v3 qemu-server 08/11] blockdev: convert drive_mirror to blockdev_mirror

[DERUMIER, Alexandre]

> +    my $path = PVE::Storage::path($storecfg, $volid);

>>is this guaranteed to be stable? also across versions? and including
>>external storage plugins?

it can't be different than the value we have use for command line
generation. But I think that I should use $path directly (It's working
for block/file  ,  but I think it'll not work with ceph,gluster,...)
I need to reuse the code used to generated the blockdev commande line.

Another way, maybe a better way,is to parse the tree from the top node
(the throttle-group) where the name is fixed.  and look for fmt|file
chain attached to this node.

(I just need need to check when we are a doing live renaming, we have 2
files nodes, with the newer file node not attached to the tree before
the switch)



> +
> +    my $node = find_blockdev_node($nodes, $path, 'fmt');

>>that one is only added in a later patch.. but I don't think lookups
>>by path are a good idea, we should probably have a deterministic node
>>naming concept instead? e.g., encode the drive + snapshot name?

I really would like to have something deterministic but:

- devices node are 31 characters max.   (snapshot name can be more big)
- we can't rename a node  (but we are renaming files for snapshot over
time)


As Fiona said, we could have random names and do 1 lookup each time to
list them.

(I really need to have our own name, because blockdev-reopen, for live
renaming of files, is not working with autogenerated block# name)





> +    return $node->{'node-name'};
> +}
> +
> +sub get_blockdev_nextid {
> +    my ($nodename, $nodes) = @_;
> +    my $version = 0;
> +    for my $nodeid (keys %$nodes) {
> + if ($nodeid =~ m/^$nodename-(\d+)$/) {
> +     my $current_version = $1;
> +     $version = $current_version if $current_version >= $version;
> + }
> +    }
> +    $version++;
> +    return "$nodename-$version";

>>since we shouldn't ever have more than one job for a drive running
(right?), couldn't we just have a deterministic name for this? that
>>would also simplify cleanup, including cleanup of a failed cleanup ;)

Still same, 
- you need 2 file nodes at the same time for live renaming
- you can have 2 fmt nodes for blockdev-mirror at same time.