From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Goldish Subject: Re: [KVM-AUTOTEST PATCH] Use new function VM.get_name() to get the VM's name, instead of VM.name Date: Wed, 3 Jun 2009 01:01:47 -0400 (EDT) Message-ID: <615550170.1060071244005307909.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> References: <1243516017.2976.1011.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Lucas Meneghel Rodrigues Return-path: Received: from mx1.redhat.com ([66.187.233.31]:33603 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752635AbZFCFBr (ORCPT ); Wed, 3 Jun 2009 01:01:47 -0400 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n5351naY016945 for ; Wed, 3 Jun 2009 01:01:49 -0400 In-Reply-To: <1243516017.2976.1011.camel@localhost.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: ----- "Lucas Meneghel Rodrigues" wrote: > On Sun, 2009-05-24 at 18:46 +0300, Michael Goldish wrote: > > kvm_vm.py: add function VM.get_name(). > > kvm_preprocessing.py: use VM.get_name() instead of directly > accessing the .name > > attribute. > > Are there any advantages of creating this method over directly > accessing > the attribute? Not really, it's just that everywhere else we use "get" interface functions and I like to be consistent. I also thought it was good OOP practice to make the interface independent of the internal implementation. For example, we may choose not to store the name attribute in the class instance, but rather retrieve it or generate it (if it gets more complex) from the params upon request. > > Signed-off-by: Michael Goldish > > --- > > client/tests/kvm_runtest_2/kvm_preprocessing.py | 6 +++--- > > client/tests/kvm_runtest_2/kvm_vm.py | 4 ++++ > > 2 files changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/client/tests/kvm_runtest_2/kvm_preprocessing.py > b/client/tests/kvm_runtest_2/kvm_preprocessing.py > > index c9eb35d..bcabf5a 100644 > > --- a/client/tests/kvm_runtest_2/kvm_preprocessing.py > > +++ b/client/tests/kvm_runtest_2/kvm_preprocessing.py > > @@ -178,7 +178,7 @@ def preprocess(test, params, env): > > if vm.is_dead(): > > continue > > if not vm.verify_process_identity(): > > - kvm_log.debug("VM '%s' seems to have been replaced by > another process" % vm.name) > > + kvm_log.debug("VM '%s' seems to have been replaced by > another process" % vm.get_name()) > > vm.pid = None > > > > # Destroy and remove VMs that are no longer needed in the > environment > > @@ -187,8 +187,8 @@ def preprocess(test, params, env): > > vm = env[key] > > if not kvm_utils.is_vm(vm): > > continue > > - if not vm.name in requested_vms: > > - kvm_log.debug("VM '%s' found in environment but not > required for test; removing it..." % vm.name) > > + if not vm.get_name() in requested_vms: > > + kvm_log.debug("VM '%s' found in environment but not > required for test; removing it..." % vm.get_name()) > > vm.destroy() > > del env[key] > > > > diff --git a/client/tests/kvm_runtest_2/kvm_vm.py > b/client/tests/kvm_runtest_2/kvm_vm.py > > index fab839f..df99859 100644 > > --- a/client/tests/kvm_runtest_2/kvm_vm.py > > +++ b/client/tests/kvm_runtest_2/kvm_vm.py > > @@ -454,6 +454,10 @@ class VM: > > """Return True iff the VM's PID does not exist.""" > > return not kvm_utils.pid_exists(self.pid) > > > > + def get_name(self): > > + """Return the VM's name.""" > > + return self.name > > + > > def get_params(self): > > """Return the VM's params dict. > > > -- > Lucas Meneghel Rodrigues > Software Engineer (QE) > Red Hat - Emerging Technologies