Hi. Repeating same logic seems ugly. How about something like the following?

def _do_dom(self, action, instance_name)
   if self.readonly:
      tmpconn ...

      action(dom)
      tmpconn.close()
   else:
      dom = ...
      action(dom)

def pause(...)
...
   self._do_dom(virtDoain.suspend)

...

thanks,

On Wed, May 25, 2011 at 08:23:29AM -0000, Kei Masumoto wrote:
> Kei Masumoto has proposed merging lp:~nttdata/nova/kvm-pause-suspend into lp:nova.
> 
> Requested reviews:
>   Nova Core (nova-core)
> 
> For more details, see:
> https://code.launchpad.net/~nttdata/nova/kvm-pause-suspend/+merge/62255
> 
> added pause/suspend implementation to nova.virt.libvirt_conn
> -- 
> https://code.launchpad.net/~nttdata/nova/kvm-pause-suspend/+merge/62255
> You are subscribed to branch lp:nova.

> === modified file 'nova/virt/libvirt_conn.py'
> --- nova/virt/libvirt_conn.py	2011-05-17 16:28:44 +0000
> +++ nova/virt/libvirt_conn.py	2011-05-25 08:17:27 +0000
> @@ -550,19 +550,54 @@
>  
>      @exception.wrap_exception
>      def pause(self, instance, callback):
> -        raise exception.ApiError("pause not supported for libvirt.")
> +        """Pause VM instance"""
> +        if self.read_only:
> +            tmpconn = self._connect(self.libvirt_uri, False)
> +            dom = tmpconn.lookupByName(instance.name)
> +            dom.suspend()
> +            tmpconn.close()
> +        else:
> +            dom = self._conn.lookupByName(instance.name)
> +            dom.suspend()
>  
>      @exception.wrap_exception
>      def unpause(self, instance, callback):
> -        raise exception.ApiError("unpause not supported for libvirt.")
> +        """Unpause paused VM instance"""
> +        if self.read_only:
> +            tmpconn = self._connect(self.libvirt_uri, False)
> +            dom = tmpconn.lookupByName(instance.name)
> +            dom.resume()
> +            tmpconn.close()
> +        else:
> +            dom = self._conn.lookupByName(instance.name)
> +            dom.resume()
>  
>      @exception.wrap_exception
>      def suspend(self, instance, callback):
> -        raise exception.ApiError("suspend not supported for libvirt")
> +        """Suspend the specified instance"""
> +        if self.read_only:
> +            tmpconn = self._connect(self.libvirt_uri, False)
> +            dom = tmpconn.lookupByName(instance.name)
> +            dom.managedSave(0)
> +            tmpconn.close()
> +        else:
> +            dom = self._conn.lookupByName(instance.name)
> +            dom.managedSave(0)
>  
>      @exception.wrap_exception
>      def resume(self, instance, callback):
> -        raise exception.ApiError("resume not supported for libvirt")
> +        """resume the specified instance"""
> +        try:
> +            if self.read_only:
> +                tmpconn = self._connect(self.libvirt_uri, False)
> +                dom = tmpconn.lookupByName(instance.name)
> +                tmpconn.close()
> +            else:
> +                dom = self._conn.lookupByName(instance.name)
> +            dom.create()
> +        except libvirt.LibvirtError:
> +            xml = self.to_xml(instance, None)
> +            self._create_new_domain(xml)
>  
>      @exception.wrap_exception
>      def rescue(self, instance):
> 


-- 
yamahata