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

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

[Jim Fehlig]

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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Wei Liu]

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-libvirt-pair/debian.guest.osstest.cfg

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

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Wim ten Have]

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-libvirt-pair/debian.guest.osstest.cfg
> 
> 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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Jim Fehlig]

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-libvirt-pair/debian.guest.osstest.cfg
>>
>> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel