FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Fedora/Linux Management Tools

 
 
LinkBack Thread Tools
 
Old 12-10-2008, 02:45 AM
Cole Robinson
 
Default Port utility functions to Solaris

John Levon wrote:
> On Tue, Dec 09, 2008 at 05:13:05PM -0500, Cole Robinson wrote:
>
>
>>> Port utility functions to Solaris
>>>
>> Hmm, 90% of this patch is a huge mechanical change which has nothing
>> to do with this commit subject. I think it's better to separate
>> the two changes.
>>
>
> Seems like busy work to me quite frankly. You can diff util.py versus
> _util.py to get the changes easily enough (I tried a rename, but hg was
> playing silly buggers).
>
>

I really don't think it's unreasonable to ask that these two unrelated
changes be split. The commits will be more self contained, and it will
be easier for review.

>> Rather than s/util/_util/ everywhere, I'd prefer:
>>
>> import _util as util
>>
>> Lot less churn, and the only potential downside is confusion
>> browsing the source, but I don't think it's a problem.
>>
>
> It's already confusing enough, please let's not make it worse.
>
>

I won't disagree with that.

>>> +lookup_pool_by_path = _util.lookup_pool_by_path
>>> +privileged_user = _util.privileged_user
>>>
>> Hmm, this isn't exactly how I saw it happening. I figured we would have
>> _util be for nonpublic functions only: we can change or remove anything
>> in that file at will. The original 'util' though needs to maintain api
>> compat for its functions, even if they are deprecated and left to rot.
>>
>
> The public API has been clearly moving towards an OO approach. This
> makes perfect sense to me: if I want to interact with a disk, use
> VirtualDisk. Anything new in virtinst.util is a bug - can you name a
> situation where it's sensible for something to be added there as part of
> the API? The very name 'util' basically 'random stuff'.
>
>

Yes, that's fair. Thinking of it that way, there will always be a better
solution than dumping something in 'util'.

>> Am I missing some benefit of the above approach?
>>
>
> APIs are really hard to maintain when they're commingled amongst a bunch
> of other completely unstable code, especially when it's not even
> documented.

I completely agree with this, that's why my suggestion was to keep the
public and private functions in separate files. Moving everything
to the private file seems like it would only encourage confusion
between what is public and what is private, increasing the likelyhood
that someone would inadvertently break API.

> Locking away the contents of util.py and never changing it
> again[1] makes it really hard to *NOT* maintain the API - any patch touches
> that file implies the patch is broken.
>

I don't follow this. By moving the actual content to _util.py, API can
_still_ be broken: someone could add a new function argument without
a default value.

I think we can both agree that one of the biggest failures of this
code base is a lack of documentation. It can be a total frustration
trying to cleanup and fix things, not knowing how the pieces interact.
I've been trying to remedy this as I go. Dealing with 'util' is an
important step, and I'm not trying to push back on it.

Thanks,
Cole

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
 
Old 12-10-2008, 11:43 AM
John Levon
 
Default Port utility functions to Solaris

On Tue, Dec 09, 2008 at 10:45:15PM -0500, Cole Robinson wrote:

> >> Hmm, 90% of this patch is a huge mechanical change which has nothing
> >> to do with this commit subject. I think it's better to separate
> >> the two changes.
>
> I really don't think it's unreasonable to ask that these two unrelated
> changes be split. The commits will be more self contained, and it will
> be easier for review.

Hmpph, OK.

> > Locking away the contents of util.py and never changing it
> > again[1] makes it really hard to *NOT* maintain the API - any patch touches
> > that file implies the patch is broken.
>
> I don't follow this. By moving the actual content to _util.py, API can
> _still_ be broken: someone could add a new function argument without
> a default value.

I was really referring to someone inadvertently *adding* to the API.

I'll go ahead and move most stuff back to util.py.

regards
john

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
 
Old 12-10-2008, 06:34 PM
 
Default Port utility functions to Solaris

# HG changeset patch
# User john.levon@sun.com
# Date 1228937605 28800
# Node ID e9045f3ae5b2243a9ee00f0913c2dcefdddd7c36
# Parent a1b50ecb6ecef44f3a86bd27da53a8f14687dc38
Port utility functions to Solaris

Port the utility functions to Solaris.

Signed-off-by: John Levon <john.levon@sun.com>

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
@@ -33,6 +33,7 @@ import re
import re
import libxml2
import logging
+import subprocess
from sys import stderr

import libvirt
@@ -44,7 +45,19 @@ KEYBOARD_DIR = "/etc/sysconfig/keyboard"
KEYBOARD_DIR = "/etc/sysconfig/keyboard"
XORG_CONF = "/etc/X11/xorg.conf"

-def default_route():
+def default_route(nic = None):
+ if platform.system() == 'SunOS':
+ cmd = [ '/usr/bin/netstat', '-rn' ]
+ if nic:
+ cmd += [ '-I', nic ]
+ proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+ for line in proc.stdout.readlines():
+ vals = line.split()
+ if len(vals) > 1 and vals[0] == 'default':
+ return vals[1]
+ return None
+
route_file = "/proc/net/route"
d = file(route_file)

@@ -63,7 +76,29 @@ def default_route():
continue
return None

+def _default_nic():
+ """Return the default NIC to use, if one is specified.
+ This is NOT part of the API and may change at will."""
+
+ dev = '
+
+ if platform.system() != 'SunOS':
+ return dev
+
+ # XXX: fails without PRIV_XVM_CONTROL
+ proc = subprocess.Popen(['/usr/lib/xen/bin/xenstore-read',
+ 'device-misc/vif/default-nic'], stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+ out = proc.stdout.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
@@ -76,6 +111,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()):
@@ -93,12 +131,16 @@ def default_network(conn):
return ["network", "default"]

def default_connection():
- if os.path.exists("/var/lib/xend") and os.path.exists("/proc/xen"):
- return "xen"
- elif os.path.exists("/usr/bin/qemu") or
- os.path.exists("/usr/bin/qemu-kvm") or
- os.path.exists("/usr/bin/kvm") or
- os.path.exists("/usr/bin/xenner"):
+ if os.path.exists('/var/lib/xend'):
+ if os.path.exists('/dev/xen/evtchn'):
+ return 'xen'
+ if os.path.exists("/proc/xen"):
+ return 'xen'
+
+ if os.path.exists("/usr/bin/qemu") or
+ os.path.exists("/usr/bin/qemu-kvm") or
+ os.path.exists("/usr/bin/kvm") or
+ os.path.exists("/usr/bin/xenner"):
if User.current().has_priv(User.PRIV_QEMU_SYSTEM):
return "qemu:///system"
else:
@@ -106,6 +148,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()
@@ -119,15 +164,16 @@ def get_cpu_flags():
return flst
return []

-def is_pae_capable():
+def is_pae_capable(conn = None):
"""Determine if a machine is PAE capable or not."""
- flags = get_cpu_flags()
- if "pae" in flags:
- return True
- return False
+ if not conn:
+ conn = libvirt.open(')
+ return "pae" in conn.getCapabilities()

def is_hvm_capable():
"""Determine if a machine is HVM capable or not."""
+ if platform.system() == 'SunOS':
+ raise OSError('HVM capability not determinible')

caps = ""
if os.path.exists("/sys/hypervisor/properties/capabilities"):
@@ -143,6 +189,9 @@ def is_kvm_capable():
return os.path.exists("/dev/kvm")

def is_blktap_capable():
+ if platform.system() == 'SunOS':
+ return False
+
#return os.path.exists("/dev/xen/blktapctrl")
f = open("/proc/modules")
lines = f.readlines()

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
 
Old 12-11-2008, 02:43 PM
Cole Robinson
 
Default Port utility functions to Solaris

john.levon@sun.com wrote:
> # HG changeset patch
> # User john.levon@sun.com
> # Date 1228937605 28800
> # Node ID e9045f3ae5b2243a9ee00f0913c2dcefdddd7c36
> # Parent a1b50ecb6ecef44f3a86bd27da53a8f14687dc38
> Port utility functions to Solaris
>
> Port the utility functions to Solaris.
>
> Signed-off-by: John Levon <john.levon@sun.com>
>

Thanks, applied.

- Cole

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
 

Thread Tools




All times are GMT. The time now is 09:34 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org