External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-11 04:15:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

snapshot: Rework virDomainSnapshotState enum

Browse Source 
The existing virDomainSnapshotState is a superset of virDomainState,
adding one more state (disk-snapshot) on top of valid domain states.
But as written, the enum cannot be used for gcc validation that all
enum values are covered in a strongly-typed switch condition, because
the enum does not explicitly include the values it is adding to.

Copy the style used in qemu_blockjob.h of creating new enum names
for every inherited value, and update most clients to use the new
enum names anywhere snapshot state is referenced. The exception is
two switch statements in qemu code, which instead gain a fixme
comment about odd type usage (which will be cleaned up in the next
patch). The rest of the patch is fairly mechanical (I actually did
it by temporarily s/state/xstate/ in snapshot_conf.h to let the
compiler find which spots in the code used the field, did the
obvious search and replace in those functions, then undid the rename).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
This commit is contained in:
Eric Blake 2019-02-26 14:14:36 -06:00
parent 7ff982f0bc
commit f43eb6807e
5 changed files with 67 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/conf/snapshot_conf.c
View File

@ -58,7 +58,7 @@ VIR_ENUM_IMPL(virDomainSnapshotLocation, VIR_DOMAIN_SNAPSHOT_LOCATION_LAST,
);
/* virDomainSnapshotState is really virDomainState plus one extra state */
VIR_ENUM_IMPL(virDomainSnapshotState, VIR_DOMAIN_SNAPSHOT_STATE_LAST,
VIR_ENUM_IMPL(virDomainSnapshotState, VIR_DOMAIN_SNAPSHOT_LAST,
"nostate",
"running",
"blocked",
@ -257,8 +257,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
state);
goto cleanup;
}
offline = (def->state == VIR_DOMAIN_SHUTOFF ||
def->state == VIR_DOMAIN_DISK_SNAPSHOT);
offline = (def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF ||
def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT);
/* Older snapshots were created with just <domain>/<uuid>, and
* lack domain/@type. In that case, leave dom NULL, and
@ -879,14 +879,14 @@ static int virDomainSnapshotObjListCopyNames(void *payload,
if (data->flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) {
if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE) &&
obj->def->state == VIR_DOMAIN_SHUTOFF)
obj->def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF)
return 0;
if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) &&
obj->def->state == VIR_DOMAIN_DISK_SNAPSHOT)
obj->def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
return 0;
if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE) &&
obj->def->state != VIR_DOMAIN_SHUTOFF &&
obj->def->state != VIR_DOMAIN_DISK_SNAPSHOT)
obj->def->state != VIR_DOMAIN_SNAPSHOT_SHUTOFF &&
obj->def->state != VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)
return 0;
}
@ -1225,7 +1225,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
bool align_match = true;
virDomainSnapshotObjPtr other;
bool external = def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
bool external = def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT ||
virDomainSnapshotDefIsExternal(def);
/* Prevent circular chains */
@ -1282,10 +1282,10 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
other = virDomainSnapshotFindByName(vm->snapshots, def->name);
if (other) {
if ((other->def->state == VIR_DOMAIN_RUNNING ||
other->def->state == VIR_DOMAIN_PAUSED) !=
(def->state == VIR_DOMAIN_RUNNING ||
def->state == VIR_DOMAIN_PAUSED)) {
if ((other->def->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
other->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED) !=
(def->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
def->state == VIR_DOMAIN_SNAPSHOT_PAUSED)) {
virReportError(VIR_ERR_INVALID_ARG,
_("cannot change between online and offline "
"snapshot state in snapshot %s"),
@ -1293,8 +1293,8 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
goto cleanup;
}
if ((other->def->state == VIR_DOMAIN_DISK_SNAPSHOT) !=
(def->state == VIR_DOMAIN_DISK_SNAPSHOT)) {
if ((other->def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) !=
(def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT)) {
virReportError(VIR_ERR_INVALID_ARG,
_("cannot change between disk only and "
"full system in snapshot %s"),

23
src/conf/snapshot_conf.h
View File

@ -1,7 +1,7 @@
/*
* snapshot_conf.h: domain snapshot XML processing
*
* Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006-2019 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@ -36,11 +36,26 @@ typedef enum {
VIR_DOMAIN_SNAPSHOT_LOCATION_LAST
} virDomainSnapshotLocation;
/**
* This enum has to map all known domain states from the public enum
* virDomainState, before adding one additional state possible only
* for snapshots.
*/
typedef enum {
/* Inherit the VIR_DOMAIN_* states from virDomainState. */
VIR_DOMAIN_DISK_SNAPSHOT = VIR_DOMAIN_LAST,
VIR_DOMAIN_SNAPSHOT_STATE_LAST
/* Mapped to public enum */
VIR_DOMAIN_SNAPSHOT_NOSTATE = VIR_DOMAIN_NOSTATE,
VIR_DOMAIN_SNAPSHOT_RUNNING = VIR_DOMAIN_RUNNING,
VIR_DOMAIN_SNAPSHOT_BLOCKED = VIR_DOMAIN_BLOCKED,
VIR_DOMAIN_SNAPSHOT_PAUSED = VIR_DOMAIN_PAUSED,
VIR_DOMAIN_SNAPSHOT_SHUTDOWN = VIR_DOMAIN_SHUTDOWN,
VIR_DOMAIN_SNAPSHOT_SHUTOFF = VIR_DOMAIN_SHUTOFF,
VIR_DOMAIN_SNAPSHOT_CRASHED = VIR_DOMAIN_CRASHED,
VIR_DOMAIN_SNAPSHOT_PMSUSPENDED = VIR_DOMAIN_PMSUSPENDED,
/* Additional enum values local to snapshots */
VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT,
VIR_DOMAIN_SNAPSHOT_LAST
} virDomainSnapshotState;
verify((int)VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT == VIR_DOMAIN_LAST);
/* Stores disk-snapshot information */
typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef;

34
src/qemu/qemu_driver.c
View File

@ -15010,7 +15010,7 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL:
found_internal = true;
if (def->state == VIR_DOMAIN_DISK_SNAPSHOT && active) {
if (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT && active) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("active qemu domains require external disk "
"snapshots; disk %s requested internal"),
@ -15087,7 +15087,7 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
}
/* disk snapshot requires at least one disk */
if (def->state == VIR_DOMAIN_DISK_SNAPSHOT && !external) {
if (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT && !external) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("disk-only snapshots require at least "
"one disk to be selected for snapshot"));
@ -15769,8 +15769,13 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
/* allow snapshots only in certain states */
state = vm->state.state;
if (redefine)
state = def->state == VIR_DOMAIN_DISK_SNAPSHOT ? VIR_DOMAIN_SHUTOFF :
state = def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT ? VIR_DOMAIN_SHUTOFF :
def->state;
/* FIXME: state should be virDomainSnapshotState, with the switch
* statement handling of VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT (the only
* enum value added beyond what virDomainState supports). But for
* now it doesn't matter, because we slammed the extra snapshot
* state into a safe domain state. */
switch (state) {
/* valid states */
case VIR_DOMAIN_RUNNING:
@ -15826,9 +15831,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
align_match = false;
if (virDomainObjIsActive(vm))
def->state = VIR_DOMAIN_DISK_SNAPSHOT;
def->state = VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT;
else
def->state = VIR_DOMAIN_SHUTOFF;
def->state = VIR_DOMAIN_SNAPSHOT_SHUTOFF;
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
} else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
def->state = virDomainObjGetState(vm, NULL);
@ -15845,7 +15850,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
goto endjob;
}
def->memory = (def->state == VIR_DOMAIN_SHUTOFF ?
def->memory = (def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF ?
VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);
}
@ -16408,8 +16413,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto endjob;
if (!vm->persistent &&
snap->def->state != VIR_DOMAIN_RUNNING &&
snap->def->state != VIR_DOMAIN_PAUSED &&
snap->def->state != VIR_DOMAIN_SNAPSHOT_RUNNING &&
snap->def->state != VIR_DOMAIN_SNAPSHOT_PAUSED &&
(flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@ -16432,8 +16437,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto endjob;
}
if (virDomainObjIsActive(vm) &&
!(snap->def->state == VIR_DOMAIN_RUNNING
|| snap->def->state == VIR_DOMAIN_PAUSED) &&
!(snap->def->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED) &&
(flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
@ -16469,6 +16474,11 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
cookie = (qemuDomainSaveCookiePtr) snap->def->cookie;
/* FIXME: This cast should be to virDomainSnapshotState, with
* better handling of VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT (the only enum
* value added beyond what virDomainState supports). But for now
* it doesn't matter, because of the above rejection of revert to
* external snapshots. */
switch ((virDomainState) snap->def->state) {
case VIR_DOMAIN_RUNNING:
case VIR_DOMAIN_PAUSED:
@ -16610,7 +16620,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
/* Touch up domain state. */
if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
(snap->def->state == VIR_DOMAIN_PAUSED ||
(snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED ||
(flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
/* Transitions 3, 6, 9 */
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
@ -16717,7 +16727,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid target domain state '%s'. Refusing "
"snapshot reversion"),
virDomainStateTypeToString(snap->def->state));
virDomainSnapshotStateTypeToString(snap->def->state));
goto endjob;
}

20
src/test/test_driver.c
View File

@ -6271,9 +6271,9 @@ testDomainSnapshotAlignDisks(virDomainObjPtr vm,
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
align_match = false;
if (virDomainObjIsActive(vm))
def->state = VIR_DOMAIN_DISK_SNAPSHOT;
def->state = VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT;
else
def->state = VIR_DOMAIN_SHUTOFF;
def->state = VIR_DOMAIN_SNAPSHOT_SHUTOFF;
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
} else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
def->state = virDomainObjGetState(vm, NULL);
@ -6281,7 +6281,7 @@ testDomainSnapshotAlignDisks(virDomainObjPtr vm,
align_match = false;
} else {
def->state = virDomainObjGetState(vm, NULL);
def->memory = def->state == VIR_DOMAIN_SHUTOFF ?
def->memory = def->state == VIR_DOMAIN_SNAPSHOT_SHUTOFF ?
VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
}
@ -6572,8 +6572,8 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto cleanup;
if (!vm->persistent &&
snap->def->state != VIR_DOMAIN_RUNNING &&
snap->def->state != VIR_DOMAIN_PAUSED &&
snap->def->state != VIR_DOMAIN_SNAPSHOT_RUNNING &&
snap->def->state != VIR_DOMAIN_SNAPSHOT_PAUSED &&
(flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) == 0) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@ -6590,8 +6590,8 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto cleanup;
}
if (virDomainObjIsActive(vm) &&
!(snap->def->state == VIR_DOMAIN_RUNNING
|| snap->def->state == VIR_DOMAIN_PAUSED) &&
!(snap->def->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED) &&
(flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
@ -6612,8 +6612,8 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if (!config)
goto cleanup;
if (snap->def->state == VIR_DOMAIN_RUNNING ||
snap->def->state == VIR_DOMAIN_PAUSED) {
if (snap->def->state == VIR_DOMAIN_SNAPSHOT_RUNNING ||
snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED) {
/* Transitions 2, 3, 5, 6, 8, 9 */
bool was_running = false;
bool was_stopped = false;
@ -6672,7 +6672,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
/* Touch up domain state. */
if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
(snap->def->state == VIR_DOMAIN_PAUSED ||
(snap->def->state == VIR_DOMAIN_SNAPSHOT_PAUSED ||
(flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {
/* Transitions 3, 6, 9 */
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,

4
src/vbox/vbox_common.c
View File

@ -6316,9 +6316,9 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
goto cleanup;
}
if (online)
def->state = VIR_DOMAIN_RUNNING;
def->state = VIR_DOMAIN_SNAPSHOT_RUNNING;
else
def->state = VIR_DOMAIN_SHUTOFF;
def->state = VIR_DOMAIN_SNAPSHOT_SHUTOFF;
virUUIDFormat(dom->uuid, uuidstr);
memcpy(def->dom->uuid, dom->uuid, VIR_UUID_BUFLEN);