Re: Review Request 16461: CLOUDSTACK-5642 - Add else if connidition for identify direct nic type

2014-02-18 Thread Howie YU

---
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

2014-02-18 Thread Howie YU

---
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

2013-12-17 Thread Howie YU

---
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

2013-12-24 Thread Howie YU

---
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

2013-12-24 Thread Howie YU

---
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