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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list