Re: [libvirt PATCH] qemu: snapshot: error out early when reverting snapshot for VM with non-file disk

On Wed, Feb 26, 2025 at 11:33:52 +0100, Pavel Hrdina wrote: > Before this patch the code would start the revert process by destroying > the VM and preparing to revert where it would fail with following error: > > error: unsupported configuration: source for disk 'sdb' is not a regular file; refusing to generate external snapshot name > > and leaving user with offline VM even if it was running. > > Make the check before we start the revert process to not destroy VMs. > > Resolves: https://issues.redhat.com/browse/RHEL-30971 > Resolves: https://issues.redhat.com/browse/RHEL-79928 > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > src/qemu/qemu_snapshot.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c > index f7d6272907..c5f70f5b10 100644 > --- a/src/qemu/qemu_snapshot.c > +++ b/src/qemu/qemu_snapshot.c > @@ -2205,6 +2205,8 @@ qemuSnapshotRevertValidate(virDomainObj *vm, > virDomainSnapshotDef *snapdef, > unsigned int flags) > { > + size_t i; > + > if (!vm->persistent && > snapdef->state != VIR_DOMAIN_SNAPSHOT_RUNNING && > snapdef->state != VIR_DOMAIN_SNAPSHOT_PAUSED && > @@ -2232,6 +2234,17 @@ qemuSnapshotRevertValidate(virDomainObj *vm, > } > } > > + for (i = 0; i < snap->def->dom->ndisks; i++) { > + virDomainDiskDef *disk = snap->def->dom->disks[i]; This code isn't doing the same logic as qemuSnapshotRevertExternalPrepare as it doesn't skip disks that weren't originally selected for snapshot in 'snapdef' ...

> + > + if (disk->src->type != VIR_STORAGE_TYPE_FILE) { ... so this'd in such case refuse the revert even if the disk state wouldn't in fact be reverted. > + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, > + _("source disk for '%1$s' is not a regural file, reverting to snapshot is not supported"), *regular > + disk->dst); > + return -1; > + } > + } > + > return 0; > } > > -- > 2.48.1 >