15.04.2019, 13:17, "Yury Kotov" <yury-ko...@yandex-team.ru>: > 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. >>
But qemu_close is already deal with monitor: It uses monitor_fdset_dup_fd_find & monitor_fdset_dup_fd_remove to find and remove fd from monitor's dup_fds list. >>> 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. > Regards, Yury
