[pve-devel] [PATCH] implement vdisk_clone and vdisk_create_base

[Alexandre DERUMIER]

> >>> local:6000/base-6000-disk-9.raw 
> >>> local:6000/base-6000-disk-9.raw/7000/vm-7000-disk-9.qcow2 
> >>> local:6000/base-6000-disk-9.raw/7000/base-7000-disk-10.qcow2 
> 
> >>That way it is easy to check if a volume is a clone or a template. Do 
> >>you think this can be implemented for all storage types? 
> 
> Any changes since what we discuss monday ? This is like this I have 
> impletemed it. 
> for file : local:6000/base-6000-disk-9.raw/7000/vm-7000-disk-9.qcow2 

>>Maybe I missed that. But I have done some tests and the volume list returned 
>>wrong names without base image information? 
Yes,Indeed, I done it differently, which change in parse_volume_id in storage.pm, 
to parse the /base/ and return volid without the base part.

But I think indeed, your implementation is better, so we make change in parse_volname in each plugin, 
to return informations about parent.


>>Also, a clone of a clone seem to return wrong values with your patches. 
Oh, maybe, sorry.

> for rbd,sheepdog,... : rbd:base-6000-disk-9/vm-7000-disk-9 
> 
> Note that currently, I assume that if volume is a base volume and that 
> storage need a snapshot to clone, the snapshot name is always @base. (I 
> create this snapshot when I create the base volume) 
> 
> But maybe a 
> 
> rbd:base-6000-disk-9 at base/vm-7000-disk-9 
> is better ? 

>>I think using a fixed name is good enough for now. No need to encode that name into the volid. 
OK

> Another thing we didn't talk, do you want to keep the full chain of 
> relationship in case of clone of clone of clone... 
> something like 6000/base-6000-disk-9.raw/7000/vm-7000-disk- 
> 9.qcow2/8000/vm-8000-disk-1.qcow2/9000/vm-9000-disk-1.qcow2 

>>No, I do not want that. 
OK
> >>Note: Clone from internal snapshot does not fit into my picture 
> >>currently. But I guess we can add that later. 
> By internal snapshot, do you mean "Live snapshot" from a vm ? (vm cloning 
> without creating a template) 

>>I just implemented volume_clone() without the 'snap' parameter now. 
>>
>>I will extend that when we have the basic things working (step by step). 
>>
>>Is that OK for you? 

For rbd,sheepdog,nexenta : 
- We can assume that snapshot name to clone is always  "@base" if the volume is a base volume.
- Or specify the snapname in base volid. (But you don't want base-6000-disk-9 at base/vm-7000-disk-9)
We only need to pass a snap parameter if we want to clone from a vm live snapshot (so if it's not a base volume), but this can wait.

> >>You patch series is simply too large for me. I need to implement things 
> step by step, 
> >>so I though it is best to start to update the storage lib. I will try to commit 
> this functionality 
> >>step by step, so that you can adapt you series. 
> Yes, I understand (I'm working on this since 2 months, so not easy to dig in 
> it). 

>>Many thanks for your patience! 
No problem , I'm waiting for copy/clone feature since a while, I can wait a little more ;-)


> Maybe can we implement it for 2 types of storage behaviour 
> -for file (no snapshot to clone, only a full volume). ( 
> -for a storage which need disk snapshot to clone, like rbd for example. 
> (sheepdog,nexenta,.. is more or less copy/paste) 
> 
> What do you think ? 

>>Yes, I have done it for file base storage (code is in git). 
>>
>>Maybe you can adopt your patch for one of the other storage types (rbd, nexenta)? 

Yes, I'm thinked about rbd.

> I'll test your patch this morning, to see how it's work and I'll send some 
> comments/questions. 

>>That would be great. From what I see you need to fix the list function, and the values 
>>return by volume_clone (now called vdisk_clone). 

Ok, I'll work on rbd today, to see if it's working fine.