All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0 of 3] [Resend v2] remus: cleanup python code
@ 2011-06-22 13:37 Shriram Rajagopalan
  2011-06-22 13:37 ` [PATCH 1 of 3] remus: remove dead code and unused modules Shriram Rajagopalan
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Shriram Rajagopalan @ 2011-06-22 13:37 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

This patch series removes unused classes/code from Remus python
modules and adds some exception handling to ensure a cleaner exit.

Changes since last revision:
 Patch 3/3: Handle specific exceptions when installing/uninstalling
 net buffers, instead of blank exceptions.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1 of 3] remus: remove dead code and unused modules
  2011-06-22 13:37 [PATCH 0 of 3] [Resend v2] remus: cleanup python code Shriram Rajagopalan
@ 2011-06-22 13:37 ` Shriram Rajagopalan
  2011-06-22 22:30   ` Brendan Cully
  2011-06-22 13:37 ` [PATCH 2 of 3] remus: move makeheader from image.py to vm.py Shriram Rajagopalan
  2011-06-22 13:37 ` [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer Shriram Rajagopalan
  2 siblings, 1 reply; 14+ messages in thread
From: Shriram Rajagopalan @ 2011-06-22 13:37 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1308683501 25200
# Node ID ca4a8d0d504344a84f64bc7e939f8910baac236e
# Parent  c31e9249893d309655a8e739ba2ecb07d2c0148b
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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/profile.py
--- a/tools/python/xen/remus/profile.py	Sat Jun 18 20:52:07 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/qdisc.py
--- a/tools/python/xen/remus/qdisc.py	Sat Jun 18 20:52:07 2011 -0700
+++ b/tools/python/xen/remus/qdisc.py	Tue Jun 21 12:11:41 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/save.py
--- a/tools/python/xen/remus/save.py	Sat Jun 18 20:52:07 2011 -0700
+++ b/tools/python/xen/remus/save.py	Tue Jun 21 12:11:41 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, flags=0):
@@ -177,10 +90,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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/tapdisk.py
--- a/tools/python/xen/remus/tapdisk.py	Sat Jun 18 20:52:07 2011 -0700
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,4 +0,0 @@
-import blkdev
-
-class TapDisk(BlkDev):
-    pass
diff -r c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/vdi.py
--- a/tools/python/xen/remus/vdi.py	Sat Jun 18 20:52:07 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/vm.py
--- a/tools/python/xen/remus/vm.py	Sat Jun 18 20:52:07 2011 -0700
+++ b/tools/python/xen/remus/vm.py	Tue Jun 21 12:11:41 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 c31e9249893d -r ca4a8d0d5043 tools/remus/remus
--- a/tools/remus/remus	Sat Jun 18 20:52:07 2011 -0700
+++ b/tools/remus/remus	Tue Jun 21 12:11:41 2011 -0700
@@ -86,12 +86,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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2 of 3] remus: move makeheader from image.py to vm.py
  2011-06-22 13:37 [PATCH 0 of 3] [Resend v2] remus: cleanup python code Shriram Rajagopalan
  2011-06-22 13:37 ` [PATCH 1 of 3] remus: remove dead code and unused modules Shriram Rajagopalan
@ 2011-06-22 13:37 ` Shriram Rajagopalan
  2011-06-22 22:32   ` Brendan Cully
  2011-06-22 13:37 ` [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer Shriram Rajagopalan
  2 siblings, 1 reply; 14+ messages in thread
From: Shriram Rajagopalan @ 2011-06-22 13:37 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1308683504 25200
# Node ID 794ead1a2be0578d70c38f006e7ab61c7abe9203
# Parent  ca4a8d0d504344a84f64bc7e939f8910baac236e
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 ca4a8d0d5043 -r 794ead1a2be0 tools/python/xen/remus/image.py
--- a/tools/python/xen/remus/image.py	Tue Jun 21 12:11:41 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 ca4a8d0d5043 -r 794ead1a2be0 tools/python/xen/remus/save.py
--- a/tools/python/xen/remus/save.py	Tue Jun 21 12:11:41 2011 -0700
+++ b/tools/python/xen/remus/save.py	Tue Jun 21 12:11:44 2011 -0700
@@ -10,7 +10,7 @@
 
 import xen.lowlevel.checkpoint
 
-import vm, image
+import vm
 
 class _proxy(object):
     "proxy simulates an object without inheritance"
@@ -70,7 +70,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 ca4a8d0d5043 -r 794ead1a2be0 tools/python/xen/remus/vm.py
--- a/tools/python/xen/remus/vm.py	Tue Jun 21 12:11:41 2011 -0700
+++ b/tools/python/xen/remus/vm.py	Tue Jun 21 12:11:44 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)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
  2011-06-22 13:37 [PATCH 0 of 3] [Resend v2] remus: cleanup python code Shriram Rajagopalan
  2011-06-22 13:37 ` [PATCH 1 of 3] remus: remove dead code and unused modules Shriram Rajagopalan
  2011-06-22 13:37 ` [PATCH 2 of 3] remus: move makeheader from image.py to vm.py Shriram Rajagopalan
@ 2011-06-22 13:37 ` Shriram Rajagopalan
  2011-06-23 15:32   ` Shriram Rajagopalan
  2 siblings, 1 reply; 14+ messages in thread
From: Shriram Rajagopalan @ 2011-06-22 13:37 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Shriram Rajagopalan <rshriram@cs.ubc.ca>
# Date 1308749695 25200
# Node ID 9eed27800ff6a2e6d73f138f20af072c1b41925e
# Parent  794ead1a2be0578d70c38f006e7ab61c7abe9203
remus: handle exceptions while installing/unstalling net buffer

Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>

diff -r 794ead1a2be0 -r 9eed27800ff6 tools/python/xen/remus/device.py
--- a/tools/python/xen/remus/device.py	Tue Jun 21 12:11:44 2011 -0700
+++ b/tools/python/xen/remus/device.py	Wed Jun 22 06:34:55 2011 -0700
@@ -169,15 +169,25 @@
         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 util.PipeException, e:
+            # check if error indicates that ingress qdisc
+            # already exists on the vif. If so, ignore it.
+            ignoreme = 'RTNETLINK answers: File exists'
+            if ignoreme in str(e):
+                pass
+            else:
+                raise e
         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 util.PipeException, e:
+            pass
         util.runcmd('ip link set %s down' % self.devname)
 
 class IMQBuffer(Netbuf):
@@ -373,9 +383,15 @@
 
     def uninstall(self):
         if self.installed:
-            req = qdisc.delrequest(self.bufdevno, self.handle)
-            self.rth.talk(req.pack())
+            try:
+                req = qdisc.delrequest(self.bufdevno, self.handle)
+                self.rth.talk(req.pack())
+            except IOError, e:
+                pass
             self.installed = False
 
-        self.bufdev.uninstall()
-        self.pool.put(self.bufdev)
+            try:
+                self.bufdev.uninstall()
+            except util.PipeException, e:
+                pass
+            self.pool.put(self.bufdev)
diff -r 794ead1a2be0 -r 9eed27800ff6 tools/python/xen/remus/util.py
--- a/tools/python/xen/remus/util.py	Tue Jun 21 12:11:44 2011 -0700
+++ b/tools/python/xen/remus/util.py	Wed Jun 22 06:34:55 2011 -0700
@@ -65,8 +65,10 @@
         proc.wait()
         if proc.returncode:
             print ' '.join(args)
-            print stderr.strip()
-            raise PipeException('%s failed' % args[0], proc.returncode)
+            errmsg = stderr.strip()
+            print errmsg
+            raise PipeException('%s failed (errmsg: %s)' % (args[0], errmsg),
+                                proc.returncode)
         return stdout
     except (OSError, IOError), inst:
         raise PipeException('could not run %s' % args[0], inst.errno)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1 of 3] remus: remove dead code and unused modules
  2011-06-22 13:37 ` [PATCH 1 of 3] remus: remove dead code and unused modules Shriram Rajagopalan
@ 2011-06-22 22:30   ` Brendan Cully
  2011-06-23 13:08     ` Shriram Rajagopalan
  0 siblings, 1 reply; 14+ messages in thread
From: Brendan Cully @ 2011-06-22 22:30 UTC (permalink / raw)
  To: Shriram Rajagopalan; +Cc: xen-devel, ian.jackson

Some of this code is indeed obsolete or otherwise not useful, but a lot of it (like CheckpointingFile) is, I think, useful for people who might want to hack on Remus. Specifically, I agree with removing vdi.py and with your change to tools/remus/remus, I am neutral on tapdisk.py, and I would prefer to keep the rest.

On 2011-06-22, at 6:37 AM, Shriram Rajagopalan wrote:

> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> # Date 1308683501 25200
> # Node ID ca4a8d0d504344a84f64bc7e939f8910baac236e
> # Parent  c31e9249893d309655a8e739ba2ecb07d2c0148b
> 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/profile.py
> --- a/tools/python/xen/remus/profile.py	Sat Jun 18 20:52:07 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/qdisc.py
> --- a/tools/python/xen/remus/qdisc.py	Sat Jun 18 20:52:07 2011 -0700
> +++ b/tools/python/xen/remus/qdisc.py	Tue Jun 21 12:11:41 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/save.py
> --- a/tools/python/xen/remus/save.py	Sat Jun 18 20:52:07 2011 -0700
> +++ b/tools/python/xen/remus/save.py	Tue Jun 21 12:11:41 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, flags=0):
> @@ -177,10 +90,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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/tapdisk.py
> --- a/tools/python/xen/remus/tapdisk.py	Sat Jun 18 20:52:07 2011 -0700
> +++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
> @@ -1,4 +0,0 @@
> -import blkdev
> -
> -class TapDisk(BlkDev):
> -    pass
> diff -r c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/vdi.py
> --- a/tools/python/xen/remus/vdi.py	Sat Jun 18 20:52:07 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/vm.py
> --- a/tools/python/xen/remus/vm.py	Sat Jun 18 20:52:07 2011 -0700
> +++ b/tools/python/xen/remus/vm.py	Tue Jun 21 12:11:41 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 c31e9249893d -r ca4a8d0d5043 tools/remus/remus
> --- a/tools/remus/remus	Sat Jun 18 20:52:07 2011 -0700
> +++ b/tools/remus/remus	Tue Jun 21 12:11:41 2011 -0700
> @@ -86,12 +86,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
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2 of 3] remus: move makeheader from image.py to vm.py
  2011-06-22 13:37 ` [PATCH 2 of 3] remus: move makeheader from image.py to vm.py Shriram Rajagopalan
@ 2011-06-22 22:32   ` Brendan Cully
  0 siblings, 0 replies; 14+ messages in thread
From: Brendan Cully @ 2011-06-22 22:32 UTC (permalink / raw)
  To: Shriram Rajagopalan; +Cc: xen-devel, ian.jackson

I think this code is useful for Remus or live migration hackers too (we don't have python parsers for the save format elsewhere that I know of), and would prefer to keep it as is.

On 2011-06-22, at 6:37 AM, Shriram Rajagopalan wrote:

> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> # Date 1308683504 25200
> # Node ID 794ead1a2be0578d70c38f006e7ab61c7abe9203
> # Parent  ca4a8d0d504344a84f64bc7e939f8910baac236e
> 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 ca4a8d0d5043 -r 794ead1a2be0 tools/python/xen/remus/image.py
> --- a/tools/python/xen/remus/image.py	Tue Jun 21 12:11:41 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 ca4a8d0d5043 -r 794ead1a2be0 tools/python/xen/remus/save.py
> --- a/tools/python/xen/remus/save.py	Tue Jun 21 12:11:41 2011 -0700
> +++ b/tools/python/xen/remus/save.py	Tue Jun 21 12:11:44 2011 -0700
> @@ -10,7 +10,7 @@
> 
> import xen.lowlevel.checkpoint
> 
> -import vm, image
> +import vm
> 
> class _proxy(object):
>     "proxy simulates an object without inheritance"
> @@ -70,7 +70,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 ca4a8d0d5043 -r 794ead1a2be0 tools/python/xen/remus/vm.py
> --- a/tools/python/xen/remus/vm.py	Tue Jun 21 12:11:41 2011 -0700
> +++ b/tools/python/xen/remus/vm.py	Tue Jun 21 12:11:44 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
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1 of 3] remus: remove dead code and unused modules
  2011-06-22 22:30   ` Brendan Cully
@ 2011-06-23 13:08     ` Shriram Rajagopalan
  2011-06-23 14:34       ` Brendan Cully
  0 siblings, 1 reply; 14+ messages in thread
From: Shriram Rajagopalan @ 2011-06-23 13:08 UTC (permalink / raw)
  To: Brendan Cully; +Cc: xen-devel, ian.jackson


[-- Attachment #1.1: Type: text/plain, Size: 14233 bytes --]

On Wed, Jun 22, 2011 at 6:30 PM, Brendan Cully <brendan@cs.ubc.ca> wrote:

> Some of this code is indeed obsolete or otherwise not useful, but a lot of
> it (like CheckpointingFile) is, I think, useful for people who might want to
> hack on Remus. Specifically, I agree with removing vdi.py and with your
> change to tools/remus/remus, I am neutral on tapdisk.py, and I would prefer
> to keep the rest.
>

Please specify what goes into "a lot of it" and "rest"

 On 2011-06-22, at 6:37 AM, Shriram Rajagopalan wrote:
>
> > # HG changeset patch
> > # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> > # Date 1308683501 25200
> > # Node ID ca4a8d0d504344a84f64bc7e939f8910baac236e
> > # Parent  c31e9249893d309655a8e739ba2ecb07d2c0148b
> > 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/profile.py
> > --- a/tools/python/xen/remus/profile.py       Sat Jun 18 20:52:07 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/qdisc.py
> > --- a/tools/python/xen/remus/qdisc.py Sat Jun 18 20:52:07 2011 -0700
> > +++ b/tools/python/xen/remus/qdisc.py Tue Jun 21 12:11:41 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/save.py
> > --- a/tools/python/xen/remus/save.py  Sat Jun 18 20:52:07 2011 -0700
> > +++ b/tools/python/xen/remus/save.py  Tue Jun 21 12:11:41 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, flags=0):
> > @@ -177,10 +90,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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/tapdisk.py
> > --- a/tools/python/xen/remus/tapdisk.py       Sat Jun 18 20:52:07 2011
> -0700
> > +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
> > @@ -1,4 +0,0 @@
> > -import blkdev
> > -
> > -class TapDisk(BlkDev):
> > -    pass
> > diff -r c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/vdi.py
> > --- a/tools/python/xen/remus/vdi.py   Sat Jun 18 20:52:07 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/vm.py
> > --- a/tools/python/xen/remus/vm.py    Sat Jun 18 20:52:07 2011 -0700
> > +++ b/tools/python/xen/remus/vm.py    Tue Jun 21 12:11:41 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 c31e9249893d -r ca4a8d0d5043 tools/remus/remus
> > --- a/tools/remus/remus       Sat Jun 18 20:52:07 2011 -0700
> > +++ b/tools/remus/remus       Tue Jun 21 12:11:41 2011 -0700
> > @@ -86,12 +86,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
> >
>
>

[-- Attachment #1.2: Type: text/html, Size: 18366 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1 of 3] remus: remove dead code and unused modules
  2011-06-23 13:08     ` Shriram Rajagopalan
@ 2011-06-23 14:34       ` Brendan Cully
  0 siblings, 0 replies; 14+ messages in thread
From: Brendan Cully @ 2011-06-23 14:34 UTC (permalink / raw)
  To: Shriram Rajagopalan; +Cc: xen-devel, ian.jackson

On Thursday, 23 June 2011 at 09:08, Shriram Rajagopalan wrote:
> On Wed, Jun 22, 2011 at 6:30 PM, Brendan Cully <brendan@cs.ubc.ca> wrote:
> 
> > Some of this code is indeed obsolete or otherwise not useful, but a lot of
> > it (like CheckpointingFile) is, I think, useful for people who might want to
> > hack on Remus. Specifically, I agree with removing vdi.py and with your
> > change to tools/remus/remus, I am neutral on tapdisk.py, and I would prefer
> > to keep the rest.
> >
> 
> Please specify what goes into "a lot of it" and "rest"

Did the second sentence of the original email not make that clear?

>  On 2011-06-22, at 6:37 AM, Shriram Rajagopalan wrote:
> >
> > > # HG changeset patch
> > > # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> > > # Date 1308683501 25200
> > > # Node ID ca4a8d0d504344a84f64bc7e939f8910baac236e
> > > # Parent  c31e9249893d309655a8e739ba2ecb07d2c0148b
> > > 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/profile.py
> > -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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/qdisc.py
> > > +++ b/tools/python/xen/remus/qdisc.py Tue Jun 21 12:11:41 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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/save.py
> > > +++ b/tools/python/xen/remus/save.py  Tue Jun 21 12:11:41 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, flags=0):
> > > @@ -177,10 +90,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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/tapdisk.py
> > -0700
> > > +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
> > > @@ -1,4 +0,0 @@
> > > -import blkdev
> > > -
> > > -class TapDisk(BlkDev):
> > > -    pass
> > > diff -r c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/vdi.py
> > > +++ /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 c31e9249893d -r ca4a8d0d5043 tools/python/xen/remus/vm.py
> > > +++ b/tools/python/xen/remus/vm.py    Tue Jun 21 12:11:41 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 c31e9249893d -r ca4a8d0d5043 tools/remus/remus
> > > +++ b/tools/remus/remus       Tue Jun 21 12:11:41 2011 -0700
> > > @@ -86,12 +86,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
> > >
> >
> >

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
  2011-06-22 13:37 ` [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer Shriram Rajagopalan
@ 2011-06-23 15:32   ` Shriram Rajagopalan
  2011-06-27 14:19     ` Ian Jackson
  0 siblings, 1 reply; 14+ messages in thread
From: Shriram Rajagopalan @ 2011-06-23 15:32 UTC (permalink / raw)
  To: ian.jackson; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 3967 bytes --]

On Wed, Jun 22, 2011 at 9:37 AM, Shriram Rajagopalan <rshriram@cs.ubc.ca>wrote:

> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@cs.ubc.ca>
> # Date 1308749695 25200
> # Node ID 9eed27800ff6a2e6d73f138f20af072c1b41925e
> # Parent  794ead1a2be0578d70c38f006e7ab61c7abe9203
> remus: handle exceptions while installing/unstalling net buffer
>
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>
> diff -r 794ead1a2be0 -r 9eed27800ff6 tools/python/xen/remus/device.py
> --- a/tools/python/xen/remus/device.py  Tue Jun 21 12:11:44 2011 -0700
> +++ b/tools/python/xen/remus/device.py  Wed Jun 22 06:34:55 2011 -0700
> @@ -169,15 +169,25 @@
>         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 util.PipeException, e:
> +            # check if error indicates that ingress qdisc
> +            # already exists on the vif. If so, ignore it.
> +            ignoreme = 'RTNETLINK answers: File exists'
> +            if ignoreme in str(e):
> +                pass
> +            else:
> +                raise e
>         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 util.PipeException, e:
> +            pass
>         util.runcmd('ip link set %s down' % self.devname)
>
>  class IMQBuffer(Netbuf):
> @@ -373,9 +383,15 @@
>
>     def uninstall(self):
>         if self.installed:
> -            req = qdisc.delrequest(self.bufdevno, self.handle)
> -            self.rth.talk(req.pack())
> +            try:
> +                req = qdisc.delrequest(self.bufdevno, self.handle)
> +                self.rth.talk(req.pack())
> +            except IOError, e:
> +                pass
>             self.installed = False
>
> -        self.bufdev.uninstall()
> -        self.pool.put(self.bufdev)
> +            try:
> +                self.bufdev.uninstall()
> +            except util.PipeException, e:
> +                pass
> +            self.pool.put(self.bufdev)
> diff -r 794ead1a2be0 -r 9eed27800ff6 tools/python/xen/remus/util.py
> --- a/tools/python/xen/remus/util.py    Tue Jun 21 12:11:44 2011 -0700
> +++ b/tools/python/xen/remus/util.py    Wed Jun 22 06:34:55 2011 -0700
> @@ -65,8 +65,10 @@
>         proc.wait()
>         if proc.returncode:
>             print ' '.join(args)
> -            print stderr.strip()
> -            raise PipeException('%s failed' % args[0], proc.returncode)
> +            errmsg = stderr.strip()
> +            print errmsg
> +            raise PipeException('%s failed (errmsg: %s)' % (args[0],
> errmsg),
> +                                proc.returncode)
>         return stdout
>     except (OSError, IOError), inst:
>         raise PipeException('could not run %s' % args[0], inst.errno)
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

As Brendan pointed out, 90% of what I proposed to remove
in patches 1 & 2 of this series might be useful for folks who want to hack
on Remus
or live migration code. I ll do the clean up later, more carefully.

Ian, if you have no objection to this patch, could you take in this one
alone and
reject patches 1 & 2? Or do you want me to send this one separately again?

shriram

[-- Attachment #1.2: Type: text/html, Size: 4971 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
  2011-06-23 15:32   ` Shriram Rajagopalan
@ 2011-06-27 14:19     ` Ian Jackson
  0 siblings, 0 replies; 14+ messages in thread
From: Ian Jackson @ 2011-06-27 14:19 UTC (permalink / raw)
  To: rshriram; +Cc: xen-devel

Shriram Rajagopalan writes ("Re: [Xen-devel] [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer"):
> As Brendan pointed out, 90% of what I proposed to remove in patches
> 1 & 2 of this series might be useful for folks who want to hack on
> Remus or live migration code. I ll do the clean up later, more
> carefully.

Right, OK.

> Ian, if you have no objection to this patch, could you take in this
> one alone and reject patches 1 & 2? Or do you want me to send this
> one separately again?

I have acked and applied the error exception patch.

Thanks,
Ian.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
  2011-06-21 17:06     ` Ian Jackson
@ 2011-06-21 18:43       ` Shriram Rajagopalan
  0 siblings, 0 replies; 14+ messages in thread
From: Shriram Rajagopalan @ 2011-06-21 18:43 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Brendan Cully, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 498 bytes --]

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. Will
send it out today.

shriram

[-- Attachment #1.2: Type: text/html, Size: 915 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
  2011-06-07 19:27   ` Brendan Cully
@ 2011-06-21 17:06     ` Ian Jackson
  2011-06-21 18:43       ` Shriram Rajagopalan
  0 siblings, 1 reply; 14+ messages in thread
From: Ian Jackson @ 2011-06-21 17:06 UTC (permalink / raw)
  To: Brendan Cully; +Cc: Shriram Rajagopalan, xen-devel, ian.jackson

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
  2011-06-06 11:51 ` [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer Shriram Rajagopalan
@ 2011-06-07 19:27   ` Brendan Cully
  2011-06-21 17:06     ` Ian Jackson
  0 siblings, 1 reply; 14+ messages in thread
From: Brendan Cully @ 2011-06-07 19:27 UTC (permalink / raw)
  To: Shriram Rajagopalan; +Cc: xen-devel, ian.jackson

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
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer
  2011-06-06 11:51 [PATCH 0 of 3] remus: cleanup python code Shriram Rajagopalan
@ 2011-06-06 11:51 ` Shriram Rajagopalan
  2011-06-07 19:27   ` Brendan Cully
  0 siblings, 1 reply; 14+ messages in thread
From: Shriram Rajagopalan @ 2011-06-06 11:51 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# 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)

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-06-27 14:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-22 13:37 [PATCH 0 of 3] [Resend v2] remus: cleanup python code Shriram Rajagopalan
2011-06-22 13:37 ` [PATCH 1 of 3] remus: remove dead code and unused modules Shriram Rajagopalan
2011-06-22 22:30   ` Brendan Cully
2011-06-23 13:08     ` Shriram Rajagopalan
2011-06-23 14:34       ` Brendan Cully
2011-06-22 13:37 ` [PATCH 2 of 3] remus: move makeheader from image.py to vm.py Shriram Rajagopalan
2011-06-22 22:32   ` Brendan Cully
2011-06-22 13:37 ` [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer Shriram Rajagopalan
2011-06-23 15:32   ` Shriram Rajagopalan
2011-06-27 14:19     ` Ian Jackson
  -- strict thread matches above, loose matches on Subject: below --
2011-06-06 11:51 [PATCH 0 of 3] remus: cleanup python code Shriram Rajagopalan
2011-06-06 11:51 ` [PATCH 3 of 3] remus: handle exceptions while installing/unstalling net buffer Shriram Rajagopalan
2011-06-07 19:27   ` Brendan Cully
2011-06-21 17:06     ` Ian Jackson
2011-06-21 18:43       ` Shriram Rajagopalan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.