This is the patch to the original commit, that has both Hansmi's and Rene's comments addressed.
Signed-off-by: Agata Murawska <[email protected]> --- lib/ovf.py | 108 +++++++++++++++++++++++++++++++++++------------------------- 1 files changed, 63 insertions(+), 45 deletions(-) diff --git a/lib/ovf.py b/lib/ovf.py index 48f7998..cf69b92 100644 --- a/lib/ovf.py +++ b/lib/ovf.py @@ -89,6 +89,17 @@ CONVERT_UNITS_TO_MB = { 'gb': lambda x: x * 1024, } +# Names of the config fields +NAME = "name" +OS = "os" +HYPERV = "hypervisor" +VCPUS = "vcpus" +MEMORY = "memory" +AUTO_BALANCE = "auto_balance" +DISK_TEMPLATE = "disk_template" +TAGS = "tags" +VERSION = "version" + class OVFReader(object): """Reader class for OVF files. @@ -725,7 +736,7 @@ class OVFImporter(Converter): @type input_path: string @param input_path: absolute path to the .ovf or .ova input file - @raises errors.OpPrereqError: if input file is neither .ovf nor .ova + @raise errors.OpPrereqError: if input file is neither .ovf nor .ova """ (input_dir, input_file) = os.path.split(input_path) @@ -1218,13 +1229,13 @@ class OVFExporter(Converter): @rtype: string @return: name of Ganeti instance - @raises errors.OpPrereqError: if name of the instance is not provided + @raise errors.OpPrereqError: if name of the instance is not provided """ if self.options.name: name = self.options.name else: - name = self.config_parser.get(constants.INISECT_INS, "name") + name = self.config_parser.get(constants.INISECT_INS, NAME) if name is None: raise errors.OpPrereqError("No instance name found") return name @@ -1235,10 +1246,10 @@ class OVFExporter(Converter): @rtype: int @return: number of virtual CPUs - @raises errors.OpPrereqError: if number of VCPUs equals 0 + @raise errors.OpPrereqError: if number of VCPUs equals 0 """ - vcpus = self.config_parser.getint(constants.INISECT_BEP, "vcpus") + vcpus = self.config_parser.getint(constants.INISECT_BEP, VCPUS) if vcpus == 0: raise errors.OpPrereqError("No CPU information found") return vcpus @@ -1249,10 +1260,10 @@ class OVFExporter(Converter): @rtype: int @return: amount of memory in MB - @raises errors.OpPrereqError: if amount of memory equals 0 + @raise errors.OpPrereqError: if amount of memory equals 0 """ - memory = self.config_parser.getint(constants.INISECT_BEP, "memory") + memory = self.config_parser.getint(constants.INISECT_BEP, MEMORY) if memory == 0: raise errors.OpPrereqError("No memory information found") return memory @@ -1267,31 +1278,31 @@ class OVFExporter(Converter): results = {} # hypervisor results["hypervisor"] = {} - results["hypervisor"]["name"] = \ - self.config_parser.get(constants.INISECT_INS, "hypervisor") + hyp_name = self.config_parser.get(constants.INISECT_INS, HYPERV) + if hyp_name is None: + raise errors.OpPrereqError("No hypervisor information found") + results["hypervisor"]["name"] = hyp_name pairs = self.config_parser.items(constants.INISECT_HYP) for (name, value) in pairs: results["hypervisor"][name] = value - if results["hypervisor"].get("name") is None: - raise errors.OpPrereqError("No hypervisor information found") # os results["os"] = {} - results["os"]["name"] = \ - self.config_parser.get(constants.INISECT_EXP, "os") + os_name = self.config_parser.get(constants.INISECT_EXP, OS) + if os_name is None: + raise errors.OpPrereqError("No operating system information found") + results["os"]["name"] = os_name pairs = self.config_parser.items(constants.INISECT_OSP) for (name, value) in pairs: results["os"][name] = value - if results["os"].get("name") is None: - raise errors.OpPrereqError("No operating system information found") # other - results["disk_template"] = \ - self.config_parser.get(constants.INISECT_INS, "disk_template") - results["auto_balance"] = \ - self.config_parser.get(constants.INISECT_BEP, "auto_balance") - results["tags"] = \ - self.config_parser.get(constants.INISECT_INS, "tags") - results["version"] = \ - self.config_parser.get(constants.INISECT_EXP, "version") + others = [ + (constants.INISECT_INS, DISK_TEMPLATE, "disk_template"), + (constants.INISECT_BEP, AUTO_BALANCE, "auto_balance"), + (constants.INISECT_INS, TAGS, "tags"), + (constants.INISECT_EXP, VERSION, "version"), + ] + for (section, element, name) in others: + results[name] = self.config_parser.get(section, element) return results def _ParseNetworks(self): @@ -1300,24 +1311,29 @@ class OVFExporter(Converter): @rtype: list @return: list of dictionaries of network options - @raises errors.OpPrereqError: then network mode is not recognized + @raise errors.OpPrereqError: then network mode is not recognized """ - nics_count = self.config_parser.getint(constants.INISECT_INS, "nic_count") results = [] - for counter in range(nics_count): - results.append({}) - results[counter]["mode"] = \ - self.config_parser.get(constants.INISECT_INS, "nic%s_mode" % counter) - results[counter]["mac"] = \ - self.config_parser.get(constants.INISECT_INS, "nic%s_mac" % counter) - results[counter]["ip"] = \ - self.config_parser.get(constants.INISECT_INS, "nic%s_ip" % counter) - results[counter]["link"] = \ + counter = 0 + while True: + data_link = \ self.config_parser.get(constants.INISECT_INS, "nic%s_link" % counter) - if results[counter]["mode"] not in ["bridged", "routed"]: - raise errors.OpPrereqError("Network mode %s not recognized" % - results[counter]["mode"]) + if data_link is None: + break + results.append({ + "mode": self.config_parser.get(constants.INISECT_INS, + "nic%s_mode" % counter), + "mac": self.config_parser.get(constants.INISECT_INS, + "nic%s_mac" % counter), + "ip": self.config_parser.get(constants.INISECT_INS, + "nic%s_ip" % counter), + "link": data_link, + }) + if results[counter]["mode"] not in constants.NIC_VALID_MODES: + raise errors.OpPrereqError("Network mode %s not recognized" + % results[counter]["mode"]) + counter += 1 return results def _GetDiskOptions(self, disk_file, compression): @@ -1336,14 +1352,15 @@ class OVFExporter(Converter): if not os.path.isfile(disk_path): raise errors.OpPrereqError("Disk image does not exist: %s" % disk_path) if os.path.dirname(disk_file): - raise errors.OpPrereqError("Unsafe path for the disk: %s" % disk_path) + raise errors.OpPrereqError("Path for the disk: %s contains a directory" + " name" % disk_path) (disk_name, _) = os.path.splitext(disk_file) new_disk_name = "%s.%s" % (disk_name, self.options.disk_format) new_disk_path = utils.PathJoin(self.output_dir, new_disk_name) self._ConvertDisk(self.options.disk_format, disk_path, new_disk_path) results["format"] = self.options.disk_format results["virt-size"] = self._GetDiskQemuInfo(new_disk_path, - "virtual size: \S+ \((\S+) bytes\)") + "virtual size: \S+ \((\d+) bytes\)") if compression: skip_removing = disk_path == new_disk_path # we do not want to delete the original disk @@ -1352,7 +1369,7 @@ class OVFExporter(Converter): new_disk_name = "%s%s" % (new_disk_name, COMPRESSION_EXT) results["compression"] = "gzip" results["real-size"] = os.path.getsize(new_disk_path) - results["path"] = new_disk_name # TODO: path or name (after checking)? + results["path"] = new_disk_name self.references_files.append(new_disk_path) return results @@ -1363,14 +1380,15 @@ class OVFExporter(Converter): @return: list of dictionaries of disk options """ - disk_count = self.config_parser.getint(constants.INISECT_INS, "disk_count") results = [] - for counter in range(disk_count): - results.append({}) + counter = 0 + while True: disk_file = \ self.config_parser.get(constants.INISECT_INS, "disk%s_dump" % counter) - results[counter] = self._GetDiskOptions(disk_file, - self.options.compression) + if disk_file is None: + break + results.append(self._GetDiskOptions(disk_file, self.options.compression)) + counter += 1 return results def Parse(self): -- 1.7.3.1
