Re: [PATCH v2 06/12] qemu: drop fetch and update status functions

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

 



On Thu, Feb 16, 2017 at 16:15:02 +0100, Jiri Denemark wrote:
> On Wed, Dec 28, 2016 at 17:39:15 +0300, Nikolay Shirokovskiy wrote:
> > qemuMigrationFetchJobStatus is rather inconvinient. If we poll
> > stats for status only then we don't need to update elapsed time.
> > Next on some paths we use this function to get stats only we don't
> > what to unexpectedly update job status. So the common part
> > is only enter/exit which is too little to have a distinct function.
> > 
> > Let's open code getting stats and update elapsed time and status
> > where appropriate.
> > 
> > The patch also drops qemuMigrationUpdateJobStatus. It's value
> > is only in keeping job status on failures. Now we have option
> > not to update status when we want to.
...
> > @@ -2624,23 +2602,6 @@ qemuMigrationJobName(virDomainObjPtr vm)
> >  
> >  
> >  static int
> > -qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver,
> > -                             virDomainObjPtr vm,
> > -                             qemuDomainAsyncJob asyncJob)
> > -{
> > -    qemuDomainObjPrivatePtr priv = vm->privateData;
> > -    qemuDomainJobInfoPtr jobInfo = priv->job.current;
> > -    qemuDomainJobInfo newInfo = *jobInfo;
> > -
> > -    if (qemuMigrationFetchJobStatus(driver, vm, asyncJob, &newInfo) < 0)
> > -        return -1;
> > -
> > -    *jobInfo = newInfo;
> > -    return 0;
> > -}
> 
> The goal of this function is to avoid touching priv->job.current in case
> of failure. Since qemuMonitorGetMigrationStats memsets
> qemuDomainJobInfo, we would lose its original contents, which is
> definitely not something we want.

And I forgot to mention that the code should not touch
priv->job.current between EnterMonitorAsync/ExitMonitor.

Jirka

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[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