[libvirt] [test-API PATCH 0/4] problems fixed during testcase run
I ran a loop of existing testcases to ensure the cleanup that we did these days is neat and successful. The four patches is the final update. first, update testcase config files in cases folder to use new params of testcases. second, Fix a framework bug. third, add missing modules. fourth, final testcases cleanup.
--- cases/basic_interface.conf | 2 +- cases/consumption_cpu_topology.conf | 6 +-- cases/consumption_domain_nfs_start.conf | 7 +--- cases/consumption_eventhandler.conf | 6 +-- cases/consumption_libvirtd.conf | 6 +-- cases/consumption_ownership_test.conf | 7 +--- cases/domain_linux_net_inst.conf | 11 ++---- cases/linux_domain.conf | 48 +++++++++++++------------------ cases/snapshot.conf | 8 ++-- cases/storage_dir.conf | 7 ---- cases/storage_disk.conf | 2 - cases/storage_iscsi.conf | 13 ++++---- cases/storage_logical.conf | 5 --- cases/storage_mpath.conf | 2 - cases/storage_netfs.conf | 13 +------- cases/storage_scsi.conf | 2 - cases/windows_domain.conf | 35 +++++----------------- 17 files changed, 55 insertions(+), 125 deletions(-) diff --git a/cases/basic_interface.conf b/cases/basic_interface.conf index c3a86e0..e2125bb 100644 --- a/cases/basic_interface.conf +++ b/cases/basic_interface.conf @@ -10,7 +10,7 @@ interface:define ifacename $testnic ifacetype - $testnic + ethernet interface:create ifacename diff --git a/cases/consumption_cpu_topology.conf b/cases/consumption_cpu_topology.conf index 1d76256..c02ef2a 100644 --- a/cases/consumption_cpu_topology.conf +++ b/cases/consumption_cpu_topology.conf @@ -1,8 +1,6 @@ domain:install_linux_cdrom guestname $defaultname - virt_type - $defaulthv guestos $defaultos guestarch @@ -11,9 +9,9 @@ domain:install_linux_cdrom $defaultvcpu memory $defaultmem - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic macaddr 54:52:00:45:c3:8a diff --git a/cases/consumption_domain_nfs_start.conf b/cases/consumption_domain_nfs_start.conf index f97cda0..13b2449 100644 --- a/cases/consumption_domain_nfs_start.conf +++ b/cases/consumption_domain_nfs_start.conf @@ -1,8 +1,6 @@ domain:install_linux_cdrom guestname $defaultname - virt_type - $defaulthv guestos $defaultos guestarch @@ -11,9 +9,9 @@ domain:install_linux_cdrom $defaultvcpu memory $defaultmem - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic macaddr 54:52:00:45:c3:8a @@ -30,4 +28,3 @@ sVirt:domain_nfs_start clean options cleanup=enable - diff --git a/cases/consumption_eventhandler.conf b/cases/consumption_eventhandler.conf index 768d616..07bee07 100644 --- a/cases/consumption_eventhandler.conf +++ b/cases/consumption_eventhandler.conf @@ -1,8 +1,6 @@ domain:install_linux_cdrom guestname $defaultname - virt_type - $defaulthv guestos $defaultos guestarch @@ -11,9 +9,9 @@ domain:install_linux_cdrom $defaultvcpu memory $defaultmem - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic macaddr 54:52:00:45:c3:8a diff --git a/cases/consumption_libvirtd.conf b/cases/consumption_libvirtd.conf index 4eb8f86..b0dfff9 100644 --- a/cases/consumption_libvirtd.conf +++ b/cases/consumption_libvirtd.conf @@ -1,8 +1,6 @@ domain:install_linux_cdrom guestname $defaultname - virt_type - $defaulthv guestos $defaultos guestarch @@ -11,9 +9,9 @@ domain:install_linux_cdrom $defaultvcpu memory $defaultmem - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic macaddr 54:52:00:45:c3:8a diff --git a/cases/consumption_ownership_test.conf b/cases/consumption_ownership_test.conf index 58468fd..a906b39 100644 --- a/cases/consumption_ownership_test.conf +++ b/cases/consumption_ownership_test.conf @@ -1,8 +1,6 @@ domain:install_linux_cdrom guestname $defaultname - virt_type - $defaulthv guestos $defaultos guestarch @@ -11,9 +9,9 @@ domain:install_linux_cdrom $defaultvcpu memory $defaultmem - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic macaddr 54:52:00:45:c3:8a @@ -27,4 +25,3 @@ domain:ownership_test enable options cleanup=enable - diff --git a/cases/domain_linux_net_inst.conf b/cases/domain_linux_net_inst.conf index 4c8ab5d..7c97818 100644 --- a/cases/domain_linux_net_inst.conf +++ b/cases/domain_linux_net_inst.conf @@ -1,8 +1,6 @@ domain:install_linux_net guestname $defaultname - virt_type - $defaulthv guestos $defaultos guestarch @@ -13,9 +11,9 @@ domain:install_linux_net $defaultvcpu memory $defaultmem - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic type define @@ -25,9 +23,9 @@ domain:install_linux_check $defaultname virt_type $defaulthv - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic domain:destroy @@ -37,4 +35,3 @@ domain:destroy domain:undefine guestname $defaultname - diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf index 5059c0d..e7d6bac 100644 --- a/cases/linux_domain.conf +++ b/cases/linux_domain.conf @@ -1,8 +1,6 @@ domain:install_linux_cdrom guestname $defaultname - virt_type - $defaulthv guestos $defaultos guestarch @@ -11,9 +9,9 @@ domain:install_linux_cdrom $defaultvcpu memory $defaultmem - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic macaddr 54:52:00:45:c3:8a @@ -23,9 +21,9 @@ domain:install_linux_check $defaultname virt_type $defaulthv - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic domain:shutdown @@ -47,16 +45,16 @@ domain:undefine domain:define guestname $defaultname - virt_type - $defaulthv + diskpath + /var/lib/libvirt/images/libvirt-test-api vcpu - $defaultvcpu + 1 memory - $defaultmem - hdmodel - $defaulthd - nicmodel - $defaultnic + 1048576 + hddriver + virtio + nicdriver + virtio macaddr 54:52:00:45:c3:8a @@ -116,24 +114,18 @@ domain:start domain:attach_disk guestname $defaultname - virt_type - $defaulthv - imagename - attacheddisk - imagesize - 1000 - hdmodel - $defaulthd + imageformat + qcow2 + hddriver + virtio domain:detach_disk guestname $defaultname - virt_type - $defaulthv - imagename - attacheddisk - hdmodel - $defaulthd + imageformat + qcow2 + hddriver + virtio domain:cpu_affinity guestname diff --git a/cases/snapshot.conf b/cases/snapshot.conf index 0557c86..4aac68b 100644 --- a/cases/snapshot.conf +++ b/cases/snapshot.conf @@ -1,8 +1,6 @@ domain:install_linux_cdrom guestname $defaultname - virt_type - $defaulthv guestos $defaultos guestarch @@ -11,12 +9,14 @@ domain:install_linux_cdrom $defaultvcpu memory $defaultmem - hdmodel + hddriver $defaulthd - nicmodel + nicdriver $defaultnic macaddr 54:52:00:45:c3:8a + imageformat + qcow2 domain:shutdown guestname diff --git a/cases/storage_dir.conf b/cases/storage_dir.conf index 5bfc491..dcac700 100644 --- a/cases/storage_dir.conf +++ b/cases/storage_dir.conf @@ -1,10 +1,6 @@ storage:define_dir_pool poolname $defaultpoolname - pooltype - dir - targetpath - $defaultpoolpath storage:build_dir_pool poolname @@ -41,10 +37,7 @@ storage:undefine_pool storage:create_dir_pool poolname $defaultpoolname - pooltype - dir storage:destroy_pool poolname $defaultpoolname - diff --git a/cases/storage_disk.conf b/cases/storage_disk.conf index f758652..d787b77 100644 --- a/cases/storage_disk.conf +++ b/cases/storage_disk.conf @@ -1,8 +1,6 @@ storage:define_disk_pool poolname $defaultpoolname - pooltype - disk sourcepath $defaultdisk diff --git a/cases/storage_iscsi.conf b/cases/storage_iscsi.conf index 7506f13..8ce7da4 100644 --- a/cases/storage_iscsi.conf +++ b/cases/storage_iscsi.conf @@ -1,9 +1,11 @@ +storage:destroy_pool + poolname + $defaultpoolname + storage:define_iscsi_pool poolname $defaultpoolname - pooltype - iscsi - sourcename + sourcehost $iscsi_server sourcepath $iscsi_target @@ -23,9 +25,7 @@ storage:undefine_pool storage:create_iscsi_pool poolname $defaultpoolname - pooltype - iscsi - sourcename + sourcehost $iscsi_server sourcepath $iscsi_target @@ -33,4 +33,3 @@ storage:create_iscsi_pool storage:destroy_pool poolname $defaultpoolname - diff --git a/cases/storage_logical.conf b/cases/storage_logical.conf index baaa4a5..3334abd 100644 --- a/cases/storage_logical.conf +++ b/cases/storage_logical.conf @@ -1,8 +1,6 @@ storage:define_logical_pool poolname $defaultpoolname - pooltype - logical sourcename $defaultpoolname sourcepath @@ -19,8 +17,6 @@ storage:activate_pool storage:create_logical_volume poolname $defaultpoolname - pooltype - logical volname $defaultvolumename capacity @@ -43,4 +39,3 @@ storage:delete_logical_pool storage:undefine_pool poolname $defaultpoolname - diff --git a/cases/storage_mpath.conf b/cases/storage_mpath.conf index 569acdf..4b0f7e2 100644 --- a/cases/storage_mpath.conf +++ b/cases/storage_mpath.conf @@ -1,8 +1,6 @@ storage:define_mpath_pool poolname $defaultpoolname - pooltype - mpath storage:activate_pool poolname diff --git a/cases/storage_netfs.conf b/cases/storage_netfs.conf index 9e17587..e764813 100644 --- a/cases/storage_netfs.conf +++ b/cases/storage_netfs.conf @@ -1,15 +1,10 @@ storage:define_netfs_pool poolname $defaultpoolname - pooltype - netfs - sourcename + sourcehost $nfs_server sourcepath $nfs_folder - targetpath - /tmp/netfs - storage:build_netfs_pool poolname @@ -46,14 +41,10 @@ storage:undefine_pool storage:create_netfs_pool poolname $defaultpoolname - pooltype - netfs - sourcename + sourcehost $nfs_server sourcepath $nfs_folder - targetpath - /tmp/netfs storage:destroy_pool poolname diff --git a/cases/storage_scsi.conf b/cases/storage_scsi.conf index efc94eb..00ae6a2 100644 --- a/cases/storage_scsi.conf +++ b/cases/storage_scsi.conf @@ -1,8 +1,6 @@ storage:define_scsi_pool poolname $defaultpoolname - pooltype - scsi sourcename $defaultscsiname diff --git a/cases/windows_domain.conf b/cases/windows_domain.conf index 6eed3f5..68396a0 100644 --- a/cases/windows_domain.conf +++ b/cases/windows_domain.conf @@ -1,8 +1,6 @@ domain:install_windows_cdrom guestname $defaultname - virt_type - $defaulthv guestos $defaultos guestarch @@ -11,9 +9,9 @@ domain:install_windows_cdrom $defaultvcpu memory $defaultmem - hdmodel + hddriver ide - nicmodel + nicdriver e1000 domain:shutdown @@ -93,23 +91,17 @@ domain:start domain:attach_disk guestname $defaultname - virt_type - $defaulthv - imagename - attacheddisk - imagesize - 1000 - hdmodel + imageformat + qcow2 + hddriver virtio domain:detach_disk guestname $defaultname - virt_type - $defaulthv - imagename - attacheddisk - hdmodel + imageformat + qcow2 + hddriver virtio domain:cpu_affinity @@ -131,14 +123,3 @@ domain:destroy domain:undefine guestname $defaultname - - - - - - - - - - - -- 1.7.7.5
On 2012年04月24日 17:40, Guannan Ren wrote:
--- cases/basic_interface.conf | 2 +- cases/consumption_cpu_topology.conf | 6 +-- cases/consumption_domain_nfs_start.conf | 7 +--- cases/consumption_eventhandler.conf | 6 +-- cases/consumption_libvirtd.conf | 6 +-- cases/consumption_ownership_test.conf | 7 +--- cases/domain_linux_net_inst.conf | 11 ++---- cases/linux_domain.conf | 48 +++++++++++++------------------ cases/snapshot.conf | 8 ++-- cases/storage_dir.conf | 7 ---- cases/storage_disk.conf | 2 - cases/storage_iscsi.conf | 13 ++++---- cases/storage_logical.conf | 5 --- cases/storage_mpath.conf | 2 - cases/storage_netfs.conf | 13 +------- cases/storage_scsi.conf | 2 - cases/windows_domain.conf | 35 +++++----------------- 17 files changed, 55 insertions(+), 125 deletions(-)
Can you explain each change type? I'm not able to review it without that, e.g. why "virt_type" is not needed anymore. Osier
On 04/25/2012 11:00 AM, Osier Yang wrote:
On 2012年04月24日 17:40, Guannan Ren wrote:
--- cases/basic_interface.conf | 2 +- cases/consumption_cpu_topology.conf | 6 +-- cases/consumption_domain_nfs_start.conf | 7 +--- cases/consumption_eventhandler.conf | 6 +-- cases/consumption_libvirtd.conf | 6 +-- cases/consumption_ownership_test.conf | 7 +--- cases/domain_linux_net_inst.conf | 11 ++---- cases/linux_domain.conf | 48 +++++++++++++------------------ cases/snapshot.conf | 8 ++-- cases/storage_dir.conf | 7 ---- cases/storage_disk.conf | 2 - cases/storage_iscsi.conf | 13 ++++---- cases/storage_logical.conf | 5 --- cases/storage_mpath.conf | 2 - cases/storage_netfs.conf | 13 +------- cases/storage_scsi.conf | 2 - cases/windows_domain.conf | 35 +++++----------------- 17 files changed, 55 insertions(+), 125 deletions(-)
Can you explain each change type? I'm not able to review it without that, e.g. why "virt_type" is not needed anymore.
Osier
basic_interface.conf: the original usage is not right, use 'ethernet' for the default ifacetype. consumption_cpu_topology.conf, consumption_domain_nfs_start.conf consumption_eventhandler.conf, consumption_libvirtd.conf consumption_ownership_test.conf, domain_linux_net_inst.conf linux_domain.conf, snapshot.conf The above testcase config file, virt-type is removed because 'kvm' is set to default value. or, we only have xml file of kvm version, 'xen' or other type need to be updated later. For the storage related testcase config files, remove 'pooltype' and 'voltype', they are redundant. Guannan Ren
On 04/25/2012 12:04 PM, Guannan Ren wrote:
On 04/25/2012 11:00 AM, Osier Yang wrote:
On 2012年04月24日 17:40, Guannan Ren wrote:
--- cases/basic_interface.conf | 2 +- cases/consumption_cpu_topology.conf | 6 +-- cases/consumption_domain_nfs_start.conf | 7 +--- cases/consumption_eventhandler.conf | 6 +-- cases/consumption_libvirtd.conf | 6 +-- cases/consumption_ownership_test.conf | 7 +--- cases/domain_linux_net_inst.conf | 11 ++---- cases/linux_domain.conf | 48 +++++++++++++------------------ cases/snapshot.conf | 8 ++-- cases/storage_dir.conf | 7 ---- cases/storage_disk.conf | 2 - cases/storage_iscsi.conf | 13 ++++---- cases/storage_logical.conf | 5 --- cases/storage_mpath.conf | 2 - cases/storage_netfs.conf | 13 +------- cases/storage_scsi.conf | 2 - cases/windows_domain.conf | 35 +++++----------------- 17 files changed, 55 insertions(+), 125 deletions(-)
Can you explain each change type? I'm not able to review it without that, e.g. why "virt_type" is not needed anymore.
Osier
basic_interface.conf: the original usage is not right, use 'ethernet' for the default ifacetype.
consumption_cpu_topology.conf, consumption_domain_nfs_start.conf consumption_eventhandler.conf, consumption_libvirtd.conf consumption_ownership_test.conf, domain_linux_net_inst.conf linux_domain.conf, snapshot.conf
The above testcase config file, virt-type is removed because 'kvm' is set to default value. or, we only have xml file of kvm version, 'xen' or other type need to be updated later. Although 'kvm' is a default hypervisor, If we plan to support other hypervisors, we should reserve virt-type argument, IMO, it will be flexible and easy to extend in the future.
For the storage related testcase config files, remove 'pooltype' and 'voltype', they are redundant. Same reason as above.
Guannan Ren
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/25/2012 01:16 PM, Alex Jia wrote:
On 04/25/2012 12:04 PM, Guannan Ren wrote:
On 04/25/2012 11:00 AM, Osier Yang wrote:
On 2012年04月24日 17:40, Guannan Ren wrote:
--- cases/basic_interface.conf | 2 +- cases/consumption_cpu_topology.conf | 6 +-- cases/consumption_domain_nfs_start.conf | 7 +--- cases/consumption_eventhandler.conf | 6 +-- cases/consumption_libvirtd.conf | 6 +-- cases/consumption_ownership_test.conf | 7 +--- cases/domain_linux_net_inst.conf | 11 ++---- cases/linux_domain.conf | 48 +++++++++++++------------------ cases/snapshot.conf | 8 ++-- cases/storage_dir.conf | 7 ---- cases/storage_disk.conf | 2 - cases/storage_iscsi.conf | 13 ++++---- cases/storage_logical.conf | 5 --- cases/storage_mpath.conf | 2 - cases/storage_netfs.conf | 13 +------- cases/storage_scsi.conf | 2 - cases/windows_domain.conf | 35 +++++----------------- 17 files changed, 55 insertions(+), 125 deletions(-)
Can you explain each change type? I'm not able to review it without that, e.g. why "virt_type" is not needed anymore.
Osier
basic_interface.conf: the original usage is not right, use 'ethernet' for the default ifacetype.
consumption_cpu_topology.conf, consumption_domain_nfs_start.conf consumption_eventhandler.conf, consumption_libvirtd.conf consumption_ownership_test.conf, domain_linux_net_inst.conf linux_domain.conf, snapshot.conf
The above testcase config file, virt-type is removed because 'kvm' is set to default value. or, we only have xml file of kvm version, 'xen' or other type need to be updated later. Although 'kvm' is a default hypervisor, If we plan to support other hypervisors, we should reserve virt-type argument, IMO, it will be flexible and easy to extend in the future.
I agree, this is the result of cleanup work, it make it easier to add support to other hypervisor, so, if we need it, the patch could be easy to write. Guannan Ren
On 04/25/2012 12:04 PM, Guannan Ren wrote:
On 04/25/2012 11:00 AM, Osier Yang wrote:
On 2012年04月24日 17:40, Guannan Ren wrote:
--- cases/basic_interface.conf | 2 +- cases/consumption_cpu_topology.conf | 6 +-- cases/consumption_domain_nfs_start.conf | 7 +--- cases/consumption_eventhandler.conf | 6 +-- cases/consumption_libvirtd.conf | 6 +-- cases/consumption_ownership_test.conf | 7 +--- cases/domain_linux_net_inst.conf | 11 ++---- cases/linux_domain.conf | 48 +++++++++++++------------------ cases/snapshot.conf | 8 ++-- cases/storage_dir.conf | 7 ---- cases/storage_disk.conf | 2 - cases/storage_iscsi.conf | 13 ++++---- cases/storage_logical.conf | 5 --- cases/storage_mpath.conf | 2 - cases/storage_netfs.conf | 13 +------- cases/storage_scsi.conf | 2 - cases/windows_domain.conf | 35 +++++----------------- 17 files changed, 55 insertions(+), 125 deletions(-)
Can you explain each change type? I'm not able to review it without that, e.g. why "virt_type" is not needed anymore.
Osier
basic_interface.conf: the original usage is not right, use 'ethernet' for the default ifacetype.
consumption_cpu_topology.conf, consumption_domain_nfs_start.conf consumption_eventhandler.conf, consumption_libvirtd.conf consumption_ownership_test.conf, domain_linux_net_inst.conf linux_domain.conf, snapshot.conf
The above testcase config file, virt-type is removed because 'kvm' is set to default value. or, we only have xml file of kvm version, 'xen' or other type need to be updated later. Although 'kvm' is a default hypervisor, If we plan to support other hypervisors, we should reserve virt-type argument, IMO, it will be flexible and easy to extend in the future.
For the storage related testcase config files, remove 'pooltype' and 'voltype', they are redundant. Similar reason as above.
Guannan Ren
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
This a bug, if we define a testcase that use xml file such as 'storage:define_dir_pool' more than once, the pop action will result in that the second call to the testcases fails to get xml string in 'xmlstr = params['xml']' Because the 'xml' element is poped out previously --- src/testcasexml.py | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/src/testcasexml.py b/src/testcasexml.py index 742116b..8485d3d 100644 --- a/src/testcasexml.py +++ b/src/testcasexml.py @@ -7,13 +7,12 @@ def xml_file_to_str(proxy_obj, mod_case, case_params): """ get xml string from xml file in case_params return a new case_params with the string in it """ - optional_params = proxy_obj.get_testcase_params(mod_case)[1] if case_params.has_key('xml'): file_name = case_params.pop('xml') elif optional_params.has_key('xml'): - file_name = optional_params.pop('xml') + file_name = optional_params['xml'] else: return None @@ -34,8 +33,6 @@ def xml_file_to_str(proxy_obj, mod_case, case_params): else: raise exception.FileDoesNotExist("xml file %s doesn't exist" % xml_file_path) - optional_params = proxy_obj.get_testcase_params(mod_case)[1] - # replace the params that in testcase.conf first for (key, value) in case_params.items(): -- 1.7.7.5
s/don't/Don't/ On 2012年04月24日 17:40, Guannan Ren wrote:
This a bug, if we define a testcase that use xml file such as
s/use/uses/
'storage:define_dir_pool' more than once, the pop action will result in that the second call to the testcases fails to get
s/testcases/testcase/, s/fails/fail/
xml string in 'xmlstr = params['xml']' Because the 'xml' element is poped out previously
Well, how about a compcat one: Don't pop "xml" from the list, as it will still be used afterwards.
--- src/testcasexml.py | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/src/testcasexml.py b/src/testcasexml.py index 742116b..8485d3d 100644 --- a/src/testcasexml.py +++ b/src/testcasexml.py
Honestly, I'm really disappointed with the filename. :-),
@@ -7,13 +7,12 @@ def xml_file_to_str(proxy_obj, mod_case, case_params): """ get xml string from xml file in case_params return a new case_params with the string in it """ - optional_params = proxy_obj.get_testcase_params(mod_case)[1]
if case_params.has_key('xml'): file_name = case_params.pop('xml') elif optional_params.has_key('xml'): - file_name = optional_params.pop('xml') + file_name = optional_params['xml'] else: return None
@@ -34,8 +33,6 @@ def xml_file_to_str(proxy_obj, mod_case, case_params): else: raise exception.FileDoesNotExist("xml file %s doesn't exist" % xml_file_path)
- optional_params = proxy_obj.get_testcase_params(mod_case)[1] - # replace the params that in testcase.conf first for (key, value) in case_params.items():
ACK with the commit message either shortened, or fixed. Osier
On 04/25/2012 11:11 AM, Osier Yang wrote:
s/don't/Don't/
On 2012年04月24日 17:40, Guannan Ren wrote:
This a bug, if we define a testcase that use xml file such as
s/use/uses/
'storage:define_dir_pool' more than once, the pop action will result in that the second call to the testcases fails to get
s/testcases/testcase/, s/fails/fail/
xml string in 'xmlstr = params['xml']' Because the 'xml' element is poped out previously
Well, how about a compcat one:
Don't pop "xml" from the list, as it will still be used afterwards.
--- src/testcasexml.py | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/src/testcasexml.py b/src/testcasexml.py index 742116b..8485d3d 100644 --- a/src/testcasexml.py +++ b/src/testcasexml.py
Honestly, I'm really disappointed with the filename. :-),
Change it as you want, it is hard for me to give a nice name :) It is used to get the xml file (from testcase file or use the default) replace key words with value, pass the xml string to testcases.
--- utils/utils.py | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/utils/utils.py b/utils/utils.py index a216fa7..455e9cf 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -23,6 +23,8 @@ import random import commands import socket import fcntl +import pty +import signal import struct import pexpect import string -- 1.7.7.5
On 2012年04月24日 17:40, Guannan Ren wrote:
--- utils/utils.py | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/utils/utils.py b/utils/utils.py index a216fa7..455e9cf 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -23,6 +23,8 @@ import random import commands import socket import fcntl +import pty +import signal import struct import pexpect import string
ACK
install_linux_cdrom.py: qemu process couldn't visit custom.iso that resides in other user's home, so we copy custom.iso into /tmp for easy use. install_linux_check.py: use hddriver and nicdriver name network/create.py: in the case of the network with 'isolate' type, we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files network/define.py: the same as network/create.py repos/snapshot/delete.py: the TESTCASE_check is reserved fuction name for framework, so change the name of its internal 'check' function --- repos/domain/install_linux_cdrom.py | 14 +++++++++++--- repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..17052d4 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -47,13 +47,18 @@ def prepare_cdrom(*args): ks_name = os.path.basename(ks) new_dir = os.path.join(HOME_PATH, guestname) + customeiso_dir = os.path.join('/tmp/libvirt-test-API', guestname) logger.info("creating a new folder for customizing custom.iso file in it") if os.path.exists(new_dir): logger.info("the folder exists, remove it") shutil.rmtree(new_dir) + if os.path.exists(customeiso_dir): + shutil.rmtree(customeiso_dir) + os.makedirs(new_dir) + os.makedirs(customeiso_dir) logger.info("the directory is %s" % new_dir) boot_path = os.path.join(ostree, 'images/boot.iso') @@ -76,8 +81,11 @@ def prepare_cdrom(*args): (status, text) = commands.getstatusoutput(shell_cmd) logger.debug(text) - logger.info("make custom.iso file, change to original directory: %s" % - src_path) + + logger.info("copy custom.iso to %s" % customeiso_dir) + shutil.copy('custom.iso', customeiso_dir) + + logger.info("go back to original directory: %s" % src_path) os.chdir(src_path) def prepare_boot_guest(domobj, xmlstr, guestname, installtype, logger): @@ -196,7 +204,7 @@ def install_linux_cdrom(params): logger.info('prepare installation...') bootcd = '%s/custom.iso' % \ - (os.path.join(HOME_PATH, guestname)) + (os.path.join('/tmp/libvirt-test-API', guestname)) logger.debug("the bootcd path is %s" % bootcd) logger.info("begin to customize the custom.iso file") prepare_cdrom(ostree, ks, guestname, logger) diff --git a/repos/domain/install_linux_check.py b/repos/domain/install_linux_check.py index d034aba..ab1e7db 100644 --- a/repos/domain/install_linux_check.py +++ b/repos/domain/install_linux_check.py @@ -15,7 +15,7 @@ from src import sharedmod from src import env_parser from utils import utils -required_params = ('guestname', 'virt_type', 'hdmodel', 'nicmodel',) +required_params = ('guestname', 'virt_type', 'hddriver', 'nicdriver',) optional_params = {} HOME_PATH = os.getcwd() @@ -71,8 +71,8 @@ def install_linux_check(params): logger.info("Now checking guest health after installation") domain_name=guestname - blk_type=params['hdmodel'] - nic_type=params['nicmodel'] + blk_type=params['hddriver'] + nic_type=params['nicdriver'] Test_Result = 0 # Ping guest from host diff --git a/repos/network/create.py b/repos/network/create.py index 839e93b..399328c 100644 --- a/repos/network/create.py +++ b/repos/network/create.py @@ -39,6 +39,7 @@ def create(params): """Create a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml'] conn = sharedmod.libvirtobj['conn'] @@ -47,6 +48,9 @@ def create(params): logger.error("the %s network is running" % networkname) return 1 + if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("%s network xml:\n%s" % (networkname, xmlstr)) net_num1 = conn.numOfNetworks() diff --git a/repos/network/define.py b/repos/network/define.py index 923db29..0c38944 100644 --- a/repos/network/define.py +++ b/repos/network/define.py @@ -42,14 +42,18 @@ def define(params): """Define a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml'] conn = sharedmod.libvirtobj['conn'] if check_network_define(networkname, logger): - logger.error("%s network is defined" % networkname) + logger.error("%s network is defined already" % networkname) return 1 + if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("network xml:\n%s" % xmlstr) net_num1 = conn.numOfDefinedNetworks() diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py index 19689b1..49ce4d9 100644 --- a/repos/snapshot/delete.py +++ b/repos/snapshot/delete.py @@ -24,7 +24,7 @@ def check_domain_state(conn, guestname, logger): else: return True -def delete_check(guestname, snapshotname, expected_flag, logger): +def delete_checkout(guestname, snapshotname, expected_flag, logger): """ after deleting, check if appropriate xml file exists or not""" guest_snapshot_dir = os.path.join(SNAPSHOT_DIR, guestname) snapshot_entries = os.listdir(guest_snapshot_dir) @@ -54,7 +54,7 @@ def delete(params): logger.error("checking failed") return 1 - if not delete_check(guestname, snapshotname, "exist", logger): + if not delete_checkout(guestname, snapshotname, "exist", logger): logger.error("no snapshot %s exists" % snapshotname) logger.debug("not corresponding xml file in %s" % SNAPSHOT_DIR) return 1 @@ -64,7 +64,7 @@ def delete(params): domobj = conn.lookupByName(guestname) snapobj = domobj.snapshotLookupByName(snapshotname, 0) snapobj.delete(0) - if not delete_check(guestname, snapshotname, "noexist", logger): + if not delete_checkout(guestname, snapshotname, "noexist", logger): logger.error("after deleting, the corresponding \ xmlfile still exists in %s" % SNAPSHOT_DIR) return 1 -- 1.7.7.5
On 2012年04月24日 17:40, Guannan Ren wrote:
install_linux_cdrom.py: qemu process couldn't visit custom.iso that resides in other user's home, so we copy custom.iso into /tmp for easy use.
We need something like /var/cache/libvirt-test-API as a global var in global.cfg.
install_linux_check.py: use hddriver and nicdriver name
Why? I guess the old "hdmode" and "nicmode" is not correct to represent the actual argument, but it should be explained.
network/create.py: in the case of the network with 'isolate' type, we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files network/define.py: the same as network/create.py repos/snapshot/delete.py: the TESTCASE_check is reserved fuction name for framework, so change the name of its internal 'check' function --- repos/domain/install_linux_cdrom.py | 14 +++++++++++--- repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..17052d4 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -47,13 +47,18 @@ def prepare_cdrom(*args): ks_name = os.path.basename(ks)
new_dir = os.path.join(HOME_PATH, guestname) + customeiso_dir = os.path.join('/tmp/libvirt-test-API', guestname)
typo, custom. And in anycase, it should use the new global var in global.cfg.
logger.info("creating a new folder for customizing custom.iso file in it")
if os.path.exists(new_dir): logger.info("the folder exists, remove it") shutil.rmtree(new_dir)
+ if os.path.exists(customeiso_dir): + shutil.rmtree(customeiso_dir) +
Why the dir should be deleted if exists? shouldn't we reuse it?
os.makedirs(new_dir) + os.makedirs(customeiso_dir) logger.info("the directory is %s" % new_dir)
boot_path = os.path.join(ostree, 'images/boot.iso') @@ -76,8 +81,11 @@ def prepare_cdrom(*args): (status, text) = commands.getstatusoutput(shell_cmd)
logger.debug(text) - logger.info("make custom.iso file, change to original directory: %s" % - src_path) + + logger.info("copy custom.iso to %s" % customeiso_dir) + shutil.copy('custom.iso', customeiso_dir) +
Unreasonable. Why not create the iso directly in the dir, instead of creating it anoter place, and copying it to the desired place afterwards?
+ logger.info("go back to original directory: %s" % src_path)
We should cleanup the logging like this to logger.debug. Nobody will care about how the scripts work in background I think.
os.chdir(src_path)
def prepare_boot_guest(domobj, xmlstr, guestname, installtype, logger): @@ -196,7 +204,7 @@ def install_linux_cdrom(params): logger.info('prepare installation...')
bootcd = '%s/custom.iso' % \ - (os.path.join(HOME_PATH, guestname)) + (os.path.join('/tmp/libvirt-test-API', guestname))
Well, hardcode again.
logger.debug("the bootcd path is %s" % bootcd) logger.info("begin to customize the custom.iso file") prepare_cdrom(ostree, ks, guestname, logger) diff --git a/repos/domain/install_linux_check.py b/repos/domain/install_linux_check.py index d034aba..ab1e7db 100644 --- a/repos/domain/install_linux_check.py +++ b/repos/domain/install_linux_check.py @@ -15,7 +15,7 @@ from src import sharedmod from src import env_parser from utils import utils
-required_params = ('guestname', 'virt_type', 'hdmodel', 'nicmodel',) +required_params = ('guestname', 'virt_type', 'hddriver', 'nicdriver',)
Finally I see why "hdmodel" and "nicmodel" are changed in 1/4, 1/4 should come after this patch, or either you break the 1/4 into several patches, the one about paramas' name changing comes after this patch.
optional_params = {}
HOME_PATH = os.getcwd() @@ -71,8 +71,8 @@ def install_linux_check(params): logger.info("Now checking guest health after installation")
domain_name=guestname - blk_type=params['hdmodel'] - nic_type=params['nicmodel'] + blk_type=params['hddriver'] + nic_type=params['nicdriver'] Test_Result = 0
# Ping guest from host diff --git a/repos/network/create.py b/repos/network/create.py index 839e93b..399328c 100644 --- a/repos/network/create.py +++ b/repos/network/create.py @@ -39,6 +39,7 @@ def create(params): """Create a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn'] @@ -47,6 +48,9 @@ def create(params): logger.error("the %s network is running" % networkname) return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) +
This looks fine.
logger.debug("%s network xml:\n%s" % (networkname, xmlstr))
net_num1 = conn.numOfNetworks() diff --git a/repos/network/define.py b/repos/network/define.py index 923db29..0c38944 100644 --- a/repos/network/define.py +++ b/repos/network/define.py @@ -42,14 +42,18 @@ def define(params): """Define a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn']
if check_network_define(networkname, logger): - logger.error("%s network is defined" % networkname) + logger.error("%s network is defined already" % networkname)
"network '%s' is already defined" % networkname
return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("network xml:\n%s" % xmlstr)
net_num1 = conn.numOfDefinedNetworks() diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py index 19689b1..49ce4d9 100644 --- a/repos/snapshot/delete.py +++ b/repos/snapshot/delete.py @@ -24,7 +24,7 @@ def check_domain_state(conn, guestname, logger): else: return True
-def delete_check(guestname, snapshotname, expected_flag, logger): +def delete_checkout(guestname, snapshotname, expected_flag, logger):
Not a good name. Still wondering why not use "check" in scripts under repo, and in framework, using "$script.check".
""" after deleting, check if appropriate xml file exists or not""" guest_snapshot_dir = os.path.join(SNAPSHOT_DIR, guestname) snapshot_entries = os.listdir(guest_snapshot_dir) @@ -54,7 +54,7 @@ def delete(params): logger.error("checking failed") return 1
- if not delete_check(guestname, snapshotname, "exist", logger): + if not delete_checkout(guestname, snapshotname, "exist", logger): logger.error("no snapshot %s exists" % snapshotname) logger.debug("not corresponding xml file in %s" % SNAPSHOT_DIR) return 1 @@ -64,7 +64,7 @@ def delete(params): domobj = conn.lookupByName(guestname) snapobj = domobj.snapshotLookupByName(snapshotname, 0) snapobj.delete(0) - if not delete_check(guestname, snapshotname, "noexist", logger): + if not delete_checkout(guestname, snapshotname, "noexist", logger): logger.error("after deleting, the corresponding \ xmlfile still exists in %s" % SNAPSHOT_DIR) return 1
V2 is needed. Osier
On 04/25/2012 11:35 AM, Osier Yang wrote:
On 2012年04月24日 17:40, Guannan Ren wrote:
install_linux_cdrom.py: qemu process couldn't visit custom.iso that resides in other user's home, so we copy custom.iso into /tmp for easy use.
We need something like /var/cache/libvirt-test-API as a global var in global.cfg.
install_linux_check.py: use hddriver and nicdriver name
Why? I guess the old "hdmode" and "nicmode" is not correct to represent the actual argument, but it should be explained.
network/create.py: in the case of the network with 'isolate' type, we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files network/define.py: the same as network/create.py repos/snapshot/delete.py: the TESTCASE_check is reserved fuction name for framework, so change the name of its internal 'check' function --- repos/domain/install_linux_cdrom.py | 14 +++++++++++--- repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..17052d4 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -47,13 +47,18 @@ def prepare_cdrom(*args): ks_name = os.path.basename(ks)
new_dir = os.path.join(HOME_PATH, guestname) + customeiso_dir = os.path.join('/tmp/libvirt-test-API', guestname)
typo, custom. And in anycase, it should use the new global var in global.cfg.
logger.info("creating a new folder for customizing custom.iso file in it")
if os.path.exists(new_dir): logger.info("the folder exists, remove it") shutil.rmtree(new_dir)
+ if os.path.exists(customeiso_dir): + shutil.rmtree(customeiso_dir) +
Why the dir should be deleted if exists? shouldn't we reuse it?
We download boot.iso first, and add kickstart file into it recreate a new iso file named custom.iso. We can not tell it is rhel6u1 or rhel6u2 from the custom.iso, so remove it before creating a new one.
os.makedirs(new_dir) + os.makedirs(customeiso_dir) logger.info("the directory is %s" % new_dir)
boot_path = os.path.join(ostree, 'images/boot.iso') @@ -76,8 +81,11 @@ def prepare_cdrom(*args): (status, text) = commands.getstatusoutput(shell_cmd)
logger.debug(text) - logger.info("make custom.iso file, change to original directory: %s" % - src_path) + + logger.info("copy custom.iso to %s" % customeiso_dir) + shutil.copy('custom.iso', customeiso_dir) +
Unreasonable. Why not create the iso directly in the dir, instead of creating it anoter place, and copying it to the desired place afterwards?
Every time we install a new guest, we create a folder named after the name of the guest in root of libvirt-test-API. All of the related temporary files will be generated in this folder. The custom.iso is created in this folder. Probably user git clone libvirt-test-API testsuits in his/her home directory. The qemu process couldn't visit the custom.iso as the permission problem, the code is to copy the custom.iso into /tmp for qemu process.
+ logger.info("go back to original directory: %s" % src_path)
We should cleanup the logging like this to logger.debug. Nobody will care about how the scripts work in background I think.
os.chdir(src_path)
def prepare_boot_guest(domobj, xmlstr, guestname, installtype, logger): @@ -196,7 +204,7 @@ def install_linux_cdrom(params): logger.info('prepare installation...')
bootcd = '%s/custom.iso' % \ - (os.path.join(HOME_PATH, guestname)) + (os.path.join('/tmp/libvirt-test-API', guestname))
Well, hardcode again.
logger.debug("the bootcd path is %s" % bootcd) logger.info("begin to customize the custom.iso file") prepare_cdrom(ostree, ks, guestname, logger) diff --git a/repos/domain/install_linux_check.py b/repos/domain/install_linux_check.py index d034aba..ab1e7db 100644 --- a/repos/domain/install_linux_check.py +++ b/repos/domain/install_linux_check.py @@ -15,7 +15,7 @@ from src import sharedmod from src import env_parser from utils import utils
-required_params = ('guestname', 'virt_type', 'hdmodel', 'nicmodel',) +required_params = ('guestname', 'virt_type', 'hddriver', 'nicdriver',)
Finally I see why "hdmodel" and "nicmodel" are changed in 1/4, 1/4 should come after this patch, or either you break the 1/4 into several patches, the one about paramas' name changing comes after this patch.
optional_params = {}
HOME_PATH = os.getcwd() @@ -71,8 +71,8 @@ def install_linux_check(params): logger.info("Now checking guest health after installation")
domain_name=guestname - blk_type=params['hdmodel'] - nic_type=params['nicmodel'] + blk_type=params['hddriver'] + nic_type=params['nicdriver'] Test_Result = 0
# Ping guest from host diff --git a/repos/network/create.py b/repos/network/create.py index 839e93b..399328c 100644 --- a/repos/network/create.py +++ b/repos/network/create.py @@ -39,6 +39,7 @@ def create(params): """Create a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn'] @@ -47,6 +48,9 @@ def create(params): logger.error("the %s network is running" % networkname) return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) +
This looks fine.
logger.debug("%s network xml:\n%s" % (networkname, xmlstr))
net_num1 = conn.numOfNetworks() diff --git a/repos/network/define.py b/repos/network/define.py index 923db29..0c38944 100644 --- a/repos/network/define.py +++ b/repos/network/define.py @@ -42,14 +42,18 @@ def define(params): """Define a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn']
if check_network_define(networkname, logger): - logger.error("%s network is defined" % networkname) + logger.error("%s network is defined already" % networkname)
"network '%s' is already defined" % networkname
return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("network xml:\n%s" % xmlstr)
net_num1 = conn.numOfDefinedNetworks() diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py index 19689b1..49ce4d9 100644 --- a/repos/snapshot/delete.py +++ b/repos/snapshot/delete.py @@ -24,7 +24,7 @@ def check_domain_state(conn, guestname, logger): else: return True
-def delete_check(guestname, snapshotname, expected_flag, logger): +def delete_checkout(guestname, snapshotname, expected_flag, logger):
Not a good name. Still wondering why not use "check" in scripts under repo, and in framework, using "$script.check".
I don't think it is a good name either, in v2 it is changed to 'delete_checking' currently, the TESTCASE_check is the name supported by framework to check something before starting test, if just "check" is better than "TESTCASE_check" we can change it.
""" after deleting, check if appropriate xml file exists or not""" guest_snapshot_dir = os.path.join(SNAPSHOT_DIR, guestname) snapshot_entries = os.listdir(guest_snapshot_dir) @@ -54,7 +54,7 @@ def delete(params): logger.error("checking failed") return 1
- if not delete_check(guestname, snapshotname, "exist", logger): + if not delete_checkout(guestname, snapshotname, "exist", logger): logger.error("no snapshot %s exists" % snapshotname) logger.debug("not corresponding xml file in %s" % SNAPSHOT_DIR) return 1 @@ -64,7 +64,7 @@ def delete(params): domobj = conn.lookupByName(guestname) snapobj = domobj.snapshotLookupByName(snapshotname, 0) snapobj.delete(0) - if not delete_check(guestname, snapshotname, "noexist", logger): + if not delete_checkout(guestname, snapshotname, "noexist", logger): logger.error("after deleting, the corresponding \ xmlfile still exists in %s" % SNAPSHOT_DIR) return 1
V2 is needed.
Osier
On 04/25/2012 12:23 PM, Guannan Ren wrote:
On 04/25/2012 11:35 AM, Osier Yang wrote:
On 2012年04月24日 17:40, Guannan Ren wrote:
install_linux_cdrom.py: qemu process couldn't visit custom.iso that resides in other user's home, so we copy custom.iso into /tmp for easy use.
We need something like /var/cache/libvirt-test-API as a global var in global.cfg.
install_linux_check.py: use hddriver and nicdriver name
Why? I guess the old "hdmode" and "nicmode" is not correct to represent the actual argument, but it should be explained.
network/create.py: in the case of the network with 'isolate' type, we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files network/define.py: the same as network/create.py repos/snapshot/delete.py: the TESTCASE_check is reserved fuction name for framework, so change the name of its internal 'check' function --- repos/domain/install_linux_cdrom.py | 14 +++++++++++--- repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..17052d4 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -47,13 +47,18 @@ def prepare_cdrom(*args): ks_name = os.path.basename(ks)
new_dir = os.path.join(HOME_PATH, guestname) + customeiso_dir = os.path.join('/tmp/libvirt-test-API', guestname)
typo, custom. And in anycase, it should use the new global var in global.cfg.
logger.info("creating a new folder for customizing custom.iso file in it")
if os.path.exists(new_dir): logger.info("the folder exists, remove it") shutil.rmtree(new_dir)
+ if os.path.exists(customeiso_dir): + shutil.rmtree(customeiso_dir) +
Why the dir should be deleted if exists? shouldn't we reuse it?
We download boot.iso first, and add kickstart file into it recreate a new iso file named custom.iso. We can not tell it is rhel6u1 or rhel6u2 from the custom.iso, so remove it before creating a new one. Maybe, we may use a unique name to identify ISO name to avoid overwrite existing ISO file such as $name+$uuid.iso, if so, we need also to let other caller know ISO name instead of default custom.iso.
os.makedirs(new_dir) + os.makedirs(customeiso_dir) logger.info("the directory is %s" % new_dir)
boot_path = os.path.join(ostree, 'images/boot.iso') @@ -76,8 +81,11 @@ def prepare_cdrom(*args): (status, text) = commands.getstatusoutput(shell_cmd)
logger.debug(text) - logger.info("make custom.iso file, change to original directory: %s" % - src_path) + + logger.info("copy custom.iso to %s" % customeiso_dir) + shutil.copy('custom.iso', customeiso_dir) +
Unreasonable. Why not create the iso directly in the dir, instead of creating it anoter place, and copying it to the desired place afterwards?
Every time we install a new guest, we create a folder named after the name of the guest in root of libvirt-test-API. All of the related temporary files will be generated in this folder. The custom.iso is created in this folder. Probably user git clone libvirt-test-API testsuits in his/her home directory. The qemu process couldn't visit the custom.iso as the permission problem, the code is to copy the custom.iso into /tmp for qemu process.
+ logger.info("go back to original directory: %s" % src_path)
We should cleanup the logging like this to logger.debug. Nobody will care about how the scripts work in background I think.
os.chdir(src_path)
def prepare_boot_guest(domobj, xmlstr, guestname, installtype, logger): @@ -196,7 +204,7 @@ def install_linux_cdrom(params): logger.info('prepare installation...')
bootcd = '%s/custom.iso' % \ - (os.path.join(HOME_PATH, guestname)) + (os.path.join('/tmp/libvirt-test-API', guestname))
Well, hardcode again.
logger.debug("the bootcd path is %s" % bootcd) logger.info("begin to customize the custom.iso file") prepare_cdrom(ostree, ks, guestname, logger) diff --git a/repos/domain/install_linux_check.py b/repos/domain/install_linux_check.py index d034aba..ab1e7db 100644 --- a/repos/domain/install_linux_check.py +++ b/repos/domain/install_linux_check.py @@ -15,7 +15,7 @@ from src import sharedmod from src import env_parser from utils import utils
-required_params = ('guestname', 'virt_type', 'hdmodel', 'nicmodel',) +required_params = ('guestname', 'virt_type', 'hddriver', 'nicdriver',)
Finally I see why "hdmodel" and "nicmodel" are changed in 1/4, 1/4 should come after this patch, or either you break the 1/4 into several patches, the one about paramas' name changing comes after this patch.
optional_params = {}
HOME_PATH = os.getcwd() @@ -71,8 +71,8 @@ def install_linux_check(params): logger.info("Now checking guest health after installation")
domain_name=guestname - blk_type=params['hdmodel'] - nic_type=params['nicmodel'] + blk_type=params['hddriver'] + nic_type=params['nicdriver'] Test_Result = 0
# Ping guest from host diff --git a/repos/network/create.py b/repos/network/create.py index 839e93b..399328c 100644 --- a/repos/network/create.py +++ b/repos/network/create.py @@ -39,6 +39,7 @@ def create(params): """Create a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn'] @@ -47,6 +48,9 @@ def create(params): logger.error("the %s network is running" % networkname) return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) +
This looks fine.
logger.debug("%s network xml:\n%s" % (networkname, xmlstr))
net_num1 = conn.numOfNetworks() diff --git a/repos/network/define.py b/repos/network/define.py index 923db29..0c38944 100644 --- a/repos/network/define.py +++ b/repos/network/define.py @@ -42,14 +42,18 @@ def define(params): """Define a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn']
if check_network_define(networkname, logger): - logger.error("%s network is defined" % networkname) + logger.error("%s network is defined already" % networkname)
"network '%s' is already defined" % networkname
return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("network xml:\n%s" % xmlstr)
net_num1 = conn.numOfDefinedNetworks() diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py index 19689b1..49ce4d9 100644 --- a/repos/snapshot/delete.py +++ b/repos/snapshot/delete.py @@ -24,7 +24,7 @@ def check_domain_state(conn, guestname, logger): else: return True
-def delete_check(guestname, snapshotname, expected_flag, logger): +def delete_checkout(guestname, snapshotname, expected_flag, logger):
Not a good name. Still wondering why not use "check" in scripts under repo, and in framework, using "$script.check".
I don't think it is a good name either, in v2 it is changed to 'delete_checking' currently, the TESTCASE_check is the name supported by framework to check something before starting test, if just "check" is better than "TESTCASE_check" we can change it.
""" after deleting, check if appropriate xml file exists or not""" guest_snapshot_dir = os.path.join(SNAPSHOT_DIR, guestname) snapshot_entries = os.listdir(guest_snapshot_dir) @@ -54,7 +54,7 @@ def delete(params): logger.error("checking failed") return 1
- if not delete_check(guestname, snapshotname, "exist", logger): + if not delete_checkout(guestname, snapshotname, "exist", logger): logger.error("no snapshot %s exists" % snapshotname) logger.debug("not corresponding xml file in %s" % SNAPSHOT_DIR) return 1 @@ -64,7 +64,7 @@ def delete(params): domobj = conn.lookupByName(guestname) snapobj = domobj.snapshotLookupByName(snapshotname, 0) snapobj.delete(0) - if not delete_check(guestname, snapshotname, "noexist", logger): + if not delete_checkout(guestname, snapshotname, "noexist", logger): logger.error("after deleting, the corresponding \ xmlfile still exists in %s" % SNAPSHOT_DIR) return 1
V2 is needed.
Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 2012年04月25日 12:23, Guannan Ren wrote:
On 04/25/2012 11:35 AM, Osier Yang wrote:
On 2012年04月24日 17:40, Guannan Ren wrote:
install_linux_cdrom.py: qemu process couldn't visit custom.iso that resides in other user's home, so we copy custom.iso into /tmp for easy use.
We need something like /var/cache/libvirt-test-API as a global var in global.cfg.
install_linux_check.py: use hddriver and nicdriver name
Why? I guess the old "hdmode" and "nicmode" is not correct to represent the actual argument, but it should be explained.
network/create.py: in the case of the network with 'isolate' type, we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files network/define.py: the same as network/create.py repos/snapshot/delete.py: the TESTCASE_check is reserved fuction name for framework, so change the name of its internal 'check' function --- repos/domain/install_linux_cdrom.py | 14 +++++++++++--- repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..17052d4 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -47,13 +47,18 @@ def prepare_cdrom(*args): ks_name = os.path.basename(ks)
new_dir = os.path.join(HOME_PATH, guestname) + customeiso_dir = os.path.join('/tmp/libvirt-test-API', guestname)
typo, custom. And in anycase, it should use the new global var in global.cfg.
logger.info("creating a new folder for customizing custom.iso file in it")
if os.path.exists(new_dir): logger.info("the folder exists, remove it") shutil.rmtree(new_dir)
+ if os.path.exists(customeiso_dir): + shutil.rmtree(customeiso_dir) +
Why the dir should be deleted if exists? shouldn't we reuse it?
We download boot.iso first, and add kickstart file into it recreate a new iso file named custom.iso. We can not tell it is rhel6u1 or rhel6u2 from the custom.iso, so remove it before creating a new one.
I mean why not change the utils/ksiso.sh to accept a argument, and create the custom.iso in the desired target dir, such as /var/cache/libvirt-test-API/$guest/
os.makedirs(new_dir) + os.makedirs(customeiso_dir) logger.info("the directory is %s" % new_dir)
boot_path = os.path.join(ostree, 'images/boot.iso') @@ -76,8 +81,11 @@ def prepare_cdrom(*args): (status, text) = commands.getstatusoutput(shell_cmd)
logger.debug(text) - logger.info("make custom.iso file, change to original directory: %s" % - src_path) + + logger.info("copy custom.iso to %s" % customeiso_dir) + shutil.copy('custom.iso', customeiso_dir) +
Unreasonable. Why not create the iso directly in the dir, instead of creating it anoter place, and copying it to the desired place afterwards?
Every time we install a new guest, we create a folder named after the name of the guest in root of libvirt-test-API. All of the related temporary files will be generated in this folder. The custom.iso is created in this folder.
I mean throw all of these stuffs into /var/cache/libvirt-test-API/$guest. Probably user git clone libvirt-test-API testsuits in his/her home
directory. The qemu process couldn't visit the custom.iso as the permission problem, the code is to copy the custom.iso into /tmp for qemu process.
+ logger.info("go back to original directory: %s" % src_path)
We should cleanup the logging like this to logger.debug. Nobody will care about how the scripts work in background I think.
os.chdir(src_path)
def prepare_boot_guest(domobj, xmlstr, guestname, installtype, logger): @@ -196,7 +204,7 @@ def install_linux_cdrom(params): logger.info('prepare installation...')
bootcd = '%s/custom.iso' % \ - (os.path.join(HOME_PATH, guestname)) + (os.path.join('/tmp/libvirt-test-API', guestname))
Well, hardcode again.
logger.debug("the bootcd path is %s" % bootcd) logger.info("begin to customize the custom.iso file") prepare_cdrom(ostree, ks, guestname, logger) diff --git a/repos/domain/install_linux_check.py b/repos/domain/install_linux_check.py index d034aba..ab1e7db 100644 --- a/repos/domain/install_linux_check.py +++ b/repos/domain/install_linux_check.py @@ -15,7 +15,7 @@ from src import sharedmod from src import env_parser from utils import utils
-required_params = ('guestname', 'virt_type', 'hdmodel', 'nicmodel',) +required_params = ('guestname', 'virt_type', 'hddriver', 'nicdriver',)
Finally I see why "hdmodel" and "nicmodel" are changed in 1/4, 1/4 should come after this patch, or either you break the 1/4 into several patches, the one about paramas' name changing comes after this patch.
optional_params = {}
HOME_PATH = os.getcwd() @@ -71,8 +71,8 @@ def install_linux_check(params): logger.info("Now checking guest health after installation")
domain_name=guestname - blk_type=params['hdmodel'] - nic_type=params['nicmodel'] + blk_type=params['hddriver'] + nic_type=params['nicdriver'] Test_Result = 0
# Ping guest from host diff --git a/repos/network/create.py b/repos/network/create.py index 839e93b..399328c 100644 --- a/repos/network/create.py +++ b/repos/network/create.py @@ -39,6 +39,7 @@ def create(params): """Create a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn'] @@ -47,6 +48,9 @@ def create(params): logger.error("the %s network is running" % networkname) return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) +
This looks fine.
logger.debug("%s network xml:\n%s" % (networkname, xmlstr))
net_num1 = conn.numOfNetworks() diff --git a/repos/network/define.py b/repos/network/define.py index 923db29..0c38944 100644 --- a/repos/network/define.py +++ b/repos/network/define.py @@ -42,14 +42,18 @@ def define(params): """Define a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn']
if check_network_define(networkname, logger): - logger.error("%s network is defined" % networkname) + logger.error("%s network is defined already" % networkname)
"network '%s' is already defined" % networkname
return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("network xml:\n%s" % xmlstr)
net_num1 = conn.numOfDefinedNetworks() diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py index 19689b1..49ce4d9 100644 --- a/repos/snapshot/delete.py +++ b/repos/snapshot/delete.py @@ -24,7 +24,7 @@ def check_domain_state(conn, guestname, logger): else: return True
-def delete_check(guestname, snapshotname, expected_flag, logger): +def delete_checkout(guestname, snapshotname, expected_flag, logger):
Not a good name. Still wondering why not use "check" in scripts under repo, and in framework, using "$script.check".
I don't think it is a good name either, in v2 it is changed to 'delete_checking' currently, the TESTCASE_check is the name supported by framework to check something before starting test, if just "check" is better than "TESTCASE_check" we can change it.
""" after deleting, check if appropriate xml file exists or not""" guest_snapshot_dir = os.path.join(SNAPSHOT_DIR, guestname) snapshot_entries = os.listdir(guest_snapshot_dir) @@ -54,7 +54,7 @@ def delete(params): logger.error("checking failed") return 1
- if not delete_check(guestname, snapshotname, "exist", logger): + if not delete_checkout(guestname, snapshotname, "exist", logger): logger.error("no snapshot %s exists" % snapshotname) logger.debug("not corresponding xml file in %s" % SNAPSHOT_DIR) return 1 @@ -64,7 +64,7 @@ def delete(params): domobj = conn.lookupByName(guestname) snapobj = domobj.snapshotLookupByName(snapshotname, 0) snapobj.delete(0) - if not delete_check(guestname, snapshotname, "noexist", logger): + if not delete_checkout(guestname, snapshotname, "noexist", logger): logger.error("after deleting, the corresponding \ xmlfile still exists in %s" % SNAPSHOT_DIR) return 1
V2 is needed.
Osier
On 04/25/2012 04:10 PM, Osier Yang wrote:
On 2012年04月25日 12:23, Guannan Ren wrote:
On 04/25/2012 11:35 AM, Osier Yang wrote:
On 2012年04月24日 17:40, Guannan Ren wrote:
install_linux_cdrom.py: qemu process couldn't visit custom.iso that resides in other user's home, so we copy custom.iso into /tmp for easy use.
We need something like /var/cache/libvirt-test-API as a global var in global.cfg.
install_linux_check.py: use hddriver and nicdriver name
Why? I guess the old "hdmode" and "nicmode" is not correct to represent the actual argument, but it should be explained.
network/create.py: in the case of the network with 'isolate' type, we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files network/define.py: the same as network/create.py repos/snapshot/delete.py: the TESTCASE_check is reserved fuction name for framework, so change the name of its internal 'check' function --- repos/domain/install_linux_cdrom.py | 14 +++++++++++--- repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..17052d4 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -47,13 +47,18 @@ def prepare_cdrom(*args): ks_name = os.path.basename(ks)
new_dir = os.path.join(HOME_PATH, guestname) + customeiso_dir = os.path.join('/tmp/libvirt-test-API', guestname)
typo, custom. And in anycase, it should use the new global var in global.cfg.
logger.info("creating a new folder for customizing custom.iso file in it")
if os.path.exists(new_dir): logger.info("the folder exists, remove it") shutil.rmtree(new_dir)
+ if os.path.exists(customeiso_dir): + shutil.rmtree(customeiso_dir) +
Why the dir should be deleted if exists? shouldn't we reuse it?
We download boot.iso first, and add kickstart file into it recreate a new iso file named custom.iso. We can not tell it is rhel6u1 or rhel6u2 from the custom.iso, so remove it before creating a new one.
I mean why not change the utils/ksiso.sh to accept a argument, and create the custom.iso in the desired target dir, such as /var/cache/libvirt-test-API/$guest/
We can do that, if that is a better way. Currently, we keep all of this stuff in local folder for debugging or checking. If we have good reason to move the folder to /var/cache/libvirt-test-API it's ok for me.
Every time we install a new guest, we create a folder named after the name of the guest in root of libvirt-test-API. All of the related temporary files will be generated in this folder. The custom.iso is created in this folder.
I mean throw all of these stuffs into /var/cache/libvirt-test-API/$guest.
install_linux_cdrom.py: use global variable with value /var/cache/libvirt-test-API copy custom.iso into it for qemu process install_linux_check.py: use hddriver and nicdriver name network/create.py: in the case of the network with 'isolate' type we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files network/define.py: Same as network/create.py snapshot/delete.py: The TESTCASE_check is reserved function name for framework, so change the internal checking function to check_xml() --- repos/domain/install_linux_cdrom.py | 20 ++++++++++++++------ repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 29 insertions(+), 13 deletions(-) diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..e46491b 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -36,7 +36,7 @@ VM_STAT = "virsh --quiet list --all| grep \"\\b%s\\b\"|grep off" VM_DESTROY = "virsh destroy %s" VM_UNDEFINE = "virsh undefine %s" -BOOT_DIR = "/var/lib/libvirt/boot/" +BOOT_ISO_DIR = "/var/cache/libvirt-test-api" HOME_PATH = os.getcwd() def prepare_cdrom(*args): @@ -47,13 +47,18 @@ def prepare_cdrom(*args): ks_name = os.path.basename(ks) new_dir = os.path.join(HOME_PATH, guestname) + customiso_dir = os.path.join(BOOT_ISO_DIR, guestname) logger.info("creating a new folder for customizing custom.iso file in it") if os.path.exists(new_dir): logger.info("the folder exists, remove it") shutil.rmtree(new_dir) + if os.path.exists(customiso_dir): + shutil.rmtree(customiso_dir) + os.makedirs(new_dir) + os.makedirs(customiso_dir) logger.info("the directory is %s" % new_dir) boot_path = os.path.join(ostree, 'images/boot.iso') @@ -65,10 +70,10 @@ def prepare_cdrom(*args): urllib.urlretrieve(ks, '%s/%s' % (new_dir, ks_name))[0] logger.info("the url of kickstart is %s" % ks) - shutil.copy('utils/ksiso.sh', new_dir) + shutil.debug('utils/ksiso.sh', new_dir) src_path = os.getcwd() - logger.info("enter into the workshop folder: %s" % new_dir) + logger.debug("enter into the workshop folder: %s" % new_dir) os.chdir(new_dir) shell_cmd = 'sh ksiso.sh %s' % ks_name @@ -76,8 +81,11 @@ def prepare_cdrom(*args): (status, text) = commands.getstatusoutput(shell_cmd) logger.debug(text) - logger.info("make custom.iso file, change to original directory: %s" % - src_path) + + logger.debug("copy custom.iso to %s" % customiso_dir) + shutil.copy('custom.iso', customiso_dir) + + logger.debug("go back to original directory: %s" % src_path) os.chdir(src_path) def prepare_boot_guest(domobj, xmlstr, guestname, installtype, logger): @@ -196,7 +204,7 @@ def install_linux_cdrom(params): logger.info('prepare installation...') bootcd = '%s/custom.iso' % \ - (os.path.join(HOME_PATH, guestname)) + (os.path.join(BOOT_ISO_DIR, guestname)) logger.debug("the bootcd path is %s" % bootcd) logger.info("begin to customize the custom.iso file") prepare_cdrom(ostree, ks, guestname, logger) diff --git a/repos/domain/install_linux_check.py b/repos/domain/install_linux_check.py index d034aba..ab1e7db 100644 --- a/repos/domain/install_linux_check.py +++ b/repos/domain/install_linux_check.py @@ -15,7 +15,7 @@ from src import sharedmod from src import env_parser from utils import utils -required_params = ('guestname', 'virt_type', 'hdmodel', 'nicmodel',) +required_params = ('guestname', 'virt_type', 'hddriver', 'nicdriver',) optional_params = {} HOME_PATH = os.getcwd() @@ -71,8 +71,8 @@ def install_linux_check(params): logger.info("Now checking guest health after installation") domain_name=guestname - blk_type=params['hdmodel'] - nic_type=params['nicmodel'] + blk_type=params['hddriver'] + nic_type=params['nicdriver'] Test_Result = 0 # Ping guest from host diff --git a/repos/network/create.py b/repos/network/create.py index 839e93b..399328c 100644 --- a/repos/network/create.py +++ b/repos/network/create.py @@ -39,6 +39,7 @@ def create(params): """Create a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml'] conn = sharedmod.libvirtobj['conn'] @@ -47,6 +48,9 @@ def create(params): logger.error("the %s network is running" % networkname) return 1 + if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("%s network xml:\n%s" % (networkname, xmlstr)) net_num1 = conn.numOfNetworks() diff --git a/repos/network/define.py b/repos/network/define.py index 923db29..dd054f7 100644 --- a/repos/network/define.py +++ b/repos/network/define.py @@ -42,14 +42,18 @@ def define(params): """Define a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml'] conn = sharedmod.libvirtobj['conn'] if check_network_define(networkname, logger): - logger.error("%s network is defined" % networkname) + logger.error("%s network is already defined" % networkname) return 1 + if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("network xml:\n%s" % xmlstr) net_num1 = conn.numOfDefinedNetworks() diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py index 19689b1..bab043e 100644 --- a/repos/snapshot/delete.py +++ b/repos/snapshot/delete.py @@ -24,7 +24,7 @@ def check_domain_state(conn, guestname, logger): else: return True -def delete_check(guestname, snapshotname, expected_flag, logger): +def check_xml(guestname, snapshotname, expected_flag, logger): """ after deleting, check if appropriate xml file exists or not""" guest_snapshot_dir = os.path.join(SNAPSHOT_DIR, guestname) snapshot_entries = os.listdir(guest_snapshot_dir) @@ -54,7 +54,7 @@ def delete(params): logger.error("checking failed") return 1 - if not delete_check(guestname, snapshotname, "exist", logger): + if not check_xml(guestname, snapshotname, "exist", logger): logger.error("no snapshot %s exists" % snapshotname) logger.debug("not corresponding xml file in %s" % SNAPSHOT_DIR) return 1 @@ -64,7 +64,7 @@ def delete(params): domobj = conn.lookupByName(guestname) snapobj = domobj.snapshotLookupByName(snapshotname, 0) snapobj.delete(0) - if not delete_check(guestname, snapshotname, "noexist", logger): + if not check_xml(guestname, snapshotname, "noexist", logger): logger.error("after deleting, the corresponding \ xmlfile still exists in %s" % SNAPSHOT_DIR) return 1 -- 1.7.7.5
On 2012年04月25日 13:23, Guannan Ren wrote:
install_linux_cdrom.py: use global variable with value /var/cache/libvirt-test-API copy custom.iso into it for qemu process
install_linux_check.py: use hddriver and nicdriver name
network/create.py: in the case of the network with 'isolate' type we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files
network/define.py: Same as network/create.py
snapshot/delete.py: The TESTCASE_check is reserved function name for framework, so change the internal checking function to check_xml() --- repos/domain/install_linux_cdrom.py | 20 ++++++++++++++------ repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..e46491b 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -36,7 +36,7 @@ VM_STAT = "virsh --quiet list --all| grep \"\\b%s\\b\"|grep off" VM_DESTROY = "virsh destroy %s" VM_UNDEFINE = "virsh undefine %s"
-BOOT_DIR = "/var/lib/libvirt/boot/" +BOOT_ISO_DIR = "/var/cache/libvirt-test-api"
As replied in previous mail. We should have this in global.cfg, and use it instead of create a $source_root/$guestname for each guest installation.
HOME_PATH = os.getcwd()
def prepare_cdrom(*args): @@ -47,13 +47,18 @@ def prepare_cdrom(*args): ks_name = os.path.basename(ks)
new_dir = os.path.join(HOME_PATH, guestname) + customiso_dir = os.path.join(BOOT_ISO_DIR, guestname) logger.info("creating a new folder for customizing custom.iso file in it")
if os.path.exists(new_dir): logger.info("the folder exists, remove it") shutil.rmtree(new_dir)
+ if os.path.exists(customiso_dir): + shutil.rmtree(customiso_dir) + os.makedirs(new_dir) + os.makedirs(customiso_dir)
And if ksiso.sh is changed to accept an argument, codes like this is not neccessary.
logger.info("the directory is %s" % new_dir)
boot_path = os.path.join(ostree, 'images/boot.iso') @@ -65,10 +70,10 @@ def prepare_cdrom(*args): urllib.urlretrieve(ks, '%s/%s' % (new_dir, ks_name))[0] logger.info("the url of kickstart is %s" % ks)
- shutil.copy('utils/ksiso.sh', new_dir) + shutil.debug('utils/ksiso.sh', new_dir) src_path = os.getcwd()
- logger.info("enter into the workshop folder: %s" % new_dir) + logger.debug("enter into the workshop folder: %s" % new_dir) os.chdir(new_dir) shell_cmd = 'sh ksiso.sh %s' % ks_name
@@ -76,8 +81,11 @@ def prepare_cdrom(*args): (status, text) = commands.getstatusoutput(shell_cmd)
logger.debug(text) - logger.info("make custom.iso file, change to original directory: %s" % - src_path) + + logger.debug("copy custom.iso to %s" % customiso_dir) + shutil.copy('custom.iso', customiso_dir) + + logger.debug("go back to original directory: %s" % src_path) os.chdir(src_path)
def prepare_boot_guest(domobj, xmlstr, guestname, installtype, logger): @@ -196,7 +204,7 @@ def install_linux_cdrom(params): logger.info('prepare installation...')
bootcd = '%s/custom.iso' % \ - (os.path.join(HOME_PATH, guestname)) + (os.path.join(BOOT_ISO_DIR, guestname)) logger.debug("the bootcd path is %s" % bootcd) logger.info("begin to customize the custom.iso file") prepare_cdrom(ostree, ks, guestname, logger) diff --git a/repos/domain/install_linux_check.py b/repos/domain/install_linux_check.py index d034aba..ab1e7db 100644 --- a/repos/domain/install_linux_check.py +++ b/repos/domain/install_linux_check.py @@ -15,7 +15,7 @@ from src import sharedmod from src import env_parser from utils import utils
-required_params = ('guestname', 'virt_type', 'hdmodel', 'nicmodel',) +required_params = ('guestname', 'virt_type', 'hddriver', 'nicdriver',) optional_params = {}
HOME_PATH = os.getcwd() @@ -71,8 +71,8 @@ def install_linux_check(params): logger.info("Now checking guest health after installation")
domain_name=guestname - blk_type=params['hdmodel'] - nic_type=params['nicmodel'] + blk_type=params['hddriver'] + nic_type=params['nicdriver'] Test_Result = 0
# Ping guest from host diff --git a/repos/network/create.py b/repos/network/create.py index 839e93b..399328c 100644 --- a/repos/network/create.py +++ b/repos/network/create.py @@ -39,6 +39,7 @@ def create(params): """Create a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn'] @@ -47,6 +48,9 @@ def create(params): logger.error("the %s network is running" % networkname) return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("%s network xml:\n%s" % (networkname, xmlstr))
net_num1 = conn.numOfNetworks() diff --git a/repos/network/define.py b/repos/network/define.py index 923db29..dd054f7 100644 --- a/repos/network/define.py +++ b/repos/network/define.py @@ -42,14 +42,18 @@ def define(params): """Define a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn']
if check_network_define(networkname, logger): - logger.error("%s network is defined" % networkname) + logger.error("%s network is already defined" % networkname) return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("network xml:\n%s" % xmlstr)
net_num1 = conn.numOfDefinedNetworks() diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py index 19689b1..bab043e 100644 --- a/repos/snapshot/delete.py +++ b/repos/snapshot/delete.py @@ -24,7 +24,7 @@ def check_domain_state(conn, guestname, logger): else: return True
-def delete_check(guestname, snapshotname, expected_flag, logger): +def check_xml(guestname, snapshotname, expected_flag, logger): """ after deleting, check if appropriate xml file exists or not""" guest_snapshot_dir = os.path.join(SNAPSHOT_DIR, guestname) snapshot_entries = os.listdir(guest_snapshot_dir) @@ -54,7 +54,7 @@ def delete(params): logger.error("checking failed") return 1
- if not delete_check(guestname, snapshotname, "exist", logger): + if not check_xml(guestname, snapshotname, "exist", logger): logger.error("no snapshot %s exists" % snapshotname) logger.debug("not corresponding xml file in %s" % SNAPSHOT_DIR) return 1 @@ -64,7 +64,7 @@ def delete(params): domobj = conn.lookupByName(guestname) snapobj = domobj.snapshotLookupByName(snapshotname, 0) snapobj.delete(0) - if not delete_check(guestname, snapshotname, "noexist", logger): + if not check_xml(guestname, snapshotname, "noexist", logger): logger.error("after deleting, the corresponding \ xmlfile still exists in %s" % SNAPSHOT_DIR) return 1
Others look good, but we will need a v3 for the /var/cache/libvirt-test-api. Osier
On 04/25/2012 04:13 PM, Osier Yang wrote:
On 2012年04月25日 13:23, Guannan Ren wrote:
install_linux_cdrom.py: use global variable with value /var/cache/libvirt-test-API copy custom.iso into it for qemu process
install_linux_check.py: use hddriver and nicdriver name
network/create.py: in the case of the network with 'isolate' type we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files
network/define.py: Same as network/create.py
snapshot/delete.py: The TESTCASE_check is reserved function name for framework, so change the internal checking function to check_xml() --- repos/domain/install_linux_cdrom.py | 20 ++++++++++++++------ repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..e46491b 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -36,7 +36,7 @@ VM_STAT = "virsh --quiet list --all| grep \"\\b%s\\b\"|grep off" VM_DESTROY = "virsh destroy %s" VM_UNDEFINE = "virsh undefine %s"
-BOOT_DIR = "/var/lib/libvirt/boot/" +BOOT_ISO_DIR = "/var/cache/libvirt-test-api"
As replied in previous mail. We should have this in global.cfg, and use it instead of create a $source_root/$guestname for each guest installation.
The variable is only used by this testcase, not by framework do you think it is right to set this variable in global.cfg?
HOME_PATH = os.getcwd()
def prepare_cdrom(*args): @@ -47,13 +47,18 @@ def prepare_cdrom(*args): ks_name = os.path.basename(ks)
new_dir = os.path.join(HOME_PATH, guestname) + customiso_dir = os.path.join(BOOT_ISO_DIR, guestname) logger.info("creating a new folder for customizing custom.iso file in it")
if os.path.exists(new_dir): logger.info("the folder exists, remove it") shutil.rmtree(new_dir)
+ if os.path.exists(customiso_dir): + shutil.rmtree(customiso_dir) + os.makedirs(new_dir) + os.makedirs(customiso_dir)
And if ksiso.sh is changed to accept an argument, codes like this is not neccessary.
logger.info("the directory is %s" % new_dir)
boot_path = os.path.join(ostree, 'images/boot.iso') @@ -65,10 +70,10 @@ def prepare_cdrom(*args): urllib.urlretrieve(ks, '%s/%s' % (new_dir, ks_name))[0] logger.info("the url of kickstart is %s" % ks)
- shutil.copy('utils/ksiso.sh', new_dir) + shutil.debug('utils/ksiso.sh', new_dir) src_path = os.getcwd()
- logger.info("enter into the workshop folder: %s" % new_dir) + logger.debug("enter into the workshop folder: %s" % new_dir) os.chdir(new_dir) shell_cmd = 'sh ksiso.sh %s' % ks_name
@@ -76,8 +81,11 @@ def prepare_cdrom(*args): (status, text) = commands.getstatusoutput(shell_cmd)
logger.debug(text) - logger.info("make custom.iso file, change to original directory: %s" % - src_path) + + logger.debug("copy custom.iso to %s" % customiso_dir) + shutil.copy('custom.iso', customiso_dir) + + logger.debug("go back to original directory: %s" % src_path) os.chdir(src_path)
def prepare_boot_guest(domobj, xmlstr, guestname, installtype, logger): @@ -196,7 +204,7 @@ def install_linux_cdrom(params): logger.info('prepare installation...')
bootcd = '%s/custom.iso' % \ - (os.path.join(HOME_PATH, guestname)) + (os.path.join(BOOT_ISO_DIR, guestname)) logger.debug("the bootcd path is %s" % bootcd) logger.info("begin to customize the custom.iso file") prepare_cdrom(ostree, ks, guestname, logger) diff --git a/repos/domain/install_linux_check.py b/repos/domain/install_linux_check.py index d034aba..ab1e7db 100644 --- a/repos/domain/install_linux_check.py +++ b/repos/domain/install_linux_check.py @@ -15,7 +15,7 @@ from src import sharedmod from src import env_parser from utils import utils
-required_params = ('guestname', 'virt_type', 'hdmodel', 'nicmodel',) +required_params = ('guestname', 'virt_type', 'hddriver', 'nicdriver',) optional_params = {}
HOME_PATH = os.getcwd() @@ -71,8 +71,8 @@ def install_linux_check(params): logger.info("Now checking guest health after installation")
domain_name=guestname - blk_type=params['hdmodel'] - nic_type=params['nicmodel'] + blk_type=params['hddriver'] + nic_type=params['nicdriver'] Test_Result = 0
# Ping guest from host diff --git a/repos/network/create.py b/repos/network/create.py index 839e93b..399328c 100644 --- a/repos/network/create.py +++ b/repos/network/create.py @@ -39,6 +39,7 @@ def create(params): """Create a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn'] @@ -47,6 +48,9 @@ def create(params): logger.error("the %s network is running" % networkname) return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("%s network xml:\n%s" % (networkname, xmlstr))
net_num1 = conn.numOfNetworks() diff --git a/repos/network/define.py b/repos/network/define.py index 923db29..dd054f7 100644 --- a/repos/network/define.py +++ b/repos/network/define.py @@ -42,14 +42,18 @@ def define(params): """Define a network from xml""" logger = params['logger'] networkname = params['networkname'] + netmode = params['netmode'] xmlstr = params['xml']
conn = sharedmod.libvirtobj['conn']
if check_network_define(networkname, logger): - logger.error("%s network is defined" % networkname) + logger.error("%s network is already defined" % networkname) return 1
+ if netmode == 'isolate': + xmlstr = re.sub('<forward.*\n', '', xmlstr) + logger.debug("network xml:\n%s" % xmlstr)
net_num1 = conn.numOfDefinedNetworks() diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py index 19689b1..bab043e 100644 --- a/repos/snapshot/delete.py +++ b/repos/snapshot/delete.py @@ -24,7 +24,7 @@ def check_domain_state(conn, guestname, logger): else: return True
-def delete_check(guestname, snapshotname, expected_flag, logger): +def check_xml(guestname, snapshotname, expected_flag, logger): """ after deleting, check if appropriate xml file exists or not""" guest_snapshot_dir = os.path.join(SNAPSHOT_DIR, guestname) snapshot_entries = os.listdir(guest_snapshot_dir) @@ -54,7 +54,7 @@ def delete(params): logger.error("checking failed") return 1
- if not delete_check(guestname, snapshotname, "exist", logger): + if not check_xml(guestname, snapshotname, "exist", logger): logger.error("no snapshot %s exists" % snapshotname) logger.debug("not corresponding xml file in %s" % SNAPSHOT_DIR) return 1 @@ -64,7 +64,7 @@ def delete(params): domobj = conn.lookupByName(guestname) snapobj = domobj.snapshotLookupByName(snapshotname, 0) snapobj.delete(0) - if not delete_check(guestname, snapshotname, "noexist", logger): + if not check_xml(guestname, snapshotname, "noexist", logger): logger.error("after deleting, the corresponding \ xmlfile still exists in %s" % SNAPSHOT_DIR) return 1
Others look good, but we will need a v3 for the /var/cache/libvirt-test-api.
Osier
On 2012年04月25日 16:27, Guannan Ren wrote:
On 04/25/2012 04:13 PM, Osier Yang wrote:
On 2012年04月25日 13:23, Guannan Ren wrote:
install_linux_cdrom.py: use global variable with value /var/cache/libvirt-test-API copy custom.iso into it for qemu process
install_linux_check.py: use hddriver and nicdriver name
network/create.py: in the case of the network with 'isolate' type we need to remove '<forward mode="NETMODE"/>'line, The bug is caused by changes on using xml files
network/define.py: Same as network/create.py
snapshot/delete.py: The TESTCASE_check is reserved function name for framework, so change the internal checking function to check_xml() --- repos/domain/install_linux_cdrom.py | 20 ++++++++++++++------ repos/domain/install_linux_check.py | 6 +++--- repos/network/create.py | 4 ++++ repos/network/define.py | 6 +++++- repos/snapshot/delete.py | 6 +++--- 5 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py index 60d12a7..e46491b 100644 --- a/repos/domain/install_linux_cdrom.py +++ b/repos/domain/install_linux_cdrom.py @@ -36,7 +36,7 @@ VM_STAT = "virsh --quiet list --all| grep \"\\b%s\\b\"|grep off" VM_DESTROY = "virsh destroy %s" VM_UNDEFINE = "virsh undefine %s"
-BOOT_DIR = "/var/lib/libvirt/boot/" +BOOT_ISO_DIR = "/var/cache/libvirt-test-api"
As replied in previous mail. We should have this in global.cfg, and use it instead of create a $source_root/$guestname for each guest installation.
The variable is only used by this testcase, not by framework do you think it is right to set this variable in global.cfg?
Yeah, that's why it's named "global.cfg", actually many variables defined in it are for case. e.g. $defaulthv..... Osier
participants (3)
-
Alex Jia -
Guannan Ren -
Osier Yang