commit 364e16645048daefb387122963c11943f3d92264
Author: Jose A. Lopes <jabolopes@google.com>
Date:   Tue Jan 14 12:57:23 2014 +0100

    Fix KVM cdrom image URL in the second cdrom drive
    
    * The previous patch introduced cdrom image URLs for KVM cdrom drives.
      However, it worked only for the first drive and not the second.
      This patch generalizes the KVM cdrom command line option to allow
      cdrom image URLs to be passed to both drives.
    
    * Simplify some functions and fix some doc strings
    
    Signed-off-by: Jose A. Lopes <jabolopes@google.com>
    Reviewed-by: Michele Tartara <mtartara@google.com>

diff --git a/lib/hypervisor/hv_base.py b/lib/hypervisor/hv_base.py
index 8556ef8..940d7f3 100644
--- a/lib/hypervisor/hv_base.py
+++ b/lib/hypervisor/hv_base.py
@@ -79,15 +79,14 @@ def _IsMultiCpuMaskWellFormed(cpu_mask):
 # Read the BaseHypervisor.PARAMETERS docstring for the syntax of the
 # _CHECK values
 
-# must be afile
+# must be a file
 _FILE_CHECK = (utils.IsNormAbsPath, "must be an absolute normalized path",
                os.path.isfile, "not found or not a file")
 
 # must be a file or a URL
-_FILE_OR_URL_CHECK = (utils.IsNormAbsPathOrURL,
+_FILE_OR_URL_CHECK = (lambda x: utils.IsNormAbsPath(x) or utils.IsUrl(x),
                       "must be an absolute normalized path or a URL",
-                      lambda x: os.path.isfile(x) or
-                      re.match(r'(https?|ftps?)://', x),
+                      lambda x: os.path.isfile(x) or utils.IsUrl(x),
                       "not found or not a file or URL")
 
 # must be a directory
diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
index fe7e834..d3932ba 100644
--- a/lib/hypervisor/hv_kvm.py
+++ b/lib/hypervisor/hv_kvm.py
@@ -319,23 +319,21 @@ def _OpenTap(vnet_hdr=True):
   return (ifname, tapfd)
 
 
+class HeadRequest(urllib2.Request):
+  def get_method(self):
+    return "HEAD"
+
+
 def _CheckUrl(url):
   """Check if a given URL exists on the server
 
   """
-  req = urllib2.Request(url)
-
-  # XXX: ugly but true
-  req.get_method = lambda: "HEAD"
-
   try:
-    resp = urllib2.urlopen(req)
+    urllib2.urlopen(HeadRequest(url))
+    return True
   except urllib2.URLError:
     return False
 
-  del resp
-  return True
-
 
 class QmpMessage:
   """QEMU Messaging Protocol (QMP) message.
@@ -698,7 +696,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     constants.HV_KVM_SPICE_USE_VDAGENT: hv_base.NO_CHECK,
     constants.HV_KVM_FLOPPY_IMAGE_PATH: hv_base.OPT_FILE_CHECK,
     constants.HV_CDROM_IMAGE_PATH: hv_base.OPT_FILE_OR_URL_CHECK,
-    constants.HV_KVM_CDROM2_IMAGE_PATH: hv_base.OPT_FILE_CHECK,
+    constants.HV_KVM_CDROM2_IMAGE_PATH: hv_base.OPT_FILE_OR_URL_CHECK,
     constants.HV_BOOT_ORDER:
       hv_base.ParamInSet(True, constants.HT_KVM_VALID_BO_TYPES),
     constants.HV_NIC_TYPE:
@@ -1352,6 +1350,68 @@ class KVMHypervisor(hv_base.BaseHypervisor):
 
     return dev_opts
 
+  @staticmethod
+  def CdromOption(kvm_cmd, cdrom_disk_type, cdrom_image, cdrom_boot,
+                  needs_boot_flag):
+    """Extends L{kvm_cmd} with the '-drive' option for a cdrom, and
+    optionally the '-boot' option.
+
+    Example: -drive file=cdrom.iso,media=cdrom,format=raw,if=ide -boot d
+
+    Example: -drive file=cdrom.iso,media=cdrom,format=raw,if=ide,boot=on
+
+    Example: -drive file=http://hostname.com/cdrom.iso,media=cdrom
+
+    @type kvm_cmd: string
+    @param kvm_cmd: KVM command line
+
+    @type cdrom_disk_type:
+    @param cdrom_disk_type:
+
+    @type cdrom_image:
+    @param cdrom_image:
+
+    @type cdrom_boot:
+    @param cdrom_boot:
+
+    @type needs_boot_flag:
+    @param needs_boot_flag:
+
+    """
+    # Check that the ISO image is accessible
+    # See https://bugs.launchpad.net/qemu/+bug/597575
+    if utils.IsUrl(cdrom_image) and not _CheckUrl(cdrom_image):
+      raise errors.HypervisorError("Cdrom ISO image '%s' is not accessible" %
+                                   cdrom_image)
+
+    # set cdrom 'media' and 'format', if needed
+    if utils.IsUrl(cdrom_image):
+      options = ",media=cdrom"
+    else:
+      options = ",media=cdrom,format=raw"
+
+    # set cdrom 'if' type
+    if cdrom_boot:
+      if_val = ",if=" + constants.HT_DISK_IDE
+    elif cdrom_disk_type == constants.HT_DISK_PARAVIRTUAL:
+      if_val = ",if=virtio"
+    else:
+      if_val = ",if=" + cdrom_disk_type
+
+    # set boot flag, if needed
+    boot_val = ""
+    if cdrom_boot:
+      kvm_cmd.extend(["-boot", "d"])
+
+      # whether this is an older KVM version that requires the 'boot=on' flag
+      # on devices
+      if needs_boot_flag:
+        boot_val = ",boot=on"
+
+    # build '-drive' option
+    drive_val = "file=%s%s%s%s" % (cdrom_image, options, if_val, boot_val)
+    kvm_cmd.extend(["-drive", drive_val])
+
   def _GenerateKVMRuntime(self, instance, block_devices, startup_paused,
                           kvmhelp):
     """Generate KVM information to start an instance.
@@ -1432,55 +1492,22 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     if boot_network:
       kvm_cmd.extend(["-boot", "n"])
 
-    # whether this is an older KVM version that uses the boot=on flag
-    # on devices
-    needs_boot_flag = self._BOOT_RE.search(kvmhelp)
-
     disk_type = hvp[constants.HV_DISK_TYPE]
 
-    #Now we can specify a different device type for CDROM devices.
+    # Now we can specify a different device type for CDROM devices.
     cdrom_disk_type = hvp[constants.HV_KVM_CDROM_DISK_TYPE]
     if not cdrom_disk_type:
       cdrom_disk_type = disk_type
 
-    iso_image = hvp[constants.HV_CDROM_IMAGE_PATH]
-    if iso_image:
-      options = ",media=cdrom"
-      if re.match(r'(https?|ftps?)://', iso_image):
-        # Check that the iso image is really there
-        # See https://bugs.launchpad.net/qemu/+bug/597575
-        if not _CheckUrl(iso_image):
-          raise errors.HypervisorError("ISO image %s is not accessible" %
-                                       iso_image)
-      else:
-        options = "%s,format=raw" % options
-      # set cdrom 'if' type
-      if boot_cdrom:
-        actual_cdrom_type = constants.HT_DISK_IDE
-      elif cdrom_disk_type == constants.HT_DISK_PARAVIRTUAL:
-        actual_cdrom_type = "virtio"
-      else:
-        actual_cdrom_type = cdrom_disk_type
-      if_val = ",if=%s" % actual_cdrom_type
-      # set boot flag, if needed
-      boot_val = ""
-      if boot_cdrom:
-        kvm_cmd.extend(["-boot", "d"])
-        if needs_boot_flag:
-          boot_val = ",boot=on"
-      # and finally build the entire '-drive' value
-      drive_val = "file=%s%s%s%s" % (iso_image, options, if_val, boot_val)
-      kvm_cmd.extend(["-drive", drive_val])
+    cdrom_image1 = hvp[constants.HV_CDROM_IMAGE_PATH]
+    if cdrom_image1:
+      needs_boot_flag = self._BOOT_RE.search(kvmhelp)
+      self.CdromOption(kvm_cmd, cdrom_disk_type, cdrom_image1, boot_cdrom,
+                       needs_boot_flag)
 
-    iso_image2 = hvp[constants.HV_KVM_CDROM2_IMAGE_PATH]
-    if iso_image2:
-      options = ",format=raw,media=cdrom"
-      if cdrom_disk_type == constants.HT_DISK_PARAVIRTUAL:
-        if_val = ",if=virtio"
-      else:
-        if_val = ",if=%s" % cdrom_disk_type
-      drive_val = "file=%s%s%s" % (iso_image2, options, if_val)
-      kvm_cmd.extend(["-drive", drive_val])
+    cdrom_image2 = hvp[constants.HV_KVM_CDROM2_IMAGE_PATH]
+    if cdrom_image2:
+      self.CdromOption(kvm_cmd, cdrom_disk_type, cdrom_image2, False, False)
 
     floppy_image = hvp[constants.HV_KVM_FLOPPY_IMAGE_PATH]
     if floppy_image:
diff --git a/lib/utils/io.py b/lib/utils/io.py
index 5f17209..9ef37e9 100644
--- a/lib/utils/io.py
+++ b/lib/utils/io.py
@@ -670,11 +670,14 @@ def IsBelowDir(root, other_path):
   return os.path.commonprefix([prepared_root, norm_other]) == prepared_root
 
 
-def IsNormAbsPathOrURL(path):
-  """Check whether a path is absolute and normalized, or an HTTP URL.
+URL_RE = re.compile(r'(https?|ftps?)://')
+
+
+def IsUrl(path):
+  """Check whether a path is a HTTP URL.
 
   """
-  return IsNormAbsPath(path) or re.match(r'(https?|ftps?)://', path)
+  return URL_RE.match(path)
 
 
 def PathJoin(*args):
