Shriram Rajagopalan
2011-Jun-06 11:51 UTC
[Xen-devel] [PATCH 0 of 3] remus: cleanup python code
This patch series removes unused classes/code from Remus python modules and adds some exception handling to ensure a cleaner exit. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Shriram Rajagopalan
2011-Jun-06 11:51 UTC
[Xen-devel] [PATCH 1 of 3] remus: remove dead code and unused modules
# HG changeset patch # User Shriram Rajagopalan <rshriram@cs.ubc.ca> # Date 1307360439 25200 # Node ID d681b8ed0d03557f896cbc28d7ec244a2bc75bf2 # Parent 6f215b893873d61fc07c693c40466299973b41d4 remus: remove dead code and unused modules remove unused modules (profile, tapdisk, vdi) and unused thread classes/qdisc classes from the python code base. Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> diff -r 6f215b893873 -r d681b8ed0d03 tools/python/xen/remus/profile.py --- a/tools/python/xen/remus/profile.py Mon Jun 06 04:40:28 2011 -0700 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,56 +0,0 @@ -"""Simple profiling module -""" - -import time - -class ProfileBlock(object): - """A section of code to be profiled""" - def __init__(self, name): - self.name = name - - def enter(self): - print "PROF: entered %s at %f" % (self.name, time.time()) - - def exit(self): - print "PROF: exited %s at %f" % (self.name, time.time()) - -class NullProfiler(object): - def enter(self, name): - pass - - def exit(self, name=None): - pass - -class Profiler(object): - def __init__(self): - self.blocks = {} - self.running = [] - - def enter(self, name): - try: - block = self.blocks[name] - except KeyError: - block = ProfileBlock(name) - self.blocks[name] = block - - block.enter() - self.running.append(block) - - def exit(self, name=None): - if name is not None: - block = None - while self.running: - tmp = self.running.pop() - if tmp.name == name: - block = tmp - break - tmp.exit() - if not block: - raise KeyError(''block %s not running'' % name) - else: - try: - block = self.running.pop() - except IndexError: - raise KeyError(''no block running'') - - block.exit() diff -r 6f215b893873 -r d681b8ed0d03 tools/python/xen/remus/qdisc.py --- a/tools/python/xen/remus/qdisc.py Mon Jun 06 04:40:28 2011 -0700 +++ b/tools/python/xen/remus/qdisc.py Mon Jun 06 04:40:39 2011 -0700 @@ -109,43 +109,6 @@ qdisc_kinds[''prio''] = PrioQdisc qdisc_kinds[''pfifo_fast''] = PrioQdisc -class CfifoQdisc(Qdisc): - fmt = ''II'' - - def __init__(self, qdict): - super(CfifoQdisc, self).__init__(qdict) - - if qdict.get(''options''): - self.unpack(qdict[''options'']) - else: - self.epoch = 0 - self.vmid = 0 - - def pack(self): - return struct.pack(self.fmt, self.epoch, self.vmid) - - def unpack(self, opts): - self.epoch, self.vmid = struct.unpack(self.fmt, opts) - - def parse(self, opts): - args = list(opts) - try: - while args: - arg = args.pop(0) - if arg == ''epoch'': - self.epoch = int(args.pop(0)) - continue - if arg.lower() == ''vmid'': - self.vmid = int(args.pop(0)) - continue - except Exception, inst: - raise QdiscException(str(inst)) - - def optstr(self): - return ''epoch %d vmID %d'' % (self.epoch, self.vmid) - -qdisc_kinds[''cfifo''] = CfifoQdisc - TC_PLUG_CHECKPOINT = 0 TC_PLUG_RELEASE = 1 diff -r 6f215b893873 -r d681b8ed0d03 tools/python/xen/remus/save.py --- a/tools/python/xen/remus/save.py Mon Jun 06 04:40:28 2011 -0700 +++ b/tools/python/xen/remus/save.py Mon Jun 06 04:40:39 2011 -0700 @@ -1,8 +1,7 @@ #!/usr/bin/env python -import os, select, socket, threading, time, signal, xmlrpclib +import os, socket, xmlrpclib -from xen.xend.XendClient import server from xen.xend.xenstore.xswatch import xswatch import xen.lowlevel.xc @@ -13,10 +12,6 @@ import vm, image -XCFLAGS_LIVE = 1 - -xcsave = ''/usr/lib/xen/bin/xc_save'' - class _proxy(object): "proxy simulates an object without inheritance" def __init__(self, obj): @@ -30,58 +25,6 @@ class CheckpointError(Exception): pass -class CheckpointingFile(_proxy): - """Tee writes into separate file objects for each round. - This is necessary because xc_save gets a single file descriptor - for the duration of checkpointing. - """ - def __init__(self, path): - self.path = path - - self.round = 0 - self.rfd, self.wfd = os.pipe() - self.fd = file(path, ''wb'') - - # this pipe is used to notify the writer thread of checkpoints - self.cprfd, self.cpwfd = os.pipe() - - super(CheckpointingFile, self).__init__(self.fd) - - wt = threading.Thread(target=self._wrthread, name=''disk-write-thread'') - wt.setDaemon(True) - wt.start() - self.wt = wt - - def fileno(self): - return self.wfd - - def close(self): - os.close(self.wfd) - # closing wfd should signal writer to stop - self.wt.join() - os.close(self.rfd) - os.close(self.cprfd) - os.close(self.cpwfd) - self.fd.close() - self.wt = None - - def checkpoint(self): - os.write(self.cpwfd, ''1'') - - def _wrthread(self): - while True: - r, o, e = select.select((self.rfd, self.cprfd), (), ()) - if self.rfd in r: - data = os.read(self.rfd, 256 * 1024) - if not data: - break - self.fd.write(data) - if self.cprfd in r: - junk = os.read(self.cprfd, 1) - self.round += 1 - self.fd = file(''%s.%d'' % (self.path, self.round), ''wb'') - self.proxy(self.fd) - class MigrationSocket(_proxy): def __init__(self, address): sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) @@ -101,36 +44,6 @@ fd = os.fdopen(filedesc, ''w+'') super(NullSocket, self).__init__(fd) -class Keepalive(object): - "Call a keepalive method at intervals" - def __init__(self, method, interval=0.1): - self.keepalive = method - self.interval = interval - - self.thread = None - self.running = False - - def start(self): - if not self.interval: - return - self.thread = threading.Thread(target=self.run, name=''keepalive-thread'') - self.thread.setDaemon(True) - self.running = True - self.thread.start() - - def stop(self): - if not self.thread: - return - self.running = False - self.thread.join() - self.thread = None - - def run(self): - while self.running: - self.keepalive() - time.sleep(self.interval) - self.keepalive(stop=True) - class Saver(object): def __init__(self, domid, fd, suspendcb=None, resumecb=None, checkpointcb=None, interval=0): @@ -174,10 +87,5 @@ pass def _resume(self): - """low-overhead version of XendDomainInfo.resumeDomain""" - # TODO: currently assumes SUSPEND_CANCEL is available - if True: xc.domain_resume(self.vm.domid, 1) xsutil.ResumeDomain(self.vm.domid) - else: - server.xend.domain.resumeDomain(self.vm.domid) diff -r 6f215b893873 -r d681b8ed0d03 tools/python/xen/remus/tapdisk.py --- a/tools/python/xen/remus/tapdisk.py Mon Jun 06 04:40:28 2011 -0700 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,4 +0,0 @@ -import blkdev - -class TapDisk(BlkDev): - pass diff -r 6f215b893873 -r d681b8ed0d03 tools/python/xen/remus/vdi.py --- a/tools/python/xen/remus/vdi.py Mon Jun 06 04:40:28 2011 -0700 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,121 +0,0 @@ -#code to play with vdis and snapshots - -import os - -def run(cmd): - fd = os.popen(cmd) - res = [l for l in fd if l.rstrip()] - return not fd.close(), res - - -_blockstore = ''/blockstore.dat'' - -def set_blockstore(blockstore): - global _blockstore - __blockstore = blockstore - - -class SnapShot: - def __init__(self, vdi, block, index): - self.__vdi = vdi - self.__block = block - self.__index = index - - #TODO add snapshot date and radix - - def __str__(self): - return ''%d %d %d'' % (self.__vdi.id(), self.__block, self.__index) - - def vdi(self): - return self.__vdi - - def block(self): - return self.__block - - def index(self): - return self.__index - - def match(self, block, index): - return self.__block == block and self.__index == index - - -class VDIException(Exception): - pass - - -class VDI: - def __init__(self, id, name): - self.__id = id - self.__name = name - - def __str__(self): - return ''vdi: %d %s'' % (self.__id, self.__name) - - def id(self): - return self.__id - - def name(self): - return self.__name - - def list_snapshots(self): - res, ls = run(''vdi_snap_list %s %d'' % (_blockstore, self.__id)) - if res: - return [SnapShot(self, int(l[0]), int(l[1])) for l in [l.split() for l in ls[1:]]] - else: - raise VDIException("Error reading snapshot list") - - def snapshot(self): - res, ls = run(''vdi_checkpoint %s %d'' % (_blockstore, self.__id)) - if res: - _, block, idx = ls[0].split() - return SnapShot(self, int(block), int(idx)) - else: - raise VDIException("Error taking vdi snapshot") - - -def create(name, snap): - res, _ = run(''vdi_create %s %s %d %d'' - % (_blockstore, name, snap.block(), snap.index())) - if res: - return lookup_by_name(name) - else: - raise VDIException(''Unable to create vdi from snapshot'') - - -def fill(name, img_file): - res, _ = run(''vdi_create %s %s'' % (_blockstore, name)) - - if res: - vdi = lookup_by_name(name) - res, _ = run(''vdi_fill %d %s'' % (vdi.id(), img_file)) - if res: - return vdi - raise VDIException(''Unable to create vdi from disk img file'') - - -def list_vdis(): - vdis = [] - res, lines = run(''vdi_list %s'' % _blockstore) - if res: - for l in lines: - r = l.split() - vdis.append(VDI(int(r[0]), r[1])) - return vdis - else: - raise VDIException("Error doing vdi list") - - -def lookup_by_id(id): - vdis = list_vdis() - for v in vdis: - if v.id() == id: - return v - raise VDIException("No match from vdi id") - - -def lookup_by_name(name): - vdis = list_vdis() - for v in vdis: - if v.name() == name: - return v - raise VDIException("No match for vdi name") diff -r 6f215b893873 -r d681b8ed0d03 tools/python/xen/remus/vm.py --- a/tools/python/xen/remus/vm.py Mon Jun 06 04:40:28 2011 -0700 +++ b/tools/python/xen/remus/vm.py Mon Jun 06 04:40:39 2011 -0700 @@ -143,10 +143,6 @@ return [blkdev.parse(disk) for disk in disks] -def fromxend(domid): - "create a VM object from xend information" - return VM(domid) - def getshadowmem(vm): "Balloon down domain0 to create free memory for shadow paging." maxmem = int(vm.dom[''maxmem'']) diff -r 6f215b893873 -r d681b8ed0d03 tools/remus/remus --- a/tools/remus/remus Mon Jun 06 04:40:28 2011 -0700 +++ b/tools/remus/remus Mon Jun 06 04:40:39 2011 -0700 @@ -78,12 +78,9 @@ # I am not sure what the best way to die is. xm destroy is another option, # or we could attempt to trigger some instant reboot. print "dying..." - print util.runcmd([''sudo'', ''ifdown'', ''eth2'']) - # dangling imq0 handle on vif locks up the system for buf in bufs: buf.uninstall() print util.runcmd([''sudo'', ''xm'', ''destroy'', cfg.domid]) - print util.runcmd([''sudo'', ''ifup'', ''eth2'']) def getcommand(): """Get a command to execute while running. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Shriram Rajagopalan
2011-Jun-06 11:51 UTC
[Xen-devel] [PATCH 2 of 3] remus: move makeheader from image.py to vm.py
# HG changeset patch # User Shriram Rajagopalan <rshriram@cs.ubc.ca> # Date 1307360442 25200 # Node ID dc243b6893366c453834734e0b4b17a3f233daa2 # Parent d681b8ed0d03557f896cbc28d7ec244a2bc75bf2 remus: move makeheader from image.py to vm.py makeheader is the only function used from image.py. Move it to vm.py and remove image.py file. Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> diff -r d681b8ed0d03 -r dc243b689336 tools/python/xen/remus/image.py --- a/tools/python/xen/remus/image.py Mon Jun 06 04:40:39 2011 -0700 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,227 +0,0 @@ -# VM image file manipulation - -import logging, struct - -import vm - -SIGNATURE = ''LinuxGuestRecord'' -LONGLEN = struct.calcsize(''L'') -INTLEN = struct.calcsize(''i'') -PAGE_SIZE = 4096 -# ~0L -P2M_EXT_SIG = 4294967295L -# frames per page -FPP = 1024 -LTAB_MASK = 0xf << 28 -BATCH_SIZE = 1024 -IDXLEN = INTLEN + BATCH_SIZE * LONGLEN - -logging.basicConfig(level=logging.DEBUG) -log = logging.getLogger() - -class VMParseException(Exception): pass - -class VMImage(object): - def __init__(self, img=None): - """img may be a path or a file object. - If compact is True, apply checkpoints to base image instead - of simply concatenating them. - """ - self.img = img - - self.dom = None - self.fd = None - self.header = None - self.nr_pfns = 0 - # p2m extension header (unparsed) - self.p2mext = None - - if self.img: - self.open(self.img) - - def open(self, img): - if isinstance(img, str): - self.fd = file(img, ''rb'') - else: - self.fd = img - - self.readheader() - - def readheader(self): - sig = self.fd.read(len(SIGNATURE)) - if sig != SIGNATURE: - raise VMParseException("Bad signature in image") - - hlen = self.fd.read(INTLEN) - hlen, = struct.unpack(''!i'', hlen) - - self.header = self.fd.read(hlen) - self.dom = parseheader(self.header) - - def readp2mfl(self): - "read the P2M frame list" - pfnlen = self.fd.read(LONGLEN) - self.nr_pfns, = struct.unpack(''L'', pfnlen) - p2m0 = self.fd.read(LONGLEN) - - p2mhdr = p2m0 - p2m0, = struct.unpack(''L'', p2m0) - if p2m0 == P2M_EXT_SIG: - elen = self.fd.read(INTLEN) - elen, = struct.unpack(''I'', elen) - - self.p2mext = self.fd.read(elen) - - p2m0 = self.fd.read(LONGLEN) - p2m0, = struct.unpack(''L'', p2m0) - p2mfl = [p2m0] - - p2mfle = (self.nr_pfns + FPP - 1)/FPP - 1 - p2ms = self.fd.read(LONGLEN * p2mfle) - p2mfl.extend(struct.unpack(''%dL'' % p2mfle, p2ms)) - - self.p2mfl = p2mfl - - def flush(self): - self.ofd.write(self.tail) - -class Writer(object): - """compress a stream of checkpoints into a single image of the - last checkpoint""" - def __init__(self, fd, compact=False): - self.fd = fd - self.compact = compact - - self.vm = None - self.tail = None - # offset to first batch of pages - self.imgstart = 0 - # PFN mappings - self.pfns = [] - - def __del__(self): - self.close() - - def writeheader(self): - hlen = struct.pack(''!i'', len(self.vm.header)) - header = ''''.join([SIGNATURE, hlen, self.vm.header]) - self.fd.write(header) - - def writep2mfl(self): - p2m = [struct.pack(''L'', self.vm.nr_pfns)] - if self.vm.p2mext: - p2m.extend([struct.pack(''L'', P2M_EXT_SIG), self.vm.p2mext]) - p2m.append(struct.pack(''%dL'' % len(self.vm.p2mfl), *self.vm.p2mfl)) - self.fd.write(''''.join(p2m)) - - def writebatch(self, batch): - def offset(pfn): - isz = (pfn / BATCH_SIZE + 1) * IDXLEN - return self.imgstart + isz + pfn * PAGE_SIZE - - if not self.compact: - return self.fd.write(batch) - - batch = parsebatch(batch) - # sort pages for better disk seek behaviour - batch.sort(lambda x, y: cmp(x[0] & ~LTAB_MASK, y[0] & ~LTAB_MASK)) - - for pfndesc, page in batch: - pfn = pfndesc & ~LTAB_MASK - if pfn > self.vm.nr_pfns: - log.error(''INVALID PFN: %d'' % pfn) - if len(self.pfns) <= pfn: - self.pfns.extend([0] * (pfn - len(self.pfns) + 1)) - self.pfns[pfn] = pfndesc - self.fd.seek(offset(pfn)) - self.fd.write(page) - - #print "max offset: %d, %d" % (len(self.pfns), offset(self.pfns[-1])) - - def writeindex(self): - "Write batch header in front of each page" - hdrlen = INTLEN + BATCH_SIZE * LONGLEN - batches = (len(self.pfns) + BATCH_SIZE - 1) / BATCH_SIZE - - for i in xrange(batches): - offset = self.imgstart + i * (hdrlen + (PAGE_SIZE * BATCH_SIZE)) - pfnoff = i * BATCH_SIZE - # python auto-clamps overreads - pfns = self.pfns[pfnoff:pfnoff + BATCH_SIZE] - - self.fd.seek(offset) - self.fd.write(struct.pack(''i'', len(pfns))) - self.fd.write(struct.pack(''%dL'' % len(pfns), *pfns)) - - def slurp(self, ifd): - """Apply an incremental checkpoint to a loaded image. - accepts a path or a file object.""" - if isinstance(ifd, str): - ifd = file(ifd, ''rb'') - - if not self.vm: - self.vm = VMImage(ifd) - self.writeheader() - - self.vm.readp2mfl() - self.writep2mfl() - self.imgstart = self.fd.tell() - - while True: - l, batch = readbatch(ifd) - if l <= 0: - break - self.writebatch(batch) - self.tail = batch + ifd.read() - - def flush(self): - if self.tail: - self.fd.seek(0, 2) - self.fd.write(self.tail) - if self.compact: - self.writeindex() - self.tail = None - - def close(self): - self.flush() - -def parseheader(header): - "parses a header sexpression" - return vm.parsedominfo(vm.strtosxpr(header)) - -def makeheader(dominfo): - "create an image header from a VM dominfo sxpr" - items = [SIGNATURE] - sxpr = vm.sxprtostr(dominfo) - items.append(struct.pack(''!i'', len(sxpr))) - items.append(sxpr) - return ''''.join(items) - -def readbatch(fd): - batch = [] - batchlen = fd.read(INTLEN) - batch.append(batchlen) - batchlen, = struct.unpack(''i'', batchlen) - log.info("batch length: %d" % batchlen) - if batchlen <= 0: - return (batchlen, batch[0]) - - batchfns = fd.read(LONGLEN * batchlen) - batch.append(batchfns) - pages = fd.read(PAGE_SIZE * batchlen) - if len(pages) != PAGE_SIZE * batchlen: - log.error(''SHORT READ: %d'' % len(pages)) - batch.append(pages) - - return (batchlen, ''''.join(batch)) - -def parsebatch(batch): - "parse a batch string into pages" - batchlen, batch = batch[:INTLEN], batch[INTLEN:] - batchlen, = struct.unpack(''i'', batchlen) - #print ''batch length: %d'' % batchlen - pfnlen = batchlen * LONGLEN - pfns = struct.unpack(''%dL'' % batchlen, batch[:pfnlen]) - pagebuf = batch[pfnlen:] - pages = [pagebuf[i*PAGE_SIZE:(i+1)*PAGE_SIZE] for i in xrange(batchlen)] - return zip(pfns, pages) diff -r d681b8ed0d03 -r dc243b689336 tools/python/xen/remus/save.py --- a/tools/python/xen/remus/save.py Mon Jun 06 04:40:39 2011 -0700 +++ b/tools/python/xen/remus/save.py Mon Jun 06 04:40:42 2011 -0700 @@ -10,7 +10,7 @@ import xen.lowlevel.checkpoint -import vm, image +import vm class _proxy(object): "proxy simulates an object without inheritance" @@ -68,7 +68,7 @@ def start(self): vm.getshadowmem(self.vm) - hdr = image.makeheader(self.vm.dominfo) + hdr = vm.makeheader(self.vm.dominfo) self.fd.write(hdr) self.fd.flush() diff -r d681b8ed0d03 -r dc243b689336 tools/python/xen/remus/vm.py --- a/tools/python/xen/remus/vm.py Mon Jun 06 04:40:39 2011 -0700 +++ b/tools/python/xen/remus/vm.py Mon Jun 06 04:40:42 2011 -0700 @@ -1,6 +1,6 @@ #!/usr/bin/env python -import xmlrpclib +import xmlrpclib, struct from xen.xend.XendClient import server from xen.xend import sxp, osdep @@ -11,6 +11,7 @@ # need a nicer way to load disk drivers import vbd +SIGNATURE = ''LinuxGuestRecord'' class VMException(Exception): pass class VM(object): @@ -162,3 +163,11 @@ # target is in MB, not KB target = (dom0cur - needed) / 1024 server.xend.domain.setMemoryTarget(0, target) + +def makeheader(dominfo): + "create an image header from a VM dominfo sxpr" + items = [SIGNATURE] + sxpr = sxprtostr(dominfo) + items.append(struct.pack(''!i'', len(sxpr))) + items.append(sxpr) + return ''''.join(items) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Shriram Rajagopalan
2011-Jun-06 11:51 UTC
[Xen-devel] [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
# HG changeset patch # User Shriram Rajagopalan <rshriram@cs.ubc.ca> # Date 1307360451 25200 # Node ID 0af6803eeb9f278abd1739cb54ceb0fc2ed14470 # Parent dc243b6893366c453834734e0b4b17a3f233daa2 remus: handle exceptions while installing/unstalling net buffer Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> diff -r dc243b689336 -r 0af6803eeb9f tools/python/xen/remus/device.py --- a/tools/python/xen/remus/device.py Mon Jun 06 04:40:42 2011 -0700 +++ b/tools/python/xen/remus/device.py Mon Jun 06 04:40:51 2011 -0700 @@ -169,15 +169,21 @@ self.vif = vif # voodoo from http://www.linuxfoundation.org/collaborate/workgroups/networking/ifb#Typical_Usage util.runcmd(''ip link set %s up'' % self.devname) - util.runcmd(''tc qdisc add dev %s ingress'' % vif.dev) + try: + util.runcmd(''tc qdisc add dev %s ingress'' % vif.dev) + except: #RTNETLINK file exists error + ##since we already track ifbs via the /var/run/remus/ifb file, + ## RTNETLINK file already exists error is not an issue. + pass util.runcmd(''tc filter add dev %s parent ffff: proto ip pref 10 '' ''u32 match u32 0 0 action mirred egress redirect '' ''dev %s'' % (vif.dev, self.devname)) def uninstall(self): - util.runcmd(''tc filter del dev %s parent ffff: proto ip pref 10 u32'' \ - % self.vif.dev) - util.runcmd(''tc qdisc del dev %s ingress'' % self.vif.dev) + try: + util.runcmd(''tc qdisc del dev %s ingress'' % self.vif.dev) + except: + pass util.runcmd(''ip link set %s down'' % self.devname) class IMQBuffer(Netbuf): @@ -373,9 +379,14 @@ def uninstall(self): if self.installed: - req = qdisc.delrequest(self.bufdevno, self.handle) - self.rth.talk(req.pack()) - self.installed = False - - self.bufdev.uninstall() + try: + req = qdisc.delrequest(self.bufdevno, self.handle) + self.rth.talk(req.pack()) + self.installed = False + except: + pass + try: + self.bufdev.uninstall() + except: + pass self.pool.put(self.bufdev) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Brendan Cully
2011-Jun-07 19:27 UTC
Re: [Xen-devel] [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
Bare except clauses are bad style. Please only handle the exact exceptions you are expecting. Nacked-by: Brendan Cully <brendan@cs.ubc.ca> On 2011-06-06, at 4:51 AM, Shriram Rajagopalan wrote:> # HG changeset patch > # User Shriram Rajagopalan <rshriram@cs.ubc.ca> > # Date 1307360451 25200 > # Node ID 0af6803eeb9f278abd1739cb54ceb0fc2ed14470 > # Parent dc243b6893366c453834734e0b4b17a3f233daa2 > remus: handle exceptions while installing/unstalling net buffer > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> > > diff -r dc243b689336 -r 0af6803eeb9f tools/python/xen/remus/device.py > --- a/tools/python/xen/remus/device.py Mon Jun 06 04:40:42 2011 -0700 > +++ b/tools/python/xen/remus/device.py Mon Jun 06 04:40:51 2011 -0700 > @@ -169,15 +169,21 @@ > self.vif = vif > # voodoo from http://www.linuxfoundation.org/collaborate/workgroups/networking/ifb#Typical_Usage > util.runcmd(''ip link set %s up'' % self.devname) > - util.runcmd(''tc qdisc add dev %s ingress'' % vif.dev) > + try: > + util.runcmd(''tc qdisc add dev %s ingress'' % vif.dev) > + except: #RTNETLINK file exists error > + ##since we already track ifbs via the /var/run/remus/ifb file, > + ## RTNETLINK file already exists error is not an issue. > + pass > util.runcmd(''tc filter add dev %s parent ffff: proto ip pref 10 '' > ''u32 match u32 0 0 action mirred egress redirect '' > ''dev %s'' % (vif.dev, self.devname)) > > def uninstall(self): > - util.runcmd(''tc filter del dev %s parent ffff: proto ip pref 10 u32'' \ > - % self.vif.dev) > - util.runcmd(''tc qdisc del dev %s ingress'' % self.vif.dev) > + try: > + util.runcmd(''tc qdisc del dev %s ingress'' % self.vif.dev) > + except: > + pass > util.runcmd(''ip link set %s down'' % self.devname) > > class IMQBuffer(Netbuf): > @@ -373,9 +379,14 @@ > > def uninstall(self): > if self.installed: > - req = qdisc.delrequest(self.bufdevno, self.handle) > - self.rth.talk(req.pack()) > - self.installed = False > - > - self.bufdev.uninstall() > + try: > + req = qdisc.delrequest(self.bufdevno, self.handle) > + self.rth.talk(req.pack()) > + self.installed = False > + except: > + pass > + try: > + self.bufdev.uninstall() > + except: > + pass > self.pool.put(self.bufdev) > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-21 17:06 UTC
Re: [Xen-devel] [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
Brendan Cully writes ("Re: [Xen-devel] [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer"):> Bare except clauses are bad style. Please only handle the exact > exceptions you are expecting. > > Nacked-by: Brendan Cully <brendan@cs.ubc.ca>I agree. Shriram ? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Shriram Rajagopalan
2011-Jun-21 18:43 UTC
Re: [Xen-devel] [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
On Tue, Jun 21, 2011 at 1:06 PM, Ian Jackson <Ian.Jackson@eu.citrix.com>wrote:> Brendan Cully writes ("Re: [Xen-devel] [PATCH 3 of 3] remus: handle > exceptions while installing/unstalling net buffer"): > > Bare except clauses are bad style. Please only handle the exact > > exceptions you are expecting. > > > > Nacked-by: Brendan Cully <brendan@cs.ubc.ca> > > I agree. Shriram ? > > Ian. > > Yes, I ve been meaning to send out the revised version for a while. Willsend it out today. shriram _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel