[libvirt PATCH v2 11/24] qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT when reverting snapshot

[Pavel Hrdina]

On Tue, Jul 25, 2023 at 03:54:54PM +0200, Peter Krempa wrote:
> On Tue, Jun 27, 2023 at 17:07:14 +0200, Pavel Hrdina wrote:
> 
> So what is this actually doing?
> 
> 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> >  src/qemu/qemu_snapshot.c | 19 ++++++++++---------
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> > index 5d2ffdeee6..1cb0ea55de 100644
> > --- a/src/qemu/qemu_snapshot.c
> > +++ b/src/qemu/qemu_snapshot.c
> > @@ -2001,7 +2001,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
> >          /* Transitions 5, 6, 8, 9 */
> >          qemuProcessStop(driver, vm,
> >                          VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
> > -                        VIR_ASYNC_JOB_START, 0);
> > +                        VIR_ASYNC_JOB_SNAPSHOT, 0);
> 
> It looks like we are declaring that an async job is used here ...
> 
> >          virDomainAuditStop(vm, "from-snapshot");
> >          detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
> >          event = virDomainEventLifecycleNewFromObj(vm,
> > @@ -2026,7 +2026,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
> >  
> >      rc = qemuProcessStart(snapshot->domain->conn, driver, vm,
> >                            cookie ? cookie->cpu : NULL,
> > -                          VIR_ASYNC_JOB_START, NULL, -1, NULL, snap,
> > +                          VIR_ASYNC_JOB_SNAPSHOT, NULL, -1, NULL, snap,
> >                            VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> >                            start_flags);
> >      virDomainAuditStart(vm, "from-snapshot", rc >= 0);
> > @@ -2059,7 +2059,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
> >          }
> >          rc = qemuProcessStartCPUs(driver, vm,
> >                                    VIR_DOMAIN_RUNNING_FROM_SNAPSHOT,
> > -                                  VIR_ASYNC_JOB_START);
> > +                                  VIR_ASYNC_JOB_SNAPSHOT);
> >          if (rc < 0)
> >              return -1;
> >      }
> > @@ -2122,7 +2122,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm,
> >      if (virDomainObjIsActive(vm)) {
> >          /* Transitions 4, 7 */
> >          qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
> > -                        VIR_ASYNC_JOB_START, 0);
> > +                        VIR_ASYNC_JOB_SNAPSHOT, 0);
> >          virDomainAuditStop(vm, "from-snapshot");
> >          detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
> >          event = virDomainEventLifecycleNewFromObj(vm,
> > @@ -2149,7 +2149,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm,
> >          start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
> >  
> >          rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
> > -                              VIR_ASYNC_JOB_START, NULL, -1, NULL, NULL,
> > +                              VIR_ASYNC_JOB_SNAPSHOT, NULL, -1, NULL, NULL,
> >                                VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> >                                start_flags);
> >          virDomainAuditStart(vm, "from-snapshot", rc >= 0);
> > @@ -2216,10 +2216,11 @@ qemuSnapshotRevert(virDomainObj *vm,
> >          return -1;
> >      }
> >  
> > -    if (qemuProcessBeginJob(vm,
> > -                            VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT,
> > -                            flags) < 0)
> 
> ... but it was never initiated as async.
> 
> It is unclear if you are fixing a bug, or doing a change for consistency
> or what is happening here actually.

qemuProcessBeginJob is just a wrapper for:

    if (virDomainObjBeginAsyncJob(vm, VIR_ASYNC_JOB_START,
                                   operation, apiFlags) < 0)
        return -1;

    qemuDomainObjSetAsyncJobMask(vm, VIR_JOB_NONE);
    return 0;

so we actually did start the job as async.

Yeah missing details in commit message bites me now as I don't remember
if this was only cosmetic change as CREATE and DELETE both use
VIR_ASYNC_JOB_SNAPSHOT instead of VIR_ASYNC_JOB_START or if there was
any other reason for this patch.

The only change this introduces is that now when revert job is running
query jobs are allowed as previously no other jobs were allowed.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230725/3dcab404/attachment.sig>