On 11/3/23 11:39, Fiona Ebner wrote:
[...]
This essentially duplicates most of the same function in the parent
plugin, i.e. LVMPlugin. What you can do to avoid it, is introduce new
helper functions for the parts that are different, call those in
LVMPlugin's implementation and overwrite the helpers appropriately in
LvmThinPlugin. Then call the parent plugin's function here and do the
base conversion at the end.
yes makes sense to work with the LVMPlugin implementation
rather then duplicating all the code, good point.
+sub volume_import {
(...)
+
+ # Request new vm-name which is needed for the import
+ if ($isBase) {
+ my $newvmname = $class->find_free_diskname($storeid, $scfg, $vmid);
+ $name = $newvmname;
+ $volname = $newvmname;
+ }
So this is one of the parts that needs to be different.
You could also just set the name to undef and alloc_image will call
find_free_diskname itself. Might be slightly nicer and avoids a new
find_free_diskname call. We could also set the name to vm-XYZ-disk-N
instead of base-XYZ-disk-N for the allocation I think. Or the whole
function could even be:
1. check if base
2. check if already exists/rename allowed
3. call parent plugin's volume_import function passing along
vm-XYZ-disk-N (or undef if it should be renamed) instead of base-XYZ-disk-N
4. if it was a base image, handle the conversion to base image
Then there's no need for new helpers either. But this is just a sketch
of course, didn't think through all details. What do you think?
that sounds good so far, but you can't get around the
"find_free_diskspace()" call, can you ?
Not specifying a name leads to an error in the volume_import of the LVM
plugin,
because without a name no plugin can be parsed and to get a new name in
the style
of "vm-XYZ-disk-N" you need "find_free_diskspace()" to make sure that
the name does not yet exist,
or am I missing something ?
So I'd propose:
1. check if base
2. check if already exists/rename allowed
3. call parent plugin's volume_import function passing along
vm-XYZ-disk-N generated with "find_free_diskspace(), instead of
base-XYZ-disk-N
4. if it was a base image, handle the conversion to base image
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
