| Message ID | 20190412122028.7067-1-yury-kotov@yandex-team.ru (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add 'inline-fd:' protocol for migration | expand |
On 4/12/19 7:20 AM, Yury Kotov wrote: > Hi, > > I've added 'inline-fd:' proto to simplify migration to/from fd. > I thought about modifying the existing 'fd:' proto but I'm not sure it's a > good idea to change it's contract. > > Existing 'fd:' proto works with previously added fd by getfd or add-fd commands. > If client doesn't want to work with this fd before or after migration then it's > easier to send an fd with the migrate-* command. Also, client shouldn't maintain > this fd. While the sentiment of making it easier by having fewer QMP commands might be worthwhile, I'm worried about whether this scales. Having just a limited set of commands that take an fd over SCM rights, and every other command wired to automagically work with existing named fds, scales a lot easier than having to teach individual commands how to take an fd inline. At the very least, if we DO decide we like this, then I'd at least hope that you made this addition introspectible (how can management learn whether the 'inline-fd:' protocol is supported, and even more importantly, which commands require an inline fd via SCM rights to be a valid command based, especially if accepting or rejecting an inline fd is conditional on other arguments to the command). > > Usage: > { 'execute': 'migrate', 'arguments': { 'uri': 'inline-fd:' } } > { 'execute': 'migrate-incoming', 'arguments': { 'uri': 'inline-fd:' } } >
On Fri, Apr 12, 2019 at 12:13:51PM -0500, Eric Blake wrote: > On 4/12/19 7:20 AM, Yury Kotov wrote: > > Hi, > > > > I've added 'inline-fd:' proto to simplify migration to/from fd. > > I thought about modifying the existing 'fd:' proto but I'm not sure it's a > > good idea to change it's contract. > > > > Existing 'fd:' proto works with previously added fd by getfd or add-fd commands. > > If client doesn't want to work with this fd before or after migration then it's > > easier to send an fd with the migrate-* command. Also, client shouldn't maintain > > this fd. > > While the sentiment of making it easier by having fewer QMP commands > might be worthwhile, I'm worried about whether this scales. Having just > a limited set of commands that take an fd over SCM rights, and every > other command wired to automagically work with existing named fds, > scales a lot easier than having to teach individual commands how to take > an fd inline. Yeah I tend to agree - we use FD passing extensively across QMP/HMP and all these commands are written to interact with "getfd". I think it would be a step backwards to introduce a special case with migration that doesn't use "getfd". Regards, Daniel
* Daniel P. Berrangé (berrange@redhat.com) wrote: > On Fri, Apr 12, 2019 at 12:13:51PM -0500, Eric Blake wrote: > > On 4/12/19 7:20 AM, Yury Kotov wrote: > > > Hi, > > > > > > I've added 'inline-fd:' proto to simplify migration to/from fd. > > > I thought about modifying the existing 'fd:' proto but I'm not sure it's a > > > good idea to change it's contract. > > > > > > Existing 'fd:' proto works with previously added fd by getfd or add-fd commands. > > > If client doesn't want to work with this fd before or after migration then it's > > > easier to send an fd with the migrate-* command. Also, client shouldn't maintain > > > this fd. > > > > While the sentiment of making it easier by having fewer QMP commands > > might be worthwhile, I'm worried about whether this scales. Having just > > a limited set of commands that take an fd over SCM rights, and every > > other command wired to automagically work with existing named fds, > > scales a lot easier than having to teach individual commands how to take > > an fd inline. > > Yeah I tend to agree - we use FD passing extensively across QMP/HMP > and all these commands are written to interact with "getfd". I think > it would be a step backwards to introduce a special case with > migration that doesn't use "getfd". Yes, agreed - as you spotted passing fd's gets a bit hairy, and hence it's best to keep it to a few places. Lets figure out the right way of fixing the double-close you spotted rather than adding new schemes. 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 / dgilbert@redhat.com / Manchester, UK
Hi, I've added 'inline-fd:' proto to simplify migration to/from fd. I thought about modifying the existing 'fd:' proto but I'm not sure it's a good idea to change it's contract. Existing 'fd:' proto works with previously added fd by getfd or add-fd commands. If client doesn't want to work with this fd before or after migration then it's easier to send an fd with the migrate-* command. Also, client shouldn't maintain this fd. Usage: { 'execute': 'migrate', 'arguments': { 'uri': 'inline-fd:' } } { 'execute': 'migrate-incoming', 'arguments': { 'uri': 'inline-fd:' } } Regards, Yury Yury Kotov (3): monitor: Add monitor_recv_fd function to work with sent fds migration: Add inline-fd protocol migration-test: Add a test for inline_fd protocol include/monitor/monitor.h | 1 + migration/Makefile.objs | 2 +- migration/inline-fd.c | 89 ++++++++++++++++++++++ migration/inline-fd.h | 22 ++++++ migration/migration.c | 15 ++++ migration/trace-events | 4 + monitor.c | 15 +++- tests/libqtest.c | 83 +++++++++++++++++++- tests/libqtest.h | 51 ++++++++++++- tests/migration-test.c | 117 ++++++++++++++++++++++++++--- tests/qemu-seccomp | Bin 0 -> 1266168 bytes tests/test-query-device-blockstats | Bin 0 -> 1272608 bytes 12 files changed, 379 insertions(+), 20 deletions(-) create mode 100644 migration/inline-fd.c create mode 100644 migration/inline-fd.h create mode 100755 tests/qemu-seccomp create mode 100755 tests/test-query-device-blockstats