| Message ID | 20231019192353.31500-3-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | migration: Modify 'migrate' and 'migrate-incoming' QAPI commands for migration | expand |
Fabiano Rosas <farosas@suse.de> writes: > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > qapi/migration.json | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/qapi/migration.json b/qapi/migration.json > index c352c7ac52..602cb706e3 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -1519,10 +1519,25 @@ > # > # @rdma: Migrate via RDMA. > # > +# @file: Direct the migration stream to a file. > +# > # Since 8.2 > ## > { 'enum': 'MigrationAddressType', > - 'data': ['socket', 'exec', 'rdma'] } > + 'data': ['socket', 'exec', 'rdma', 'file'] } I don't like our use of spaces around parenthesis in the QAPI schema, but I like inconsistency even less: please insert a space after '['. > + > +## > +# @FileMigrationArgs: > +# > +# @path: file path Name this @filename for local consistency. We use both @filename and @path for filenames in the schema, which is sad. However, migration.json uses only @filename so far. Let's keep it that way. "file path" is awfully terse. Maybe "file to receive the migration stream"? > +# > +# @offset: initial offset for the file Again, too terse. How is @offset to be used? Start writing at this file offset? > +# > +# Since 8.2 > +## > +{ 'struct': 'FileMigrationArgs', > + 'data': {'path': 'str', Please insert a space after '{', and reindent the next line. > + 'offset': 'uint64' } } > > ## > # @MigrationExecCommand: > @@ -1547,7 +1562,8 @@ > 'data': { > 'socket': 'SocketAddress', > 'exec': 'MigrationExecCommand', > - 'rdma': 'InetSocketAddress' } } > + 'rdma': 'InetSocketAddress', > + 'file': 'FileMigrationArgs' } } > > ## > # @migrate:
Markus Armbruster <armbru@redhat.com> writes: > Fabiano Rosas <farosas@suse.de> writes: > >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> qapi/migration.json | 20 ++++++++++++++++++-- >> 1 file changed, 18 insertions(+), 2 deletions(-) >> >> diff --git a/qapi/migration.json b/qapi/migration.json >> index c352c7ac52..602cb706e3 100644 >> --- a/qapi/migration.json >> +++ b/qapi/migration.json >> @@ -1519,10 +1519,25 @@ >> # >> # @rdma: Migrate via RDMA. >> # >> +# @file: Direct the migration stream to a file. >> +# >> # Since 8.2 >> ## >> { 'enum': 'MigrationAddressType', >> - 'data': ['socket', 'exec', 'rdma'] } >> + 'data': ['socket', 'exec', 'rdma', 'file'] } > > I don't like our use of spaces around parenthesis in the QAPI schema, > but I like inconsistency even less: please insert a space after '['. > Yes. But, a contributor today has to guess what is the preferred syntax. Could we have a checkpatch rule for this? Or should I send a patch to make the whole file consistent at once? Side question: are we using valid JSON at all? I threw this in a random online linter and it complains about the single quotes. We could have a proper tool doing the validation in CI.
Fabiano Rosas <farosas@suse.de> writes: > Markus Armbruster <armbru@redhat.com> writes: > >> Fabiano Rosas <farosas@suse.de> writes: >> >>> Signed-off-by: Fabiano Rosas <farosas@suse.de> >>> --- >>> qapi/migration.json | 20 ++++++++++++++++++-- >>> 1 file changed, 18 insertions(+), 2 deletions(-) >>> >>> diff --git a/qapi/migration.json b/qapi/migration.json >>> index c352c7ac52..602cb706e3 100644 >>> --- a/qapi/migration.json >>> +++ b/qapi/migration.json >>> @@ -1519,10 +1519,25 @@ >>> # >>> # @rdma: Migrate via RDMA. >>> # >>> +# @file: Direct the migration stream to a file. >>> +# >>> # Since 8.2 >>> ## >>> { 'enum': 'MigrationAddressType', >>> - 'data': ['socket', 'exec', 'rdma'] } >>> + 'data': ['socket', 'exec', 'rdma', 'file'] } >> >> I don't like our use of spaces around parenthesis in the QAPI schema, >> but I like inconsistency even less: please insert a space after '['. >> > > Yes. But, > > a contributor today has to guess what is the preferred syntax. Could we > have a checkpatch rule for this? Or should I send a patch to make the > whole file consistent at once? > > Side question: are we using valid JSON at all? I threw this in a random > online linter and it complains about the single quotes. We could have a > proper tool doing the validation in CI. You've come a sad, sad place. docs/devel/qapi-code-gen.rst: Schema syntax ------------- Syntax is loosely based on `JSON <http://www.ietf.org/rfc/rfc8259.txt>`_. Differences: * Comments: start with a hash character (``#``) that is not part of a string, and extend to the end of the line. * Strings are enclosed in ``'single quotes'``, not ``"double quotes"``. * Strings are restricted to printable ASCII, and escape sequences to just ``\\``. * Numbers and ``null`` are not supported. If your reaction to item 2 is "this is stupid", you'd be exactly right. Here's the conclusion of a discussion on possible improvements we had in 2020: https://lore.kernel.org/qemu-devel/877dt5ofoi.fsf@dusky.pond.sub.org/
On Fri, Oct 20, 2023 at 02:37:16PM +0200, Markus Armbruster wrote: > Fabiano Rosas <farosas@suse.de> writes: > > > Markus Armbruster <armbru@redhat.com> writes: > > > >> Fabiano Rosas <farosas@suse.de> writes: > >> > >>> Signed-off-by: Fabiano Rosas <farosas@suse.de> > > > > Side question: are we using valid JSON at all? I threw this in a random > > online linter and it complains about the single quotes. We could have a > > proper tool doing the validation in CI. > > You've come a sad, sad place. > > docs/devel/qapi-code-gen.rst: > > Schema syntax > ------------- > > Syntax is loosely based on `JSON <http://www.ietf.org/rfc/rfc8259.txt>`_. > Differences: > > * Comments: start with a hash character (``#``) that is not part of a > string, and extend to the end of the line. > > * Strings are enclosed in ``'single quotes'``, not ``"double quotes"``. > > * Strings are restricted to printable ASCII, and escape sequences to > just ``\\``. > > * Numbers and ``null`` are not supported. > > If your reaction to item 2 is "this is stupid", you'd be exactly right. > > Here's the conclusion of a discussion on possible improvements we had in > 2020: > https://lore.kernel.org/qemu-devel/877dt5ofoi.fsf@dusky.pond.sub.org/ Looking at those options again I so strongly want to be able to say "none of the above". We have a need to describe data structures, and generate code for serializing/deserializing them on the wire. We created a language for this and wrote our own C code generator, our own docs generator, own our json parser & formatter, and now are also writing our own Go code generator (Python, Rust too ?). IMHO this is way too much NiH for my likely, and creates a maint burden for ourselves that we could do without. <open-can-of-worms> At the point in time we invented QAPI this was perhaps justifiable, though even back then I thought we should have done a binary format and used XDR to describe it, as a standard pre-existing language and toolset. Today I wouldn't suggest XDR, but would be inclined to suggest Protobuf. This already has code & docs generators for every programming language we would ever need. Protobuf has a defined serialization format too though which is binary based IIUC, so distinct from our JSON wire format. The interesting question though is whether it would be feasible to transition to this today, despite our historical baggage ? With recent interest in accessing QAPI for many more languages, it is timely to consider if we can adopt a standardized toolset to reduce this burden of talking to QEMU from other languages. A transition would need several big ticket items * A QAPI visitor impl that can spit out the protobuf document. ie to translate all our existing QAPI formats into protobuf, as doing this manually would be madness. This is probably the easy bit. * A custom protobuf visitor impl that can spit out C code that has the same API as our QAPI C generator. ie so we can avoid a big bang to convert all of QEMU internals. I suspect this is quite alot of work. * A custom protobuf serializer compatible with our current JSON format. ie so that QEMU can continue to expose the current QMP protocol to apps for legacy compat for some period of time, while also exposing a new native binary protobuf protocol for future usage. Also probably quite alot of work. That's all certainly quite alot of work and probably things I have forgotten. Above all it does assume it is possible to define a loss-less mapping from QAPI -> Protobuf language. I've not proved this is possible, but my inclination is that it probably should be, and if not, we could likely enable it by strategically deprecated and then deleting troublesome bits prior to a conversion. </open-can-of-worms> With regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Fri, Oct 20, 2023 at 02:37:16PM +0200, Markus Armbruster wrote: >> Fabiano Rosas <farosas@suse.de> writes: >> >> > Markus Armbruster <armbru@redhat.com> writes: >> > >> >> Fabiano Rosas <farosas@suse.de> writes: >> >> >> >>> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> > >> > Side question: are we using valid JSON at all? I threw this in a random >> > online linter and it complains about the single quotes. We could have a >> > proper tool doing the validation in CI. >> >> You've come a sad, sad place. >> >> docs/devel/qapi-code-gen.rst: >> >> Schema syntax >> ------------- >> >> Syntax is loosely based on `JSON <http://www.ietf.org/rfc/rfc8259.txt>`_. >> Differences: >> >> * Comments: start with a hash character (``#``) that is not part of a >> string, and extend to the end of the line. >> >> * Strings are enclosed in ``'single quotes'``, not ``"double quotes"``. >> >> * Strings are restricted to printable ASCII, and escape sequences to >> just ``\\``. >> >> * Numbers and ``null`` are not supported. >> >> If your reaction to item 2 is "this is stupid", you'd be exactly right. >> >> Here's the conclusion of a discussion on possible improvements we had in >> 2020: >> https://lore.kernel.org/qemu-devel/877dt5ofoi.fsf@dusky.pond.sub.org/ > > Looking at those options again I so strongly want to be able to > say "none of the above". > > We have a need to describe data structures, and generate code for > serializing/deserializing them on the wire. We created a language > for this and wrote our own C code generator, our own docs generator, > own our json parser & formatter, and now are also writing our own > Go code generator (Python, Rust too ?). > > IMHO this is way too much NiH for my likely, and creates a maint > burden for ourselves that we could do without. > > <open-can-of-worms> > At the point in time we invented QAPI this was perhaps justifiable, > though even back then I thought we should have done a binary format > and used XDR to describe it, as a standard pre-existing language and > toolset. Path dependence... I wasn't involved in the decisions that led to QAPI, nor its initial design and implementation. Its design was substantially constrained by QMP, which predates QAPI by almost two years. I was involved in QMP's design for a bit. We argued back and forth, and I eventually stepped aside to let the guys doing the actual work make the decisions. QAPI was further constrained by the desire to use it with QOM. The two are joined at the hip by visitors. Nevertheless, QOM and QAPI are integrated poorly. My general attitude towards generating source code is "don't; use a more powerful programming language". Instead of creating yet another pillar supporting Greenspun's Tenth Rule, I feel we should have embedded a sufficiently powerful programming language, and used it for the control plane. My choice would've been Lisp. The maintenance burden is modest, but real. The QAPI generator has gotten ~1.6 patches per week for the last five years, trending down. In the last year, it's been 68 patches, 437 insertions, 338 deletions total, much of it in docs/devel/qapi-code-gen.rst. Meanwhile, the entire project has had 130 times as many patches, 620 times as many insertions, and 400 times as many deletions. QAPI infrastructure maintenance is dwarved several times over by QAPI schema maintenance. Chiefly patch review. To be fair, adding language bindings will take a non-trivial one time investment plus ongoing maintenance for each language. > Today I wouldn't suggest XDR, but would be inclined to suggest > Protobuf. This already has code & docs generators for every > programming language we would ever need. > > Protobuf has a defined serialization format too though which is > binary based IIUC, so distinct from our JSON wire format. > > The interesting question though is whether it would be feasible to > transition to this today, despite our historical baggage ? With > recent interest in accessing QAPI for many more languages, it Go, Rust, and what else? > is timely to consider if we can adopt a standardized toolset to > reduce this burden of talking to QEMU from other languages. > > A transition would need several big ticket items > > * A QAPI visitor impl that can spit out the protobuf document. > ie to translate all our existing QAPI formats into protobuf, > as doing this manually would be madness. This is probably > the easy bit. This is about machine-assisted translation of the QAPI schema to protobuf, isn't it? > * A custom protobuf visitor impl that can spit out C code > that has the same API as our QAPI C generator. ie so we can > avoid a big bang to convert all of QEMU internals. I suspect > this is quite alot of work. The alternative is a big bang to convert, which could be less work or more. > * A custom protobuf serializer compatible with our current > JSON format. ie so that QEMU can continue to expose the > current QMP protocol to apps for legacy compat for some > period of time, while also exposing a new native binary > protobuf protocol for future usage. Also probably quite > alot of work. No alternative. > That's all certainly quite alot of work and probably things I have > forgotten. QOM's use of QAPI might add problems. > Above all it does assume it is possible to define a > loss-less mapping from QAPI -> Protobuf language. I've not proved > this is possible, but my inclination is that it probably should be, > and if not, we could likely enable it by strategically deprecated > and then deleting troublesome bits prior to a conversion. > </open-can-of-worms> While I share your distaste for the massive NIH QAPI has become, I'm not sure replacing it now is economical. It requires a massive one-time investment, offset by saving us one-time investments into QAPI bindings for other languages of interest. Whether it can actually reduce ongoing maintenance appreciably after the replacement is unclear. Pretty much the same for QOM.
On Thu, Oct 19, 2023 at 04:23:41PM -0300, Fabiano Rosas wrote: > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > qapi/migration.json | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> from a functional POV, and assuming Markus' feedback will be included. > > diff --git a/qapi/migration.json b/qapi/migration.json > index c352c7ac52..602cb706e3 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -1519,10 +1519,25 @@ > # > # @rdma: Migrate via RDMA. > # > +# @file: Direct the migration stream to a file. > +# > # Since 8.2 > ## > { 'enum': 'MigrationAddressType', > - 'data': ['socket', 'exec', 'rdma'] } > + 'data': ['socket', 'exec', 'rdma', 'file'] } > + > +## > +# @FileMigrationArgs: > +# > +# @path: file path > +# > +# @offset: initial offset for the file > +# > +# Since 8.2 > +## > +{ 'struct': 'FileMigrationArgs', > + 'data': {'path': 'str', > + 'offset': 'uint64' } } > > ## > # @MigrationExecCommand: > @@ -1547,7 +1562,8 @@ > 'data': { > 'socket': 'SocketAddress', > 'exec': 'MigrationExecCommand', > - 'rdma': 'InetSocketAddress' } } > + 'rdma': 'InetSocketAddress', > + 'file': 'FileMigrationArgs' } } > > ## > # @migrate: > -- > 2.35.3 > With regards, Daniel
On Mon, Oct 23, 2023 at 11:01:43AM +0200, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > On Fri, Oct 20, 2023 at 02:37:16PM +0200, Markus Armbruster wrote: > >> Fabiano Rosas <farosas@suse.de> writes: > >> > >> > Markus Armbruster <armbru@redhat.com> writes: > >> > > >> >> Fabiano Rosas <farosas@suse.de> writes: > >> >> > >> >>> Signed-off-by: Fabiano Rosas <farosas@suse.de> > >> > > >> > Side question: are we using valid JSON at all? I threw this in a random > >> > online linter and it complains about the single quotes. We could have a > >> > proper tool doing the validation in CI. > >> > >> You've come a sad, sad place. > >> > >> docs/devel/qapi-code-gen.rst: > >> > >> Schema syntax > >> ------------- > >> > >> Syntax is loosely based on `JSON <http://www.ietf.org/rfc/rfc8259.txt>`_. > >> Differences: > >> > >> * Comments: start with a hash character (``#``) that is not part of a > >> string, and extend to the end of the line. > >> > >> * Strings are enclosed in ``'single quotes'``, not ``"double quotes"``. > >> > >> * Strings are restricted to printable ASCII, and escape sequences to > >> just ``\\``. > >> > >> * Numbers and ``null`` are not supported. > >> > >> If your reaction to item 2 is "this is stupid", you'd be exactly right. > >> > >> Here's the conclusion of a discussion on possible improvements we had in > >> 2020: > >> https://lore.kernel.org/qemu-devel/877dt5ofoi.fsf@dusky.pond.sub.org/ > > > > Looking at those options again I so strongly want to be able to > > say "none of the above". > > > > We have a need to describe data structures, and generate code for > > serializing/deserializing them on the wire. We created a language > > for this and wrote our own C code generator, our own docs generator, > > own our json parser & formatter, and now are also writing our own > > Go code generator (Python, Rust too ?). > > > > IMHO this is way too much NiH for my likely, and creates a maint > > burden for ourselves that we could do without. > > > > <open-can-of-worms> > > At the point in time we invented QAPI this was perhaps justifiable, > > though even back then I thought we should have done a binary format > > and used XDR to describe it, as a standard pre-existing language and > > toolset. > > Path dependence... > > I wasn't involved in the decisions that led to QAPI, nor its initial > design and implementation. > > Its design was substantially constrained by QMP, which predates QAPI by > almost two years. I was involved in QMP's design for a bit. We argued > back and forth, and I eventually stepped aside to let the guys doing the > actual work make the decisions. I'm somewhat conflating QMP & QAPI in my previous message too. > The maintenance burden is modest, but real. The QAPI generator has > gotten ~1.6 patches per week for the last five years, trending down. In > the last year, it's been 68 patches, 437 insertions, 338 deletions > total, much of it in docs/devel/qapi-code-gen.rst. Meanwhile, the > entire project has had 130 times as many patches, 620 times as many > insertions, and 400 times as many deletions. > > QAPI infrastructure maintenance is dwarved several times over by QAPI > schema maintenance. Chiefly patch review. Yeah, patch review on new additions is primary amount and work and indeed the bit we actually do want to be investing most time in, as API modelling is a hard problem that needs careful review. > To be fair, adding language bindings will take a non-trivial one time > investment plus ongoing maintenance for each language. As well as the developer & reviewer man hours cost from creatnig such language bindings, there's also the negative impact of potential developers being discouraged from interacting with QEMU precisely to avoid taking on this man hours cost. Impossible to quantify, as we can't measure how many projects haven't even been started, due to such avoidance. This hidden cost of discouragement is where I think the value of using standard protocols wins out the most. > > > Today I wouldn't suggest XDR, but would be inclined to suggest > > Protobuf. This already has code & docs generators for every > > programming language we would ever need. > > > > Protobuf has a defined serialization format too though which is > > binary based IIUC, so distinct from our JSON wire format. > > > > The interesting question though is whether it would be feasible to > > transition to this today, despite our historical baggage ? With > > recent interest in accessing QAPI for many more languages, it > > Go, Rust, and what else? The big one is Python, which we should have done years ago, since we use it widely in our own test suites. Over time QEMU is widely used enough though, that it would be conceptually useful for many other non-niche languages to have formal APIs for talking to QEMU, but not compelling enough to justify manually writing such APIs or code generators ourselves. > > is timely to consider if we can adopt a standardized toolset to > > reduce this burden of talking to QEMU from other languages. > > > > A transition would need several big ticket items > > > > * A QAPI visitor impl that can spit out the protobuf document. > > ie to translate all our existing QAPI formats into protobuf, > > as doing this manually would be madness. This is probably > > the easy bit. > > This is about machine-assisted translation of the QAPI schema to > protobuf, isn't it? Yeah, schema conversion. > > Above all it does assume it is possible to define a > > loss-less mapping from QAPI -> Protobuf language. I've not proved > > this is possible, but my inclination is that it probably should be, > > and if not, we could likely enable it by strategically deprecated > > and then deleting troublesome bits prior to a conversion. > > </open-can-of-worms> > > While I share your distaste for the massive NIH QAPI has become, I'm not > sure replacing it now is economical. It requires a massive one-time > investment, offset by saving us one-time investments into QAPI bindings > for other languages of interest. Whether it can actually reduce ongoing > maintenance appreciably after the replacement is unclear. This is the really hard to answer question, as the calculating value of a conversion relies on predicting an unknowable future. It is right to be sceptical about the value of changing, but also worth thinking about it none the less. Its probably the kind of project that someone who doesn't have to justify their time would do as an experiment in a fork of QEMU for a while to see how it actually works out. With regards, Daniel
diff --git a/qapi/migration.json b/qapi/migration.json index c352c7ac52..602cb706e3 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -1519,10 +1519,25 @@ # # @rdma: Migrate via RDMA. # +# @file: Direct the migration stream to a file. +# # Since 8.2 ## { 'enum': 'MigrationAddressType', - 'data': ['socket', 'exec', 'rdma'] } + 'data': ['socket', 'exec', 'rdma', 'file'] } + +## +# @FileMigrationArgs: +# +# @path: file path +# +# @offset: initial offset for the file +# +# Since 8.2 +## +{ 'struct': 'FileMigrationArgs', + 'data': {'path': 'str', + 'offset': 'uint64' } } ## # @MigrationExecCommand: @@ -1547,7 +1562,8 @@ 'data': { 'socket': 'SocketAddress', 'exec': 'MigrationExecCommand', - 'rdma': 'InetSocketAddress' } } + 'rdma': 'InetSocketAddress', + 'file': 'FileMigrationArgs' } } ## # @migrate:
Signed-off-by: Fabiano Rosas <farosas@suse.de> --- qapi/migration.json | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)