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, 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())
>
> - if guestname not in guest_names:
> + if guestname not in domain_names:
> logger.error("%s is not running or does not exist" % guestname)
> return False
> else:
> diff --git a/repos/domain/domain_uuid.py b/repos/domain/domain_uuid.py
> index e66c3ee..0d3996c 100644
> --- a/repos/domain/domain_uuid.py
> +++ b/repos/domain/domain_uuid.py
> @@ -18,15 +18,15 @@ VIRSH_DOMUUID = "virsh domuuid"
>
> 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/domblkinfo.py b/repos/domain/domblkinfo.py
> index b3c3c3e..557b651 100644
> --- a/repos/domain/domblkinfo.py
> +++ b/repos/domain/domblkinfo.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/eventhandler.py b/repos/domain/eventhandler.py
> index 1b0c579..47c9642 100644
> --- a/repos/domain/eventhandler.py
> +++ b/repos/domain/eventhandler.py
> @@ -43,13 +43,13 @@ def detailToString(event, detail):
>
> def check_domain_running(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())
>
> - if guestname not in guest_names:
> + if guestname not in domain_names:
> logger.error("%s doesn't exist or not running" % guestname)
> return 1
> else:
> diff --git a/repos/domain/install_linux_cdrom.py b/repos/domain/install_linux_cdrom.py
> index 1d5fb28..7f0019e 100644
> --- a/repos/domain/install_linux_cdrom.py
> +++ b/repos/domain/install_linux_cdrom.py
> @@ -310,13 +310,13 @@ def install_linux_cdrom(params):
> interval += 10
> logger.info('%s seconds passed away...' % interval)
> elif installtype == 'create':
> - 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.info("guest installation of create type is complete.")
> logger.info("define the vm and boot it up")
> ret = prepare_boot_guest(domobj, params, logger, installtype)
> diff --git a/repos/domain/install_linux_net.py b/repos/domain/install_linux_net.py
> index a47bcea..de8ff0a 100644
> --- a/repos/domain/install_linux_net.py
> +++ b/repos/domain/install_linux_net.py
> @@ -312,13 +312,13 @@ def install_linux_net(params):
> interval += 10
> logger.info('%s seconds passed away...' % interval)
> elif installtype == 'create':
> - 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.info("guest installation of create type is complete")
> logger.info("define the vm and boot it up")
> ret = prepare_boot_guest(domobj, params, logger, \
> diff --git a/repos/domain/install_windows_cdrom.py b/repos/domain/install_windows_cdrom.py
> index 402fa25..d87935f 100644
> --- a/repos/domain/install_windows_cdrom.py
> +++ b/repos/domain/install_windows_cdrom.py
> @@ -338,13 +338,13 @@ def install_windows_cdrom(params):
> interval += 20
> logger.info('%s seconds passed away...' % interval)
> elif installtype == 'create':
> - 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.info("guest installation of create type is complete.")
> logger.info("define the vm and boot it up")
> ret = prepare_boot_guest(domobj, params, installtype)
> diff --git a/repos/domain/migrate.py b/repos/domain/migrate.py
> index 7a38a29..ce1b353 100644
> --- a/repos/domain/migrate.py
> +++ b/repos/domain/migrate.py
> @@ -200,8 +200,8 @@ def migrate(params):
> srcdom = srcconn.lookupByName(guestname)
>
> if predstconfig == "true":
> - guest_names = dstconn.listDefinedDomains()
> - if guestname in guest_names:
> + domain_names = dstconn.listDefinedDomains()
> + if guestname in domain_names:
> logger.info("Dst VM exists")
> else:
> logger.error("Dst VM missing config, should define VM on Dst first")
> @@ -232,14 +232,14 @@ def migrate(params):
> env_clean(srcconn, dstconn, target_machine, guestname, logger)
> return 1
> else:
> - guest_names = []
> + domain_names = []
> ids = srcconn.listDomainsID()
> for id in ids:
> obj = srcconn.lookupByID(id)
> - guest_names.append(obj.name())
> - guest_names += srcconn.listDefinedDomains()
> + domain_names.append(obj.name())
> + domain_names += srcconn.listDefinedDomains()
>
> - if guestname in guest_names:
> + if guestname in domain_names:
> logger.error("Source VM still exists")
> env_clean(srcconn, dstconn, target_machine, guestname, logger)
> return 1
> diff --git a/repos/domain/ownership_test.py b/repos/domain/ownership_test.py
> index 7e03526..3ee7d85 100644
> --- a/repos/domain/ownership_test.py
> +++ b/repos/domain/ownership_test.py
> @@ -23,13 +23,13 @@ TEMP_FILE = "/tmp/test.save"
>
> def check_domain_running(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())
>
> - if guestname not in guest_names:
> + if guestname not in domain_names:
> logger.error("%s doesn't exist or not running" % guestname)
> return 1
> else:
> diff --git a/repos/libvirtd/qemu_hang.py b/repos/libvirtd/qemu_hang.py
> index 12fb09f..b764802 100644
> --- a/repos/libvirtd/qemu_hang.py
> +++ b/repos/libvirtd/qemu_hang.py
> @@ -21,13 +21,13 @@ RESTART_CMD = "service libvirtd restart"
>
> def check_domain_running(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())
>
> - if guestname not in guest_names:
> + if guestname not in domain_names:
> logger.error("%s doesn't exist or not running" % guestname)
> return 1
> else:
> diff --git a/repos/libvirtd/restart.py b/repos/libvirtd/restart.py
> index 3e06d4c..daa8e50 100644
> --- a/repos/libvirtd/restart.py
> +++ b/repos/libvirtd/restart.py
> @@ -21,13 +21,13 @@ RESTART_CMD = "service libvirtd restart"
>
> def check_domain_running(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())
>
> - if guestname not in guest_names:
> + if guestname not in domain_names:
> logger.error("%s doesn't exist or not running" % guestname)
> return 1
> else:
> diff --git a/repos/snapshot/delete.py b/repos/snapshot/delete.py
> index a0f28a4..c7c7453 100644
> --- a/repos/snapshot/delete.py
> +++ b/repos/snapshot/delete.py
> @@ -16,9 +16,9 @@ SNAPSHOT_DIR = "/var/lib/libvirt/qemu/snapshot"
>
> def check_domain_state(conn, guestname, logger):
> """ check if the domain exists and in shutdown state as well """
> - guest_names = conn.listDefinedDomains()
> + domain_names = conn.listDefinedDomains()
>
> - if guestname not in guest_names:
> + if guestname not in domain_names:
> logger.error("%s is running or does not exist" % guestname)
> return False
> else:
> diff --git a/repos/snapshot/file_flag.py b/repos/snapshot/file_flag.py
> index 5f8a17e..8e4f7ec 100644
> --- a/repos/snapshot/file_flag.py
> +++ b/repos/snapshot/file_flag.py
> @@ -20,13 +20,13 @@ MAKE_FLAG = "rm -f /tmp/%s; touch /tmp/%s " % (FLAG_FILE, FLAG_FILE)
>
> def check_domain_running(conn, guestname, logger):
> """ check if the domain exists and in running state as well """
> - 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("%s is not running or does not exist" % guestname)
> return False
> else:
> diff --git a/repos/snapshot/flag_check.py b/repos/snapshot/flag_check.py
> index 3a0f3ec..d7b2020 100644
> --- a/repos/snapshot/flag_check.py
> +++ b/repos/snapshot/flag_check.py
> @@ -19,13 +19,13 @@ FLAG_CHECK = "ls %s" % FLAG_FILE
>
> def check_domain_running(conn, guestname, logger):
> """ check if the domain exists and in running state as well """
> - 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("%s is not running or does not exist" % guestname)
> return False
> else:
> diff --git a/repos/snapshot/internal_create.py b/repos/snapshot/internal_create.py
> index 6ddf350..fdcbddd 100644
> --- a/repos/snapshot/internal_create.py
> +++ b/repos/snapshot/internal_create.py
> @@ -51,8 +51,8 @@ def internal_create(params):
> params['snapshotname'] = str(int(time.time()))
>
> conn = sharedmod.libvirtobj['conn']
> - guest_names = conn.listDefinedDomains()
> - if guestname not in guest_names:
> + domain_names = conn.listDefinedDomains()
> + if guestname not in domain_names:
> logger.error("%s is not poweroff or doesn't exist" % guestname)
> return 1
>
> diff --git a/repos/snapshot/revert.py b/repos/snapshot/revert.py
> index d89d926..f11ba0a 100644
> --- a/repos/snapshot/revert.py
> +++ b/repos/snapshot/revert.py
> @@ -14,9 +14,9 @@ optional_params = ()
>
> def check_domain_state(conn, guestname, logger):
> """ check if the domain exists and in shutdown state as well """
> - guest_names = conn.listDefinedDomains()
> + domain_names = conn.listDefinedDomains()
>
> - if guestname not in guest_names:
> + if guestname not in domain_names:
> logger.error("%s is running or does not exist" % guestname)
> return False
> else:
Looks good, ACK.
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list