[XEND] Fix bugs in vbd_create logic when handling VDIs

Attempt to handle 'tap' devices as 'vbd' devices even though the
internal configuration has no idea that 'tap' is a subclass of 'vbd'.

Add option parsing to vbd-create and vdi-create so you can override
the configuration file options.

Signed-off-by: Alastair Tse <atse@xensource.com>

tools/python/scripts/xapi.py

@@ -35,7 +35,9 @@ LOGIN = ('atse', 'passwd')
 COMMANDS = {
     'host-info': ('', 'Get Xen Host Info'),
     'sr-list':   ('', 'List all SRs'),
-    'vbd-create': ('<domname> <pycfg>', 'Create VBD attached to domname'),
+    'vbd-create': ('<domname> <pycfg> [opts]',
+                   'Create VBD attached to domname'),
+    'vdi-create': ('<pycfg> [opts]', 'Create a VDI'),
     'vdi-list'  : ('', 'List all VDI'),
     'vdi-rename': ('<vdi_uuid> <new_name>', 'Rename VDI'),
     'vdi-delete': ('<vdi_uuid>', 'Delete VDI'),
@@ -57,7 +59,28 @@ OPTIONS = {
                  {'action':'store_true',
                   'help':'List all properties of VMs'})
                ],
-   
+    
+    'vdi-create': [(('--label',), {'help': 'Name for VDI'}),
+                   (('--description',), {'help': 'Description for VDI'}),
+                   (('--sector-size',), {'type': 'int',
+                                         'help': 'Sector size'}),
+                   (('--virtual-size',), {'type': 'int',
+                                          'help': 'Size of VDI in sectors'}),
+                   (('--type',), {'choices': ['system', 'user', 'ephemeral'],
+                                  'help': 'VDI type'}),
+                   (('--sharable',), {'action': 'store_true',
+                                      'help': 'VDI sharable'}),
+                   (('--read-only',), {'action': 'store_true',
+                                       'help': 'Read only'})],
+    
+    'vbd-create': [(('--VDI',), {'help': 'UUID of VDI to attach to.'}),
+                   (('--mode',), {'choices': ['RO', 'RW'],
+                                  'help': 'device mount mode'}),
+                   (('--driver',), {'choices':['paravirtualised', 'ioemu'],
+                                    'help': 'Driver for VBD'}),
+                   (('--device',), {'help': 'Device name on guest domain'}),
+                   (('--image',), {'help': 'Location of drive image.'})]
+                   
 }
 
 class OptionError(Exception):
@@ -70,6 +93,16 @@ class XenAPIError(Exception):
 # Extra utility functions
 #
 
+class IterableValues(Values):
+    """Better interface to the list of values from optparse."""
+
+    def __iter__(self):
+        for opt, val in self.__dict__.items():
+            if opt[0] == '_' or callable(val):
+                continue
+            yield opt, val        
+
+
 def parse_args(cmd_name, args):
     argstring, desc = COMMANDS[cmd_name]
     parser = OptionParser(usage = 'xapi %s %s' % (cmd_name, argstring),
@@ -77,8 +110,11 @@ def parse_args(cmd_name, args):
     if cmd_name in OPTIONS:
         for optargs, optkwds in OPTIONS[cmd_name]:
             parser.add_option(*optargs, **optkwds)
-            
-    (opts, extraargs) = parser.parse_args(list(args))
+
+    default_values = parser.get_default_values()
+    defaults = IterableValues(default_values.__dict__)
+    (opts, extraargs) = parser.parse_args(args = list(args),
+                                          values = defaults)
     return opts, extraargs
 
 def execute(fn, *args):
@@ -106,7 +142,7 @@ def _read_python_cfg(filename):
     return cfg
 
 def resolve_vm(server, session, vm_name):
-    vm_uuid = execute(server.VM.get_by_label, session, vm_name)
+    vm_uuid = execute(server.VM.get_by_name_label, session, vm_name)
     if not vm_uuid:
         return None
     else:
@@ -204,7 +240,7 @@ def xapi_vm_start(*args):
         raise OptionError("No Domain name specified.")
     
     server, session = _connect()
-    vm_uuid = execute(server.VM.get_by_label, session, args[0])
+    vm_uuid = resolve_vm(server, session, args[0])
     print 'Starting VM %s (%s)' % (args[0], vm_uuid)
     success = execute(server.VM.start, session, vm_uuid)
     print 'Done.'
@@ -233,9 +269,16 @@ def xapi_vbd_create(*args):
     if len(args) < 2:
         raise OptionError("Configuration file not specified")
 
+    opts, args = parse_args('vbd-create', args)
     domname = args[0]
     filename = args[1]
-    cfg = _read_python_cfg(filename)
+
+    cfg = {}
+    for opt, val in opts:
+        cfg[opt] = val
+    cfg.update(_read_python_cfg(filename))
+    
+    
     print 'Creating VBD from %s ..' % filename
     server, session = _connect()
     vm_uuid = resolve_vm(server, session, domname)
@@ -250,6 +293,7 @@ def xapi_vif_create(*args):
     domname = args[0]
     filename = args[1]
     cfg = _read_python_cfg(filename)
+    
     print 'Creating VIF from %s ..' % filename
     server, session = _connect()
     vm_uuid = resolve_vm(server, session, domname)
@@ -283,9 +327,17 @@ def xapi_sr_list(*args):
         print SR_LIST_FORMAT % sr_struct
 
 def xapi_vdi_create(*args):
-    server, session = _connect()
-    cfg = _read_python_cfg(args[0])
+    opts, args = parse_args('vdi-create', args)
 
+    if len(args) < 1:
+        raise OptionError("Not enough arguments.")
+
+    cfg = {}
+    for opt, val in opts:
+        cfg[opt] = val
+    cfg.update(_read_python_cfg(args[0]))
+
+    server, session = _connect()
     srs = execute(server.SR.get_all, session)
     sr = srs[0]
     cfg['SR'] = sr
@@ -359,6 +411,8 @@ def main(args):
     except XenAPIError, e:
         print 'Error: %s' % str(e.args[1])
         sys.exit(2)
+    except OptionError, e:
+        print 'Error: %s' % e
 
     sys.exit(0)
     

tools/python/xen/xend/XendAPI.py

@@ -1001,10 +1001,10 @@ class XendAPI:
         xendom = XendDomain.instance()
         vm = xendom.get_vm_with_dev_uuid('vbd', vbd_ref)
         if not vm:
-            return xen_api_error(XEND_ERROR_VIF_INVALID)
+            return xen_api_error(XEND_ERROR_VBD_INVALID)
         cfg = vm.get_dev_xenapi_config('vbd', vbd_ref)
         if not cfg:
-            return xen_api_error(XEND_ERROR_VIF_INVALID)
+            return xen_api_error(XEND_ERROR_VBD_INVALID)
         return xen_api_success(cfg)
     
     # class methods
@@ -1016,7 +1016,7 @@ class XendAPI:
         dom = xendom.get_vm_by_uuid(vbd_struct['VM'])
         vbd_ref = ''
         try:
-            if vbd_struct.get('VDI', None):
+            if not vbd_struct.get('VDI', None):
                 # this is a traditional VBD without VDI and SR 
                 vbd_ref = dom.create_vbd(vbd_struct)
             else:
@@ -1024,8 +1024,10 @@ class XendAPI:
                 vdi_ref = vbd_struct.get('VDI')
                 sr = XendNode.instance().get_sr()
                 vdi_image = sr.xen_api_get_by_uuid(vdi_ref)
-                vdi_image_path = vdi_image.image_path
-                vbd_ref = dom.create_vbd_with_vdi(vbd_struct, vdi_image_path)
+                if not vdi_image:
+                    return xen_api_error(XEND_ERROR_VDI_INVALID)
+                vdi_image = vdi_image.qcow_path
+                vbd_ref = dom.create_vbd_with_vdi(vbd_struct, vdi_image)
         except XendError:
             return xen_api_todo()
 

tools/python/xen/xend/XendDomainInfo.py

@@ -28,6 +28,7 @@ import logging
 import time
 import threading
 import re
+import copy
 
 import xen.lowlevel.xc
 from xen.util import asserts
@@ -1719,28 +1720,44 @@ class XendDomainInfo:
 
     def get_dev_config_by_uuid(self, dev_class, dev_uuid):
         """ Get's a device configuration either from XendConfig or
-        from the DevController."""
+        from the DevController.
+
+        @param dev_class: device class, either, 'vbd' or 'vif'
+        @param dev_uuid: device UUID
+
+        @rtype: dictionary
+        """
+        dev_type_config = self.info['device'].get(dev_uuid)
+
+        # shortcut if the domain isn't started because
+        # the devcontrollers will have no better information
+        # than XendConfig.
         if self.state in (XEN_API_VM_POWER_STATE_HALTED,):
-            dev = self.info['device'].get(dev_uuid)
-            if dev:
-                return dev[1].copy()
+            if dev_type_config:
+                return copy.deepcopy(dev_type_config[1])
+            return None
+
+        # instead of using dev_class, we use the dev_type
+        # that is from XendConfig.
+        # This will accomdate 'tap' as well as 'vbd'
+        dev_type = dev_type_config[0]
+        
+        controller = self.getDeviceController(dev_type)
+        if not controller:
             return None
-        else:
-            controller = self.getDeviceController(dev_class)
-            if not controller:
-                return None
             
-            all_configs = controller.getAllDeviceConfigurations()
-            if not all_configs:
-                return None
+        all_configs = controller.getAllDeviceConfigurations()
+        if not all_configs:
+            return None
 
-            for _devid, _devcfg in all_configs.items():
-                if _devcfg.get('uuid') == dev_uuid:
-                    devcfg = _devcfg.copy()
-                    devcfg['id'] = _devid
-                    return devcfg
+        dev_config = copy.deepcopy(dev_type_config[1])
+        for _devid, _devcfg in all_configs.items():
+            if _devcfg.get('uuid') == dev_uuid:
+                dev_config.update(_devcfg)
+                dev_config['id'] = _devid
+                return dev_config
 
-        return None
+        return dev_config
                     
     def get_dev_xenapi_config(self, dev_class, dev_uuid):
         config = self.get_dev_config_by_uuid(dev_class, dev_uuid)
@@ -1770,10 +1787,11 @@ class XendDomainInfo:
             config['IO_bandwidth_incoming_kbs'] = 0.0
             config['IO_bandwidth_outgoing_kbs'] = 0.0
 
-        if dev_class == 'vbd':
+        if dev_class =='vbd':
             config['VDI'] = '' # TODO
             config['device'] = config.get('dev', '')
-            config['driver'] = config.get('uname', '')
+            config['driver'] = 'paravirtualised' # TODO
+            config['image'] = config.get('uname', '')
             config['IO_bandwidth_incoming_kbs'] = 0.0
             config['IO_bandwidth_outgoing_kbs'] = 0.0
             if config['mode'] == 'r':
@@ -1835,6 +1853,7 @@ class XendDomainInfo:
         @return: uuid of the device
         """
         xenapi_vbd['image'] = vdi_image_path
+        log.debug('create_vbd_with_vdi: %s' % xenapi_vbd)
         dev_uuid = self.info.device_add('tap', cfg_xenapi = xenapi_vbd)
         if not dev_uuid:
             raise XendError('Failed to create device')