[libvirt] [PATCH 7/9] conf: Refactor virDomainCachetuneDefParse

[bing.niu]

On 2018年07月28日 00:58, John Ferlan wrote:
> 
> [...]
> 
>>>
>>> Secondary to that you've added a new error related to identical vcpus in
>>> the parsing logic. Unfortunately that could make a domain disappear on a
>>> libvirtd restart if for some reason it was already defined in that
>>> manner. If there's a parsing error because two entries had identical
>>> cpus, then there will be an error. Errors would cause a previously
>>> OK/found domain to not be defined. So that type of check (now that the
>>
>> Sorry, I am not sure if I understand correctly. Are you talking about
>> two domains(or VMs) with the same vcpus setting?
> 
> It's the:
> 
> +    } else {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("Identical vcpus in cachetunes found"));
> +        goto cleanup;
> +    }
> 
> that was new... Consider what would happen before any of these changes
> were merged if that condition was true.
> 
> So I found tests/genericxml2xmlindata/cachetune-colliding-tunes.xml and
> did a little test... Before your changes, the test fails with "error:
> XML error: Overlapping vcpus in cachetunes", but with your changes the
> test fails with "error: XML error: Identical vcpus in cachetunes found".
> 

Yes! You are right. The error message is different.
> So since both fail, that's good, no issue then. It was different which
> is what drew my attention. In any case, just mention it in the commit
> message that part of the change will "clarify" whether it's an overlap
> or a redefinition.

Okay~, will update commit message in v2.
> 
> John
> 
> [...]
>