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

[Daniel P. Berrange]

On Wed, Dec 23, 2015 at 02:10:28PM +0100, Wido den Hollander wrote:
> 
> 
> On 23-12-15 14:05, Daniel P. Berrange wrote:
> > 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.
> > 
> 
> Let me verify this with the Ceph people.
> 
> > 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
> > 
> 
> What do you suggest? use rbd_write() in a loop and overwrite the whole
> volume?

That would match intended semantics of WIPE_ALG_ZERO albeit
rather slow

> The problem with this option is that the RBD volume will then grow to
> it's maximum size on the underlying storage.

This is the same semantics as we have for the other impls of the
Wipe API. The API is defined to fill the contents of the file
with the requested byte pattern, which neccessarily means that
the volume will grow to its maximum size.

> With rbd_discard() being called on the exact object size all those
> objects will be trimmed, effectively wiping the volume's data. So I
> figured it was OK.

We could introduce a  VIR_STORAGE_VOL_WIPE_ALG_DISCARD which is
defined to be the same as VIR_STORAGE_VOL_WIPE_ALG_ZERO, but
using discard semantics to release space.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|