On Thu, Jan 16, 2014 at 1:28 PM, Jose A. Lopes <[email protected]> wrote: > * 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 some fix doc strings
s/and some fix doc strings/and fix some doc strings/ > > Signed-off-by: Jose A. Lopes <[email protected]> > --- > lib/hypervisor/hv_base.py | 7 ++- > lib/hypervisor/hv_kvm.py | 131 > ++++++++++++++++++++++++++++------------------ > lib/utils/io.py | 9 ++-- > 3 files changed, 88 insertions(+), 59 deletions(-) > > 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): > -- > 1.8.5.2 > LGTM, thanks. Michele -- Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores
