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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
