[libvirt] [PATCH] storage: Remove redundant refreshPool check

Cole Robinson crobinso at redhat.com
Thu Jun 23 14:07:27 UTC 2016


On 06/23/2016 10:06 AM, Martin Kletzander wrote:
> On Thu, Jun 23, 2016 at 09:20:19AM -0400, John Ferlan wrote:
>>
>>
>> On 06/23/2016 09:03 AM, Cole Robinson wrote:
>>> On 06/23/2016 03:32 AM, Martin Kletzander wrote:
>>>> On Wed, Jun 22, 2016 at 08:29:35PM -0400, Cole Robinson wrote:
>>>>> Every driver provides a refreshPool impl, and many other critical
>>>>> places in the code unconditionally call it without checking if
>>>>> it exists, so this check is pointless
>>>>
>>>> I'm not entirely sure about it, but it'd be nicer if we actually checked
>>>> that it's non-NULL.  Just to future-proof the code in case someone adds
>>>> another backend.
>>>
>>> Please check the other storage_driver.c code... every 'startPool' invocation
>>> is followed by an uncondtional refreshPool call. If a driver is added without
>>> a refreshPool impl, it will crash libvirtd from any avenue that the pool can
>>> be started, so to support a driver like that will need much more work. This is
>>> the one place in the code that checks for backend->refreshPool
>>
>>
>> Hmm.. this check was caused by commit id '4a85bf3e2' where IIRC I was
>> probably being really paranoid.
>>
>> Digging a bit more finds commit id '318ea3cb77' which seems to indicate
>> refreshPool *must* be supplied.
>>
>> So ACK to the change,
>>
> 
> Fair enough, sorry for the noise, ACK.
> 

Thanks guys, I've pushed this

- Cole




More information about the libvir-list mailing list