KVM: add the network script to the ancillary files
[ganeti-local] / lib / hypervisor / hv_xen.py
index 0ee8911..614e680 100644 (file)
 import os
 import os.path
 import time
+import logging
 from cStringIO import StringIO
 
 from ganeti import constants
 from ganeti import errors
-from ganeti import logger
 from ganeti import utils
 from ganeti.hypervisor import hv_base
 
@@ -42,43 +42,70 @@ class XenHypervisor(hv_base.BaseHypervisor):
   all the functionality that is identical for both.
 
   """
+  REBOOT_RETRY_COUNT = 60
+  REBOOT_RETRY_INTERVAL = 10
 
-  @staticmethod
-  def _WriteConfigFile(instance, block_devices, extra_args):
+  ANCILLARY_FILES = [
+    '/etc/xen/xend-config.sxp',
+    '/etc/xen/scripts/vif-bridge',
+    ]
+
+  @classmethod
+  def _WriteConfigFile(cls, instance, block_devices):
     """Write the Xen config file for the instance.
 
     """
     raise NotImplementedError
 
   @staticmethod
-  def _RemoveConfigFile(instance):
+  def _WriteConfigFileStatic(instance_name, data):
+    """Write the Xen config file for the instance.
+
+    This version of the function just writes the config file from static data.
+
+    """
+    utils.WriteFile("/etc/xen/%s" % instance_name, data=data)
+
+  @staticmethod
+  def _ReadConfigFile(instance_name):
+    """Returns the contents of the instance config file.
+
+    """
+    try:
+      file_content = utils.ReadFile("/etc/xen/%s" % instance_name)
+    except EnvironmentError, err:
+      raise errors.HypervisorError("Failed to load Xen config file: %s" % err)
+    return file_content
+
+  @staticmethod
+  def _RemoveConfigFile(instance_name):
     """Remove the xen configuration file.
 
     """
-    utils.RemoveFile("/etc/xen/%s" % instance.name)
+    utils.RemoveFile("/etc/xen/%s" % instance_name)
 
   @staticmethod
   def _GetXMList(include_node):
     """Return the list of running instances.
 
-    If the `include_node` argument is True, then we return information
+    If the include_node argument is True, then we return information
     for dom0 also, otherwise we filter that from the return value.
 
-    The return value is a list of (name, id, memory, vcpus, state, time spent)
+    @return: list of (name, id, memory, vcpus, state, time spent)
 
     """
     for dummy in range(5):
       result = utils.RunCmd(["xm", "list"])
       if not result.failed:
         break
-      logger.Error("xm list failed (%s): %s" % (result.fail_reason,
-                                                result.output))
+      logging.error("xm list failed (%s): %s", result.fail_reason,
+                    result.output)
       time.sleep(1)
 
     if result.failed:
       raise errors.HypervisorError("xm list failed, retries"
                                    " exceeded (%s): %s" %
-                                   (result.fail_reason, result.stderr))
+                                   (result.fail_reason, result.output))
 
     # skip over the heading
     lines = result.stdout.splitlines()[1:]
@@ -117,11 +144,10 @@ class XenHypervisor(hv_base.BaseHypervisor):
   def GetInstanceInfo(self, instance_name):
     """Get instance properties.
 
-    Args:
-      instance_name: the instance name
+    @param instance_name: the instance name
+
+    @return: tuple (name, id, memory, vcpus, stat, times)
 
-    Returns:
-      (name, id, memory, vcpus, stat, times)
     """
     xm_list = self._GetXMList(instance_name=="Domain-0")
     result = None
@@ -134,15 +160,17 @@ class XenHypervisor(hv_base.BaseHypervisor):
   def GetAllInstancesInfo(self):
     """Get properties of all instances.
 
-    Returns:
-      [(name, id, memory, vcpus, stat, times),...]
+    @return: list of tuples (name, id, memory, vcpus, stat, times)
+
     """
     xm_list = self._GetXMList(False)
     return xm_list
 
-  def StartInstance(self, instance, block_devices, extra_args):
-    """Start an instance."""
-    self._WriteConfigFile(instance, block_devices, extra_args)
+  def StartInstance(self, instance, block_devices):
+    """Start an instance.
+
+    """
+    self._WriteConfigFile(instance, block_devices)
     result = utils.RunCmd(["xm", "create", instance.name])
 
     if result.failed:
@@ -151,8 +179,10 @@ class XenHypervisor(hv_base.BaseHypervisor):
                                     result.output))
 
   def StopInstance(self, instance, force=False):
-    """Stop an instance."""
-    self._RemoveConfigFile(instance)
+    """Stop an instance.
+
+    """
+    self._RemoveConfigFile(instance.name)
     if force:
       command = ["xm", "destroy", instance.name]
     else:
@@ -160,35 +190,60 @@ class XenHypervisor(hv_base.BaseHypervisor):
     result = utils.RunCmd(command)
 
     if result.failed:
-      raise errors.HypervisorError("Failed to stop instance %s: %s" %
-                                   (instance.name, result.fail_reason))
+      raise errors.HypervisorError("Failed to stop instance %s: %s, %s" %
+                                   (instance.name, result.fail_reason,
+                                    result.output))
 
   def RebootInstance(self, instance):
-    """Reboot an instance."""
+    """Reboot an instance.
+
+    """
+    ini_info = self.GetInstanceInfo(instance.name)
     result = utils.RunCmd(["xm", "reboot", instance.name])
 
     if result.failed:
-      raise errors.HypervisorError("Failed to reboot instance %s: %s" %
-                                   (instance.name, result.fail_reason))
+      raise errors.HypervisorError("Failed to reboot instance %s: %s, %s" %
+                                   (instance.name, result.fail_reason,
+                                    result.output))
+    done = False
+    retries = self.REBOOT_RETRY_COUNT
+    while retries > 0:
+      new_info = self.GetInstanceInfo(instance.name)
+      # check if the domain ID has changed or the run time has
+      # decreased
+      if new_info[1] != ini_info[1] or new_info[5] < ini_info[5]:
+        done = True
+        break
+      time.sleep(self.REBOOT_RETRY_INTERVAL)
+      retries -= 1
+
+    if not done:
+      raise errors.HypervisorError("Failed to reboot instance %s: instance"
+                                   " did not reboot in the expected interval" %
+                                   (instance.name, ))
 
   def GetNodeInfo(self):
     """Return information about the node.
 
-    The return value is a dict, which has to have the following items:
-      (all values in MiB)
-      - memory_total: the total memory size on the node
-      - memory_free: the available memory on the node for instances
-      - memory_dom0: the memory used by the node itself, if available
+    @return: a dict with the following keys (memory values in MiB):
+          - memory_total: the total memory size on the node
+          - memory_free: the available memory on the node for instances
+          - memory_dom0: the memory used by the node itself, if available
+          - nr_cpus: total number of CPUs
+          - nr_nodes: in a NUMA system, the number of domains
+          - nr_sockets: the number of physical CPU sockets in the node
 
     """
     # note: in xen 3, memory has changed to total_memory
     result = utils.RunCmd(["xm", "info"])
     if result.failed:
-      logger.Error("Can't run 'xm info': %s" % result.fail_reason)
+      logging.error("Can't run 'xm info' (%s): %s", result.fail_reason,
+                    result.output)
       return None
 
     xmoutput = result.stdout.splitlines()
     result = {}
+    cores_per_socket = threads_per_core = nr_cpus = None
     for line in xmoutput:
       splitfields = line.split(":", 1)
 
@@ -200,15 +255,26 @@ class XenHypervisor(hv_base.BaseHypervisor):
         elif key == 'free_memory':
           result['memory_free'] = int(val)
         elif key == 'nr_cpus':
-          result['cpu_total'] = int(val)
+          nr_cpus = result['cpu_total'] = int(val)
+        elif key == 'nr_nodes':
+          result['cpu_nodes'] = int(val)
+        elif key == 'cores_per_socket':
+          cores_per_socket = int(val)
+        elif key == 'threads_per_core':
+          threads_per_core = int(val)
+
+    if (cores_per_socket is not None and
+        threads_per_core is not None and nr_cpus is not None):
+      result['cpu_sockets'] = nr_cpus / (cores_per_socket * threads_per_core)
+
     dom0_info = self.GetInstanceInfo("Domain-0")
     if dom0_info is not None:
       result['memory_dom0'] = dom0_info[2]
 
     return result
 
-  @staticmethod
-  def GetShellCommandForConsole(instance):
+  @classmethod
+  def GetShellCommandForConsole(cls, instance, hvparams, beparams):
     """Return a command for connecting to the console of an instance.
 
     """
@@ -223,7 +289,7 @@ class XenHypervisor(hv_base.BaseHypervisor):
     """
     result = utils.RunCmd(["xm", "info"])
     if result.failed:
-      return "'xm info' failed: %s" % result.fail_reason
+      return "'xm info' failed: %s, %s" % (result.fail_reason, result.output)
 
   @staticmethod
   def _GetConfigFileDiskData(disk_template, block_devices):
@@ -232,15 +298,12 @@ class XenHypervisor(hv_base.BaseHypervisor):
     This method builds the xen config disk directive according to the
     given disk_template and block_devices.
 
-    Args:
-      disk_template: String containing instance disk template
-      block_devices: List[tuple1,tuple2,...]
-        tuple: (cfdev, rldev)
-          cfdev: dict containing ganeti config disk part
-          rldev: ganeti.bdev.BlockDev object
+    @param disk_template: string containing instance disk template
+    @param block_devices: list of tuples (cfdev, rldev):
+        - cfdev: dict containing ganeti config disk part
+        - rldev: ganeti.bdev.BlockDev object
 
-    Returns:
-      String containing disk directive for xen instance config file
+    @return: string containing disk directive for xen instance config file
 
     """
     FILE_DRIVER_MAP = {
@@ -248,29 +311,80 @@ class XenHypervisor(hv_base.BaseHypervisor):
       constants.FD_BLKTAP: "tap:aio",
       }
     disk_data = []
-    for cfdev, rldev in block_devices:
+    if len(block_devices) > 24:
+      # 'z' - 'a' = 24
+      raise errors.HypervisorError("Too many disks")
+    # FIXME: instead of this hardcoding here, each of PVM/HVM should
+    # directly export their info (currently HVM will just sed this info)
+    namespace = ["sd" + chr(i + ord('a')) for i in range(24)]
+    for sd_name, (cfdev, dev_path) in zip(namespace, block_devices):
+      if cfdev.mode == constants.DISK_RDWR:
+        mode = "w"
+      else:
+        mode = "r"
       if cfdev.dev_type == constants.LD_FILE:
-        line = "'%s:%s,%s,w'" % (FILE_DRIVER_MAP[cfdev.physical_id[0]],
-                                 rldev.dev_path, cfdev.iv_name)
+        line = "'%s:%s,%s,%s'" % (FILE_DRIVER_MAP[cfdev.physical_id[0]],
+                                  dev_path, sd_name, mode)
       else:
-        line = "'phy:%s,%s,w'" % (rldev.dev_path, cfdev.iv_name)
+        line = "'phy:%s,%s,%s'" % (dev_path, sd_name, mode)
       disk_data.append(line)
 
     return disk_data
 
-  def MigrateInstance(self, instance, target, live):
-    """Migrate an instance to a target node.
+  def MigrationInfo(self, instance):
+    """Get instance information to perform a migration.
+
+    @type instance: L{objects.Instance}
+    @param instance: instance to be migrated
+    @rtype: string
+    @return: content of the xen config file
+
+    """
+    return self._ReadConfigFile(instance.name)
+
+  def AcceptInstance(self, instance, info, target):
+    """Prepare to accept an instance.
+
+    @type instance: L{objects.Instance}
+    @param instance: instance to be accepted
+    @type info: string
+    @param info: content of the xen config file on the source node
+    @type target: string
+    @param target: target host (usually ip), on this node
+
+    """
+    pass
+
+  def FinalizeMigration(self, instance, info, success):
+    """Finalize an instance migration.
 
-    Arguments:
-      - instance: the name of the instance
-      - target: the ip of the target node
-      - live: whether to do live migration or not
+    After a successful migration we write the xen config file.
+    We do nothing on a failure, as we did not change anything at accept time.
 
-    Returns: none, errors will be signaled by exception.
+    @type instance: L{objects.Instance}
+    @param instance: instance whose migration is being aborted
+    @type info: string
+    @param info: content of the xen config file on the source node
+    @type success: boolean
+    @param success: whether the migration was a success or a failure
+
+    """
+    if success:
+      self._WriteConfigFileStatic(instance.name, info)
+
+  def MigrateInstance(self, instance, target, live):
+    """Migrate an instance to a target node.
 
     The migration will not be attempted if the instance is not
     currently running.
 
+    @type instance: string
+    @param instance: instance name
+    @type target: string
+    @param target: ip address of the target node
+    @type live: boolean
+    @param live: perform a live migration
+
     """
     if self.GetInstanceInfo(instance) is None:
       raise errors.HypervisorError("Instance not running, cannot migrate")
@@ -282,73 +396,38 @@ class XenHypervisor(hv_base.BaseHypervisor):
     if result.failed:
       raise errors.HypervisorError("Failed to migrate instance %s: %s" %
                                    (instance, result.output))
+    # remove old xen file after migration succeeded
+    try:
+      self._RemoveConfigFile(instance)
+    except EnvironmentError:
+      logging.exception("Failure while removing instance config file")
 
 
 class XenPvmHypervisor(XenHypervisor):
   """Xen PVM hypervisor interface"""
 
-  PARAMETERS = [
-    constants.HV_KERNEL_PATH,
-    constants.HV_INITRD_PATH,
-    ]
-
-  @classmethod
-  def CheckParameterSyntax(cls, hvparams):
-    """Check the given parameters for validity.
-
-    For the PVM hypervisor, this only check the existence of the
-    kernel.
-
-    @type hvparams:  dict
-    @param hvparams: dictionary with parameter names/value
-    @raise errors.HypervisorError: when a parameter is not valid
-
-    """
-    super(XenPvmHypervisor, cls).CheckParameterSyntax(hvparams)
-
-    if not hvparams[constants.HV_KERNEL_PATH]:
-      raise errors.HypervisorError("Need a kernel for the instance")
-
-    if not os.path.isabs(hvparams[constants.HV_KERNEL_PATH]):
-      raise errors.HypervisorError("The kernel path must an absolute path")
-
-    if hvparams[constants.HV_INITRD_PATH]:
-      if not os.path.isabs(hvparams[constants.HV_INITRD_PATH]):
-        raise errors.HypervisorError("The initrd path must an absolute path"
-                                     ", if defined")
-
-  def ValidateParameters(self, hvparams):
-    """Check the given parameters for validity.
-
-    For the PVM hypervisor, this only check the existence of the
-    kernel.
-
-    """
-    super(XenPvmHypervisor, self).ValidateParameters(hvparams)
-
-    kernel_path = hvparams[constants.HV_KERNEL_PATH]
-    if not os.path.isfile(kernel_path):
-      raise errors.HypervisorError("Instance kernel '%s' not found or"
-                                   " not a file" % kernel_path)
-    initrd_path = hvparams[constants.HV_INITRD_PATH]
-    if initrd_path and not os.path.isfile(initrd_path):
-      raise errors.HypervisorError("Instance initrd '%s' not found or"
-                                   " not a file" % initrd_path)
+  PARAMETERS = {
+    constants.HV_KERNEL_PATH: hv_base.REQ_FILE_CHECK,
+    constants.HV_INITRD_PATH: hv_base.OPT_FILE_CHECK,
+    constants.HV_ROOT_PATH: hv_base.REQUIRED_CHECK,
+    constants.HV_KERNEL_ARGS: hv_base.NO_CHECK,
+    }
 
   @classmethod
-  def _WriteConfigFile(cls, instance, block_devices, extra_args):
+  def _WriteConfigFile(cls, instance, block_devices):
     """Write the Xen config file for the instance.
 
     """
+    hvp = instance.hvparams
     config = StringIO()
     config.write("# this is autogenerated by Ganeti, please do not edit\n#\n")
 
     # kernel handling
-    kpath = instance.hvparams[constants.HV_KERNEL_PATH]
+    kpath = hvp[constants.HV_KERNEL_PATH]
     config.write("kernel = '%s'\n" % kpath)
 
     # initrd handling
-    initrd_path = instance.hvparams[constants.HV_INITRD_PATH]
+    initrd_path = hvp[constants.HV_INITRD_PATH]
     if initrd_path:
       config.write("ramdisk = '%s'\n" % initrd_path)
 
@@ -369,137 +448,89 @@ class XenPvmHypervisor(XenHypervisor):
     config.write("disk = [%s]\n" % ",".join(
                  cls._GetConfigFileDiskData(instance.disk_template,
                                             block_devices)))
-    config.write("root = '/dev/sda ro'\n")
+
+    config.write("root = '%s'\n" % hvp[constants.HV_ROOT_PATH])
     config.write("on_poweroff = 'destroy'\n")
     config.write("on_reboot = 'restart'\n")
     config.write("on_crash = 'restart'\n")
-    if extra_args:
-      config.write("extra = '%s'\n" % extra_args)
+    config.write("extra = '%s'\n" % hvp[constants.HV_KERNEL_ARGS])
     # just in case it exists
     utils.RemoveFile("/etc/xen/auto/%s" % instance.name)
     try:
-      f = open("/etc/xen/%s" % instance.name, "w")
-      try:
-        f.write(config.getvalue())
-      finally:
-        f.close()
-    except IOError, err:
-      raise errors.OpExecError("Cannot write Xen instance confile"
-                               " file /etc/xen/%s: %s" % (instance.name, err))
+      utils.WriteFile("/etc/xen/%s" % instance.name, data=config.getvalue())
+    except EnvironmentError, err:
+      raise errors.HypervisorError("Cannot write Xen instance confile"
+                                   " file /etc/xen/%s: %s" %
+                                   (instance.name, err))
+
     return True
 
 
 class XenHvmHypervisor(XenHypervisor):
   """Xen HVM hypervisor interface"""
 
-  PARAMETERS = [
-    constants.HV_ACPI,
-    constants.HV_BOOT_ORDER,
-    constants.HV_CDROM_IMAGE_PATH,
-    constants.HV_DISK_TYPE,
-    constants.HV_NIC_TYPE,
-    constants.HV_PAE,
-    constants.HV_VNC_BIND_ADDRESS,
-    ]
-
-  @classmethod
-  def CheckParameterSyntax(cls, hvparams):
-    """Check the given parameter syntax.
-
-    """
-    super(XenHvmHypervisor, cls).CheckParameterSyntax(hvparams)
-    # boot order verification
-    boot_order = hvparams[constants.HV_BOOT_ORDER]
-    if len(boot_order.strip("acdn")) != 0:
-      raise errors.HypervisorError("Invalid boot order '%s' specified,"
-                                   " must be one or more of [acdn]" %
-                                   boot_order)
-    # device type checks
-    nic_type = hvparams[constants.HV_NIC_TYPE]
-    if nic_type not in constants.HT_HVM_VALID_NIC_TYPES:
-      raise errors.HypervisorError("Invalid NIC type %s specified for Xen HVM"
-                                   " hypervisor" % nic_type)
-    disk_type = hvparams[constants.HV_DISK_TYPE]
-    if disk_type not in constants.HT_HVM_VALID_DISK_TYPES:
-      raise errors.HypervisorError("Invalid disk type %s specified for Xen HVM"
-                                   " hypervisor" % disk_type)
-    # vnc_bind_address verification
-    vnc_bind_address = hvparams[constants.HV_VNC_BIND_ADDRESS]
-    if vnc_bind_address is not None:
-      if not utils.IsValidIP(vnc_bind_address):
-        raise errors.OpPrereqError("given VNC bind address '%s' doesn't look"
-                                   " like a valid IP address" %
-                                   vnc_bind_address)
-
-    iso_path = hvparams[constants.HV_CDROM_IMAGE_PATH]
-    if iso_path and not os.path.isabs(iso_path):
-        raise errors.HypervisorError("The path to the HVM CDROM image must"
-                                     " be an absolute path or None, not %s" %
-                                     iso_path)
-
-  def ValidateParameters(self, hvparams):
-    """Check the given parameters for validity.
-
-    For the PVM hypervisor, this only check the existence of the
-    kernel.
-
-    @type hvparams:  dict
-    @param hvparams: dictionary with parameter names/value
-    @raise errors.HypervisorError: when a parameter is not valid
-
-    """
-    super(XenHvmHypervisor, self).ValidateParameters(hvparams)
-
-    # hvm_cdrom_image_path verification
-    iso_path = hvparams[constants.HV_CDROM_IMAGE_PATH]
-    if iso_path and not os.path.isfile(iso_path):
-        raise errors.HypervisorError("The HVM CDROM image must either be a"
-                                     " regular file or a symlink pointing to"
-                                     " an existing regular file, not %s" %
-                                     iso_path)
+  ANCILLARY_FILES = XenHypervisor.ANCILLARY_FILES + \
+    [constants.VNC_PASSWORD_FILE]
+
+  PARAMETERS = {
+    constants.HV_ACPI: hv_base.NO_CHECK,
+    constants.HV_BOOT_ORDER: (True, ) + \
+    (lambda x: x and len(x.strip("acdn")) == 0,
+     "Invalid boot order specified, must be one or more of [acdn]",
+     None, None),
+    constants.HV_CDROM_IMAGE_PATH: hv_base.OPT_FILE_CHECK,
+    constants.HV_DISK_TYPE: \
+    hv_base.ParamInSet(True, constants.HT_HVM_VALID_DISK_TYPES),
+    constants.HV_NIC_TYPE: \
+    hv_base.ParamInSet(True, constants.HT_HVM_VALID_NIC_TYPES),
+    constants.HV_PAE: hv_base.NO_CHECK,
+    constants.HV_VNC_BIND_ADDRESS: \
+    (False, utils.IsValidIP,
+     "VNC bind address is not a valid IP address", None, None),
+    constants.HV_KERNEL_PATH: hv_base.REQ_FILE_CHECK,
+    constants.HV_DEVICE_MODEL: hv_base.REQ_FILE_CHECK,
+    }
 
   @classmethod
-  def _WriteConfigFile(cls, instance, block_devices, extra_args):
+  def _WriteConfigFile(cls, instance, block_devices):
     """Create a Xen 3.1 HVM config file.
 
     """
+    hvp = instance.hvparams
+
     config = StringIO()
     config.write("# this is autogenerated by Ganeti, please do not edit\n#\n")
-    config.write("kernel = '/usr/lib/xen/boot/hvmloader'\n")
+
+    # kernel handling
+    kpath = hvp[constants.HV_KERNEL_PATH]
+    config.write("kernel = '%s'\n" % kpath)
+
     config.write("builder = 'hvm'\n")
     config.write("memory = %d\n" % instance.beparams[constants.BE_MEMORY])
     config.write("vcpus = %d\n" % instance.beparams[constants.BE_VCPUS])
     config.write("name = '%s'\n" % instance.name)
-    if instance.hvparams[constants.HV_PAE]:
+    if hvp[constants.HV_PAE]:
       config.write("pae = 1\n")
     else:
       config.write("pae = 0\n")
-    if instance.hvparams[constants.HV_ACPI]:
+    if hvp[constants.HV_ACPI]:
       config.write("acpi = 1\n")
     else:
       config.write("acpi = 0\n")
     config.write("apic = 1\n")
-    arch = os.uname()[4]
-    if '64' in arch:
-      config.write("device_model = '/usr/lib64/xen/bin/qemu-dm'\n")
-    else:
-      config.write("device_model = '/usr/lib/xen/bin/qemu-dm'\n")
-    if instance.hvparams[constants.HV_BOOT_ORDER] is None:
-      config.write("boot = '%s'\n" % constants.HT_HVM_DEFAULT_BOOT_ORDER)
-    else:
-      config.write("boot = '%s'\n" % instance.hvparams["boot_order"])
+    config.write("device_model = '%s'\n" % hvp[constants.HV_DEVICE_MODEL])
+    config.write("boot = '%s'\n" % hvp[constants.HV_BOOT_ORDER])
     config.write("sdl = 0\n")
     config.write("usb = 1\n")
     config.write("usbdevice = 'tablet'\n")
     config.write("vnc = 1\n")
-    if instance.hvparams[constants.HV_VNC_BIND_ADDRESS] is None:
+    if hvp[constants.HV_VNC_BIND_ADDRESS] is None:
       config.write("vnclisten = '%s'\n" % constants.VNC_DEFAULT_BIND_ADDRESS)
     else:
-      config.write("vnclisten = '%s'\n" %
-                   instance.hvparams["vnc_bind_address"])
+      config.write("vnclisten = '%s'\n" % hvp[constants.HV_VNC_BIND_ADDRESS])
 
-    if instance.network_port > constants.HT_HVM_VNC_BASE_PORT:
-      display = instance.network_port - constants.HT_HVM_VNC_BASE_PORT
+    if instance.network_port > constants.VNC_BASE_PORT:
+      display = instance.network_port - constants.VNC_BASE_PORT
       config.write("vncdisplay = %s\n" % display)
       config.write("vncunused = 0\n")
     else:
@@ -507,14 +538,10 @@ class XenHvmHypervisor(XenHypervisor):
       config.write("vncunused = 1\n")
 
     try:
-      password_file = open(constants.VNC_PASSWORD_FILE, "r")
-      try:
-        password = password_file.readline()
-      finally:
-        password_file.close()
-    except IOError:
-      raise errors.OpExecError("failed to open VNC password file %s " %
-                               constants.VNC_PASSWORD_FILE)
+      password = utils.ReadFile(constants.VNC_PASSWORD_FILE)
+    except EnvironmentError, err:
+      raise errors.HypervisorError("Failed to open VNC password file %s: %s" %
+                                   (constants.VNC_PASSWORD_FILE, err))
 
     config.write("vncpasswd = '%s'\n" % password.rstrip())
 
@@ -522,11 +549,11 @@ class XenHvmHypervisor(XenHypervisor):
     config.write("localtime = 1\n")
 
     vif_data = []
-    nic_type = instance.hvparams[constants.HV_NIC_TYPE]
+    nic_type = hvp[constants.HV_NIC_TYPE]
     if nic_type is None:
       # ensure old instances don't change
       nic_type_str = ", type=ioemu"
-    elif nic_type == constants.HT_HVM_DEV_PARAVIRTUAL:
+    elif nic_type == constants.HT_NIC_PARAVIRTUAL:
       nic_type_str = ", type=paravirtualized"
     else:
       nic_type_str = ", model=%s, type=ioemu" % nic_type
@@ -540,13 +567,13 @@ class XenHvmHypervisor(XenHypervisor):
     config.write("vif = [%s]\n" % ",".join(vif_data))
     disk_data = cls._GetConfigFileDiskData(instance.disk_template,
                                             block_devices)
-    disk_type = instance.hvparams[constants.HV_DISK_TYPE]
-    if disk_type in (None, constants.HT_HVM_DEV_IOEMU):
+    disk_type = hvp[constants.HV_DISK_TYPE]
+    if disk_type in (None, constants.HT_DISK_IOEMU):
       replacement = ",ioemu:hd"
     else:
       replacement = ",hd"
     disk_data = [line.replace(",sd", replacement) for line in disk_data]
-    iso_path = instance.hvparams[constants.HV_CDROM_IMAGE_PATH]
+    iso_path = hvp[constants.HV_CDROM_IMAGE_PATH]
     if iso_path:
       iso = "'file:%s,hdc:cdrom,r'" % iso_path
       disk_data.append(iso)
@@ -556,17 +583,14 @@ class XenHvmHypervisor(XenHypervisor):
     config.write("on_poweroff = 'destroy'\n")
     config.write("on_reboot = 'restart'\n")
     config.write("on_crash = 'restart'\n")
-    if extra_args:
-      config.write("extra = '%s'\n" % extra_args)
     # just in case it exists
     utils.RemoveFile("/etc/xen/auto/%s" % instance.name)
     try:
-      f = open("/etc/xen/%s" % instance.name, "w")
-      try:
-        f.write(config.getvalue())
-      finally:
-        f.close()
-    except IOError, err:
-      raise errors.OpExecError("Cannot write Xen instance confile"
-                               " file /etc/xen/%s: %s" % (instance.name, err))
+      utils.WriteFile("/etc/xen/%s" % instance.name,
+                      data=config.getvalue())
+    except EnvironmentError, err:
+      raise errors.HypervisorError("Cannot write Xen instance confile"
+                                   " file /etc/xen/%s: %s" %
+                                   (instance.name, err))
+
     return True