[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Libvirt config converter can't handle file not ending with new line

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:


If you hexdump -C it, you can see the last character is 0a. Remove it and
feed the file into the converter.

   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 == '+') {
	-    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;
	-    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



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]