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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> > ---
> >  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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux