Re: [libvirt] [PATCH 2/4] conf: Fix parsing python style triple quotes

2010-11-22 Thread Cole Robinson 
On 11/19/2010 04:56 PM, Eric Blake wrote:
 On 11/19/2010 02:51 PM, Cole Robinson wrote:
 On 11/19/2010 03:38 PM, Eric Blake wrote:
 On 11/19/2010 09:15 AM, Cole Robinson wrote:
 An incorrect check broke matching the closing set of quotes. Update
 tests to cover this case for XM config files, and update the domain schema
 to allow more path characters.

 -  param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]*/param
 +  param 
 name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param

 So far, so good...

  /data
/define
define name=devicePath
  data type=string
 -  param name=pattern/[a-zA-Z0-9_\+\-/%]+/param
 +  param 
 name=pattern/[a-zA-Z0-9_\+\-\\amp;quot;apos;lt;gt;/%]+/param

 but given that a devicePath can't have '.', should it really be allowed
 to have other characters like , , ', , or ?


 I didn't notice the lack of '.'  but should probably also be added. From
 the XML point of view, a devicePath could really just be any old FS path.
 
 If that's the case, then can we consolidate things rather than repeating
 the same pattern multiple times?  That is, can the schema use _just_
 filePath and absFilePath, rather than confusing things by adding devicePath?
 

Sounds good, I'll update the patch.

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/4] conf: Fix parsing python style triple quotes

2010-11-19 Thread Cole Robinson 
An incorrect check broke matching the closing set of quotes. Update
tests to cover this case for XM config files, and update the domain schema
to allow more path characters.

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 docs/schemas/domain.rng  |   10 +-
 src/util/conf.c  |3 ++-
 tests/xmconfigdata/test-escape-paths.cfg |2 +-
 tests/xmconfigdata/test-escape-paths.xml |5 +
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index bbbc846..870bea1 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -2028,27 +2028,27 @@
   /define
   define name=filePath
 data type=string
-  param name=pattern[a-zA-Z0-9_\.\+\-amp;/%]+/param
+  param 
name=pattern[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]+/param
 /data
   /define
   define name=absFilePath
 data type=string
-  param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]+/param
+  param 
name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]+/param
 /data
   /define
   define name=absDirPath
 data type=string
-  param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]*/param
+  param 
name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param
 /data
   /define
   define name=devicePath
 data type=string
-  param name=pattern/[a-zA-Z0-9_\+\-/%]+/param
+  param 
name=pattern/[a-zA-Z0-9_\+\-\\amp;quot;apos;lt;gt;/%]+/param
 /data
   /define
   define name=deviceName
 data type=string
-  param name=pattern[a-zA-Z0-9_\.\-:/]+/param
+  param 
name=pattern[a-zA-Z0-9_\.\-\\amp;quot;apos;lt;gt;:/]+/param
 /data
   /define
   define name=bridgeMode
diff --git a/src/util/conf.c b/src/util/conf.c
index a31bbc4..d9a7603 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -400,8 +400,9 @@ virConfParseString(virConfParserCtxtPtr ctxt)
 ctxt-cur += 3;
 base = ctxt-cur;
 
+/* Find the ending triple quotes */
 while ((ctxt-cur + 2  ctxt-end) 
-   (STRPREFIX(ctxt-cur, \\\))) {
+   !(STRPREFIX(ctxt-cur, \\\))) {
 if (CUR == '\n')
 ctxt-line++;
 NEXT;
diff --git a/tests/xmconfigdata/test-escape-paths.cfg 
b/tests/xmconfigdata/test-escape-paths.cfg
index f9f2cb8..e3e6db9 100644
--- a/tests/xmconfigdata/test-escape-paths.cfg
+++ b/tests/xmconfigdata/test-escape-paths.cfg
@@ -19,7 +19,7 @@ vnc = 1
 vncunused = 1
 vnclisten = 127.0.0.1
 vncpasswd = 123poi
-disk = [ phy:/dev/HostVG/XenGuest2,hda,w, 
file:/root/boot.isotest,hdc:cdrom,r ]
+disk = [ phy:/dev/HostVG/XenGuest2,hda,w, 
file:/root/boot.isotest,hdc:cdrom,r, phy:/dev/HostVG/XenGuest',hdb,w ]
 vif = [ 
mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu ]
 parallel = none
 serial = none
diff --git a/tests/xmconfigdata/test-escape-paths.xml 
b/tests/xmconfigdata/test-escape-paths.xml
index dabf492..13e6e29 100644
--- a/tests/xmconfigdata/test-escape-paths.xml
+++ b/tests/xmconfigdata/test-escape-paths.xml
@@ -31,6 +31,11 @@
   target dev='hdc' bus='ide'/
   readonly/
 /disk
+disk type='block' device='disk'
+  driver name='phy'/
+  source dev='/dev/HostVG/XenGuestapos;quot;'/
+  target dev='hdb' bus='ide'/
+/disk
 interface type='bridge'
   mac address='00:16:3e:66:92:9c'/
   source bridge='xenbr1'/
-- 
1.7.3.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/4] conf: Fix parsing python style triple quotes

2010-11-19 Thread Eric Blake 
On 11/19/2010 09:15 AM, Cole Robinson wrote:
 An incorrect check broke matching the closing set of quotes. Update
 tests to cover this case for XM config files, and update the domain schema
 to allow more path characters.
 
 -  param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]*/param
 +  param 
 name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param

So far, so good...

  /data
/define
define name=devicePath
  data type=string
 -  param name=pattern/[a-zA-Z0-9_\+\-/%]+/param
 +  param 
 name=pattern/[a-zA-Z0-9_\+\-\\amp;quot;apos;lt;gt;/%]+/param

but given that a devicePath can't have '.', should it really be allowed
to have other characters like , , ', , or ?

  /data
/define
define name=deviceName
  data type=string
 -  param name=pattern[a-zA-Z0-9_\.\-:/]+/param
 +  param 
 name=pattern[a-zA-Z0-9_\.\-\\amp;quot;apos;lt;gt;:/]+/param

Likewise for deviceName.

 +++ b/src/util/conf.c
 @@ -400,8 +400,9 @@ virConfParseString(virConfParserCtxtPtr ctxt)
  ctxt-cur += 3;
  base = ctxt-cur;
  
 +/* Find the ending triple quotes */
  while ((ctxt-cur + 2  ctxt-end) 
 -   (STRPREFIX(ctxt-cur, \\\))) {
 +   !(STRPREFIX(ctxt-cur, \\\))) {

Ah, the bug fix for patch 1.  ACK to patch 1, then.

 +++ b/tests/xmconfigdata/test-escape-paths.cfg
 @@ -19,7 +19,7 @@ vnc = 1
  vncunused = 1
  vnclisten = 127.0.0.1
  vncpasswd = 123poi
 -disk = [ phy:/dev/HostVG/XenGuest2,hda,w, 
 file:/root/boot.isotest,hdc:cdrom,r ]
 +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, 
 file:/root/boot.isotest,hdc:cdrom,r, 
 phy:/dev/HostVG/XenGuest',hdb,w ]
  vif = [ 
 mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu
  ]
  parallel = none
  serial = none
 diff --git a/tests/xmconfigdata/test-escape-paths.xml 
 b/tests/xmconfigdata/test-escape-paths.xml
 index dabf492..13e6e29 100644
 --- a/tests/xmconfigdata/test-escape-paths.xml
 +++ b/tests/xmconfigdata/test-escape-paths.xml
 @@ -31,6 +31,11 @@
target dev='hdc' bus='ide'/
readonly/
  /disk
 +disk type='block' device='disk'
 +  driver name='phy'/
 +  source dev='/dev/HostVG/XenGuestapos;quot;'/

Hmm; this really is a case of deviceName in the domain.rng schema.  Are
there really devices named with ' or  in the name?

If so, then ACK to patch 2.  If not, then it would be better to use
disk type='file'source file='/.../XenGuestapos;quot;'/, since that
would pick up on absFilePath, which is more likely to match reality of
having ' or  in the name.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/4] conf: Fix parsing python style triple quotes

2010-11-19 Thread Cole Robinson 
On 11/19/2010 03:38 PM, Eric Blake wrote:
 On 11/19/2010 09:15 AM, Cole Robinson wrote:
 An incorrect check broke matching the closing set of quotes. Update
 tests to cover this case for XM config files, and update the domain schema
 to allow more path characters.

 -  param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]*/param
 +  param 
 name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param
 
 So far, so good...
 
  /data
/define
define name=devicePath
  data type=string
 -  param name=pattern/[a-zA-Z0-9_\+\-/%]+/param
 +  param 
 name=pattern/[a-zA-Z0-9_\+\-\\amp;quot;apos;lt;gt;/%]+/param
 
 but given that a devicePath can't have '.', should it really be allowed
 to have other characters like , , ', , or ?
 

I didn't notice the lack of '.'  but should probably also be added. From
the XML point of view, a devicePath could really just be any old FS path.

Looking again, deviceName/Path are used in very different ways in the
schema, so this might need additional cleanup anyways. But anything that
represents a path should probably allow all valid path characters,
device or not.

  /data
/define
define name=deviceName
  data type=string
 -  param name=pattern[a-zA-Z0-9_\.\-:/]+/param
 +  param 
 name=pattern[a-zA-Z0-9_\.\-\\amp;quot;apos;lt;gt;:/]+/param
 
 Likewise for deviceName.
 
 +++ b/src/util/conf.c
 @@ -400,8 +400,9 @@ virConfParseString(virConfParserCtxtPtr ctxt)
  ctxt-cur += 3;
  base = ctxt-cur;
  
 +/* Find the ending triple quotes */
  while ((ctxt-cur + 2  ctxt-end) 
 -   (STRPREFIX(ctxt-cur, \\\))) {
 +   !(STRPREFIX(ctxt-cur, \\\))) {
 
 Ah, the bug fix for patch 1.  ACK to patch 1, then.
 
 +++ b/tests/xmconfigdata/test-escape-paths.cfg
 @@ -19,7 +19,7 @@ vnc = 1
  vncunused = 1
  vnclisten = 127.0.0.1
  vncpasswd = 123poi
 -disk = [ phy:/dev/HostVG/XenGuest2,hda,w, 
 file:/root/boot.isotest,hdc:cdrom,r ]
 +disk = [ phy:/dev/HostVG/XenGuest2,hda,w, 
 file:/root/boot.isotest,hdc:cdrom,r, 
 phy:/dev/HostVG/XenGuest',hdb,w ]
  vif = [ 
 mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu
  ]
  parallel = none
  serial = none
 diff --git a/tests/xmconfigdata/test-escape-paths.xml 
 b/tests/xmconfigdata/test-escape-paths.xml
 index dabf492..13e6e29 100644
 --- a/tests/xmconfigdata/test-escape-paths.xml
 +++ b/tests/xmconfigdata/test-escape-paths.xml
 @@ -31,6 +31,11 @@
target dev='hdc' bus='ide'/
readonly/
  /disk
 +disk type='block' device='disk'
 +  driver name='phy'/
 +  source dev='/dev/HostVG/XenGuestapos;quot;'/
 
 Hmm; this really is a case of deviceName in the domain.rng schema.  Are
 there really devices named with ' or  in the name?
 

Probably not, but someone could always create a symbolic link with
whatever valid pathname they want.

 If so, then ACK to patch 2.  If not, then it would be better to use
 disk type='file'source file='/.../XenGuestapos;quot;'/, since that
 would pick up on absFilePath, which is more likely to match reality of
 having ' or  in the name.
 

Having a whacky named file is probably more likely, but behind the
scenes it's all the same code here that is being tested (xm disk block
generation). The restrictions in RNG were fairly arbitrary anyways.

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/4] conf: Fix parsing python style triple quotes

2010-11-19 Thread Eric Blake 
On 11/19/2010 02:51 PM, Cole Robinson wrote:
 On 11/19/2010 03:38 PM, Eric Blake wrote:
 On 11/19/2010 09:15 AM, Cole Robinson wrote:
 An incorrect check broke matching the closing set of quotes. Update
 tests to cover this case for XM config files, and update the domain schema
 to allow more path characters.

 -  param name=pattern/[a-zA-Z0-9_\.\+\-amp;/%]*/param
 +  param 
 name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param

 So far, so good...

  /data
/define
define name=devicePath
  data type=string
 -  param name=pattern/[a-zA-Z0-9_\+\-/%]+/param
 +  param 
 name=pattern/[a-zA-Z0-9_\+\-\\amp;quot;apos;lt;gt;/%]+/param

 but given that a devicePath can't have '.', should it really be allowed
 to have other characters like , , ', , or ?

 
 I didn't notice the lack of '.'  but should probably also be added. From
 the XML point of view, a devicePath could really just be any old FS path.

If that's the case, then can we consolidate things rather than repeating
the same pattern multiple times?  That is, can the schema use _just_
filePath and absFilePath, rather than confusing things by adding devicePath?

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list