From b77b203cac1ad5e3eec1c73728c670bf464ce069 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 6 Oct 2011 15:01:18 -0600 Subject: [PATCH] snapshot: virsh shorthand for operating on current snap Rather than having to do: $ virsh snapshot-revert dom $(virsh snapshot-current dom --name) I thought it would be nice to do: $ virsh snapshot-revert dom --current I didn't add 'virsh snapshot-dumpxml --current' since we already have 'virsh snapshot-current' for the same task. snapshot-list accepted a name but did not require it, and that remains the case, with --current serving in place of that name. For all other commands, name used to be required, and can now be replaced by --current; I intentionally made it so that omitting both --current and a name is an error (having the absence of a name imply --current seems just a bit too magic, so --current must be explicit). I also had to keep snapshot-edit backwards-compatible, as the only command that already had a --current argument alongside a name, which still works to both edit a named snapshot and make it current. * tools/virsh.c (vshLookupSnapshot): New helper function. (cmdSnapshotEdit, cmdSnapshotList, cmdSnapshotParent) (cmdSnapshotDelete, cmdDomainSnapshotRevert): Use it, adding an option where needed. * tools/virsh.pod (snapshot-delete, snapshot-edit) (snapshot-list, snapshot-parent, snapshot-revert): Document use of --current. (snapshot-dumpxml): Mention alternative. --- tools/virsh.c | 106 +++++++++++++++++++++++++++++++----------------- tools/virsh.pod | 31 ++++++++------ 2 files changed, 87 insertions(+), 50 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 067d3e5b64..bcf06034f6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12817,6 +12817,45 @@ cleanup: return ret; } +/* Helper for resolving {--current | --ARG name} into a snapshot + * belonging to DOM. If EXCLUSIVE, fail if both --current and arg are + * present. On success, populate *SNAP and *NAME, before returning 0. + * On failure, return -1 after issuing an error message. */ +static int +vshLookupSnapshot(vshControl *ctl, const vshCmd *cmd, + const char *arg, bool exclusive, virDomainPtr dom, + virDomainSnapshotPtr *snap, const char **name) +{ + bool current = vshCommandOptBool(cmd, "current"); + const char *snapname = NULL; + + if (vshCommandOptString(cmd, arg, &snapname) < 0) { + vshError(ctl, _("invalid argument for --%s"), arg); + return -1; + } + + if (exclusive && current && snapname) { + vshError(ctl, _("--%s and --current are mutually exclusive"), arg); + return -1; + } + + if (snapname) { + *snap = virDomainSnapshotLookupByName(dom, snapname, 0); + } else if (current) { + *snap = virDomainSnapshotCurrent(dom, 0); + } else { + vshError(ctl, _("--%s or --current is required"), arg); + return -1; + } + if (!*snap) { + virshReportError(ctl); + return -1; + } + + *name = virDomainSnapshotGetName(*snap); + return 0; +} + /* * "snapshot-edit" command */ @@ -12828,7 +12867,7 @@ static const vshCmdInfo info_snapshot_edit[] = { static const vshCmdOptDef opts_snapshot_edit[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"snapshotname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("snapshot name")}, + {"snapshotname", VSH_OT_DATA, 0, N_("snapshot name")}, {"current", VSH_OT_BOOL, 0, N_("also set edited snapshot as current")}, {"rename", VSH_OT_BOOL, 0, N_("allow renaming an existing snapshot")}, {"clone", VSH_OT_BOOL, 0, N_("allow cloning to new name")}, @@ -12858,21 +12897,19 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) return false; } - if (vshCommandOptBool(cmd, "current")) + if (vshCommandOptBool(cmd, "current") && + vshCommandOptBool(cmd, "snapshotname")) define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT; if (!vshConnectionUsability(ctl, ctl->conn)) return false; - if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) - goto cleanup; - dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) goto cleanup; - snapshot = virDomainSnapshotLookupByName(dom, name, 0); - if (snapshot == NULL) + if (vshLookupSnapshot(ctl, cmd, "snapshotname", false, dom, + &snapshot, &name) < 0) goto cleanup; /* Get the XML configuration of the snapshot. */ @@ -13147,6 +13184,8 @@ static const vshCmdOptDef opts_snapshot_list[] = { N_("list only snapshots that have metadata that would prevent undefine")}, {"tree", VSH_OT_BOOL, 0, N_("list snapshots in a tree")}, {"from", VSH_OT_DATA, 0, N_("limit list to children of given snapshot")}, + {"current", VSH_OT_BOOL, 0, + N_("limit list to children of current snapshot")}, {"descendants", VSH_OT_BOOL, 0, N_("with --from, list all descendants")}, {NULL, 0, 0, NULL} }; @@ -13180,10 +13219,17 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) int start_index = -1; bool descendants = false; - if (vshCommandOptString(cmd, "from", &from) < 0) { - vshError(ctl, _("invalid from argument '%s'"), from); + if (!vshConnectionUsability(ctl, ctl->conn)) + goto cleanup; + + dom = vshCommandOptDomain(ctl, cmd, NULL); + if (dom == NULL) + goto cleanup; + + if ((vshCommandOptBool(cmd, "from") || + vshCommandOptBool(cmd, "current")) && + vshLookupSnapshot(ctl, cmd, "from", true, dom, &start, &from) < 0) goto cleanup; - } if (vshCommandOptBool(cmd, "parent")) { if (vshCommandOptBool(cmd, "roots")) { @@ -13214,18 +13260,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SNAPSHOT_LIST_METADATA; } - if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; - - dom = vshCommandOptDomain(ctl, cmd, NULL); - if (dom == NULL) - goto cleanup; - if (from) { descendants = vshCommandOptBool(cmd, "descendants"); - start = virDomainSnapshotLookupByName(dom, from, 0); - if (!start) - goto cleanup; if (descendants || tree) { flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; } @@ -13556,7 +13592,8 @@ static const vshCmdInfo info_snapshot_parent[] = { static const vshCmdOptDef opts_snapshot_parent[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"snapshotname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("snapshot name")}, + {"snapshotname", VSH_OT_DATA, 0, N_("find parent of snapshot name")}, + {"current", VSH_OT_BOOL, 0, N_("find parent of current snapshot")}, {NULL, 0, 0, NULL} }; @@ -13576,11 +13613,8 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) - goto cleanup; - - snapshot = virDomainSnapshotLookupByName(dom, name, 0); - if (snapshot == NULL) + if (vshLookupSnapshot(ctl, cmd, "snapshotname", true, dom, + &snapshot, &name) < 0) goto cleanup; if (vshGetSnapshotParent(ctl, snapshot, &parent) < 0) @@ -13615,7 +13649,8 @@ static const vshCmdInfo info_snapshot_revert[] = { static const vshCmdOptDef opts_snapshot_revert[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"snapshotname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("snapshot name")}, + {"snapshotname", VSH_OT_DATA, 0, N_("snapshot name")}, + {"current", VSH_OT_BOOL, 0, N_("revert to current snapshot")}, {"running", VSH_OT_BOOL, 0, N_("after reverting, change state to running")}, {"paused", VSH_OT_BOOL, 0, N_("after reverting, change state to paused")}, {"force", VSH_OT_BOOL, 0, N_("try harder on risky reverts")}, @@ -13651,11 +13686,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) - goto cleanup; - - snapshot = virDomainSnapshotLookupByName(dom, name, 0); - if (snapshot == NULL) + if (vshLookupSnapshot(ctl, cmd, "snapshotname", true, dom, + &snapshot, &name) < 0) goto cleanup; result = virDomainRevertToSnapshot(snapshot, flags); @@ -13691,7 +13723,8 @@ static const vshCmdInfo info_snapshot_delete[] = { static const vshCmdOptDef opts_snapshot_delete[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, - {"snapshotname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("snapshot name")}, + {"snapshotname", VSH_OT_DATA, 0, N_("snapshot name")}, + {"current", VSH_OT_BOOL, 0, N_("delete current snapshot")}, {"children", VSH_OT_BOOL, 0, N_("delete snapshot and all children")}, {"children-only", VSH_OT_BOOL, 0, N_("delete children but not snapshot")}, {"metadata", VSH_OT_BOOL, 0, @@ -13715,7 +13748,8 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) + if (vshLookupSnapshot(ctl, cmd, "snapshotname", true, dom, + &snapshot, &name) < 0) goto cleanup; if (vshCommandOptBool(cmd, "children")) @@ -13725,10 +13759,6 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "metadata")) flags |= VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY; - snapshot = virDomainSnapshotLookupByName(dom, name, 0); - if (snapshot == NULL) - goto cleanup; - /* XXX If we wanted, we could emulate DELETE_CHILDREN_ONLY even on * older servers that reject the flag, by manually computing the * list of descendants. But that's a lot of code to maintain. */ diff --git a/tools/virsh.pod b/tools/virsh.pod index 7712db4731..d0b79379a9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1951,12 +1951,14 @@ the XML. With I, this is a request to make the existing named snapshot become the current snapshot, without reverting the domain. -=item B I I [I<--current>] +=item B I [I] [I<--current>] {[I<--rename>] | [I<--clone>]} Edit the XML configuration file for I of a domain. If -I<--current> is specified, also force the edited snapshot to become -the current snapshot. +both I and I<--current> are specified, also force the +edited snapshot to become the current snapshot. If I +is omitted, then I<--current> must be supplied, to edit the current +snapshot. This is equivalent to: @@ -1978,7 +1980,7 @@ snapshots, such as internal snapshots within a single qcow2 file, are accessible only from the original name. =item B I [{I<--parent> | I<--roots> | I<--tree>}] -[I<--metadata>] [[I<--from>] B [I<--descendants>]] +[I<--metadata>] [{[I<--from>] B | I<--current>} [I<--descendants>]] List all of the available snapshots for the given domain, defaulting to show columns for the snapshot name, creation time, and domain state. @@ -1990,7 +1992,8 @@ If I<--tree> is specified, the output will be in a tree format, listing just snapshot names. These three options are mutually exclusive. If I<--from> is provided, filter the list to snapshots which are -children of the given B. When used in isolation or with +children of the given B; or if I<--current> is provided, +start at the current snapshot. When used in isolation or with I<--parent>, the list is limited to direct children unless I<--descendants> is also present. When used with I<--tree>, the use of I<--descendants> is implied. This option is not compatible @@ -2005,15 +2008,18 @@ a transient domain. Output the snapshot XML for the domain's snapshot named I. Using I<--security-info> will also include security sensitive information. +Use B to easily access the XML of the current snapshot. -=item B I I +=item B I {I | I<--current>} -Output the name of the parent snapshot for the given I, if any. +Output the name of the parent snapshot, if any, for the given +I, or for the current snapshot with I<--current>. -=item B I I [{I<--running> | I<--paused>}] -[I<--force>] +=item B I {I | I<--current>} +[{I<--running> | I<--paused>}] [I<--force>] -Revert the given domain to the snapshot specified by I. Be aware +Revert the given domain to the snapshot specified by I, or to +the current snapshot with I<--current>. Be aware that this is a destructive action; any changes in the domain since the last snapshot was taken will be lost. Also note that the state of the domain after snapshot-revert is complete will be the state of the domain at the time @@ -2043,10 +2049,11 @@ snapshot that uses a provably incompatible configuration, as well as with an inactive snapshot that is combined with the I<--start> or I<--pause> flag. -=item B I I [I<--metadata>] +=item B I {I | I<--current>} [I<--metadata>] [{I<--children> | I<--children-only>}] -Delete the snapshot for the domain named I. If this snapshot +Delete the snapshot for the domain named I, or the current +snapshot with I<--current>. If this snapshot has child snapshots, changes from this snapshot will be merged into the children. If I<--children> is passed, then delete this snapshot and any children of this snapshot. If I<--children-only> is passed, then delete -- GitLab