On 06/20/2018 09:20 AM, Michal Privoznik wrote: > On 06/20/2018 12:54 AM, John Ferlan wrote: >> If the the snapshot revert involves a forced revert option, then >> let's not cause startup to change the genid flag in order to signify >> that we're still running the same/previous guest and not some >> snapshot reversion. >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1149445 >> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >> --- >> src/qemu/qemu_driver.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >> index 01011906d1..f737f4d350 100644 >> --- a/src/qemu/qemu_driver.c >> +++ b/src/qemu/qemu_driver.c >> @@ -16136,12 +16136,14 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, >> } >> >> /* If using VM GenID, there is no way currently to change >> - * the genid for the running guest, so set an error and >> - * mark as incompatible. */ >> + * the genid for the running guest, so set an error, >> + * mark as incompatible, and don't allow change of genid >> + * if the revert force flag would start the guest again. */ >> if (compatible && config->genidRequested) { >> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> _("domain genid update requires restart")); >> compatible = false; >> + start_flags &= ~VIR_QEMU_PROCESS_START_GEN_VMID; > > Pre-existing, but what is the point of this virReportError() if it does > not cause domain starting? > > Michal > See a few lines lower: if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { /* Re-spawn error using correct category. */ if (err->code == VIR_ERR_CONFIG_UNSUPPORTED) virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s", err->str2); goto endjob; } virResetError(err); It's thus "similar to" what virDomainDefCheckABIStabilityFlags would do (see a few lines above) when generating an error such as "Target domain disk count 1 does not match source 2" which is seen in bz1591628. A virsh failure would see : error: revert requires force: "%s" (message) which immediately for qe is followed by --force ;-) Tks for the review... John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
