Hi Daniel,
Thank you for your review and valuable feedback.
You're absolutely right about the concerns. Let me clarify the scenario
this patch addresses:
The remove_fd_in_watch() function handles the client-side connection case.
However, when the chardev is configured in server mode
(e.g., -qmp unix:/var/lib/libvirt/qemu/qmp-xxx/qmp.monitor,server=on,wait=off),
there's listener that needs cleanup. The socket_chr_listener_cleanup()
is specifically intended to handle this server-side listener to prevent the
race condition between the main thread and IOThread monitoring the same
listener fd.
I apologize for the unsafe assumption that the chardev would always be a
SocketChardev.
You're correct that this could cause crashes with other chardev types.
To fix this properly, I’m considering a more general design.
Would the following approach be acceptable?
1.Add a chr_listener_cleanup callback to the ChardevClass structure
2.Implement this callback in SocketChardev
3.Register it in char_socket_class_init()
4.In monitor/qmp.c, call it through the class method
remove_fd_in_watch(chr);
ChardevClass *cc = CHARDEV_GET_CLASS(chr);
if (cc->chr_listener_cleanup) {
cc->chr_listener_cleanup(chr);
}
This would maintain type safety while keeping the fix properly abstracted
at the chardev layer. Would this fix make sense?
Looking forward to your guidance.
Best regards,
Jie Song
