Re: [PATCH] Add vdisk support

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

 



john.levon@xxxxxxx wrote:
> # HG changeset patch
> # User john.levon@xxxxxxx
> # Date 1228365031 28800
> # Node ID 152e0bfb277efb24679d7c7f440df0b0f6b21529
> # Parent  35baacfe79834400949ebdba69f952ed873ae442
> Add vdisk support
>
> Add support for the vdisk format used in Solaris.
>
> Signed-off-by: John Levon <john.levon@xxxxxxx>
>
> diff --git a/virt-install b/virt-install
> --- a/virt-install
> +++ b/virt-install
> @@ -189,9 +189,13 @@ def get_disk(disk, size, sparse, guest, 
>                                   readOnly=readOnly, device=device, bus=bus,
>                                   conn=guest.conn)
>          # Default file backed PV guests to tap driver
> -        if d.type == virtinst.VirtualDisk.TYPE_FILE \
> -           and not(hvm) and virtinst.util.is_blktap_capable():
> -            d.driver_name = virtinst.VirtualDisk.DRIVER_TAP
> +        if d.type == virtinst.VirtualDisk.TYPE_FILE and not(hvm):
> +            if virtinst.util.is_blktap_capable():
> +                d.driver_name = virtinst.VirtualDisk.DRIVER_TAP
> +            elif virtinst.util.is_vdisk(path):
> +                d.driver_name = Guest.VirtualDisk.DRIVER_TAP
> +                d.driver_type = Guest.VirtualDisk.DRIVER_TAP_VDISK
> +
>   

Hmm, this whole block should probably be moved into the VirtualDisk
class, and used to set the default driver if the user doesn't
specify one. But it's not a blocker.

>      except ValueError, e:
>          fail(_("Error with storage parameters: %s" % str(e)))
>  
> diff --git a/virtinst/CloneManager.py b/virtinst/CloneManager.py
> --- a/virtinst/CloneManager.py
> +++ b/virtinst/CloneManager.py
> @@ -19,7 +19,6 @@
>  # MA 02110-1301 USA.
>  
>  import os
> -import stat
>  import libxml2
>  import logging
>  import urlgrabber.progress as progress
> @@ -394,13 +393,9 @@ class CloneDesign(object):
>          logging.debug("original device list: %s" % (lst))
>  
>          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?

>          logging.debug("original device size: %s" % (size))
>          logging.debug("original device type: %s" % (typ))
>  
> @@ -443,18 +438,9 @@ class CloneDesign(object):
>          typ  = []
>  
>          for i in cln_dev_lst:
> -            if os.path.exists(i) ==  False:
> -                size.append(0)
> -                # if not exists, create file necessary
> -                typ.append(True)
> -                continue
> -            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)
>  
>          logging.debug("clone device list: %s" % (cln_dev_lst))
>          logging.debug("clone device size: %s" % (size))
> @@ -535,6 +521,14 @@ def _do_duplicate(design):
>              if src_dev == "/dev/null" or src_dev == dst_dev:
>                  meter.end(size)
>                  continue
> +
> +            if util.is_vdisk(src_dev) or (os.path.exists(dst_dev) and util.is_vdisk(dst_dev)):
> +                if not util.is_vdisk(src_dev) or os.path.exists(dst_dev):
> +                    raise RuntimeError, _("copying to an existing vdisk is not supported")
> +                if not util.vdisk_clone(src_dev, dst_dev):
> +                    raise RuntimeError, _("failed to clone disk")
> +                continue
> +
>              #
>              # create sparse file
>              # if a destination file exists and sparse flg is True,
> diff --git a/virtinst/Guest.py b/virtinst/Guest.py
> --- a/virtinst/Guest.py
> +++ b/virtinst/Guest.py
> @@ -474,6 +474,9 @@ class Installer(object):
>             or guest.disks[0].device != VirtualDisk.DEVICE_DISK:
>              return True
>  
> +        if util.is_vdisk(guest.disks[0].path):
> +            return True
> +
>          # Check for the 0xaa55 signature at the end of the MBR
>          try:
>              fd = os.open(guest.disks[0].path, os.O_RDONLY)
> diff --git a/virtinst/ImageManager.py b/virtinst/ImageManager.py
> --- a/virtinst/ImageManager.py
> +++ b/virtinst/ImageManager.py
> @@ -101,8 +101,13 @@ class ImageInstaller(Guest.Installer):
>              d = VirtualDisk(p, s,
>                              device = device,
>                              type = VirtualDisk.TYPE_FILE)
> -            if self.boot_caps.type == "xen" and util.is_blktap_capable():
> -                d.driver_name = VirtualDisk.DRIVER_TAP
> +            if util.is_vdisk(p):
> +                d.driver_name = Guest.VirtualDisk.DRIVER_TAP
> +                d.driver_type = Guest.VirtualDisk.DRIVER_TAP_VDISK
> +            else:
> +                if self.boot_caps.type == "xen" and util.is_blktap_capable():
> +                    d.driver_name = Guest.VirtualDisk.DRIVER_TAP
> +
>              d.target = m.target
>  
>              guest._install_disks.append(d)
> diff --git a/virtinst/VirtualDisk.py b/virtinst/VirtualDisk.py
> --- a/virtinst/VirtualDisk.py
> +++ b/virtinst/VirtualDisk.py
> @@ -68,7 +68,9 @@ class VirtualDisk(VirtualDevice):
>      DRIVER_TAP_RAW = "aio"
>      DRIVER_TAP_QCOW = "qcow"
>      DRIVER_TAP_VMDK = "vmdk"
> -    driver_types = [DRIVER_TAP_RAW, DRIVER_TAP_QCOW, DRIVER_TAP_VMDK]
> +    DRIVER_TAP_VDISK = "vdisk"
> +    driver_types = [DRIVER_TAP_RAW, DRIVER_TAP_QCOW,
> +        DRIVER_TAP_VMDK, DRIVER_TAP_VDISK]
>  
>      DEVICE_DISK = "disk"
>      DEVICE_CDROM = "cdrom"
> @@ -424,7 +426,8 @@ class VirtualDisk(VirtualDevice):
>                          or self.vol_object):
>              logging.debug("VirtualDisk storage exists.")
>  
> -            if using_path and os.path.isdir(self.path):
> +            if (using_path and os.path.isdir(self.path) and
> +                not util.is_vdisk(self.path)):
>                  raise ValueError, _("The path must be a file or a device,"
>                                      " not a directory")
>              self.__set_dev_type()
> @@ -476,9 +479,24 @@ class VirtualDisk(VirtualDevice):
>              self._set_vol_object(self.vol_install.install(meter=progresscb),
>                                   validate=False)
>              return
> -        elif self.type == VirtualDisk.TYPE_FILE and self.path is not None \
> -             and not os.path.exists(self.path):
> +        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.

> +            return
> +        elif (self.type == VirtualDisk.TYPE_FILE and self.path is not None
> +             and not os.path.exists(self.path)):
>              size_bytes = long(self.size * 1024L * 1024L * 1024L)
> +
> +            if util.is_vdisk(self.path):
> +                progresscb.update(1024)
> +                if (not util.vdisk_create(self.path, size_bytes, "vmdk",
> +                    self.sparse)):
> +                    raise RuntimeError, _("Error creating vdisk %s" % self.path)
> +                self._driverName = self.DRIVER_TAP
> +                self._driverType = self.DRIVER_TAP_VDISK
> +                progresscb.end(self.size)
> +                return
>  
>              if progresscb:
>                  progresscb.start(filename=self.path,size=long(size_bytes), \
> diff --git a/virtinst/util.py b/virtinst/util.py
> --- a/virtinst/util.py
> +++ b/virtinst/util.py
> @@ -29,6 +29,7 @@ import stat
>  import stat
>  import popen2
>  from sys import stderr
> +from subprocess import call
>  
>  import libvirt
>  from virtinst import _virtinst as _
> @@ -283,17 +284,71 @@ def xml_escape(str):
>      str = str.replace(">", "&gt;")
>      return str
>   
> -def blkdev_size(path):
> -    if platform.system() == 'SunOS':
> -        return os.stat(path)[stat.ST_SIZE]
> -    else:
> -        dummy, msg = commands.getstatusoutput('fdisk -s %s' % path)
> -        # check
> -        if msg.isdigit() == False:
> -            lines = msg.splitlines()
> -            # retry eg. for the GPT disk
> -            msg = lines[len(lines)-1]
> -        return (int(msg) * 1024)
> +def is_vdisk(path):
> +    if not os.path.exists("/usr/sbin/vdiskadm"):
> +        return False
> +    if not os.path.exists(path):
> +        return True
> +    if os.path.isdir(path) and \
> +       os.path.exists(path + "/vdisk.xml"):
> +        return True
> +    return False
> +
> +def vdisk_create(path, size, kind, sparse = True):
> +    force_fixed = "raw"
> +    path = os.path.expanduser(path)
> +    if kind in force_fixed or not sparse:
> +        type = kind + ":fixed"
> +    else:
> +        type = kind + ":sparse"
> +    args = " create -t " + type + " -s " + str(size) + " " + path
> +    try:
> +        rc = call("/usr/sbin/vdiskadm" + args, shell=True)
> +        if rc != 0:
> +            return False
> +        return True
> +    except OSError, e:
> +        return False
> +
> +def vdisk_clone(path, clone):
> +    path = os.path.expanduser(path)
> +    clone = os.path.expanduser(clone)
> +    args = path + " " + clone
> +    try:
> +        rc = call("/usr/sbin/vdiskadm clone " + args, shell=True)
> +        if rc != 0:
> +            return False
> +        return True
> +    except OSError, e:
> +        return False
> +
>   

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.

> +def stat_disk(path):
> +    """Returns the tuple (isreg, size)."""
> +    if not os.path.exists(path):
> +        return True, 0
> +
> +    if is_vdisk(path):
> +        size = int(commands.getoutput(
> +            "vdiskadm prop-get -p max-size " + path))
> +        return True, size
> +
> +    mode = os.stat(path)[stat.ST_MODE]
> +    if stat.S_ISBLK(mode):
> +        if platform.system() == 'SunOS':
> +            size = os.stat(path)[stat.ST_SIZE]
> +        else:
> +            dummy, msg = commands.getstatusoutput('fdisk -s %s' % path)
> +            # check
> +            if msg.isdigit() == False:
> +                lines = msg.splitlines()
> +                # retry eg. for the GPT disk
> +                msg = lines[len(lines)-1]
> +            size = int(msg) * 1024
> +        return False, size
> +    elif stat.S_ISREG(mode):
> +        return True, os.path.getsize(path)
> +
> +    return True, 0
>  
>  def compareMAC(p, q):
>      """Compare two MAC addresses""

Thanks,
Cole

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

[Index of Archives]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux