From: Eric Price <ecprice@mit.edu>
Date: Wed, 10 Oct 2007 05:56:37 +0000 (-0400)
Subject: Several modifications.
X-Git-Tag: sipb-xen-www/1~64
X-Git-Url: http://xvm.mit.edu/gitweb/invirt/packages/invirt-web.git/commitdiff_plain/5d070af36a7da6e1b25023ae96b30d66573343dc

Several modifications.


svn path=/trunk/web/; revision=177
---

diff --git a/templates/getafsgroups.py b/templates/getafsgroups.py
index 0567efe..c98f708 100644
--- a/templates/getafsgroups.py
+++ b/templates/getafsgroups.py
@@ -28,35 +28,42 @@ def checkAfsGroup(user, group, cell):
     """
     checkAfsGroup(user, group) returns True if and only if user is in AFS group group in cell cell
     """
-    print user, group
-    p = subprocess.Popen(["pts", "membership", group, '-c', cell], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    p2 = subprocess.Popen(["grep", "-v", "^Members"], stdin=p.stdout, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    if p2.wait():
+    p = subprocess.Popen(["pts", "membership", group, '-c', cell], 
+                         stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    if p.wait():
         return False
-    for member in p2.stdout.read().split():
-        if member == user:
+    for line in p.stdout.readlines()[1:]:
+        if line.strip() == user:
             return True
     return False
 
-def checkLockerOwner(user, locker):
+def checkLockerOwner(user, locker, verbose=False):
     """
     checkLockerOwner(user, locker) returns True if and only if user administers locker
     """
-    p = subprocess.Popen(["fs", "whichcell", "/mit/" + locker], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    if (p.wait()):
+    p = subprocess.Popen(["fs", "whichcell", "/mit/" + locker], 
+                         stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    if p.wait():
+        if verbose:
+            return p.stderr.read()
         return False
     cell = p.stdout.read().split()[-1][1:-1]
-    p = subprocess.Popen(["fs", "listacl", "/mit/" + locker], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    p2 = subprocess.Popen(["grep", "^  .* rlidwka$"], stdin=p.stdout, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    if (p2.wait()):
+    p = subprocess.Popen(["fs", "listacl", "/mit/" + locker], 
+                         stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    if p.wait():
+        if verbose:
+            return p.stderr.read()
         return False
-    for line in p2.stdout.read().split('\n'):
+    for line in p.stdout.readlines()[1:]:
         entry = line.split()
-        if entry == [] or entry[0] == "Negative":
+        if not entry or entry[0] == "Negative":
             break
         if entry[1] == "rlidwka":
-            if entry[0] == user or (entry[0][0:6] == "system" and checkAfsGroup(user, entry[0], cell)):
+            if entry[0] == user or (entry[0][0:6] == "system" and 
+                                    checkAfsGroup(user, entry[0], cell)):
                 return True
+    if verbose:
+        return "You don't have admin bits on /mit/" + locker
     return False
 
 
diff --git a/templates/info.tmpl b/templates/info.tmpl
index f9b3941..f4a3d5c 100644
--- a/templates/info.tmpl
+++ b/templates/info.tmpl
@@ -62,7 +62,7 @@ Info on $machine.name
 <form action="modify" method="POST">
   <input type="hidden" name="machine_id" value="$machine.machine_id"/>
   <table>
-    <tr><td>Owner$owner_help:</td><td><input type="text" name="owner", value="$machine.owner"/></td></tr>
+    <tr><td>Owner${helppopup("owner")}:</td><td><input type="text" name="owner", value="$machine.owner"/></td></tr>
     <tr><td>Contact email:</td><td><input type="text" name="contact" value="$machine.contact"/></td></tr>
 #if $machine.nics
     <tr><td>Hostname:</td><td><input type="text" name="hostname" value="$machine.nics[0].hostname"/>.servers.csail.mit.edu</td></tr>
diff --git a/templates/main.py b/templates/main.py
index f64fe3d..74a6116 100755
--- a/templates/main.py
+++ b/templates/main.py
@@ -87,16 +87,18 @@ def getMachinesByOwner(owner):
     """Return the machines owned by a given owner."""
     return Machine.select_by(owner=owner)
 
-def maxMemory(user, machine=None):
+def maxMemory(user, machine=None, on=True):
     """Return the maximum memory for a machine or a user.
 
     If machine is None, return the memory available for a new 
     machine.  Else, return the maximum that machine can have.
 
-    on is a dictionary from machines to booleans, whether a machine is
-    on.  If None, it is recomputed. XXX make this global?
+    on is whether the machine should be turned on.  If false, the max
+    memory for the machine to change to, if it is left off, is
+    returned.
     """
-
+    if not on:
+        return MAX_MEMORY_SINGLE
     machines = getMachinesByOwner(user.username)
     active_machines = [x for x in machines if g.uptimes[x]]
     mem_usage = sum([x.memory for x in active_machines if x != machine])
@@ -118,7 +120,7 @@ def haveAccess(user, machine):
     """Return whether a user has access to a machine"""
     if user.username == 'moo':
         return True
-    return getafsgroups.checkLockerOwner(user.username,machine.owner)
+    return getafsgroups.checkLockerOwner(user.username, machine.owner)
 
 def error(op, user, fields, err, emsg):
     """Print an error page when a CodeError occurs"""
@@ -174,8 +176,9 @@ def remctl(*args, **kws):
         p.wait()
         return p.stdout.read(), p.stderr.read()
     if p.wait():
-        raise CodeError('ERROR on remctl %s: %s' %
-                          (args, p.stderr.read()))
+        print >> sys.stderr, 'Error on remctl %s:' % args
+        print >> sys.stderr, p.stderr.read()
+        raise CodeError('ERROR on remctl')
     return p.stdout.read()
 
 def lvcreate(machine, disk):
@@ -319,8 +322,12 @@ def createVm(user, name, memory, disk, is_hvm, cdrom):
 
     return machine
 
-def validMemory(user, memory, machine=None):
-    """Parse and validate limits for memory for a given user and machine."""
+def validMemory(user, memory, machine=None, on=True):
+    """Parse and validate limits for memory for a given user and machine.
+
+    on is whether the memory must be valid after the machine is
+    switched on.
+    """
     try:
         memory = int(memory)
         if memory < MIN_MEMORY_SINGLE:
@@ -328,7 +335,7 @@ def validMemory(user, memory, machine=None):
     except ValueError:
         raise InvalidInput('memory', memory, 
                            "Minimum %s MB" % MIN_MEMORY_SINGLE)
-    if memory > maxMemory(user, machine):
+    if memory > maxMemory(user, machine, on):
         raise InvalidInput('memory', memory,
                            'Maximum %s MB' % maxMemory(user, machine))
     return memory
@@ -360,7 +367,7 @@ def create(user, fields):
                            "Already exists")
     
     memory = fields.getfirst('memory')
-    memory = validMemory(user, memory)
+    memory = validMemory(user, memory, on=True)
     
     disk = fields.getfirst('disk')
     disk = validDisk(user, disk)
@@ -565,81 +572,79 @@ def command(user, fields):
     return Template(file="command.tmpl", searchList=[d, global_dict])
 
 def testOwner(user, owner, machine=None):
-    if not getafsgroups.checkLockerOwner(user.username, owner):
-        raise InvalidInput('owner', owner,
-                           "Invalid")
-    return owner
+    if owner == machine.owner:   #XXX What do we do when you lose access to the locker?
+        return owner
+    value = getafsgroups.checkLockerOwner(user.username, owner, verbose=True)
+    if value == True:
+        return owner
+    raise InvalidInput('owner', owner, value)
 
 def testContact(user, contact, machine=None):
-    if contact != user.email:
-        raise InvalidInput('contact', contact,
-                           "Invalid")
+    if not re.match("^[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,4}$", contact, re.I):
+        raise InvalidInput('contact', contact, "Not a valid email")
     return contact
 
 def testDisk(user, disksize, machine=None):
     return disksize
 
 def testName(user, name, machine=None):
-    if Machine.select_by(name=name) == []:
+    if name is None:
+        return None
+    if not Machine.select_by(name=name):
         return name
     if name == machine.name:
         return name
-    raise InvalidInput('name', name,
-                       "Already taken")
+    raise InvalidInput('name', name, "Already taken")
 
 def testHostname(user, hostname, machine):
     for nic in machine.nics:
         if hostname == nic.hostname:
             return hostname
     # check if doesn't already exist
-    if NIC.select_by(hostname=hostname) == []:
-        return hostname
-    raise InvalidInput('hostname', hostname,
-                       "Different from before")
-
+    if NIC.select_by(hostname=hostname):
+        raise InvalidInput('hostname', hostname,
+                           "Already exists")
+    if not re.match("^[A-Z0-9-]{1,22}$", hostname, re.I):
+        raise InvalidInput('hostname', hostname, "Not a valid hostname; must only use number, letters, and dashes.")
+    return hostname
 
 def modify(user, fields):
     """Handler for modifying attributes of a machine."""
-    #XXX not written yet
 
+    olddisk = {}
     transaction = ctx.current.create_transaction()
     try:
         machine = testMachineId(user, fields.getfirst('machine_id'))
         owner = testOwner(user, fields.getfirst('owner'), machine)
         contact = testContact(user, fields.getfirst('contact'))
         hostname = testHostname(owner, fields.getfirst('hostname'),
-                            machine)
+                                machine)
         name = testName(user, fields.getfirst('name'), machine)
         oldname = machine.name
         command="modify"
-        olddisk = {}
 
         memory = fields.getfirst('memory')
         if memory is not None:
-            memory = validMemory(user, memory, machine)
-        else:
-            memory = machine.memory
-        if memory != machine.memory:
+            memory = validMemory(user, memory, machine, on=False)
             machine.memory = memory
-
+ 
         disksize = testDisk(user, fields.getfirst('disk'))
         if disksize is not None:
             disksize = validDisk(user, disksize, machine)
-        else:
-            disksize = machine.disks[0].size
-        for disk in machine.disks:
-            olddisk[disk.guest_device_name] = disk.size
-            disk.size = disksize
-            ctx.current.save(disk)
+            disk = machine.disks[0]
+            if disk.size != disksize:
+                olddisk[disk.guest_device_name] = disksize
+                disk.size = disksize
+                ctx.current.save(disk)
         
-        # XXX all NICs get same hostname on change?  Interface doesn't support more.
-        for nic in machine.nics:
+        # XXX first NIC gets hostname on change?  Interface doesn't support more.
+        for nic in machine.nics[:1]:
             nic.hostname = hostname
             ctx.current.save(nic)
 
-        if owner != machine.owner:
+        if owner is not None and owner != machine.owner:
             machine.owner = owner
-        if name != machine.name:
+        if name is not None and name != machine.name:
             machine.name = name
             
         ctx.current.save(machine)
@@ -647,13 +652,13 @@ def modify(user, fields):
     except:
         transaction.rollback()
         raise
-    remctl("web", "moveregister", oldname, name)
-    for disk in machine.disks:
-        # XXX all disks get the same size on change?  Interface doesn't support more.
-        if disk.size != olddisk[disk.guest_device_name]:
-            remctl("web", "lvresize", oldname, disk.guest_device_name, str(disk.size))
-        if oldname != name:
-            remctl("web", "lvrename", oldname, disk.guest_device_name, name)
+    for diskname in olddisk:
+        remctl("web", "lvresize", oldname, diskname, str(olddisk[diskname]))
+    if name is not None and name != oldname:
+        for disk in machine.disks:
+            if oldname != name:
+                remctl("web", "lvrename", oldname, disk.guest_device_name, name)
+        remctl("web", "moveregister", oldname, name)
     d = dict(user=user,
              command=command,
              machine=machine)
@@ -814,6 +819,7 @@ if __name__ == '__main__':
         print 'Content-Type: text/html\n'
         sys.stderr.seek(0)
         e = sys.stderr.read()
+        sys.stderr=sys.stdout
         if e:
             output = str(output)
             output = output.replace('<body>', '<body><p>STDERR:</p><pre>'+e+'</pre>')