[libvirt] [PATCH 3/5] storage: Add TRIM algorithm to storage volume API

[John Ferlan]

On 01/27/2016 05:20 AM, Wido den Hollander wrote:
> This new algorithm adds support for wiping volumes using TRIM.
> 
> It does not overwrite all the data in a volume, but it tells the
> backing storage pool/driver that all bytes in a volume can be
> discarded.
> 
> It depends on the backing storage pool how this is handled.
> 
> A SCSI backend might send UNMAP commands to remove all data present
> on a LUN.
> 
> A Ceph backend might use rbd_discard() to instruct the Ceph cluster
> that all data on that RBD volume can be discarded.
> 
> Signed-off-by: Wido den Hollander <wido at widodh.nl>
> ---
>  include/libvirt/libvirt-storage.h | 3 +++
>  src/storage/storage_backend.c     | 4 ++++
>  tools/virsh-volume.c              | 2 +-
>  tools/virsh.pod                   | 1 +
>  4 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h
> index 2c55c93..232b4d3 100644
> --- a/include/libvirt/libvirt-storage.h
> +++ b/include/libvirt/libvirt-storage.h
> @@ -153,6 +153,9 @@ typedef enum {
>  
>      VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */
>  
> +    VIR_STORAGE_VOL_WIPE_ALG_TRIM = 9, /* 1-pass, trim all data on the
> +                                          volume by using TRIM or DISCARD */
> +
>  # ifdef VIR_ENUM_SENTINELS
>      VIR_STORAGE_VOL_WIPE_ALG_LAST
>      /*
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 15e9109..1bb44a7 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -2065,6 +2065,7 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
>                    VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 |
>                    VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 |
>                    VIR_STORAGE_VOL_WIPE_ALG_RANDOM |
> +                  VIR_STORAGE_VOL_WIPE_ALG_TRIM |
>                    VIR_STORAGE_VOL_WIPE_ALG_LAST, -1);
>  
>      VIR_DEBUG("Wiping volume with path '%s' and algorithm %u",
> @@ -2112,6 +2113,9 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
>          case VIR_STORAGE_VOL_WIPE_ALG_RANDOM:
>              alg_char = "random";
>              break;
> +        case VIR_STORAGE_VOL_WIPE_ALG_TRIM:
> +            alg_char = "trim";
> +            break;

This would be bad... Passing "trim" to the SCRUB image would cause a
failure...

I've replaced with:

    case VIR_STORAGE_VOL_WIPE_ALG_TRIM:
        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
                       _("'trim' algorithm not supported"));
        goto cleanup;

Since at this point none of the drivers that utilize this function
supports the 'trim' option (in fact none do at this point).

>          default:
>              virReportError(VIR_ERR_INVALID_ARG,
>                             _("unsupported algorithm %d"),
> diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
> index 661c876..35f0cbd 100644
> --- a/tools/virsh-volume.c
> +++ b/tools/virsh-volume.c
> @@ -906,7 +906,7 @@ static const vshCmdOptDef opts_vol_wipe[] = {
>  VIR_ENUM_DECL(virStorageVolWipeAlgorithm)
>  VIR_ENUM_IMPL(virStorageVolWipeAlgorithm, VIR_STORAGE_VOL_WIPE_ALG_LAST,
>                "zero", "nnsa", "dod", "bsi", "gutmann", "schneier",
> -              "pfitzner7", "pfitzner33", "random");
> +              "pfitzner7", "pfitzner33", "random", "trim");
>  
>  static bool
>  cmdVolWipe(vshControl *ctl, const vshCmd *cmd)
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index e830c59..b259507 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -3546,6 +3546,7 @@ B<Supported algorithms>
>    pfitzner7  - Roy Pfitzner's 7-random-pass method: random x7.
>    pfitzner33 - Roy Pfitzner's 33-random-pass method: random x33.
>    random     - 1-pass pattern: random.
> +  trim       - 1-pass trimming the volume using TRIM or DISCARD
>  
>  B<Note>: The availability of algorithms may be limited by the version
>  of the C<scrub> binary installed on the host.
> 

I adjusted the <Note> text to be:

B<Note>: The C<scrub> binary will be used to handle the 'nnsa', 'dod',
'bsi', 'gutmann', 'schneier', 'pfitzner7' and 'pfitzner33' algorithms.
The availability of the algorithms may be limited by the version of
the C<scrub> binary installed on the host. The 'zero' algorithm will
write zeroes to the entire volume. For some volumes, such as sparse
or rbd volumes, this may result in completely filling the volume with
zeroes making it appear to be completely full. As an alternative, the
'trim' algorithm does not overwrite all the data in a volume, rather
it expects the storage driver to be able to discard all bytes in a
volume. It is up to the storage driver to handle how the discarding
occurs. Not all storage drivers or volume types can support 'trim'.


Also, since I fixed the switch in 2/5 for storage_backend_rbd.c to not
use 'default:' - I added a temporary case for TRIM there (which gets
resolved in the next patch).

Tks - This has been pushed.

John