| Message ID | 20200611111703.159590-1-dgilbert@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | monitor openfd commands | expand |
On 6/11/20 6:17 AM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The monitors currently have a 'getfd' command that lets you pass an fd > via the monitor socket. 'openfd' is a new command that opens a file > and puts the fd in the same fd pool. The file is opened RW and created > if it doesn't exist. > It makes it easy to test migration to and from a file. We have two fd-passing mechanisms: getfd and add-fd. add-fd is newer, and allows things like /dev/fdset/NNN to work anywhere a filename works. I'm guessing that the issue here is that migration hasn't been tweaked to work nicely with the newer add-fd, but instead insists on the older getfd interface (where you have to use getfd to associate an fd with a name, then tell migration to use that special name, but the special name is via a different parameter than the normal filename parameter). At which point openfd looks like it is just sugar to make getfd easier to use. Would it instead be worth modifying migration to work with add-fd? Or does add-fd need the same sort of sugar? > > Dr. David Alan Gilbert (2): > qmp: Add 'openfd' command > hmp: Add 'openfd' command > > hmp-commands.hx | 16 +++++++++++++- > include/monitor/hmp.h | 1 + > monitor/hmp-cmds.c | 10 +++++++++ > monitor/misc.c | 48 +++++++++++++++++++++++++++++++++--------- > qapi/misc.json | 23 +++++++++++++++++++- > tests/qtest/test-hmp.c | 2 ++ > 6 files changed, 88 insertions(+), 12 deletions(-) >
* Eric Blake (eblake@redhat.com) wrote: > On 6/11/20 6:17 AM, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > The monitors currently have a 'getfd' command that lets you pass an fd > > via the monitor socket. 'openfd' is a new command that opens a file > > and puts the fd in the same fd pool. The file is opened RW and created > > if it doesn't exist. > > It makes it easy to test migration to and from a file. > > We have two fd-passing mechanisms: getfd and add-fd. add-fd is newer, and > allows things like /dev/fdset/NNN to work anywhere a filename works. Ewww I do dislike fake paths, they tend to be the source of great security bugs. > I'm > guessing that the issue here is that migration hasn't been tweaked to work > nicely with the newer add-fd, but instead insists on the older getfd > interface (where you have to use getfd to associate an fd with a name, then > tell migration to use that special name, but the special name is via a > different parameter than the normal filename parameter). At which point > openfd looks like it is just sugar to make getfd easier to use. Yep, openfd is just intended to be sugar; it's a pain to use getfd at runtime because you have to play all the passing fd's over socket magic. My main reason here is because I wanted an easy way to migrate to /dev/null for performance testing, but it would make life easier when migrating to/from a file. > Would it instead be worth modifying migration to work with add-fd? Probably. At the moment what we have is an 'fd:string' syntax for both inbound and outbound migration. The outbound migration looks up the string in the getfd index and uses it. (via monitor_get_fd) The inbound migration checks if the string starts with a number, if it is then it uses it raw as the unix fd; else it passes it to getfd index. (via monitor_fd_param). (getfd disallows names that are numeric) I can see a few solutions here: a) Teach qemu a new fdset:number syntax - it's a bit of a pain but is discoverable. b) Modify the getfd string lookup to parse /dev/fdset and go use the fdset The problem with (b) is that the getfd mechanism doesn't have a concept of open mode, and only has a single fd bound to a name, so none of the existing parsing code would know which entry to use in an fdset. (I'm assuming here no one created a getfd entry named /dev/fdset/0 - although it seems legal). The problem with (a) is that adding a new syntax is a bit more code; but I guess it's probably more discoverable. ANy preferences? > add-fd need the same sort of sugar? Yep, I'd like to have a way to open a file other than via scmrights. I'd also need to add an HMP equivalent. Dave > > > > > Dr. David Alan Gilbert (2): > > qmp: Add 'openfd' command > > hmp: Add 'openfd' command > > > > hmp-commands.hx | 16 +++++++++++++- > > include/monitor/hmp.h | 1 + > > monitor/hmp-cmds.c | 10 +++++++++ > > monitor/misc.c | 48 +++++++++++++++++++++++++++++++++--------- > > qapi/misc.json | 23 +++++++++++++++++++- > > tests/qtest/test-hmp.c | 2 ++ > > 6 files changed, 88 insertions(+), 12 deletions(-) > > > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> The monitors currently have a 'getfd' command that lets you pass an fd via the monitor socket. 'openfd' is a new command that opens a file and puts the fd in the same fd pool. The file is opened RW and created if it doesn't exist. It makes it easy to test migration to and from a file. Dr. David Alan Gilbert (2): qmp: Add 'openfd' command hmp: Add 'openfd' command hmp-commands.hx | 16 +++++++++++++- include/monitor/hmp.h | 1 + monitor/hmp-cmds.c | 10 +++++++++ monitor/misc.c | 48 +++++++++++++++++++++++++++++++++--------- qapi/misc.json | 23 +++++++++++++++++++- tests/qtest/test-hmp.c | 2 ++ 6 files changed, 88 insertions(+), 12 deletions(-)