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



[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