Skip to content

Fix KVM cloudstack-agent start if there is vm not managed by cloudstack on the host #8049

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.

Sign up for GitHub

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

Merged
weizhouapache merged 1 commit into from
Nov 3, 2023
Merged

Fix KVM cloudstack-agent start if there is vm not managed by cloudstack on the host #8049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions ...rvisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1313,8 +1313,8 @@ protected void setupMemoryBalloonStatsPeriod(Connect conn) {
}
s_logger.debug(String.format("The memory balloon stats period [%s] has been set successfully for the VM (Libvirt Domain) with ID [%s] and name [%s].",
currentVmBalloonStatsPeriod, vmId, dm.getName()));
} catch (final LibvirtException e) {
s_logger.warn("Failed to set up memory balloon stats period." + e.getMessage());
} catch (final Exception e) {
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Oct 6, 2023

Choose a reason for hiding this comment

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

Is this needed or can we add a specific Exception to the catch clause?

Copy link
Copy Markdown
Member Author

@harikrishna-patnala harikrishna-patnala Oct 9, 2023

Choose a reason for hiding this comment

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

@DaanHoogland we are trying to read many other parameters in that parser. so there are chances of other errors such as NPEs, cast errors. While discussing with Wei several other found in IDE, so thought a common exception could catch it and warn the message.
image

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Oct 9, 2023

Choose a reason for hiding this comment

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

ok, I hate it and I would like to see a message per exception, but no -1 for that

Copy link
Copy Markdown
Member Author

@harikrishna-patnala harikrishna-patnala Oct 9, 2023

Choose a reason for hiding this comment

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

:) So you are giving +1 on this right ;)

s_logger.warn(String.format("Failed to set up memory balloon stats period for the VM %s with exception %s", parser.getName(), e.getMessage()));
}
}
}
Expand Down
25 changes: 18 additions & 7 deletions ...pervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ public class LibvirtDomainXMLParser {
private Integer vncPort;
private String desc;

private String name;

public boolean parseDomainXML(String domXML) {
DocumentBuilder builder;
try {
Expand All @@ -71,6 +73,7 @@ public boolean parseDomainXML(String domXML) {
Element rootElement = doc.getDocumentElement();

desc = getTagValue("description", rootElement);
name = getTagValue("name", rootElement);

Element devices = (Element)rootElement.getElementsByTagName("devices").item(0);
NodeList disks = devices.getElementsByTagName("disk");
Expand Down Expand Up @@ -303,15 +306,19 @@ public boolean parseDomainXML(String domXML) {
String path = getTagValue("backend", rng);
String bytes = getAttrValue("rate", "bytes", rng);
String period = getAttrValue("rate", "period", rng);

if (StringUtils.isEmpty(backendModel)) {
def = new RngDef(path, Integer.parseInt(bytes), Integer.parseInt(period));
if (StringUtils.isAnyEmpty(bytes, period)) {
s_logger.debug(String.format("Bytes and period in the rng section should not be null, please check the VM %s", name));
} else {
def = new RngDef(path, RngBackendModel.valueOf(backendModel.toUpperCase()),
Integer.parseInt(bytes), Integer.parseInt(period));
if (StringUtils.isEmpty(backendModel)) {
def = new RngDef(path, Integer.parseInt(bytes), Integer.parseInt(period));
} else {
def = new RngDef(path, RngBackendModel.valueOf(backendModel.toUpperCase()),
Integer.parseInt(bytes), Integer.parseInt(period));
}
}
if (def != null) {
rngDefs.add(def);
}

rngDefs.add(def);
}

NodeList watchDogs = devices.getElementsByTagName("watchdog");
Expand Down Expand Up @@ -418,4 +425,8 @@ public List<WatchDogDef> getWatchDogs() {
public String getDescription() {
return desc;
}

public String getName() {
return name;
}
}