On Tue, 2026-01-27 at 10:45 +0530, Ani Sinha wrote:
> + } else {
> + QLIST_FOREACH(kp, &kernel_port_list, node) {
> + if (kp->port == port) {
> + QLIST_REMOVE(kp, node);
> + g_free(kp);
> + }
> + }
> + QLIST_FOREACH(ef, &eventfd_list, node) {
> + if (ef->port == port) {
> + QLIST_REMOVE(ef, node);
> + g_free(ef);
> + }
> + }
> }
> }
>
Do those not need to be QLIST_FOREACH_SAFE ?
> @@ -565,6 +655,8 @@ static int assign_kernel_port(uint16_t type,
> evtchn_port_t port,
> {
> CPUState *cpu = qemu_get_cpu(vcpu_id);
> struct kvm_xen_hvm_attr ha;
> + struct kernel_ports *kp = g_malloc0(sizeof(*kp));
> + int ret;
>
> if (!cpu) {
> return -ENOENT;
> @@ -578,12 +670,21 @@ static int assign_kernel_port(uint16_t type,
> evtchn_port_t port,
> ha.u.evtchn.deliver.port.vcpu = kvm_arch_vcpu_id(cpu);
> ha.u.evtchn.deliver.port.priority =
> KVM_IRQ_ROUTING_XEN_EVTCHN_PRIO_2LEVEL;
>
> - return kvm_vm_ioctl(kvm_state, KVM_XEN_HVM_SET_ATTR, &ha);
> + ret = kvm_vm_ioctl(kvm_state, KVM_XEN_HVM_SET_ATTR, &ha);
> + if (ret == 0) {
> + kp->type = type;
> + kp->port = port;
> + kp->vcpu_id = vcpu_id;
> + QLIST_INSERT_HEAD(&kernel_port_list, kp, node);
> + }
> + return ret;
> }
>
I think 'kp' leaks in the error case there? And the next one in
assign_kernel_eventfd().
I don't much like duplicating the code which performs the ioctls.
Couldn't you have a helper function which does that when given a
'struct kernel_ports', then invoke *that* from both places?
Or even better, make use of the fact that we *already* have the ability
to replay these for serialisation/deserialisation?
But backing up a little more... if this is for rebooting guests,
shouldn't we be wiping the event channel setup completely, like a soft
reset? Do we preserve to do *any* of this? Are you sure you don't see a
soft reset wiping it all away, either before or after this code runs?
smime.p7s
Description: S/MIME cryptographic signature
