[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol
From: |
Yury Kotov |
Subject: |
Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol |
Date: |
Mon, 15 Apr 2019 15:20:01 +0300 |
15.04.2019, 14:30, "Dr. David Alan Gilbert" <address@hidden>:
> * Daniel P. Berrangé (address@hidden) wrote:
>> On Mon, Apr 15, 2019 at 12:15:12PM +0100, Dr. David Alan Gilbert wrote:
>> > * Daniel P. Berrangé (address@hidden) wrote:
>> > > On Mon, Apr 15, 2019 at 01:33:21PM +0300, Yury Kotov wrote:
>> > > > 15.04.2019, 13:25, "Daniel P. Berrangé" <address@hidden>:
>> > > > > On Mon, Apr 15, 2019 at 01:17:06PM +0300, Yury Kotov wrote:
>> > > > >> 15.04.2019, 13:11, "Daniel P. Berrangé" <address@hidden>:
>> > > > >> > 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
>>
>> Yeah I think this is a screw up on QMEU's part when introducing 'defer'.
>>
>> We should have mandated use of 'add-fd' when using 'defer', since FD
>> inheritance-over-execve() should only be used for command line args,
>> not monitor commands.
>>
>> Not sure how to best fix this is QEMU though without breaking back
>> compat for apps using 'defer' already.
>
> We could add mon-fd: transports that has the same behaviour as now for
> outgoing, and for incoming uses the add-fd stash.
>
May be it's better to use monitor_fd_param for both incoming/outgoing?
So, "migrate" will know fd:<int> semantics and "migrate-incoming" will
know fd:<fd_name> semantics. And also modify monitor_get_fd to
remove fd from list before return.
This is a backwards compatible change.
Regards,
Yury
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, (continued)
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Yury Kotov, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Daniel P . Berrangé, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Yury Kotov, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Yury Kotov, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Daniel P . Berrangé, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Yury Kotov, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Daniel P . Berrangé, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Dr. David Alan Gilbert, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Daniel P . Berrangé, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Dr. David Alan Gilbert, 2019/04/15
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol,
Yury Kotov <=
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Yury Kotov, 2019/04/16
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Yury Kotov, 2019/04/16
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Dr. David Alan Gilbert, 2019/04/18
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Yury Kotov, 2019/04/18
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Dr. David Alan Gilbert, 2019/04/18
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Yury Kotov, 2019/04/18
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Dr. David Alan Gilbert, 2019/04/18
- Re: [Qemu-devel] [PATCH] migration: Fix handling fd protocol, Yury Kotov, 2019/04/18