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

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.