Skip to content

Update analysis_manager.py #2611

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update analysis_manager.py #2611

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
12 changes: 7 additions & 5 deletions lib/cuckoo/core/analysis_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ def machine_running(self) -> Generator[None, None, None]:
with self.db.session.begin():
self.db.guest_remove(self.guest.id)
self.db.assign_machine_to_task(self.task, None)
# ToDo do we really need to delete machine here?
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This "ToDo" raises an important question about the machine lifecycle when a CuckooMachineError occurs. Could you elaborate on the specific concerns or alternative behaviors you're considering here?

For example:

  • Are there scenarios where preserving the crashed VM state for post-mortem analysis would be beneficial, rather than immediately deleting it from the hypervisor?
  • Could a failed machine be marked as 'error' or 'needs_inspection' instead of being deleted, allowing for potential recovery or manual review?
  • What are the potential downsides of not deleting the machine in this context (e.g., resource consumption, cluttering the hypervisor)?

Clarifying the intent behind this "ToDo" would be helpful. If this points to a larger design consideration, perhaps it should be tracked as a separate issue or task.

self.machinery_manager.machinery.delete_machine(self.machine.name)

# Remove the analysis directory that has been created so
Expand Down Expand Up @@ -471,21 +472,22 @@ def launch_analysis(self) -> None:
success = self.perform_analysis()
except CuckooDeadMachine:
with self.db.session.begin():
# Put the task back in pending so that the schedule can attempt to
# choose a new machine.
# Put the task back in pending so that the schedule can attempt to choose a new machine.
self.db.set_status(self.task.id, TASK_PENDING)
raise
else:
with self.db.session.begin():
self.db.set_status(self.task.id, TASK_COMPLETED)
self.log.info("Completed analysis %ssuccessfully.", "" if success else "un")
# Need to be release on unsucess
if not success and hasattr(self, "machine") and self.machine:
self.db.unlock_machine(self.machine)

self.update_latest_symlink()

def update_latest_symlink(self):
# We make a symbolic link ("latest") which links to the latest
# analysis - this is useful for debugging purposes. This is only
# supported under systems that support symbolic links.
# We make a symbolic link ("latest") which links to the latest analysis this is useful for debugging purposes.
# This is only supported under systems that support symbolic links.
if not hasattr(os, "symlink"):
return

Expand Down