Skip to content

VMware: match nic mac for ip address fetch #10641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: 4.20
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ public class GetVmIpAddressCommand extends Command {
String vmName;
String vmNetworkCidr;
boolean windows = false;
String macAddress;

public GetVmIpAddressCommand(String vmName, String vmNetworkCidr, boolean windows) {
public GetVmIpAddressCommand(String vmName, String vmNetworkCidr, boolean windows, String macAddress) {
this.vmName = vmName;
this.windows = windows;
this.vmNetworkCidr = vmNetworkCidr;
this.macAddress = macAddress;
}

@Override
Expand All @@ -47,4 +49,8 @@ public boolean isWindows(){
public String getVmNetworkCidr() {
return vmNetworkCidr;
}

public String getMacAddress() {
return macAddress;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5738,7 +5738,6 @@ protected Answer execute(GetVmIpAddressCommand cmd) {
String details = "Unable to find IP Address of VM. ";
String vmName = cmd.getVmName();
boolean result = false;
String ip = null;
Answer answer = null;

VmwareContext context = getServiceContext();
Expand All @@ -5757,11 +5756,19 @@ protected Answer execute(GetVmIpAddressCommand cmd) {
if (toolsStatus == VirtualMachineToolsStatus.TOOLS_NOT_INSTALLED) {
details += "Vmware tools not installed.";
} else {
ip = guestInfo.getIpAddress();
if (ip != null) {
result = true;
var normalizedMac = cmd.getMacAddress().replaceAll("-", ":");
for(var guestInfoNic : guestInfo.getNet()) {
var normalizedNicMac = guestInfoNic.getMacAddress().replaceAll("-", ":");
if (!result && normalizedNicMac.equalsIgnoreCase(normalizedMac)) {
result = true;
details = null;
for (var ipAddr : guestInfoNic.getIpAddress()) {
if(NetUtils.isIpWithInCidrRange(ipAddr, cmd.getVmNetworkCidr())) {
details = ipAddr;
}
}
}
}
details = ip;
}
} else {
details += "VM " + vmName + " no longer exists on vSphere host: " + hyperHost.getHyperHostName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1934,7 +1934,7 @@ public void testGetVmIpAddressCommand() throws XenAPIException, XmlRpcException
vmIpsMap.put("Test", "127.0.0.1");
rec.networks = vmIpsMap;

final GetVmIpAddressCommand getVmIpAddrCmd = new GetVmIpAddressCommand("Test", "127.0.0.1/24", false);
final GetVmIpAddressCommand getVmIpAddrCmd = new GetVmIpAddressCommand("Test", "127.0.0.1/24", false, null);

final CitrixRequestWrapper wrapper = CitrixRequestWrapper.getInstance();
assertNotNull(wrapper);
Expand Down
23 changes: 12 additions & 11 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -749,20 +749,22 @@ private class VmIpAddrFetchThread extends ManagedContextRunnable {
boolean isWindows;
Long hostId;
String networkCidr;
String macAddress;

public VmIpAddrFetchThread(long vmId, String vmUuid, long nicId, String instanceName, boolean windows, Long hostId, String networkCidr) {
public VmIpAddrFetchThread(long vmId, String vmUuid, long nicId, String instanceName, boolean windows, Long hostId, String networkCidr, String macAddress) {
this.vmId = vmId;
this.vmUuid = vmUuid;
this.nicId = nicId;
this.vmName = instanceName;
this.isWindows = windows;
this.hostId = hostId;
this.networkCidr = networkCidr;
this.macAddress = macAddress;
}

@Override
protected void runInContext() {
GetVmIpAddressCommand cmd = new GetVmIpAddressCommand(vmName, networkCidr, isWindows);
GetVmIpAddressCommand cmd = new GetVmIpAddressCommand(vmName, networkCidr, isWindows, macAddress);
boolean decrementCount = true;

NicVO nic = _nicDao.findById(nicId);
Expand All @@ -771,8 +773,13 @@ protected void runInContext() {
Answer answer = _agentMgr.send(hostId, cmd);
if (answer.getResult()) {
String vmIp = answer.getDetails();

if (NetUtils.isValidIp4(vmIp)) {
if (null == vmIp) {
// we got a valid response and the NIC does not have an IP assigned, as such we will update the database with null
if (nic.getIPv4Address() != null) {
nic.setIPv4Address(null);
_nicDao.update(nicId, nic);
}
} else if (NetUtils.isValidIp4(vmIp)) {
// set this vm ip addr in vm nic.
if (nic != null) {
nic.setIPv4Address(vmIp);
Expand All @@ -787,12 +794,6 @@ protected void runInContext() {
}
}
} else {
//previously vm has ip and nic table has ip address. After vm restart or stop/start
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, if we cannot know if the VM still has the IP assigned or not then we should not modify the database. In other words, we should be sure that the VM's NIC does not have an IP assigned before updating the database.

//if vm doesnot get the ip then set the ip in nic table to null
if (nic.getIPv4Address() != null) {
nic.setIPv4Address(null);
_nicDao.update(nicId, nic);
}
if (answer.getDetails() != null) {
logger.debug("Failed to get vm ip for Vm [id: {}, uuid: {}, name: {}], details: {}",
vmId, vmUuid, vmName, answer.getDetails());
Expand Down Expand Up @@ -2687,7 +2688,7 @@ protected void runInContext() {
boolean isWindows = _guestOSCategoryDao.findById(_guestOSDao.findById(vm.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows");

_vmIpFetchThreadExecutor.execute(new VmIpAddrFetchThread(vmId, vmInstance.getUuid(), nicId, vmInstance.getInstanceName(),
isWindows, vm.getHostId(), network.getCidr()));
isWindows, vm.getHostId(), network.getCidr(), nicVo.getMacAddress()));

}
} catch (Exception e) {
Expand Down
Loading