qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Various changes "backportability"


From: Michael Tokarev
Subject: Re: Various changes "backportability"
Date: Wed, 13 Sep 2023 17:44:38 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.0

13.09.2023 17:27, Stefan Hajnoczi wrote:
...
For example, recent tpm bugfix, which is trivial by its own,
uses RETRY_ON_EINTR helper which were introduced recently and
which is now used everywhere.  coroutine_fn et al markers is
another example, translator_io_start is yet another, and so
on and so on.

The general concept makes sense to me but I'm not sure what the
specific issue with adding (?) coroutine_fn was. Can you link to the
patch that caused difficulties so I can review it?

There's nothing really exciting here, and coroutine_fn example isn't
a best one really.  I'm talking about this:

https://gitlab.com/mjt0k/qemu/-/commit/c5034f827726f5876234bf4c6a0fab648fd8b020

which is a current back-port of 92e2e6a867334a990f8d29f07ca34e3162fdd6ec
"virtio: Drop out of coroutine context in virtio_load()":

https://gitlab.com/mjt0k/qemu/-/commit/92e2e6a867334a990f8d29f07ca34e3162fdd6ec

This is a bugfix which I tried to cherry-pick (btw, I dunno yet if it should
go to 8.0 or 7.2 to begin with, asked this in another email, but it still
serves as an example).  Original patch adds coroutine_mixed_fn to some existing
functions and to a newly added function.

The patch introducing coroutine_mixed_fn marker is v7.2.0-909-g0f3de970 .
This is actually a very good example of a way how things are done best,
an excellent example of what I'm talking here, - this 0f3de970 only introduces
the new concept (to be widely used), not converting everything to it
right away.  So it's a good example of how things can be done right.

But this 0f3de970 change is based on earlier change which split things up
and moved stuff from one place to another, and which is too large to
backport.  So even if 0f3de970 did an excellent job, it is still of no
use in this context.

I decided to drop coroutine_mixed_fn markings in the fix for 7.2 in this
context, - again, if this particular fix is needed there to begin with,
which is a question unrelated to this topic.


A better example is a trivial thing with RETRY_ON_EINTR introduction.
A trivial macro which replaced TFR in

commit 37b0b24e933c18269dddbf6b83f91823cacf8105
Author: Nikita Ivanov <nivanov@cloudlinux.com>
Date:   Sun Oct 23 12:04:22 2022 +0300

    error handling: Use RETRY_ON_EINTR() macro where applicable

if this change were split into two, first introducing the new macro
and second converting existing code & removing old macro, it'd be
possible to just cherry-pick the first part and thered' be no need
to modify further cherry-picks which uses RETRY_ON_EINTR.

But once again, this all is definitely not as important as getting
good code into main :)

Thanks,

/mjt



reply via email to

[Prev in Thread] Current Thread [Next in Thread]