| Message ID | 20220715175521.126649-1-sj@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix persistent grants negotiation with a behavior change | expand |
Hi all, On Fri, 15 Jul 2022 17:55:19 +0000 SeongJae Park <sj@kernel.org> wrote: > The first patch of this patchset fixes 'feature_persistent' parameter > handling in 'blkback' to respect the frontend's persistent grants > support always. The fix makes a behavioral change, so the second patch > makes the counterpart of 'blkfront' to consistently follow the behavior > change. I made the behavior change as requested by Andrii[1]. I therefore made similar behavior change to blkfront and Cc-ed stable for the second change, too. To make the change history clear and reduce the stable side overhead, however, it might be better to apply the v2, which don't make behavior change but only fix the issue, Cc stable@ for it, make the behavior change commits for both blkback and blkfront, update the documents, and don't Cc stable@ for the behavior change and documents update commits. One downside of that would be that it will make a behavioral difference in pre-5.19.x and post-5.19.x. I think both downsides are not critical, so posted this patchset in this shape. If anyone prefer some changes, please let me know, though. [1] https://lore.kernel.org/xen-devel/CAJwUmVB6H3iTs-C+U=v-pwJB7-_ZRHPxHzKRJZ22xEPW7z8a=g@mail.gmail.com/ Thanks, SJ > > Changes from v2 > (https://lore.kernel.org/xen-devel/20220714224410.51147-1-sj@kernel.org/) > - Keep the behavioral change of v1 > - Update blkfront's counterpart to follow the changed behavior > - Update documents for the changed behavior > > Changes from v1 > (https://lore.kernel.org/xen-devel/20220106091013.126076-1-mheyne@amazon.de/) > - Avoid the behavioral change > (https://lore.kernel.org/xen-devel/20220121102309.27802-1-sj@kernel.org/) > - Rebase on latest xen/tip/linux-next > - Re-work by SeongJae Park <sj@kernel.org> > - Cc stable@ > > > > Maximilian Heyne (1): > xen, blkback: fix persistent grants negotiation > > SeongJae Park (1): > xen-blkfront: Apply 'feature_persistent' parameter when connect > > Documentation/ABI/testing/sysfs-driver-xen-blkback | 2 +- > Documentation/ABI/testing/sysfs-driver-xen-blkfront | 2 +- > drivers/block/xen-blkback/xenbus.c | 9 +++------ > drivers/block/xen-blkfront.c | 4 +--- > 4 files changed, 6 insertions(+), 11 deletions(-) > > -- > 2.25.1
Hi all, On Fri, 15 Jul 2022 18:12:26 +0000 SeongJae Park <sj@kernel.org> wrote: > Hi all, > > On Fri, 15 Jul 2022 17:55:19 +0000 SeongJae Park <sj@kernel.org> wrote: > > > The first patch of this patchset fixes 'feature_persistent' parameter > > handling in 'blkback' to respect the frontend's persistent grants > > support always. The fix makes a behavioral change, so the second patch > > makes the counterpart of 'blkfront' to consistently follow the behavior > > change. > > I made the behavior change as requested by Andrii[1]. I therefore made similar > behavior change to blkfront and Cc-ed stable for the second change, too. Now I realize that commit aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent grants") introduced two issues. One is what Max reported with his patch, and the second one is unintended behavioral change that broke Andrii's use case. That is, Andrii's use case should had no problem at all before the introduction of 'feature_persistent', as at that time 'blkback' checked if the frontend support the persistent grants for every 'reconnect()' and enables it if so. However, introduction of the parameter made it behaves differently. Yes, we intended to make the prameter to make effects to newly created devices. But, as it breaks user workflows, this should be fixed. Same for the 'blkfront' side 'feature_persistent'. > > To make the change history clear and reduce the stable side overhead, however, > it might be better to apply the v2, which don't make behavior change but only > fix the issue, Cc stable@ for it, make the behavior change commits for both > blkback and blkfront, update the documents, and don't Cc stable@ for the > behavior change and documents update commits. I'd say having one patch for each issue would be the right way to go, and all fixes should Cc stable@. > > One downside of that would be that it will make a behavioral difference in > pre-5.19.x and post-5.19.x. The unintended behavioral fix should also be considered fix and therefore should be merged into stable@, so above concern is not valid. I will send the next spin soon. Thanks, SJ [...]