Re: [KVM-AUTOTEST PATCH] Use new function VM.get_name() to get the VM's name, instead of VM.name

[Lucas Meneghel Rodrigues <lmr@redhat.com>]

On Wed, 2009-06-03 at 01:01 -0400, Michael Goldish wrote:
> ----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> 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 <mgoldish@redhat.com>
> > > ---
> > >  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