Re: [pve-devel] [PATCH v2 qemu-server 1/5] disk reassign: add API endpoint

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On September 3, 2020 10:30 am, Aaron Lauterer wrote:
> 
> 
> On 9/3/20 9:46 AM, Fabian Grünbichler wrote:
>> On September 1, 2020 2:44 pm, Aaron Lauterer wrote:
> 
> 
> [..]
> 
>>> +
>>> +	    my $drive = PVE::QemuServer::parse_drive($disk, $conf->{$disk});
>>> +	    die "disk '$disk' has no associated volume\n" if !$drive->{file};
>> 
>> missing check for whether it's actually volume-based, and not
>> pass-through.. what about templates/base volumes/linked
>> clones/snapshots?
> 
> Passed through disks will fail later on in the code but having a check right away with a nicer error msg is probably a good idea. I haven't thought about templates, base volumes and linked clones yet. Same goes for Snapshots :/
> 
> I guess we won't want to rename a ZFS dataset with snapshots present? Another thing is that renaming the dataset will most likely break replication.
> 

from the top of my head, but please verify/check for missing stuff:

snapshot: can't re-assign if volume is referenced in a snapshot (would 
invalidate that reference!), need to remove snapshot(s) first
base volume: does not make much sense (except if it does not have any clones, in 
which case we could re-assign to another template but that seems rather 
contrived)
linked volume: needs special handling in storage (pass and encode base 
volume/template VMID so that new volid is still correct)
replicated source: reassigning from a VM that is replicated should work (from 
replication's PoV, that's just like removing a disk -> it will get 
cleaned up on the next run), but some special handling to also remove 
the replication snapshots might be a good idea to return the volume to a 
clean slate
replicated target: should work, but check that volume is replicatable 
(like when adding a disk) might make sense?