ah, now I get it. please correct me if I misunderstood something

- get all targettoextent mappings and loop over them
- only look at those with the desired target and with a lunid that is in
  the "special range" reserved for snapshots and state volumes
- if the volume has an extent, check if it matches the one of the
  targettoextent mapping
- else, proceed to the next potentially still free slot and repeat

This is correct.

I see two remaining issues:
- you rely on an implict sorting of t2extents by lunid
- you always iterate over the whole thing (except for the "exist" case)

I'd suggest dropping the id++, and instead marking which of the
potential slots you have already seen (or how many, since LUNs should
not be duplicated in the result anyway I guess?), and moving the check
and abort into the foreach.

You might have a point here. I will reconsider this.

yes, but why do you GET "storage/snapshot?limit=$limit" in
freenas_list_zvol, but GET "storage/snapshot" without a limit here? it's
the same API endpoint, but inconsistenly used.

You are right but this is fixed in the new freenas_request construction for GET (All gets will be looped until no more rows returned)

no, you would just require manual setup just like for other storage
types as well:
- LVM, LVMThin, local ZFS all require local setup before adding the
- Ceph requires at least putting the keyring into place
- probably others I missed?

So have the user create a file in /etc/pve/private and read the values from this file instead of from storage.cfg?

you have use regular expressions like /^(base|vm)-(\d+)-disk-\d+$/ in
your code, where you just want to extract the VMID or similar. IMHO
those should use parse_volname, so that you only have one place where
you need to make sure that the RE is correct, and where you need to
change it if it needs changing.

I see:
- /^(base|vm)-(\d+)-disk-\d+$/ in freenas_list_zvols, only $2 is used
- /^(vm|base)-(\d+)-/ in freenas_get_target_name, only $2 is used
- /(vm|base)-$vmid-disk-(\d+)/ in freenas_find_free_diskname, only $2 is
  used (but this might be left as is, it is a special case and not
  returned by parse_volname anyway)
- /^(vm|base)-\d+-disk-(\d+)$ in freenas_get_lun_number, only $2 is used
  (not returned by parse_volname)

at least the former two should be updated, the latter two can stay as is
I guess.


so the first child is a meta-child that represents the sum of all
children? the more I know about this API the less I like it ;)

It is not the API, this is how FreeNAS does it ;-(
Each time you create a new pool a dataset by the same name is created under rpool (Not the why this is done in any other ZFS based implementations I know of)

just to make sure there is no misunderstanding here - the JSON module
provides ::true and ::false, so you don't need to bless anything

$ perl -e '
use strict;
use warnings;
use JSON;
my $one = { force => bless( do{\(my $o = 0)}, "JSON::XS::Boolean") };
my $two = { force => JSON::false };
print JSON::encode_json($one), "\n";
print JSON::encode_json($two), "\n";


see alloc_image in ZFSPoolPlugin.pm

This is also the way I have done it in v6 (missed if-exists construct)

Michael Rasmussen

Get my public GnuPG keys:
michael <at> rasmussen <dot> cc
mir <at> datanom <dot> net
mir <at> miras <dot> org


This mail was virus scanned and spam checked before delivery.
This mail is also DKIM signed. See header dkim-signature.

pve-devel mailing list

Reply via email to