* Daniel P. Berrangé (berra...@redhat.com) wrote: > On Mon, Apr 15, 2019 at 01:33:21PM +0300, Yury Kotov wrote: > > 15.04.2019, 13:25, "Daniel P. Berrangé" <berra...@redhat.com>: > > > On Mon, Apr 15, 2019 at 01:17:06PM +0300, Yury Kotov wrote: > > >> 15.04.2019, 13:11, "Daniel P. Berrangé" <berra...@redhat.com>: > > >> > On Mon, Apr 15, 2019 at 12:50:08PM +0300, Yury Kotov wrote: > > >> >> Hi, > > >> >> > > >> >> Just to clarify. I see two possible solutions: > > >> >> > > >> >> 1) Since the migration code doesn't receive fd, it isn't > > >> responsible for > > >> >> closing it. So, it may be better to use migrate_fd_param for both > > >> >> incoming/outgoing and add dupping for migrate_fd_param. Thus, > > >> clients must > > >> >> close the fd themselves. But existing clients will have a leak. > > >> > > > >> > We can't break existing clients in this way as they are correctly > > >> > using the monitor with its current semantics. > > >> > > > >> >> 2) If we don't duplicate fd, then at least we should remove fd from > > >> >> the corresponding list. Therefore, the solution is to fix > > >> qemu_close to find > > >> >> the list and remove fd from it. But qemu_close is currently > > >> consistent with > > >> >> qemu_open (which opens/dups fd), so adding additional logic might > > >> not be > > >> >> a very good idea. > > >> > > > >> > qemu_close is not appropriate place to deal with something speciifc > > >> > to the montor. > > >> > > > >> >> I don't see any other solution, but I might miss something. > > >> >> What do you think? > > >> > > > >> > All callers of monitor_get_fd() will close() the FD they get back. > > >> > Thus monitor_get_fd() should remove it from the list when it returns > > >> > it, and we should add API docs to monitor_get_fd() to explain this. > > >> > > > >> Ok, it sounds reasonable. But monitor_get_fd is only about outgoing > > >> migration. > > >> But what about the incoming migration? It doesn't use monitor_get_fd > > >> but just > > >> converts input string to int and use it as fd. > > > > > > The incoming migration expects the FD to be passed into QEMU by the mgmt > > > app when it is exec'ing the QEMU binary. It doesn't interact with the > > > monitor at all AFAIR. > > > > > > > Oh, sorry. This use case is not obvious. We used add-fd to pass fd for > > migrate-incoming and such way has described problems. > > That's a bug in your usage of QEMU IMHO, as the incoming code is not > designed to use add-fd.
Hmm, that's true - although: a) It's very non-obvious b) Unfortunate, since it would go well with -incoming defer Dave > 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 :| -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK