[et-mgmt-tools] [PATCH] Port utility functions to Solaris

[Cole Robinson]

john.levon at sun.com wrote:
> # HG changeset patch
> # User john.levon at sun.com
> # Date 1228484283 28800
> # Node ID 271ceb01335da50c05fe6c2f54f3eb1721f51bb4
> # Parent  8ad095a908ad89a0c83c94bd486647b79817acbe
> Port utility functions to Solaris
>
> Port various utils to Solaris, and remove some unused ones that don't
> work.
>
> Signed-off-by: John Levon <john.levon at sun.com>
>
>   

Unfortunately, 'util' is part of the public API, so removing dead
functions isn't a good idea. Granted, we probably have a grand total
of 3 API users (virt cli tools, virt-manager, koan), but I think it's
better to just play by the rules. I'm certainly not opposed to moving
those functions away from the still relevant code and clearly labeling
them as deprecated.

It might be a good idea to make an internal util file, so we have an
alternate place to dump utility methods without them automatically
entering the public API.

> diff --git a/virtinst/FullVirtGuest.py b/virtinst/FullVirtGuest.py
> --- a/virtinst/FullVirtGuest.py
> +++ b/virtinst/FullVirtGuest.py
> @@ -38,7 +38,8 @@ class FullVirtGuest(Guest):
>              installer = DistroManager.DistroInstaller(type = type, os_type = "hvm")
>          Guest.__init__(self, type, connection, hypervisorURI, installer)
>          self.disknode = "hd"
> -        self.features = { "acpi": None, "pae": util.is_pae_capable(), "apic": None }
> +        self.features = { "acpi": None, "pae":
> +            util.is_pae_capable(connection), "apic": None }
>          if arch is None:
>              arch = platform.machine()
>          self.arch = arch
> diff --git a/virtinst/util.py b/virtinst/util.py
> --- a/virtinst/util.py
> +++ b/virtinst/util.py
> @@ -25,6 +25,7 @@ import re
>  import re
>  import libxml2
>  import logging
> +import popen2
>  from sys import stderr
>  
>  import libvirt
> @@ -54,8 +55,29 @@ def default_route():
>              continue
>      return None
>  
> -# Legacy for compat only.
> +def default_nic():
> +    """Return the default NIC to use, if one is specified."""
> +
>   

Since this seems only be used for the public default_bridge
function, this is a candidate for an internal util file.

> +    dev = ''
> +
> +    if platform.system() != 'SunOS':
> +        return dev
> +
> +    # XXX: fails without PRIV_XVM_CONTROL
> +    proc = popen2.Popen3(["/usr/lib/xen/bin/xenstore-read",
> +        "device-misc/vif/default-nic"], capturestderr=True)
>   

The 'subprocess' module is intended to replace popen2. We aren't
really using it in the codebase yet, but it's worth mentioning.
Certainly not a deal breaker.

> +    proc.tochild.close()
> +    proc.wait()
> +    out = proc.fromchild.readlines()
> +    if len(out) > 0:
> +        dev = out[0].rstrip()
> +
> +    return dev
> +
>  def default_bridge():
> +    if platform.system() == 'SunOS':
> +        return default_nic()
> +
>      rt = default_route()
>      if rt is None:
>          defn = None
> @@ -68,6 +90,9 @@ def default_bridge():
>          return "xenbr%d"%(defn)
>  
>  def default_network(conn):
> +    if platform.system() == 'SunOS':
> +        return ["bridge", default_nic()]
> +
>      dev = default_route()
>  
>      if dev is not None and not is_uri_remote(conn.getURI()):
> @@ -98,6 +123,9 @@ def default_connection():
>      return None
>  
>  def get_cpu_flags():
> +    if platform.system() == 'SunOS':
> +        raise OSError("CPU flags not available")
> +
>      f = open("/proc/cpuinfo")
>      lines = f.readlines()
>      f.close()
> @@ -111,31 +139,16 @@ def get_cpu_flags():
>          return flst
>      return []
>  
> -def is_pae_capable():
> +def is_pae_capable(conn):
>   

If altering the command list here, please make conn optional
to maintain back compat.

>      """Determine if a machine is PAE capable or not."""
> -    flags = get_cpu_flags()
> -    if "pae" in flags:
> -        return True
> -    return False
> -
> -def is_hvm_capable():
> -    """Determine if a machine is HVM capable or not."""
> -
> -    caps = ""
> -    if os.path.exists("/sys/hypervisor/properties/capabilities"):
> -        caps = open("/sys/hypervisor/properties/capabilities").read()
> -    if caps.find("hvm") != -1:
> -        return True
> -    return False
> -
> -def is_kqemu_capable():
> -    return os.path.exists("/dev/kqemu")
> -
> -def is_kvm_capable():
> -    return os.path.exists("/dev/kvm")
> +    if not conn:
> +        conn = libvirt.open('')
> +    return "pae" in conn.getCapabilities()
>  
>  def is_blktap_capable():
> -    #return os.path.exists("/dev/xen/blktapctrl")
> +    if platform.system() == 'SunOS':
> +        return False
> +
>      f = open("/proc/modules")
>      lines = f.readlines()
>      f.close()
>
>   

Thanks,
Cole