On Thu, Feb 3, 2022 at 4:19 AM Kevin Wolf <kw...@redhat.com> wrote: > > Am 02.02.2022 um 20:08 hat John Snow geschrieben: > > > I guess the relevant question in the context of this patch is whether > > > sync.py will need a similar two-phase setup as legacy.py. Do you think > > > you can do without it when you have to reintroduce this behaviour here > > > to fix bugs? > > > > > > > Hm, honestly, no. I think you'll still need the two-phase in the sync > > version no matter what -- if you expect to be able to launch QMP and > > QEMU from the same process, anyway. I suppose it's just a matter of > > what you *call* it and what/where the arguments are. > > > > I could just call it bind(), and it does whatever it needs to (Either > > creating a socket or, in 3.7+, instantiating more of the asyncio loop > > machinery). > > The signature for accept() would need to change to (perhaps) > > optionally accepting no arguments; i.e. you can do either of the > > following: > > > > (1) qmp.bind('/tmp/sock') > > qmp.accept() > > > > (2) qmp.accept('/tmp/sock') > > > > The risk is that the interface is just a touch more complex, the docs > > get a bit more cluttered, there's a slight loss of clarity, the > > accept() method would need to check to make sure you didn't give it an > > address argument if you've already given it one, etc. Necessary > > trade-off for the flexibility, though. > > Hm, how about copying the create_server() interface instead? So it > would become: > > (1) qmp.create_server('/tmp/sock', start_serving=False) > qmp.start_serving() > > (2) qmp.create_server('/tmp/sock') > > Then you get the connection details only in a single place. (The names > should probably be changed because we're still a QMP client even though > we're technically the server on the socket level, but you get the idea.)
Good idea. I'll experiment. (I'm worried that the way I initially factored the code to begin with might make this ugly, but maybe I'm just dreading nothing. I'll have to see.) Thanks for taking a look! --js