[libvirt] [PATCH 0/3] Add delete-snapshots option to virsh commands
Commit id '3c7590e0' added the ability to delete snapshots for rbd backends, but did not provide the virsh commands in order to handle that option. The first two patches fix a couple of minor nits - not using virCheckFlags in virStorageBackendRBDDeleteVol even though the flags are used. Also, the virStorageVolDeleteFlags did not document the possible flags. Added the reference for the API. The third patch is the meat where the --delete-snapshots flag to virsh commands 'undefine' and 'vol-delete' was added and handled. cc'd author of '3c7590e0' - hopefully this series could be applied and tested in a "real" environment. John Ferlan (3): storage: Add virCheckFlags to virStorageBackendRBDDeleteVol libvirt: Add virStorageVolDeleteFlags to virStorageVolDelete virsh: Add --delete-snapshots flag for undefine and vol-delete src/libvirt-storage.c | 2 +- src/storage/storage_backend_rbd.c | 3 +++ tools/virsh-domain.c | 14 +++++++++++++- tools/virsh-volume.c | 12 +++++++++++- tools/virsh.pod | 14 +++++++++++++- 5 files changed, 41 insertions(+), 4 deletions(-) -- 2.5.0
The initial commit '74951eade' did not include the proper check for whether any flags are supported by the driver. Even though the driver doesn't support VIR_STORAGE_VOL_DELETE_ZEROED, it still checks and allows the processing to continue Also add the new VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS since it is handled as of commit id '3c7590e0a'. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_rbd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index b66fcbe..cdbfdee 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -513,6 +513,9 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn, ptr.cluster = NULL; ptr.ioctx = NULL; + virCheckFlags(VIR_STORAGE_VOL_DELETE_ZEROED | + VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS, -1); + VIR_DEBUG("Removing RBD image %s/%s", pool->def->source.name, vol->name); if (flags & VIR_STORAGE_VOL_DELETE_ZEROED) -- 2.5.0
Although they've been present for quite a while, they weren't added to the API definition, so add them there to make it clearer. Currently only the RBD backend even checks for any flags. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt-storage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c index 66dd9f0..c195ca9 100644 --- a/src/libvirt-storage.c +++ b/src/libvirt-storage.c @@ -1684,7 +1684,7 @@ virStorageVolUpload(virStorageVolPtr vol, /** * virStorageVolDelete: * @vol: pointer to storage volume - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virStorageVolDeleteFlags * * Delete the storage volume from the pool * -- 2.5.0
https://bugzilla.redhat.com/show_bug.cgi?id=1281710 Commit id '3c7590e0a' added the flag to the rbd backend, but provided no means via virsh to use the flag. This patch adds a '--delete-snapshots' option to both the "undefine" and "vol-delete" commands. For "undefine", the flag is combined with the "--remove-all-storage" flag in order to add the appropriate flag for the virStorageVolDelete call; whereas, for the "vol-delete" command, just the flag is sufficient since it's only operating on one volume. Currently only supported for rbd backends. Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-domain.c | 14 +++++++++++++- tools/virsh-volume.c | 12 +++++++++++- tools/virsh.pod | 14 +++++++++++++- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 413220b..726e995 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3674,6 +3674,11 @@ static const vshCmdOptDef opts_undefine[] = { .type = VSH_OT_BOOL, .help = N_("remove all associated storage volumes (use with caution)") }, + {.name = "delete-snapshots", + .type = VSH_OT_BOOL, + .help = N_("delete snapshots associated with volume(s), requires " + "--remove-all-storage (must be supported by storage driver)") + }, {.name = "wipe-storage", .type = VSH_OT_BOOL, .help = N_("wipe data on the removed volumes") @@ -3703,11 +3708,13 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; /* Flags to attempt. */ unsigned int flags = 0; + unsigned int vol_flags = 0; /* User-requested actions. */ bool managed_save = vshCommandOptBool(cmd, "managed-save"); bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata"); bool wipe_storage = vshCommandOptBool(cmd, "wipe-storage"); bool remove_all_storage = vshCommandOptBool(cmd, "remove-all-storage"); + bool delete_snapshots = vshCommandOptBool(cmd, "delete-snapshots"); bool nvram = vshCommandOptBool(cmd, "nvram"); /* Positive if these items exist. */ int has_managed_save = 0; @@ -3736,6 +3743,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) size_t j; virshControlPtr priv = ctl->privData; + VSH_REQUIRE_OPTION("delete-snapshots", "remove-all-storage"); + ignore_value(vshCommandOptString(ctl, cmd, "storage", &vol_string)); if (!(vol_string || remove_all_storage) && wipe_storage) { @@ -3745,6 +3754,9 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) return false; } + if (delete_snapshots) + vol_flags |= VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS; + if (managed_save) { flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE; managed_save_safe = true; @@ -4011,7 +4023,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) } /* delete the volume */ - if (virStorageVolDelete(vols[i].vol, 0) < 0) { + if (virStorageVolDelete(vols[i].vol, vol_flags) < 0) { vshError(ctl, _("Failed to remove storage volume '%s'(%s)"), vols[i].target, vols[i].source); ret = false; diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 7d76a06..c9c656f 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -887,6 +887,11 @@ static const vshCmdOptDef opts_vol_delete[] = { .type = VSH_OT_STRING, .help = N_("pool name or uuid") }, + {.name = "delete-snapshots", + .type = VSH_OT_BOOL, + .help = N_("delete snapshots associated with volume (must be " + "supported by storage driver)") + }, {.name = NULL} }; @@ -896,11 +901,16 @@ cmdVolDelete(vshControl *ctl, const vshCmd *cmd) virStorageVolPtr vol; bool ret = true; const char *name; + bool delete_snapshots = vshCommandOptBool(cmd, "delete-snapshots"); + unsigned int flags = 0; if (!(vol = virshCommandOptVol(ctl, cmd, "vol", "pool", &name))) return false; - if (virStorageVolDelete(vol, 0) == 0) { + if (delete_snapshots) + flags |= VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS; + + if (virStorageVolDelete(vol, flags) == 0) { vshPrint(ctl, _("Vol %s deleted\n"), name); } else { vshError(ctl, _("Failed to delete vol %s"), name); diff --git a/tools/virsh.pod b/tools/virsh.pod index 21ae4a3..075bf03 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2295,7 +2295,9 @@ Output the device used for the TTY console of the domain. If the information is not available the processes will provide an exit code of 1. =item B<undefine> I<domain> [I<--managed-save>] [I<--snapshots-metadata>] -[I<--nvram>] [ {I<--storage> B<volumes> | I<--remove-all-storage>} I<--wipe-storage>] +[I<--nvram>] +[ {I<--storage> B<volumes> | I<--remove-all-storage> [I<--delete-snapshots>]} +I<--wipe-storage>] Undefine a domain. If the domain is running, this converts it to a transient domain, without stopping it. If the domain is inactive, @@ -2330,6 +2332,11 @@ Example: --storage vda,/path/to/storage.img The I<--remove-all-storage> flag specifies that all of the domain's storage volumes should be deleted. +The I<--delete-snapshots> flag specifies that any snapshots associated with +the storage volume should be deleted as well. Requires the +I<--remove-all-storage> flag to be provided. Not all storage drivers +support this option, presently only rbd. + The flag I<--wipe-storage> specifies that the storage volumes should be wiped before removal. @@ -3438,12 +3445,17 @@ where the data blocks are copied only when modified. If this is not possible, the copy fails. =item B<vol-delete> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path> +[I<--delete-snapshots>] Delete a given volume. I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool the volume is in. I<vol-name-or-key-or-path> is the name or key or path of the volume to delete. +The I<--delete-snapshots> flag specifies that any snapshots associated with +the storage volume should be deleted as well. Not all storage drivers +support this option, presently only rbd. + =item B<vol-upload> [I<--pool> I<pool-or-uuid>] [I<--offset> I<bytes>] [I<--length> I<bytes>] I<vol-name-or-key-or-path> I<local-file> -- 2.5.0
ping? Thanks - John On 12/02/2015 06:18 PM, John Ferlan wrote:
Commit id '3c7590e0' added the ability to delete snapshots for rbd backends, but did not provide the virsh commands in order to handle that option.
The first two patches fix a couple of minor nits - not using virCheckFlags in virStorageBackendRBDDeleteVol even though the flags are used. Also, the virStorageVolDeleteFlags did not document the possible flags. Added the reference for the API.
The third patch is the meat where the --delete-snapshots flag to virsh commands 'undefine' and 'vol-delete' was added and handled.
cc'd author of '3c7590e0' - hopefully this series could be applied and tested in a "real" environment.
John Ferlan (3): storage: Add virCheckFlags to virStorageBackendRBDDeleteVol libvirt: Add virStorageVolDeleteFlags to virStorageVolDelete virsh: Add --delete-snapshots flag for undefine and vol-delete
src/libvirt-storage.c | 2 +- src/storage/storage_backend_rbd.c | 3 +++ tools/virsh-domain.c | 14 +++++++++++++- tools/virsh-volume.c | 12 +++++++++++- tools/virsh.pod | 14 +++++++++++++- 5 files changed, 41 insertions(+), 4 deletions(-)
On 12/17/2015 09:54 PM, John Ferlan wrote:
ping?
Looks good to me. Didn't think you needed a reply from me. But yes, I forgot this part. Only went for the API since that was what I needed in CloudStack.
Thanks -
John
On 12/02/2015 06:18 PM, John Ferlan wrote:
Commit id '3c7590e0' added the ability to delete snapshots for rbd backends, but did not provide the virsh commands in order to handle that option.
The first two patches fix a couple of minor nits - not using virCheckFlags in virStorageBackendRBDDeleteVol even though the flags are used. Also, the virStorageVolDeleteFlags did not document the possible flags. Added the reference for the API.
The third patch is the meat where the --delete-snapshots flag to virsh commands 'undefine' and 'vol-delete' was added and handled.
cc'd author of '3c7590e0' - hopefully this series could be applied and tested in a "real" environment.
John Ferlan (3): storage: Add virCheckFlags to virStorageBackendRBDDeleteVol libvirt: Add virStorageVolDeleteFlags to virStorageVolDelete virsh: Add --delete-snapshots flag for undefine and vol-delete
src/libvirt-storage.c | 2 +- src/storage/storage_backend_rbd.c | 3 +++ tools/virsh-domain.c | 14 +++++++++++++- tools/virsh-volume.c | 12 +++++++++++- tools/virsh.pod | 14 +++++++++++++- 5 files changed, 41 insertions(+), 4 deletions(-)
On 03.12.2015 00:18, John Ferlan wrote:
Commit id '3c7590e0' added the ability to delete snapshots for rbd backends, but did not provide the virsh commands in order to handle that option.
The first two patches fix a couple of minor nits - not using virCheckFlags in virStorageBackendRBDDeleteVol even though the flags are used. Also, the virStorageVolDeleteFlags did not document the possible flags. Added the reference for the API.
The third patch is the meat where the --delete-snapshots flag to virsh commands 'undefine' and 'vol-delete' was added and handled.
cc'd author of '3c7590e0' - hopefully this series could be applied and tested in a "real" environment.
John Ferlan (3): storage: Add virCheckFlags to virStorageBackendRBDDeleteVol libvirt: Add virStorageVolDeleteFlags to virStorageVolDelete virsh: Add --delete-snapshots flag for undefine and vol-delete
src/libvirt-storage.c | 2 +- src/storage/storage_backend_rbd.c | 3 +++ tools/virsh-domain.c | 14 +++++++++++++- tools/virsh-volume.c | 12 +++++++++++- tools/virsh.pod | 14 +++++++++++++- 5 files changed, 41 insertions(+), 4 deletions(-)
ACK series Michal
participants (3)
-
John Ferlan -
Michal Privoznik -
Wido den Hollander