On Tue, Apr 15, 2014 at 2:38 PM, Jose A. Lopes <[email protected]> wrote:
> Extract common code between the PVM and HVM Xen hypervisors regarding > NIC configuration. > > Signed-off-by: Jose A. Lopes <[email protected]> > --- > lib/hypervisor/hv_xen.py | 68 > +++++++++++++++++++++++------------------------- > 1 file changed, 33 insertions(+), 35 deletions(-) > > diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py > index 88ee37a..af13df8 100644 > --- a/lib/hypervisor/hv_xen.py > +++ b/lib/hypervisor/hv_xen.py > @@ -524,6 +524,37 @@ class XenHypervisor(hv_base.BaseHypervisor): > """ > raise NotImplementedError > > + def _WriteNicConfig(self, config, instance, hvp, nic_type=None): > + if nic_type is None: > + nic_type = "" > Nitpick: Aren't these two lines superfluous? The if nic_type: check later on will react the same to both these values. > + > + vif_data = [] > + > + for idx, nic in enumerate(instance.nics): > + if nic_type: > + nic_str = "mac=%s, %s" % (nic.mac, nic_type) > + else: > + nic_str = "mac=%s" % nic.mac > + > + ip = getattr(nic, "ip", None) > + if ip is not None: > + nic_str += ", ip=%s" % ip > + > + if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED: > + nic_str += ", bridge=%s" % nic.nicparams[constants.NIC_LINK] > + elif nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_OVS: > + nic_str += ", bridge=%s" % nic.nicparams[constants.NIC_LINK] > + if nic.nicparams[constants.NIC_VLAN]: > + nic_str += "%s" % nic.nicparams[constants.NIC_VLAN] > + > + if hvp[constants.HV_VIF_SCRIPT]: > + nic_str += ", script=%s" % hvp[constants.HV_VIF_SCRIPT] > + > + vif_data.append("'%s'" % nic_str) > + self._WriteNICInfoFile(instance, idx, nic) > + > + config.write("vif = [%s]\n" % ",".join(vif_data)) > + > def _WriteConfigFile(self, instance_name, data): > """Write the Xen config file for the instance. > > @@ -1177,27 +1208,10 @@ class XenPvmHypervisor(XenHypervisor): > > config.write("name = '%s'\n" % instance.name) > > - vif_data = [] > - for idx, nic in enumerate(instance.nics): > - nic_str = "mac=%s" % (nic.mac) > - ip = getattr(nic, "ip", None) > - if ip is not None: > - nic_str += ", ip=%s" % ip > - if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED: > - nic_str += ", bridge=%s" % nic.nicparams[constants.NIC_LINK] > - if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_OVS: > - nic_str += ", bridge=%s" % nic.nicparams[constants.NIC_LINK] > - if nic.nicparams[constants.NIC_VLAN]: > - nic_str += "%s" % nic.nicparams[constants.NIC_VLAN] > - if hvp[constants.HV_VIF_SCRIPT]: > - nic_str += ", script=%s" % hvp[constants.HV_VIF_SCRIPT] > - vif_data.append("'%s'" % nic_str) > - self._WriteNICInfoFile(instance, idx, nic) > + self._WriteNicConfig(config, instance, hvp) > > disk_data = \ > _GetConfigFileDiskData(block_devices, > hvp[constants.HV_BLOCKDEV_PREFIX]) > - > - config.write("vif = [%s]\n" % ",".join(vif_data)) > config.write("disk = [%s]\n" % ",".join(disk_data)) > > if hvp[constants.HV_ROOT_PATH]: > @@ -1340,12 +1354,7 @@ class XenHvmHypervisor(XenHypervisor): > if hvp[constants.HV_USE_LOCALTIME]: > config.write("localtime = 1\n") > > - vif_data = [] > - # Note: what is called 'nic_type' here, is used as value for the xen > nic > - # vif config parameter 'model'. For the xen nic vif parameter 'type', > we use > - # the 'vif_type' to avoid a clash of notation. > nic_type = hvp[constants.HV_NIC_TYPE] > - > if nic_type is None: > vif_type_str = "" > if hvp[constants.HV_VIF_TYPE]: > @@ -1358,19 +1367,8 @@ class XenHvmHypervisor(XenHypervisor): > # parameter 'model' is only valid with type 'ioemu' > nic_type_str = ", model=%s, type=%s" % \ > (nic_type, constants.HT_HVM_VIF_IOEMU) > - for idx, nic in enumerate(instance.nics): > - nic_str = "mac=%s%s" % (nic.mac, nic_type_str) > - ip = getattr(nic, "ip", None) > - if ip is not None: > - nic_str += ", ip=%s" % ip > - if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED: > - nic_str += ", bridge=%s" % nic.nicparams[constants.NIC_LINK] > - if hvp[constants.HV_VIF_SCRIPT]: > - nic_str += ", script=%s" % hvp[constants.HV_VIF_SCRIPT] > - vif_data.append("'%s'" % nic_str) > - self._WriteNICInfoFile(instance, idx, nic) > > - config.write("vif = [%s]\n" % ",".join(vif_data)) > + self._WriteNicConfig(config, instance, hvp, nic_type=nic_type_str) > > disk_data = \ > _GetConfigFileDiskData(block_devices, > hvp[constants.HV_BLOCKDEV_PREFIX]) > -- > 1.9.1.423.g4596e3a > > Otherwise LGTM, thanks
