On Fri, Oct 04, 2024 at 10:43:59 +0200, Nikolai Barybin wrote:
> On 10/3/24 15:46, Peter Krempa wrote:
>
> > From: Nikolai Barybin via Devel <devel@xxxxxxxxxxxxxxxxx>
> >
> > 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 and by
> > default chooses first writable non-shared qcow2 disk (if present)
> > as target for VM state.
> >
> > Signed-off-by: Nikolai Barybin <nikolai.barybin@xxxxxxxxxxxxx>
> > Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> > ---
> > src/qemu/qemu_snapshot.c | 124 ++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 116 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> > index f5260c4a22..d4602d386f 100644
> > --- a/src/qemu/qemu_snapshot.c
> > +++ b/src/qemu/qemu_snapshot.c
> > @@ -309,6 +309,99 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
> > }
> >
> >
> > +static char **
> > +qemuSnapshotActiveInternalCreateGetDevices(virDomainObj *vm,
> > + virDomainSnapshotDef *snapdef)
> > +{
> > + g_auto(GStrv) devices = g_new0(char *, vm->def->ndisks + 2);
> > + size_t ndevs = 0;
> > + size_t i = 0;
> > +
> > + /* This relies on @snapdef being aligned and validated via
> > + * virDomainSnapshotAlignDisks() and qemuSnapshotPrepare(), which also
> > + * ensures that all disks are backed by qcow2. */
> > + for (i = 0; i < snapdef->ndisks; i++) {
> > + virDomainSnapshotDiskDef *snapdisk = snapdef->disks + i;
> > + virDomainDiskDef *domdisk = vm->def->disks[i];
> > +
> Could you clarify why to use 2 different types of addressing in 2
> consecutive lines of code? ( '+i' and [i])
Because :
In struct _virDomainSnapshotDef {
virDomainSnapshotDiskDef *disks;
and in struct _virDomainDef {
virDomainDiskDef **disks;
the snapshot definition is a array of 'virDomainSnapshotDiskDef'
structs, whereas domain definition has an array of pointers to the
'virDomainDiskDef' struct. The two lines differ in the extra dereference
(hidden inside the [] operator)
