On 07/13/2017 10:41 AM, Michal Privoznik wrote:
> On 06/02/2017 12:25 PM, John Ferlan wrote:
>> After returning from virNWFilterObjListAssignDef the @obj is locked;
>> however, if virNWFilterSaveConfig fails to save the generated UUID
>> the code jumped to error and returns NULL meaning the caller will
>> not call virNWFilterObjUnlock on the object leaving the object
>> locked on list and ripe for a deadlock on any subsequent Find
>> of all objects or object removal.
>>
>> So rather than jumping to error alter the comment prior to the
>> virNWFilterSaveConfig and just provide a VIR_INFO message for anyone
>> that cares, but allow the code to continue and a subsequent LoadConfig
>> to once again attempt the save of a newly generated UUID.
>>
>> Signed-off-by: John Ferlan <jfer...@redhat.com>
>> ---
>> src/conf/virnwfilterobj.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
>> index 3fb6046..0343c0a 100644
>> --- a/src/conf/virnwfilterobj.c
>> +++ b/src/conf/virnwfilterobj.c
>> @@ -507,10 +507,13 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr
>> nwfilters,
>> def = NULL;
>> objdef = obj->def;
>>
>> - /* We generated a UUID, make it permanent by saving the config to disk
>> */
>> + /* We generated a UUID, atttempt to make it permanent by saving the
>
> s/ttt/tt/
>
>> + * config to disk. If not successful, no need to fail or remove the
>> + * object as a future load would regenerate a UUID and try again,
>> + * but the existing config would still exist and can be used. */
>> if (!objdef->uuid_specified &&
>> virNWFilterSaveConfig(configDir, objdef) < 0)
>> - goto error;
>> + VIR_INFO("failed to save generated UUID for filter '%s'",
>> objdef->name);
>>
>> VIR_FREE(configFile);
>> return obj;
>>
>
> Well, frankly I'd say that we should not even try to save the config in
> the first place. Load() should really just load. We shouldn't try to
> "fix" XML configs at load time rather then when saving it in define phase.
>
IIRC: this one's a bit weird since if the UUID doesn't exist we "can"
generate one. If we generate one, then we should save it. However,
failing to save shouldn't be called an error because having a UUID isn't
required it's just something we try to "enforce" at some point in time
of the nwfilter. I kind of didn't want to "adjust" that logic. That's a
different patch ;->
John
> Michal
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
