On 17.02.2017 11:23, Jiri Denemark wrote:
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.
Ooh, i missed that. Using domain object without domain lock... Nikolay