You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "wido (via GitHub)" <gi...@apache.org> on 2026/08/02 07:24:31 UTC
[PR] KVM: packet capture of Instance NICs via operator-defined script [cloudstack]
wido opened a new pull request, #13767:
URL: https://github.com/apache/cloudstack/pull/13767
# Description
Adds root-admin-only APIs to capture the traffic of an Instance NIC on the KVM host it is running on, for debugging and lawful interception:
* enablePacketCapture nicid=<uuid>
* disablePacketCapture nicid=<uuid>
* getPacketCaptureStatus nicid=<uuid>
Packet capture is a flag on the NIC (stored as a nic detail). When enabled, the agent resolves the host-side tap device by MAC address, writes the NIC context (VM name/UUID, NIC UUID/MAC, IPv4/IPv6, bridge, network UUID) to /run/cloudstack/pcap-<dev>.env and starts the systemd template unit cloudstack-pcap@<dev>.service. The flag follows the Instance: a VM state listener restarts the capture after start and migration, and BindsTo= on the tap device stops the unit when the VM stops, migrates away or the NIC is unplugged.
The unit executes pcap-capture.sh, shipped between the other KVM scripts in /usr/share/cloudstack-common as an example that runs tcpdump writing to /tmp with 256 MB rotation. What to capture and what to do with the data differs too much per environment for CloudStack to decide it, so operators run their own script by copying the unit, pointing its ExecStart at it and naming their unit in packet.capture.service.
New agent.properties keys:
- packet.capture.service (systemd template unit name)
- packet.capture.env.dir (environment file directory)
This is in no way a "fits all" solution as capturing traffic will really differ per environment. I thought about storing it on Secondary Storage, but that would add so many different API calls and logic to CloudStack while in most cases every admin wants to do this differently. Therefor I have choosen this option.
Admins can override the packet capture script with a version that suits their environment.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KVM: packet capture of Instance NICs via operator-defined script [cloudstack]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13767:
URL: https://github.com/apache/cloudstack/pull/13767#discussion_r3701578865
##########
server/src/main/java/org/apache/cloudstack/network/packetcapture/PacketCaptureServiceImpl.java:
##########
@@ -0,0 +1,213 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.network.packetcapture;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+
+import org.apache.cloudstack.api.command.admin.nic.DisablePacketCaptureCmd;
+import org.apache.cloudstack.api.command.admin.nic.EnablePacketCaptureCmd;
+import org.apache.cloudstack.api.command.admin.nic.GetPacketCaptureStatusCmd;
+import org.apache.cloudstack.api.response.PacketCaptureResponse;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.dao.NetworkVO;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.component.PluggableService;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.StateListener;
+import com.cloud.utils.fsm.StateMachine2;
+import com.cloud.vm.NicDetailVO;
+import com.cloud.vm.NicVO;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VirtualMachine.Event;
+import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.dao.NicDao;
+import com.cloud.vm.dao.NicDetailsDao;
+import com.cloud.vm.dao.VMInstanceDao;
+
+public class PacketCaptureServiceImpl extends ManagerBase implements PacketCaptureService, PluggableService,
+ StateListener<State, Event, VirtualMachine> {
+
+ @Inject
+ private NicDao nicDao;
+ @Inject
+ private NicDetailsDao nicDetailsDao;
+ @Inject
+ private VMInstanceDao vmInstanceDao;
+ @Inject
+ private NetworkDao networkDao;
+ @Inject
+ private AgentManager agentManager;
+
+ @Override
+ public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
+ VirtualMachine.State.getStateMachine().registerListener(this);
+ return true;
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_NIC_PACKET_CAPTURE_ENABLE, eventDescription = "enabling packet capture", async = true)
+ public void enablePacketCapture(long nicId) {
+ NicVO nic = validateAndGetNic(nicId);
+ VMInstanceVO vm = validateAndGetVm(nic);
+ if (isVmRunningOnHost(vm)) {
+ sendCommand(PacketCaptureCommand.Action.START, vm, nic);
+ }
+ nicDetailsDao.removeDetail(nic.getId(), PACKET_CAPTURE_NIC_DETAIL);
+ nicDetailsDao.addDetail(nic.getId(), PACKET_CAPTURE_NIC_DETAIL, Boolean.TRUE.toString(), true);
+ logger.info("Enabled packet capture on NIC {} of VM {}", nic, vm);
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_NIC_PACKET_CAPTURE_DISABLE, eventDescription = "disabling packet capture", async = true)
+ public void disablePacketCapture(long nicId) {
+ NicVO nic = validateAndGetNic(nicId);
+ VMInstanceVO vm = validateAndGetVm(nic);
+ if (isVmRunningOnHost(vm)) {
+ sendCommand(PacketCaptureCommand.Action.STOP, vm, nic);
+ }
+ nicDetailsDao.removeDetail(nic.getId(), PACKET_CAPTURE_NIC_DETAIL);
+ logger.info("Disabled packet capture on NIC {} of VM {}", nic, vm);
+ }
+
+ @Override
+ public PacketCaptureResponse getPacketCaptureStatus(long nicId) {
+ NicVO nic = validateAndGetNic(nicId);
+ VMInstanceVO vm = validateAndGetVm(nic);
+
+ boolean running = false;
+ if (isPacketCaptureEnabled(nic.getId()) && isVmRunningOnHost(vm)) {
+ PacketCaptureAnswer answer = sendCommand(PacketCaptureCommand.Action.STATUS, vm, nic);
+ running = answer.isRunning();
+ }
+
+ PacketCaptureResponse response = new PacketCaptureResponse();
+ response.setNicId(nic.getUuid());
+ response.setVirtualMachineId(vm.getUuid());
+ response.setVirtualMachineName(vm.getInstanceName());
+ response.setMacAddress(nic.getMacAddress());
+ response.setEnabled(isPacketCaptureEnabled(nic.getId()));
+ response.setRunning(running);
+ return response;
+ }
+
+ @Override
+ public boolean preStateTransitionEvent(State oldState, Event event, State newState, VirtualMachine vo, boolean status, Object opaque) {
+ return true;
+ }
+
+ @Override
+ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> transition, VirtualMachine vm, boolean status, Object opaque) {
+ if (!status) {
+ return true;
+ }
+ State oldState = transition.getCurrentState();
+ State newState = transition.getToState();
+ Event event = transition.getEvent();
+ if (State.isVmStarted(oldState, event, newState) || State.isVmMigrated(oldState, event, newState)) {
+ startEnabledCapturesForVm(vm);
+ }
+ return true;
+ }
+
+ /**
+ * Starts the capture on the current host of the VM for every NIC that has
+ * packet capture enabled. Called after a VM started or migrated; failures
+ * are logged and do not fail the VM operation.
+ */
+ private void startEnabledCapturesForVm(VirtualMachine vm) {
+ if (vm.getHypervisorType() != HypervisorType.KVM || vm.getHostId() == null) {
+ return;
+ }
+ for (NicVO nic : nicDao.listByVmId(vm.getId())) {
+ if (!isPacketCaptureEnabled(nic.getId())) {
+ continue;
+ }
+ try {
+ VMInstanceVO vmVo = vmInstanceDao.findById(vm.getId());
+ sendCommand(PacketCaptureCommand.Action.START, vmVo, nic);
+ logger.info("Started packet capture on NIC {} of VM {} on host {}", nic, vm, vm.getHostId());
+ } catch (Exception e) {
+ logger.warn("Failed to start packet capture on NIC {} of VM {} on host {}", nic, vm, vm.getHostId(), e);
+ }
+ }
+ }
+
+ private boolean isPacketCaptureEnabled(long nicId) {
+ NicDetailVO detail = nicDetailsDao.findDetail(nicId, PACKET_CAPTURE_NIC_DETAIL);
+ return detail != null && Boolean.parseBoolean(detail.getValue());
+ }
+
+ private NicVO validateAndGetNic(long nicId) {
+ NicVO nic = nicDao.findById(nicId);
+ if (nic == null || nic.getRemoved() != null) {
+ throw new InvalidParameterValueException("Unable to find a NIC with the specified id");
+ }
+ return nic;
+ }
+
+ private VMInstanceVO validateAndGetVm(NicVO nic) {
+ Long vmId = nic.getInstanceId();
+ VMInstanceVO vm = vmId == null ? null : vmInstanceDao.findById(vmId);
+ if (vm == null) {
+ throw new InvalidParameterValueException(String.format("NIC %s is not attached to an Instance", nic.getUuid()));
+ }
+ if (vm.getHypervisorType() != null && vm.getHypervisorType() != HypervisorType.KVM) {
+ throw new InvalidParameterValueException("Packet capture is only supported on KVM");
+ }
+ return vm;
+ }
+
+ private boolean isVmRunningOnHost(VMInstanceVO vm) {
+ return vm.getState() == State.Running && vm.getHostId() != null;
+ }
+
+ private PacketCaptureAnswer sendCommand(PacketCaptureCommand.Action action, VMInstanceVO vm, NicVO nic) {
+ NetworkVO network = networkDao.findById(nic.getNetworkId());
+ PacketCaptureCommand command = new PacketCaptureCommand(action, vm.getInstanceName(), vm.getUuid(),
+ nic.getUuid(), nic.getMacAddress(), nic.getIPv4Address(), nic.getIPv6Address(),
+ network == null ? null : network.getUuid());
+ Answer answer = agentManager.easySend(vm.getHostId(), command);
+ if (answer == null || !answer.getResult()) {
+ throw new CloudRuntimeException(String.format("Failed to %s packet capture for NIC %s of VM %s on host %d: %s",
+ action.name().toLowerCase(), nic.getUuid(), vm.getInstanceName(), vm.getHostId(),
+ answer == null ? "no answer from host" : answer.getDetails()));
+ }
+ return (PacketCaptureAnswer) answer;
Review Comment:
sendCommand assumes the agent will always return PacketCaptureAnswer and unconditionally casts the Answer. In mixed-version deployments or when a host doesn't support the command, easySend can return a different Answer type, causing a ClassCastException and masking the real failure. Add an instanceof check and throw a clear CloudRuntimeException when the answer type is unexpected.
##########
packaging/systemd/cloudstack-pcap@.service:
##########
@@ -0,0 +1,41 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Systemd unit file for CloudStack packet capture on a single VM NIC.
+#
+# Started by the CloudStack KVM agent as cloudstack-pcap@<interface>.service
+# when packet capture is enabled on a NIC. The agent writes the NIC context
+# to /run/cloudstack/pcap-<interface>.env before starting this unit.
+#
+# BindsTo ties the unit to the tap device: when the VM is stopped, migrated
+# away or the NIC is unplugged, the device disappears and systemd stops the
+# capture automatically.
+#
+# The capture script shipped with CloudStack is an example. To run your own,
+# copy this unit, point its ExecStart at your script and set the property
+# packet.capture.service in agent.properties to the name of your unit.
+
+[Unit]
+Description=CloudStack packet capture on %I
+BindsTo=sys-subsystem-net-devices-%i.device
+After=sys-subsystem-net-devices-%i.device
+
+[Service]
+Type=simple
+EnvironmentFile=/run/cloudstack/pcap-%i.env
+ExecStart=/usr/share/cloudstack-common/scripts/vm/hypervisor/kvm/pcap-capture.sh
+Restart=no
Review Comment:
The unit runs a root packet capture process that writes output files; without an explicit umask, captures can be created world-readable depending on the system default. Set UMask=0077 so capture output (and any other files created by the script) are root-only by default.
##########
server/src/main/java/org/apache/cloudstack/network/packetcapture/PacketCaptureServiceImpl.java:
##########
@@ -0,0 +1,213 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.network.packetcapture;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+
+import org.apache.cloudstack.api.command.admin.nic.DisablePacketCaptureCmd;
+import org.apache.cloudstack.api.command.admin.nic.EnablePacketCaptureCmd;
+import org.apache.cloudstack.api.command.admin.nic.GetPacketCaptureStatusCmd;
+import org.apache.cloudstack.api.response.PacketCaptureResponse;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.dao.NetworkVO;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.component.PluggableService;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.StateListener;
+import com.cloud.utils.fsm.StateMachine2;
+import com.cloud.vm.NicDetailVO;
+import com.cloud.vm.NicVO;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VirtualMachine.Event;
+import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.dao.NicDao;
+import com.cloud.vm.dao.NicDetailsDao;
+import com.cloud.vm.dao.VMInstanceDao;
+
+public class PacketCaptureServiceImpl extends ManagerBase implements PacketCaptureService, PluggableService,
+ StateListener<State, Event, VirtualMachine> {
+
+ @Inject
+ private NicDao nicDao;
+ @Inject
+ private NicDetailsDao nicDetailsDao;
+ @Inject
+ private VMInstanceDao vmInstanceDao;
+ @Inject
+ private NetworkDao networkDao;
+ @Inject
+ private AgentManager agentManager;
+
+ @Override
+ public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
+ VirtualMachine.State.getStateMachine().registerListener(this);
+ return true;
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_NIC_PACKET_CAPTURE_ENABLE, eventDescription = "enabling packet capture", async = true)
+ public void enablePacketCapture(long nicId) {
+ NicVO nic = validateAndGetNic(nicId);
+ VMInstanceVO vm = validateAndGetVm(nic);
+ if (isVmRunningOnHost(vm)) {
+ sendCommand(PacketCaptureCommand.Action.START, vm, nic);
+ }
+ nicDetailsDao.removeDetail(nic.getId(), PACKET_CAPTURE_NIC_DETAIL);
+ nicDetailsDao.addDetail(nic.getId(), PACKET_CAPTURE_NIC_DETAIL, Boolean.TRUE.toString(), true);
+ logger.info("Enabled packet capture on NIC {} of VM {}", nic, vm);
+ }
+
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_NIC_PACKET_CAPTURE_DISABLE, eventDescription = "disabling packet capture", async = true)
+ public void disablePacketCapture(long nicId) {
+ NicVO nic = validateAndGetNic(nicId);
+ VMInstanceVO vm = validateAndGetVm(nic);
+ if (isVmRunningOnHost(vm)) {
+ sendCommand(PacketCaptureCommand.Action.STOP, vm, nic);
+ }
+ nicDetailsDao.removeDetail(nic.getId(), PACKET_CAPTURE_NIC_DETAIL);
+ logger.info("Disabled packet capture on NIC {} of VM {}", nic, vm);
+ }
+
+ @Override
+ public PacketCaptureResponse getPacketCaptureStatus(long nicId) {
+ NicVO nic = validateAndGetNic(nicId);
+ VMInstanceVO vm = validateAndGetVm(nic);
+
+ boolean running = false;
+ if (isPacketCaptureEnabled(nic.getId()) && isVmRunningOnHost(vm)) {
+ PacketCaptureAnswer answer = sendCommand(PacketCaptureCommand.Action.STATUS, vm, nic);
+ running = answer.isRunning();
+ }
+
+ PacketCaptureResponse response = new PacketCaptureResponse();
+ response.setNicId(nic.getUuid());
+ response.setVirtualMachineId(vm.getUuid());
+ response.setVirtualMachineName(vm.getInstanceName());
+ response.setMacAddress(nic.getMacAddress());
+ response.setEnabled(isPacketCaptureEnabled(nic.getId()));
+ response.setRunning(running);
+ return response;
+ }
+
+ @Override
+ public boolean preStateTransitionEvent(State oldState, Event event, State newState, VirtualMachine vo, boolean status, Object opaque) {
+ return true;
+ }
+
+ @Override
+ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> transition, VirtualMachine vm, boolean status, Object opaque) {
+ if (!status) {
+ return true;
+ }
+ State oldState = transition.getCurrentState();
+ State newState = transition.getToState();
+ Event event = transition.getEvent();
+ if (State.isVmStarted(oldState, event, newState) || State.isVmMigrated(oldState, event, newState)) {
+ startEnabledCapturesForVm(vm);
+ }
+ return true;
+ }
+
+ /**
+ * Starts the capture on the current host of the VM for every NIC that has
+ * packet capture enabled. Called after a VM started or migrated; failures
+ * are logged and do not fail the VM operation.
+ */
+ private void startEnabledCapturesForVm(VirtualMachine vm) {
+ if (vm.getHypervisorType() != HypervisorType.KVM || vm.getHostId() == null) {
+ return;
+ }
+ for (NicVO nic : nicDao.listByVmId(vm.getId())) {
+ if (!isPacketCaptureEnabled(nic.getId())) {
+ continue;
+ }
+ try {
+ VMInstanceVO vmVo = vmInstanceDao.findById(vm.getId());
+ sendCommand(PacketCaptureCommand.Action.START, vmVo, nic);
+ logger.info("Started packet capture on NIC {} of VM {} on host {}", nic, vm, vm.getHostId());
+ } catch (Exception e) {
+ logger.warn("Failed to start packet capture on NIC {} of VM {} on host {}", nic, vm, vm.getHostId(), e);
+ }
+ }
Review Comment:
startEnabledCapturesForVm re-queries VMInstanceVO inside the NIC loop and can end up calling sendCommand with a null vmVo (leading to a caught NPE + log spam). Fetch the VMInstanceVO once before the loop and bail out early if it's missing / not on a host, then reuse it for all NIC starts.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPacketCaptureCommandWrapper.java:
##########
@@ -0,0 +1,166 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.cloudstack.network.packetcapture.PacketCaptureAnswer;
+import org.apache.cloudstack.network.packetcapture.PacketCaptureCommand;
+import org.apache.commons.lang3.StringUtils;
+import org.libvirt.Connect;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.properties.AgentProperties;
+import com.cloud.agent.properties.AgentPropertiesFileHandler;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.InterfaceDef;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+/**
+ * Starts, stops or queries the packet capture systemd unit
+ * (cloudstack-pcap@<interface>.service by default) for a NIC of a
+ * running VM. Before starting the unit, the NIC context is written to an
+ * environment file so the capture script can decide what and where to capture.
+ */
+@ResourceWrapper(handles = PacketCaptureCommand.class)
+public class LibvirtPacketCaptureCommandWrapper extends CommandWrapper<PacketCaptureCommand, Answer, LibvirtComputingResource> {
+
+ @Override
+ public Answer execute(PacketCaptureCommand command, LibvirtComputingResource resource) {
+ if (StringUtils.isBlank(command.getVmName()) || StringUtils.isBlank(command.getMacAddress())) {
+ return new PacketCaptureAnswer(command, false, "VM name and NIC MAC address are required", false);
+ }
+
+ InterfaceDef nicDevice = resolveNicDevice(command, resource);
+ String unit = getUnitName(nicDevice);
+
+ switch (command.getAction()) {
+ case START:
+ if (nicDevice == null) {
+ return new PacketCaptureAnswer(command, false, String.format(
+ "no interface with MAC address %s found on a running domain %s", command.getMacAddress(), command.getVmName()), false);
+ }
+ return start(command, nicDevice, unit);
+ case STOP:
+ if (nicDevice == null) {
+ // The VM is not running (anymore) on this host; the unit died with the tap device.
+ return new PacketCaptureAnswer(command, true, "no running capture found", false);
+ }
+ return stop(command, nicDevice, unit);
+ case STATUS:
+ boolean running = nicDevice != null && systemctl("is-active", "--quiet", unit) == null;
+ return new PacketCaptureAnswer(command, true, null, running);
+ default:
+ return new PacketCaptureAnswer(command, false, "unknown action " + command.getAction(), false);
+ }
+ }
+
+ private Answer start(PacketCaptureCommand command, InterfaceDef nicDevice, String unit) {
+ try {
+ writeEnvironmentFile(command, nicDevice);
+ } catch (IOException e) {
+ logger.error("Failed to write packet capture environment file for NIC {} of VM {}", nicDevice.getDevName(), command.getVmName(), e);
+ return new PacketCaptureAnswer(command, false, "failed to write environment file: " + e.getMessage(), false);
+ }
+ String result = systemctl("start", unit);
+ if (result != null) {
+ return new PacketCaptureAnswer(command, false, String.format("failed to start %s: %s", unit, result), false);
+ }
+ logger.info("Started packet capture unit {} for VM {}", unit, command.getVmName());
+ return new PacketCaptureAnswer(command, true, null, true);
+ }
+
+ private Answer stop(PacketCaptureCommand command, InterfaceDef nicDevice, String unit) {
+ String result = systemctl("stop", unit);
+ if (result != null) {
+ return new PacketCaptureAnswer(command, false, String.format("failed to stop %s: %s", unit, result), true);
+ }
+ try {
+ Files.deleteIfExists(getEnvironmentFile(nicDevice.getDevName()));
+ } catch (IOException e) {
+ logger.warn("Failed to delete packet capture environment file for {}", nicDevice.getDevName(), e);
+ }
+ logger.info("Stopped packet capture unit {} for VM {}", unit, command.getVmName());
+ return new PacketCaptureAnswer(command, true, null, false);
+ }
+
+ /**
+ * Finds the host-side interface of the NIC by matching the MAC address on
+ * the running domain. Returns null when the domain is not running on this
+ * host or has no interface with the MAC address.
+ */
+ private InterfaceDef resolveNicDevice(PacketCaptureCommand command, LibvirtComputingResource resource) {
+ try {
+ Connect conn = resource.getLibvirtUtilitiesHelper().getConnectionByVmName(command.getVmName());
+ for (InterfaceDef iface : resource.getInterfaces(conn, command.getVmName())) {
+ if (command.getMacAddress().equalsIgnoreCase(iface.getMacAddress())) {
+ return iface;
+ }
+ }
+ } catch (LibvirtException e) {
+ logger.debug("Unable to look up interfaces of VM {}: {}", command.getVmName(), e.getMessage());
+ }
+ return null;
+ }
+
+ private String getUnitName(InterfaceDef nicDevice) {
+ String service = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.PACKET_CAPTURE_SERVICE);
+ return String.format("%s@%s.service", service, nicDevice == null ? "" : nicDevice.getDevName());
+ }
+
+ private Path getEnvironmentFile(String deviceName) {
+ String envDir = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.PACKET_CAPTURE_ENV_DIR);
+ return Paths.get(envDir, String.format("pcap-%s.env", deviceName));
+ }
+
+ private void writeEnvironmentFile(PacketCaptureCommand command, InterfaceDef nicDevice) throws IOException {
+ List<String> lines = new ArrayList<>();
+ lines.add("CS_VM_NAME=" + StringUtils.defaultString(command.getVmName()));
+ lines.add("CS_VM_UUID=" + StringUtils.defaultString(command.getVmUuid()));
+ lines.add("CS_NIC_UUID=" + StringUtils.defaultString(command.getNicUuid()));
+ lines.add("CS_NIC_MAC=" + StringUtils.defaultString(command.getMacAddress()));
+ lines.add("CS_NIC_DEV=" + StringUtils.defaultString(nicDevice.getDevName()));
+ lines.add("CS_NIC_BRIDGE=" + StringUtils.defaultString(nicDevice.getBrName()));
+ lines.add("CS_NIC_IP4=" + StringUtils.defaultString(command.getIp4Address()));
+ lines.add("CS_NIC_IP6=" + StringUtils.defaultString(command.getIp6Address()));
+ lines.add("CS_NETWORK_UUID=" + StringUtils.defaultString(command.getNetworkUuid()));
+
+ Path file = getEnvironmentFile(nicDevice.getDevName());
+ Files.createDirectories(file.getParent());
+ Files.write(file, lines);
+ }
Review Comment:
The environment file written under packet.capture.env.dir is currently created with default permissions (often 0644). Since it can include VM/NIC identifiers and IPs, it should be restricted to root-readable (0600) on POSIX filesystems (best-effort).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KVM: packet capture of Instance NICs via operator-defined script [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #13767:
URL: https://github.com/apache/cloudstack/pull/13767#issuecomment-5156233409
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/13767?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 3.41%. Comparing base ([`4f11707`](https://app.codecov.io/gh/apache/cloudstack/commit/4f117071c9397b1e4714c8fb61c384883c872842?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`759bab3`](https://app.codecov.io/gh/apache/cloudstack/commit/759bab3936cc9e407b5e7b0c6c53bd4c00d796b7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (4f11707) and HEAD (759bab3). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (4f11707) | HEAD (759bab3) |
>|------|------|------|
>|unittests|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #13767 +/- ##
=============================================
- Coverage 19.65% 3.41% -16.24%
=============================================
Files 6368 487 -5881
Lines 574881 41867 -533014
Branches 70351 7912 -62439
=============================================
- Hits 112970 1429 -111541
+ Misses 449639 40238 -409401
+ Partials 12272 200 -12072
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/13767/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/13767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `3.41% <ø> (ø)` | |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/13767/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/cloudstack/pull/13767?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org