shaba/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 17:34:18 +03:00
Code Releases Activity

snapshot: avoid accidental renames with snapshot-edit

Browse Source 
I was a bit surprised that 'virsh snapshot-edit dom name' silently
allowed me to clone things, while still telling me the old name,
especially since other commands like 'virsh edit dom' reject rename
attempts (*).  This fixes things to be more explicit (**).

(*) Technically, 'virsh edit dom' relies on virDomainDefineXML
behavior, which rejects attempts to mix a new name with existing
uuid or new uuid with existing name, but you can create a new
domain by changing both uuid and name.  On the other hand, while
snapshot-edit --clone is a true clone, creating a new domain
would also have to decide whether to clone snapshot metadata,
managed save, and any other secondary data related to the domain.
Domain renames are not trivial either.

(**) Renaming or creating a clone is still a risky proposition -
for offline snapshots and system checkpoints, if the new name
does not match an actual name recorded in the qcow2 internal
snapshots, then you cannot revert to the new checkpoint.  But it
is assumed that anyone using the new virsh flags knows what they
are doing, and can deal with the fallout caused by a rename/clone;
that is, we can't completely prevent a user from shooting
themselves in the foot, so much as we are making the default
action less risky.

* tools/virsh.c (cmdSnapshotEdit): Add --rename, --clone.
* tools/virsh.pod (snapshot-edit): Document them.
This commit is contained in:
Eric Blake 2011-10-07 16:20:58 -06:00
parent 40baa1c899
commit bab4f31c78
2 changed files with 49 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
tools/virsh.c
View File

@ -12830,6 +12830,8 @@ 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")},
{"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")},
{NULL, 0, 0, NULL}
};
@ -12838,13 +12840,23 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
virDomainSnapshotPtr snapshot = NULL;
virDomainSnapshotPtr edited = NULL;
const char *name;
const char *edited_name;
bool ret = false;
char *tmp = NULL;
char *doc = NULL;
char *doc_edited = NULL;
unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE;
unsigned int define_flags = VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
bool rename_okay = vshCommandOptBool(cmd, "rename");
bool clone_okay = vshCommandOptBool(cmd, "clone");
if (rename_okay && clone_okay) {
vshError(ctl, "%s",
_("--rename and --clone are mutually exclusive"));
return false;
}
if (vshCommandOptBool(cmd, "current"))
define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT;
@ -12867,8 +12879,6 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
doc = virDomainSnapshotGetXMLDesc(snapshot, getxml_flags);
if (!doc)
goto cleanup;
virDomainSnapshotFree(snapshot);
snapshot = NULL;
/* strstr is safe here, since xml came from libvirt API and not user */
if (strstr(doc, "<state>disk-snapshot</state>"))
@ -12899,13 +12909,36 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
}
/* Everything checks out, so redefine the xml. */
snapshot = virDomainSnapshotCreateXML(dom, doc_edited, define_flags);
if (!snapshot) {
edited = virDomainSnapshotCreateXML(dom, doc_edited, define_flags);
if (!edited) {
vshError(ctl, _("Failed to update %s"), name);
goto cleanup;
}
vshPrint(ctl, _("Snapshot %s edited.\n"), name);
edited_name = virDomainSnapshotGetName(edited);
if (STREQ(name, edited_name)) {
vshPrint(ctl, _("Snapshot %s edited.\n"), name);
} else if (clone_okay) {
vshPrint(ctl, _("Snapshot %s cloned to %s.\n"), name,
edited_name);
} else {
unsigned int delete_flags;
delete_flags = VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY;
if (virDomainSnapshotDelete(rename_okay ? snapshot : edited,
delete_flags) < 0) {
virshReportError(ctl);
vshError(ctl, _("Failed to clean up %s"),
rename_okay ? name : edited_name);
goto cleanup;
}
if (!rename_okay) {
vshError(ctl, _("Must use --rename or --clone to change %s to %s"),
name, edited_name);
goto cleanup;
}
}
ret = true;
cleanup:
@ -12917,6 +12950,8 @@ cleanup:
}
if (snapshot)
virDomainSnapshotFree(snapshot);
if (edited)
virDomainSnapshotFree(edited);
if (dom)
virDomainFree(dom);
return ret;

9
tools/virsh.pod
View File

@ -1952,6 +1952,7 @@ With I<snapshotname>, this is a request to make the existing named
snapshot become the current snapshot, without reverting the domain.
=item B<snapshot-edit> I<domain> I<snapshotname> [I<--current>]
{[I<--rename>] | [I<--clone>]}
Edit the XML configuration file for I<snapshotname> of a domain. If
I<--current> is specified, also force the edited snapshot to become
@ -1968,6 +1969,14 @@ except that it does some error checking.
The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment
variables, and defaults to C<vi>.
If I<--rename> is specified, then the edits can change the snapshot
name. If I<--clone> is specified, then changing the snapshot name
will create a clone of the snapshot metadata. If neither is specified,
then the edits must not change the snapshot name. Note that changing
a snapshot name must be done with care, since the contents of some
snapshots, such as internal snapshots within a single qcow2 file, are
accessible only from the original name.
=item B<snapshot-list> I<domain> [{I<--parent> | I<--roots> | I<--tree>}]
[I<--metadata>]