On Tue, 7 Nov 2017 12:20:05 +0000
Wei Liu <wei.liu2(a)citrix.com> wrote:
> On Mon, Nov 06, 2017 at 09:41:01PM -0700, Jim Fehlig wrote:
>> On 10/30/2017 06:17 AM, Wei Liu wrote:
>>> Hi Jim
>>>
>>> I discover a problem when using xen_xl converter. When the file in
>>> question doesn't end with a new line, I get the following error:
>>>
>>> error: configuration file syntax error: memory conf:53: expecting a
value
>>
>> I'm not able to reproduce this issue. The libvirt.git tree I tried was a bit
>> dated, but even after updating to latest master I can't reproduce.
>>
>>> After digging a bit (but haven't read libvirt code), it appears that the
>>> file didn't end with a new line.
>>
>> I tried several files without ending new lines, going both directions
>> (domxml-to-native and domxml-from-native), but didn't see the mentioned
>> error. Perhaps your config is revealing another bug which is being
>> improperly reported. Can you provide an example of the problematic config?
>>
>
> I tried to get the exact file that caused the problem but it is already
> destroyed by osstest.
>
> A similar file:
>
>
http://logs.test-lab.xenproject.org/osstest/logs/115436/test-amd64-amd64-...
>
> If you hexdump -C it, you can see the last character is 0a. Remove it and
> feed the file into the converter.
> Wei.
The phenonomem you point out is indeed weird. And my first response
is that this is a bug parsing the cfg input. I did little explore and
think that src/util/virconf.c (virConfParseLong(), virConfParseValue())
should be reworked as pointed out in below context diffs.
<wtenhave@nina:140> git diff
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 39c2bd917..bc8e57ec3 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -352,7 +352,7 @@ virConfParseLong(virConfParserCtxtPtr ctxt, long long *val)
} else if (CUR == '+') {
NEXT;
}
- if ((ctxt->cur >= ctxt->end) || (!c_isdigit(CUR))) {
+ if ((ctxt->cur > ctxt->end) || (!c_isdigit(CUR))) {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("unterminated number"));
return -1;
}
@@ -456,7 +456,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt)
long long l = 0;
SKIP_BLANKS;
- if (ctxt->cur >= ctxt->end) {
+ if (ctxt->cur > ctxt->end) {
virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("expecting a value"));
return NULL;
}
I did not go beyond this yet.
Thanks Wim. I noticed Cole fixed a similar issue when parsing content from a
file with commit 3cc2a9e0d4. But I think instead of replicating that fix in
virConfReadString(), we should just set the end of content correctly in
virConfParse(). I've sent a patch along those lines that fixes Wei's test case
and doesn't regress Cole's test case