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/[email protected]:
##########
@@ -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@&lt;interface&gt;.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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to