Re: [libvirt] [PATCH 1/2] rbd: Implement wiping RBD volumes using rbd_discard()

On Wed, Dec 23, 2015 at 01:50:52PM +0100, Wido den Hollander wrote: > > > On 23-12-15 10:45, Daniel P. Berrange wrote: >> On Wed, Dec 23, 2015 at 10:29:04AM +0100, Wido den Hollander wrote: >>> This allows user to use the volume wiping functionality of the libvirt >>> storage driver. >>> >>> All data from the volume will be wiped by calling rbd_discard() in chunks of the >>> underlying object and stripe size inside Ceph. >>> >>> This will ensure all data is zeroed and leaves the user with a completely empty volume >>> ready for use again. >> >> Based on the name 'rbd_discard' it sounds like this is going to call >> TRIM/DISCARD on the underlying storage too ? If so, then I don't think >> that this is an appropriate approach for this API. The virStorageVolWipe >> API should clear the data, *without* having any effect on the storage of >> the API - ie we don't want to discard underling storage blocks as a >> side effect >> > > Afaik it zeroes/trims all the RBD objects on the Ceph cluster, but it > doesn't TRIM the lower SSD on it's turn. > > So it will send these calls to Ceph/RBD and it will zero all the data of > that specific volume. A rather simple way to get rid of the data in a > volume. That's not what I see that API impl of rbd_discard() doing. It looks very much like it is discarding extents from the RBD volume, at least if the discarded region is large enough. Only if the discarded region is small, does it merely zero the data.

So I really don't think this is a suitable API for use with the virStorageVolWipe() API, whose *only* effect should be to overwrite the data, not have any side effect on volume extent allocation

Regards, Daniel