Re: Review Request 16461: CLOUDSTACK-5642 - Add else if connidition for identify direct nic type
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16461/ --- (Updated Feb. 19, 2014, 2:26 a.m.) Review request for cloudstack, Wei Zhou and Wido den Hollander. Changes --- Merge with master Bugs: CLOUDSTACK-5642 https://issues.apache.org/jira/browse/CLOUDSTACK-5642 Repository: cloudstack-git Description --- Libvirt supports direct attachment of the guest VM's network to a physical interface. But I found the following code may cause some problem, when the nic interface type is direct If type is direct , then this nic will be ignore. String type = nic.getAttribute("type"); String mac = getAttrValue("mac", "address", nic); String dev = getAttrValue("target", "dev", nic); String model = getAttrValue("model", "type", nic); InterfaceDef def = new InterfaceDef(); NodeList bandwidth = nic.getElementsByTagName("bandwidth"); Integer networkRateKBps = 0; if ((bandwidth != null) && (bandwidth.getLength() != 0)) { Integer inbound = Integer.valueOf(getAttrValue("inbound", "average", (Element)bandwidth.item(0))); Integer outbound = Integer.valueOf(getAttrValue("outbound", "average", (Element)bandwidth.item(0))); if (inbound == outbound) networkRateKBps = inbound; } if (type.equalsIgnoreCase("network")) { String network = getAttrValue("source", "network", nic); def.defPrivateNet(network, dev, mac, nicModel.valueOf(model.toUpperCase()), networkRateKBps); } else if (type.equalsIgnoreCase("bridge")) { String bridge = getAttrValue("source", "bridge", nic); def.defBridgeNet(bridge, dev, mac, nicModel.valueOf(model.toUpperCase()), networkRateKBps); } else if (type.equalsIgnoreCase("ethernet")) { String scriptPath = getAttrValue("script", "path", nic); def.defEthernet(dev, mac, nicModel.valueOf(model.toUpperCase()), scriptPath, networkRateKBps); } interfaces.add(def); Diffs (updated) - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java 127f648 plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java PRE-CREATION Diff: https://reviews.apache.org/r/16461/diff/ Testing --- I add a unit test LibvirtDomainXMLParserTest in test directory , and add a direct nic type domain xml for test ubuntu 77919cf3-b2a7-994f-5f3c-0a91fb9bcbe8 1048576 1048576 1 hvm destroy restart restart /usr/libexec/qemu-kvm Thanks, Howie YU
Re: Review Request 16337: [CLOUDSTACK-5526] Fix LibvirtDomainXMLParser error when diskCacheMode is empty string
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16337/ --- (Updated Feb. 19, 2014, 2:38 a.m.) Review request for cloudstack, Wei Zhou and Wido den Hollander. Changes --- Patch again from master Bugs: CLOUDSTACK-5526 https://issues.apache.org/jira/browse/CLOUDSTACK-5526 Repository: cloudstack-git Description --- When using LibvirtDomainXMLParser parser xml from Domain. The attribute diskCacheMode not always have value , and will be empty string String diskCacheMode = getAttrValue("driver", "cache", disk); when the code go to here valueOf } else if (type.equalsIgnoreCase("block")) { def.defBlockBasedDisk(diskDev, diskLabel, DiskDef.diskBus.valueOf(bus.toUpperCase())); def.setCacheMode(DiskDef.diskCacheMode.valueOf(diskCacheMode)); } There will be cause IllegalArgumentException at at java.lang.Enum.valueOf(Enum.java:196) I suggest we may check if diskCacheMode is empty string , such as if (diskCacheMode == null || diskCacheMode.isEmpty()) { def.setCacheMode(DiskDef.diskCacheMode.NONE); } else { def.setCacheMode(DiskDef.diskCacheMode.valueOf(diskCacheMode)); } Diffs (updated) - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java 9cf6a90 Diff: https://reviews.apache.org/r/16337/diff/ Testing --- Thanks, Howie YU
Review Request 16337: [CLOUDSTACK-5526] Fix LibvirtDomainXMLParser error when diskCacheMode is empty string
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16337/ --- Review request for cloudstack and Wido den Hollander. Bugs: CLOUDSTACK-5526 https://issues.apache.org/jira/browse/CLOUDSTACK-5526 Repository: cloudstack-git Description --- When using LibvirtDomainXMLParser parser xml from Domain. The attribute diskCacheMode not always have value , and will be empty string String diskCacheMode = getAttrValue("driver", "cache", disk); when the code go to here valueOf } else if (type.equalsIgnoreCase("block")) { def.defBlockBasedDisk(diskDev, diskLabel, DiskDef.diskBus.valueOf(bus.toUpperCase())); def.setCacheMode(DiskDef.diskCacheMode.valueOf(diskCacheMode)); } There will be cause IllegalArgumentException at at java.lang.Enum.valueOf(Enum.java:196) I suggest we may check if diskCacheMode is empty string , such as if (diskCacheMode == null || diskCacheMode.isEmpty()) { def.setCacheMode(DiskDef.diskCacheMode.NONE); } else { def.setCacheMode(DiskDef.diskCacheMode.valueOf(diskCacheMode)); } Diffs - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java 127f648 Diff: https://reviews.apache.org/r/16337/diff/ Testing --- Thanks, Howie YU
Re: Review Request 16337: [CLOUDSTACK-5526] Fix LibvirtDomainXMLParser error when diskCacheMode is empty string
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16337/ --- (Updated Dec. 25, 2013, 1:47 a.m.) Review request for cloudstack, Wei Zhou and Wido den Hollander. Bugs: CLOUDSTACK-5526 https://issues.apache.org/jira/browse/CLOUDSTACK-5526 Repository: cloudstack-git Description --- When using LibvirtDomainXMLParser parser xml from Domain. The attribute diskCacheMode not always have value , and will be empty string String diskCacheMode = getAttrValue("driver", "cache", disk); when the code go to here valueOf } else if (type.equalsIgnoreCase("block")) { def.defBlockBasedDisk(diskDev, diskLabel, DiskDef.diskBus.valueOf(bus.toUpperCase())); def.setCacheMode(DiskDef.diskCacheMode.valueOf(diskCacheMode)); } There will be cause IllegalArgumentException at at java.lang.Enum.valueOf(Enum.java:196) I suggest we may check if diskCacheMode is empty string , such as if (diskCacheMode == null || diskCacheMode.isEmpty()) { def.setCacheMode(DiskDef.diskCacheMode.NONE); } else { def.setCacheMode(DiskDef.diskCacheMode.valueOf(diskCacheMode)); } Diffs - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java 127f648 Diff: https://reviews.apache.org/r/16337/diff/ Testing --- Thanks, Howie YU
Review Request 16461: CLOUDSTACK-5642 - Add else if connidition for identify direct nic type
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16461/ --- Review request for cloudstack, Wei Zhou and Wido den Hollander. Bugs: CLOUDSTACK-5642 https://issues.apache.org/jira/browse/CLOUDSTACK-5642 Repository: cloudstack-git Description --- Libvirt supports direct attachment of the guest VM's network to a physical interface. But I found the following code may cause some problem, when the nic interface type is direct If type is direct , then this nic will be ignore. String type = nic.getAttribute("type"); String mac = getAttrValue("mac", "address", nic); String dev = getAttrValue("target", "dev", nic); String model = getAttrValue("model", "type", nic); InterfaceDef def = new InterfaceDef(); NodeList bandwidth = nic.getElementsByTagName("bandwidth"); Integer networkRateKBps = 0; if ((bandwidth != null) && (bandwidth.getLength() != 0)) { Integer inbound = Integer.valueOf(getAttrValue("inbound", "average", (Element)bandwidth.item(0))); Integer outbound = Integer.valueOf(getAttrValue("outbound", "average", (Element)bandwidth.item(0))); if (inbound == outbound) networkRateKBps = inbound; } if (type.equalsIgnoreCase("network")) { String network = getAttrValue("source", "network", nic); def.defPrivateNet(network, dev, mac, nicModel.valueOf(model.toUpperCase()), networkRateKBps); } else if (type.equalsIgnoreCase("bridge")) { String bridge = getAttrValue("source", "bridge", nic); def.defBridgeNet(bridge, dev, mac, nicModel.valueOf(model.toUpperCase()), networkRateKBps); } else if (type.equalsIgnoreCase("ethernet")) { String scriptPath = getAttrValue("script", "path", nic); def.defEthernet(dev, mac, nicModel.valueOf(model.toUpperCase()), scriptPath, networkRateKBps); } interfaces.add(def); Diffs - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java 127f648 plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java PRE-CREATION Diff: https://reviews.apache.org/r/16461/diff/ Testing --- I add a unit test LibvirtDomainXMLParserTest in test directory , and add a direct nic type domain xml for test ubuntu 77919cf3-b2a7-994f-5f3c-0a91fb9bcbe8 1048576 1048576 1 hvm destroy restart restart /usr/libexec/qemu-kvm Thanks, Howie YU