Re: [PATCH 3/4] qemu snapshot: use QMP snapshot-save/delete for internal snapshots

On 7/16/24 00:42, Nikolai Barybin wrote: > The usage of HMP commands are highly discouraged by qemu. Moreover, > current snapshot creation routine does not provide flexibility in > choosing target device for VM state snapshot. > > This patch makes use of QMP commands snapshot-save/delete and by > default chooses first writable disk (if present) as target for VM > state, NVRAM - otherwise. > > Signed-off-by: Nikolai Barybin <nikolai.barybin(a)virtuozzo.com&gt; > --- > src/qemu/qemu_snapshot.c | 158 ++++++++++++++++++++++++++++++++++++--- > 1 file changed, 148 insertions(+), 10 deletions(-) > > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c > index f5260c4a22..83949a9a27 100644 > --- a/src/qemu/qemu_snapshot.c > +++ b/src/qemu/qemu_snapshot.c > @@ -308,6 +308,96 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver, > return ret; > } > +static int > +qemuSnapshotActiveInternalGetWrdevListHelper(virDomainObj *vm, > + char **wrdevs) > +{ > + size_t wrdevCount = 0; > + size_t i = 0; > + > + for (i = 0; i < vm->def->ndisks; i++) { > + virDomainDiskDef *disk = vm->def->disks[i]; > + if (!disk->src->readonly) { > + wrdevs[wrdevCount] = g_strdup(disk->src->nodenameformat); > + wrdevCount++; > + } > + } > + > + if (wrdevCount == 0) { > + if (vm->def->os.loader->nvram) { > + wrdevs[0] = g_strdup(vm->def->os.loader->nvram->nodenameformat); > + } else { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("no writable device for internal snapshot creation/deletion")); > + return -1; > + } > + } > + should we select NVRAM at all? IMHO that would be very problematic. I think that we should generate error in this case.