[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [et-mgmt-tools] [PATCH] Add vdisk support



On Mon, Dec 08, 2008 at 10:09:16PM -0500, Cole Robinson wrote:

> >          for i in lst:
> > -            mode = os.stat(i)[stat.ST_MODE]
> > -            if stat.S_ISBLK(mode):
> > -                size.append(util.blkdev_size(i))
> > -                typ.append(False)
> > -            elif stat.S_ISREG(mode):
> > -                size.append(os.path.getsize(i))
> > -                typ.append(True)
> > +            (t, sz) = util.stat_disk(i)
> > +            typ.append(t)
> > +            size.append(sz)
> 
> This, and all the other stat_disk code, seems like a separate cleanup.
> Could we get this in a separate patch?

It's not exactly an exciting change without the vdisk part, but OK.

> > +        elif (self.type == VirtualDisk.TYPE_FILE and self.path is not None
> > +            and os.path.exists(self.path) and util.is_vdisk(self.path)):
> > +            self._driverName = self.DRIVER_TAP
> > +            self._driverType = self.DRIVER_TAP_VDISK
> >   
> 
> This could use a debug statement stating we are forcing the vdisk
> driver.

I'll add a debug statement, but we're not forcing it: if we detect a
vdisk, then we know we will have to use vdisk to read it.

> > +def vdisk_create(path, size, kind, sparse = True):
> > +def vdisk_clone(path, clone):
> 
> These two can be moved to VirtualDisk and CloneManager respectively
> as non class functions. Please prefix them with an underscore to
> mark them as private.

OK.

regards
john


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]