From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lucas Meneghel Rodrigues Subject: Re: [KVM-AUTOTEST PATCH] Use new function VM.get_name() to get the VM's name, instead of VM.name Date: Wed, 03 Jun 2009 20:26:38 -0300 Message-ID: <1244071598.2901.10.camel@localhost.localdomain> References: <615550170.1060071244005307909.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Michael Goldish Return-path: Received: from mx2.redhat.com ([66.187.237.31]:52041 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752653AbZFCXcI (ORCPT ); Wed, 3 Jun 2009 19:32:08 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n53NW7Og009041 for ; Wed, 3 Jun 2009 19:32:10 -0400 In-Reply-To: <615550170.1060071244005307909.JavaMail.root@zmail05.collab.prod.int.phx2.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, 2009-06-03 at 01:01 -0400, Michael Goldish wrote: > ----- "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. Fair enough, now that you've put it that way I agree with you. > > > 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 -- Lucas Meneghel Rodrigues Software Engineer (QE) Red Hat - Emerging Technologies