[libvirt] Libvirt config converter can't handle file not ending with new line
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 After digging a bit (but haven't read libvirt code), it appears that the file didn't end with a new line. I have worked around this, but I think this is an issue that should be fixed. Thanks, Wei.
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? Regards, Jim
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-lib... If you hexdump -C it, you can see the last character is 0a. Remove it and feed the file into the converter. Wei.
On Tue, 7 Nov 2017 12:20:05 +0000 Wei Liu <wei.liu2@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-lib...
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. Rgds, - Wim.
On 11/07/2017 08:54 AM, Wim ten Have wrote:
On Tue, 7 Nov 2017 12:20:05 +0000 Wei Liu <wei.liu2@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-lib...
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 https://www.redhat.com/archives/libvir-list/2017-November/msg00286.html Regards, Jim
participants (3)
-
Jim Fehlig -
Wei Liu -
Wim ten Have