All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxl: update check-xl-disk-parse
@ 2015-12-08 20:02 Wei Liu
  2015-12-09 10:06 ` Ian Campbell
  0 siblings, 1 reply; 5+ messages in thread
From: Wei Liu @ 2015-12-08 20:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Ian Jackson, Jim Fehlig, Wei Liu, Ian Campbell

The block-attach command now returns 1 when fails. Update first test
case to expect return value 1 instead of 255.

The parser now doesn't generate output for default values. Remove them
from expected output.

The "discard=" variant is never not supported, so delete two test cases
with that variant.

Reported-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/check-xl-disk-parse | 100 ++++------------------------------------
 1 file changed, 8 insertions(+), 92 deletions(-)

diff --git a/tools/libxl/check-xl-disk-parse b/tools/libxl/check-xl-disk-parse
index 1bec4ca..03572e4 100755
--- a/tools/libxl/check-xl-disk-parse
+++ b/tools/libxl/check-xl-disk-parse
@@ -40,7 +40,7 @@ complete () {
     fi
 }
 
-e=255
+e=1
 
 
 #---------- test data ----------
@@ -52,18 +52,10 @@ one $e foo
 
 expected <<END
 disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
     "pdev_path": "/dev/vg/guest-volume",
     "vdev": "hda",
-    "backend": "unknown",
     "format": "raw",
-    "script": null,
-    "removable": 0,
-    "readwrite": 1,
-    "is_cdrom": 0,
-    "direct_io_safe": false,
-    "discard_enable": "True"
+    "readwrite": 1
 }
 
 END
@@ -75,18 +67,11 @@ one 0 raw:/dev/vg/guest-volume,hda,w
 
 expected <<END
 disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
     "pdev_path": "/root/image.iso",
     "vdev": "hdc",
-    "backend": "unknown",
     "format": "raw",
-    "script": null,
     "removable": 1,
-    "readwrite": 0,
-    "is_cdrom": 1,
-    "direct_io_safe": false,
-    "discard_enable": "False"
+    "is_cdrom": 1
 }
 
 END
@@ -99,18 +84,11 @@ one 0 raw:/root/image.iso,hdc:cdrom,ro
 
 expected <<EOF
 disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
     "pdev_path": "/dev/vg/guest-volume",
     "vdev": "xvdb",
     "backend": "phy",
     "format": "raw",
-    "script": null,
-    "removable": 0,
-    "readwrite": 1,
-    "is_cdrom": 0,
-    "direct_io_safe": false,
-    "discard_enable": "True"
+    "readwrite": 1
 }
 
 EOF
@@ -118,18 +96,11 @@ one 0 backendtype=phy,vdev=xvdb,access=w,target=/dev/vg/guest-volume
 
 expected <<EOF
 disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
     "pdev_path": "",
     "vdev": "hdc",
-    "backend": "unknown",
     "format": "empty",
-    "script": null,
     "removable": 1,
-    "readwrite": 0,
-    "is_cdrom": 1,
-    "direct_io_safe": false,
-    "discard_enable": "False"
+    "is_cdrom": 1
 }
 
 EOF
@@ -141,18 +112,10 @@ one 0 ,empty,hdc:cdrom,r
 
 expected <<EOF
 disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
-    "pdev_path": null,
     "vdev": "hdc",
-    "backend": "unknown",
     "format": "empty",
-    "script": null,
     "removable": 1,
-    "readwrite": 0,
-    "is_cdrom": 1,
-    "direct_io_safe": false,
-    "discard_enable": "False"
+    "is_cdrom": 1
 }
 
 EOF
@@ -161,18 +124,11 @@ one 0 vdev=hdc,access=r,devtype=cdrom
 
 expected <<EOF
 disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
     "pdev_path": "iqn.2001-05.com.equallogic:0-8a0906-23fe93404-c82797962054a96d-examplehost",
     "vdev": "xvda",
-    "backend": "unknown",
     "format": "raw",
     "script": "block-iscsi",
-    "removable": 0,
-    "readwrite": 1,
-    "is_cdrom": 0,
-    "direct_io_safe": false,
-    "discard_enable": "True"
+    "readwrite": 1
 }
 
 EOF
@@ -183,18 +139,11 @@ one 0 vdev=xvda,access=w,script=block-iscsi,target=iqn.2001-05.com.equallogic:0-
 
 expected <<EOF
 disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
     "pdev_path": "app01",
     "vdev": "hda",
-    "backend": "unknown",
     "format": "raw",
     "script": "block-drbd",
-    "removable": 0,
-    "readwrite": 1,
-    "is_cdrom": 0,
-    "direct_io_safe": false,
-    "discard_enable": "True"
+    "readwrite": 1
 }
 
 EOF
@@ -205,57 +154,24 @@ one 0 drbd:app01,hda,w
 
 expected <<END
 disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
     "pdev_path": "/some/disk/image.raw",
     "vdev": "hda",
-    "backend": "unknown",
     "format": "raw",
-    "script": null,
-    "removable": 0,
     "readwrite": 1,
-    "is_cdrom": 0,
-    "direct_io_safe": false,
     "discard_enable": "True"
 }
 
 END
-one 0  discard=on  vdev=hda target=/some/disk/image.raw
-one 0  discard=1   vdev=hda target=/some/disk/image.raw
-
-expected <<END
-disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
-    "pdev_path": "/some/disk/image.raw",
-    "vdev": "hda",
-    "backend": "unknown",
-    "format": "raw",
-    "script": null,
-    "removable": 0,
-    "readwrite": 1,
-    "is_cdrom": 0,
-    "direct_io_safe": false,
-    "discard_enable": "False"
-}
-
-END
 one 0  discard vdev=hda target=/some/disk/image.raw
 one 0  discard vdev=hda target=/some/disk/image.raw
 
 expected <<END
 disk: {
-    "backend_domid": 0,
-    "backend_domname": null,
     "pdev_path": "/some/disk/image.iso",
     "vdev": "hda",
-    "backend": "unknown",
     "format": "raw",
-    "script": null,
     "removable": 1,
-    "readwrite": 0,
     "is_cdrom": 1,
-    "direct_io_safe": false,
     "discard_enable": "False"
 }
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] libxl: update check-xl-disk-parse
  2015-12-08 20:02 [PATCH] libxl: update check-xl-disk-parse Wei Liu
@ 2015-12-09 10:06 ` Ian Campbell
  2015-12-09 10:15   ` Wei Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2015-12-09 10:06 UTC (permalink / raw)
  To: Wei Liu, Xen-devel; +Cc: Jim Fehlig, Ian Jackson

On Tue, 2015-12-08 at 20:02 +0000, Wei Liu wrote:
> The block-attach command now returns 1 when fails. Update first test
> case to expect return value 1 instead of 255.
> 
> The parser now doesn't generate output for default values. Remove them
> from expected output.

This it looks good.

> The "discard=" variant is never not supported, so delete two test cases
> with that variant.

I don't follow (the double negative "never not" doesn't help).

However, assuming you are saying that discard= is always supported, I don't
then see the rationale for removing the test. Of course we do want to test
things which are supported.


> Reported-by: Jim Fehlig <jfehlig@suse.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Jim Fehlig <jfehlig@suse.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  tools/libxl/check-xl-disk-parse | 100 ++++----------------------------
> --------
>  1 file changed, 8 insertions(+), 92 deletions(-)
> 
> diff --git a/tools/libxl/check-xl-disk-parse b/tools/libxl/check-xl-disk-
> parse
> index 1bec4ca..03572e4 100755
> --- a/tools/libxl/check-xl-disk-parse
> +++ b/tools/libxl/check-xl-disk-parse
> @@ -40,7 +40,7 @@ complete () {
>      fi
>  }
>  
> -e=255
> +e=1
>  
>  
>  #---------- test data ----------
> @@ -52,18 +52,10 @@ one $e foo
>  
>  expected <<END
>  disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
>      "pdev_path": "/dev/vg/guest-volume",
>      "vdev": "hda",
> -    "backend": "unknown",
>      "format": "raw",
> -    "script": null,
> -    "removable": 0,
> -    "readwrite": 1,
> -    "is_cdrom": 0,
> -    "direct_io_safe": false,
> -    "discard_enable": "True"
> +    "readwrite": 1
>  }
>  
>  END
> @@ -75,18 +67,11 @@ one 0 raw:/dev/vg/guest-volume,hda,w
>  
>  expected <<END
>  disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
>      "pdev_path": "/root/image.iso",
>      "vdev": "hdc",
> -    "backend": "unknown",
>      "format": "raw",
> -    "script": null,
>      "removable": 1,
> -    "readwrite": 0,
> -    "is_cdrom": 1,
> -    "direct_io_safe": false,
> -    "discard_enable": "False"
> +    "is_cdrom": 1
>  }
>  
>  END
> @@ -99,18 +84,11 @@ one 0 raw:/root/image.iso,hdc:cdrom,ro
>  
>  expected <<EOF
>  disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
>      "pdev_path": "/dev/vg/guest-volume",
>      "vdev": "xvdb",
>      "backend": "phy",
>      "format": "raw",
> -    "script": null,
> -    "removable": 0,
> -    "readwrite": 1,
> -    "is_cdrom": 0,
> -    "direct_io_safe": false,
> -    "discard_enable": "True"
> +    "readwrite": 1
>  }
>  
>  EOF
> @@ -118,18 +96,11 @@ one 0
> backendtype=phy,vdev=xvdb,access=w,target=/dev/vg/guest-volume
>  
>  expected <<EOF
>  disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
>      "pdev_path": "",
>      "vdev": "hdc",
> -    "backend": "unknown",
>      "format": "empty",
> -    "script": null,
>      "removable": 1,
> -    "readwrite": 0,
> -    "is_cdrom": 1,
> -    "direct_io_safe": false,
> -    "discard_enable": "False"
> +    "is_cdrom": 1
>  }
>  
>  EOF
> @@ -141,18 +112,10 @@ one 0 ,empty,hdc:cdrom,r
>  
>  expected <<EOF
>  disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
> -    "pdev_path": null,
>      "vdev": "hdc",
> -    "backend": "unknown",
>      "format": "empty",
> -    "script": null,
>      "removable": 1,
> -    "readwrite": 0,
> -    "is_cdrom": 1,
> -    "direct_io_safe": false,
> -    "discard_enable": "False"
> +    "is_cdrom": 1
>  }
>  
>  EOF
> @@ -161,18 +124,11 @@ one 0 vdev=hdc,access=r,devtype=cdrom
>  
>  expected <<EOF
>  disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
>      "pdev_path": "iqn.2001-05.com.equallogic:0-8a0906-23fe93404-
> c82797962054a96d-examplehost",
>      "vdev": "xvda",
> -    "backend": "unknown",
>      "format": "raw",
>      "script": "block-iscsi",
> -    "removable": 0,
> -    "readwrite": 1,
> -    "is_cdrom": 0,
> -    "direct_io_safe": false,
> -    "discard_enable": "True"
> +    "readwrite": 1
>  }
>  
>  EOF
> @@ -183,18 +139,11 @@ one 0 vdev=xvda,access=w,script=block-
> iscsi,target=iqn.2001-05.com.equallogic:0-
>  
>  expected <<EOF
>  disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
>      "pdev_path": "app01",
>      "vdev": "hda",
> -    "backend": "unknown",
>      "format": "raw",
>      "script": "block-drbd",
> -    "removable": 0,
> -    "readwrite": 1,
> -    "is_cdrom": 0,
> -    "direct_io_safe": false,
> -    "discard_enable": "True"
> +    "readwrite": 1
>  }
>  
>  EOF
> @@ -205,57 +154,24 @@ one 0 drbd:app01,hda,w
>  
>  expected <<END
>  disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
>      "pdev_path": "/some/disk/image.raw",
>      "vdev": "hda",
> -    "backend": "unknown",
>      "format": "raw",
> -    "script": null,
> -    "removable": 0,
>      "readwrite": 1,
> -    "is_cdrom": 0,
> -    "direct_io_safe": false,
>      "discard_enable": "True"
>  }
>  
>  END
> -one 0  discard=on  vdev=hda target=/some/disk/image.raw
> -one 0  discard=1   vdev=hda target=/some/disk/image.raw
> -
> -expected <<END
> -disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
> -    "pdev_path": "/some/disk/image.raw",
> -    "vdev": "hda",
> -    "backend": "unknown",
> -    "format": "raw",
> -    "script": null,
> -    "removable": 0,
> -    "readwrite": 1,
> -    "is_cdrom": 0,
> -    "direct_io_safe": false,
> -    "discard_enable": "False"
> -}
> -
> -END
>  one 0  discard vdev=hda target=/some/disk/image.raw
>  one 0  discard vdev=hda target=/some/disk/image.raw
>  
>  expected <<END
>  disk: {
> -    "backend_domid": 0,
> -    "backend_domname": null,
>      "pdev_path": "/some/disk/image.iso",
>      "vdev": "hda",
> -    "backend": "unknown",
>      "format": "raw",
> -    "script": null,
>      "removable": 1,
> -    "readwrite": 0,
>      "is_cdrom": 1,
> -    "direct_io_safe": false,
>      "discard_enable": "False"
>  }
>  
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libxl: update check-xl-disk-parse
  2015-12-09 10:06 ` Ian Campbell
@ 2015-12-09 10:15   ` Wei Liu
  2015-12-09 10:24     ` Wei Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Wei Liu @ 2015-12-09 10:15 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Xen-devel, Jim Fehlig, Wei Liu, Ian Jackson

On Wed, Dec 09, 2015 at 10:06:17AM +0000, Ian Campbell wrote:
> On Tue, 2015-12-08 at 20:02 +0000, Wei Liu wrote:
> > The block-attach command now returns 1 when fails. Update first test
> > case to expect return value 1 instead of 255.
> > 
> > The parser now doesn't generate output for default values. Remove them
> > from expected output.
> 
> This it looks good.
> 
> > The "discard=" variant is never not supported, so delete two test cases
> > with that variant.
> 
> I don't follow (the double negative "never not" doesn't help).
> 

Duh, I didn't intend to use double negation.

> However, assuming you are saying that discard= is always supported, I don't
> then see the rationale for removing the test. Of course we do want to test
> things which are supported.
> 

No, it is not supported. I dug into mailing list archive last night and
confirmed that Ian J asked Olaf to remove such variant. Those test cases
were residual from a previous version Olaf failed to remove.

Wei.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libxl: update check-xl-disk-parse
  2015-12-09 10:15   ` Wei Liu
@ 2015-12-09 10:24     ` Wei Liu
  2015-12-09 10:32       ` Ian Campbell
  0 siblings, 1 reply; 5+ messages in thread
From: Wei Liu @ 2015-12-09 10:24 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Xen-devel, Jim Fehlig, Wei Liu, Ian Jackson

On Wed, Dec 09, 2015 at 10:15:35AM +0000, Wei Liu wrote:
> On Wed, Dec 09, 2015 at 10:06:17AM +0000, Ian Campbell wrote:
> > On Tue, 2015-12-08 at 20:02 +0000, Wei Liu wrote:
> > > The block-attach command now returns 1 when fails. Update first test
> > > case to expect return value 1 instead of 255.
> > > 
> > > The parser now doesn't generate output for default values. Remove them
> > > from expected output.
> > 
> > This it looks good.
> > 
> > > The "discard=" variant is never not supported, so delete two test cases
> > > with that variant.
> > 
> > I don't follow (the double negative "never not" doesn't help).
> > 
> 
> Duh, I didn't intend to use double negation.
> 
> > However, assuming you are saying that discard= is always supported, I don't
> > then see the rationale for removing the test. Of course we do want to test
> > things which are supported.
> > 
> 
> No, it is not supported. I dug into mailing list archive last night and
> confirmed that Ian J asked Olaf to remove such variant. Those test cases
> were residual from a previous version Olaf failed to remove.
> 

Forgot to paste in the references:

commit 417e6b70d73ffe8f8d3938aa30a413b35098e614 in xen.git

http://lists.xen.org/archives/html/xen-devel/2014-05/msg01063.html

> Wei.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libxl: update check-xl-disk-parse
  2015-12-09 10:24     ` Wei Liu
@ 2015-12-09 10:32       ` Ian Campbell
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2015-12-09 10:32 UTC (permalink / raw)
  To: Wei Liu; +Cc: Xen-devel, Jim Fehlig, Ian Jackson

On Wed, 2015-12-09 at 10:24 +0000, Wei Liu wrote:
> On Wed, Dec 09, 2015 at 10:15:35AM +0000, Wei Liu wrote:
> > On Wed, Dec 09, 2015 at 10:06:17AM +0000, Ian Campbell wrote:
> > > On Tue, 2015-12-08 at 20:02 +0000, Wei Liu wrote:
> > > > The block-attach command now returns 1 when fails. Update first
> > > > test
> > > > case to expect return value 1 instead of 255.
> > > > 
> > > > The parser now doesn't generate output for default values. Remove
> > > > them
> > > > from expected output.
> > > 
> > > This it looks good.
> > > 
> > > > The "discard=" variant is never not supported, so delete two test
> > > > cases
> > > > with that variant.
> > > 
> > > I don't follow (the double negative "never not" doesn't help).
> > > 
> > 
> > Duh, I didn't intend to use double negation.

You meant "...was never supported..." then I think?

> > > However, assuming you are saying that discard= is always supported, I
> > > don't
> > > then see the rationale for removing the test. Of course we do want to
> > > test
> > > things which are supported.
> > > 
> > 
> > No, it is not supported. I dug into mailing list archive last night and
> > confirmed that Ian J asked Olaf to remove such variant. Those test
> > cases
> > were residual from a previous version Olaf failed to remove.
> > 
> 
> Forgot to paste in the references:
> 
> commit 417e6b70d73ffe8f8d3938aa30a413b35098e614 in xen.git
> 
> http://lists.xen.org/archives/html/xen-devel/2014-05/msg01063.html

Thanks, please include the commit reference in the v2 commit for this
change.

Ian.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-12-09 10:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-08 20:02 [PATCH] libxl: update check-xl-disk-parse Wei Liu
2015-12-09 10:06 ` Ian Campbell
2015-12-09 10:15   ` Wei Liu
2015-12-09 10:24     ` Wei Liu
2015-12-09 10:32       ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.