Index: trunk/packages/invirt-base/debian/changelog =================================================================== --- trunk/packages/invirt-base/debian/changelog (revision 2096) +++ trunk/packages/invirt-base/debian/changelog (revision 2097) @@ -1,8 +1,2 @@ -invirt-base (0.0.21) unstable; urgency=low - - * Get rid of confusing err=True option to invirt.remctl.remctl - - -- Evan Broder Fri, 30 Jan 2009 19:49:46 -0500 - invirt-base (0.0.20) unstable; urgency=low Index: trunk/packages/invirt-base/python/invirt/common.py =================================================================== --- trunk/packages/invirt-base/python/invirt/common.py (revision 2096) +++ trunk/packages/invirt-base/python/invirt/common.py (revision 2097) @@ -76,7 +76,5 @@ class CodeError(Exception): """Exception for internal errors or bad faith input.""" - def __init__(self, message, code=None): - Exception.__init__(self, message) - self.code = code + pass # Index: trunk/packages/invirt-base/python/invirt/remctl.py =================================================================== --- trunk/packages/invirt-base/python/invirt/remctl.py (revision 2096) +++ trunk/packages/invirt-base/python/invirt/remctl.py (revision 2097) @@ -37,5 +37,9 @@ stderr=subprocess.PIPE) v = p.wait() + if kws.get('err'): + return p.stdout.read(), p.stderr.read() if v: - raise CodeError('ERROR on remctl %s:\n%s' % (args, p.stderr.read()), v) + print >> sys.stderr, 'Error', v, 'on remctl', args, ':' + print >> sys.stderr, p.stderr.read() + raise CodeError('ERROR on remctl') return p.stdout.read() Index: trunk/packages/invirt-remote/debian/changelog =================================================================== --- trunk/packages/invirt-remote/debian/changelog (revision 2096) +++ trunk/packages/invirt-remote/debian/changelog (revision 2097) @@ -1,11 +1,2 @@ -invirt-remote (0.3.2) unstable; urgency=low - - * Don't pass untrusted arguments to xm info. - * Exit with a non-0 error code on the host if there's an error. - * Always exec instead of fork&exec on the remctl server so that return - codes get passed on. - - -- Evan Broder Sat, 31 Jan 2009 03:51:54 -0500 - invirt-remote (0.3.1) unstable; urgency=low Index: trunk/packages/invirt-remote/host/usr/sbin/invirt-remote =================================================================== --- trunk/packages/invirt-remote/host/usr/sbin/invirt-remote (revision 2096) +++ trunk/packages/invirt-remote/host/usr/sbin/invirt-remote (revision 2097) @@ -22,5 +22,5 @@ ;; info) - exec /usr/sbin/xm info + COMMAND=/usr/sbin/xm ;; vnccert) @@ -39,3 +39,3 @@ esac -exec "$COMMAND" "$SERVICE" "$@" +$COMMAND "$SERVICE" "$@" Index: trunk/packages/invirt-remote/host/usr/sbin/invirt-vmcontrol =================================================================== --- trunk/packages/invirt-remote/host/usr/sbin/invirt-vmcontrol (revision 2096) +++ trunk/packages/invirt-remote/host/usr/sbin/invirt-vmcontrol (revision 2097) @@ -12,8 +12,4 @@ ACTION="$2" MACHINE="d_$ORIGMACHINE" - -xm () { - command xm "$@" || exit 35 -} case "$ACTION" in Index: trunk/packages/invirt-remote/server/usr/sbin/invirt-remote-proxy =================================================================== --- trunk/packages/invirt-remote/server/usr/sbin/invirt-remote-proxy (revision 2096) +++ trunk/packages/invirt-remote/server/usr/sbin/invirt-remote-proxy (revision 2097) @@ -14,17 +14,17 @@ case "$TYPE/$SERVICE" in web/listvms ) - exec invirt-remote-listvms "$@" ;; + invirt-remote-listvms "$@" ;; web/vnccert ) - exec invirt-remote-vnccert "$@" ;; + invirt-remote-vnccert "$@" ;; control/help ) - exec invirt-remctl-help ;; + invirt-remctl-help ;; control/create|control/install ) - exec invirt-remote-create "$SERVICE" "$MACHINE" "$@" ;; + invirt-remote-create "$SERVICE" "$MACHINE" "$@" ;; control/listhost|control/list-host ) - exec invirt-remote-listhost "$MACHINE" "$@" ;; + invirt-remote-listhost "$MACHINE" "$@" ;; control/* ) # Everything but create must go where the VM is already running. - exec invirt-remote-control "$MACHINE" "$SERVICE" "$@" ;; + invirt-remote-control "$MACHINE" "$SERVICE" "$@" ;; * ) - exec remctl "$(invirt-getconf hosts.0.hostname)" remote "$TYPE" "$SERVICE" "$@" ;; + remctl "$(invirt-getconf hosts.0.hostname)" remote "$TYPE" "$SERVICE" "$@" ;; esac Index: trunk/packages/invirt-web/code/controls.py =================================================================== --- trunk/packages/invirt-web/code/controls.py (revision 2096) +++ trunk/packages/invirt-web/code/controls.py (revision 2097) @@ -65,15 +65,14 @@ id of the CD (e.g. 'gutsy_i386') """ - try: - if cdtype is not None: - out = remctl('control', machine.name, 'create', - cdtype) - else: - out = remctl('control', machine.name, 'create') - except CodeError, e: - if 'already running' in e.message: - raise InvalidInput('action', 'create', - 'VM %s is already on' % machine.name) - else: + if cdtype is not None: + out, err = remctl('control', machine.name, 'create', + cdtype, err=True) + else: + out, err = remctl('control', machine.name, 'create', + err=True) + if 'already running' in err: + raise InvalidInput('action', 'create', + 'VM %s is already on' % machine.name) + elif err: raise CodeError('"%s" on "control %s create %s' % (err, machine.name, cdtype)) @@ -158,11 +157,14 @@ Gets and parses xm list --long """ - try: - value_string = remctl('control', machine.name, 'list-long') - except CodeError, e: - if 'is not on' in e.message: - return None - else: - raise + value_string, err_string = remctl('control', machine.name, 'list-long', + err=True) + if 'Unknown command' in err_string: + raise CodeError("ERROR in remctl list-long %s is not registered" % + (machine.name,)) + elif 'is not on' in err_string: + return None + elif err_string: + raise CodeError("ERROR in remctl list-long %s: %s" % + (machine.name, err_string)) status = parseStatus(value_string) return status @@ -170,7 +172,6 @@ def listHost(machine): """Return the host a machine is running on""" - try: - out = remctl('control', machine.name, 'listhost') - except CodeError, e: + out, err = remctl('control', machine.name, 'listhost', err=True) + if err: return None return out.strip() @@ -178,7 +179,6 @@ def vnctoken(machine): """Return a time-stamped VNC token""" - try: - out = remctl('control', machine.name, 'vnctoken') - except CodeError, e: + out, err = remctl('control', machine.name, 'vnctoken', err=True) + if err: return None return out.strip() @@ -186,8 +186,5 @@ def deleteVM(machine): """Delete a VM.""" - try: - remctl('control', machine.name, 'destroy') - except CodeError: - pass + remctl('control', machine.name, 'destroy', err=True) session.begin() delete_disk_pairs = [(machine.name, d.guest_device_name) @@ -219,15 +216,18 @@ raise CodeError("Invalid action '%s'" % action) if action == 'Reboot': - try: - if cdrom is not None: - out = remctl('control', machine.name, 'reboot', cdrom) - else: - out = remctl('control', machine.name, 'reboot') - except CodeError, e: - if re.match("machine '.*' is not on", e.message): + if cdrom is not None: + out, err = remctl('control', machine.name, 'reboot', cdrom, + err=True) + else: + out, err = remctl('control', machine.name, 'reboot', + err=True) + if err: + if re.match("machine '.*' is not on", err): raise InvalidInput("action", "reboot", "Machine is not on") else: - raise + print >> sys.stderr, 'Error on reboot:' + print >> sys.stderr, err + raise CodeError('ERROR on remctl') elif action == 'Power on': @@ -238,21 +238,23 @@ bootMachine(machine, cdrom) elif action == 'Power off': - try: - out = remctl('control', machine.name, 'destroy') - except CodeError, e: - if re.match("machine '.*' is not on", e.message): + out, err = remctl('control', machine.name, 'destroy', err=True) + if err: + if re.match("machine '.*' is not on", err): raise InvalidInput("action", "Power off", "Machine is not on.") else: - raise + print >> sys.stderr, 'Error on power off:' + print >> sys.stderr, err + raise CodeError('ERROR on remctl') elif action == 'Shutdown': - try: - out = remctl('control', machine.name, 'shutdown') - except CodeError, e: - if re.match("machine '.*' is not on", e.message): + out, err = remctl('control', machine.name, 'shutdown', err=True) + if err: + if re.match("machine '.*' is not on", err): raise InvalidInput("action", "Shutdown", "Machine is not on.") else: - raise + print >> sys.stderr, 'Error on Shutdown:' + print >> sys.stderr, err + raise CodeError('ERROR on remctl') elif action == 'Delete VM': deleteVM(machine)