On Wed, Oct 17, 2018 at 4:46 PM Martin Kletzander <mkletzan@xxxxxxxxxx> wrote:
On Tue, Oct 16, 2018 at 07:19:41PM -0400, John Ferlan wrote:
>
>
>On 10/14/18 10:26 AM, Han Han wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1535930
>>
>> Report more clear err msg instead of unknown error when coalesce
>> settings is incomplete.
>>
Incomplete is not an error. It's request for removal of the missing setting.
There is no problem if it is incomplete.
Well, I think incomplete xml is the problem because I have reproduce it on an inactive
I am not sure it will be something wrong when update the device lively, but we can fix
this first.
If you look at the BZ the problem is not the incomplete XML, but the fact that
the code that should update the device fails somewhere without setting an error.
Actually, there should not be an error, it should work. So this just works
around the actual issue.
Having said that maybe the problem is somewhere in the parsing part, but this is
not the solution. We need to "go deeper" to find out why the updating code
fails and then figure out what to fix from there, not put a sheet over the
problem.
>> Signed-off-by: Han Han <hhan@xxxxxxxxxx>
>> ---
>> src/conf/domain_conf.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 9911d56130..e755f45d3d 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -7804,8 +7804,12 @@ virDomainNetDefCoalesceParseXML(xmlNodePtr node,
>> ctxt->node = node;
>>
>> str = virXPathString("string(./rx/frames/@max)", ctxt);
>> - if (!str)
>> + if (!str) {
>> + virReportError(VIR_ERR_XML_DETAIL,
>> + "%s",
>
>This can be put on the previous line
>
>> + _("incomplete coalesce settings in interface xml"));
>
>and specifically this could be is missing rx frames max attributes
>
>However, according to the RNG from commit 523c9960, it seems the 'rx' is
>optional as is the '@max' value. Maybe Martin should provide a comment
>on this series since he added it.
>
>Of course that would cause the whole <coalesce> to disappear on Format.
>It would also cause problems because def->coalesce would have something
>that's empty.
>
>So perhaps the best thing to do is pass the @def into here, then only if
>we get beyond the initial !str comparison do we allocate and fill it in;
>otherwise, we return 0 if rx/frames/@max is not there. Prepares us for
>the future.
>
>I guess I'm not 100% clear if max frames == 0 what would happen. Maybe
>Martin knows (I've CC'd him).
>
`rx-frames=0` turns that option off. It's like not having the parameter there
at all (it also works like this with ethtool).
Set `rx-frames=0` is a better solution compared with report a error to incomplete
the incomplete coalesce setting.
>John
>
>> goto cleanup;
>> + }
>>
>> if (VIR_ALLOC(ret) < 0)
>> goto cleanup;
>>
--
Best regards,
-----------------------------------
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
