from:"Martin Kletzander"

[libvirt] [PATCH v3] Fixed URI parsing

2012-02-24 Thread Martin Kletzander

Function xmlParseURI does not remove square brackets around IPv6
address when parsing. One of the solutions is making wrappers around
functions working with xmlURI*. This assures that uri-server will be
always properly assigned and it doesn't have to be changed when used
on some new place in the code.
For this purpose, functions virParseURI and virSaveURI were
added. These function are wrappers around xmlParseURI and xmlSaveUri
respectively.
Also there is one new syntax check function to prohibit these functions
anywhere else.

File changes:
 - src/util/viruri.h-- declaration
 - src/util/viruri.c-- definition
 - src/libvirt_private.syms -- symbol export
 - src/Makefile.am  -- added source and header files
 - cfg.mk   -- added sc_prohibit_xmlURI
 - all others   -- ID name and include fixes
---
v3:
 - wrappers moved to new files
 - syntax check added
 - virAsprintf used instead of nasty memory mechanics

v2:
 - added virSaveURI for building back the original string

 cfg.mk |8 
 src/Makefile.am|3 +-
 src/datatypes.h|2 +-
 src/driver.h   |2 +-
 src/esx/esx_driver.c   |5 +-
 src/esx/esx_util.c |2 +-
 src/esx/esx_util.h |4 +-
 src/hyperv/hyperv_util.c   |2 +-
 src/hyperv/hyperv_util.h   |5 +-
 src/libvirt.c  |   10 ++--
 src/libvirt_private.syms   |5 ++
 src/libxl/libxl_driver.c   |3 +-
 src/lxc/lxc_driver.c   |3 +-
 src/openvz/openvz_driver.c |3 +-
 src/qemu/qemu_driver.c |2 +-
 src/qemu/qemu_migration.c  |7 ++-
 src/remote/remote_driver.c |9 ++--
 src/uml/uml_driver.c   |3 +-
 src/util/qparams.c |3 +-
 src/util/viruri.c  |   91 
 src/util/viruri.h  |   18 +
 src/vbox/vbox_tmpl.c   |3 +-
 src/vmx/vmx.c  |6 +-
 src/xen/xen_driver.c   |4 +-
 src/xen/xen_hypervisor.h   |3 +-
 src/xen/xend_internal.c|4 +-
 src/xen/xend_internal.h|2 +-
 src/xenapi/xenapi_driver.c |2 +-
 src/xenapi/xenapi_utils.c  |4 +-
 src/xenapi/xenapi_utils.h  |4 +-
 30 files changed, 174 insertions(+), 48 deletions(-)
 create mode 100644 src/util/viruri.c
 create mode 100644 src/util/viruri.h

diff --git a/cfg.mk b/cfg.mk
index dcf44bb..9759d87 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -457,6 +457,12 @@ sc_prohibit_xmlGetProp:
halt='use virXMLPropString, not xmlGetProp' \
  $(_sc_search_regexp)

+# xml(ParseURI|SaveUri) doesn't handle IPv6 URIs well
+sc_prohibit_xmlURI:
+   @prohibit='\xml(ParseURI|SaveUri) *\(' \
+   halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)'  \
+ $(_sc_search_regexp)
+
 # ATTRIBUTE_UNUSED should only be applied in implementations, not
 # header declarations
 sc_avoid_attribute_unused_in_header:
@@ -758,6 +764,8 @@ exclude_file_name_regexp--sc_prohibit_strncpy = \

 exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/xml\.c$$

+exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$
+
 exclude_file_name_regexp--sc_require_config_h = ^examples/

 exclude_file_name_regexp--sc_require_config_h_first = ^examples/
diff --git a/src/Makefile.am b/src/Makefile.am
index d5f52a0..e2542b1 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -104,7 +104,8 @@ UTIL_SOURCES =  
\
util/virnetlink.c util/virnetlink.h \
util/virrandom.h util/virrandom.c   \
util/virsocketaddr.h util/virsocketaddr.c \
-   util/virtime.h util/virtime.c
+   util/virtime.h util/virtime.c \
+   util/viruri.h util/viruri.c

 EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
$(srcdir)/util/virkeycode-mapgen.py
diff --git a/src/datatypes.h b/src/datatypes.h
index 47058ed..fc284d2 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -151,7 +151,7 @@ struct _virConnect {
  */
 unsigned int magic; /* specific value to check */
 unsigned int flags; /* a set of connection flags */
-xmlURIPtr uri;  /* connection URI */
+virURIPtr uri;  /* connection URI */

 /* The underlying hypervisor driver and network driver. */
 virDriverPtr  driver;
diff --git a/src/driver.h b/src/driver.h
index d27fa99..b04b254 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -9,9 +9,9 @@
 # include config.h

 # include unistd.h
-# include libxml/uri.h

 # include internal.h
+# include viruri.h
 /*
  * List of registered drivers numbers
  */
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f5e1cc7..b6b22f8 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -44,6 +44,7 @@
 #include esx_vi.h
 #include esx_vi_methods.h
 #include esx_util.h
+#include

[libvirt] [PATCH] Fixed URI parsing

2012-02-24 Thread Martin Kletzander

Function xmlParseURI does not remove square brackets around IPv6
address when parsing. One of the solutions is making wrappers around
functions working with xmlURI*. This assures that uri-server will be
always properly assigned and it doesn't have to be changed when used
on some new place in the code.
For this purpose, functions virParseURI and virSaveURI were
added. These function are wrappers around xmlParseURI and xmlSaveUri
respectively.
Also there is one new syntax check function to prohibit these functions
anywhere else.

File changes:
 - src/util/viruri.h-- declaration
 - src/util/viruri.c-- definition
 - src/libvirt_private.syms -- symbol export
 - src/Makefile.am  -- added source and header files
 - cfg.mk   -- added sc_prohibit_xmlURI
 - all others   -- ID name and include fixes
---
v4:
 - fixed NULL pointer dereference
 - fixed minor typo
 - copyright added to header

v3:
 - wrappers moved to new files
 - syntax check added
 - virAsprintf used instead of nasty memory mechanics

v2:
 - added virSaveURI for building back the original string

 cfg.mk |8 
 src/Makefile.am|3 +-
 src/datatypes.h|2 +-
 src/driver.h   |2 +-
 src/esx/esx_driver.c   |5 +-
 src/esx/esx_util.c |2 +-
 src/esx/esx_util.h |4 +-
 src/hyperv/hyperv_util.c   |2 +-
 src/hyperv/hyperv_util.h   |5 +-
 src/libvirt.c  |   10 ++--
 src/libvirt_private.syms   |5 ++
 src/libxl/libxl_driver.c   |3 +-
 src/lxc/lxc_driver.c   |3 +-
 src/openvz/openvz_driver.c |3 +-
 src/qemu/qemu_driver.c |2 +-
 src/qemu/qemu_migration.c  |7 ++-
 src/remote/remote_driver.c |9 ++--
 src/uml/uml_driver.c   |3 +-
 src/util/qparams.c |3 +-
 src/util/viruri.c  |   93 
 src/util/viruri.h  |   22 ++
 src/vbox/vbox_tmpl.c   |3 +-
 src/vmx/vmx.c  |6 +-
 src/xen/xen_driver.c   |4 +-
 src/xen/xen_hypervisor.h   |3 +-
 src/xen/xend_internal.c|4 +-
 src/xen/xend_internal.h|2 +-
 src/xenapi/xenapi_driver.c |2 +-
 src/xenapi/xenapi_utils.c  |4 +-
 src/xenapi/xenapi_utils.h  |4 +-
 30 files changed, 180 insertions(+), 48 deletions(-)
 create mode 100644 src/util/viruri.c
 create mode 100644 src/util/viruri.h

diff --git a/cfg.mk b/cfg.mk
index dcf44bb..9759d87 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -457,6 +457,12 @@ sc_prohibit_xmlGetProp:
halt='use virXMLPropString, not xmlGetProp' \
  $(_sc_search_regexp)

+# xml(ParseURI|SaveUri) doesn't handle IPv6 URIs well
+sc_prohibit_xmlURI:
+   @prohibit='\xml(ParseURI|SaveUri) *\(' \
+   halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)'  \
+ $(_sc_search_regexp)
+
 # ATTRIBUTE_UNUSED should only be applied in implementations, not
 # header declarations
 sc_avoid_attribute_unused_in_header:
@@ -758,6 +764,8 @@ exclude_file_name_regexp--sc_prohibit_strncpy = \

 exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/xml\.c$$

+exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$
+
 exclude_file_name_regexp--sc_require_config_h = ^examples/

 exclude_file_name_regexp--sc_require_config_h_first = ^examples/
diff --git a/src/Makefile.am b/src/Makefile.am
index d5f52a0..e2542b1 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -104,7 +104,8 @@ UTIL_SOURCES =  
\
util/virnetlink.c util/virnetlink.h \
util/virrandom.h util/virrandom.c   \
util/virsocketaddr.h util/virsocketaddr.c \
-   util/virtime.h util/virtime.c
+   util/virtime.h util/virtime.c \
+   util/viruri.h util/viruri.c

 EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
$(srcdir)/util/virkeycode-mapgen.py
diff --git a/src/datatypes.h b/src/datatypes.h
index 47058ed..fc284d2 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -151,7 +151,7 @@ struct _virConnect {
  */
 unsigned int magic; /* specific value to check */
 unsigned int flags; /* a set of connection flags */
-xmlURIPtr uri;  /* connection URI */
+virURIPtr uri;  /* connection URI */

 /* The underlying hypervisor driver and network driver. */
 virDriverPtr  driver;
diff --git a/src/driver.h b/src/driver.h
index d27fa99..b04b254 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -9,9 +9,9 @@
 # include config.h

 # include unistd.h
-# include libxml/uri.h

 # include internal.h
+# include viruri.h
 /*
  * List of registered drivers numbers
  */
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f5e1cc7..b6b22f8 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -44,6

[libvirt] [PATCH] Removed more AMD-specific features from cpu64-rhel* models

2012-03-08 Thread Martin Kletzander

We found few more AMD-specific features in cpu64-rhel* models that
made it impossible to start qemu guest on Intel host (with this
setting) even though qemu itself starts correctly with them.
---
 src/cpu/cpu_map.xml |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 7ef230e..6a6603b 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -347,7 +347,6 @@
 /model

 model name='cpu64-rhel6'
-  feature name='abm'/
   feature name='apic'/
   feature name='clflush'/
   feature name='cmov'/
@@ -373,7 +372,6 @@
   feature name='sse'/
   feature name='sse2'/
   feature name='pni'/
-  feature name='sse4a'/
   feature name='syscall'/
   feature name='tsc'/
 /model
-- 
1.7.3.4

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

[libvirt] [PATCH v2] Removed more AMD-specific features from cpu64-rhel* models

2012-03-09 Thread Martin Kletzander

We found few more AMD-specific features in cpu64-rhel* models that
made it impossible to start qemu guest on Intel host (with this
setting) even though qemu itself starts correctly with them.
This impacts one test, thus the fix in tests/cputestdata/.
---
 src/cpu/cpu_map.xml|2 --
 .../cputestdata/x86-baseline-no-vendor-result.xml  |3 +--
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 7ef230e..6a6603b 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -347,7 +347,6 @@
 /model

 model name='cpu64-rhel6'
-  feature name='abm'/
   feature name='apic'/
   feature name='clflush'/
   feature name='cmov'/
@@ -373,7 +372,6 @@
   feature name='sse'/
   feature name='sse2'/
   feature name='pni'/
-  feature name='sse4a'/
   feature name='syscall'/
   feature name='tsc'/
 /model
diff --git a/tests/cputestdata/x86-baseline-no-vendor-result.xml 
b/tests/cputestdata/x86-baseline-no-vendor-result.xml
index 4b4921c..00e03b2 100644
--- a/tests/cputestdata/x86-baseline-no-vendor-result.xml
+++ b/tests/cputestdata/x86-baseline-no-vendor-result.xml
@@ -1,4 +1,3 @@
 cpu mode='custom' match='exact'
-  model fallback='allow'kvm64/model
-  feature policy='require' name='lahf_lm'/
+  model fallback='allow'cpu64-rhel6/model
 /cpu
--
1.7.3.4

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

Re: [libvirt] [PATCH] Removed more AMD-specific features from cpu64-rhel* models

2012-03-09 Thread Martin Kletzander

On 03/08/2012 04:11 PM, Eric Blake wrote:
 On 03/08/2012 08:02 AM, Martin Kletzander wrote:
 We found few more AMD-specific features in cpu64-rhel* models that
 made it impossible to start qemu guest on Intel host (with this
 setting) even though qemu itself starts correctly with them.
 ---
  src/cpu/cpu_map.xml |2 --
  1 files changed, 0 insertions(+), 2 deletions(-)

 
 @@ -373,7 +372,6 @@
feature name='sse'/
feature name='sse2'/
feature name='pni'/
 -  feature name='sse4a'/
 
 Should we use this opportunity to sort the remaining feature names?  At
 any rate, ACK.

NACK, Jiri reminded me I forgot to run the tests and there is one fix
missing in the xml files. The v2 is coming right up.

Martin

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

[libvirt] [PATCH] Added support for AMD Bulldozer CPU

2012-03-13 Thread Martin Kletzander

AMD Bulldozer (or Opteron_G4 as called in QEMU) was added to the list
of cpu models, flags were taken from upstream qemu cpu specifications
and should be sorted by bit values (or first occurence in the feature
specification part of cpu_map.xml).

Based on QEMU upstream commit 885bb0369a4f0abe2c0185178f3cb347cb02cdf1.
---
 src/cpu/cpu_map.xml |   48 
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 6a6603b..8f80dd9 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -692,5 +692,53 @@
   feature name='misalignsse'/
   feature name='lahf_lm'/
 /model
+
+model name='Opteron_G4'
+  vendor name='AMD'/
+  feature name='fpu'/
+  feature name='de'/
+  feature name='pse'/
+  feature name='tsc'/
+  feature name='msr'/
+  feature name='pae'/
+  feature name='mce'/
+  feature name='cx8'/
+  feature name='apic'/
+  feature name='sep'/
+  feature name='mtrr'/
+  feature name='pge'/
+  feature name='mca'/
+  feature name='cmov'/
+  feature name='pat'/
+  feature name='pse36'/
+  feature name='clflush'/
+  feature name='mmx'/
+  feature name='fxsr'/
+  feature name='sse'/
+  feature name='sse2'/
+  feature name='pni'/
+  feature name='pclmuldq'/
+  feature name='ssse3'/
+  feature name='cx16'/
+  feature name='sse4.1'/
+  feature name='sse4.2'/
+  feature name='popcnt'/
+  feature name='aes'/
+  feature name='xsave'/
+  feature name='avx'/
+  feature name='syscall'/
+  feature name='nx'/
+  feature name='pdpe1gb'/
+  feature name='rdtscp'/
+  feature name='lm'/
+  feature name='lahf_lm'/
+  feature name='svm'/
+  feature name='abm'/
+  feature name='sse4a'/
+  feature name='misalignsse'/
+  feature name='3dnowprefetch'/
+  feature name='xop'/
+  feature name='fma4'/
+/model
   /arch
 /cpus
-- 
1.7.3.4

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

Re: [libvirt] [PATCH] Added support for AMD Bulldozer CPU

2012-03-13 Thread Martin Kletzander

On 03/13/2012 12:56 PM, Peter Krempa wrote:
 On 03/13/2012 12:35 PM, Martin Kletzander wrote:
 AMD Bulldozer (or Opteron_G4 as called in QEMU) was added to the list
 of cpu models, flags were taken from upstream qemu cpu specifications
 and should be sorted by bit values (or first occurence in the feature
 specification part of cpu_map.xml).

 Based on QEMU upstream commit 885bb0369a4f0abe2c0185178f3cb347cb02cdf1.
 ---
 
 ACK  pushed.
 

Thanks

 Sadly, qemu is not sticking to that ordering sometimes :(
 

I thought QEMU does, as well as /proc/cpuinfo etc. Anyway, remind me
when you have the Sandy bridge ACK'd, so I can do the whole cleanup
thing. Everything will be sorted then.

 Peter

Martin

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

[libvirt] [PATCH] Fixed NULL pointer check

2012-03-17 Thread Martin Kletzander

This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
 src/util/conf.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..e76362c 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
 /**
  * conf.c: parser for a subset of the Python encoded Xen configuration files
  *
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
 {
 virConfEntryPtr cur;

+if (conf == NULL)
+return(NULL);
+
 cur = conf-entries;
 while (cur != NULL) {
 if ((cur-name != NULL) 
-- 
1.7.3.4

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

Re: [libvirt] [PATCH] Fixed NULL pointer check

2012-03-19 Thread Martin Kletzander

On 03/19/2012 08:43 AM, Wen Congyang wrote:
 At 03/18/2012 08:29 AM, Martin Kletzander Wrote:
 This patch fixes a NULL pointer check that was causing SegFault on
 some specific configurations.
 ---
  src/util/conf.c |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)

 diff --git a/src/util/conf.c b/src/util/conf.c
 index 8ad60e0..e76362c 100644
 --- a/src/util/conf.c
 +++ b/src/util/conf.c
 @@ -1,7 +1,7 @@
  /**
   * conf.c: parser for a subset of the Python encoded Xen configuration files
   *
 - * Copyright (C) 2006-2011 Red Hat, Inc.
 + * Copyright (C) 2006-2012 Red Hat, Inc.
   *
   * See COPYING.LIB for the License of this software
   *
 @@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
  {
  virConfEntryPtr cur;

 +if (conf == NULL)
 +return(NULL);
 
 Please use 'return NULL;' instead of 'return(NULL);'
 
 'return(NULL);' is old style, and we donot use it now and later.
 

It seems weird to me too, it's just that it's almost everywhere in this
file so that's why I wanted to keep the same look.

 +
  cur = conf-entries;
  while (cur != NULL) {
  if ((cur-name != NULL) 
 
 ACK with the nit fixed.
 

I'll send a second version, I don't have push permissions.

 Wen Congyang
 

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

[libvirt] [PATCH v2] Fixed NULL pointer check

2012-03-19 Thread Martin Kletzander

This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations.
---
 src/util/conf.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..3370337 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
 /**
  * conf.c: parser for a subset of the Python encoded Xen configuration files
  *
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
 {
 virConfEntryPtr cur;

+if (conf == NULL)
+return NULL;
+
 cur = conf-entries;
 while (cur != NULL) {
 if ((cur-name != NULL) 
-- 
1.7.3.4

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

[libvirt] [PATCH v3] Fixed NULL pointer check

2012-03-19 Thread Martin Kletzander

This patch fixes a NULL pointer check that was causing SegFault on
some specific configurations. It also reverts commit 59d0c9801c1ab
that was checking for this value in one place.
---
v3:
 - added revert of 59d0c9801c1ab that's not needed anymore

 - (comment) ATTRIBUTE_RETURN_CHECK not added as it is not used with other
   functions defined in this file and the commit could be confusing modifying
   all of them together. However, there should be one patch to fix all these
   at once if possible

v2:
 - removed parenthesis around return value

 src/libvirt.c   |3 +--
 src/util/conf.c |5 -
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 7f8d42c..99b263e 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1085,8 +1085,7 @@ virConnectOpenResolveURIAlias(virConfPtr conf,

 *uri = NULL;

-if (conf 
-(value = virConfGetValue(conf, uri_aliases)))
+if ((value = virConfGetValue(conf, uri_aliases)))
 ret = virConnectOpenFindURIAliasMatch(value, alias, uri);
 else
 ret = 0;
diff --git a/src/util/conf.c b/src/util/conf.c
index 8ad60e0..3370337 100644
--- a/src/util/conf.c
+++ b/src/util/conf.c
@@ -1,7 +1,7 @@
 /**
  * conf.c: parser for a subset of the Python encoded Xen configuration files
  *
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
 {
 virConfEntryPtr cur;

+if (conf == NULL)
+return NULL;
+
 cur = conf-entries;
 while (cur != NULL) {
 if ((cur-name != NULL) 
--
1.7.3.4

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

[libvirt] [PATCH] Cpu mapping cleanup

2012-03-19 Thread Martin Kletzander

Using inheritance, this patch cleans up the cpu_map.xml file and also
sorts all CPU features according to the feature and registry
values. Model features are sorted the same way as foeatures in the
specification.
Also few models that are related were organized together and parts of
the XML are marked with comments
---
 src/cpu/cpu_map.xml |  455 +-
 1 files changed, 82 insertions(+), 373 deletions(-)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 6af9c40..9a89e2e 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -268,6 +268,7 @@
   feature name='pse'/
 /model

+!-- Intel-based QEMU generic CPU models --
 model name='pentium'
   model name='486'/
   feature name='de'/
@@ -302,482 +303,190 @@
   feature name='pse'/
   feature name='tsc'/
   feature name='msr'/
+  feature name='pae'/
   feature name='mce'/
   feature name='cx8'/
+  feature name='apic'/
+  feature name='sep'/
   feature name='pge'/
   feature name='cmov'/
   feature name='pat'/
-  feature name='fxsr'/
   feature name='mmx'/
-  feature name='sse'/
-  feature name='sse2'/
-  feature name='pae'/
-  feature name='sep'/
-  feature name='apic'/
-/model
-
-model name='qemu32'
-  model name='pentiumpro'/
-  feature name='pni'/
-/model
-
-model name='cpu64-rhel5'
-  feature name='apic'/
-  feature name='clflush'/
-  feature name='cmov'/
-  feature name='cx8'/
-  feature name='de'/
-  feature name='fpu'/
   feature name='fxsr'/
-  feature name='lm'/
-  feature name='mca'/
-  feature name='mce'/
-  feature name='mmx'/
-  feature name='msr'/
-  feature name='mtrr'/
-  feature name='nx'/
-  feature name='pae'/
-  feature name='pat'/
-  feature name='pge'/
-  feature name='pse'/
-  feature name='pse36'/
-  feature name='sep'/
   feature name='sse'/
   feature name='sse2'/
-  feature name='pni'/
-  feature name='syscall'/
-  feature name='tsc'/
-/model
-
-model name='cpu64-rhel6'
-  feature name='apic'/
-  feature name='clflush'/
-  feature name='cmov'/
-  feature name='cx16'/
-  feature name='cx8'/
-  feature name='de'/
-  feature name='fpu'/
-  feature name='fxsr'/
-  feature name='lahf_lm'/
-  feature name='lm'/
-  feature name='mca'/
-  feature name='mce'/
-  feature name='mmx'/
-  feature name='msr'/
-  feature name='mtrr'/
-  feature name='nx'/
-  feature name='pae'/
-  feature name='pat'/
-  feature name='pge'/
-  feature name='pse'/
-  feature name='pse36'/
-  feature name='sep'/
-  feature name='sse'/
-  feature name='sse2'/
-  feature name='pni'/
-  feature name='syscall'/
-  feature name='tsc'/
-/model
-
-model name='kvm32'
-  model name='pentiumpro'/
-  feature name='mtrr'/
-  feature name='clflush'/
-  feature name='mca'/
-  feature name='pse36'/
-  feature name='pni'/
 /model

 model name='coreduo'
   model name='pentiumpro'/
+  vendor name='Intel'/
   feature name='vme'/
   feature name='mtrr'/
-  feature name='clflush'/
   feature name='mca'/
+  feature name='clflush'/
   feature name='pni'/
   feature name='monitor'/
   feature name='nx'/
 /model

-model name='qemu64'
-  model name='pentiumpro'/
-  feature name='mtrr'/
-  feature name='clflush'/
-  feature name='mca'/
-  feature name='pse36'/
-  feature name='pni'/
-  feature name='cx16'/
-  feature name='lm'/
-  feature name='syscall'/
-  feature name='nx'/
-  feature name='svm'/
-  !-- These are supported only by TCG.  KVM supports them only if the
-   host does.  So we leave them out:
-
-   feature name='popcnt'/
-   feature name='lahf_lm'/
-   feature name='sse4a'/
-   feature name='abm'/
-  --
+model name='n270'
+  model name='coreduo'/
+  feature name='ssse3'/
 /model

-model name='kvm64'
-  model name='pentiumpro'/
-  feature name='mtrr'/
-  feature name='clflush'/
-  feature name='mca'/
+model name='core2duo'
+  model name='n270'/
   feature name='pse36'/
-  feature name='pni'/
-  feature name='cx16'/
-  feature name='lm'/
   feature name='syscall'/
-  feature name='nx'/
+  feature name='lm'/
 /model

-model name='core2duo'
+!-- Generic QEMU CPU models --
+model name='qemu32'
   model name='pentiumpro'/
-  feature name='mtrr'/
-  feature name='clflush'/
-  feature name='mca'/
-  feature name='vme'/
-  feature name='pse36'/
   feature name='pni'/
-  feature name='monitor'/
-  feature name='ssse3'/
-  feature name='lm'/
-  feature name='syscall'/
-  feature name='nx'/

Re: [libvirt] [PATCH] Fixed NULL pointer check

2012-03-19 Thread Martin Kletzander

On 03/19/2012 11:32 AM, Daniel P. Berrange wrote:
 On Mon, Mar 19, 2012 at 08:51:24AM +0100, Martin Kletzander wrote:
 On 03/19/2012 08:43 AM, Wen Congyang wrote:
 At 03/18/2012 08:29 AM, Martin Kletzander Wrote:
 This patch fixes a NULL pointer check that was causing SegFault on
 some specific configurations.
 ---
  src/util/conf.c |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)

 diff --git a/src/util/conf.c b/src/util/conf.c
 index 8ad60e0..e76362c 100644
 --- a/src/util/conf.c
 +++ b/src/util/conf.c
 @@ -1,7 +1,7 @@
  /**
   * conf.c: parser for a subset of the Python encoded Xen configuration 
 files
   *
 - * Copyright (C) 2006-2011 Red Hat, Inc.
 + * Copyright (C) 2006-2012 Red Hat, Inc.
   *
   * See COPYING.LIB for the License of this software
   *
 @@ -836,6 +836,9 @@ virConfGetValue(virConfPtr conf, const char *setting)
  {
  virConfEntryPtr cur;

 +if (conf == NULL)
 +return(NULL);

 Please use 'return NULL;' instead of 'return(NULL);'

 'return(NULL);' is old style, and we donot use it now and later.


 It seems weird to me too, it's just that it's almost everywhere in this
 file so that's why I wanted to keep the same look.
 
 
 That is a historical remant. Do feel free to send another followup patch to
 cleanup all the cases of 'return(NULL)' as well.
 
 Daniel

Did you mean in that file or globally? Because I just tried the first
thing that came to my mind and look at the output:

libvirt $ find . -name '*.[ch]' -type f -exec grep -nH -e \
'return(.*);' {} + | wc -l
852

Not that I wouldn't do it, it just seem as a pretty big change. On the
other hand, I don't see the point in changing that in only one file.

Martin

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

Re: [libvirt] [PATCH] Fixed NULL pointer check

2012-03-19 Thread Martin Kletzander

On 03/19/2012 04:43 PM, Eric Blake wrote:
 On 03/19/2012 06:43 AM, Daniel P. Berrange wrote:
[...]
 since the open '(' lets the rest of the code indent nicely when using
 default emacs indentation.  But it's still pretty easy to recognize the
 difference between complex returns and the real offenders.  I think the
 number of false positives and false negatives is pretty near zero if you
 boil it down to detecting uses where there are no spaces between the '('
 and ')'.  Thus, for cfg.mk, I suggest a pattern something like:
 
 sc_prohibit_return_as_function:
 @prohibit='\return *([^ ]*)' \
 halt='avoid extra () with return statements'  \
   $(_sc_search_regexp)

I agree with the first part. There are some places that should be kept
as is. However, the '\return *([^ ]*)' would generate some false
positives, for example 'return (buf[0]  0) | (buf[1]  8)' and few
others I've found.
Don't worry though, I've created a regexp that matches just what's
needed, composing the patch right now.

Martin

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

Re: [libvirt] [PATCH] Cpu mapping cleanup

2012-03-19 Thread Martin Kletzander

On 03/19/2012 06:10 PM, Eric Blake wrote:
 On 03/19/2012 06:29 AM, Martin Kletzander wrote:
 Using inheritance, this patch cleans up the cpu_map.xml file and also
 sorts all CPU features according to the feature and registry
 values. Model features are sorted the same way as foeatures in the
 
 s/foeatures/features/
 
 specification.
 Also few models that are related were organized together and parts of
 the XML are marked with comments
 ---
  src/cpu/cpu_map.xml |  455 
 +-
  1 files changed, 82 insertions(+), 373 deletions(-)
 
 The inheritance is definitely an improvement for reducing lines of code.
  The patch was a bit hard to read (splitting things into three patches:
 reordering options, reordering CPU models into groups, then taking
 advantage of inheritance; rather than doing all three actions in one
 patch, might have been easier), but seems correct after the few minutes
 I've spent on it, and isn't worth the time to respin the series just to
 split the patch.

I was thinking how to split the patch and this variant didn't cross my
mind. Definitely next time =)

 
 ACK.
 

Could you also push it, please (I don't have permissions for that)?
Thanks

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

[libvirt] [PATCH] Minor docs fix

2012-03-19 Thread Martin Kletzander

End tag for host element was missing in example configuration
---
 docs/formatnetwork.html.in |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 907c046..7e8e991 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -363,6 +363,7 @@
   lt;host ip='192.168.122.2'gt;
 lt;hostnamegt;myhostlt;/hostnamegt;
 lt;hostnamegt;myhostaliaslt;/hostnamegt;
+  lt;/hostgt;
 lt;/dnsgt;
 lt;ip address=192.168.122.1 netmask=255.255.255.0gt;
   lt;dhcpgt;
-- 
1.7.3.4

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

Re: [libvirt] [test-API PATCH 0/7] Multiple fixes and improvements series

2012-03-21 Thread Martin Kletzander

On 03/21/2012 01:46 PM, Peter Krempa wrote:
 This is a set of more or less independent fixes and improvements to the
 test API. I ran across these while trying to write a basic test case as 
 a Hello world! to the test-API.
 
 Improvements are in fields of cross-distro compatibility, broken API's and
 typos and usability.
 
 Peter Krempa (7):
   utils: Make ipget.sh more portable
   lib: fix streamAPI class
   domainAPI: Add wrapper method to work with domain's console
 connections
   repos/domain/create.py: Fix typo in path to python executable
   utils: Allow suffixes in path to the main checkout folder
   parser: Be more specific on mistakes in case files
   domain/[start|destroy]: Add a optional noping flag to skip the ping
 test
 
  exception.py|   10 +
  lib/domainAPI.py|9 
  lib/streamAPI.py|   52 +++---
  proxy.py|4 +++
  repos/domain/create.py  |2 +-
  repos/domain/destroy.py |   50 +---
  repos/domain/start.py   |   50 +---
  utils/Python/utils.py   |2 +-
  utils/ipget.sh  |4 +-
  9 files changed, 105 insertions(+), 78 deletions(-)
 

I don't know if my ACK would mean something, it should be probably
checked by someone else, too.
There's no need for parentheses around the second '.*' block in 5/7 and
you have different types of quotes and string formatting in 6/7 and 7/7,
but I feel there is bigger need for this to work then spending huge
amount of time creating some python coding style, so (probably
meaningless) ACK from me for the whole series.

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

Re: [libvirt] [test-API PATCH 7/7] domain/[start|destroy]: Add a optional noping flag to skip the ping test

2012-03-22 Thread Martin Kletzander

On 03/21/2012 06:13 PM, Guannan Ren wrote:
 On 03/21/2012 08:46 PM, Peter Krempa wrote:
 For some tests it's not needed to ping the guest in the startup process.
 This patch adds a flag to the start and destroy test to skip such
 attempts (that consume a lot of time)
 ---
   repos/domain/destroy.py |   54
 ++
   repos/domain/start.py   |   50
 --
   2 files changed, 54 insertions(+), 50 deletions(-)

 diff --git a/repos/domain/destroy.py b/repos/domain/destroy.py
 index f98b602..12399d6 100644
 --- a/repos/domain/destroy.py
 +++ b/repos/domain/destroy.py
 @@ -50,7 +50,10 @@ def destroy(params):
  {'guestname': guestname}

  logger -- an object of utils/Python/log.py
 -   guestname -- same as the domain name
 +   guestname -- the domain name
 +   flags -- optional arguments:
 +  noping: Don't do the ping test
 +

  Return 0 on SUCCESS or 1 on FAILURE
   
 @@ -62,6 +65,7 @@ def destroy(params):
   if params_check_result:
   return 1
   guestname = params['guestname']
 +flags = params['flags']
 
   The 'flags' is optional, then we have to check if the
 dictionary of params has key or not
   if params.has_key('flags'):
   ...
   otherwise, it will report KeyError: 
 
   If 'flags' is mandatory,  it'd better to to check it in
 check_params function.
 

I'd rather do it using the get() method for dictionaries with some
default, i.e. params.get('flags', None).

Just my $0.02
Martin

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

[libvirt] [PATCH 0/2] Cleanup for return with parentheses

2012-03-22 Thread Martin Kletzander

There was one big inconsistency in our source files when return was
used. Sometimes it was used with parentheses and sometimes
without. The old-style return(value) was removed in the first commit,
the second commit introduces new syntax-check to ensure the style will
remain the same in the future.

Martin Kletzander (2):
  Cleanup for a return statement in source files
  Added syntax-check rule for return with parentheses

 cfg.mk|6 +
 examples/dominfo/info1.c  |2 +-
 examples/domsuspend/suspend.c |6 +-
 python/generator.py   |8 +-
 python/libvirt-override.c |  216 +-
 python/libvirt-qemu-override.c|4 +-
 python/typewrappers.c |   76 +-
 src/conf/domain_conf.c|   22 ++--
 src/conf/interface_conf.c |   70 +-
 src/conf/nwfilter_params.c|2 +-
 src/cpu/cpu_x86.c |2 +-
 src/datatypes.c   |   80 +-
 src/interface/netcf_driver.c  |2 +-
 src/lxc/lxc_driver.c  |4 +-
 src/nwfilter/nwfilter_ebiptables_driver.c |4 +-
 src/qemu/qemu_command.c   |6 +-
 src/qemu/qemu_driver.c|4 +-
 src/qemu/qemu_process.c   |2 +-
 src/remote/remote_driver.c|2 +-
 src/security/security_selinux.c   |2 +-
 src/test/test_driver.c|8 +-
 src/util/conf.c   |  106 +++---
 src/util/hooks.c  |   20 ++--
 src/util/sexpr.c  |   14 +-
 src/util/util.c   |8 +-
 src/util/uuid.c   |6 +-
 src/util/virhash.c|   30 ++--
 src/util/virmacaddr.c |2 +-
 src/util/virnetlink.c |2 +-
 src/util/virsocketaddr.c  |   48 +++---
 src/util/virterror.c  |   14 +-
 src/util/xml.c|   50 +++---
 src/xen/xen_driver.c  |   32 ++--
 src/xen/xen_hypervisor.c  |  228 ++--
 src/xen/xend_internal.c   |  234 ++--
 src/xen/xm_internal.c |   98 ++--
 src/xen/xs_internal.c |  120 
 src/xenapi/xenapi_driver.c|2 +-
 src/xenxs/xen_sxpr.c  |2 +-
 src/xenxs/xen_xm.c|2 +-
 tests/commandtest.c   |4 +-
 tests/cputest.c   |2 +-
 tests/domainsnapshotxml2xmltest.c |4 +-
 tests/interfacexml2xmltest.c  |2 +-
 tests/networkxml2argvtest.c   |2 +-
 tests/networkxml2xmltest.c|2 +-
 tests/nodedevxml2xmltest.c|2 +-
 tests/nodeinfotest.c  |2 +-
 tests/nwfilterxml2xmltest.c   |2 +-
 tests/qemuargv2xmltest.c  |2 +-
 tests/qemuxml2argvtest.c  |2 +-
 tests/qemuxml2xmltest.c   |4 +-
 tests/qemuxmlnstest.c |2 +-
 tests/qparamtest.c|2 +-
 tests/sexpr2xmltest.c |4 +-
 tests/sockettest.c|2 +-
 tests/statstest.c |2 +-
 tests/storagepoolxml2xmltest.c|2 +-
 tests/storagevolxml2xmltest.c |2 +-
 tests/virbuftest.c|2 +-
 tests/virnetmessagetest.c |2 +-
 tests/virnetsockettest.c  |4 +-
 tests/virnettlscontexttest.c  |2 +-
 tests/virshtest.c |2 +-
 tests/virtimetest.c   |2 +-
 tests/xencapstest.c   |2 +-
 tests/xmconfigtest.c  |4 +-
 tests/xml2sexprtest.c |4 +-
 tools/virsh.c |8 +-
 69 files changed, 815 insertions(+), 809 deletions(-)

--
1.7.3.4

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

[libvirt] [PATCH 2/2] Added syntax-check rule for return with parentheses

2012-03-22 Thread Martin Kletzander

After cleanup introduced with previous commit, there is a need for
syntax-check rule taking care of return(). Regexp used in 'prohibit'
parameter is taken from the cleanup commit and modified so it fits
'grep -E' format. Semicolon at the end is needed, otherwise the regexp
could match return with cast.

No exception is needed thanks to files excused by default. The
exception is not even needed for python files because 1) when
returning a tuple, you can still omit parentheses around (unless there
is only one value inside, but that's very unusual case), 2) return in
python doesn't need semicolon at the end of statement when there is no
statement following (and statement following 'return' doesn't make
sense).
---
 cfg.mk |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 24e6a69..59b07ef 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -469,6 +469,12 @@ sc_prohibit_xmlURI:
halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)'  \
  $(_sc_search_regexp)

+# we don't want old old-style return with parentheses around argument
+sc_prohibit_return_as_function:
+   @prohibit='\return *\(([^()]*(\([^()]*\)[^()]*)*)\) *;'\
+   halt='avoid extra () with return statements'\
+ $(_sc_search_regexp)
+
 # ATTRIBUTE_UNUSED should only be applied in implementations, not
 # header declarations
 sc_avoid_attribute_unused_in_header:
-- 
1.7.3.4

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

Re: [libvirt] [PATCH 2/2] Added syntax-check rule for return with parentheses

2012-03-23 Thread Martin Kletzander

On 03/23/2012 07:21 AM, Osier Yang wrote:
 On 03/22/2012 07:33 PM, Martin Kletzander wrote:
 After cleanup introduced with previous commit, there is a need for
 syntax-check rule taking care of return(). Regexp used in 'prohibit'
 parameter is taken from the cleanup commit and modified so it fits
 'grep -E' format. Semicolon at the end is needed, otherwise the regexp
 could match return with cast.

 No exception is needed thanks to files excused by default. The
 exception is not even needed for python files because 1) when
 returning a tuple, you can still omit parentheses around (unless there
 is only one value inside, but that's very unusual case),  2) return in
 python doesn't need semicolon at the end of statement when there is no
 statement following (and statement following 'return' doesn't make
 sense).
 
 This forces the Python coding style actually, prohibiting one
 writes Python codes like:
 
 def test():
 return (1);
 
 def test1():
 return (1,2,3);
 
 regardless of whether coding like this is good or bad, usual or
 unsual, they are legal in Python, and we don't say they are not
 allowed in HACKING or somewhere else.
 
 Not sure if we should allow this, even we will allow that, we
 need to document it somewhere, or more clear and specific for .py
 files in halt message.
 

You're right. I feel inclined to the version where we don't force this
for python code. My initial concern was about python-generated C code,
but that's a small portion of the code and it's fixed already. Instead
of creating some internal python code style, I'll send v2 with the
exception in a minute.

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

[libvirt] [PATCH v2 2/2] Added syntax-check rule for return with parentheses

2012-03-23 Thread Martin Kletzander

After cleanup introduced with previous commit, there is a need for
syntax-check rule taking care of return(). Regexp used in 'prohibit'
parameter is taken from the cleanup commit and modified so it fits
'grep -E' format. Semicolon at the end is needed, otherwise the regexp
could match return with cast.

Exception is created for python source files because we don't have any
documentation restricting the use of return that matches this case.
---
 cfg.mk |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 24e6a69..eae629b 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -469,6 +469,12 @@ sc_prohibit_xmlURI:
halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)'  \
  $(_sc_search_regexp)

+# we don't want old old-style return with parentheses around argument
+sc_prohibit_return_as_function:
+   @prohibit='\return *\(([^()]*(\([^()]*\)[^()]*)*)\) *;'\
+   halt='avoid extra () with return statements'\
+ $(_sc_search_regexp)
+
 # ATTRIBUTE_UNUSED should only be applied in implementations, not
 # header declarations
 sc_avoid_attribute_unused_in_header:
@@ -791,6 +797,8 @@ exclude_file_name_regexp--sc_prohibit_xmlGetProp = 
^src/util/xml\.c$$

 exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$

+exclude_file_name_regexp--sc_prohibit_return_as_function = ^\.py$$
+
 exclude_file_name_regexp--sc_require_config_h = ^examples/

 exclude_file_name_regexp--sc_require_config_h_first = ^examples/
--
1.7.3.4

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

[libvirt] [test-API PATCH 0/6] General improvements and fixes

2012-03-24 Thread Martin Kletzander

This series fixed some of the issues I came across while trying to
make some tests working. I still haven't managed to run more than
define, start, destroy and undefine tests, but at least this should
help someone to get a start at this.

There are many things that could (should?) be changed (either with or
without a discussion), but maybe I missed something, so feel free to
comment on all the patches if you have any input, I'll be glad to
discuss/change anything in order to move forward.

Each of the patches has it's own description and they are not that
much related to each other, so I won't describe it all here again, I
just didn't want to create unnecessary threads in the mailing list, so
I'm sending this as a series.

Martin Kletzander (6):
  Slight cross-distribution support
  Added support for Gentoo
  Fixed domain/start.py
  Add default 'uri' parameter to all tests
  Cleanup and fix of domain/define test
  Make use of new 'uri' parameter in tests

 env_inspect.py = dist/gentoo/env_inspect.py   |   88 +-
 env_inspect.py = dist/redhat/env_inspect.py   |0
 generator.py   |   18 +++-
 repos/domain/attach_disk.py|2 +-
 repos/domain/attach_interface.py   |2 +-
 repos/domain/autostart.py  |2 +-
 repos/domain/balloon_memory.py |2 +-
 repos/domain/blkstats.py   |2 +-
 repos/domain/cpu_affinity.py   |2 +-
 repos/domain/cpu_topology.py   |2 +-
 repos/domain/create.py |2 +-
 repos/domain/define.py |  132 +++
 repos/domain/destroy.py|2 +-
 repos/domain/detach_disk.py|2 +-
 repos/domain/detach_interface.py   |2 +-
 repos/domain/domain_blkinfo.py |2 +-
 repos/domain/domain_id.py  |2 +-
 repos/domain/domain_uuid.py|2 +-
 repos/domain/dump.py   |2 +-
 repos/domain/eventhandler.py   |2 +-
 repos/domain/ifstats.py|2 +-
 repos/domain/install_image.py  |6 +-
 repos/domain/install_linux_cdrom.py|6 +-
 repos/domain/install_linux_check.py|2 +-
 repos/domain/install_linux_net.py  |7 +-
 repos/domain/install_windows_cdrom.py  |7 +-
 repos/domain/ownership_test.py |2 +-
 repos/domain/reboot.py |2 +-
 repos/domain/restore.py|2 +-
 repos/domain/resume.py |2 +-
 repos/domain/save.py   |2 +-
 repos/domain/sched_params.py   |2 +-
 repos/domain/shutdown.py   |2 +-
 repos/domain/start.py  |   17 ++-
 repos/domain/suspend.py|2 +-
 repos/domain/undefine.py   |2 +-
 repos/domain/update_devflag.py |2 +-
 repos/interface/create.py  |2 +-
 repos/interface/define.py  |2 +-
 repos/interface/destroy.py |2 +-
 repos/interface/undefine.py|2 +-
 repos/libvirtd/qemu_hang.py|2 +-
 repos/libvirtd/restart.py  |2 +-
 repos/network/autostart.py |2 +-
 repos/network/create.py|2 +-
 repos/network/define.py|2 +-
 repos/network/destroy.py   |2 +-
 repos/network/network_list.py  |2 +-
 repos/network/network_name.py  |2 +-
 repos/network/network_uuid.py  |2 +-
 repos/network/start.py |2 +-
 repos/network/undefine.py  |2 +-
 repos/nodedevice/detach.py |2 +-
 repos/nodedevice/reattach.py   |2 +-
 repos/nodedevice/reset.py  |2 +-
 repos/npiv/create_virtual_hba.py   |2 +-
 .../multiple_thread_block_on_domain_create.py  |2 +-
 repos/sVirt/domain_nfs_start.py|4 +-
 repos/snapshot/delete.py   |2 +-
 repos/snapshot/file_flag.py|2 +-
 repos/snapshot/flag_check.py   |2 +-
 repos/snapshot/internal_create.py  |2 +-
 repos/snapshot/revert.py   |2 +-
 repos/storage/activate_pool.py |2 +-
 repos/storage/build_dir_pool.py

[libvirt] [test-API PATCH 5/6] Cleanup and fix of domain/define test

2012-03-24 Thread Martin Kletzander

I added support for 'uri' parameter and moved some functions into
util/Python/utils.py in order not to lose them from the code and keep
them accessible for other tests.
---
 repos/domain/define.py |  132 
 utils/Python/utils.py  |   45 -
 2 files changed, 65 insertions(+), 112 deletions(-)

diff --git a/repos/domain/define.py b/repos/domain/define.py
index 8f0095a..25630c5 100644
--- a/repos/domain/define.py
+++ b/repos/domain/define.py
@@ -19,7 +19,7 @@
 __author__ = 'Alex Jia: a...@redhat.com'
 __date__ = 'Mon Jan 28, 2010'
 __version__ = '0.1.0'
-__credits__ = 'Copyright (C) 2009 Red Hat, Inc.'
+__credits__ = 'Copyright (C) 2009, 2012 Red Hat, Inc.'
 __all__ = ['usage', 'check_define_domain', 'define']

 import os
@@ -63,9 +63,6 @@ def usage():
macaddr
ifacetype
source
-   target_machine
-   username
-   password
   '''

 def check_params(params):
@@ -107,58 +104,17 @@ def ssh_keygen(logger):

 return 0

-def ssh_tunnel(hostname, username, password, logger):
-setup a tunnel to a give host
-logger.info(setup ssh tunnel with host %s % hostname)
-user_host = %s@%s % (username, hostname)
-child = pexpect.spawn(SSH_COPY_ID, [ user_host])
-while True:
-index = child.expect(['yes\/no', 'password: ',
-   pexpect.EOF,
-   pexpect.TIMEOUT])
-if index == 0:
-child.sendline(yes)
-elif index == 1:
-child.sendline(password)
-elif index == 2:
-logger.debug(string.strip(child.before))
-child.close()
-return 0
-elif index == 3:
-logger.error(setup tunnel timeout)
-logger.debug(string.strip(child.before))
-child.close()
-return 1
-
-return 0
-
-def check_define_domain(guestname, guesttype, target_machine, username, \
-password, util, logger):
-Check define domain result, if define domain is successful,
-   guestname.xml will exist under /etc/libvirt/qemu/
-   and can use virt-xml-validate tool to check the file validity
+def check_define_domain(conn, guestname, logger):
+Check define domain result. To make this work on all
+hypervisors and with all configuration posibilities, use the
+default way through libvirt to check if the guest was defined
 
-if kvm in guesttype:
-path = /etc/libvirt/qemu/%s.xml % guestname
-elif xen in guesttype:
-path = /etc/xen/%s % guestname
-else:
-logger.error(unknown guest type)
-
-if target_machine:
-cmd = ls %s % path
-ret, output = util.remote_exec_pexpect(target_machine, username, \
-   password, cmd)
-if ret:
-logger.error(guest %s xml file doesn't exsits % guestname)
-return False
-else:
-return True
-else:
-if os.access(path, os.R_OK):
-return True
-else:
-return False
+try:
+conn.lookupByName(guestname + 'asdf')
+return True
+except libvirtError, e:
+logger.error(e.message())
+return False

 def define(params):
 Define a domain from xml
@@ -169,41 +125,10 @@ def define(params):
 logger = params['logger']
 guestname = params['guestname']
 guesttype = params['guesttype']
+uri = params['uri']
 test_result = False

-if params.has_key('target_machine'):
-logger.info(define domain on remote host)
-target_machine = params['target_machine']
-username = params['username']
-password = params['password']
-else:
-logger.info(define domain on local host)
-target_machine = None
-username = None
-password = None
-
 # Connect to hypervisor connection URI
-util = utils.Utils()
-if target_machine:
-uri = util.get_uri(target_machine)
-
-#generate ssh key pair
-ret = ssh_keygen(logger)
-if ret:
-logger.error(failed to generate RSA key)
-return 1
-#setup ssh tunnel with target machine
-ret = ssh_tunnel(target_machine, username, password, logger)
-if ret:
-logger.error(faild to setup ssh tunnel with target machine %s % \
-  target_machine)
-return 1
-
-commands.getstatusoutput(ssh-add)
-
-else:
-uri = util.get_uri('127.0.0.1')
-
 conn = connectAPI.ConnectAPI()
 virconn = conn.open(uri)

@@ -222,35 +147,20 @@ def define(params):

 # Define domain from xml
 try:
-try:
-dom_obj.define(dom_xml)
-if check_define_domain(guestname, guesttype, target_machine, \
-

[libvirt] [test-API PATCH 1/6] Slight cross-distribution support

2012-03-24 Thread Martin Kletzander

There is no support for distributions without 'rpm' as a package
manager. This patch modifies (at this time) the only
distribution-specific import in order to ease the broadening of
distribution list supported by libvirt-test-API.
---
 env_inspect.py = dist/redhat/env_inspect.py |0
 generator.py |   12 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 dist/__init__.py
 create mode 100644 dist/redhat/__init__.py
 rename env_inspect.py = dist/redhat/env_inspect.py (100%)

diff --git a/dist/__init__.py b/dist/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/dist/redhat/__init__.py b/dist/redhat/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/env_inspect.py b/dist/redhat/env_inspect.py
similarity index 100%
rename from env_inspect.py
rename to dist/redhat/env_inspect.py
diff --git a/generator.py b/generator.py
index 6108963..4f4478b 100644
--- a/generator.py
+++ b/generator.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 #
-# libvirt-test-API is copyright 2010 Red Hat, Inc.
+# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
 #
 # libvirt-test-API is free software: you can redistribute it and/or modify it
 # under the terms of the GNU General Public License as published by
@@ -23,10 +23,18 @@ import sys
 import traceback

 import mapper
-import env_inspect
 from utils.Python import log
 from utils.Python import format

+# Import of distribution-specific code.  If this is needed somewhere
+# else in the future, please don't copy-paste this, but create some
+# sensible distribution-specific package
+import os
+for dist in os.listdir('dist'):
+if os.path.exists('/etc/%s-release' % dist):
+exec('from dist.%s import env_inspect' % dist)
+break
+
 class FuncGen(object):
  To generate a callable testcase
 def __init__(self, cases_func_ref_dict,
-- 
1.7.3.4

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

[libvirt] [test-API PATCH 2/6] Added support for Gentoo

2012-03-24 Thread Martin Kletzander

In order to support libvirt-test-API on more distributions, this
commit adds support for Gentoo.
The file is copy-paste from dist/redhat/env_update.py just modified to
make the get_* functions work on Gentoo, some removed.
---
 dist/gentoo/env_inspect.py |   98 
 1 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100644 dist/gentoo/__init__.py
 create mode 100644 dist/gentoo/env_inspect.py

diff --git a/dist/gentoo/__init__.py b/dist/gentoo/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/dist/gentoo/env_inspect.py b/dist/gentoo/env_inspect.py
new file mode 100644
index 000..e8fccc0
--- /dev/null
+++ b/dist/gentoo/env_inspect.py
@@ -0,0 +1,98 @@
+#!/usr/bin/env python
+#
+# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
+#
+# libvirt-test-API is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version. This program is distributed in
+# the hope that it will be useful, but WITHOUT ANY WARRANTY; without
+# even the implied warranties of TITLE, NON-INFRINGEMENT,
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+#
+# The GPL text is available in the file COPYING that accompanies this
+# distribution and at http://www.gnu.org/licenses.
+#
+# Filename: envinspect.py
+# Summary: To generate a callable class for clearing testing environment
+# Description: The module match the reference of clearing function
+#  from each testcase to the corresponding testcase's
+#  argument in the order of testcase running
+
+import os
+import portage
+
+vt_dbapi = portage.db[portage.root]['vartree'].dbapi
+
+def get_libvirt_ver():
+pkg = vt_dbapi.match('app-emulation/libvirt')
+if pkg:
+return 0, pkg[-1]
+else:
+return 100, No libvirt installed
+
+def get_libvirt_pyth_ver():
+pkgs = vt_dbapi.match('app-emulation/libvirt')
+for pkg in pkgs:
+if 'python' in vt_dbapi.aux_get(pkg, ['USE'])[0].split():
+return 0, '%s[python]' % pkg
+
+return 100, USE flag 'python' not enabled for libvirt
+
+def get_qemu_kvm_ver():
+pkg = vt_dbapi.match('qemu-kvm') or vt_dbapi.match('qemu')
+if pkg:
+return 0, pkg[-1]
+else:
+return 100, No qemu installed
+
+def get_kernel_ver():
+# on Gentoo, there is no need to check for kernel
+return 0, os.uname()[2]
+
+
+class EnvInspect(object):
+to check and collect the testing enviroment infomation
+   before performing testing
+
+
+def __init__(self, logger):
+self.logger = logger
+
+def env_checking(self):
+flag = 0
+result = 
+if get_libvirt_ver()[0] == 100:
+result = NOTOK
+flag = 1
+else:
+result = OK
+self.logger.info(%-36s%-6s % (get_libvirt_ver()[1], result))
+
+if get_libvirt_pyth_ver()[0] == 100:
+result = NOTOK
+flag = 1
+else:
+result = OK
+self.logger.info(%-36s%-6s % (get_libvirt_pyth_ver()[1], result))
+
+if get_qemu_kvm_ver()[0] == 150 and flag == 0:
+flag = 0
+elif get_qemu_kvm_ver()[0] == 150 and flag == 1:
+flag = 1
+else:
+pass
+self.logger.info(%-36s%-6s % (get_qemu_kvm_ver()[1], OK))
+
+if get_kernel_ver()[0] == 100:
+result = NOTOK
+flag = 1
+else:
+result = OK
+self.logger.info(%-36s%-6s % (get_kernel_ver()[1], result))
+
+return flag
+
+
+OK = ok
+NOTOK = not ok
-- 
1.7.3.4

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

[libvirt] [test-API PATCH 3/6] Fixed domain/start.py

2012-03-24 Thread Martin Kletzander

Function usage() was called in the code but was missing.
Parameter 'flags' was defined as optional but it was still required in
the code, so I fixed the handling of it.
---
 repos/domain/start.py |   15 +++
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/repos/domain/start.py b/repos/domain/start.py
index cd028bf..38bad7f 100644
--- a/repos/domain/start.py
+++ b/repos/domain/start.py
@@ -1,12 +1,13 @@
 #!/usr/bin/env python
 for testing the start function of domain
mandatory arguments: guestname
+   optional arguments: flags
 

 __author__ = Osier Yang jy...@redhat.com
 __date__ = Tue Oct 27, 2009
 __version__ = 0.1.0
-__credits__ = Copyright (C) 2009 Red Hat, Inc.
+__credits__ = Copyright (C) 2009, 2012 Red Hat, Inc.
 __all__ = ['start', 'check_params', 'parse_opts',
'usage', 'version', 'append_path']

@@ -34,6 +35,11 @@ from exception import LibvirtAPI
 NONE = 0
 START_PAUSED = 1

+def usage():
+print '''usage: mandatory arguments: guestname
+   optional arguments: flags
+  '''
+
 def return_close(conn, logger, ret):
 conn.close()
 logger.info(closed hypervisor connection)
@@ -48,7 +54,7 @@ def check_params(params):

 logger = params['logger']

-keys = ['guestname', 'flags', 'logger']
+keys = ['guestname', 'logger']
 for key in keys:
 if key not in params:
 logger.error(key '%s' is required % key)
@@ -66,7 +72,7 @@ def start(params):

 logger -- an object of utils/Python/log.py
 mandatory arguments : guestname -- same as the domain name
-   optional arguments : flags -- domain create flags 
none|start_paused|noping
+optional arguments : flags -- domain create flags 
none|start_paused|noping

 Return 0 on SUCCESS or 1 on FAILURE
 
@@ -75,7 +81,7 @@ def start(params):
 check_params(params)
 domname = params['guestname']
 logger = params['logger']
-flags = params['flags']
+flags = params.get('flags', [])

 if none in flags and start_paused in flags:
 logger.error(Flags error: Can't specify none and start_paused 
simultaneously)
@@ -98,6 +104,7 @@ def start(params):
 elif start_paused in flags:
 dom_obj.start_with_flags(domname, START_PAUSED)
 else:
+# this covers flags = None as well as flags = 'noping'
 dom_obj.start(domname)
 except LibvirtAPI, e:
 logger.error(str(e))
-- 
1.7.3.4

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

[libvirt] [test-API PATCH 4/6] Add default 'uri' parameter to all tests

2012-03-24 Thread Martin Kletzander

Tests should be able to run on various hypervisors and machines. Now
we have only two tests that have an option to do something
remotely. This commit allows tests to see 'uri' parameter that they
should connect to. However it doesn't have to be used always
(migration etc.)
---
 generator.py |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/generator.py b/generator.py
index 4f4478b..5a4652a 100644
--- a/generator.py
+++ b/generator.py
@@ -154,6 +154,12 @@ class FuncGen(object):
 clean_ret = -1
 try:
 try:
+# having 'None' as default ensures we use the
+# correct URI taken from libvirt (configuration,
+# environment variables, etc.), but one can always
+# specify a different machine
+if 'uri' not in case_params:
+case_params['uri'] = None
 if case_ref_name != 'sleep':
 case_params['logger'] = case_logger

-- 
1.7.3.4

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

[libvirt] [test-API PATCH 6/6] Make use of new 'uri' parameter in tests

2012-03-24 Thread Martin Kletzander

I changed all tests to make use of the 'uri' parameter. This change
should be more re-factored and lots of things could be changed to make
the code more readable, but unfortunately there's no time for that
right now, so at least this small step towards the goal.
---
 repos/domain/attach_disk.py|2 +-
 repos/domain/attach_interface.py   |2 +-
 repos/domain/autostart.py  |2 +-
 repos/domain/balloon_memory.py |2 +-
 repos/domain/blkstats.py   |2 +-
 repos/domain/cpu_affinity.py   |2 +-
 repos/domain/cpu_topology.py   |2 +-
 repos/domain/create.py |2 +-
 repos/domain/destroy.py|2 +-
 repos/domain/detach_disk.py|2 +-
 repos/domain/detach_interface.py   |2 +-
 repos/domain/domain_blkinfo.py |2 +-
 repos/domain/domain_id.py  |2 +-
 repos/domain/domain_uuid.py|2 +-
 repos/domain/dump.py   |2 +-
 repos/domain/eventhandler.py   |2 +-
 repos/domain/ifstats.py|2 +-
 repos/domain/install_image.py  |6 --
 repos/domain/install_linux_cdrom.py|6 +++---
 repos/domain/install_linux_check.py|2 +-
 repos/domain/install_linux_net.py  |7 ---
 repos/domain/install_windows_cdrom.py  |7 ---
 repos/domain/ownership_test.py |2 +-
 repos/domain/reboot.py |2 +-
 repos/domain/restore.py|2 +-
 repos/domain/resume.py |2 +-
 repos/domain/save.py   |2 +-
 repos/domain/sched_params.py   |2 +-
 repos/domain/shutdown.py   |2 +-
 repos/domain/start.py  |2 +-
 repos/domain/suspend.py|2 +-
 repos/domain/undefine.py   |2 +-
 repos/domain/update_devflag.py |2 +-
 repos/interface/create.py  |2 +-
 repos/interface/define.py  |2 +-
 repos/interface/destroy.py |2 +-
 repos/interface/undefine.py|2 +-
 repos/libvirtd/qemu_hang.py|2 +-
 repos/libvirtd/restart.py  |2 +-
 repos/network/autostart.py |2 +-
 repos/network/create.py|2 +-
 repos/network/define.py|2 +-
 repos/network/destroy.py   |2 +-
 repos/network/network_list.py  |2 +-
 repos/network/network_name.py  |2 +-
 repos/network/network_uuid.py  |2 +-
 repos/network/start.py |2 +-
 repos/network/undefine.py  |2 +-
 repos/nodedevice/detach.py |2 +-
 repos/nodedevice/reattach.py   |2 +-
 repos/nodedevice/reset.py  |2 +-
 repos/npiv/create_virtual_hba.py   |2 +-
 .../multiple_thread_block_on_domain_create.py  |2 +-
 repos/sVirt/domain_nfs_start.py|4 ++--
 repos/snapshot/delete.py   |2 +-
 repos/snapshot/file_flag.py|2 +-
 repos/snapshot/flag_check.py   |2 +-
 repos/snapshot/internal_create.py  |2 +-
 repos/snapshot/revert.py   |2 +-
 repos/storage/activate_pool.py |2 +-
 repos/storage/build_dir_pool.py|2 +-
 repos/storage/build_disk_pool.py   |2 +-
 repos/storage/build_logical_pool.py|2 +-
 repos/storage/build_netfs_pool.py  |2 +-
 repos/storage/create_dir_pool.py   |2 +-
 repos/storage/create_dir_volume.py |2 +-
 repos/storage/create_fs_pool.py|2 +-
 repos/storage/create_iscsi_pool.py |2 +-
 repos/storage/create_logical_volume.py |2 +-
 repos/storage/create_netfs_pool.py |2 +-
 repos/storage/create_netfs_volume.py   |2 +-
 repos/storage/create_partition_volume.py   |2 +-
 repos/storage/define_dir_pool.py   |2 +-
 repos/storage/define_disk_pool.py  |2 +-
 repos/storage/define_iscsi_pool.py |2 +-
 repos/storage/define_logical_pool.py   |2 +-

Re: [libvirt] [test-API PATCHv2] libs: Add flags for streams and the console

2012-03-26 Thread Martin Kletzander

On 03/26/2012 11:07 AM, Peter Krempa wrote:
 Add the local copy of the flags.
 ---
  lib/domainAPI.py |3 +++
  lib/streamAPI.py |3 +++
  2 files changed, 6 insertions(+), 0 deletions(-)
 
 diff --git a/lib/domainAPI.py b/lib/domainAPI.py
 index bc0069b..43565c2 100644
 --- a/lib/domainAPI.py
 +++ b/lib/domainAPI.py
 @@ -912,3 +912,6 @@ VIR_DOMAIN_AFFECT_CURRENT = 0
  VIR_DOMAIN_AFFECT_LIVE = 1
  VIR_DOMAIN_AFFECT_CONFIG = 2
 
 +# virDomainConsoleFlags
 +VIR_DOMAIN_CONSOLE_FORCE = 1
 +VIR_DOMAIN_CONSOLE_SAFE  = 2
 diff --git a/lib/streamAPI.py b/lib/streamAPI.py
 index 18ce71e..0dfda28 100644
 --- a/lib/streamAPI.py
 +++ b/lib/streamAPI.py
 @@ -118,3 +118,6 @@ VIR_EVENT_HANDLE_READABLE = 1
  VIR_EVENT_HANDLE_WRITABLE = 2
  VIR_EVENT_HANDLE_ERROR = 4
  VIR_EVENT_HANDLE_HANGUP = 8
 +
 +# virStreamFlags
 +VIR_STREAM_NONBLOCK = 1

No flag is missing, flags are in right files according to the API, I'd
definitely say ACK, depends on you if you want a second opinion.

Martin

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

Re: [libvirt] [PATCH v2 2/2] Added syntax-check rule for return with parentheses

2012-03-26 Thread Martin Kletzander

On 03/26/2012 05:43 AM, Osier Yang wrote:
 On 2012年03月23日 15:34, Martin Kletzander wrote:
 After cleanup introduced with previous commit, there is a need for
 syntax-check rule taking care of return(). Regexp used in 'prohibit'
 parameter is taken from the cleanup commit and modified so it fits
 'grep -E' format. Semicolon at the end is needed, otherwise the regexp
 could match return with cast.

 Exception is created for python source files because we don't have any
 documentation restricting the use of return that matches this case.
 ---
   cfg.mk |8 
   1 files changed, 8 insertions(+), 0 deletions(-)

 diff --git a/cfg.mk b/cfg.mk
 index 24e6a69..eae629b 100644
 --- a/cfg.mk
 +++ b/cfg.mk
 @@ -469,6 +469,12 @@ sc_prohibit_xmlURI:
   halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)'\
 $(_sc_search_regexp)

 +# we don't want old old-style return with parentheses around argument
 +sc_prohibit_return_as_function:
 +@prohibit='\return *\(([^()]*(\([^()]*\)[^()]*)*)\) *;'\
 +halt='avoid extra () with return statements'\
 +  $(_sc_search_regexp)
 +
   # ATTRIBUTE_UNUSED should only be applied in implementations, not
   # header declarations
   sc_avoid_attribute_unused_in_header:
 @@ -791,6 +797,8 @@ exclude_file_name_regexp--sc_prohibit_xmlGetProp =
 ^src/util/xml\.c$$

   exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$

 +exclude_file_name_regexp--sc_prohibit_return_as_function = ^\.py$$
 +
   exclude_file_name_regexp--sc_require_config_h = ^examples/

   exclude_file_name_regexp--sc_require_config_h_first = ^examples/
 -- 
 1.7.3.4
 
 Looks good from my point of view, ACK. But we might need to
 push these later after 0.9.11.
 
 Regards,
 Osier
 

We've totally missed the '^' in the exception, that shouldn't be there,
so either please fix this before pushing it or send me NACK or ping me
on IRC so I know if I should send fixed version, thanks and sorry for
the trouble.

Martin

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

Re: [libvirt] LIBVIRT-PHP List of domains

2012-03-27 Thread Martin Kletzander

On 03/24/2012 03:24 PM, Ali Raza Memon wrote:
 Hi.. I am trying to get the names of all the xen domains using
 libvirt-php. I saw this php script: 
 
 ?php
  $conn = libvirt_connect('null', false);
  $doms = libvirt_list_domains($conn);
  print_r($doms); ?
 
  It gives me error. Could you please tell me how to connect to xen
 hypervisor? I have also tried: $conn = libvirt_connect('xen:///',
 false); but it wont work..
 

Hi,

I can only guess without further information. Is libvirt running on the
same machine as the script? What authentication are you using (you want
read/write connection)? As what user is the script running? Also the
error you get could help us solve this.

Martin

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

Re: [libvirt] [test-API PATCH 1/6] Slight cross-distribution support

2012-03-27 Thread Martin Kletzander

On 03/26/2012 02:42 PM, Michal Novotny wrote:
 On 03/26/2012 02:30 PM, Peter Krempa wrote:
 On 03/24/2012 06:42 PM, Martin Kletzander wrote:
 There is no support for distributions without 'rpm' as a package
 manager. This patch modifies (at this time) the only
 distribution-specific import in order to ease the broadening of
 distribution list supported by libvirt-test-API.
 ---
   env_inspect.py =  dist/redhat/env_inspect.py |0
   generator.py |   12 ++--
   2 files changed, 10 insertions(+), 2 deletions(-)
   create mode 100644 dist/__init__.py
   create mode 100644 dist/redhat/__init__.py
   rename env_inspect.py =  dist/redhat/env_inspect.py (100%)

 diff --git a/dist/__init__.py b/dist/__init__.py
 new file mode 100644
 index 000..e69de29
 diff --git a/dist/redhat/__init__.py b/dist/redhat/__init__.py
 new file mode 100644
 index 000..e69de29
 diff --git a/env_inspect.py b/dist/redhat/env_inspect.py
 similarity index 100%
 rename from env_inspect.py
 rename to dist/redhat/env_inspect.py
 diff --git a/generator.py b/generator.py
 index 6108963..4f4478b 100644
 --- a/generator.py
 +++ b/generator.py
 @@ -1,6 +1,6 @@
   #!/usr/bin/env python
   #
 -# libvirt-test-API is copyright 2010 Red Hat, Inc.
 +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
   #
   # libvirt-test-API is free software: you can redistribute it and/or 
 modify it
   # under the terms of the GNU General Public License as published by
 @@ -23,10 +23,18 @@ import sys
   import traceback

   import mapper
 -import env_inspect
   from utils.Python import log
   from utils.Python import format

 +# Import of distribution-specific code.  If this is needed somewhere
 +# else in the future, please don't copy-paste this, but create some
 +# sensible distribution-specific package
 +import os
 +for dist in os.listdir('dist'):
 +if os.path.exists('/etc/%s-release' % dist):
 +exec('from dist.%s import env_inspect' % dist)
 +break
 +
 Works great on Gentoo, but I'm afraid a bit that it could break Fedora 
 if it uses a different version file name. (but I don't have any at hand 
 so I can't test this )

 Peter
 
 Fedora is using /etc/redhat-release file so you use the same handling
 for Fedora and RHEL then it's fine.
 
 Michal
 

Exactly. I remembered that all Red Hat-based distributions have the file
(and should have it), thus rpm can be use to get all the info (same
imported file).

Little OT while on that note:
In the future it would be great to have one place where we examine the
sys.path used for importing other packages (instead of having the same
10 lines in every file in the whole libvirt-test-API) and in this place
this code could be done globally.

Martin

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

Re: [libvirt] [test-API PATCH 2/6] Added support for Gentoo

2012-03-27 Thread Martin Kletzander

On 03/26/2012 02:23 PM, Peter Krempa wrote:
 On 03/24/2012 06:42 PM, Martin Kletzander wrote:
 In order to support libvirt-test-API on more distributions, this
 commit adds support for Gentoo.
 The file is copy-paste from dist/redhat/env_update.py just modified to
 make the get_* functions work on Gentoo, some removed.
 
 Probably no one else that uses the test api uses Gentoo, so I'll do a
 review of this patch.
 
 ---
   dist/gentoo/env_inspect.py |   98
 
   1 files changed, 98 insertions(+), 0 deletions(-)
   create mode 100644 dist/gentoo/__init__.py
   create mode 100644 dist/gentoo/env_inspect.py

 diff --git a/dist/gentoo/__init__.py b/dist/gentoo/__init__.py
 new file mode 100644
 index 000..e69de29
 diff --git a/dist/gentoo/env_inspect.py b/dist/gentoo/env_inspect.py
 new file mode 100644
 index 000..e8fccc0
 --- /dev/null
 +++ b/dist/gentoo/env_inspect.py
 @@ -0,0 +1,98 @@
 +#!/usr/bin/env python
 +#
 +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
 
 s/2010, //
 

We should have 2010, 2012 or 2010-2012 in the file, not just 2010, I guess.

 +# libvirt-test-API is free software: you can redistribute it and/or
 modify it
 +# under the terms of the GNU General Public License as published by
 +# the Free Software Foundation, either version 2 of the License, or
 +# (at your option) any later version. This program is distributed in
 +# the hope that it will be useful, but WITHOUT ANY WARRANTY; without
 +# even the implied warranties of TITLE, NON-INFRINGEMENT,
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 +#
 +# The GPL text is available in the file COPYING that accompanies this
 +# distribution and athttp://www.gnu.org/licenses.
 +#
 +# Filename: envinspect.py
 
 s/envinspect/env_inspect/
 
 +# Summary: To generate a callable class for clearing testing environment
 +# Description: The module match the reference of clearing function
 +#  from each testcase to the corresponding testcase's
 +#  argument in the order of testcase running
 
 A little re-wording would help to understand what's going on here.
 

This and others are just a copy-paste from redhat/env_inspect.py and I
didn't write it but when writing v2, I'll try to rephrase this.

 +
 +import os
 +import portage
 +
 +vt_dbapi = portage.db[portage.root]['vartree'].dbapi
 +
 +def get_libvirt_ver():
 +pkg = vt_dbapi.match('app-emulation/libvirt')
 +if pkg:
 +return 0, pkg[-1]
 +else:
 +return 100, No libvirt installed
 +
 +def get_libvirt_pyth_ver():
 +pkgs = vt_dbapi.match('app-emulation/libvirt')
 +for pkg in pkgs:
 +if 'python' in vt_dbapi.aux_get(pkg, ['USE'])[0].split():
 +return 0, '%s[python]' % pkg
 +
 +return 100, USE flag 'python' not enabled for libvirt
 +
 +def get_qemu_kvm_ver():
 +pkg = vt_dbapi.match('qemu-kvm') or vt_dbapi.match('qemu')
 +if pkg:
 +return 0, pkg[-1]
 +else:
 +return 100, No qemu installed
 
 This function returns codes 0 or 100 ...
 
 +
 +def get_kernel_ver():
 +# on Gentoo, there is no need to check for kernel
 +return 0, os.uname()[2]
 +
 +
 +class EnvInspect(object):
 +to check and collect the testing enviroment infomation
 +   before performing testing
 +
 +
 +def __init__(self, logger):
 +self.logger = logger
 +
 +def env_checking(self):
 +flag = 0
 +result = 
 +if get_libvirt_ver()[0] == 100:
 +result = NOTOK
 +flag = 1
 +else:
 +result = OK
 +self.logger.info(%-36s%-6s % (get_libvirt_ver()[1],
 result))
 +
 +if get_libvirt_pyth_ver()[0] == 100:
 +result = NOTOK
 +flag = 1
 +else:
 +result = OK
 +self.logger.info(%-36s%-6s %
 (get_libvirt_pyth_ver()[1], result))
 +
 +if get_qemu_kvm_ver()[0] == 150 and flag == 0:
 +flag = 0
 +elif get_qemu_kvm_ver()[0] == 150 and flag == 1:
 +flag = 1
 
 ... so these tests here are not needed. (And are quite strange too ...
 but that's probably a cutpaste leftover)
 
 +else:
 +pass
 +self.logger.info(%-36s%-6s % (get_qemu_kvm_ver()[1], OK))
 
 And in any case, the test succeeds, so it's probably ment to be only a
 version check, that is not mandatory. IMO we should make this check
 mandatory for now, as most of the tests use the local hypervisor for
 testing machines and only a few tests actualy deal with remote code.
 
 +
 +if get_kernel_ver()[0] == 100:
 +result = NOTOK
 +flag = 1
 +else:
 
 Well, as was said in the comment, on Gentoo the kernel test always
 succeeds, so the return value check is not necessary and can be changed
 to something like:
 
self.logger.info(%-36s%-6s % (get_kernel_ver()[1], OK))
 
 and leave out the condition.

I wanted to completely rewrite this (I don't understand

Re: [libvirt] [test-API PATCH 4/6] Add default 'uri' parameter to all tests

2012-03-27 Thread Martin Kletzander

On 03/26/2012 03:15 PM, Guannan Ren wrote:
 On 03/25/2012 01:42 AM, Martin Kletzander wrote:
 Tests should be able to run on various hypervisors and machines. Now
 we have only two tests that have an option to do something
 remotely. This commit allows tests to see 'uri' parameter that they
 should connect to. However it doesn't have to be used always
 (migration etc.)
 ---
   generator.py |6 ++
   1 files changed, 6 insertions(+), 0 deletions(-)

 diff --git a/generator.py b/generator.py
 index 4f4478b..5a4652a 100644
 --- a/generator.py
 +++ b/generator.py
 @@ -154,6 +154,12 @@ class FuncGen(object):
   clean_ret = -1
   try:
   try:
 +# having 'None' as default ensures we use the
 +# correct URI taken from libvirt (configuration,
 +# environment variables, etc.), but one can always
 +# specify a different machine
 +if 'uri' not in case_params:
 +case_params['uri'] = None
   if case_ref_name != 'sleep':
   case_params['logger'] = case_logger

 
   This will create a default option named uri on the framework
 side like logger for all of testcases.
   If we want all of testcases to test a remote server that
 results in adding uri for each of them in test.conf
   that's not so good.
   Do you think if it is better to write the default uri in
 env.cfg, then parser it for all of testcases.
 

This will create the parameter only if it doesn't exist yet. I see no
problem with modifying env.cfg, adding defaulturi='xen:///' in there and
using $defaulturi in all the tests. This should still work. However if
you want to use one URI for all the tests, the default 'None' ensures
that the default libvirt URI will be used. Thus it's enough to modify
/etc/libvirt.conf or use LIBVIRT_DEFAULT_URI, etc. All of the options
should work with the code like this (defaulting to None if no uri is given).

Martin

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

Re: [libvirt] [test-API PATCH 5/6] Cleanup and fix of domain/define test

2012-03-27 Thread Martin Kletzander

On 03/26/2012 03:02 PM, Guannan Ren wrote:
 On 03/25/2012 01:42 AM, Martin Kletzander wrote:
 I added support for 'uri' parameter and moved some functions into
 util/Python/utils.py in order not to lose them from the code and keep
 them accessible for other tests.
 ---
   repos/domain/define.py |  132
 
   utils/Python/utils.py  |   45 -
   2 files changed, 65 insertions(+), 112 deletions(-)

 diff --git a/repos/domain/define.py b/repos/domain/define.py
 index 8f0095a..25630c5 100644
 --- a/repos/domain/define.py
 +++ b/repos/domain/define.py
 @@ -19,7 +19,7 @@
   __author__ = 'Alex Jia: a...@redhat.com'
   __date__ = 'Mon Jan 28, 2010'
   __version__ = '0.1.0'
 -__credits__ = 'Copyright (C) 2009 Red Hat, Inc.'
 +__credits__ = 'Copyright (C) 2009, 2012 Red Hat, Inc.'
   __all__ = ['usage', 'check_define_domain', 'define']

   import os
 @@ -63,9 +63,6 @@ def usage():
  macaddr
  ifacetype
  source
 -   target_machine
 -   username
 -   password
 '''

   def check_params(params):
 @@ -107,58 +104,17 @@ def ssh_keygen(logger):

   return 0

 -def ssh_tunnel(hostname, username, password, logger):
 -setup a tunnel to a give host
 -logger.info(setup ssh tunnel with host %s % hostname)
 -user_host = %s@%s % (username, hostname)
 -child = pexpect.spawn(SSH_COPY_ID, [ user_host])
 -while True:
 -index = child.expect(['yes\/no', 'password: ',
 -   pexpect.EOF,
 -   pexpect.TIMEOUT])
 -if index == 0:
 -child.sendline(yes)
 -elif index == 1:
 -child.sendline(password)
 -elif index == 2:
 -logger.debug(string.strip(child.before))
 -child.close()
 -return 0
 -elif index == 3:
 -logger.error(setup tunnel timeout)
 -logger.debug(string.strip(child.before))
 -child.close()
 -return 1
 -
 -return 0
 -
 -def check_define_domain(guestname, guesttype, target_machine,
 username, \
 -password, util, logger):
 -Check define domain result, if define domain is successful,
 -   guestname.xml will exist under /etc/libvirt/qemu/
 -   and can use virt-xml-validate tool to check the file validity
 +def check_define_domain(conn, guestname, logger):
 +Check define domain result. To make this work on all
 +hypervisors and with all configuration posibilities, use the
 +default way through libvirt to check if the guest was defined
   
 -if kvm in guesttype:
 -path = /etc/libvirt/qemu/%s.xml % guestname
 -elif xen in guesttype:
 -path = /etc/xen/%s % guestname
 -else:
 -logger.error(unknown guest type)
 -
 -if target_machine:
 -cmd = ls %s % path
 -ret, output = util.remote_exec_pexpect(target_machine,
 username, \
 -   password, cmd)
 -if ret:
 -logger.error(guest %s xml file doesn't exsits % guestname)
 -return False
 -else:
 -return True
 -else:
 -if os.access(path, os.R_OK):
 -return True
 -else:
 -return False
 +try:
 +conn.lookupByName(guestname + 'asdf')
 
  the testing code?
 

Yes, sorry =) I needed to test if this fails etc. and didn't remove it.

 +return True
 +except libvirtError, e:
 +logger.error(e.message())
 +return False
 
  It's better not to use libvirt API to check the result of one
 another API.
  We should use native approach to do the checking. So I insist
 on the original checking.
 

There was no lookupByName function, but I agree it's better to use the
same approach, so I'll add this method to the connection API.
One more question, whilst on this subject, though. I still didn't get
why we encapsulate libvirt API into one more class where we don't make
use of any persistence, inheritance nor any other OOP approach. It would
help me to understand so I don't make future mistakes.


   def define(params):
   Define a domain from xml
 @@ -169,41 +125,10 @@ def define(params):
   logger = params['logger']
   guestname = params['guestname']
   guesttype = params['guesttype']
 +uri = params['uri']
 
  If uri is not None, we need to get the IP or hostname of
 target machine from the uri
  Use that IP or hostname to perform ssh operation.
 

I dropped the part of the code that connected to the machine by ssh in
order to check for the domain to be created. It was very limited and in
case any other tests needs the IP of the target machine, there are 2
ways it can get it:
 1) have another parameter for the address

Re: [libvirt] [test-API PATCH 1/6] Slight cross-distribution support

2012-03-27 Thread Martin Kletzander

On 03/27/2012 10:32 AM, Christophe Fergeau wrote:
 On Mon, Mar 26, 2012 at 02:42:17PM +0200, Michal Novotny wrote:
 Fedora is using /etc/redhat-release file so you use the same handling
 for Fedora and RHEL then it's fine.
 
 I also have a /etc/fedora-release on my f17. systemd people are pushing for
 standardization around
 http://www.freedesktop.org/software/systemd/man/os-release.html
 
 Christophe

Oh, didn't know about this, but when I'm looking at it, it reminds me of
http://xkcd.com/927/ =) There is already /etc/issue /etc/issue.net
/etc/type-release etc.
I'm aware of these not having very standardized format, but I don't see
any standardized field in /etc/os-release that would tell me on what
type of distribution this particular one was built.
Maybe I'm the only one who wants to get this information using
/etc/*-release, but I want to get it without keeping list of rpm-based
distributions in the code and matching in them with the distribution name.

In this particular case (I needed to know wherther I can use 'rpm' or
not) it seems like the best to check for /etc/redhat-release. It is
there even when you have /etc/distribution-release in there.

Anyway, thanks for the info.

Martin

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

Re: [libvirt] [test-API PATCH 5/6] Cleanup and fix of domain/define test

2012-03-27 Thread Martin Kletzander

On 03/27/2012 10:59 AM, Guannan Ren wrote:
 On 03/27/2012 03:57 PM, Martin Kletzander wrote:
   It's better not to use libvirt API to check the result of one
 another API.
   We should use native approach to do the checking. So I insist
 on the original checking.


 There was no lookupByName function, but I agree it's better to use the
 same approach, so I'll add this method to the connection API.
 One more question, whilst on this subject, though. I still didn't get
 why we encapsulate libvirt API into one more class where we don't make
 use of any persistence, inheritance nor any other OOP approach. It would
 help me to understand so I don't make future mistakes.
 
 I think you don't need to add  lookupByName() in connectAPI.py
 The APIs is domain related, so we suggest to make use of it in
 domainAPI.py
 only. The ideas is that all of hypervisor related APIs go to
 connectAPI.py.
 The relationship between classes domain, network, storage,
 nodedev is
 loose. If we had a virtnetwork subclass, It would be better to make
 virtnetwork inherite
 network for better OOP.
 The encapsulated API files in lib directory is easy to manage
 and use. For example
 If we want to write a domain testcases, we probably don't need to
 import network
 and storage module in lib.


We both probably talk about something else, let me clarify.

My apologies first, because I misunderstood that you wanted to use the
native approach. I thought you meant implementing the function in
connectAPI whether you meant checking on the machine without using
libvirt. That said, there is no point in talking about lookupByName
implementation for now.

About the actual checking if the domain was created. Leaving it as it is
now, any misconfiguration will make this check fail (meaning the
configuration of sshd, libvirt, etc.). It will work for now as we are
the only ones using that right now (I guess), I'm just trying to think
ahead and I don't see that big problem with checking the domain being
created using libvirt again, but that's just my opinion.

About the re-implementation of the API, I was just looking into the
connectAPI class for now, but what I see there is only constructor that
saves the connection from libvirt and then for each method of libvirts
connection, there is connectAPI method that does the same, just using
different method names (and raising different exception). Unfortunately
these aren't even consistent. For example:

libvirts openAuth is encapsulated as openAuth
libvirts isEncrypted is encapsulated as isEncrypted

but

libvirts getCapabilities is encapsulated as get_caps
libvirts getHostname is encapsulated as get_host_name

and so on.

Don't get me wrong, I'm not trying to complain or anything, I'm just
trying to understand because for me it doesn't make any sense to use
this class.

I missed few other answers in my previous mail, so just to inform you
that I acknowledge them:

try ... except ...finally is new syntax since 2.5,
But we need to support 2.4, so should use
try:
 try:
 except:
finally
 

I didn't know that, good point, Python 2.4 is still used somewhere probably.


 -if target_machine:
 -REMOVE_SSH = ssh %s \rm -rf /root/.ssh/*\ % (target_machine)
 -logger.info(remove ssh key on remote machine)
 -status, ret = util.exec_cmd(REMOVE_SSH, shell=True)
 -if status:
 -logger.error(failed to remove ssh key)
 -
 -REMOVE_LOCAL_SSH = rm -rf /root/.ssh/*
 
   Never remove or change the local ssh directory like this.
   The autotest have ssh configuration file stored here.
 

This is a code that what was already in the test before, however I
probably copy-pasted it into the utils as it is. Better approach would
definitely be generating the keys somewhere, then pasting them into ssh
parameter '-i' and backing up the keys on the remote, while putting them
back after the test. However the _best_ option in this case is not using
keys (we have to use pexpect and connect there with password anyway) at all.

 
 -logger.info(remove local ssh key)
 -status, ret = util.exec_cmd(REMOVE_LOCAL_SSH, shell=True)
 -if status:
 -logger.error(failed to remove local ssh key)
 -
   if test_result:
   return 0
   else:
 diff --git a/utils/Python/utils.py b/utils/Python/utils.py
 index 55c1cb5..7382abb 100644
 --- a/utils/Python/utils.py
 +++ b/utils/Python/utils.py
 @@ -1,6 +1,6 @@
   #!/usr/bin/env python
   #
 -# libvirt-test-API is copyright 2010 Red Hat, Inc.
 +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
   #
   # libvirt-test-API is free software: you can redistribute it and/or
 modify it
   # under the terms of the GNU General Public License as published by
 @@ -433,3 +433,46 @@ class Utils(object):
   return 1

Re: [libvirt] [test-API PATCH 5/6] Cleanup and fix of domain/define test

2012-03-27 Thread Martin Kletzander

On 03/27/2012 12:35 PM, Guannan Ren wrote:
 On 03/27/2012 05:42 PM, Martin Kletzander wrote:
 On 03/27/2012 10:59 AM, Guannan Ren wrote:
 On 03/27/2012 03:57 PM, Martin Kletzander wrote:
It's better not to use libvirt API to check the result
 of one
 another API.
We should use native approach to do the checking. So I
 insist
 on the original checking.

 There was no lookupByName function, but I agree it's better to use the
 same approach, so I'll add this method to the connection API.
 One more question, whilst on this subject, though. I still didn't get
 why we encapsulate libvirt API into one more class where we don't make
 use of any persistence, inheritance nor any other OOP approach. It
 would
 help me to understand so I don't make future mistakes.
  I think you don't need to add  lookupByName() in connectAPI.py
  The APIs is domain related, so we suggest to make use of it in
 domainAPI.py
  only. The ideas is that all of hypervisor related APIs go to
 connectAPI.py.
  The relationship between classes domain, network, storage,
 nodedev is
  loose. If we had a virtnetwork subclass, It would be better to make
 virtnetwork inherite
  network for better OOP.
  The encapsulated API files in lib directory is easy to manage
 and use. For example
  If we want to write a domain testcases, we probably don't need to
 import network
  and storage module in lib.

 We both probably talk about something else, let me clarify.

 My apologies first, because I misunderstood that you wanted to use the
 native approach. I thought you meant implementing the function in
 connectAPI whether you meant checking on the machine without using
 libvirt. That said, there is no point in talking about lookupByName
 implementation for now.

 About the actual checking if the domain was created. Leaving it as it is
 now, any misconfiguration will make this check fail (meaning the
 configuration of sshd, libvirt, etc.). It will work for now as we are
 the only ones using that right now (I guess), I'm just trying to think
 ahead and I don't see that big problem with checking the domain being
 created using libvirt again, but that's just my opinion.

 About the re-implementation of the API, I was just looking into the
 connectAPI class for now, but what I see there is only constructor that
 saves the connection from libvirt and then for each method of libvirts
 connection, there is connectAPI method that does the same, just using
 different method names (and raising different exception). Unfortunately
 these aren't even consistent. For example:

 libvirts openAuth is encapsulated as openAuth
 libvirts isEncrypted is encapsulated as isEncrypted

 but

 libvirts getCapabilities is encapsulated as get_caps
 libvirts getHostname is encapsulated as get_host_name

 and so on.

 Don't get me wrong, I'm not trying to complain or anything, I'm just
 trying to understand because for me it doesn't make any sense to use
 this class.
 
   Ok, let me keep it temporarily, because it doesn't affect testing.
 After some practice,  it proves to be useless,  we can remove them
 anytime.
 Is that okay?
 

No problem, we can keep it for now, or not even temporarily, I just
wanted to know if I missed something or misunderstood.

I'll be sending v2 for some of the patches anyway, so we'll keep this as
is and we can discuss it with some other major changes (if there will be
any).

Have a nice day
Martin

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

Re: [libvirt] [PATCH test-API 1/2] lib: pass instance of ConnectAPI into other lib modules

2012-03-29 Thread Martin Kletzander

On 03/26/2012 07:18 PM, Guannan Ren wrote:
  This change make any instance of subclasses in libvirt.py
  invisible to testcases in order to catch libvirtError.
 
connectAPI.py
domainAPI.py
interfaceAPI.py
networkAPI.py
nodedevAPI.py
nwfilterAPI.py
secretAPI.py
snapshotAPI.py
storageAPI.py
streamAPI.py
 ---
  lib/connectAPI.py   |   21 +++--
  lib/domainAPI.py|2 +-
  lib/interfaceAPI.py |2 +-
  lib/networkAPI.py   |2 +-
  lib/nodedevAPI.py   |2 +-
  lib/nwfilterAPI.py  |2 +-
  lib/secretAPI.py|2 +-
  lib/snapshotAPI.py  |2 +-
  lib/storageAPI.py   |2 +-
  lib/streamAPI.py|5 +++--
  10 files changed, 22 insertions(+), 20 deletions(-)
 
 diff --git a/lib/connectAPI.py b/lib/connectAPI.py
 index 9f2b728..796df33 100644
 --- a/lib/connectAPI.py
 +++ b/lib/connectAPI.py
 @@ -39,36 +39,37 @@ append_path(result.group(0))
  import exception
  
  class ConnectAPI(object):
 -def __init__(self):
 +def __init__(self, uri):
 +self.uri = uri
  self.conn = None
  
 -def open(self, uri):
 +def open(self):
  try:
 -self.conn = libvirt.open(uri)
 -return self.conn
 +self.conn = libvirt.open(self.uri)
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)
  
 -def open_read_only(self, uri):
 +def open_read_only(self):
  try:
 -self.conn = libvirt.openReadOnly(uri)
 -return self.conn
 +self.conn = libvirt.openReadOnly(self.uri)
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)
  
 -def openAuth(self, uri, auth, flags = 0):
 +def openAuth(self, auth, flags = 0):
  try:
 -self.conn = libvirt.openAuth(uri, auth, flags)
 -return self.conn
 +self.conn = libvirt.openAuth(self.uri, auth, flags)
  except libvirt.libvirtError, e:
  message = e.get_error_message()
  code = e.get_error_code()
  raise exception.LibvirtAPI(message, code)
  
 +def get_conn(self):
 +return self.conn
 +
  def get_caps(self):
  try:
  caps = self.conn.getCapabilities()
 diff --git a/lib/domainAPI.py b/lib/domainAPI.py
 index 43565c2..e38acb6 100644
 --- a/lib/domainAPI.py
 +++ b/lib/domainAPI.py
 @@ -42,7 +42,7 @@ import exception
  
  class DomainAPI(object):
  def __init__(self, connection):
 -self.conn = connection
 +self.conn = connection.get_conn()
  
This is one option how to keep the object, however maybe we can make use
of the encapsulation everywhere, not just in the test cases, but this
would require rewriting a lot more code and is not needed at this point.
  def get_list(self):
  dom_list = []
 diff --git a/lib/interfaceAPI.py b/lib/interfaceAPI.py
 index 1abf861..2f4c13b 100644
 --- a/lib/interfaceAPI.py
 +++ b/lib/interfaceAPI.py
 @@ -44,7 +44,7 @@ VIR_INTERFACE_ERROR = -1
  
  class InterfaceAPI(object):
  def __init__(self, connection):
 -self.conn = connection
 +self.conn = connection.get_conn()
  
  def get_active_list(self):
  try:
 diff --git a/lib/networkAPI.py b/lib/networkAPI.py
 index d28f699..e0f0721 100644
 --- a/lib/networkAPI.py
 +++ b/lib/networkAPI.py
 @@ -39,7 +39,7 @@ import exception
  
  class NetworkAPI(object):
  def __init__(self, connection):
 -self.conn = connection
 +self.conn = connection.get_conn()
  
  def define(self, netxmldesc):
  try:
 diff --git a/lib/nodedevAPI.py b/lib/nodedevAPI.py
 index 64fc4b8..4ce3cf1 100644
 --- a/lib/nodedevAPI.py
 +++ b/lib/nodedevAPI.py
 @@ -40,7 +40,7 @@ import exception
  
  class NodedevAPI:
  def __init__(self, connection):
 -self.conn = connection
 +self.conn = connection.get_conn()
  
  def create(self, device_xml):
  try:
 diff --git a/lib/nwfilterAPI.py b/lib/nwfilterAPI.py
 index 9cf7050..4f5c58f 100644
 --- a/lib/nwfilterAPI.py
 +++ b/lib/nwfilterAPI.py
 @@ -39,7 +39,7 @@ import exception
  
  class nwfilterAPI(object):
  def __init__(self, connection):
 -self.conn = connection
 +self.conn = connection.get_conn()
  
  def get_list(self):
  try:
 diff --git a/lib/secretAPI.py b/lib/secretAPI.py
 index 4aac27f..149517c 100644
 --- a/lib/secretAPI.py
 +++ b/lib/secretAPI.py
 @@ -39,7 +39,7 @@ import exception
  
  class SecretAPI(object):
  def __init__(self, connection):
 -self.conn = connection
 +self.conn = connection.get_conn()
  
  def get_defined_list(self):
  try:
 diff --git a/lib/snapshotAPI.py b/lib/snapshotAPI.py

[libvirt] [test-API] RFC: Stabilization of libvirt-test-API

2012-03-29 Thread Martin Kletzander

Hi everyone,

following our minutes, I'd like to start a discussion on what should be
done with libvirt-test-API so we can say it's stable and usable.

I would like to stress out that everything mentioned here is just an
opinion and I don't mean to talk down to someone as it may have seemed
earlier.

I think we should get some ideas from everyone, mostly QE as they will
be (are) the ones using this the most (if I understood correctly), and
then I'll be happy to help getting the code to the agreed status. I was
thinking about this from the wrong way probably and changing the angle
from what I look at it (and knowing there is some deadline) makes me
think of few levels of changes, which when introduced, could speed up
the test development and code understandability.

So here are the things I would like to do definitely (the optional
things follow later on):
 - fix hard-coded options into real options (e.g. commit 65449e)
 - fix some env_* and util* code (functions duplicated with different
behavior)
 - fix or remove harmful and pointless code (at this point, when
creating domain on remote machine, be prepared for the test to fail with
any other user then root and with root, have backup of both local and
remote '/root/.ssh' directories as the contents will be erased!)
 - fix method names for the {connect,domain,etc.}API (get_host_name vs.
lookupByUUID etc.)

The optional things:
 - get rid of classes in lib and make just few utility functions
covering *only* the methods that do something else than call the same
method in underlying class from the libvirt module.
 - get rid of the new exception (I don't see any other difference than
in the name, which can make a difference in except: clause, but it's
converted everywhere)
 - be able to share variables between tests (connection object and
anything else)
 - introduce new config file for tests (.ini format, can be parsed by
ConfigParser, same as env.cfg, define variables used throughout the test
specifications
 - update the documentation
 - use some python code style (PEP-8?), make use of True/False, None
 - eliminate duplicated (and x-plicated) code (append_path in all the
files, etc.)

I have all of these figured out, so I'm willing to discuss all of them,
but in most cases changing it in the current code seems very
time-consumable to me.

Please, feel free to comment on any of these, add yours, discuss, shout
at me, etc. =)

Regards,
Martin

P.S.: I don't see any point in sending my patches until some of these
points are resolved as that could mean rewriting more code.

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

Re: [libvirt] [test-API] RFC: Stabilization of libvirt-test-API

2012-03-30 Thread Martin Kletzander

On 03/29/2012 03:42 PM, Osier Yang wrote:
 On 2012年03月29日 20:14, Martin Kletzander wrote:
 Hi everyone,

 following our minutes, I'd like to start a discussion on what should be
 done with libvirt-test-API so we can say it's stable and usable.

 I would like to stress out that everything mentioned here is just an
 opinion and I don't mean to talk down to someone as it may have seemed
 earlier.

 I think we should get some ideas from everyone, mostly QE as they will
 be (are) the ones using this the most (if I understood correctly), and
 then I'll be happy to help getting the code to the agreed status. I was
 thinking about this from the wrong way probably and changing the angle
 from what I look at it (and knowing there is some deadline) makes me
 think of few levels of changes, which when introduced, could speed up
 the test development and code understandability.

 So here are the things I would like to do definitely (the optional
 things follow later on):
   - fix hard-coded options into real options (e.g. commit 65449e)
 
 Absolutely we should either change/destroy it or generalize it
 as global config.
 
   - fix some env_* and util* code (functions duplicated with different
 behavior)
 
 This should be caused by mutilple persons worked on that, but lacked
 reviewing.
 
   - fix or remove harmful and pointless code (at this point, when
 creating domain on remote machine, be prepared for the test to fail with
 any other user then root and with root, have backup of both local and
 remote '/root/.ssh' directories as the contents will be erased!)
 
 So this means test-API only supports qemu:///system testing now,
 needs to be improved for qemu:///session too.
 
 Also I'd guess there are cases which only considers the QEMU/KVM
 driver testing. If so, we need to either generalize it or seperate
 it (in case of it are too specific to generalize), perhaps seperate
 directories for different drivers. But this should be the future
 plan, what we should do currently is try to generalize the tests
 as much as we could.
 
   - fix method names for the {connect,domain,etc.}API (get_host_name vs.
 lookupByUUID etc.)
 
 Yes, we need the consistent function/variable name, also consistent
 coding (including the comments on top of scripts) style.
 

 The optional things:
   - get rid of classes in lib and make just few utility functions
 covering *only* the methods that do something else than call the same
 method in underlying class from the libvirt module.
 
 Agreed, it looks to me many of the lib functions just simple pass
 parameter to the underlying libvirt-python API, that's just meaningless/
 useless.
 
   - get rid of the new exception (I don't see any other difference than
 in the name, which can make a difference in except: clause, but it's
 converted everywhere)
 
 Agreed. Just like the classes method in lib, it's feet of snake, ;)
 
 
   - be able to share variables between tests (connection object and
 anything else)
 
 Not sure what's your exact meaning, could you explain more?
 
   - introduce new config file for tests (.ini format, can be parsed by
 ConfigParser, same as env.cfg, define variables used throughout the test
 specifications
 
 Do you mean destroy current config parsing codes? if so, we need
 to rewrite (or much modification) codes of generator too. Any
 critical disadvantage you see in the current parsing/generator
 codes? I'd think the pricinple of current parsing (no generator)
 is right, though it might have bugs or disadvantages, we can
 improve/extend it.
 

To answer two of your questions at once, I'll show you an example of
what I had in mind (BTW: one of the current disadvantages is also the
fact that each indentation level _must_ be 4 spaces, otherwise you'll
get an error). Please be aware that this doesn't make almost any sense,
it just shows a couple of things I think could help everyone a lot.

file testcase.cfg:

# Local variables. You can use these variables only in this testcase
# file. This requires 1 more line of code in CofigParser.
[LocalVariables]
my_hyper = qemu
module = connections

[GlobalVariables]
# Could be named Defaults as these variables will be passed to all
# test in this testcase file...
uri = %(my_hyper)s:///system

# This section is not needed if the tests are named Test1.Connect and
# so on, but it is more readable a understandable for some readers
[Tests]
Test_1 = Connect
Test_2 = Disconnect

[Test.Connect]
Module = $(connections)s
Testcase = connect
# if not specified, this could default to Test.name.Params
Params = SomethingElse

[SomethingElse]
# ...unless they are overwritten with some others
uri = %(my_hyper)s:///session

[Test.Connect]
Module = $(connections)s
Testcase = disconnect
# No parameters here (none needed)

And then you will have two tests that look something like this (very
rough idea with lots of things I had in my mind, just to show how nice
it could look):

file tests/connections/connect.py:

def get_params(params):
# clean

Re: [libvirt] [test-API] RFC: Stabilization of libvirt-test-API

2012-03-30 Thread Martin Kletzander

On 03/29/2012 04:20 PM, Guannan Ren wrote:
 On 03/29/2012 08:14 PM, Martin Kletzander wrote:

   - fix hard-coded options into real options (e.g. commit 65449e)
   - fix some env_* and util* code (functions duplicated with different
 behavior)
   - fix or remove harmful and pointless code (at this point, when
 creating domain on remote machine, be prepared for the test to fail with
 any other user then root and with root, have backup of both local and
 remote '/root/.ssh' directories as the contents will be erased!)
   - fix method names for the {connect,domain,etc.}API (get_host_name vs.
 lookupByUUID etc.)

 The optional things:
   - get rid of classes in lib and make just few utility functions
 covering *only* the methods that do something else than call the same
 method in underlying class from the libvirt module.
   - get rid of the new exception (I don't see any other difference than
 in the name, which can make a difference in except: clause, but it's
 converted everywhere)
 
 the above should be easy to fix to cleanup.
 I can do it.
 
   - be able to share variables between tests (connection object and
 anything else)
 
  This belongs to new feature, we better consider it later.
 
No problem with that.
   - introduce new config file for tests (.ini format, can be parsed by
 ConfigParser, same as env.cfg, define variables used throughout the test
 specifications
 
Please list out some critical cause, why?
 
Look at the mail I sent to Osier, I think it could ease the look of it
pretty much.
   - update the documentation
 
   I can do it.
 
   - use some python code style (PEP-8?), make use of True/False, None
 
  we used pylint to review it, It is fine.(maybe could be better)
 
   - eliminate duplicated (and x-plicated) code (append_path in all the
 files, etc.)
 
   easy to do.
 
   Guannan Ren
 
 -- 
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

Thanks for your opinion, The only thing I'm thinking about is if it is
worth doing in the current code, but as Dave said, the main thing is the
deadline now and it's the end of April. After that, when I'm done with
school, I can put something together in my free time and present it,
maybe you'll like it, but there is no time for that right now.

Martin

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

[libvirt] [test-API PATCH] Added screenshot test

2012-04-02 Thread Martin Kletzander

This patch adds a test that obtains a screenshot of a domain and saves
it in a file.
---
 repos/domain/screenshot.py |   57 
 1 files changed, 57 insertions(+), 0 deletions(-)
 create mode 100644 repos/domain/screenshot.py

diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
new file mode 100644
index 000..9986cab
--- /dev/null
+++ b/repos/domain/screenshot.py
@@ -0,0 +1,57 @@
+#!/usr/bin/env python
+This test is used for creating a screenshot of a domain and saving
+   it in a file. The Screenshot format is hypervisor specific.
+
+   mandatory arguments: guestname
+screen
+filename
+
+
+import os
+import mimetypes
+
+import libvirt
+
+def check_params(params):
+Verify input parameters
+for key in ('guestname', 'screen', 'filename'):
+if key not in params:
+raise KeyError('Missing key %s required for screenshot test' % key)
+
+params['screen'] = int(params['screen'])
+params['filename'] = os.path.abspath(params['filename'])
+
+def saver(stream, data, file_):
+return file_.write(data)
+
+def screenshot(params):
+This method takes a screenshot of a running machine and saves
+it in a filename
+ret = 1
+try:
+logger = params['logger']
+
+check_params(params)
+
+conn = libvirt.open(params['uri'])
+dom = conn.lookupByName(params['guestname'])
+
+st = conn.newStream(0)
+mime = dom.screenshot(st, params['screen'], 0)
+
+ext = mimetypes.guess_extension(mime) or '.ppm'
+filename = params['filename'] + ext
+f = file(filename, 'w')
+
+logger.debug('Saving screenshot into %s' % filename)
+st.recvAll(saver, f)
+logger.debug('Mimetype of the file is %s' % mime)
+
+ret = st.finish()
+
+finally:
+# Some error occurred, cleanup
+if 'conn' in locals() and conn.isAlive():
+conn.close()
+
+return ret
-- 
1.7.8.5

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

Re: [libvirt] [test-API] RFC: Stabilization of libvirt-test-API

2012-04-02 Thread Martin Kletzander

On 04/01/2012 04:30 PM, Guannan Ren wrote:
 On 03/29/2012 10:20 PM, Guannan Ren wrote:
 On 03/29/2012 08:14 PM, Martin Kletzander wrote:
[...]
Hi Martin
 
  Could you have a review on the code.
  Anything require changes, you could sent patch based on
 git head
  or list them out, let me know.
 
Guannan Ren

Hi, I went through almost the whole patch and it looks good to me. The
tests are more readable and easier to write. I still haven't manage to
go through all the code, I'll keep you posted.

In the meantime, I created a test case Dave asked me for (thus the Cc).
It is a simple screenshot test (it creates a screenshot into a specified
file), could you have a look it, please?
It is here:
https://www.redhat.com/archives/libvir-list/2012-April/msg00038.html

Thanks and have a nice day,
Martin

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

Re: [libvirt] [test-API PATCH] Added screenshot test

2012-04-04 Thread Martin Kletzander

On 04/04/2012 07:13 AM, Guannan Ren wrote:
 
 
   Hi
 
  Currently, the testcase parser supports to cleanup testing
 environment
  after each testcase finished. we only need to add a flag
 command 'clean'
  after each testcase in testcase config, for example:
 
   ...
   domain:screenshot
   guestname
   rhel6
   filename
   /tmp/filescreen
   screen
   0
   clean
 
   domain: start
guestname
   ...
 
 
  the 'clean' flag will make the framework invoke the function
 named screenshot_clean
  in screenshot.py.  That means each testcase needs two mandatory
 functions, one is the
  main function, the other is main function name with '_clean'
 appended.
  So I send a patch for this testcase as an example.  According
 to the above testcase config
  the '/tmp/filescreen.ppm' file will be removed after running.
  There are some other flags, I need to update the documentation
 sooner:)
 
 ---
  repos/domain/screenshot.py |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)
 
 diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
 index 9986cab..eeda2b5 100644
 --- a/repos/domain/screenshot.py
 +++ b/repos/domain/screenshot.py
 @@ -55,3 +55,8 @@ def screenshot(params):
  conn.close()
 
  return ret
 +
 +def screenshot_clean(params):
 +clean testing environment
 +filename = params['filename']
 +os.system('rm -f %s.*' % filename)

The extension can be different every time, so we have to check that. I'd
prefer something like this:

diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
index 9986cab..c620085 100644
--- a/repos/domain/screenshot.py
+++ b/repos/domain/screenshot.py
@@ -39,8 +39,8 @@ def screenshot(params):
 st = conn.newStream(0)
 mime = dom.screenshot(st, params['screen'], 0)

-ext = mimetypes.guess_extension(mime) or '.ppm'
-filename = params['filename'] + ext
+params['ext'] = mimetypes.guess_extension(mime) or '.ppm'
+filename = params['filename'] + params['ext']
 f = file(filename, 'w')

 logger.debug('Saving screenshot into %s' % filename)
@@ -55,3 +55,8 @@ def screenshot(params):
 conn.close()

 return ret
+
+def screenshot_clean(params):
+clean testing environment
+filename = params['filename'] + params['ext']
+os.remove(filename)

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

Re: [libvirt] [test-API PATCH] Added screenshot test

2012-04-04 Thread Martin Kletzander

On 04/04/2012 01:23 PM, Guannan Ren wrote:
 On 04/04/2012 06:30 PM, Martin Kletzander wrote:
 On 04/04/2012 07:13 AM, Guannan Ren wrote:
 ---
   repos/domain/screenshot.py |5 +
   1 files changed, 5 insertions(+), 0 deletions(-)

 diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
 index 9986cab..eeda2b5 100644
 --- a/repos/domain/screenshot.py
 +++ b/repos/domain/screenshot.py
 @@ -55,3 +55,8 @@ def screenshot(params):
   conn.close()

   return ret
 +
 +def screenshot_clean(params):
 +clean testing environment
 +filename = params['filename']
 +os.system('rm -f %s.*' % filename)
 The extension can be different every time, so we have to check that. I'd
 prefer something like this:

 diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
 index 9986cab..c620085 100644
 --- a/repos/domain/screenshot.py
 +++ b/repos/domain/screenshot.py
 @@ -39,8 +39,8 @@ def screenshot(params):
   st = conn.newStream(0)
   mime = dom.screenshot(st, params['screen'], 0)

 -ext = mimetypes.guess_extension(mime) or '.ppm'
 -filename = params['filename'] + ext
 +params['ext'] = mimetypes.guess_extension(mime) or '.ppm'
 
 
  This modification on params couldn't be passed in
 screenshot_clean()
  The params to screenshot_clean() is the same as the
 screenshot() which
  is from testcase config file.
 
 

Darn :( That's exactly why I wanted the parameter passing between tests
:) What do you suggest? Should I save the extension into another file or
do we have any other option?

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

Re: [libvirt] [test-API PATCH] Added screenshot test

2012-04-04 Thread Martin Kletzander

On 04/04/2012 03:10 PM, Guannan Ren wrote:
 On 04/04/2012 07:53 PM, Martin Kletzander wrote:
 On 04/04/2012 01:23 PM, Guannan Ren wrote:
 On 04/04/2012 06:30 PM, Martin Kletzander wrote:
 On 04/04/2012 07:13 AM, Guannan Ren wrote:
 ---
repos/domain/screenshot.py |5 +
1 files changed, 5 insertions(+), 0 deletions(-)

 diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
 index 9986cab..eeda2b5 100644
 --- a/repos/domain/screenshot.py
 +++ b/repos/domain/screenshot.py
 @@ -55,3 +55,8 @@ def screenshot(params):
conn.close()

return ret
 +
 +def screenshot_clean(params):
 +clean testing environment
 +filename = params['filename']
 +os.system('rm -f %s.*' % filename)
 The extension can be different every time, so we have to check that.
 I'd
 prefer something like this:

 diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
 index 9986cab..c620085 100644
 --- a/repos/domain/screenshot.py
 +++ b/repos/domain/screenshot.py
 @@ -39,8 +39,8 @@ def screenshot(params):
st = conn.newStream(0)
mime = dom.screenshot(st, params['screen'], 0)

 -ext = mimetypes.guess_extension(mime) or '.ppm'
 -filename = params['filename'] + ext
 +params['ext'] = mimetypes.guess_extension(mime) or '.ppm'

   This modification on params couldn't be passed in
 screenshot_clean()
   The params to screenshot_clean() is the same as the
 screenshot() which
   is from testcase config file.


 Darn :( That's exactly why I wanted the parameter passing between tests
 :) What do you suggest? Should I save the extension into another file or
 do we have any other option?
 
   The idea to share variable is reasonable, we can create a
 shared_mod.py in
 root of test-API, then import it in all of testcases, The first
 testcase put the value in
 this module for sharing with other modules.  the connection object,
 domain
 network .etc could be shared in this way.
   Except these fixed share object. we could define 3 or 5 customized
 shared variable
 for use.   The contents of shared_mod.py should be like this:
 
   #shared_mod.py
   con = None
   domobj = None
   netojb = None
   stgobj = None
   ...
   defined_var1 = None
   defined_var2 = None
   defined_var3 = None
 
  Guannan Ren
 

I had an idea with expanding the parameters, tests could then share
whatever they want, but this would break current tests when not handled
correctly. I think your idea can be used now, just to fix these kind of
things and we can treat the expansion into more sophisticated code as a
major change and implement it after some release.

Martin

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

Re: [libvirt] [test-API PATCHv2 2/2] repo: Add test for mutualy exclusive console access

2012-04-06 Thread Martin Kletzander

On 04/05/2012 01:58 PM, Peter Krempa wrote:
 This test case checks if the console connection code works in a safe way
 that the connection don't get messed up.
 ---
 Diff to v1:
 -- removed semicolons at the end of statements (the C language left some 
 habits :D)
 -- added call to usage function (I thought it's called automaticaly. With 
 this this could
  be another good function to incorporate in 
 the class and 
  free the test-writer from re-implementing it 
 again and again)
 -- added cleanup function (same comment as above; note: there are some tests 
 that don't have this func.)
 -- refactored getting of optional parameter device
 -- added cleanup of stream objects (they'd got cleaned up automaticaly with 
 the disconnect, but
 this is safer and enables testing if the 
 stream was in a good state)
 
  repos/domain/console_mutex.py |  107 
 +
  1 files changed, 107 insertions(+), 0 deletions(-)
  create mode 100644 repos/domain/console_mutex.py
 
 diff --git a/repos/domain/console_mutex.py b/repos/domain/console_mutex.py
 new file mode 100644
 index 000..50730e0
 --- /dev/null
 +++ b/repos/domain/console_mutex.py
 @@ -0,0 +1,107 @@
 +#!/usr/bin/env python
 + A test case to test console mutual exclusivity
 +mandatory arguments: guestname
 +
 +import libvirt
 +from libvirt import libvirtError
 +from exception import TestError
 +
 +from utils.Python import utils

Sorry for rubbing in this after it's pushed, but why do you need utils?

 +
 +def usage(params):
 +Verify parameter dictionary
 +logger = params['logger']
 +keys = ['guestname']
 +for key in keys:
 +if key not in params:
 +logger.error(%s is required %key)
 +return 1
 +
 +def console_mutex(params):
 +Attach to console
 +usage(params);
 +logger = params['logger']
 +guest = params['guestname']
 +device = params.get('device', 'serial0')
 +
 +util = utils.Utils()

This is the only line using it and I believe util is not used
anywhere. Maybe it's copy-paste error from other tests where these were
leftover even though they are not needed anymore after the cleanup of
get_uri().

 +uri = params['uri']
 +
 +try:
 +logger.info(Connecting to hypervisor: '%s' % uri)
 +conn = libvirt.open(uri)
 +dom = conn.lookupByName(guest)
 +
 +if not dom.isActive():
 +raise TestError(Guest '%s' is not active % guest)
 +
 +logger.info(Creating stream object)
 +stream = conn.newStream(0)
 +
 +logger.info(Forcibly open console on domain)
 +dom.openConsole(device, stream, libvirt.VIR_DOMAIN_CONSOLE_FORCE)
 +
 +logger.info(Creating another stream object)
 +stream2 = conn.newStream(0)
 +
 +logger.info(Open safe console connection while an existing one is 
 open)
 +try:
 +dom.openConsole(device, stream2, libvirt.VIR_DOMAIN_CONSOLE_SAFE)
 +except libvirtError, e:
 +if e.get_error_code() == libvirt.VIR_ERR_OPERATION_FAILED:
 +logger.info(Opening failed - OK)
 +else:
 +raise e
 +else:
 +raise TestError(Opening of console succeeded although shoud 
 fail)
 +
 +logger.info(Abort the existing stream)
 +stream.abort()
 +
 +logger.info(Re-try connecting to the console)
 +dom.openConsole(device, stream2, libvirt.VIR_DOMAIN_CONSOLE_SAFE)
 +
 +logger.info(Re-try forcibly on already open console)
 +
 +logger.info(Creating stream object)
 +stream = conn.newStream(0)
 +
 +dom.openConsole(device, stream, libvirt.VIR_DOMAIN_CONSOLE_FORCE)
 +
 +logger.info(Clean up streams)
 +stream.finish()
 +
 +try:
 +stream2.finish()
 +except libvirtError, e:
 +if e.get_error_code() == libvirt.VIR_ERR_RPC and \
 +   e.get_error_domain() == libvirt.VIR_FROM_STREAMS:
 +logger.info(Stream was aborted successfuly)
 +else:
 +raise e
 +else:
 +raise TestError(stream2 should be aborted after forced console 
 connection)
 +
 +except libvirtError, e:
 +logger.error(Libvirt call failed:  + str(e))
 +ret = 1
 +
 +except TestError, e:
 +logger.error(Test failed:  + str(e))
 +ret = 1
 +
 +else:
 +logger.info(All tests succeeded)
 +ret = 0
 +
 +finally:
 +logger.info(Closing hypervisor connection)
 +conn.close()
 +
 +logger.info(Done)
 +
 +return ret
 +
 +def console_mutex_clean(params):
 +clean testing environment
 +pass

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

Re: [libvirt] [test-API PATCH 1/2] sharedmod: Add a new file for variable sharing in testcases

2012-04-10 Thread Martin Kletzander

On 04/06/2012 11:14 AM, Guannan Ren wrote:
 sharedmod.py: in root directory
 ---
  sharedmod.py |   13 +
  1 files changed, 13 insertions(+), 0 deletions(-)
  create mode 100644 sharedmod.py
 
 diff --git a/sharedmod.py b/sharedmod.py
 new file mode 100644
 index 000..f3de5a6
 --- /dev/null
 +++ b/sharedmod.py
 @@ -0,0 +1,13 @@
 +# This is a module for variable sharing across testcases during
 +# running. You have to import it in each of testcases which want
 +# to share data. The framwork have already set 'conn' for use in
 +# testcases.
 +
 +# connection object in libvirt.py
 +conn = None
 +
 +# shared variables for customized use in testcases
 +# Note: please set them to None at the end of sharing
 +defined_var1 = None
 +defined_var2 = None
 +defined_var3 = None

I see this could be little more error-prone. And maybe little more
variable. I know we can add a variable for every test making use of some
sharemod persistence, but that would mean there will be big mess very
early. I would probably just do something like this for example:

data = {}

And then in the test when you want to use shared variable, you can do:

Setting:
sharedmod.data['my_test_variable'] = 'test_value'

Checking:
if sharedmod.data.has_key('my_test_valiable'):
# The value is set

Getting:
sharemod.data.get('my_test_variable', 'test_variable_default_value')

But if the current suits you better, I think you can go with it as well.

Martin

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

Re: [libvirt] [test-API PATCH 1/4] filter: new class for filter or extract data

2012-04-12 Thread Martin Kletzander

On 04/11/2012 04:04 PM, Guannan Ren wrote:
 activityfilter.py
 ---
  activityfilter.py |   74 
 +
  1 files changed, 74 insertions(+), 0 deletions(-)
  create mode 100644 activityfilter.py
 
 diff --git a/activityfilter.py b/activityfilter.py
 new file mode 100644
 index 000..d99d690
 --- /dev/null
 +++ b/activityfilter.py
 @@ -0,0 +1,74 @@
 +#!/usr/bin/env python
 +#
 +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
 +#
 +# libvirt-test-API is free software: you can redistribute it and/or modify it
 +# under the terms of the GNU General Public License as published by
 +# the Free Software Foundation, either version 2 of the License, or
 +# (at your option) any later version. This program is distributed in
 +# the hope that it will be useful, but WITHOUT ANY WARRANTY; without
 +# even the implied warranties of TITLE, NON-INFRINGEMENT,
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 +#
 +# The GPL text is available in the file COPYING that accompanies this
 +# distribution and at http://www.gnu.org/licenses.
 +#
 +
 +
 +class Filter(object):
 +filter activity list to form various data list
 +def __init__(self, activities_list):
 +self.testcase_keys = []
 +for activity in activities_list:
 +for testcase in activity:
 +testcases_key = testcase.keys()
 +self.testcase_keys += testcases_key
 +
 +def unique_testcase_cleansuffix(self):
 +get a list of module:testcase from activities_list
 +   eliminate duplicate items, with 'module:testcase_clean'
 +
 +keylist_clean = self._keylist_cleanappended_without_sleep()
 +return list(set(keylist_clean))
 +
 +def unique_testcases(self):
 + get a list of module:testcase from activities_list
 +eliminate duplicate items
 +
 +keylist = self._keylist_without_sleep_clean()
 +return list(set(keylist))
 +
 +def _keylist_without_sleep_clean(self):
 + filter out 'clean' and 'sleep' flag
 +to generate a list of testcases
 +
 +keylist = []
 +for key in self.testcase_keys:
 +key = key.lower()
 +if key == 'clean' or key == 'sleep':
 +continue
 +
 +keylist.append(key)
 +
 +return keylist
 +
 +def _keylist_cleanappended_without_sleep(self):
 + remove 'sleep' flag, and append ':_clean' to
 +the previous testcase to form a new element
 +
 +keylist_clean = []
 +prev_casename = ''
 +
 +for key in self.testcase_keys:
 +key = key.lower()
 +if key == 'sleep':
 +continue
 +
 +if key == 'clean':
 +keylist_clean.append(prev_casename + :_clean)
 +continue
 +
 +prev_casename = key
 +keylist_clean.append(prev_casename)
 +
 +return keylist_clean

This looks ok, ACK.
I'm looking at the rest now.

Martin

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

Re: [libvirt] [test-API PATCH 2/4] cfgcheck: new class implement testcase config file checking

2012-04-12 Thread Martin Kletzander

On 04/11/2012 04:04 PM, Guannan Ren wrote:
 casecfgcheck.py
 ---
  casecfgcheck.py |   66 
 +++
  1 files changed, 66 insertions(+), 0 deletions(-)
  create mode 100644 casecfgcheck.py
 
 diff --git a/casecfgcheck.py b/casecfgcheck.py
 new file mode 100644
 index 000..3c4696d
 --- /dev/null
 +++ b/casecfgcheck.py
 @@ -0,0 +1,66 @@
 +#!/usr/bin/env python
 +#
 +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
 +#
 +# libvirt-test-API is free software: you can redistribute it and/or modify it
 +# under the terms of the GNU General Public License as published by
 +# the Free Software Foundation, either version 2 of the License, or
 +# (at your option) any later version. This program is distributed in
 +# the hope that it will be useful, but WITHOUT ANY WARRANTY; without
 +# even the implied warranties of TITLE, NON-INFRINGEMENT,
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 +#
 +# The GPL text is available in the file COPYING that accompanies this
 +# distribution and at http://www.gnu.org/licenses.
 +#
 +
 +import proxy
 +
 +class CaseCfgCheck(object):
 +validate the options in testcase config file
 +def __init__(self, unique_testcases, activities_list):
 +self.unique_testcases = unique_testcases
 +
 +# XXX to check the first testcase list in activities_list
 +self.activitie = activities_list[0]
 +
 +proxy_obj = proxy.Proxy(self.unique_testcases)
 +self.case_params = proxy_obj.get_params_variables()

typo:
s/activitie/activity/

plus I'm not sure if we can be sure that all the activities are the same
in this call (if it's just a multiplication of one array)? On other
places (like 'libvirt-test-api.py') we check for all members in the array.

 +
 +def check(self):
 +check options to each testcase in case config file
 +case_number = 0
 +error_flag = 0
 +passed_testcase = []
 +for testcase in self.activitie:

second typo or copy paste error :)
s/activitie/activity/

 +case_number += 1
 +if testcase in passed_testcase:
 +continue
 +
 +testcase_name = testcase.keys()[0]
 +actual_params = testcase.values()[0]

I don't quite understand what are you trying to achieve here. The order
of keys and values in python dict cannot be guaranteed if I'm not wrong.
And even if yes, that would mean you are checking just for the first
testcase, I guess. Or maybe I just don't get this right.

 +
 +required_params, optional_params = 
 self.case_params[testcase_name]
 +ret = self._check_params(required_params, optional_params, 
 actual_params)
 +if ret:
 +error_flag = 1
 +print the No.%s : %s\n % (case_number, testcase_name)
 +
 +passed_testcase.append(testcase)
 +
 +if error_flag:
 +return 1
 +return 0
 +
 +def _check_params(self, required_params, optional_params, actual_params):
 +for p in required_params:
 +if p not in actual_params.keys():
 +print Parameter %s is required % p
 +return 1
 +
 +for p in actual_params.keys():
 +if p not in required_params and p not in optional_params:
 +print Unknown parameter '%s' % p
 +return 1
 +
 +return 0

Martin

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

Re: [libvirt] [test-API PATCH 0/4]Add feature to check case file before running

2012-04-13 Thread Martin Kletzander

On 04/12/2012 07:03 PM, Guannan Ren wrote:
 On 04/12/2012 09:43 PM, Guannan Ren wrote:
 On 04/12/2012 07:53 PM, Peter Krempa wrote:
 I don't think pushing this series without a review was a good idea.
 You actualy broke all of the tests in the repos/ as you didn't do the
 modifications to the parameter checking algorithm in a way that
 didn't require modification of the tests, neither did you change the
 tests to cope with the new code. The result is now:

 exception.TestCaseError: 'required_params or optional_params not
 found in interface:destroy'

 or similar for every test case.

 Peter


 Yes, sorry about this.
 There is some new feature and cleanup work on my hand, I don't know
 the exact time to get review.
 The error is generated by framework.  the part job of framework is
 done.
 The cleanup work on testcase is ongoing,  I am sure that I will
 finish the work today.

 
 The cleanup is done and pushed.
 I only wan to send framework code here, the cleaning code in
 testcases is huge
 and mechanical, maybe nobody likes seeing it :)
 Sorry about the intact commit again.
 
 Guannan Ren

I was writing next email about this when the internet at my place (and
thus VPN) started disconnecting me.
I had a thought in mind how we can keep all the tests working without
any change. However if this is now fixed then the thought is not needed
anymore.
Next time if there is some major change like this, then I'd like to keep
it that way. In case we don't rewrite it from scratch of course :)

Martin

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

Re: [libvirt] [test-API PATCH] repo: Add test for console input and output operations

2012-04-13 Thread Martin Kletzander

On 04/13/2012 11:41 AM, Peter Krempa wrote:
 This test checks if the console input and output work as desired. The
 test takes as an argument a filename, whose contents are sent to the
 console. The optional parameter 'expect' can be set to a filename that
 should contain expected output from the guest and is compared with the
 actual output. The optional parameter 'output' can be set to a filename
 where the test script saves the output of the host (useful for initial
 test setup).
 
 This test requires that the guest machine runs code that handles input and
 output on the serial console (programs such as agetty or something like that).
 ---
  repos/domain/console_io.py |  123 
 
  1 files changed, 123 insertions(+), 0 deletions(-)
  create mode 100644 repos/domain/console_io.py
 
 diff --git a/repos/domain/console_io.py b/repos/domain/console_io.py
 new file mode 100644
 index 000..98fd5b6
 --- /dev/null
 +++ b/repos/domain/console_io.py
 @@ -0,0 +1,123 @@
 +#!/usr/bin/env python
 +# test console interactions
 +# This test sends contents of file 'input' to the guest's console
 +# and reads from the console the reply and compares it to 'expect' or
 +# writes the output to file 'output'
 +
 +import libvirt
 +import signal
 +import os
 +
 +from libvirt import libvirtError
 +from exception import TestError
 +
 +required_params = ('guestname',)
 +optional_params = ('device', 'timeout', 'input', 'output', 'expect',)
 +
 +def alarm_handler(signum, frame):
 +raise TestError(Timed out while waiting for console)
 +
 +def console_io(params):
 +Attach to console
 +logger = params['logger']
 +guest = params['guestname']
 +device = params.get('device', 'serial0')
 +infile = params.get('input', None)
 +outfile = params.get('output', None)
 +expect = params.get('expect', None)
 +timeout = params.get('timeout', 5)
 +
 +uri = params['uri']
 +
 +#store the old signal handler
 +oldhandler = signal.getsignal(signal.SIGALRM)
 +
 +try:
 +logger.info(Connecting to hypervisor: '%s' % uri)
 +conn = libvirt.open(uri)
 +dom = conn.lookupByName(guest)
 +if not dom.isActive():
 +raise TestError(Guest '%s' not active % guest)
 +
 +logger.info(Creating stream object)
 +stream = conn.newStream(0)
 +
 +logger.info(Open a new console connection)
 +dom.openConsole(device, stream, libvirt.VIR_DOMAIN_CONSOLE_FORCE)
 +
 +if infile != None:
 +try:
 +f = open(infile, 'r')
 +instr = f.read()
 +f.close()
 +except e:
 +raise TestError(Can't read input file '%s': %s % (infile, 
 str(e)))
 +
 +logger.info(Sending %d bytes of contents of file '%s' to 
 console '%s' % (len(instr), infile, device))
 +stream.send(instr)
 +
 +if expect != None or outfile != None:
 +logger.info(Recieving data from console device. Timeout %d 
 seconds. % timeout)
 +
 +# register a new signal handler
 +logger.info(Registering custom SIGALRM handler)
 +signal.signal(signal.SIGALRM, alarm_handler)
 +signal.alarm(timeout)
 +
 +reply = 
 +try:
 +while True:
 +recv = stream.recv(1024)
 +reply += recv
 +except TestError:
 +pass
 +
 +logger.info(Recieved %d bytes. % len(reply))
 +
 +if outfile != None:
 +try:
 +f = open(outfile, 'w')
 +f.write(reply)
 +f.close()
 +except e:
 +raise TestError(Can't write output to file '%s': %s % 
 (outfile, str(e)))
 +
 +if expect != None:
 +try:
 +f = open(expect, 'r')
 +expectstr = f.read()
 +f.close()
 +except Exception, e:
 +raise TestError(Can't read expected output file '%s': 
 '%s' % (expect, str(e)))
 +
 +if reply.startswith(expectstr):
 +logger.info(Recieved expected output from the host)
 +else:
 +raise TestError(Reply from the guest doesn't match with 
 expected reply)
 +
 +except libvirtError, e:
 +logger.error(Libvirt call failed:  + str(e))
 +ret = 1
 +
 +except TestError, e:
 +logger.error(Test failed:  + str(e))
 +ret = 1
 +
 +else:
 +logger.info(All tests succeeded)
 +ret = 0
 +
 +finally:
 +logger.info(Restoring signal handler)
 +signal.signal(signal.SIGALRM, oldhandler)
 +logger.info(Closing hypervisor connection)
 +try:
 +stream.abort()
 +except:
 +pass
 +conn.close()
 +
 +logger.info(Done)
 +
 +

Re: [libvirt] [test-API PATCH 2/2] parser: put the syntax checking to testcase line at end of flags checking

2012-04-13 Thread Martin Kletzander

On 04/13/2012 09:56 AM, Guannan Ren wrote:
 we use the re.match(.+:.+, tripped_casename) only check testcase
 name line, such as domain:start, we don't want it to check flag.
 placing it just after flags checking is right place.
 ---
  parser.py |5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/parser.py b/parser.py
 index 18f6ccd..e569d7f 100644
 --- a/parser.py
 +++ b/parser.py
 @@ -327,8 +327,6 @@ class CaseFileParser(object):
  
  tripped_caselist = casestring.strip().split()
  tripped_casename = tripped_caselist[0]
 -if not re.match(.+:.+, tripped_casename):
 -raise exception.CaseConfigfileError(casename line 
 format error!)
  
  if self.debug:
  self.debug_print(we begin to handle the case,
 @@ -416,6 +414,9 @@ class CaseFileParser(object):
  list.append(option_case)
  continue
  
 +if not re.match(.+:.+, tripped_casename):
 +raise exception.CaseConfigfileError(%s line format 
 error! % tripped_casename)
 +
  for caselist in list:
  newdict = {}
  newdict[tripped_casename] = {}

I think both of these are fine, ACK series.

Martin

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

Re: [libvirt] [test-API PATCHv2 1/2] sharemod: Add a new file for variable sharing in testcases

2012-04-13 Thread Martin Kletzander

On 04/13/2012 02:23 PM, Guannan Ren wrote:
 On 04/10/2012 09:55 PM, Guannan Ren wrote:
  sharedmod.py
 ---
   sharedmod.py |   16 
   1 files changed, 16 insertions(+), 0 deletions(-)
   create mode 100644 sharedmod.py

 diff --git a/sharedmod.py b/sharedmod.py
 new file mode 100644
 index 000..8af26d8
 --- /dev/null
 +++ b/sharedmod.py
 @@ -0,0 +1,16 @@
 +# This is a module for variable sharing across testcases during
 +# running. You have to import it in each of testcases which want
 +# to share data. The framwork have already set {'conn': connobj}
 +# in libvirtobj dictionary for use in testcases.
 +
 +# The libvirtobj dictionary is only set and used by framework
 +# in testcases you could use sharedmod.libvirtobj['conn'] to get
 +# the connection object in libvirt.py, you need not to close it,
 +# the framework do it.
 +libvirtobj = {}
 +
 +# shared variables for customized use in testcases
 +# set variable: sharedmod.data['my_test_variable'] = 'test_value'
 +# check the variable: sharedmod.data.has_key('my_test_variable')
 +# get the varialbe: sharedmod.data.get('my_test_variable',
 'test_variable_default_value')
 +data = {}
 
 If no more comments, I will push these two patches.
 
 Guannan Ren
 
 -- 
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

This one looks ok, I haven't had a chance to look at the second one yet.

Martin

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

[libvirt] [PATCH] domain:screenshot: fixed screen parameter

2012-04-13 Thread Martin Kletzander

The screen parameter must be an integer.
I also chnaged it to variable parameter with the reasonable default.
---
 repos/domain/screenshot.py |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
index d73e980..3e727a7 100644
--- a/repos/domain/screenshot.py
+++ b/repos/domain/screenshot.py
@@ -7,8 +7,8 @@ import mimetypes

 import libvirt

-required_params = ('guestname', 'screen', 'filename',)
-optional_params = ()
+required_params = ('guestname', 'filename',)
+optional_params = ('screen',)

 def saver(stream, data, file_):
 return file_.write(data)
@@ -24,7 +24,8 @@ def screenshot(params):
 dom = conn.lookupByName(params['guestname'])

 st = conn.newStream(0)
-mime = dom.screenshot(st, params['screen'], 0)
+screen = params.get('screen', 0)
+mime = dom.screenshot(st, int('screen'), 0)

 ext = mimetypes.guess_extension(mime) or '.ppm'
 filename = params['filename'] + ext
--
1.7.8.5

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

[libvirt] [PATCH] domain:screenshot: Added cleanup function

2012-04-13 Thread Martin Kletzander

Added cleanup function to the screeshot testcase. This makes use of
the new sharedmod module.
---

WARNING: don't push this before the patch with sharedmod is pushed in
the repo, otherwise this will not work. Thanks.

 repos/domain/screenshot.py |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
index 3e727a7..5a12c4b 100644
--- a/repos/domain/screenshot.py
+++ b/repos/domain/screenshot.py
@@ -6,6 +6,7 @@ import os
 import mimetypes

 import libvirt
+import sharedmod

 required_params = ('guestname', 'filename',)
 optional_params = ('screen',)
@@ -36,6 +37,7 @@ def screenshot(params):
 logger.debug('Mimetype of the file is %s' % mime)

 ret = st.finish()
+sharedmod.dict['screenshot_filename'] = filename

 finally:
 # Some error occurred, cleanup
@@ -43,3 +45,7 @@ def screenshot(params):
 conn.close()

 return ret
+
+def cleanup(params):
+if sharedmod.has_key('screenshot_filename'):
+os.remove(sharedmod['screenshot_filename'])
--
1.7.8.5

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

[libvirt] [PATCH v2] domain:screenshot: fixed screen parameter

2012-04-13 Thread Martin Kletzander

The screen parameter must be an integer.
I also chnaged it to variable parameter with the reasonable default.
---
v2:
 - fixed 'screen' to screen (stupid typo)

 repos/domain/screenshot.py |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
index d73e980..3c0f85b 100644
--- a/repos/domain/screenshot.py
+++ b/repos/domain/screenshot.py
@@ -7,8 +7,8 @@ import mimetypes

 import libvirt

-required_params = ('guestname', 'screen', 'filename',)
-optional_params = ()
+required_params = ('guestname', 'filename',)
+optional_params = ('screen',)

 def saver(stream, data, file_):
 return file_.write(data)
@@ -24,7 +24,8 @@ def screenshot(params):
 dom = conn.lookupByName(params['guestname'])

 st = conn.newStream(0)
-mime = dom.screenshot(st, params['screen'], 0)
+screen = params.get('screen', 0)
+mime = dom.screenshot(st, int(screen), 0)

 ext = mimetypes.guess_extension(mime) or '.ppm'
 filename = params['filename'] + ext
--
1.7.8.5

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

Re: [libvirt] [PATCH] domain:screenshot: Added cleanup function

2012-04-16 Thread Martin Kletzander

On 04/16/2012 04:43 AM, Osier Yang wrote:
 On 2012年04月13日 23:44, Martin Kletzander wrote:
 Added cleanup function to the screeshot testcase. This makes use of
 the new sharedmod module.
 ---

 WARNING: don't push this before the patch with sharedmod is pushed in
 the repo, otherwise this will not work. Thanks.

   repos/domain/screenshot.py |6 ++
   1 files changed, 6 insertions(+), 0 deletions(-)

 diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
 index 3e727a7..5a12c4b 100644
 --- a/repos/domain/screenshot.py
 +++ b/repos/domain/screenshot.py
 @@ -6,6 +6,7 @@ import os
   import mimetypes

   import libvirt
 +import sharedmod

   required_params = ('guestname', 'filename',)
   optional_params = ('screen',)
 @@ -36,6 +37,7 @@ def screenshot(params):
   logger.debug('Mimetype of the file is %s' % mime)

   ret = st.finish()
 +sharedmod.dict['screenshot_filename'] = filename
 
 Is it neccessary to set a shared variable in the same case?
 IIUC the purpose of sharemod.py, it's for variable sharing
 among different test cases. Though the case will work fine,
 but it's just redundant, and let's keep things simple.
 
 Osier
 
 -- 
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

We can share just the extension if you want. I can also change it to
share it in the module global variable. However, the parameter can be
used by other tests this way, just in case.

We talked about what came to this patch here a little bit:
https://www.redhat.com/archives/libvir-list/2012-April/msg00172.html

I'm willing to change it to whatever suits the others but I'd like to
have it asap so I can finish part of documentation based on this test.

Martin

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

Re: [libvirt] [test-API PATCH 2/2] proxy: import each testcase file only once, initialize proxy once

2012-04-16 Thread Martin Kletzander

On 04/16/2012 08:15 AM, Guannan Ren wrote:
 *libvirt-test-api.py: initialize proxy module only once
 *casecfgcheck.py: use proxy object rather than initialize it by
   itself
 *proxy.py: make get_func_call_dict more flexible
 ---
  casecfgcheck.py |5 +
  libvirt-test-api.py |   12 +---
  proxy.py|   22 ++
  3 files changed, 20 insertions(+), 19 deletions(-)
 
 diff --git a/casecfgcheck.py b/casecfgcheck.py
 index 40d7c6e..9a4f8e6 100644
 --- a/casecfgcheck.py
 +++ b/casecfgcheck.py
 @@ -18,13 +18,10 @@ import proxy
  
  class CaseCfgCheck(object):
  validate the options in testcase config file
 -def __init__(self, unique_testcases, activities_list):
 -self.unique_testcases = unique_testcases
 -
 +def __init__(self, proxy_obj, activities_list):
  # XXX to check the first testcase list in activities_list
  self.activity = activities_list[0]
  
 -proxy_obj = proxy.Proxy(self.unique_testcases)
  self.case_params = proxy_obj.get_params_variables()
  
  def check(self):
 diff --git a/libvirt-test-api.py b/libvirt-test-api.py
 index 385b52d..7b38aaa 100644
 --- a/libvirt-test-api.py
 +++ b/libvirt-test-api.py
 @@ -112,20 +112,18 @@ class Main(object):
  
  unique_testcases = filterobj.unique_testcases()
  
 +# __import__ TESTCASE.py once for duplicate testcase names
 +proxy_obj = proxy.Proxy(unique_testcases)
 +
  # check the options to each testcase in case config file
 -casechk = CaseCfgCheck(unique_testcases, activities_list)
 +casechk = CaseCfgCheck(proxy_obj, activities_list)
  if casechk.check():
  return 1
  
  # get a list of unique testcase
  # with 'clean' flag appended to its previous testcase
  unique_testcase_keys = filterobj.unique_testcase_cleansuffix()
 -
 -# call and initilize proxy component to
 -# get a list of reference of testcases
 -proxy_obj = proxy.Proxy(unique_testcase_keys)
 -
 -cases_func_ref_dict = proxy_obj.get_func_call_dict()
 +cases_func_ref_dict = 
 proxy_obj.get_func_call_dict(unique_testcase_keys)
  
  # create a null list, then, initilize generator to
  # get the callable testcase function
 diff --git a/proxy.py b/proxy.py
 index bc82a84..49a0420 100644
 --- a/proxy.py
 +++ b/proxy.py
 @@ -1,6 +1,6 @@
  #!/usr/bin/env python
  #
 -# libvirt-test-API is copyright 2010 Red Hat, Inc.
 +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software: you can redistribute it and/or modify it
  # under the terms of the GNU General Public License as published by
 @@ -37,12 +37,13 @@ class Proxy(object):
  casename = elements[1]
  
  casemod_ref = self.get_call_dict(module, casename)
 -self.testcase_ref_dict[testcase_name] = casemod_ref
 +modcase = module + ':' + casename
 +self.testcase_ref_dict[modcase] = casemod_ref
  
 -def get_func_call_dict(self):
 -Return running function reference dictionary 
 +def get_func_call_dict(self, unique_testcase_keys):
 +get reference to functions defined in testcase file 
  func_dict = {}
 -for testcase_name in self.testcases_names:
 +for testcase_name in unique_testcase_keys:
  # Get module, casename
  elements = testcase_name.split(':')
  module = elements[0]
 @@ -55,16 +56,21 @@ class Proxy(object):
  flag = elements[2]
  func = casename + flag
  
 -casemod_ref = self.testcase_ref_dict[testcase_name]
 +# use modcase key to get the reference to corresponding
 +# testcase module
 +modcase = module + ':' + casename
 +casemod_ref = self.testcase_ref_dict[modcase]
  var_func_names = dir(casemod_ref)
  
 -key = module + ':' + casename + ':' + func
 +key = modcase + ':' + func
 +# check if the expected function is present in
 +# the list of string name from dir()
  if func in var_func_names:
  func_ref = getattr(casemod_ref, func)
  func_dict[key] = func_ref
  else:
  raise exception.TestCaseError(function %s not found in %s 
 % \
 -  (func, testcase_name))
 +  (func, modcase))
  return func_dict
  
  def get_clearfunc_call_dict(self):

ACK both.

Martin

P.S.: This is very useful, I don't have to type it manually =)

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

Re: [libvirt] [test-API PATCHv2 1/2] sharemod: Add a new file for variable sharing in testcases

2012-04-16 Thread Martin Kletzander

On 04/16/2012 08:03 AM, Osier Yang wrote:
 s/sharing in/sharing among/, but given it's already
 pushed. let's live with it.
 
 On 2012年04月10日 21:38, Guannan Ren wrote:
  sharedmod.py
 ---
   sharedmod.py |   16 
   1 files changed, 16 insertions(+), 0 deletions(-)
   create mode 100644 sharedmod.py

 diff --git a/sharedmod.py b/sharedmod.py
 new file mode 100644
 index 000..8af26d8
 --- /dev/null
 +++ b/sharedmod.py
 @@ -0,0 +1,16 @@
 +# This is a module for variable sharing across testcases during
 +# running. You have to import it in each of testcases which want
 +# to share data. The framwork have already set {'conn': connobj}
 +# in libvirtobj dictionary for use in testcases.
 +
 +# The libvirtobj dictionary is only set and used by framework
 
 Any checking? I.e could testcase r/w it too?
 
 +# in testcases you could use sharedmod.libvirtobj['conn'] to get
 +# the connection object in libvirt.py, you need not to close it,
 
 IMHO need not should be should never. How about the
 follow up use of connection if it's already closed, reopen one
 again?
 
 +# the framework do it.
 
 snip
 The framwork have already set {'conn': connobj}
 +# in libvirtobj dictionary for use in testcases.
 +
 /snip
 
 The comment above should resides here.
 
 +libvirtobj = {}
 +
 +# shared variables for customized use in testcases
 +# set variable: sharedmod.data['my_test_variable'] = 'test_value'
 +# check the variable: sharedmod.data.has_key('my_test_variable')
 +# get the varialbe: sharedmod.data.get('my_test_variable',
 'test_variable_default_value')
 
 From my p.o.v, libvirtobj is a confused name, also data is
 not enough to tell what the meaning is. How about use
 framework_shares and case_shares instead?
 
 And implement get/set/has functions for both framework_shares
 and case_shares, but not access them directly. e.g.
 
 def case_shares_get(key):
 pass
 
 def case_shares_set(key, value):
 pass
 
 def case_shares_has(key)
 pass
 
 Even perhaps a class is good.
 
 Osier
 
 -- 
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

That's getting closer and closer to what I recommended as a redesign for
the whole test-API. Class would be better, tests shouldn't need to
import the mod every time, there should be checking etc. However I feel
the need to make this stable asap and after some release, we can go
ahead with some major redesigns (or at least that's how I understood our
talk about this with Dave few weeks ago).

Martin

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

Re: [libvirt] [PATCH] domain:screenshot: Added cleanup function

2012-04-16 Thread Martin Kletzander

On 04/16/2012 10:00 AM, Osier Yang wrote:
 On 2012年04月16日 14:52, Martin Kletzander wrote:
 On 04/16/2012 04:43 AM, Osier Yang wrote:
 On 2012年04月13日 23:44, Martin Kletzander wrote:
 Added cleanup function to the screeshot testcase. This makes use of
 the new sharedmod module.
 ---

 WARNING: don't push this before the patch with sharedmod is pushed in
 the repo, otherwise this will not work. Thanks.

repos/domain/screenshot.py |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)

 diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
 index 3e727a7..5a12c4b 100644
 --- a/repos/domain/screenshot.py
 +++ b/repos/domain/screenshot.py
 @@ -6,6 +6,7 @@ import os
import mimetypes

import libvirt
 +import sharedmod

required_params = ('guestname', 'filename',)
optional_params = ('screen',)
 @@ -36,6 +37,7 @@ def screenshot(params):
logger.debug('Mimetype of the file is %s' % mime)

ret = st.finish()
 +sharedmod.dict['screenshot_filename'] = filename

 Is it neccessary to set a shared variable in the same case?
 IIUC the purpose of sharemod.py, it's for variable sharing
 among different test cases. Though the case will work fine,
 but it's just redundant, and let's keep things simple.

 Osier

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

 We can share just the extension if you want. I can also change it to
 share it in the module global variable. However, the parameter can be
 used by other tests this way, just in case.
 
 I don't see any other test case will need the screenshot_filename
 now, if we have some ones in future, change it to shared var at
 that time then. The principle is not to put the parameter in shared
 module if it's no need.
 
 Osier

OK then, I'll save it in the test and send v2.

Martin

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

[libvirt] [libvirt-test-API PATCH] domain:screenshot: Added cleanup function

2012-04-16 Thread Martin Kletzander

---
v2:
 - removed sharedmod for persistence of the filename

 repos/domain/screenshot.py |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
index 82425f3..2761dc5 100644
--- a/repos/domain/screenshot.py
+++ b/repos/domain/screenshot.py
@@ -10,6 +10,8 @@ import libvirt
 required_params = ('guestname', 'filename',)
 optional_params = ('screen',)

+last_filename = None
+
 def saver(stream, data, file_):
 return file_.write(data)

@@ -27,7 +29,7 @@ def screenshot(params):
 mime = dom.screenshot(st, int(screen), 0)

 ext = mimetypes.guess_extension(mime) or '.ppm'
-filename = params['filename'] + ext
+last_filename = params['filename'] + ext
 f = file(filename, 'w')

 logger.debug('Saving screenshot into %s' % filename)
@@ -37,3 +39,7 @@ def screenshot(params):
 ret = st.finish()

 return ret
+
+def cleanup(params):
+if last_filename:
+os.remove(sharedmod['last_filename'])
--
1.7.8.5

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

Re: [libvirt] [libvirt-test-API PATCH] domain:screenshot: Added cleanup function

2012-04-16 Thread Martin Kletzander

On 04/16/2012 12:14 PM, Osier Yang wrote:
 On 2012年04月16日 17:32, Martin Kletzander wrote:
 ---
 v2:
   - removed sharedmod for persistence of the filename

   repos/domain/screenshot.py |8 +++-
   1 files changed, 7 insertions(+), 1 deletions(-)

 diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
 index 82425f3..2761dc5 100644
 --- a/repos/domain/screenshot.py
 +++ b/repos/domain/screenshot.py
 @@ -10,6 +10,8 @@ import libvirt
   required_params = ('guestname', 'filename',)
   optional_params = ('screen',)

 +last_filename = None
 +
   def saver(stream, data, file_):
   return file_.write(data)

 @@ -27,7 +29,7 @@ def screenshot(params):
   mime = dom.screenshot(st, int(screen), 0)

   ext = mimetypes.guess_extension(mime) or '.ppm'
 -filename = params['filename'] + ext
 +last_filename = params['filename'] + ext
   f = file(filename, 'w')

   logger.debug('Saving screenshot into %s' % filename)
 @@ -37,3 +39,7 @@ def screenshot(params):
   ret = st.finish()

   return ret
 +
 +def cleanup(params):
 +if last_filename:
 +os.remove(sharedmod['last_filename'])
 
 Shoud this be the following instead?
 
os.remove(last_filename)
 
 ACK with the nit fixed.
 
 Regards,
 Osier

Yes, of course, stupid error, sorry.
Fixed and pushed.

Thanks,
Martin

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

[libvirt] [libvirt-test-API PATCH 2/2] Documentation: Update to current information

2012-04-16 Thread Martin Kletzander

Update of all the things found to be outdated. The test described in
the chapter Writing a test case was changed to simpler one, with only
one source file included. It shows how to use the libvirt API (no need
for connectAPI etc. anymore) and mainly libvirt test API.
---
 .../en-US/Understanding_libvirt-test-API.xml   |6 -
 .../en-US/Using_libvirt-test-API.xml   |   26 +-
 .../en-US/Writing_a_test_case.xml  |  325 
 .../libvirt-test-API_Guide/en-US/extras/log.txt|   53 ++--
 4 files changed, 164 insertions(+), 246 deletions(-)

diff --git 
a/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
 
b/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
index 7fe1e97..446e9e2 100644
--- 
a/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
+++ 
b/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
@@ -186,12 +186,6 @@ repos
 /blockquote


-bridgehead renderas=sect3/lib/bridgehead
-blockquote
-  parafilename/lib/filename is a directory that contains the basic 
subroutines that call the API functions of libvirt bindings languages to form 
basic unit classes. The test case initiates the first action by calling these 
subroutines./para
-/blockquote
-
-
 bridgehead renderas=sect3/utils/bridgehead
 blockquote
   parafilename/utils/filename is a directory which contains various 
scripts to assist in creating and verifying test cases./para
diff --git 
a/docs/User_Guide/libvirt-test-API_Guide/en-US/Using_libvirt-test-API.xml 
b/docs/User_Guide/libvirt-test-API_Guide/en-US/Using_libvirt-test-API.xml
index 65653e3..7fa14f5 100644
--- a/docs/User_Guide/libvirt-test-API_Guide/en-US/Using_libvirt-test-API.xml
+++ b/docs/User_Guide/libvirt-test-API_Guide/en-US/Using_libvirt-test-API.xml
@@ -13,21 +13,25 @@
section id=Using_libvirt-test-API-Running_a_test
  titleRunning a test/title

- paraThe filenamemain.py/filename file, located in the root 
directory of the test tool, is the file that runs the test and performs other 
operations in libvirt-test-API./para
+ paraThe filenamelibvirt-test-api.py/filename file, located in 
the root directory of the test tool, is the file that runs the test and 
performs other operations in libvirt-test-API./para

 paraTo run a test, from the libvirt-test-API root directory enter:/para
 screen
-# python main.py
+# python libvirt-test-api.py
 /screen

 paraThe following switches can be used with the above command:/para
 example
-  titlepython main.py switches/title
+  titlepython libvirt-test-api.py options/title
 screen
--C, --configfile   Specify the configuration file to parse, default is 
'case.conf'.
--D, --delete-log   Delete a log item.
--H, --help Display the command usage.
--L, --xmlfile  Specify the name of the log file, default is 'log.xml'.
+-h, --help : Display usage information
+-c, --casefile: Specify configuration file
+-t, --template: Print testcase config file template
+-f, --logxml: Specify log file with type xml
+-l, --log-level: 0 or 1 currently
+-d, --delete-log: Delete log items
+-m, --merge: Merge two log xmlfiles
+-r, --rerun: Rerun one or more test
 /screen
 /example

@@ -53,25 +57,25 @@

 bridgehead renderas=sect3Deleting log information/bridgehead

-paraThe switch command-D, --delete-log/command in the 
commandpython main.py/command command can be used to delete a test item, a 
test run, or all test runs in a log file./para
+paraThe switch command-d, --delete-log/command in the 
commandpython libvirt-test-api.py/command command can be used to delete a 
test item, a test run, or all test runs in a log file./para
 paraFor example:/para
 itemizedlist
   listitem
 paraTo delete the test item (testid) in the test run 
(testrunid):/para
 screen
-# python main.py -D log.xml testrunid testid
+# python libvirt-test-api.py -d log.xml testrunid testid
 /screen
   /listitem
   listitem
 paraTo delete the test run (testrunid):/para
 screen
-# python main.py -D log.xml testrunid
+# python libvirt-test-api.py -d log.xml testrunid
 /screen
   /listitem
   listitem
 paraTo delete all test runs:/para
 screen
-# python main.py -D log.xml all
+# python libvirt-test-api.py -d log.xml all
 /screen
   /listitem
 /itemizedlist
diff --git 
a/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml 
b/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml
index 3dd714e..49ad0b5 100644
--- a/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml
+++ b/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml
@@ -7,7 +7,7 @@

paraTest cases form the foundation of the test tool. They are the 
building blocks from which test runs can be created. Test cases are written as 
Python scripts and define the

[libvirt] [libvirt-test-API PATCH 1/2] domain:screenshot: fixes and cleanup

2012-04-16 Thread Martin Kletzander

After the conn object was transferred into sharedmod, its import was missing.
I also cleaned up and commented few lines in order to use them for
documentation purposes.
---
 repos/domain/screenshot.py |   45 ++-
 1 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/repos/domain/screenshot.py b/repos/domain/screenshot.py
index eb5d0e2..49028fe 100644
--- a/repos/domain/screenshot.py
+++ b/repos/domain/screenshot.py
@@ -4,42 +4,67 @@

 import os
 import mimetypes
-
 import libvirt

+# sharedmod is needed in order to get the shared connection object
+import sharedmod
+
+# These variables are mandatory for running the test
+# The test won't be run unless both guestname and filename are specified.
 required_params = ('guestname', 'filename',)
 optional_params = ('screen',)

-last_filename = None
+# In this filename we store the last filename that has a screenshot
+# saved inside in order to be able to clean it after
+filename = None

+# This is just a helper function that will be used later.
 def saver(stream, data, file_):
 return file_.write(data)

+# This is the main test
 def screenshot(params):
 This method takes a screenshot of a running machine and saves
 it in a filename
-ret = 1
+
+# Shared logger to be used for messages output
 logger = params['logger']

+# Shared connection object to be used for messages output
 conn = sharedmod.libvirtobj['conn']
-dom = conn.lookupByName(params['guestname'])
+guestname = params['guestname']
+
+logger.info('Looking for domain %s' % guestname)
+dom = conn.lookupByName(guestname)
+logger.debug('Domain %s found' % guestname)

 st = conn.newStream(0)
 screen = params.get('screen', 0)
+logger.info('Taking a screenshot')
 mime = dom.screenshot(st, int(screen), 0)
+logger.debug('Screenshot taken')

+# As mentioned before, the screenshot format is
+# hypervisor-specific
 ext = mimetypes.guess_extension(mime) or '.ppm'
-last_filename = params['filename'] + ext
+filename = params['filename'] + ext
+
+# Here we create a file object used later
 f = file(filename, 'w')

-logger.debug('Saving screenshot into %s' % filename)
+logger.info('Saving screenshot into %s' % filename)
+logger.info('Mimetype of the file is %s' % mime)
+# Here is the use of the saver function
 st.recvAll(saver, f)
-logger.debug('Mimetype of the file is %s' % mime)
-
-ret = st.finish()
+logger.debug('Screenshot saved')

-return ret
+# The test fails (return is different from 0) if the stream is not
+# properly ended
+return st.finish()

+# This cleanup function is called if specified by the configuration
+# file. In this file we remove the saved screenshot if the filename is
+# known.
 def cleanup(params):
 if last_filename:
 os.remove(last_filename)
--
1.7.8.5

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

[libvirt] [libvirt-test-API PATCH 0/2] Documentation update

2012-04-16 Thread Martin Kletzander

I updated part of the documentation that was changed in last few
weeks. Screenshot test was updated again a little bit to suit the
needs of a documentation and replaced the source codes of previous two
tests mentioned in the documentation as it describe just what is
needed and has no overhead on the reader.

I'm not sure how the numbering goes in this case, but it can be
updated later or I can send a v2 in case it needs to be fixed right
away.

Martin Kletzander (2):
  domain:screenshot: fixes and cleanup
  Documentation: Update to current information

 .../en-US/Understanding_libvirt-test-API.xml   |6 -
 .../en-US/Using_libvirt-test-API.xml   |   26 +-
 .../en-US/Writing_a_test_case.xml  |  325 
 .../libvirt-test-API_Guide/en-US/extras/log.txt|   53 ++--
 repos/domain/screenshot.py |   45 ++-
 5 files changed, 199 insertions(+), 256 deletions(-)

--
1.7.8.5

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

Re: [libvirt] [test-API 04/14] Move directory dist to src

2012-04-19 Thread Martin Kletzander

On 04/19/2012 11:38 AM, Guannan Ren wrote:
 On 04/19/2012 10:25 AM, Osier Yang wrote:
 env_inspect.py is part of the framework.
 ---
   {dist =  src/dist}/__init__.py   |0
   {dist =  src/dist}/redhat/__init__.py|0
   {dist =  src/dist}/redhat/env_inspect.py |0
   src/generator.py |4 ++--
   4 files changed, 2 insertions(+), 2 deletions(-)
   rename {dist =  src/dist}/__init__.py (100%)
   rename {dist =  src/dist}/redhat/__init__.py (100%)
   rename {dist =  src/dist}/redhat/env_inspect.py (100%)

 diff --git a/dist/__init__.py b/src/dist/__init__.py
 similarity index 100%
 rename from dist/__init__.py
 rename to src/dist/__init__.py
 diff --git a/dist/redhat/__init__.py b/src/dist/redhat/__init__.py
 similarity index 100%
 rename from dist/redhat/__init__.py
 rename to src/dist/redhat/__init__.py
 diff --git a/dist/redhat/env_inspect.py b/src/dist/redhat/env_inspect.py
 similarity index 100%
 rename from dist/redhat/env_inspect.py
 rename to src/dist/redhat/env_inspect.py
 diff --git a/src/generator.py b/src/generator.py
 index e3bc344..26e1553 100644
 --- a/src/generator.py
 +++ b/src/generator.py
 @@ -32,9 +32,9 @@ from utils import env_parser
   # Import of distribution-specific code.  If this is needed somewhere
   # else in the future, please don't copy-paste this, but create some
   # sensible distribution-specific package
 -for dist in os.listdir('dist'):
 +for dist in os.listdir('src/dist'):
   if os.path.exists('/etc/%s-release' % dist):
 -exec('from dist.%s import env_inspect' % dist)
 +exec('from src.dist.%s import env_inspect' % dist)
   break

   class FuncGen(object):
 
 maybe the dist folder should be removed.
 I rewrote the env_inspect, it is more potable than checking the
 rpm package.
 so, the dist/redhat is not necessary strongly
 
 ACK
 Guannan Ren
 

The new env_inspect is written universally, so it can be moved back, yes.

Martin

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

Re: [libvirt] [test-API 01/17] Destroy src/dist and move env_inspect.py back

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 As env_inspect.py is rewrote, and has good portability now.
 ---
  src/{dist/redhat = }/env_inspect.py |0
  src/generator.py |9 +
  2 files changed, 1 insertions(+), 8 deletions(-)
  delete mode 100644 src/dist/__init__.py
  delete mode 100644 src/dist/redhat/__init__.py
  rename src/{dist/redhat = }/env_inspect.py (100%)
 
 diff --git a/src/dist/__init__.py b/src/dist/__init__.py
 deleted file mode 100644
 index e69de29..000
 diff --git a/src/dist/redhat/__init__.py b/src/dist/redhat/__init__.py
 deleted file mode 100644
 index e69de29..000
 diff --git a/src/dist/redhat/env_inspect.py b/src/env_inspect.py
 similarity index 100%
 rename from src/dist/redhat/env_inspect.py
 rename to src/env_inspect.py
 diff --git a/src/generator.py b/src/generator.py
 index 4247fda..fc6cec3 100644
 --- a/src/generator.py
 +++ b/src/generator.py
 @@ -26,17 +26,10 @@ import traceback
  
  from src import mapper
  from src import env_parser
 +from src import env_inspect
  from utils import log
  from utils import format
  
 -# Import of distribution-specific code.  If this is needed somewhere
 -# else in the future, please don't copy-paste this, but create some
 -# sensible distribution-specific package
 -for dist in os.listdir('src/dist'):
 -if os.path.exists('/etc/%s-release' % dist):
 -exec('from src.dist.%s import env_inspect' % dist)
 -break
 -
  class FuncGen(object):
   To generate a callable testcase
  def __init__(self, cases_func_ref_dict,

ACK, thanks for clearing it up ;-)

Martin

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

Re: [libvirt] [test-API 02/17] Substitute guest_names with domain_names

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 Same as commit af470a72, let's use the same TERM 'domain' instead
 of 'guest' in test-API, this patch just substitute guest_names
 with domain_names, there will be follow up patches to change
 others, because it will be really large patch if do all the
 changing together.
 ---
  repos/domain/cpu_affinity.py  |6 +++---
  repos/domain/cpu_topology.py  |4 ++--
  repos/domain/destroy.py   |6 +++---
  repos/domain/domain_blkinfo.py|8 
  repos/domain/domain_id.py |6 +++---
  repos/domain/domain_uuid.py   |8 
  repos/domain/domblkinfo.py|8 
  repos/domain/eventhandler.py  |6 +++---
  repos/domain/install_linux_cdrom.py   |6 +++---
  repos/domain/install_linux_net.py |6 +++---
  repos/domain/install_windows_cdrom.py |6 +++---
  repos/domain/migrate.py   |   12 ++--
  repos/domain/ownership_test.py|6 +++---
  repos/libvirtd/qemu_hang.py   |6 +++---
  repos/libvirtd/restart.py |6 +++---
  repos/snapshot/delete.py  |4 ++--
  repos/snapshot/file_flag.py   |6 +++---
  repos/snapshot/flag_check.py  |6 +++---
  repos/snapshot/internal_create.py |4 ++--
  repos/snapshot/revert.py  |4 ++--
  20 files changed, 62 insertions(+), 62 deletions(-)
 
 diff --git a/repos/domain/cpu_affinity.py b/repos/domain/cpu_affinity.py
 index afc0f9b..fc99664 100644
 --- a/repos/domain/cpu_affinity.py
 +++ b/repos/domain/cpu_affinity.py
 @@ -206,13 +206,13 @@ def cpu_affinity(params):
  hypervisor = uri.split(':')[0]
  
  # Get cpu affinity
 -guest_names = []
 +domain_names = []
  ids = conn.listDomainsID()
  for id in ids:
  obj = conn.lookupByID(id)
 -guest_names.append(obj.name())
 +domain_names.append(obj.name())
  
 -if domain_name not in guest_names:
 +if domain_name not in domain_names:

This has nothing to do with this particular patch, but it's more like a
hint for future refactoring.
Why do we lookup all the IDs, then get their names and then check if the
name is in the list, then do lookup by name? Not commenting that all
that is commented with Get cpu affinity? =)
My opinion is we could do lookup by name in try-except block or even
without it (the error usually means failed test anyway).

  logger.error(guest %s doesn't exist or not be running. %
domain_name)
  return 1
 diff --git a/repos/domain/cpu_topology.py b/repos/domain/cpu_topology.py
 index 5dbe27b..c3cc553 100644
 --- a/repos/domain/cpu_topology.py
 +++ b/repos/domain/cpu_topology.py
 @@ -23,9 +23,9 @@ optional_params = ()
  
  def check_domain_running(conn, guestname, logger):
  check if the domain exists
 -defined_guest_names = conn.listDefinedDomains()
 +defined_domain_names = conn.listDefinedDomains()
  
 -if guestname not in defined_guest_names:
 +if guestname not in defined_domain_names:
  logger.error(%s doesn't exist or still in running % guestname)
  return 1
  else:
 diff --git a/repos/domain/destroy.py b/repos/domain/destroy.py
 index 89de3e2..6359274 100644
 --- a/repos/domain/destroy.py
 +++ b/repos/domain/destroy.py
 @@ -39,13 +39,13 @@ def destroy(params):
  conn = sharedmod.libvirtobj['conn']
  
  # Get running domain by name
 -guest_names = []
 +domain_names = []
  ids = conn.listDomainsID()
  for id in ids:
  obj = conn.lookupByID(id)
 -guest_names.append(obj.name())
 +domain_names.append(obj.name())
  
 -if guestname not in guest_names:
 +if guestname not in domain_names:
  logger.error(guest %s doesn't exist or isn't running. % guestname)
  return 1
  
 diff --git a/repos/domain/domain_blkinfo.py b/repos/domain/domain_blkinfo.py
 index 9aaecb2..ec7f14c 100644
 --- a/repos/domain/domain_blkinfo.py
 +++ b/repos/domain/domain_blkinfo.py
 @@ -30,15 +30,15 @@ def get_output(command, logger):
  
  def check_domain_exists(conn, guestname, logger):
   check if the domain exists, may or may not be active 
 -guest_names = []
 +domain_names = []
  ids = conn.listDomainsID()
  for id in ids:
  obj = conn.lookupByID(id)
 -guest_names.append(obj.name())
 +domain_names.append(obj.name())
  
 -guest_names += conn.listDefinedDomains()
 +domain_names += conn.listDefinedDomains()
  
 -if guestname not in guest_names:
 +if guestname not in domain_names:
  logger.error(%s doesn't exist % guestname)
  return False
  else:
 diff --git a/repos/domain/domain_id.py b/repos/domain/domain_id.py
 index ff246ad..384bcf3 100644
 --- a/repos/domain/domain_id.py
 +++ b/repos/domain/domain_id.py
 @@ -27,13 +27,13 @@ def get_output(logger, command):
  
  def check_domain_exists(conn,

Re: [libvirt] [test-API 03/17] Rename src/env_parser.py as src/global_parser.py

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 And:
 
 % for i in $(grep 'envparser' * -r | awk -F':' '{print $1}' | uniq); do \
 sed -i -e 's/envparser/global_parser/g' $i; \
   done
 
 % for i in $(grep 'Envparser' * -r | awk -F':' '{print $1}' | uniq); do \
 sed -i -e 's/Envparser/GlobalParser/g' $i; \
   done
 
 % for i in $(grep 'env_parser' * -r | awk -F':' '{print $1}' | uniq); do \
 sed -i -e 's/envparser/GlobalParser/g' $i; \
   done
 
 Typo fixes:
 
 % for i in $(grep 'Envpaser' * -r | awk -F':' '{print $1}' | uniq); do \
 sed -i -e 's/Envpaser/GlobalParser/g' $i; \
   done
 ---
  .../en-US/Understanding_libvirt-test-API.xml   |2 +-
  .../en-US/Writing_a_test_case.xml  |   16 
  repos/domain/install_image.py  |6 +++---
  repos/domain/install_linux_cdrom.py|8 
  repos/domain/install_linux_check.py|6 +++---
  repos/domain/install_linux_net.py  |   12 ++--
  repos/domain/install_windows_cdrom.py  |   12 ++--
  .../multiple_thread_block_on_domain_create.py  |8 
  src/env_inspect.py |   14 +++---
  src/generator.py   |4 ++--
  src/{env_parser.py = global_parser.py}|4 ++--
  src/parser.py  |4 ++--
  12 files changed, 48 insertions(+), 48 deletions(-)
  rename src/{env_parser.py = global_parser.py} (97%)
 
 diff --git 
 a/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
  
 b/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
 index 88c1b76..a563953 100644
 --- 
 a/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
 +++ 
 b/docs/User_Guide/libvirt-test-API_Guide/en-US/Understanding_libvirt-test-API.xml
 @@ -206,7 +206,7 @@ repos
paraRandom MAC address generator. Useful when installing a guest 
 machine./para
  /listitem
  listitem
 -  parafilenameenv_parser.py/filename is the parser component 
 of the environment INI file./para
 +  parafilenameglobal_parser.py/filename is the parser 
 component of the environment INI file./para
  /listitem
  listitem
parafilenamecheck.py/filename verifies if a hypervisor is 
 running./para
 diff --git 
 a/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml 
 b/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml
 index 2a74518..b36c660 100644
 --- a/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml
 +++ b/docs/User_Guide/libvirt-test-API_Guide/en-US/Writing_a_test_case.xml
 @@ -141,7 +141,7 @@ sys.path.append(rootdir)
  import exception
  from lib import connectAPI
  from lib import storageAPI
 -from utils import env_parser
 +from utils import global_parser
  from utils import xml_builder
  
  envfile = 'env.ini'
 @@ -149,9 +149,9 @@ envfile = 'env.ini'
  def initialize_storage(dict):
  logger = dict['logger']
  dict['hypertype'] = 'xen'
 -envparser = env_parser.Envpaser(envfile)
 -dict['sourcename'] = envparser.get_value('storage', 'sourcename')
 -dict['sourcepath'] = envparser.get_value('storage', 'sourcepath')
 +global_parser = global_parser.GlobalParser(envfile)

This is a little misleading, before this line, global_parser is a class,
but after this line global_parser is a generated object of this class os
something? I'm not sure this improves readability. As I see it is done
in almost the whole patch. Do we really want it this way?
How about at least:

import utils
global_parser = utils.global_parser.GlobalParser(envfile)

We don't have to have from module import class_or_whatever everywhere =)

 +dict['sourcename'] = global_parser.get_value('storage', 'sourcename')
 +dict['sourcepath'] = global_parser.get_value('storage', 'sourcepath')
  
  logger.info('prepare create storage pool')
  xmlobj = xml_builder.XmlBuilder()
 @@ -200,7 +200,7 @@ sys.path.append(rootdir)
  import exception
  from lib import connectAPI
  from lib import domainAPI
 -from utils import env_parser
 +from utils import global_parser
  from utils import xml_builder
  
  envfile = 'env.ini'
 @@ -237,9 +237,9 @@ def install_guest(dict):
  dict['bootcd'] = '/tmp/%s/custom.iso' %gname
  
  logger.info('get system environment information')
 -envparser = env_parser.Envpaser(envfile)
 -url = envparser.get_value(guest, gname + src)
 -dict['kickstart'] = envparser.get_value(guest, gname + ks)
 +global_parser = global_parser.GlobalParser(envfile)
 +url = global_parser.get_value(guest, gname + src)
 +dict['kickstart'] = global_parser.get_value(guest, gname + ks)
  logger.debug('install source: \n%s' %url)
  logger.debug('kisckstart file: \n%s' %dict['kickstart'])
  
snip
 diff --git a/src/parser.py

Re: [libvirt] [test-API 05/17] Substitue guestobj with domobj

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 ---
  repos/domain/update_devflag.py |   10 +-
  .../multiple_thread_block_on_domain_create.py  |4 ++--
  2 files changed, 7 insertions(+), 7 deletions(-)
 
 diff --git a/repos/domain/update_devflag.py b/repos/domain/update_devflag.py
 index 6c75fd6..48efd72 100644
 --- a/repos/domain/update_devflag.py
 +++ b/repos/domain/update_devflag.py
 @@ -189,17 +189,17 @@ def update_devflag(params):
  return 1
  
  guestxml = domobj.XMLDesc(0)
 -guestobj = minidom.parseString(guestxml)
 +domobj = minidom.parseString(guestxml)
  
  # Generat device XML for original use
  origxmlobj = xml_builder.XmlBuilder()
  
  if devtype == 'cdrom':
 -origxmlobj.add_cdrom(xmlargs, guestobj)
 -guestxml = origxmlobj.build_domain(guestobj)
 +origxmlobj.add_cdrom(xmlargs, domobj)
 +guestxml = origxmlobj.build_domain(domobj)
  elif devtype == 'floppy':
 -origxmlobj.add_floppy(xmlargs, guestobj)
 -guestxml = origxmlobj.build_domain(guestobj)
 +origxmlobj.add_floppy(xmlargs, domobj)
 +guestxml = origxmlobj.build_domain(domobj)
  
  try:
  domobj.undefine()
 diff --git a/repos/regression/multiple_thread_block_on_domain_create.py 
 b/repos/regression/multiple_thread_block_on_domain_create.py
 index 08a9190..4224510 100644
 --- a/repos/regression/multiple_thread_block_on_domain_create.py
 +++ b/repos/regression/multiple_thread_block_on_domain_create.py
 @@ -81,8 +81,8 @@ class domain_install(Thread):
  self.logger.debug(guestxml is %s % guestxml)
  self.logger.info('create guest %sfrom xml description' % self.name)
  try:
 -guestobj = self.conn.createXML(guestxml, 0)
 -self.logger.info('guest %s API createXML returned successfuly' % 
 guestobj.name())
 +domobj = self.conn.createXML(guestxml, 0)
 +self.logger.info('guest %s API createXML returned successfuly' % 
 domobj.name())
  except libvirtError, e:
  logger.error(API error message: %s, error code is %s \
   % (e.message, e.get_error_code()))

ACK

Martin

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

Re: [libvirt] [test-API 04/17] Substitute guest_ with domain_

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 ---
  repos/domain/attach_disk.py|4 ++--
  repos/domain/attach_interface.py   |2 +-
  repos/domain/autostart.py  |4 ++--
  repos/domain/balloon_memory.py |8 
  repos/domain/blkstats.py   |4 ++--
  repos/domain/cpu_topology.py   |   12 ++--
  repos/domain/detach_disk.py|4 ++--
  repos/domain/detach_interface.py   |4 ++--
  repos/domain/dump.py   |   10 +-
  repos/domain/ifstats.py|4 ++--
  repos/domain/install_linux_cdrom.py|6 +++---
  repos/domain/install_linux_net.py  |6 +++---
  repos/domain/restore.py|   14 +++---
  repos/domain/save.py   |   16 
  repos/domain/sched_params.py   |4 ++--
  .../multiple_thread_block_on_domain_create.py  |   16 
  repos/snapshot/delete.py   |4 ++--
  repos/snapshot/snapshot_list.py|   16 
  18 files changed, 69 insertions(+), 69 deletions(-)
 
 diff --git a/repos/domain/attach_disk.py b/repos/domain/attach_disk.py
 index 4711ad1..f07b5bb 100644
 --- a/repos/domain/attach_disk.py
 +++ b/repos/domain/attach_disk.py
 @@ -32,7 +32,7 @@ def create_image(name, size, logger):
  else:
  return False
  
 -def check_guest_status(domobj):
 +def check_domain_status(domobj):
  Check guest current status
  state = domobj.info()[0]
  if state == libvirt.VIR_DOMAIN_SHUTOFF or state == 
 libvirt.VIR_DOMAIN_SHUTDOWN:
 @@ -76,7 +76,7 @@ def attach_disk(params):
  logger.debug(original disk number: %s %disk_num1)
  
  if disktype == virtio:
 -if check_guest_status(domobj):
 +if check_domain_status(domobj):
  pass
  else:
  domobj.create()
 diff --git a/repos/domain/attach_interface.py 
 b/repos/domain/attach_interface.py
 index 4d605f9..ce4282a 100644
 --- a/repos/domain/attach_interface.py
 +++ b/repos/domain/attach_interface.py
 @@ -15,7 +15,7 @@ from utils import xml_builder
  required_params = ('guestname', 'ifacetype', 'source',)
  optional_params = ('hdmodel',)
  
 -def check_guest_status(guestname, domobj):
 +def check_domain_status(guestname, domobj):
  Check guest current status
  state = domobj.get_state(guestname)
  if state == shutoff or state == shutdown:
 diff --git a/repos/domain/autostart.py b/repos/domain/autostart.py
 index da428c2..0b11022 100644
 --- a/repos/domain/autostart.py
 +++ b/repos/domain/autostart.py
 @@ -13,7 +13,7 @@ from src import sharedmod
  required_params = ('guestname', 'autostart',)
  optional_params = ()
  
 -def check_guest_autostart(*args):
 +def check_domain_autostart(*args):
  Check domain start automatically result, if setting domain is
 successful, guestname.xml will exist under
 /etc/libvirt/{hypervisor}/autostart/
 @@ -59,7 +59,7 @@ def autostart(params):
  
  try:
  domobj.setAutostart(flag)
 -if check_guest_autostart(guestname, uri.split(:)[0], flag, logger):
 +if check_domain_autostart(guestname, uri.split(:)[0], flag, 
 logger):
  logger.info(current %s autostart: %s %
  (guestname, domobj.autostart()))
  logger.info(executing autostart operation is successful)
 diff --git a/repos/domain/balloon_memory.py b/repos/domain/balloon_memory.py
 index fc7383c..642c1fa 100644
 --- a/repos/domain/balloon_memory.py
 +++ b/repos/domain/balloon_memory.py
 @@ -64,7 +64,7 @@ def redefine_memory_size(domobj, domname, memsize):
  
  return doc.toxml()
  
 -def guest_power_on(domobj, domname, mac):
 +def domain_power_on(domobj, domname, mac):
   power on guest virtual machine
  
  try:
 @@ -96,7 +96,7 @@ def guest_power_on(domobj, domname, mac):
  
  return 0
  
 -def guest_power_off(domobj, domname):
 +def domain_power_off(domobj, domname):
   power off guest virtual machine
  
  state = domobj.info()[0]
 @@ -161,7 +161,7 @@ def balloon_memory(params):
   power off it to set memory to maximum memory %
   domname)
  domobj = conn.lookupByName(domname)
 -ret = guest_power_off(domobj, domname)
 +ret = domain_power_off(domobj, domname)
  if ret:
  return 1
  
 @@ -188,7 +188,7 @@ def balloon_memory(params):
  return 1
  
  logger.info(memory set is finished, boot up the guest %s  % domname)
 -ret = guest_power_on(domobj, domname, mac)
 +ret = domain_power_on(domobj, domname, mac)
  if ret:
  return 1
  
 diff --git a/repos/domain/blkstats.py b/repos/domain/blkstats.py
 index 31bd37f..24c4b85 100644

Re: [libvirt] [test-API 06/17] Substitute guestxml with domxml

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 ---
  repos/domain/balloon_memory.py |   12 ++--
  repos/domain/cpu_affinity.py   |   12 ++--
  repos/domain/cpu_topology.py   |6 +++---
  repos/domain/install_image.py  |6 +++---
  repos/domain/install_linux_cdrom.py|   14 +++---
  repos/domain/install_linux_net.py  |   14 +++---
  repos/domain/install_windows_cdrom.py  |   14 +++---
  repos/domain/update_devflag.py |   10 +-
  .../multiple_thread_block_on_domain_create.py  |6 +++---
  utils/xml_builder.py   |2 +-
  10 files changed, 48 insertions(+), 48 deletions(-)
 
 diff --git a/repos/domain/balloon_memory.py b/repos/domain/balloon_memory.py
 index 642c1fa..11d4565 100644
 --- a/repos/domain/balloon_memory.py
 +++ b/repos/domain/balloon_memory.py
 @@ -42,10 +42,10 @@ def redefine_memory_size(domobj, domname, memsize):
   dump domain xml description to change the memory size,
  then, define the domain again
  
 -guestxml = domobj.XMLDesc(0)
 -logger.debug('''original guest %s xml :\n%s''' % (domname, guestxml))
 +domxml = domobj.XMLDesc(0)
 +logger.debug('''original guest %s xml :\n%s''' % (domname, domxml))
  
 -doc = minidom.parseString(guestxml)
 +doc = minidom.parseString(domxml)
  
  newmem = doc.createElement('memory')
  newmemval = doc.createTextNode(str(memsize))
 @@ -166,8 +166,8 @@ def balloon_memory(params):
  return 1
  
  # Redefine domain with specified memory size
 -newguestxml = redefine_memory_size(domobj, domname, maxmem)
 -logger.debug('''new guest %s xml :\n%s''' %(domname, newguestxml))
 +newdomxml = redefine_memory_size(domobj, domname, maxmem)
 +logger.debug('''new guest %s xml :\n%s''' %(domname, newdomxml))
  
  logger.info(undefine the original guest)
  try:
 @@ -180,7 +180,7 @@ def balloon_memory(params):
  
  logger.info(define guest with new xml)
  try:
 -conn.defineXML(newguestxml)
 +conn.defineXML(newdomxml)
  except libvirtError, e:
  logger.error(API error message: %s, error code is %s \
   % (e.message, e.get_error_code()))
 diff --git a/repos/domain/cpu_affinity.py b/repos/domain/cpu_affinity.py
 index fc99664..1b300dc 100644
 --- a/repos/domain/cpu_affinity.py
 +++ b/repos/domain/cpu_affinity.py
 @@ -22,10 +22,10 @@ def redefine_vcpu_number(domobj, domain_name, vcpu):
  dump domain xml description to change the vcpu number,
 then, define the domain again
  
 -guestxml = domobj.XMLDesc(0)
 -logger.debug('''original guest %s xml :\n%s''' %(domain_name, guestxml))
 +domxml = domobj.XMLDesc(0)
 +logger.debug('''original guest %s xml :\n%s''' %(domain_name, domxml))
  
 -doc = minidom.parseString(guestxml)
 +doc = minidom.parseString(domxml)
  
  newvcpu = doc.createElement('vcpu')
  newvcpuval = doc.createTextNode(vcpu)
 @@ -78,8 +78,8 @@ def set_vcpus(util, domobj, domain_name, vcpu):
  logger.error(the domain couldn't be destroied within 60 secs.)
  return 1
  
 -newguestxml = redefine_vcpu_number(domobj, domain_name, vcpu)
 -logger.debug('''new guest %s xml :\n%s''' %(domain_name, newguestxml))
 +newdomxml = redefine_vcpu_number(domobj, domain_name, vcpu)
 +logger.debug('''new guest %s xml :\n%s''' %(domain_name, newdomxml))
  
  logger.info(undefine the original guest)
  try:
 @@ -93,7 +93,7 @@ def set_vcpus(util, domobj, domain_name, vcpu):
  logger.info(define guest with new xml)
  try:
  conn = domobj._conn
 -conn.defineXML(newguestxml)
 +conn.defineXML(newdomxml)
  except libvirtError, e:
  logger.error(API error message: %s, error code is %s \
   % (e.message, e.get_error_code()))
 diff --git a/repos/domain/cpu_topology.py b/repos/domain/cpu_topology.py
 index a463668..ccde97f 100644
 --- a/repos/domain/cpu_topology.py
 +++ b/repos/domain/cpu_topology.py
 @@ -34,10 +34,10 @@ def check_domain_running(conn, guestname, logger):
  def add_cpu_xml(domobj, guestname, sockets, cores, threads, logger):
  edit domain xml description and insert cpu element
  
 -guestxml = domobj.XMLDesc(0)
 -logger.debug('''original guest %s xml :\n%s''' %(guestname, guestxml))
 +domxml = domobj.XMLDesc(0)
 +logger.debug('''original guest %s xml :\n%s''' %(guestname, domxml))
  
 -doc = minidom.parseString(guestxml)
 +doc = minidom.parseString(domxml)
  cpu = doc.createElement('cpu')
  topology = doc.createElement('topology')
  topology.setAttribute('sockets', sockets)
 diff --git a/repos/domain/install_image.py b/repos/domain/install_image.py
 index 84df56d..fe05ca8 100644
 --- a/repos/domain/install_image.py
 +++

Re: [libvirt] [test-API 07/17] Substitute _guest with _domain

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 ---
  .../en-US/Creating_a_configuration_file.xml|   82 
 ++--
  .../en-US/Writing_a_test_case.xml  |   24 +++---
  .../libvirt-test-API_Guide/en-US/extras/log.txt|   18 ++--
  repos/domain/domain_list.py|   12 ++--
  repos/domain/install_linux_cdrom.py|   18 ++--
  repos/domain/install_linux_net.py  |8 +-
  repos/domain/install_windows_cdrom.py  |6 +-
  repos/domain/update_devflag.py |6 +-
  8 files changed, 87 insertions(+), 87 deletions(-)
 
 diff --git 
 a/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml
  
 b/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml
 index 80ebdb1..ffac187 100644
 --- 
 a/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml
 +++ 
 b/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml
 @@ -90,7 +90,7 @@ module:test_case
   example
 titleConfiguration file/title
  programlisting
 -Domain:install_guest
 +Domain:install_domain
  guestname
  rhel5u4
  memory
 @@ -105,7 +105,7 @@ Domain:install_guest
 titleList data structure/title
  programlisting
  [
 -  [{'domain:install_guest': {'guestname': 'rhel5u4', 'memory': '1024', 
 'vcpu': '1'}}]
 +  [{'domain:install_domain': {'guestname': 'rhel5u4', 'memory': '1024', 
 'vcpu': '1'}}]

I'm not sure about this. The thing we're creating is a domain, but we're
installing a guest, am I right? (thus the guestname and later
guestip and so on. But I could be wrong. Also even if I'm not, the
question is how much we want to separate this in order to keep it readable.

Martin

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

Re: [libvirt] [test-API 09/17] Substitute guestos with os_type

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 ---
  cases/consumption_cpu_topology.conf|2 +-
  cases/consumption_domain_nfs_start.conf|2 +-
  cases/consumption_eventhandler.conf|2 +-
  cases/consumption_libvirtd.conf|2 +-
  cases/consumption_ownership_test.conf  |2 +-
  cases/domain_linux_net_inst.conf   |2 +-
  cases/linux_domain.conf|2 +-
  cases/snapshot.conf|2 +-
  cases/windows_domain.conf  |2 +-
  repos/domain/install_image.py  |8 +++---
  repos/domain/install_linux_cdrom.py|8 +++---
  repos/domain/install_linux_net.py  |   14 +-
  repos/domain/install_windows_cdrom.py  |   24 
 ++--
  .../multiple_thread_block_on_domain_create.py  |   12 +-
  14 files changed, 42 insertions(+), 42 deletions(-)
 
 diff --git a/cases/consumption_cpu_topology.conf 
 b/cases/consumption_cpu_topology.conf
 index 1d76256..429f856 100644
 --- a/cases/consumption_cpu_topology.conf
 +++ b/cases/consumption_cpu_topology.conf
 @@ -3,7 +3,7 @@ domain:install_linux_cdrom
  $defaultname
  virt_type
  $defaulthv
 -guestos
 +os_type
  $defaultos
  guestarch
  $defaultarch
 diff --git a/cases/consumption_domain_nfs_start.conf 
 b/cases/consumption_domain_nfs_start.conf
 index f97cda0..acc7f9e 100644
 --- a/cases/consumption_domain_nfs_start.conf
 +++ b/cases/consumption_domain_nfs_start.conf
 @@ -3,7 +3,7 @@ domain:install_linux_cdrom
  $defaultname
  virt_type
  $defaulthv
 -guestos
 +os_type
  $defaultos
  guestarch
  $defaultarch
 diff --git a/cases/consumption_eventhandler.conf 
 b/cases/consumption_eventhandler.conf
 index 768d616..0fc4509 100644
 --- a/cases/consumption_eventhandler.conf
 +++ b/cases/consumption_eventhandler.conf
 @@ -3,7 +3,7 @@ domain:install_linux_cdrom
  $defaultname
  virt_type
  $defaulthv
 -guestos
 +os_type
  $defaultos
  guestarch
  $defaultarch
 diff --git a/cases/consumption_libvirtd.conf b/cases/consumption_libvirtd.conf
 index 4eb8f86..330d7fc 100644
 --- a/cases/consumption_libvirtd.conf
 +++ b/cases/consumption_libvirtd.conf
 @@ -3,7 +3,7 @@ domain:install_linux_cdrom
  $defaultname
  virt_type
  $defaulthv
 -guestos
 +os_type
  $defaultos
  guestarch
  $defaultarch
 diff --git a/cases/consumption_ownership_test.conf 
 b/cases/consumption_ownership_test.conf
 index 58468fd..dc0d7ab 100644
 --- a/cases/consumption_ownership_test.conf
 +++ b/cases/consumption_ownership_test.conf
 @@ -3,7 +3,7 @@ domain:install_linux_cdrom
  $defaultname
  virt_type
  $defaulthv
 -guestos
 +os_type
  $defaultos
  guestarch
  $defaultarch
 diff --git a/cases/domain_linux_net_inst.conf 
 b/cases/domain_linux_net_inst.conf
 index 4c8ab5d..76bb6ba 100644
 --- a/cases/domain_linux_net_inst.conf
 +++ b/cases/domain_linux_net_inst.conf
 @@ -3,7 +3,7 @@ domain:install_linux_net
  $defaultname
  virt_type
  $defaulthv
 -guestos
 +os_type
  $defaultos
  guestarch
  $defaultarch
 diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf
 index 5059c0d..bae93e6 100644
 --- a/cases/linux_domain.conf
 +++ b/cases/linux_domain.conf
 @@ -3,7 +3,7 @@ domain:install_linux_cdrom
  $defaultname
  virt_type
  $defaulthv
 -guestos
 +os_type
  $defaultos
  guestarch
  $defaultarch
 diff --git a/cases/snapshot.conf b/cases/snapshot.conf
 index 0557c86..924ef61 100644
 --- a/cases/snapshot.conf
 +++ b/cases/snapshot.conf
 @@ -3,7 +3,7 @@ domain:install_linux_cdrom
  $defaultname
  virt_type
  $defaulthv
 -guestos
 +os_type
  $defaultos
  guestarch
  $defaultarch
 diff --git a/cases/windows_domain.conf b/cases/windows_domain.conf
 index 6eed3f5..917f3ec 100644
 --- a/cases/windows_domain.conf
 +++ b/cases/windows_domain.conf
 @@ -3,7 +3,7 @@ domain:install_windows_cdrom
  $defaultname
  virt_type
  $defaulthv
 -guestos
 +os_type
  $defaultos
  guestarch
  $defaultarch
 diff --git a/repos/domain/install_image.py b/repos/domain/install_image.py
 index 80dcf1a..12c638e 100644
 --- a/repos/domain/install_image.py
 +++ b/repos/domain/install_image.py
 @@ -17,7 +17,7 @@ from utils import xml_builder
  
  HOME_PATH = os.getcwd()
  
 -required_params = ('guestname', 'virt_type', 'guestos', 'guestarch',)
 +required_params = ('guestname', 'virt_type', 'os_type', 'guestarch',)
  optional_params = ('uuid',
 'memory',
 'vcpu',
 @@ -35,12 +35,12 @@ def install_image(params):

Re: [libvirt] [test-API 07/17] Substitute _guest with _domain

2012-04-20 Thread Martin Kletzander

On 04/20/2012 10:29 AM, Osier Yang wrote:
 On 2012年04月20日 16:17, Martin Kletzander wrote:
 On 04/20/2012 08:46 AM, Osier Yang wrote:
 ---
   .../en-US/Creating_a_configuration_file.xml|   82
 ++--
   .../en-US/Writing_a_test_case.xml  |   24 +++---
   .../libvirt-test-API_Guide/en-US/extras/log.txt|   18 ++--
   repos/domain/domain_list.py|   12 ++--
   repos/domain/install_linux_cdrom.py|   18 ++--
   repos/domain/install_linux_net.py  |8 +-
   repos/domain/install_windows_cdrom.py  |6 +-
   repos/domain/update_devflag.py |6 +-
   8 files changed, 87 insertions(+), 87 deletions(-)

 diff --git
 a/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml
 b/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml

 index 80ebdb1..ffac187 100644
 ---
 a/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml

 +++
 b/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml

 @@ -90,7 +90,7 @@ module:test_case
   example
   titleConfiguration file/title
   programlisting
 -Domain:install_guest
 +Domain:install_domain
   guestname
   rhel5u4
   memory
 @@ -105,7 +105,7 @@ Domain:install_guest
   titleList data structure/title
   programlisting
   [
 -  [{'domain:install_guest': {'guestname': 'rhel5u4', 'memory':
 '1024', 'vcpu': '1'}}]
 +  [{'domain:install_domain': {'guestname': 'rhel5u4', 'memory':
 '1024', 'vcpu': '1'}}]

 I'm not sure about this. The thing we're creating is a domain, but we're
 installing a guest, am I right? (thus the guestname and later
 guestip and so on. But I could be wrong. Also even if I'm not, the
 question is how much we want to separate this in order to keep it
 readable.
 
 
 I'm not quite sure about it either, but I would like to post the patch
 so there could be discussion, IIRC, there was discussion about the TERM
 domain and guest before too. Anyway, there should be guidelines for
 it, mixing use of both of them at liberty can just produce messy codes.
 
 Osier

It's written in the guide: http://libvirt.org/devguide.html but I'm not
sure even after reading it. It's not _defined_ there, so we should
probably go with our gut on this one. After looking at the code with all
the patches applied, I think this could be ok, we have only few words
with guest in it and they make sense, so for the sake of stabilizing the
code asap, ACK.

Martin

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

Re: [libvirt] [test-API 07/17] Substitute _guest with _domain

2012-04-20 Thread Martin Kletzander

On 04/20/2012 11:16 AM, Alex Jia wrote:
 On 04/20/2012 04:29 PM, Osier Yang wrote:
 On 2012年04月20日 16:17, Martin Kletzander wrote:
 On 04/20/2012 08:46 AM, Osier Yang wrote:
 ---
   .../en-US/Creating_a_configuration_file.xml|   82
 ++--
   .../en-US/Writing_a_test_case.xml  |   24 +++---
   .../libvirt-test-API_Guide/en-US/extras/log.txt|   18 ++--
   repos/domain/domain_list.py|   12 ++--
   repos/domain/install_linux_cdrom.py|   18 ++--
   repos/domain/install_linux_net.py  |8 +-
   repos/domain/install_windows_cdrom.py  |6 +-
   repos/domain/update_devflag.py |6 +-
   8 files changed, 87 insertions(+), 87 deletions(-)

 diff --git
 a/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml
 b/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml

 index 80ebdb1..ffac187 100644
 ---
 a/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml

 +++
 b/docs/User_Guide/libvirt-test-API_Guide/en-US/Creating_a_configuration_file.xml

 @@ -90,7 +90,7 @@ module:test_case
 example
 titleConfiguration file/title
 programlisting
 -Domain:install_guest
 +Domain:install_domain
   guestname
   rhel5u4
   memory
 @@ -105,7 +105,7 @@ Domain:install_guest
 titleList data structure/title
 programlisting
   [
 -  [{'domain:install_guest': {'guestname': 'rhel5u4', 'memory':
 '1024', 'vcpu': '1'}}]
 +  [{'domain:install_domain': {'guestname': 'rhel5u4', 'memory':
 '1024', 'vcpu': '1'}}]

 I'm not sure about this. The thing we're creating is a domain, but we're
 installing a guest, am I right? (thus the guestname and later
 guestip and so on. But I could be wrong. Also even if I'm not, the
 question is how much we want to separate this in order to keep it
 readable.


 I'm not quite sure about it either, but I would like to post the patch
 so there could be discussion, IIRC, there was discussion about the TERM
 domain and guest before too. Anyway, there should be guidelines for
 it, mixing use of both of them at liberty can just produce messy codes.

 I feel 'domain' should be a xen terminology such as domain 0(dom0) and
 domain U(domU),
 and domU is a user domains, as usual, called a guest in kvm/qemu,
 however, it's not very
 clear such as some guys say xen guest domain :-(

I feel it a little bit different. I know I'm in no position to have a
firm stance on this subject, but let me explain what I mean.

From my point of view, domain is a (virtual) machine. The system running
inside the VM is guest. However, there are things that might have
connection with both. This is clearly the where do we draw the line
kind of case. I think HW-related terms could be called 'domain' (e.g.
MAC address), which would make the SW-related called 'guest'. When it
sounds good, there is no problem with calling something 'guest domain',
but just when it feels right (and that's probably just when it means
domain, as it is the subjective noun in this case, whether guest is
just an adjective). I got caught in a moment and went a little deep, I
know, sorry for that.

Just my $0.02

Martin

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

Re: [libvirt] [test-API 08/17] Substitute 'guest ' with 'domain '

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 ---
  cases/migration/ssh_persistent_paused_no_dst.conf  |2 +-
  .../migration/ssh_persistent_paused_with_dst.conf  |2 +-
  cases/migration/ssh_persistent_running_no_dst.conf |2 +-
  .../migration/ssh_persistent_running_with_dst.conf |2 +-
  cases/migration/ssh_transient_paused_no_dst.conf   |2 +-
  cases/migration/ssh_transient_paused_with_dst.conf |2 +-
  cases/migration/ssh_transient_running_no_dst.conf  |2 +-
  .../migration/ssh_transient_running_with_dst.conf  |2 +-
  cases/migration/tcp_persistent_paused_no_dst.conf  |2 +-
  .../migration/tcp_persistent_paused_with_dst.conf  |2 +-
  cases/migration/tcp_persistent_running_no_dst.conf |2 +-
  .../migration/tcp_persistent_running_with_dst.conf |2 +-
  .../tcp_sasl_persistent_paused_no_dst.conf |2 +-
  .../tcp_sasl_persistent_paused_with_dst.conf   |2 +-
  .../tcp_sasl_persistent_running_no_dst.conf|2 +-
  .../tcp_sasl_persistent_running_with_dst.conf  |2 +-
  .../tcp_sasl_transient_paused_no_dst.conf  |2 +-
  .../tcp_sasl_transient_paused_with_dst.conf|2 +-
  .../tcp_sasl_transient_running_no_dst.conf |2 +-
  .../tcp_sasl_transient_running_with_dst.conf   |2 +-
  cases/migration/tcp_transient_paused_no_dst.conf   |2 +-
  cases/migration/tcp_transient_paused_with_dst.conf |2 +-
  cases/migration/tcp_transient_running_no_dst.conf  |2 +-
  .../migration/tcp_transient_running_with_dst.conf  |2 +-
  cases/migration/tls_persistent_paused_no_dst.conf  |2 +-
  .../migration/tls_persistent_paused_with_dst.conf  |2 +-
  cases/migration/tls_persistent_running_no_dst.conf |2 +-
  .../migration/tls_persistent_running_with_dst.conf |2 +-
  .../tls_sasl_persistent_paused_no_dst.conf |2 +-
  .../tls_sasl_persistent_paused_with_dst.conf   |2 +-
  .../tls_sasl_persistent_running_no_dst.conf|2 +-
  .../tls_sasl_persistent_running_with_dst.conf  |2 +-
  .../tls_sasl_transient_paused_no_dst.conf  |2 +-
  .../tls_sasl_transient_paused_with_dst.conf|2 +-
  .../tls_sasl_transient_running_no_dst.conf |2 +-
  .../tls_sasl_transient_running_with_dst.conf   |2 +-
  cases/migration/tls_transient_paused_no_dst.conf   |2 +-
  cases/migration/tls_transient_paused_with_dst.conf |2 +-
  cases/migration/tls_transient_running_no_dst.conf  |2 +-
  .../migration/tls_transient_running_with_dst.conf  |2 +-
  .../en-US/Understanding_libvirt-test-API.xml   |2 +-
  .../en-US/Writing_a_test_case.xml  |   14 +++---
  .../libvirt-test-API_Guide/en-US/extras/log.txt|8 ++--
  global.cfg |   14 +++---
  repos/domain/attach_disk.py|4 +-
  repos/domain/attach_interface.py   |4 +-
  repos/domain/autostart.py  |2 +-
  repos/domain/balloon_memory.py |   42 +++---
  repos/domain/blkstats.py   |2 +-
  repos/domain/console_io.py |4 +-
  repos/domain/console_mutex.py  |2 +-
  repos/domain/cpu_affinity.py   |   22 
  repos/domain/cpu_topology.py   |   14 +++---
  repos/domain/create.py |4 +-
  repos/domain/define.py |2 +-
  repos/domain/destroy.py|6 +-
  repos/domain/detach_disk.py|4 +-
  repos/domain/detach_interface.py   |4 +-
  repos/domain/domain_blkinfo.py |2 +-
  repos/domain/domain_id.py  |2 +-
  repos/domain/domain_list.py|   10 ++--
  repos/domain/domain_name.py|4 +-
  repos/domain/domain_uuid.py|4 +-
  repos/domain/domblkinfo.py |2 +-
  repos/domain/dump.py   |   14 +++---
  repos/domain/eventhandler.py   |6 +-
  repos/domain/ifstats.py|2 +-
  repos/domain/install_image.py  |   14 +++---
  repos/domain/install_linux_cdrom.py|   58 
 ++--
  repos/domain/install_linux_check.py|   36 ++--
  repos/domain/install_linux_net.py  |   52 +-
  repos/domain/install_windows_cdrom.py  |   50 +-
  repos/domain/migrate.py|2 +-
  repos/domain/restore.py|   16 +++---
  repos/domain/save.py   |   16 +++---
  repos/domain/sched_params.py   |2 +-
  repos/domain/shutdown.py

Re: [libvirt] [test-API 11/17] Substitute 'guestip' with 'domip'

2012-04-20 Thread Martin Kletzander

On 04/20/2012 08:46 AM, Osier Yang wrote:
 ---
  repos/domain/update_devflag.py |   20 ++--
  1 files changed, 10 insertions(+), 10 deletions(-)
 
 diff --git a/repos/domain/update_devflag.py b/repos/domain/update_devflag.py
 index f1ecf32..e87b1e5 100644
 --- a/repos/domain/update_devflag.py
 +++ b/repos/domain/update_devflag.py
 @@ -77,7 +77,7 @@ def create_image(params, util, img_name):
  
  return True
  
 -def check_device_in_domain(params, util, guestip):
 +def check_device_in_domain(params, util, domip):
  Check updated device in guest
  logger = params['logger']
  
 @@ -89,7 +89,7 @@ def check_device_in_domain(params, util, guestip):
  logger.error(it's not a cdrom or floppy device.)
  return False, None
  
 -ret, output = utils.remote_exec_pexpect(guestip, params['username'], \
 +ret, output = utils.remote_exec_pexpect(domip, params['username'], \
 params['password'], cmd)
  logger.debug(output)
  if ret:
 @@ -98,7 +98,7 @@ def check_device_in_domain(params, util, guestip):
  
  time.sleep(5)
  
 -ret, output = utils.remote_exec_pexpect(guestip, params['username'], \
 +ret, output = utils.remote_exec_pexpect(domip, params['username'], \
 params['password'], umount 
 /media)
  logger.debug(output)
  if ret:
 @@ -107,7 +107,7 @@ def check_device_in_domain(params, util, guestip):
  
  time.sleep(5)
  
 -ret, output = utils.remote_exec_pexpect(guestip, params['username'], \
 +ret, output = utils.remote_exec_pexpect(domip, params['username'], \
 params['password'], ls /media)
  logger.debug(output)
  if ret:
 @@ -117,7 +117,7 @@ def check_device_in_domain(params, util, guestip):
  
  return True, output
  
 -def check_updated_device(params, output, util, guestip, domobj, srcfile):
 +def check_updated_device(params, output, util, domip, domobj, srcfile):
  Check if the device is updated
  logger = params['logger']
  xmlobj = domobj.XMLDesc(0)
 @@ -129,7 +129,7 @@ def check_updated_device(params, output, util, guestip, 
 domobj, srcfile):
  elif diskTag.parentNode.getAttribute('device') == 'floppy':
  upfile = diskTag.getAttribute(file)
  
 -res = check_device_in_domain(params, util, guestip)
 +res = check_device_in_domain(params, util, domip)
  if res[0] and cmp(res[1], output):
  if upfile == srcfile:
  logger.debug(checking fail.)
 @@ -172,8 +172,8 @@ def update_devflag(params):
  flag = libvirt.VIR_DOMAIN_AFFECT_CONFIG
  
  mac = utils.get_dom_mac_addr(guestname)
 -guestip = utils.mac_to_ip(mac, 180)
 -logger.debug(ip address: %s % guestip)
 +domip = utils.mac_to_ip(mac, 180)
 +logger.debug(ip address: %s % domip)
  
  conn = sharedmod.libvirtobj['conn']
  
 @@ -211,7 +211,7 @@ def update_devflag(params):
  return 1
  
  time.sleep(60)
 -ret, output = check_device_in_domain(params, util, guestip)
 +ret, output = check_device_in_domain(params, util, domip)
  logger.debug(output)
  if not ret:
  return 1
 @@ -249,7 +249,7 @@ def update_devflag(params):
  return 1
  
  result = check_updated_device(params, output, util, \
 -   guestip, domobj, srcfile)
 +   domip, domobj, srcfile)
  if result[0]:
  logger.error(fail to update '%s' device: %s\n % (devtype, 
 result[1]))
  return 1

This one I'd rather reconsider with the others, guest ip is better I
think. Just my opinion though.

Martin

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

Re: [libvirt] [PATCH 1/3] usb: create functions to search usb device accurately

2012-04-30 Thread Martin Kletzander

On 04/28/2012 12:13 PM, Guannan Ren wrote:
 usbFindDevice():get usb device according to
 idVendor, idProduct, bus, device
 it is the most strict search
 
 usbFindDevByBus():get usb device according to bus, device
   it returns only one usb device same as usbFindDevice
 
 usbFindDevByVendor():get usb device according to idVendor,idProduct
  it probably returns multiple usb devices.
 
 usbDeivceSearch(): a helper function to do the actual search

Typo:

s/Deivce/Device/

but unfortunately in the whole patch =)

 ---
  src/util/hostusb.c |  162 ++-
  src/util/hostusb.h |   20 ++-
  2 files changed, 138 insertions(+), 44 deletions(-)
 
 diff --git a/src/util/hostusb.c b/src/util/hostusb.c
 index 92f52a2..73dc959 100644
 --- a/src/util/hostusb.c
 +++ b/src/util/hostusb.c
 @@ -94,13 +94,22 @@ cleanup:
  return ret;
  }
  
 -static int usbFindBusByVendor(unsigned vendor, unsigned product,
 -  unsigned *bus, unsigned *devno)
 +static usbDeviceList *
 +usbDeivceSearch(unsigned vendor,
 +unsigned product,
 +unsigned bus,
 +unsigned devno,
 +unsigned flags)
  {
  DIR *dir = NULL;
 -int ret = -1, found = 0;
 +int found = 0;
  char *ignore = NULL;
  struct dirent *de;
 +usbDeviceList *list = NULL, *ret = NULL;
 +usbDevice *usb;
 +
 +if (!(list = usbDeviceListNew()))
 +goto cleanup;
  
  dir = opendir(USB_SYSFS /devices);
  if (!dir) {
 @@ -111,48 +120,72 @@ static int usbFindBusByVendor(unsigned vendor, unsigned 
 product,
  }
  
  while ((de = readdir(dir))) {
 -unsigned found_prod, found_vend;
 +unsigned found_prod, found_vend, found_bus, found_devno;
 +char *tmpstr = de-d_name;
 +
  if (de-d_name[0] == '.' || strchr(de-d_name, ':'))
  continue;
  
  if (usbSysReadFile(idVendor, de-d_name,
 16, found_vend)  0)
  goto cleanup;
 +
  if (usbSysReadFile(idProduct, de-d_name,
 16, found_prod)  0)
  goto cleanup;
  
 -if (found_prod == product  found_vend == vendor) {
 -/* Lookup bus.addr info */
 -char *tmpstr = de-d_name;
 -unsigned found_bus, found_addr;
 +if (STRPREFIX(de-d_name, usb))
 +tmpstr += 3;
  
 -if (STRPREFIX(de-d_name, usb))
 -tmpstr += 3;
 +if (virStrToLong_ui(tmpstr, ignore, 10, found_bus)  0) {
 +usbReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Failed to parse dir name '%s'),
 +   de-d_name);
 +goto cleanup;
 +}
 +
 +if (usbSysReadFile(devnum, de-d_name,
 +   10, found_devno)  0)
 +goto cleanup;
  
 -if (virStrToLong_ui(tmpstr, ignore, 10, found_bus)  0) {
 -usbReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(Failed to parse dir name '%s'),
 -   de-d_name);
 -goto cleanup;
 -}
 +switch (flags) {
 +/*
 + * Don't set found to 1 in order to continue the loop
 + * to find multiple USB devices with same idVendor and idProduct
 + */
 +case USB_DEVICE_FIND_BY_VENDOR:
 +if (found_prod != product || found_vend != vendor)
 +continue;
 +break;
  
 -if (usbSysReadFile(devnum, de-d_name,
 -   10, found_addr)  0)
 -goto cleanup;
 +case USB_DEVICE_FIND_BY_BUS:
 +if (found_bus != bus || found_devno != devno)
 +continue;
 +found = 1;
 +break;
  
 -*bus = found_bus;
 -*devno = found_addr;
 +case USB_DEVICE_FIND_BY_ALL:
 +if (found_prod != product
 +|| found_vend != vendor
 +|| found_bus != bus
 +|| found_devno != devno)
 +continue;
  found = 1;
  break;
  }
 -}
  
 -if (!found)
 -usbReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(Did not find USB device %x:%x), vendor, product);
 -else
 -ret = 0;
 +usb = usbGetDevice(found_bus, found_devno);
 +if (!usb)
 +goto cleanup;
 +
 +if (usbDeviceListAdd(list, usb)  0) {
 +usbFreeDevice(usb);
 +goto cleanup;
 +}
 +
 +if (found) break;
 +}
 +ret = list;
  
  cleanup:
  if (dir) {
 @@ -160,9 +193,69 @@ cleanup:
  closedir (dir);
  errno = saved_errno;
  }
 +
 +if (!ret)
 +usbDeviceListFree(list);
  return ret;
  }
  
 +usbDeviceList *

Re: [libvirt] [PATCH v2 1/3] usb: create functions to search usb device accurately

2012-05-02 Thread Martin Kletzander

On 05/01/2012 10:16 AM, Guannan Ren wrote:
 usbFindDevice():get usb device according to
 idVendor, idProduct, bus, device
 it is the most strict search
 
 usbFindDevByBus():get usb device according to bus, device
   it returns only one usb device same as usbFindDevice
 
 usbFindDevByVendor():get usb device according to idVendor,idProduct
  it probably returns multiple usb devices.
 
 usbDeviceSearch(): a helper function to do the actual search
 ---
  src/util/hostusb.c |  162 ++-
  src/util/hostusb.h |   20 ++-
  2 files changed, 138 insertions(+), 44 deletions(-)
 
 diff --git a/src/util/hostusb.c b/src/util/hostusb.c
 index 92f52a2..73dc959 100644
 --- a/src/util/hostusb.c
 +++ b/src/util/hostusb.c
 @@ -94,13 +94,22 @@ cleanup:
  return ret;
  }
  
 -static int usbFindBusByVendor(unsigned vendor, unsigned product,
 -  unsigned *bus, unsigned *devno)
 +static usbDeviceList *
 +usbDeviceSearch(unsigned vendor,
 +unsigned product,
 +unsigned bus,
 +unsigned devno,
 +unsigned flags)
  {
  DIR *dir = NULL;
 -int ret = -1, found = 0;
 +int found = 0;
  char *ignore = NULL;
  struct dirent *de;
 +usbDeviceList *list = NULL, *ret = NULL;
 +usbDevice *usb;
 +
 +if (!(list = usbDeviceListNew()))
 +goto cleanup;
  
  dir = opendir(USB_SYSFS /devices);
  if (!dir) {
 @@ -111,48 +120,72 @@ static int usbFindBusByVendor(unsigned vendor, unsigned 
 product,
  }
  
  while ((de = readdir(dir))) {
 -unsigned found_prod, found_vend;
 +unsigned found_prod, found_vend, found_bus, found_devno;
 +char *tmpstr = de-d_name;
 +
  if (de-d_name[0] == '.' || strchr(de-d_name, ':'))
  continue;
  
  if (usbSysReadFile(idVendor, de-d_name,
 16, found_vend)  0)
  goto cleanup;
 +
  if (usbSysReadFile(idProduct, de-d_name,
 16, found_prod)  0)
  goto cleanup;
  
 -if (found_prod == product  found_vend == vendor) {
 -/* Lookup bus.addr info */
 -char *tmpstr = de-d_name;
 -unsigned found_bus, found_addr;
 +if (STRPREFIX(de-d_name, usb))
 +tmpstr += 3;
  
 -if (STRPREFIX(de-d_name, usb))
 -tmpstr += 3;
 +if (virStrToLong_ui(tmpstr, ignore, 10, found_bus)  0) {
 +usbReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Failed to parse dir name '%s'),
 +   de-d_name);
 +goto cleanup;
 +}
 +
 +if (usbSysReadFile(devnum, de-d_name,
 +   10, found_devno)  0)
 +goto cleanup;
  
 -if (virStrToLong_ui(tmpstr, ignore, 10, found_bus)  0) {
 -usbReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(Failed to parse dir name '%s'),
 -   de-d_name);
 -goto cleanup;
 -}
 +switch (flags) {
 +/*
 + * Don't set found to 1 in order to continue the loop
 + * to find multiple USB devices with same idVendor and idProduct
 + */
 +case USB_DEVICE_FIND_BY_VENDOR:
 +if (found_prod != product || found_vend != vendor)
 +continue;
 +break;
  
 -if (usbSysReadFile(devnum, de-d_name,
 -   10, found_addr)  0)
 -goto cleanup;
 +case USB_DEVICE_FIND_BY_BUS:
 +if (found_bus != bus || found_devno != devno)
 +continue;
 +found = 1;
 +break;
  
 -*bus = found_bus;
 -*devno = found_addr;
 +case USB_DEVICE_FIND_BY_ALL:
 +if (found_prod != product
 +|| found_vend != vendor
 +|| found_bus != bus
 +|| found_devno != devno)
 +continue;
  found = 1;
  break;
  }
 -}
  
 -if (!found)
 -usbReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(Did not find USB device %x:%x), vendor, product);
 -else
 -ret = 0;
 +usb = usbGetDevice(found_bus, found_devno);
 +if (!usb)
 +goto cleanup;
 +
 +if (usbDeviceListAdd(list, usb)  0) {
 +usbFreeDevice(usb);
 +goto cleanup;
 +}
 +
 +if (found) break;
 +}
 +ret = list;
  
  cleanup:
  if (dir) {
 @@ -160,9 +193,69 @@ cleanup:
  closedir (dir);
  errno = saved_errno;
  }
 +
 +if (!ret)
 +usbDeviceListFree(list);
  return ret;
  }
  
 +usbDeviceList *
 +usbFindDevByVendor(unsigned vendor, unsigned product)
 +{
 +
 +usbDeviceList *list;

Re: [libvirt] [PATCH v2 2/3] qemu: make use of usb search function to initialize usb devices

2012-05-02 Thread Martin Kletzander

On 05/01/2012 10:16 AM, Guannan Ren wrote:
 refactor qemuPrepareHostdevUSBDevices function, make it focus on
 adding usb device to activeUsbHostdevs after check. After that,
 the usb hotplug function qemuDomainAttachHostDevice also could use
 it.
 
 expand qemuPrepareHostUSBDevices to perform the usb search,
 rollback on failure.
 ---
  src/qemu/qemu_hostdev.c |  118 
 ++-
  src/qemu/qemu_hostdev.h |3 +-
  2 files changed, 76 insertions(+), 45 deletions(-)
 
 diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
 index 8594fb2..a3d4ad4 100644
 --- a/src/qemu/qemu_hostdev.c
 +++ b/src/qemu/qemu_hostdev.c
 @@ -565,13 +565,51 @@ qemuPrepareHostPCIDevices(struct qemud_driver *driver,
  int
  qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
   const char *name,
 - virDomainHostdevDefPtr *hostdevs,
 - int nhostdevs)
 + usbDeviceList *list)
  {
 -int ret = -1;
  int i;
 +usbDevice *tmp;
 +
 +for (i = 0; i  usbDeviceListCount(list); i++) {
 +usbDevice *usb = usbDeviceListGet(list, i);
 +if ((tmp = usbDeviceListFind(driver-activeUsbHostdevs, usb))) {
 +const char *other_name = usbDeviceGetUsedBy(tmp);
 +
 +if (other_name)
 +qemuReportError(VIR_ERR_OPERATION_INVALID,
 +_(USB device %s is in use by domain %s),
 +usbDeviceGetName(tmp), other_name);
 +else
 +qemuReportError(VIR_ERR_OPERATION_INVALID,
 +_(USB device %s is already in use),
 +usbDeviceGetName(tmp));
 +return -1;
 +}
 +
 +usbDeviceSetUsedBy(usb, name);
 +VIR_DEBUG(Adding %03d.%03d dom=%s to activeUsbHostdevs,
 +  usbDeviceGetBus(usb), usbDeviceGetDevno(usb), name);
 +/*
 + * The caller is responsible to steal the list of usb
 + * from the usbDeviceList that passed in on success,
 + * perform rollback on failure.
 + */
 +if (usbDeviceListAdd(driver-activeUsbHostdevs, usb)  0)
 +return -1;
 +
 +}
 +return 0;
 +}
 +
 +static int
 +qemuPrepareHostUSBDevices(struct qemud_driver *driver,
 +  virDomainDefPtr def)
 +{
 +int i, ret = -1;
  usbDeviceList *list;
  usbDevice *tmp;
 +virDomainHostdevDefPtr *hostdevs = def-hostdevs;
 +int nhostdevs = def-nhostdevs;
  
  /* To prevent situation where USB device is assigned to two domains
   * we need to keep a list of currently assigned USB devices.
 @@ -586,64 +624,65 @@ qemuPrepareHostdevUSBDevices(struct qemud_driver 
 *driver,
   */
  for (i = 0 ; i  nhostdevs ; i++) {
  virDomainHostdevDefPtr hostdev = hostdevs[i];
 +usbDevice *usb = NULL;
  
  if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
  continue;
  if (hostdev-source.subsys.type != 
 VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
  continue;
  
 -/* Resolve a vendor/product to bus/device */
 -if (hostdev-source.subsys.u.usb.vendor) {
 -usbDevice *usb
 -= usbFindDevice(hostdev-source.subsys.u.usb.vendor,
 -hostdev-source.subsys.u.usb.product);
 +unsigned vendor = hostdev-source.subsys.u.usb.vendor;
 +unsigned product = hostdev-source.subsys.u.usb.product;
 +unsigned bus = hostdev-source.subsys.u.usb.bus;
 +unsigned device = hostdev-source.subsys.u.usb.device;
  
 +if (vendor  bus) {
 +usb = usbFindDevice(vendor, product, bus, device);
  if (!usb)
  goto cleanup;
  
 -if ((tmp = usbDeviceListFind(driver-activeUsbHostdevs, usb))) {
 -const char *other_name = usbDeviceGetUsedBy(tmp);
 -
 -if (other_name)
 -qemuReportError(VIR_ERR_OPERATION_INVALID,
 -_(USB device %s is in use by domain 
 %s),
 -usbDeviceGetName(tmp), other_name);
 -else
 -qemuReportError(VIR_ERR_OPERATION_INVALID,
 -_(USB device %s is already in use),
 -usbDeviceGetName(tmp));
 -usbFreeDevice(usb);
 +} else if (vendor  !bus) {
 +usbDeviceList *devs = usbFindDevByVendor(vendor, product);
 +if (!devs)
 +goto cleanup;
 +
 +if (usbDeviceListCount(devs)  1) {
 +qemuReportError(VIR_ERR_XML_ERROR,
 +_(multiple USB deivces %x:%x, 
 +  use address to specify one), vendor, 
 product);
 +usbDeviceListFree(devs);

Re: [libvirt] [PATCH v2 3/3] qemu: search usb device accurately to improve usb device hotplug

2012-05-02 Thread Martin Kletzander

On 05/01/2012 10:16 AM, Guannan Ren wrote:
 One usb device could be allowed to hotplug in at a time. If user
 give a xml as follows. Probably there are two usb devices avaiable
 but with different value of bus, device
 
 we give a error to let user use address to specify the desired one.
 
 hostdev mode='subsystem' type='usb' managed='yes'
   source
 vendor id='0x15e1'/
 product id='0x2007'/
   /source
 /hostdev
 ---
  src/qemu/qemu_hotplug.c |   68 +-
  1 files changed, 54 insertions(+), 14 deletions(-)
 
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 7cf7b90..15693a0 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -1121,6 +1121,9 @@ int qemuDomainAttachHostDevice(struct qemud_driver 
 *driver,
 virDomainObjPtr vm,
 virDomainHostdevDefPtr hostdev)
  {
 +usbDeviceList *list;
 +usbDevice * usb = NULL;
 +
  if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
  qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
  _(hostdev mode '%s' not supported),
 @@ -1128,29 +1131,62 @@ int qemuDomainAttachHostDevice(struct qemud_driver 
 *driver,
  return -1;
  }
  
 -/* Resolve USB product/vendor to bus/device */
 -if (hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB 
 -hostdev-source.subsys.u.usb.vendor) {
 -if (qemuPrepareHostdevUSBDevices(driver, vm-def-name, hostdev, 1) 
  0)
 -goto error;
 +if (!(list = usbDeviceListNew()))
 +goto cleanup;
 +
 +if (hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
 +unsigned vendor = hostdev-source.subsys.u.usb.vendor;
 +unsigned product = hostdev-source.subsys.u.usb.product;
 +unsigned bus = hostdev-source.subsys.u.usb.bus;
 +unsigned device = hostdev-source.subsys.u.usb.device;
 +
 +if (vendor  bus) {
 +usb = usbFindDevice(vendor, product, bus, device);
 +if (!usb)
 +goto cleanup;
 +
 +} else if (vendor  !bus) {
 +usbDeviceList *devs = usbFindDevByVendor(vendor, product);
 +if (!devs)
 +goto cleanup;
 +
 +if (usbDeviceListCount(devs)  1) {
 +qemuReportError(VIR_ERR_XML_ERROR,
 +_(multiple USB deivces %x:%x, 
 +  use address to specify one), vendor, 
 product);
 +usbDeviceListFree(devs);
 +goto cleanup;
 +}
 +usb = usbDeviceListGet(devs, 0);
 +usbDeviceListSteal(devs, usb);
 +usbDeviceListFree(devs);
  
 -usbDevice *usb
 -= usbFindDevice(hostdev-source.subsys.u.usb.vendor,
 -hostdev-source.subsys.u.usb.product);
 +hostdev-source.subsys.u.usb.bus = usbDeviceGetBus(usb);
 +hostdev-source.subsys.u.usb.device = usbDeviceGetDevno(usb);
 +
 +} else if (!vendor  bus) {
 +usb = usbFindDevByBus(bus, device);
 +if (!usb)
 +goto cleanup;
 +}
  
  if (!usb)
 -return -1;
 +goto cleanup;
  
 -hostdev-source.subsys.u.usb.bus = usbDeviceGetBus(usb);
 -hostdev-source.subsys.u.usb.device = usbDeviceGetDevno(usb);
 +if (usbDeviceListAdd(list, usb)  0) {
 +usbFreeDevice(usb);
 +goto cleanup;
 +}
  
 -usbFreeDevice(usb);
 -}
 +if (qemuPrepareHostdevUSBDevices(driver, vm-def-name, list)  0)
 +goto cleanup;
  
 +usbDeviceListSteal(list, usb);
 +}
  
  if (virSecurityManagerSetHostdevLabel(driver-securityManager,
vm-def, hostdev)  0)
 -return -1;
 +goto cleanup;
  
  switch (hostdev-source.subsys.type) {
  case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
 @@ -1172,6 +1208,7 @@ int qemuDomainAttachHostDevice(struct qemud_driver 
 *driver,
  goto error;
  }
  
 +usbDeviceListFree(list);
  return 0;
  
  error:
 @@ -1179,6 +1216,9 @@ error:
vm-def, hostdev)  0)
  VIR_WARN(Unable to restore host device labelling on hotplug fail);
  
 +cleanup:
 +usbDeviceListFree(list);
 +usbDeviceListSteal(driver-activeUsbHostdevs, usb);
  return -1;
  }
  

Seems all is well for me, ACK.

Martin

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

Re: [libvirt] [PATCH] Add support for qxl.revision in domain XML

2013-02-22 Thread Martin Kletzander

On 02/21/2013 04:32 PM, Christophe Fergeau wrote:
 Hey Martin,
 
 Sorry, took me a while to get back to this patch,
 

No problem, I had the same issue :)

 On Thu, Feb 14, 2013 at 05:54:02PM +0100, Martin Kletzander wrote:
 On 02/04/2013 04:16 PM, Christophe Fergeau wrote:
 ---
  docs/formatdomain.html.in   |  5 -
  docs/schemas/domaincommon.rng   |  5 +
  src/conf/domain_conf.c  | 17 
 +
  src/conf/domain_conf.h  |  1 +
  src/qemu/qemu_command.c |  8 
  .../qemuxml2argv-graphics-spice-compression.args|  3 ++-
  .../qemuxml2argv-graphics-spice-compression.xml |  4 ++--
  .../qemuxml2argv-graphics-spice-qxl-vga.args|  3 ++-
  .../qemuxml2argv-graphics-spice-qxl-vga.xml |  4 ++--
  tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args |  3 ++-
  tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml  |  4 ++--
  11 files changed, 47 insertions(+), 10 deletions(-)

 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 7ad8aea..4b269c8 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -3569,7 +3569,10 @@ qemu-kvm -net nic,model=? /dev/null
  attribute coderam/code (span class=sincesince
  1.0.2/span) is allowed for qxl type only and specifies
  the size of the primary bar, while codevram/code specifies the
 -secondary bar size.  If ram is not supplied a default value is 
 used.
 +secondary bar size.  If ram or vram are not supplied a default

 Good fix, but it should in a be separate patch.
 
 Yes, split, I'll push this doc change under the trivial rule (unless
 there's a freeze going on).
 

 +value is used. The optional attribute coderevision/code (span
 +class=sincesince 1.0.3/span) specifies the revision of
 +the QXL device, newer revisions provides more functionality.

 s/provides/provide/ if I'm not mistaken
 
 Yes, I agree, changed.
 

/dd
  
dtcodemodel/code/dt
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 049f232..fc78e2d 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -2280,6 +2280,11 @@
ref name=unsignedInt/
  /attribute
/optional
 +  optional
 +attribute name=revision
 +  ref name=unsignedInt/

 This should be  0 according to my experiments with qemu, but I believe
 we don't deal with such nuances in the RNG scheme.
 
 I indeed don't know how to do that, and I couldn't find attributes doing it
 so it's going to stay as is for now ;)
 

 +/attribute
 +  /optional
  /group
/choice
optional
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 5782abb..83be711 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -7391,6 +7391,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node,
  char *vram = NULL;
  char *ram = NULL;
  char *primary = NULL;
 +char *revision = NULL;
  
  if (VIR_ALLOC(def)  0) {
  virReportOOMError();
 @@ -7406,6 +7407,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node,
  ram = virXMLPropString(cur, ram);
  vram = virXMLPropString(cur, vram);
  heads = virXMLPropString(cur, heads);
 +revision = virXMLPropString(cur, revision);

 You're leaking the revision string in here.
 
 Oops, thanks! Bad news is that there are already other leaks there in error
 paths, I'll send patches.
 
 diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
 index 9a9e0b1..81925b1 100644
 --- a/src/conf/domain_conf.h
 +++ b/src/conf/domain_conf.h
 @@ -1160,6 +1160,7 @@ struct _virDomainVideoDef {
  unsigned int ram;  /* kibibytes (multiples of 1024) */
  unsigned int vram; /* kibibytes (multiples of 1024) */
  unsigned int heads;
 +unsigned int revision;
  bool primary;
  virDomainVideoAccelDefPtr accel;
  virDomainDeviceInfo info;
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index f6273c1..e45c808 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -3598,6 +3598,9 @@ qemuBuildDeviceVideoStr(virDomainVideoDefPtr video,
  
  /* QEMU accepts bytes for vram_size. */
  virBufferAsprintf(buf, ,vram_size=%u, video-vram * 1024);
 +
 +if (video-revision != 0)

 you can drop the '!= 0' in here, but that's unimportant.
 
 I prefer the more explicit version (with != 0), I've kept it this way as
 there are other similar if () in the same file.
 

I haven't noticed the other if()s and we have no consistent style for
this, so this doesn't matter, keep it this way.

 I'm a bit lost by your conversation with Alon, do you expect

[libvirt] [PATCH 0/4] Various virsh cleanups and enhancements

2013-03-14 Thread Martin Kletzander

Wanting to resolve rhbz#919372 (fixed in [3/4]) I came accross few
things that could've used some polishing, so here's the series.

Martin Kletzander (4):
  Make vshDebug work when parsing parameters
  Fix snapshot-create-as syntax in help output
  Allow multiple parameters for schedinfo
  Cleanup useless flags specifications

 tools/virsh-domain-monitor.c |  26 
 tools/virsh-domain.c | 274 ---
 tools/virsh-host.c   |  10 --
 tools/virsh-interface.c  |   7 --
 tools/virsh-network.c|  12 --
 tools/virsh-nodedev.c|   3 -
 tools/virsh-pool.c   |  23 
 tools/virsh-secret.c |   4 -
 tools/virsh-snapshot.c   |  61 +-
 tools/virsh-volume.c |  29 -
 tools/virsh.c|  13 +-
 11 files changed, 29 insertions(+), 433 deletions(-)

--
1.8.1.5

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

[libvirt] [PATCH 3/4] Allow multiple parameters for schedinfo

2013-03-14 Thread Martin Kletzander

virsh schedinfo was able to set only one parameter at a time (not
counting the deprecated options), but it is useful to set more at
once, so this patch adds the possibility to do stuff like this:

virsh schedinfo domain cpu_shares=0 vcpu_period=0 vcpu_quota=0 \
emulator_period=0 emulator_quota=0

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919372
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919375

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 tools/virsh-domain.c | 55 ++--
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ab90f58..a7cc9d5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4026,36 +4026,33 @@ static const vshCmdOptDef opts_schedinfo[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_(domain name, id or uuid)
 },
-{.name = set,
- .type = VSH_OT_STRING,
- .flags = VSH_OFLAG_NONE,
- .help = N_(parameter=value)
-},
 {.name = weight,
  .type = VSH_OT_INT,
- .flags = VSH_OFLAG_NONE,
+ .flags = VSH_OFLAG_REQ_OPT,
  .help = N_(weight for XEN_CREDIT)
 },
 {.name = cap,
  .type = VSH_OT_INT,
- .flags = VSH_OFLAG_NONE,
+ .flags = VSH_OFLAG_REQ_OPT,
  .help = N_(cap for XEN_CREDIT)
 },
 {.name = current,
  .type = VSH_OT_BOOL,
- .flags = 0,
  .help = N_(get/set current scheduler info)
 },
 {.name = config,
  .type = VSH_OT_BOOL,
- .flags = 0,
  .help = N_(get/set value to be used on next boot)
 },
 {.name = live,
  .type = VSH_OT_BOOL,
- .flags = 0,
  .help = N_(get/set value from running domain)
 },
+{.name = set,
+ .type = VSH_OT_ARGV,
+ .flags = VSH_OFLAG_NONE,
+ .help = N_(parameter=value)
+},
 {.name = NULL}
 };

@@ -4064,9 +4061,9 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
virTypedParameterPtr src_params, int nsrc_params,
virTypedParameterPtr *update_params)
 {
-const char *set_arg;
 char *set_field = NULL;
 char *set_val = NULL;
+const vshCmdOpt *opt = NULL;
 virTypedParameterPtr param;
 virTypedParameterPtr params = NULL;
 int nparams = 0;
@@ -4076,17 +4073,6 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
 int val;
 int i;

-if (vshCommandOptString(cmd, set, set_arg)  0) {
-set_field = vshStrdup(ctl, set_arg);
-if (!(set_val = strchr(set_field, '='))) {
-vshError(ctl, %s, _(Invalid syntax for --set, expecting 
name=value));
-goto cleanup;
-}
-
-*set_val = '\0';
-set_val++;
-}
-
 for (i = 0; i  nsrc_params; i++) {
 param = (src_params[i]);

@@ -4108,15 +4094,28 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
 continue;
 }

-if (set_field  STREQ(set_field, param-field)) {
-if (virTypedParamsAddFromString(params, nparams, maxparams,
-set_field, param-type,
-set_val)  0) {
-vshSaveLibvirtError();
+opt = NULL;
+while ((opt = vshCommandOptArgv(cmd, opt))) {
+set_field = vshStrdup(ctl, opt-data);
+if (!(set_val = strchr(set_field, '='))) {
+vshError(ctl, %s, _(Invalid syntax for --set, expecting 
name=value));
 goto cleanup;
 }

-continue;
+*set_val = '\0';
+set_val++;
+
+if (STREQ(set_field, param-field)) {
+if (virTypedParamsAddFromString(params, nparams, maxparams,
+set_field, param-type,
+set_val)  0) {
+vshSaveLibvirtError();
+goto cleanup;
+}
+VIR_FREE(set_field);
+break;
+}
+VIR_FREE(set_field);
 }
 }

-- 
1.8.1.5

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

[libvirt] [PATCH 2/4] Fix snapshot-create-as syntax in help output

2013-03-14 Thread Martin Kletzander

According to the man page, the memspec parameter should have the
'--memspec' option mandatory and this is as close as we can get to
that.  What this change does is explained below.

man virsh:

snapshot-create-as ... [[--live] [--memspec memspec]]

virsh help snapshot-create-as before this patch:

  SYNOPSIS
snapshot-create-as ... [memspec] ...
...
  OPTIONS
[--memspec] string ...

virsh help snapshot-create-as after this patch:

  SYNOPSIS
snapshot-create-as ... [--memspec string] ...
...
  OPTIONS
--memspec string ...

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 tools/virsh-snapshot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index d994fd9..c2db99b 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -397,7 +397,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = {
  .help = N_(take a live snapshot)
 },
 {.name = memspec,
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
  .flags = VSH_OFLAG_REQ_OPT,
  .help = N_(memory attributes: [file=]name[,snapshot=type])
 },
-- 
1.8.1.5

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

[libvirt] [PATCH 1/4] Make vshDebug work when parsing parameters

2013-03-14 Thread Martin Kletzander

The vshInit initializes ctl-debug by which vshDebug (which is also
called in vshParseArgv) decides whether to print out the message or
not.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 tools/virsh.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index d822e09..9ed038a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -3100,15 +3100,13 @@ main(int argc, char **argv)
 ctl-name = vshStrdup(ctl, defaultConn);
 }

-if (!vshParseArgv(ctl, argc, argv)) {
+if (!vshInit(ctl)) {
 vshDeinit(ctl);
 exit(EXIT_FAILURE);
 }

-if (!vshInit(ctl)) {
-vshDeinit(ctl);
+if (!vshParseArgv(ctl, argc, argv))
 exit(EXIT_FAILURE);
-}

 if (!ctl-imode) {
 ret = vshCommandRun(ctl, ctl-cmd);
-- 
1.8.1.5

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

Re: [libvirt] [PATCH] Fix size probing for VDI images

2013-03-14 Thread Martin Kletzander

On 03/14/2013 01:45 PM, Ján Tomko wrote:
 Commit 027bf2ea used the wrong offset: the text field at the start
 of the header has 64 bytes, not 68. [1]
 
 Bug: https://bugzilla.redhat.com/show_bug.cgi?id=921452
 
 [1] https://forums.virtualbox.org/viewtopic.php?p=29267#p29267
 ---
 Relevant for v1.0.3-maint.
 
  src/util/virstoragefile.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
 index cbcab5c..aabb5c8 100644
 --- a/src/util/virstoragefile.c
 +++ b/src/util/virstoragefile.c
 @@ -199,7 +199,7 @@ static struct FileTypeInfo const fileTypeInfo[] = {
  [VIR_STORAGE_FILE_VDI] = {
  64, \x7f\x10\xda\xbe, .vdi,
  LV_LITTLE_ENDIAN, 68, 0x00010001,
 -68 + 5 * 4 + 256 + 7 * 4, 8, 1, -1, NULL},
 +64 + 5 * 4 + 256 + 7 * 4, 8, 1, -1, NULL},
  
  /* Not direct file formats, but used for various drivers */
  [VIR_STORAGE_FILE_FAT] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
 

ACK, that was my fault in 027bf2ea, looking back at 'block/vdi.c' in
qemu.git, this is the right number.

Martin

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

Re: [libvirt] [PATCH 1/4] Make vshDebug work when parsing parameters

2013-03-15 Thread Martin Kletzander

On 03/14/2013 06:15 PM, Daniel P. Berrange wrote:
 On Thu, Mar 14, 2013 at 10:27:32AM +0100, Martin Kletzander wrote:
 The vshInit initializes ctl-debug by which vshDebug (which is also
 called in vshParseArgv) decides whether to print out the message or
 not.

 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  tools/virsh.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

 diff --git a/tools/virsh.c b/tools/virsh.c
 index d822e09..9ed038a 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -3100,15 +3100,13 @@ main(int argc, char **argv)
  ctl-name = vshStrdup(ctl, defaultConn);
  }

 -if (!vshParseArgv(ctl, argc, argv)) {
 +if (!vshInit(ctl)) {
  vshDeinit(ctl);
 
 Hmm, we previously called vshDeinit() even though we'd not
 got to vshInit yet !
 
  exit(EXIT_FAILURE);
  }

 -if (!vshInit(ctl)) {
 -vshDeinit(ctl);
 +if (!vshParseArgv(ctl, argc, argv))
 
 But here you've lost the vshDeinit now. I think we need to
 put that back to keep valgrind happy, don't we ? Or is there
 some reason which forced to you drop the vshDeinit here ?
 

No reason, just my fault.  I removed it at first when the vshParseArgv
was before vshInit and then switched those two without adding it back,
thanks for noticing.  This is how the patch should've looked like:

diff --git a/tools/virsh.c b/tools/virsh.c
index d822e09..58a604b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -3100,12 +3100,12 @@ main(int argc, char **argv)
 ctl-name = vshStrdup(ctl, defaultConn);
 }

-if (!vshParseArgv(ctl, argc, argv)) {
+if (!vshInit(ctl)) {
 vshDeinit(ctl);
 exit(EXIT_FAILURE);
 }

-if (!vshInit(ctl)) {
+if (!vshParseArgv(ctl, argc, argv)) {
 vshDeinit(ctl);
 exit(EXIT_FAILURE);
 }
--

  exit(EXIT_FAILURE);
 -}
 
 Regards
 Daniel
 

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

Re: [libvirt] [PATCH 2/4] Fix snapshot-create-as syntax in help output

2013-03-15 Thread Martin Kletzander

On 03/14/2013 06:48 PM, Eric Blake wrote:
 On 03/14/2013 11:16 AM, Daniel P. Berrange wrote:
 On Thu, Mar 14, 2013 at 10:27:33AM +0100, Martin Kletzander wrote:
 According to the man page, the memspec parameter should have the
 '--memspec' option mandatory and this is as close as we can get to
 that.  What this change does is explained below.

 

 diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
 index d994fd9..c2db99b 100644
 --- a/tools/virsh-snapshot.c
 +++ b/tools/virsh-snapshot.c
 @@ -397,7 +397,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = {
   .help = N_(take a live snapshot)
  },
  {.name = memspec,
 - .type = VSH_OT_DATA,
 + .type = VSH_OT_STRING,
   .flags = VSH_OFLAG_REQ_OPT,
   .help = N_(memory attributes: [file=]name[,snapshot=type])
  },

 ACK, looks reasonable, but would like Eric to confirm since he did
 this bit of code.
 
 Concur - this is a reasonable change.
 
 (I'm still not sure why we have VSH_OT_DATA vs. VSH_OT_STRING, as about
 the ONLY place where they make a difference is in help ouput.)
 

Thanks, pushed.

Martin

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

Re: [libvirt] [PATCH 1/4] Make vshDebug work when parsing parameters

2013-03-15 Thread Martin Kletzander

On 03/15/2013 02:17 PM, Daniel P. Berrange wrote:
 On Fri, Mar 15, 2013 at 02:10:08PM +0100, Martin Kletzander wrote:
 On 03/14/2013 06:15 PM, Daniel P. Berrange wrote:
 On Thu, Mar 14, 2013 at 10:27:32AM +0100, Martin Kletzander wrote:
 The vshInit initializes ctl-debug by which vshDebug (which is also
 called in vshParseArgv) decides whether to print out the message or
 not.

 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
[...]
 diff --git a/tools/virsh.c b/tools/virsh.c
 index d822e09..58a604b 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -3100,12 +3100,12 @@ main(int argc, char **argv)
  ctl-name = vshStrdup(ctl, defaultConn);
  }

 -if (!vshParseArgv(ctl, argc, argv)) {
 +if (!vshInit(ctl)) {
  vshDeinit(ctl);
  exit(EXIT_FAILURE);
  }

 -if (!vshInit(ctl)) {
 +if (!vshParseArgv(ctl, argc, argv)) {
  vshDeinit(ctl);
  exit(EXIT_FAILURE);
  }
 
 ACK
 

Thanks, pushed

Martin

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

Re: [libvirt] [PATCH 4/4] Cleanup useless flags specifications

2013-03-15 Thread Martin Kletzander

On 03/14/2013 06:25 PM, Daniel P. Berrange wrote:
 On Thu, Mar 14, 2013 at 10:27:35AM +0100, Martin Kletzander wrote:
 After we switched to C99 initialization, I noticed there were many
 places where the specification of .flags parameter differed.  After
 going through many options and deciding whether to unify the
 initialization to be '.flags = 0' or '.flags = VSH_OFLAG_NONE', I
 realized both can be removed and it makes the code easier to go
 through.

 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  tools/virsh-domain-monitor.c |  26 -
  tools/virsh-domain.c | 221 
 ---
  tools/virsh-host.c   |  10 --
  tools/virsh-interface.c  |   7 --
  tools/virsh-network.c|  12 ---
  tools/virsh-nodedev.c|   3 -
  tools/virsh-pool.c   |  23 -
  tools/virsh-secret.c |   4 -
  tools/virsh-snapshot.c   |  59 
  tools/virsh-volume.c |  29 --
  tools/virsh.c|   7 --
  11 files changed, 401 deletions(-)
 
 ACK
 

Thanks, pushed.

Martin

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

Re: [libvirt] [PATCH 3/4] Allow multiple parameters for schedinfo

2013-03-21 Thread Martin Kletzander

On 03/14/2013 09:53 PM, Eric Blake wrote:
 On 03/14/2013 03:27 AM, Martin Kletzander wrote:
 virsh schedinfo was able to set only one parameter at a time (not
 counting the deprecated options), but it is useful to set more at
 once, so this patch adds the possibility to do stuff like this:

 virsh schedinfo domain cpu_shares=0 vcpu_period=0 vcpu_quota=0 \
 emulator_period=0 emulator_quota=0

 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919372
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919375

 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  tools/virsh-domain.c | 55 
 ++--
  1 file changed, 27 insertions(+), 28 deletions(-)
 
 No change to tools/virsh.pod?  I would have expected something like:
 
 =item Bschedinfo Idomain [[I--config] [I--live] | [I--current]]
 [I--set Bparameter=value]...
 
 +++ b/tools/virsh-domain.c
 @@ -4026,36 +4026,33 @@ static const vshCmdOptDef opts_schedinfo[] = {
   .flags = VSH_OFLAG_REQ,
   .help = N_(domain name, id or uuid)
  },
 -{.name = set,
 - .type = VSH_OT_STRING,
 - .flags = VSH_OFLAG_NONE,
 - .help = N_(parameter=value)
 -},
  {.name = weight,
   .type = VSH_OT_INT,
 - .flags = VSH_OFLAG_NONE,
 + .flags = VSH_OFLAG_REQ_OPT,
   .help = N_(weight for XEN_CREDIT)
 
 Previously, 'schedinfo domain 1' was parsed as --set=1, but then errored
 out because there was no '=' in the argument to set; a user doing weight
 in isolation had to do an explicit --weight=1 to skip the --set field.
 Now that you have re-ordered parameters, but used VSH_OFLAG_REQ_OPT on
 all parameters that got moved before set, a single argument still parses
 as --set, and the user still has to do an explicit --weight=1 to use the
 weight option instead.  That's good - no semantic change for the
 single-argument case.
 
 For the multi-argument case: previously, 'schedinfo domain foo=bar 1'
 was parsed as --set=foo=bar --weight=1, now it will parse as
 --set=foo=bar --set=1 and error out.  But I don't think that anyone was
 relying on mixing old and new syntax (the man page called out --weight
 on a different line than --set), so I can live with that change.
 
 Thus, even though I see a difference in parse, that difference is only
 on a case that users should not have been doing, and I'm happy with your
 patch.
 
 ACK, if you touch up virsh.pod to match.
 

Thanks for that, but I've found out one more inconsistency which is
bothering me a bit (even though it was present there even before this
patch), so I'll be sending a v2 for this one.  This time with the manual
fixed as well.

Martin

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

[libvirt] [PATCH] lxc: Prevent shutting down the host

2013-03-21 Thread Martin Kletzander

When the container has the same '/dev' mount as host (no chroot),
calling domainShutdown(WithFlags) shouldn't shutdown the host it is
running on.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
This is also valid for 1.0.[23]-maint branches, so in case this gets
ACK'd I'll either send a follow-up for those or push it there as well
(if the ACK says so).

 src/lxc/lxc_driver.c | 45 -
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 8603078..ba14db7 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2010-2013 Red Hat, Inc.
  * Copyright IBM Corp. 2008
  *
  * lxc_driver.c: linux container driver functions
@@ -2778,13 +2778,19 @@ lxcDomainShutdownFlags(virDomainPtr dom,
 virLXCDriverPtr driver = dom-conn-privateData;
 virLXCDomainObjPrivatePtr priv;
 virDomainObjPtr vm;
+virDomainFSDefPtr root;
 char *vroot = NULL;
 int ret = -1;
-int rc;
+int rc = 0;
+bool methodSignal;
+bool methodInitctl;

 virCheckFlags(VIR_DOMAIN_SHUTDOWN_INITCTL |
   VIR_DOMAIN_SHUTDOWN_SIGNAL, -1);

+methodSignal = !!(flags  VIR_DOMAIN_SHUTDOWN_SIGNAL);
+methodInitctl = !!(flags  VIR_DOMAIN_SHUTDOWN_INITCTL);
+
 lxcDriverLock(driver);
 vm = virDomainObjListFindByUUID(driver-domains, dom-uuid);
 lxcDriverUnlock(driver);
@@ -2798,6 +2804,7 @@ lxcDomainShutdownFlags(virDomainPtr dom,
 }

 priv = vm-privateData;
+root = virDomainGetRootFilesystem(vm-def);

 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID,
@@ -2817,27 +2824,31 @@ lxcDomainShutdownFlags(virDomainPtr dom,
 goto cleanup;
 }

-if (flags == 0 ||
-(flags  VIR_DOMAIN_SHUTDOWN_INITCTL)) {
-if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
-vroot))  0) {
+if (root  root-src) {
+if (flags == 0)
+methodSignal = methodInitctl = true;
+} else if (methodInitctl) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(Cannot shutdown container using initctl 
+ without separated namespace));
+goto cleanup;
+} else {
+methodSignal = true;
+}
+
+if (methodInitctl) {
+rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, vroot);
+if (rc  0)
 goto cleanup;
-}
-if (rc == 0  flags != 0 
-((flags  ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
+if (rc == 0  !methodSignal) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
_(Container does not provide an initctl pipe));
 goto cleanup;
 }
-} else {
-rc = 0;
 }
-
-if (rc == 0 
-(flags == 0 ||
- (flags  VIR_DOMAIN_SHUTDOWN_SIGNAL))) {
-if (kill(priv-initpid, SIGTERM)  0 
-errno != ESRCH) {
+if (rc == 0  methodSignal) {
+ret = kill(priv-initpid, SIGTERM);
+if (ret  0  errno != ESRCH) {
 virReportSystemError(errno,
  _(Unable to send SIGTERM to init pid %llu),
  (unsigned long long)priv-initpid);
--
1.8.1.5

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

[libvirt] [PATCH] Correct invalid RNG schemas.

2013-03-21 Thread Martin Kletzander

The 'trang' utility, which is able to transform '.rng' files into
'.rnc' files, reported some errors in our schemas that weren't caught
by the tools we use in the build.  I haven't added a test for this,
but the validity can be checked by the following command:

trang -I rng -O rnc domain.rng domain.rnc

There were unescaped minuses in regular expressions and we were
constraining int (which is by default in the range of [-2^31;2^31-1]
to maximum of 2^32.  But what we wanted was exactly an unsignedInt.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Thanks to that, the '.rnc' files can be used by nxml-mode which makes
editing libvirt xml files a *lot* easier.

 docs/schemas/domaincommon.rng |  2 +-
 docs/schemas/nwfilter.rng | 19 ---
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c4e7b7a..3240e1c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3929,7 +3929,7 @@
   /define
   define name='aliasName'
 data type=string
-  param name=pattern[a-zA-Z0-9_-]+/param
+  param name=pattern[a-zA-Z0-9_\-]+/param
 /data
   /define
   define name='alias'
diff --git a/docs/schemas/nwfilter.rng b/docs/schemas/nwfilter.rng
index cfd9ba5..f1aa699 100644
--- a/docs/schemas/nwfilter.rng
+++ b/docs/schemas/nwfilter.rng
@@ -308,25 +308,25 @@
 choice
   valueroot/value
   data type=string
-param name=patternmac[a-zA-Z0-9_\.:-]{0,9}/param
+param name=patternmac[a-zA-Z0-9_\.:\-]{0,9}/param
   /data
   data type=string
-param name=patternstp[a-zA-Z0-9_\.:-]{0,9}/param
+param name=patternstp[a-zA-Z0-9_\.:\-]{0,9}/param
   /data
   data type=string
-param name=patternvlan[a-zA-Z0-9_\.:-]{0,8}/param
+param name=patternvlan[a-zA-Z0-9_\.:\-]{0,8}/param
   /data
   data type=string
-param name=patternarp[a-zA-Z0-9_\.:-]{0,9}/param
+param name=patternarp[a-zA-Z0-9_\.:\-]{0,9}/param
   /data
   data type=string
-param name=patternrarp[a-zA-Z0-9_\.:-]{0,8}/param
+param name=patternrarp[a-zA-Z0-9_\.:\-]{0,8}/param
   /data
   data type=string
-param name=patternipv4[a-zA-Z0-9_\.:-]{0,8}/param
+param name=patternipv4[a-zA-Z0-9_\.:\-]{0,8}/param
   /data
   data type=string
-param name=patternipv6[a-zA-Z0-9_\.:-]{0,8}/param
+param name=patternipv6[a-zA-Z0-9_\.:\-]{0,8}/param
   /data
 /choice
   /attribute
@@ -950,10 +950,7 @@
 param name=pattern0x[0-9a-fA-F]{1,8}/param
   /data

-  data type=int
-param name=minInclusive0/param
-param name=maxInclusive4294967295/param
-  /data
+  data type=unsignedInt/
 /choice
   /define

--
1.8.1.5

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

[libvirt] [PATCH v2] Allow multiple parameters for schedinfo

2013-03-21 Thread Martin Kletzander

virsh schedinfo was able to set only one parameter at a time (not
counting the deprecated options), but it is useful to set more at
once, so this patch adds the possibility to do stuff like this:

virsh schedinfo domain cpu_shares=0 vcpu_period=0 vcpu_quota=0 \
emulator_period=0 emulator_quota=0

Invalid scheduler options are reported as well.  These were previously
reported only if the command hadn't updated any values (when
cmdSchedInfoUpdate returned 0).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=810078
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919372
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919375

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
v2:
 - correctly report unsupported options
 - man page updated

 tests/virsh-schedinfo |   4 +-
 tools/virsh-domain.c  | 119 --
 tools/virsh.pod   |   4 +-
 3 files changed, 72 insertions(+), 55 deletions(-)

diff --git a/tests/virsh-schedinfo b/tests/virsh-schedinfo
index 4f462f8..37f7bd3 100755
--- a/tests/virsh-schedinfo
+++ b/tests/virsh-schedinfo
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Ensure that virsh schedinfo --set invalid=val fails

-# Copyright (C) 2010-2011 Red Hat, Inc.
+# Copyright (C) 2010-2011, 2013 Red Hat, Inc.

 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -37,7 +37,7 @@ fi
 . $srcdir/test-lib.sh

 printf 'Scheduler  : fair\n\n'  exp-out || framework_failure
-printf 'error: invalid scheduler option: j=k\n'  exp-err || framework_failure
+printf 'error: invalid scheduler option: j\n'  exp-err || framework_failure

 fail=0

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 128e516..cc2eddc 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3918,16 +3918,14 @@ static const vshCmdOptDef opts_schedinfo[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_(domain name, id or uuid)
 },
-{.name = set,
- .type = VSH_OT_STRING,
- .help = N_(parameter=value)
-},
 {.name = weight,
  .type = VSH_OT_INT,
+ .flags = VSH_OFLAG_REQ_OPT,
  .help = N_(weight for XEN_CREDIT)
 },
 {.name = cap,
  .type = VSH_OT_INT,
+ .flags = VSH_OFLAG_REQ_OPT,
  .help = N_(cap for XEN_CREDIT)
 },
 {.name = current,
@@ -3942,72 +3940,100 @@ static const vshCmdOptDef opts_schedinfo[] = {
  .type = VSH_OT_BOOL,
  .help = N_(get/set value from running domain)
 },
+{.name = set,
+ .type = VSH_OT_ARGV,
+ .flags = VSH_OFLAG_NONE,
+ .help = N_(parameter=value)
+},
 {.name = NULL}
 };

 static int
+cmdSchedInfoUpdateOne(vshControl *ctl,
+  virTypedParameterPtr src_params, int nsrc_params,
+  virTypedParameterPtr *params,
+  int *nparams, int *maxparams,
+  const char *field, const char *value)
+{
+virTypedParameterPtr param;
+int ret = -1;
+int i;
+
+for (i = 0; i  nsrc_params; i++) {
+param = (src_params[i]);
+
+if (STRNEQ(field, param-field))
+continue;
+
+if (virTypedParamsAddFromString(params, nparams, maxparams,
+field, param-type,
+value)  0) {
+vshSaveLibvirtError();
+goto cleanup;
+}
+ret = 0;
+break;
+}
+
+if (ret  0)
+vshError(ctl, _(invalid scheduler option: %s), field);
+
+ cleanup:
+return ret;
+}
+
+static int
 cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd,
virTypedParameterPtr src_params, int nsrc_params,
virTypedParameterPtr *update_params)
 {
-const char *set_arg;
 char *set_field = NULL;
 char *set_val = NULL;
-virTypedParameterPtr param;
+const char *val = NULL;
+const vshCmdOpt *opt = NULL;
 virTypedParameterPtr params = NULL;
 int nparams = 0;
 int maxparams = 0;
 int ret = -1;
 int rv;
-int val;
-int i;

-if (vshCommandOptString(cmd, set, set_arg)  0) {
-set_field = vshStrdup(ctl, set_arg);
+while ((opt = vshCommandOptArgv(cmd, opt))) {
+set_field = vshStrdup(ctl, opt-data);
 if (!(set_val = strchr(set_field, '='))) {
-vshError(ctl, %s, _(Invalid syntax for --set, expecting 
name=value));
+vshError(ctl, %s, _(Invalid syntax for --set, 
+  expecting name=value));
 goto cleanup;
 }

 *set_val = '\0';
 set_val++;
-}

-for (i = 0; i  nsrc_params; i++) {
-param = (src_params[i]);
-
-/* Legacy 'weight' and 'cap'  parameter */
-if (param-type == VIR_TYPED_PARAM_UINT 
-(STREQ(param-field, weight) || STREQ(param-field, cap)) 
-(rv = vshCommandOptInt(cmd, param-field, val)) != 0) {
-if (rv  0

Re: [libvirt] [PATCH] Correct invalid RNG schemas.

2013-03-21 Thread Martin Kletzander

On 03/21/2013 05:00 PM, Ján Tomko wrote:
 On 03/21/2013 04:28 PM, Martin Kletzander wrote:
 The 'trang' utility, which is able to transform '.rng' files into
 '.rnc' files, reported some errors in our schemas that weren't caught
 by the tools we use in the build.  I haven't added a test for this,
 but the validity can be checked by the following command:

 trang -I rng -O rnc domain.rng domain.rnc

 There were unescaped minuses in regular expressions and we were
 constraining int (which is by default in the range of [-2^31;2^31-1]
 to maximum of 2^32.  But what we wanted was exactly an unsignedInt.

 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
 Thanks to that, the '.rnc' files can be used by nxml-mode which makes
 editing libvirt xml files a *lot* easier.

  docs/schemas/domaincommon.rng |  2 +-
  docs/schemas/nwfilter.rng | 19 ---
  2 files changed, 9 insertions(+), 12 deletions(-)
 
 ACK
 
 Jan
 

Thanks, pushed (and removed the dot at the end of subject).

Martin

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

1 2 3 4 5 6 7 8 9 10 >

1 - 100 of 5738 matches

Mail list logo