On Tue, May 04, 2021 at 08:29:50AM +0200, Pankaj Gupta wrote: > +CC Danpb > > > >>> Marc-André, I'd like your opinion for this one, in particular the use of > > >>> g_source_remove(). > > >>> > > >> > > >> My opinion isn't really worth much, my review would have a bit more > > >> value. > > >> > > >> GSource has indeed some peculiar lifetime management, that I got wrong in > > >> the past. So I would be extra careful. > > >> > > >> But before spending time on review, I would also clarify the motivation > > >> and ask for testing. > > >> > > >> Markus, hot-adding/removing monitors isn't supported? > > >> > > >> > > > I realize you answered my question below. That's surprising me. Wouldn't > > > it > > > make more sense to support it rather than having a pre-opened null-based > > > monitor that can have its chardev swapped? > > > > Yes, it would. Patches welcome. > > > > This patch is a somewhat ham-fisted and limited solution to the problem > > stated in the commit message. However, it might *also* be a reasonable > > improvement to chardev-change on its own. Not for me to judge. > > > > chardev-change comes with a number of restrictions. Let's have a closer > > look. It fails > > > > 1. when no such character device exists (d'oh) > > > > 2. for chardev-mux devices > > > > 3. in record/replay mode > > > > 4. when a backend is connected that doesn't implement the chr_be_change() > > method > > > > 5. when chr_be_change() fails > > > > 6. when creating the new chardev fails[*] > > > > Items 2, 3, 4 are restrictions. I figure 2 and 4 are simply not > > implemented, yet. I'm not sure about 3. > > > > Whether we want to accept patches lifting restrictions is up to the > > chardev maintainers. > > Maybe we can handle or already handle the restrictions at libvirt side?
>From the libvirt side I'd just like to see the real problem addressed. ie actally implement monitor hotplug/unplug, instead of this hack where you pre-create a monitor with null backend. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
