Messages in this thread | | | Date | Tue, 22 Nov 2022 14:45:44 +0100 | Subject | Re: [PATCH v2] virtio_console: Introduce an ID allocator for virtual console numbers | From | Cédric Le Goater <> |
| |
On 11/22/22 12:12, Thomas Huth wrote: > On 14/11/2022 18.38, Cédric Le Goater wrote: >> When a virtio console port is initialized, it is registered as an hvc >> console using a virtual console number. If a KVM guest is started with >> multiple virtio console devices, the same vtermno (or virtual console >> number) can be used to allocate different hvc consoles, which leads to >> various communication problems later on. >> >> This is also reported in debugfs : >> >> # grep vtermno /sys/kernel/debug/virtio-ports/* >> /sys/kernel/debug/virtio-ports/vport1p1:console_vtermno: 1 >> /sys/kernel/debug/virtio-ports/vport2p1:console_vtermno: 1 >> /sys/kernel/debug/virtio-ports/vport3p1:console_vtermno: 2 >> /sys/kernel/debug/virtio-ports/vport4p1:console_vtermno: 3 >> >> Replace the next_vtermno global with an ID allocator and start the >> allocation at 1 as it is today. Also recycle IDs when a console port >> is removed. > > Sounds like a good idea! > >> @@ -1244,8 +1236,11 @@ static int init_port_console(struct port *port) >> * pointers. The final argument is the output buffer size: we >> * can do any size, so we put PAGE_SIZE here. >> */ >> - port->cons.vtermno = pdrvdata.next_vtermno; >> + ret = ida_alloc_range(&vtermno_ida, 1, ~0, GFP_KERNEL); > > Just cosmetics: I think you could use ida_alloc_min() instead. > >> + if (ret < 0) >> + return ret; >> + port->cons.vtermno = ret; >> port->cons.hvc = hvc_alloc(port->cons.vtermno, 0, &hv_ops, PAGE_SIZE); >> if (IS_ERR(port->cons.hvc)) { >> ret = PTR_ERR(port->cons.hvc); > > What if this if (IS_ERR()) error happens? The code seems to return early in this case - shouldn't the ID be freed again in this case?
Oh yes. I forgot that.
Thanks,
C.
| |