Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Fri, Dec 04, 2020 at 06:08:13PM +0100, Paolo Bonzini wrote: On 04/12/20 16:49, Sasha Levin wrote: On Fri, Dec 04, 2020 at 09:27:28AM +0100, Paolo Bonzini wrote: On 01/12/20 00:59, Sasha Levin wrote: It's quite easy to NAK a patch too, just reply saying "no" and it'll be dropped (just like this patch was dropped right after your first reply) so the burden on maintainers is minimal. The maintainers are _already_ marking patches with "Cc: stable". That They're not, though. Some forget, some subsystems don't mark anything, some don't mark it as it's not stable material when it lands in their tree but then it turns out to be one if it sits there for too long. That means some subsystems will be worse as far as stable release support goes. That's not a problem: - some subsystems have people paid to do backports to LTS releases when patches don't apply; others don't, if the patch doesn't apply the bug is simply not fixed in LTS releases Why not? A warning mail is originated and folks fix those up. I fixed a whole bunch of these myself for subsystems I'm not "paid" to do so. - some subsystems are worse than others even in "normal" releases :) Agree with that. (plus backports) is where the burden on maintainers should start and end. I don't see the need to second guess them. This is similar to describing our CI infrastructure as "second guessing": why are we second guessing authors and maintainers who are obviously doing the right thing by testing their patches and reporting issues to them? No, it's not the same. CI helps finding bugs before you have to waste time spending bisecting regressions across thousands of commits. The lack of stable tags _can_ certainly be a problem, but it solves itself sooner or later when people upgrade their kernel. If just waiting with fixing issues is ok until a user might "eventually" upgrade is acceptable then why bother with a stable tree to begin with? -- Thanks, Sasha ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 04/12/20 16:49, Sasha Levin wrote: On Fri, Dec 04, 2020 at 09:27:28AM +0100, Paolo Bonzini wrote: On 01/12/20 00:59, Sasha Levin wrote: It's quite easy to NAK a patch too, just reply saying "no" and it'll be dropped (just like this patch was dropped right after your first reply) so the burden on maintainers is minimal. The maintainers are _already_ marking patches with "Cc: stable". That They're not, though. Some forget, some subsystems don't mark anything, some don't mark it as it's not stable material when it lands in their tree but then it turns out to be one if it sits there for too long. That means some subsystems will be worse as far as stable release support goes. That's not a problem: - some subsystems have people paid to do backports to LTS releases when patches don't apply; others don't, if the patch doesn't apply the bug is simply not fixed in LTS releases - some subsystems are worse than others even in "normal" releases :) (plus backports) is where the burden on maintainers should start and end. I don't see the need to second guess them. This is similar to describing our CI infrastructure as "second guessing": why are we second guessing authors and maintainers who are obviously doing the right thing by testing their patches and reporting issues to them? No, it's not the same. CI helps finding bugs before you have to waste time spending bisecting regressions across thousands of commits. The lack of stable tags _can_ certainly be a problem, but it solves itself sooner or later when people upgrade their kernel. Are you saying that you have always gotten stable tags right? never missed a stable tag where one should go? Of course I did, just like I have introduced bugs. But at least I try to do my best both at adding stable tags and at not introducing bugs. Paolo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Fri, 2020-12-04 at 10:49 -0500, Sasha Levin wrote: > On Fri, Dec 04, 2020 at 09:27:28AM +0100, Paolo Bonzini wrote: > > On 01/12/20 00:59, Sasha Levin wrote: > > > > > > It's quite easy to NAK a patch too, just reply saying "no" and it'll be > > > dropped (just like this patch was dropped right after your first reply) > > > so the burden on maintainers is minimal. > > > > The maintainers are _already_ marking patches with "Cc: stable". That > > They're not, though. Some forget, some subsystems don't mark anything, > some don't mark it as it's not stable material when it lands in their > tree but then it turns out to be one if it sits there for too long. > > > (plus backports) is where the burden on maintainers should start and > > end. I don't see the need to second guess them. > > This is similar to describing our CI infrastructure as "second > guessing": why are we second guessing authors and maintainers who are > obviously doing the right thing by testing their patches and reporting > issues to them? > > Are you saying that you have always gotten stable tags right? never > missed a stable tag where one should go? I think this simply adds to the burden of being a maintainer without all that much value. I think the primary value here would be getting people to upgrade to current versions rather than backporting to nominally stable and relatively actively changed old versions. This is very much related to this thread about trivial patches and maintainer burdening: https://lore.kernel.org/lkml/1c7d7fde126bc0acf825766de64bf2f9b888f216.ca...@hansenpartnership.com/ ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Fri, Dec 04, 2020 at 09:27:28AM +0100, Paolo Bonzini wrote: On 01/12/20 00:59, Sasha Levin wrote: It's quite easy to NAK a patch too, just reply saying "no" and it'll be dropped (just like this patch was dropped right after your first reply) so the burden on maintainers is minimal. The maintainers are _already_ marking patches with "Cc: stable". That They're not, though. Some forget, some subsystems don't mark anything, some don't mark it as it's not stable material when it lands in their tree but then it turns out to be one if it sits there for too long. (plus backports) is where the burden on maintainers should start and end. I don't see the need to second guess them. This is similar to describing our CI infrastructure as "second guessing": why are we second guessing authors and maintainers who are obviously doing the right thing by testing their patches and reporting issues to them? Are you saying that you have always gotten stable tags right? never missed a stable tag where one should go? -- Thanks, Sasha ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 01/12/20 00:59, Sasha Levin wrote: It's quite easy to NAK a patch too, just reply saying "no" and it'll be dropped (just like this patch was dropped right after your first reply) so the burden on maintainers is minimal. The maintainers are _already_ marking patches with "Cc: stable". That (plus backports) is where the burden on maintainers should start and end. I don't see the need to second guess them. Paolo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Mon, Nov 30, 2020 at 09:29:02PM +0100, Paolo Bonzini wrote: On 30/11/20 20:44, Mike Christie wrote: I have never seen a public/open-source vhost-scsi testsuite. For patch 23 (the one that adds the lun reset support which is built on patch 22), we can't add it to stable right now if you wanted to, because it has a bug in it. Michael T, sent the fix: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/commit/?h=linux-next=b4fffc177fad3c99ee049611a508ca9561bb6871 to Linus today. Ok, so at least it was only a close call and anyway not for something that most people would be running on their machines. But it still seems to me that the state of CI in Linux is abysmal compared to what is needed to arbitrarily(*) pick up patches and commit them to "stable" trees. Paolo (*) A ML bot is an arbitrary choice as far as we are concerned since we cannot know how it makes a decision. The choice of patches is "arbitrary", but the decision is human. The patches are reviewed coming out of the AI, sent to public mailing list(s) for review, followed by 2 reminders asking for reviews. The process for AUTOSEL patches generally takes longer than most patches do for upstream. It's quite easy to NAK a patch too, just reply saying "no" and it'll be dropped (just like this patch was dropped right after your first reply) so the burden on maintainers is minimal. -- Thanks, Sasha ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 30/11/20 20:44, Mike Christie wrote: I have never seen a public/open-source vhost-scsi testsuite. For patch 23 (the one that adds the lun reset support which is built on patch 22), we can't add it to stable right now if you wanted to, because it has a bug in it. Michael T, sent the fix: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/commit/?h=linux-next=b4fffc177fad3c99ee049611a508ca9561bb6871 to Linus today. Ok, so at least it was only a close call and anyway not for something that most people would be running on their machines. But it still seems to me that the state of CI in Linux is abysmal compared to what is needed to arbitrarily(*) pick up patches and commit them to "stable" trees. Paolo (*) A ML bot is an arbitrary choice as far as we are concerned since we cannot know how it makes a decision. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 30/11/20 18:38, Sasha Levin wrote: I am not aware of any public CI being done _at all_ done on vhost-scsi, by CKI or everyone else. So autoselection should be done only on subsystems that have very high coverage in CI. Where can I find a testsuite for virtio/vhost? I see one for KVM, but where is the one that the maintainers of virtio/vhost run on patches that come in? I don't know of any, especially for vhost-scsi. MikeC? Paolo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Mon, Nov 30, 2020 at 09:33:46AM +0100, Paolo Bonzini wrote: On 29/11/20 22:06, Sasha Levin wrote: Plus all the testing we have for the stable trees, yes. It goes beyond just compiling at this point. Your very own co-workers (https://cki-project.org/) are pushing hard on this effort around stable kernel testing, and statements like these aren't helping anyone. I am not aware of any public CI being done _at all_ done on vhost-scsi, by CKI or everyone else. So autoselection should be done only on subsystems that have very high coverage in CI. Where can I find a testsuite for virtio/vhost? I see one for KVM, but where is the one that the maintainers of virtio/vhost run on patches that come in? -- Thanks, Sasha ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Mon, Nov 30, 2020 at 03:00:13PM +0100, Paolo Bonzini wrote: On 30/11/20 14:57, Greg KH wrote: Every patch should be "fixing a real issue"---even a new feature. But the larger the patch, the more the submitters and maintainers should be trusted rather than a bot. The line between feature and bugfix_sometimes_ is blurry, I would say that in this case it's not, and it makes me question how the bot decided that this patch would be acceptable for stable (which AFAIK is not something that can be answered). I thought that earlier Sasha said that this patch was needed as a prerequisite patch for a later fix, right? If not, sorry, I've lost the train of thought in this thread... Yeah---sorry I am replying to 22/33 but referring to 23/33, which is the one that in my opinion should not be blindly accepted for stable kernels without the agreement of the submitter or maintainer. But it's not "blindly", right? I've sent this review mail over a week ago, and if it goes into the queue there will be at least two more emails going out to the author/maintainers. During all this time it gets tested by various entities who do things that go beyond simple boot testing. I'd argue that the backports we push in the stable tree sometimes get tested and reviewed better than the commits that land upstream. -- Thanks, Sasha ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 30/11/20 14:57, Greg KH wrote: Every patch should be "fixing a real issue"---even a new feature. But the larger the patch, the more the submitters and maintainers should be trusted rather than a bot. The line between feature and bugfix_sometimes_ is blurry, I would say that in this case it's not, and it makes me question how the bot decided that this patch would be acceptable for stable (which AFAIK is not something that can be answered). I thought that earlier Sasha said that this patch was needed as a prerequisite patch for a later fix, right? If not, sorry, I've lost the train of thought in this thread... Yeah---sorry I am replying to 22/33 but referring to 23/33, which is the one that in my opinion should not be blindly accepted for stable kernels without the agreement of the submitter or maintainer. Paolo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Mon, Nov 30, 2020 at 02:52:11PM +0100, Paolo Bonzini wrote: > On 30/11/20 14:28, Greg KH wrote: > > > > Lines of code is not everything. If you think that this needs additional > > > > testing then that's fine and we can drop it, but not picking up a fix > > > > just because it's 120 lines is not something we'd do. > > > Starting with the first two steps in stable-kernel-rules.rst: > > > > > > Rules on what kind of patches are accepted, and which ones are not, into > > > the > > > "-stable" tree: > > > > > > - It must be obviously correct and tested. > > > - It cannot be bigger than 100 lines, with context. > > We do obviously take patches that are bigger than 100 lines, as there > > are always exceptions to the rules here. Look at all of the > > spectre/meltdown patches as one such example. Should we refuse a patch > > just because it fixes a real issue yet is 101 lines long? > > Every patch should be "fixing a real issue"---even a new feature. But the > larger the patch, the more the submitters and maintainers should be trusted > rather than a bot. The line between feature and bugfix _sometimes_ is > blurry, I would say that in this case it's not, and it makes me question how > the bot decided that this patch would be acceptable for stable (which AFAIK > is not something that can be answered). I thought that earlier Sasha said that this patch was needed as a prerequisite patch for a later fix, right? If not, sorry, I've lost the train of thought in this thread... thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 30/11/20 14:28, Greg KH wrote: Lines of code is not everything. If you think that this needs additional testing then that's fine and we can drop it, but not picking up a fix just because it's 120 lines is not something we'd do. Starting with the first two steps in stable-kernel-rules.rst: Rules on what kind of patches are accepted, and which ones are not, into the "-stable" tree: - It must be obviously correct and tested. - It cannot be bigger than 100 lines, with context. We do obviously take patches that are bigger than 100 lines, as there are always exceptions to the rules here. Look at all of the spectre/meltdown patches as one such example. Should we refuse a patch just because it fixes a real issue yet is 101 lines long? Every patch should be "fixing a real issue"---even a new feature. But the larger the patch, the more the submitters and maintainers should be trusted rather than a bot. The line between feature and bugfix _sometimes_ is blurry, I would say that in this case it's not, and it makes me question how the bot decided that this patch would be acceptable for stable (which AFAIK is not something that can be answered). Paolo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Mon, Nov 30, 2020 at 09:33:46AM +0100, Paolo Bonzini wrote: > On 29/11/20 22:06, Sasha Levin wrote: > > On Sun, Nov 29, 2020 at 06:34:01PM +0100, Paolo Bonzini wrote: > > > On 29/11/20 05:13, Sasha Levin wrote: > > > > > Which doesn't seem to be suitable for stable either... Patch 3/5 in > > > > > > > > Why not? It was sent as a fix to Linus. > > > > > > Dunno, 120 lines of new code? Even if it's okay for an rc, I don't > > > see why it is would be backported to stable releases and release it > > > without any kind of testing. Maybe for 5.9 the chances of breaking > > > > Lines of code is not everything. If you think that this needs additional > > testing then that's fine and we can drop it, but not picking up a fix > > just because it's 120 lines is not something we'd do. > > Starting with the first two steps in stable-kernel-rules.rst: > > Rules on what kind of patches are accepted, and which ones are not, into the > "-stable" tree: > > - It must be obviously correct and tested. > - It cannot be bigger than 100 lines, with context. We do obviously take patches that are bigger than 100 lines, as there are always exceptions to the rules here. Look at all of the spectre/meltdown patches as one such example. Should we refuse a patch just because it fixes a real issue yet is 101 lines long? thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 29/11/20 22:06, Sasha Levin wrote: On Sun, Nov 29, 2020 at 06:34:01PM +0100, Paolo Bonzini wrote: On 29/11/20 05:13, Sasha Levin wrote: Which doesn't seem to be suitable for stable either... Patch 3/5 in Why not? It was sent as a fix to Linus. Dunno, 120 lines of new code? Even if it's okay for an rc, I don't see why it is would be backported to stable releases and release it without any kind of testing. Maybe for 5.9 the chances of breaking Lines of code is not everything. If you think that this needs additional testing then that's fine and we can drop it, but not picking up a fix just because it's 120 lines is not something we'd do. Starting with the first two steps in stable-kernel-rules.rst: Rules on what kind of patches are accepted, and which ones are not, into the "-stable" tree: - It must be obviously correct and tested. - It cannot be bigger than 100 lines, with context. Plus all the testing we have for the stable trees, yes. It goes beyond just compiling at this point. Your very own co-workers (https://cki-project.org/) are pushing hard on this effort around stable kernel testing, and statements like these aren't helping anyone. I am not aware of any public CI being done _at all_ done on vhost-scsi, by CKI or everyone else. So autoselection should be done only on subsystems that have very high coverage in CI. Paolo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Sun, Nov 29, 2020 at 06:34:01PM +0100, Paolo Bonzini wrote: On 29/11/20 05:13, Sasha Levin wrote: Which doesn't seem to be suitable for stable either... Patch 3/5 in Why not? It was sent as a fix to Linus. Dunno, 120 lines of new code? Even if it's okay for an rc, I don't see why it is would be backported to stable releases and release it without any kind of testing. Maybe for 5.9 the chances of breaking Lines of code is not everything. If you think that this needs additional testing then that's fine and we can drop it, but not picking up a fix just because it's 120 lines is not something we'd do. things are low, but stuff like locking rules might have changed since older releases like 5.4 or 4.19. The autoselection bot does not know that, it basically crosses fingers that these larger-scale changes cause the patches not to apply or compile anymore. Plus all the testing we have for the stable trees, yes. It goes beyond just compiling at this point. Your very own co-workers (https://cki-project.org/) are pushing hard on this effort around stable kernel testing, and statements like these aren't helping anyone. If on the other hand, you'd like to see specific KVM/virtio/etc tests as part of the stable release process, we should all work together to make sure they're included in the current test suite. Maybe it's just me, but the whole "autoselect stable patches" and release them is very suspicious. You are basically crossing fingers Historically autoselected patches were later fixed/reverted at a lower ratio than patches tagged with a stable tag. I *think* that it's because they get a longer review cycle than some of the stable tagged patches. and are ready to release any kind of untested crap, because you do not trust maintainers of marking stable patches right. Only then, when a It's not that I don't trust - some folks forget, or not realize that something should go in stable. We're all humans. This is to complement the work done by maintainers, not replace it. backport is broken, it's maintainers who get the blame and have to fix it. What blame? Who's blaming who? Personally I don't care because I have asked you to opt KVM out of autoselection, but this is the opposite of what Greg brags about when he touts the virtues of the upstream stable process over vendor kernels. What, that we try and include all fixes rather than the ones I'm paid to pick up? If you have a vendor you pay $$$ to, then yes - you're probably better off with a vendor kernel. This is actually in line (I think) with Greg's views on this (http://kroah.com/log/blog/2018/08/24/what-stable-kernel-should-i-use/). -- Thanks, Sasha ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 29/11/20 05:13, Sasha Levin wrote: Which doesn't seem to be suitable for stable either... Patch 3/5 in Why not? It was sent as a fix to Linus. Dunno, 120 lines of new code? Even if it's okay for an rc, I don't see why it is would be backported to stable releases and release it without any kind of testing. Maybe for 5.9 the chances of breaking things are low, but stuff like locking rules might have changed since older releases like 5.4 or 4.19. The autoselection bot does not know that, it basically crosses fingers that these larger-scale changes cause the patches not to apply or compile anymore. Maybe it's just me, but the whole "autoselect stable patches" and release them is very suspicious. You are basically crossing fingers and are ready to release any kind of untested crap, because you do not trust maintainers of marking stable patches right. Only then, when a backport is broken, it's maintainers who get the blame and have to fix it. Personally I don't care because I have asked you to opt KVM out of autoselection, but this is the opposite of what Greg brags about when he touts the virtues of the upstream stable process over vendor kernels. Paolo the series might be (vhost scsi: fix cmd completion race), so I can understand including 1/5 and 2/5 just in case, but not the rest. Does the bot not understand diffstats? Not on their own, no. What's wrong with the diffstats? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Wed, Nov 25, 2020 at 07:08:54PM +0100, Paolo Bonzini wrote:
On 25/11/20 19:01, Sasha Levin wrote:
On Wed, Nov 25, 2020 at 06:48:21PM +0100, Paolo Bonzini wrote:
On 25/11/20 16:35, Sasha Levin wrote:
From: Mike Christie
[ Upstream commit 18f1becb6948cd411fd01968a0a54af63732e73c ]
Move code to parse lun from req's lun_buf to helper, so tmf code
can use it in the next patch.
Signed-off-by: Mike Christie
Reviewed-by: Paolo Bonzini
Acked-by: Jason Wang
Link:
https://lore.kernel.org/r/1604986403-4931-5-git-send-email-michael.chris...@oracle.com
Signed-off-by: Michael S. Tsirkin
Acked-by: Stefan Hajnoczi
Signed-off-by: Sasha Levin
This doesn't seem like stable material, does it?
It went in as a dependency for efd838fec17b ("vhost scsi: Add support
for LUN resets."), which is the next patch.
Which doesn't seem to be suitable for stable either... Patch 3/5 in
Why not? It was sent as a fix to Linus.
the series might be (vhost scsi: fix cmd completion race), so I can
understand including 1/5 and 2/5 just in case, but not the rest. Does
the bot not understand diffstats?
Not on their own, no. What's wrong with the diffstats?
--
Thanks,
Sasha
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 25/11/20 19:01, Sasha Levin wrote:
On Wed, Nov 25, 2020 at 06:48:21PM +0100, Paolo Bonzini wrote:
On 25/11/20 16:35, Sasha Levin wrote:
From: Mike Christie
[ Upstream commit 18f1becb6948cd411fd01968a0a54af63732e73c ]
Move code to parse lun from req's lun_buf to helper, so tmf code
can use it in the next patch.
Signed-off-by: Mike Christie
Reviewed-by: Paolo Bonzini
Acked-by: Jason Wang
Link:
https://lore.kernel.org/r/1604986403-4931-5-git-send-email-michael.chris...@oracle.com
Signed-off-by: Michael S. Tsirkin
Acked-by: Stefan Hajnoczi
Signed-off-by: Sasha Levin
This doesn't seem like stable material, does it?
It went in as a dependency for efd838fec17b ("vhost scsi: Add support
for LUN resets."), which is the next patch.
Which doesn't seem to be suitable for stable either... Patch 3/5 in the
series might be (vhost scsi: fix cmd completion race), so I can
understand including 1/5 and 2/5 just in case, but not the rest. Does
the bot not understand diffstats?
Paolo
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On Wed, Nov 25, 2020 at 06:48:21PM +0100, Paolo Bonzini wrote:
On 25/11/20 16:35, Sasha Levin wrote:
From: Mike Christie
[ Upstream commit 18f1becb6948cd411fd01968a0a54af63732e73c ]
Move code to parse lun from req's lun_buf to helper, so tmf code
can use it in the next patch.
Signed-off-by: Mike Christie
Reviewed-by: Paolo Bonzini
Acked-by: Jason Wang
Link:
https://lore.kernel.org/r/1604986403-4931-5-git-send-email-michael.chris...@oracle.com
Signed-off-by: Michael S. Tsirkin
Acked-by: Stefan Hajnoczi
Signed-off-by: Sasha Levin
This doesn't seem like stable material, does it?
It went in as a dependency for efd838fec17b ("vhost scsi: Add support
for LUN resets."), which is the next patch.
--
Thanks,
Sasha
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
On 25/11/20 16:35, Sasha Levin wrote:
From: Mike Christie
[ Upstream commit 18f1becb6948cd411fd01968a0a54af63732e73c ]
Move code to parse lun from req's lun_buf to helper, so tmf code
can use it in the next patch.
Signed-off-by: Mike Christie
Reviewed-by: Paolo Bonzini
Acked-by: Jason Wang
Link:
https://lore.kernel.org/r/1604986403-4931-5-git-send-email-michael.chris...@oracle.com
Signed-off-by: Michael S. Tsirkin
Acked-by: Stefan Hajnoczi
Signed-off-by: Sasha Levin
This doesn't seem like stable material, does it?
Paolo
---
drivers/vhost/scsi.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 5d8850f5aef16..ed7dc6b998f65 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -898,6 +898,11 @@ vhost_scsi_get_req(struct vhost_virtqueue *vq, struct
vhost_scsi_ctx *vc,
return ret;
}
+static u16 vhost_buf_to_lun(u8 *lun_buf)
+{
+ return ((lun_buf[2] << 8) | lun_buf[3]) & 0x3FFF;
+}
+
static void
vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
{
@@ -1036,12 +1041,12 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct
vhost_virtqueue *vq)
tag = vhost64_to_cpu(vq, v_req_pi.tag);
task_attr = v_req_pi.task_attr;
cdb = _req_pi.cdb[0];
- lun = ((v_req_pi.lun[2] << 8) | v_req_pi.lun[3]) &
0x3FFF;
+ lun = vhost_buf_to_lun(v_req_pi.lun);
} else {
tag = vhost64_to_cpu(vq, v_req.tag);
task_attr = v_req.task_attr;
cdb = _req.cdb[0];
- lun = ((v_req.lun[2] << 8) | v_req.lun[3]) & 0x3FFF;
+ lun = vhost_buf_to_lun(v_req.lun);
}
/*
* Check that the received CDB size does not exceeded our
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper
From: Mike Christie
[ Upstream commit 18f1becb6948cd411fd01968a0a54af63732e73c ]
Move code to parse lun from req's lun_buf to helper, so tmf code
can use it in the next patch.
Signed-off-by: Mike Christie
Reviewed-by: Paolo Bonzini
Acked-by: Jason Wang
Link:
https://lore.kernel.org/r/1604986403-4931-5-git-send-email-michael.chris...@oracle.com
Signed-off-by: Michael S. Tsirkin
Acked-by: Stefan Hajnoczi
Signed-off-by: Sasha Levin
---
drivers/vhost/scsi.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 5d8850f5aef16..ed7dc6b998f65 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -898,6 +898,11 @@ vhost_scsi_get_req(struct vhost_virtqueue *vq, struct
vhost_scsi_ctx *vc,
return ret;
}
+static u16 vhost_buf_to_lun(u8 *lun_buf)
+{
+ return ((lun_buf[2] << 8) | lun_buf[3]) & 0x3FFF;
+}
+
static void
vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
{
@@ -1036,12 +1041,12 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct
vhost_virtqueue *vq)
tag = vhost64_to_cpu(vq, v_req_pi.tag);
task_attr = v_req_pi.task_attr;
cdb = _req_pi.cdb[0];
- lun = ((v_req_pi.lun[2] << 8) | v_req_pi.lun[3]) &
0x3FFF;
+ lun = vhost_buf_to_lun(v_req_pi.lun);
} else {
tag = vhost64_to_cpu(vq, v_req.tag);
task_attr = v_req.task_attr;
cdb = _req.cdb[0];
- lun = ((v_req.lun[2] << 8) | v_req.lun[3]) & 0x3FFF;
+ lun = vhost_buf_to_lun(v_req.lun);
}
/*
* Check that the received CDB size does not exceeded our
--
2.27.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
