Re: [PATCH] virtio-blk: Fix kconfig option
Kent Overstreet writes: > On Fri, Sep 07, 2012 at 09:10:25AM +0930, Rusty Russell wrote: >> Kent Overstreet writes: >> >> > On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: >> >> On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: >> >> > Do you not understand the difference between depends an selects? >> >> > Or did you not read my original mail? >> >> Now you're getting insulting. > > Yes, but at least I'm not being intentionally obtuse. Insulting again. Wow. It took me this long to understand your complaint. Perhaps I'm stupid. Or perhaps you are terrible at explaining yourself, and it is only through our patient and heroic efforts that we can comprehend you at all? > Think about it from the user's pov. They check what VIRTIO_BLK depends > on - just VIRTIO. > > So they try to figure out how to flip on VIRTIO, or what VIRTIO even is. > > See how that last step might be problematic? CONFIG_VIRTIO is not > exposed! It doesn't even seem to control anything! > > Go back to your example. Checking the dependencies for E1000 would tell > you the user needs to flip on CONFIG_PCI. Done. Easy. Actually, it depends on NET_VENDOR_INTEL which depends on CONFIG_PCI, but yes, it's discoverable. So your actual complaint is that: 1) CONFIG_VIRTIO is misleadingly documented both in comment and name. 2) It's not discoverable, since it's only selected via other things. > And, if that is what you're doing with CONFIG_VIRTIO (I'm still not > sure) the comment at the top of drivers/virtio/Kconfig is _wrong_: As grep would show you, it's selected by LGUEST, S390_GUEST, RPMSG, VIRTIO_PCI and VIRTIO_MMIO (VIRTIO_BALLOON is a cut & paste bug, already patched by MST). We could change every virtio device to depend on (CONFIG_LGUEST | CONFIG_S390_GUEST | CONFIG_RPMSG | CONFIG_VIRTIO_PCI | CONFIG_VIRTIO_MMIO), which is more discoverable but uglier. How's this workaround? From: Rusty Russell Subject: virtio: add help to CONFIG_VIRTIO option. Trying to enable a virtio driver (eg CONFIG_VIRTIO_BLK) is painful because it depends on CONFIG_VIRTIO. CONFIG_VIRTIO doesn't tell you how to turn it on (it's selected from anything which provides a virtio bus). This patch at least adds some documentation, visible in menuconfig, as a hint. Reported-by: Kent Overstreet Signed-off-by: Rusty Russell diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -1,8 +1,10 @@ -# Virtio always gets selected by whoever wants it. config VIRTIO tristate + ---help--- + This option is selected by any driver which implements the virtio + bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_LGUEST, + CONFIG_RPMSG or CONFIG_S390_GUEST. -# Similarly the virtio ring implementation. config VIRTIO_RING tristate depends on VIRTIO -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Fri, Sep 07, 2012 at 09:10:25AM +0930, Rusty Russell wrote: > Kent Overstreet writes: > > > On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: > >> On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: > >> > Do you not understand the difference between depends an selects? > >> > Or did you not read my original mail? > > Now you're getting insulting. Yes, but at least I'm not being intentionally obtuse. > It's normal for options to depend on other options. Sometimes they're > directly nested (eg. E1000 depends on NETDEVICES, and it's nested under > that option), sometimes they're not (eg. E1000 depends on PCI, which is > selected elsewhere). > > The fact that you are only just realizing this is not Michael's problem. Like I said, I'm well aware of that. The issue here isn't the dependency, it's that it depends on something that isn't exposed anywhere! Think about it from the user's pov. They check what VIRTIO_BLK depends on - just VIRTIO. So they try to figure out how to flip on VIRTIO, or what VIRTIO even is. See how that last step might be problematic? CONFIG_VIRTIO is not exposed! It doesn't even seem to control anything! Go back to your example. Checking the dependencies for E1000 would tell you the user needs to flip on CONFIG_PCI. Done. Easy. User checks the dependencies here and... what do _you_ expect people to do? Look, depending on a kconfig option that's supposed to be user controllable but isn't exposed anywhere is flat out broken. The fact that it's in a different submenu just makes it worse. The problem is that VIRTIO_BLK's dependencies are not actually specified in the kconfig. If it depends on VIRTIO_PCI, that's what the kconfig should say. If it depends on having any of multiple virtio backends enabled, then specify that! depends VIRTIO_PCI || VIRTIO_WHATEVER Or if you really want to have a fake config option that's enabled if you have any virtio backend enabled, fix the damn comments and naming! How is anyone supposed to know that CONFIG_VIRTIO really means "any virtio backend?" Call it VIRTIO_ANY_BACKEND if that's what it really is. And, if that is what you're doing with CONFIG_VIRTIO (I'm still not sure) the comment at the top of drivers/virtio/Kconfig is _wrong_: # Virtio always gets selected by whoever wants it. VIRTIO tristate How is _anyone_ supposed to know that really means "VIRTIO gets selected by things that provide a virtio backend?" C'mon, you've had to debug other people's code before. What would _you_ think if you were tripped up by something like that? > >> > Flip off everything in drivers -> virtio > >> > > >> > Now go to drivers -> block and try to turn on virtio-blk. > >> > > >> > It's not listed! > >> > >> Yes. Because you disabled all virtio backends. > >> It does not make sense to have any frontends. > > > > How's a user - or even another kernel developer who isn't familiar with > > virtio - supposed to know that? > > I get annoyed that menuconfig doesn't show options whose dependencies > aren't possible, too. (I got bitten the other way: it doesn't show > dependencies which can't be disabled, and I was trying to turn KALLSYMS > off). > > But as I found out just last week, the '/' key allows you to find any > option, and shows what dependencies it has, and their values. Yep, use it all the time. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
Kent Overstreet writes: > On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: >> On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: >> > Do you not understand the difference between depends an selects? >> > Or did you not read my original mail? Now you're getting insulting. It's normal for options to depend on other options. Sometimes they're directly nested (eg. E1000 depends on NETDEVICES, and it's nested under that option), sometimes they're not (eg. E1000 depends on PCI, which is selected elsewhere). The fact that you are only just realizing this is not Michael's problem. >> > Flip off everything in drivers -> virtio >> > >> > Now go to drivers -> block and try to turn on virtio-blk. >> > >> > It's not listed! >> >> Yes. Because you disabled all virtio backends. >> It does not make sense to have any frontends. > > How's a user - or even another kernel developer who isn't familiar with > virtio - supposed to know that? I get annoyed that menuconfig doesn't show options whose dependencies aren't possible, too. (I got bitten the other way: it doesn't show dependencies which can't be disabled, and I was trying to turn KALLSYMS off). But as I found out just last week, the '/' key allows you to find any option, and shows what dependencies it has, and their values. Hope that helps, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 03:31:44AM -0700, Kent Overstreet wrote: > On Thu, Sep 06, 2012 at 01:18:43PM +0300, Michael S. Tsirkin wrote: > > On Thu, Sep 06, 2012 at 03:02:48AM -0700, Kent Overstreet wrote: > > > On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: > > > > On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: > > > > > On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: > > > > > > On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: > > > > > > > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > > > > > > > > Kent Overstreet writes: > > > > > > > > > > > > > > > > > CONFIG_VIRTIO isn't exposed, everything else is supposed to > > > > > > > > > select it > > > > > > > > > instead. > > > > > > > > > > > > > > > > This is a slight mis-understanding. It's supposed to be > > > > > > > > selected by > > > > > > > > the particular driver, probably virtio_pci in your case. > > > > > > > > > > > > > > So are you saying virtio-blk depends on virtio-pci? If so, the > > > > > > > kconfig > > > > > > > should have that. > > > > > > > > > > > > > > As is, VIRTIO_BLK just has: > > > > > > > depends on EXPERIMENTAL && VIRTIO > > > > > > > > > > > > > > which is flat out broken. > > > > > > > > > > > > I don't think anything is broken. > > > > > > Can you show an example of a broken configuration? > > > > > > > > > > Do you not understand the difference between depends an selects? > > > > > Or did you not read my original mail? > > > > > Flip off everything in drivers -> virtio > > > > > > > > > > Now go to drivers -> block and try to turn on virtio-blk. > > > > > > > > > > It's not listed! > > > > > > > > Yes. Because you disabled all virtio backends. > > > > It does not make sense to have any frontends. > > > > > > How's a user - or even another kernel developer who isn't familiar with > > > virtio - supposed to know that? > > > > > > I still don't know what exactly a virtio backend is - the term isn't > > > even mentioned anywhere that I've seen. > > > > > > Whatever it is though virtio-blk should be depending on _that_, not a > > > config option that _isn't exposed in the menu_! > > > > > > > > Now go back to drivers -> virtio and turn on (randomly) balloon. > > > > > > > > > > Go back to drivers -> block, and now you can turn on virtio-blk! > > > > > > > > > > Do you see what's wrong with this picture? > > > > > > > > Yes. You got unlucky with your random guess. > > > > It's a bug in balloon kconfig: it should not > > > > select virtio. > > > > I sent a patch to fix that yesterday. > > > > > > Then it's also a bug in the comments at the top of > > > drivers/virtio/Kconfig. > > > > > > And besides that, how the _hell_ is a user supposed to know to turn on > > > VIRTIO_PCI before VIRTIO_BLK? It's not documented anywhere (if that is > > > what's supposed to happen! I still don't know) > > > > Well, what kind of device do you have? Tell us :) > > If it's a virtio pci device, > > you need to enable virtio-pci and virtio-blk. > > I run qemu with -drive if=virtio. You tell me! -drive if= is a compatibility option. qemu makes an effort to guess what it is you want to do. Result is usually correct but it means people building their own kernels get confused. For x86 kvm the modern equivalent is: -device virtio-blk-pci,drive=foobar -drive if=no,... If you use this you get what you asked for :). Yes this usage is not documented anywhere, but this is not guest driver's problem. > Better yet, tell me how the user is supposed to figure it out! As usual when you do not know which driver to select. Boot a distro kernel and look around. Where is your virtio device? On a pci bus? There you are. > > > > > and even if it was > > > documented, having one kconfig option depend on something that's exposed > > > in a _completely different menu_ is just made of fail. > > > > Fine, but why pick on virtio? > > This is extremely common in kconfig. > > For example, a ton of network drivers depend > > on PCI, it's exactly the same thing. > > Never noticed where CONFIG_PCI is exposed in bus options? I see it: CONFIG_PCI: │ Find out whether you have a PCI motherboard. PCI is the name of a │ │ bus system, i.e. the way the CPU talks to the other stuff inside │ │ your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or │ │ VESA. If you have PCI, say Y, otherwise N. │ > Nope, not the same thing. You just happen to know what PCI is but not what VIRTIO PCI is. This is fair enough, but not sure how to help in this case. Your patch won't help though. -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 01:18:43PM +0300, Michael S. Tsirkin wrote: > On Thu, Sep 06, 2012 at 03:02:48AM -0700, Kent Overstreet wrote: > > On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: > > > On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: > > > > On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: > > > > > On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: > > > > > > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > > > > > > > Kent Overstreet writes: > > > > > > > > > > > > > > > CONFIG_VIRTIO isn't exposed, everything else is supposed to > > > > > > > > select it > > > > > > > > instead. > > > > > > > > > > > > > > This is a slight mis-understanding. It's supposed to be selected > > > > > > > by > > > > > > > the particular driver, probably virtio_pci in your case. > > > > > > > > > > > > So are you saying virtio-blk depends on virtio-pci? If so, the > > > > > > kconfig > > > > > > should have that. > > > > > > > > > > > > As is, VIRTIO_BLK just has: > > > > > > depends on EXPERIMENTAL && VIRTIO > > > > > > > > > > > > which is flat out broken. > > > > > > > > > > I don't think anything is broken. > > > > > Can you show an example of a broken configuration? > > > > > > > > Do you not understand the difference between depends an selects? > > > > Or did you not read my original mail? > > > > Flip off everything in drivers -> virtio > > > > > > > > Now go to drivers -> block and try to turn on virtio-blk. > > > > > > > > It's not listed! > > > > > > Yes. Because you disabled all virtio backends. > > > It does not make sense to have any frontends. > > > > How's a user - or even another kernel developer who isn't familiar with > > virtio - supposed to know that? > > > > I still don't know what exactly a virtio backend is - the term isn't > > even mentioned anywhere that I've seen. > > > > Whatever it is though virtio-blk should be depending on _that_, not a > > config option that _isn't exposed in the menu_! > > > > > > Now go back to drivers -> virtio and turn on (randomly) balloon. > > > > > > > > Go back to drivers -> block, and now you can turn on virtio-blk! > > > > > > > > Do you see what's wrong with this picture? > > > > > > Yes. You got unlucky with your random guess. > > > It's a bug in balloon kconfig: it should not > > > select virtio. > > > I sent a patch to fix that yesterday. > > > > Then it's also a bug in the comments at the top of > > drivers/virtio/Kconfig. > > > > And besides that, how the _hell_ is a user supposed to know to turn on > > VIRTIO_PCI before VIRTIO_BLK? It's not documented anywhere (if that is > > what's supposed to happen! I still don't know) > > Well, what kind of device do you have? Tell us :) > If it's a virtio pci device, > you need to enable virtio-pci and virtio-blk. I run qemu with -drive if=virtio. You tell me! Better yet, tell me how the user is supposed to figure it out! > > > and even if it was > > documented, having one kconfig option depend on something that's exposed > > in a _completely different menu_ is just made of fail. > > Fine, but why pick on virtio? > This is extremely common in kconfig. > For example, a ton of network drivers depend > on PCI, it's exactly the same thing. Never noticed where CONFIG_PCI is exposed in bus options? Nope, not the same thing. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 03:02:48AM -0700, Kent Overstreet wrote: > On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: > > On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: > > > On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: > > > > On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: > > > > > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > > > > > > Kent Overstreet writes: > > > > > > > > > > > > > CONFIG_VIRTIO isn't exposed, everything else is supposed to > > > > > > > select it > > > > > > > instead. > > > > > > > > > > > > This is a slight mis-understanding. It's supposed to be selected by > > > > > > the particular driver, probably virtio_pci in your case. > > > > > > > > > > So are you saying virtio-blk depends on virtio-pci? If so, the kconfig > > > > > should have that. > > > > > > > > > > As is, VIRTIO_BLK just has: > > > > > depends on EXPERIMENTAL && VIRTIO > > > > > > > > > > which is flat out broken. > > > > > > > > I don't think anything is broken. > > > > Can you show an example of a broken configuration? > > > > > > Do you not understand the difference between depends an selects? > > > Or did you not read my original mail? > > > Flip off everything in drivers -> virtio > > > > > > Now go to drivers -> block and try to turn on virtio-blk. > > > > > > It's not listed! > > > > Yes. Because you disabled all virtio backends. > > It does not make sense to have any frontends. > > How's a user - or even another kernel developer who isn't familiar with > virtio - supposed to know that? > > I still don't know what exactly a virtio backend is - the term isn't > even mentioned anywhere that I've seen. > > Whatever it is though virtio-blk should be depending on _that_, not a > config option that _isn't exposed in the menu_! > > > > Now go back to drivers -> virtio and turn on (randomly) balloon. > > > > > > Go back to drivers -> block, and now you can turn on virtio-blk! > > > > > > Do you see what's wrong with this picture? > > > > Yes. You got unlucky with your random guess. > > It's a bug in balloon kconfig: it should not > > select virtio. > > I sent a patch to fix that yesterday. > > Then it's also a bug in the comments at the top of > drivers/virtio/Kconfig. > > And besides that, how the _hell_ is a user supposed to know to turn on > VIRTIO_PCI before VIRTIO_BLK? It's not documented anywhere (if that is > what's supposed to happen! I still don't know) Well, what kind of device do you have? Tell us :) If it's a virtio pci device, you need to enable virtio-pci and virtio-blk. > and even if it was > documented, having one kconfig option depend on something that's exposed > in a _completely different menu_ is just made of fail. Fine, but why pick on virtio? This is extremely common in kconfig. For example, a ton of network drivers depend on PCI, it's exactly the same thing. -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: > On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: > > On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: > > > On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: > > > > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > > > > > Kent Overstreet writes: > > > > > > > > > > > CONFIG_VIRTIO isn't exposed, everything else is supposed to select > > > > > > it > > > > > > instead. > > > > > > > > > > This is a slight mis-understanding. It's supposed to be selected by > > > > > the particular driver, probably virtio_pci in your case. > > > > > > > > So are you saying virtio-blk depends on virtio-pci? If so, the kconfig > > > > should have that. > > > > > > > > As is, VIRTIO_BLK just has: > > > > depends on EXPERIMENTAL && VIRTIO > > > > > > > > which is flat out broken. > > > > > > I don't think anything is broken. > > > Can you show an example of a broken configuration? > > > > Do you not understand the difference between depends an selects? > > Or did you not read my original mail? > > Flip off everything in drivers -> virtio > > > > Now go to drivers -> block and try to turn on virtio-blk. > > > > It's not listed! > > Yes. Because you disabled all virtio backends. > It does not make sense to have any frontends. How's a user - or even another kernel developer who isn't familiar with virtio - supposed to know that? I still don't know what exactly a virtio backend is - the term isn't even mentioned anywhere that I've seen. Whatever it is though virtio-blk should be depending on _that_, not a config option that _isn't exposed in the menu_! > > Now go back to drivers -> virtio and turn on (randomly) balloon. > > > > Go back to drivers -> block, and now you can turn on virtio-blk! > > > > Do you see what's wrong with this picture? > > Yes. You got unlucky with your random guess. > It's a bug in balloon kconfig: it should not > select virtio. > I sent a patch to fix that yesterday. Then it's also a bug in the comments at the top of drivers/virtio/Kconfig. And besides that, how the _hell_ is a user supposed to know to turn on VIRTIO_PCI before VIRTIO_BLK? It's not documented anywhere (if that is what's supposed to happen! I still don't know) and even if it was documented, having one kconfig option depend on something that's exposed in a _completely different menu_ is just made of fail. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: > On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: > > On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: > > > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > > > > Kent Overstreet writes: > > > > > > > > > CONFIG_VIRTIO isn't exposed, everything else is supposed to select it > > > > > instead. > > > > > > > > This is a slight mis-understanding. It's supposed to be selected by > > > > the particular driver, probably virtio_pci in your case. > > > > > > So are you saying virtio-blk depends on virtio-pci? If so, the kconfig > > > should have that. > > > > > > As is, VIRTIO_BLK just has: > > > depends on EXPERIMENTAL && VIRTIO > > > > > > which is flat out broken. > > > > I don't think anything is broken. > > Can you show an example of a broken configuration? > > Do you not understand the difference between depends an selects? > Or did you not read my original mail? > Flip off everything in drivers -> virtio > > Now go to drivers -> block and try to turn on virtio-blk. > > It's not listed! Yes. Because you disabled all virtio backends. It does not make sense to have any frontends. > Now go back to drivers -> virtio and turn on (randomly) balloon. > > Go back to drivers -> block, and now you can turn on virtio-blk! > > Do you see what's wrong with this picture? Yes. You got unlucky with your random guess. It's a bug in balloon kconfig: it should not select virtio. I sent a patch to fix that yesterday. -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: > On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: > > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > > > Kent Overstreet writes: > > > > > > > CONFIG_VIRTIO isn't exposed, everything else is supposed to select it > > > > instead. > > > > > > This is a slight mis-understanding. It's supposed to be selected by > > > the particular driver, probably virtio_pci in your case. > > > > So are you saying virtio-blk depends on virtio-pci? If so, the kconfig > > should have that. > > > > As is, VIRTIO_BLK just has: > > depends on EXPERIMENTAL && VIRTIO > > > > which is flat out broken. > > I don't think anything is broken. > Can you show an example of a broken configuration? Do you not understand the difference between depends an selects? Or did you not read my original mail? Flip off everything in drivers -> virtio Now go to drivers -> block and try to turn on virtio-blk. It's not listed! Now go back to drivers -> virtio and turn on (randomly) balloon. Go back to drivers -> block, and now you can turn on virtio-blk! Do you see what's wrong with this picture? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > > Kent Overstreet writes: > > > > > CONFIG_VIRTIO isn't exposed, everything else is supposed to select it > > > instead. > > > > This is a slight mis-understanding. It's supposed to be selected by > > the particular driver, probably virtio_pci in your case. > > So are you saying virtio-blk depends on virtio-pci? If so, the kconfig > should have that. > > As is, VIRTIO_BLK just has: > depends on EXPERIMENTAL && VIRTIO > > which is flat out broken. I don't think anything is broken. Can you show an example of a broken configuration? -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > Kent Overstreet writes: > > > CONFIG_VIRTIO isn't exposed, everything else is supposed to select it > > instead. > > This is a slight mis-understanding. It's supposed to be selected by > the particular driver, probably virtio_pci in your case. So are you saying virtio-blk depends on virtio-pci? If so, the kconfig should have that. As is, VIRTIO_BLK just has: depends on EXPERIMENTAL && VIRTIO which is flat out broken. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. So are you saying virtio-blk depends on virtio-pci? If so, the kconfig should have that. As is, VIRTIO_BLK just has: depends on EXPERIMENTAL VIRTIO which is flat out broken. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. So are you saying virtio-blk depends on virtio-pci? If so, the kconfig should have that. As is, VIRTIO_BLK just has: depends on EXPERIMENTAL VIRTIO which is flat out broken. I don't think anything is broken. Can you show an example of a broken configuration? -- MST -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. So are you saying virtio-blk depends on virtio-pci? If so, the kconfig should have that. As is, VIRTIO_BLK just has: depends on EXPERIMENTAL VIRTIO which is flat out broken. I don't think anything is broken. Can you show an example of a broken configuration? Do you not understand the difference between depends an selects? Or did you not read my original mail? Flip off everything in drivers - virtio Now go to drivers - block and try to turn on virtio-blk. It's not listed! Now go back to drivers - virtio and turn on (randomly) balloon. Go back to drivers - block, and now you can turn on virtio-blk! Do you see what's wrong with this picture? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. So are you saying virtio-blk depends on virtio-pci? If so, the kconfig should have that. As is, VIRTIO_BLK just has: depends on EXPERIMENTAL VIRTIO which is flat out broken. I don't think anything is broken. Can you show an example of a broken configuration? Do you not understand the difference between depends an selects? Or did you not read my original mail? Flip off everything in drivers - virtio Now go to drivers - block and try to turn on virtio-blk. It's not listed! Yes. Because you disabled all virtio backends. It does not make sense to have any frontends. Now go back to drivers - virtio and turn on (randomly) balloon. Go back to drivers - block, and now you can turn on virtio-blk! Do you see what's wrong with this picture? Yes. You got unlucky with your random guess. It's a bug in balloon kconfig: it should not select virtio. I sent a patch to fix that yesterday. -- MST -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. So are you saying virtio-blk depends on virtio-pci? If so, the kconfig should have that. As is, VIRTIO_BLK just has: depends on EXPERIMENTAL VIRTIO which is flat out broken. I don't think anything is broken. Can you show an example of a broken configuration? Do you not understand the difference between depends an selects? Or did you not read my original mail? Flip off everything in drivers - virtio Now go to drivers - block and try to turn on virtio-blk. It's not listed! Yes. Because you disabled all virtio backends. It does not make sense to have any frontends. How's a user - or even another kernel developer who isn't familiar with virtio - supposed to know that? I still don't know what exactly a virtio backend is - the term isn't even mentioned anywhere that I've seen. Whatever it is though virtio-blk should be depending on _that_, not a config option that _isn't exposed in the menu_! Now go back to drivers - virtio and turn on (randomly) balloon. Go back to drivers - block, and now you can turn on virtio-blk! Do you see what's wrong with this picture? Yes. You got unlucky with your random guess. It's a bug in balloon kconfig: it should not select virtio. I sent a patch to fix that yesterday. Then it's also a bug in the comments at the top of drivers/virtio/Kconfig. And besides that, how the _hell_ is a user supposed to know to turn on VIRTIO_PCI before VIRTIO_BLK? It's not documented anywhere (if that is what's supposed to happen! I still don't know) and even if it was documented, having one kconfig option depend on something that's exposed in a _completely different menu_ is just made of fail. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 03:02:48AM -0700, Kent Overstreet wrote: On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. So are you saying virtio-blk depends on virtio-pci? If so, the kconfig should have that. As is, VIRTIO_BLK just has: depends on EXPERIMENTAL VIRTIO which is flat out broken. I don't think anything is broken. Can you show an example of a broken configuration? Do you not understand the difference between depends an selects? Or did you not read my original mail? Flip off everything in drivers - virtio Now go to drivers - block and try to turn on virtio-blk. It's not listed! Yes. Because you disabled all virtio backends. It does not make sense to have any frontends. How's a user - or even another kernel developer who isn't familiar with virtio - supposed to know that? I still don't know what exactly a virtio backend is - the term isn't even mentioned anywhere that I've seen. Whatever it is though virtio-blk should be depending on _that_, not a config option that _isn't exposed in the menu_! Now go back to drivers - virtio and turn on (randomly) balloon. Go back to drivers - block, and now you can turn on virtio-blk! Do you see what's wrong with this picture? Yes. You got unlucky with your random guess. It's a bug in balloon kconfig: it should not select virtio. I sent a patch to fix that yesterday. Then it's also a bug in the comments at the top of drivers/virtio/Kconfig. And besides that, how the _hell_ is a user supposed to know to turn on VIRTIO_PCI before VIRTIO_BLK? It's not documented anywhere (if that is what's supposed to happen! I still don't know) Well, what kind of device do you have? Tell us :) If it's a virtio pci device, you need to enable virtio-pci and virtio-blk. and even if it was documented, having one kconfig option depend on something that's exposed in a _completely different menu_ is just made of fail. Fine, but why pick on virtio? This is extremely common in kconfig. For example, a ton of network drivers depend on PCI, it's exactly the same thing. -- MST -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 01:18:43PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 03:02:48AM -0700, Kent Overstreet wrote: On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. So are you saying virtio-blk depends on virtio-pci? If so, the kconfig should have that. As is, VIRTIO_BLK just has: depends on EXPERIMENTAL VIRTIO which is flat out broken. I don't think anything is broken. Can you show an example of a broken configuration? Do you not understand the difference between depends an selects? Or did you not read my original mail? Flip off everything in drivers - virtio Now go to drivers - block and try to turn on virtio-blk. It's not listed! Yes. Because you disabled all virtio backends. It does not make sense to have any frontends. How's a user - or even another kernel developer who isn't familiar with virtio - supposed to know that? I still don't know what exactly a virtio backend is - the term isn't even mentioned anywhere that I've seen. Whatever it is though virtio-blk should be depending on _that_, not a config option that _isn't exposed in the menu_! Now go back to drivers - virtio and turn on (randomly) balloon. Go back to drivers - block, and now you can turn on virtio-blk! Do you see what's wrong with this picture? Yes. You got unlucky with your random guess. It's a bug in balloon kconfig: it should not select virtio. I sent a patch to fix that yesterday. Then it's also a bug in the comments at the top of drivers/virtio/Kconfig. And besides that, how the _hell_ is a user supposed to know to turn on VIRTIO_PCI before VIRTIO_BLK? It's not documented anywhere (if that is what's supposed to happen! I still don't know) Well, what kind of device do you have? Tell us :) If it's a virtio pci device, you need to enable virtio-pci and virtio-blk. I run qemu with -drive if=virtio. You tell me! Better yet, tell me how the user is supposed to figure it out! and even if it was documented, having one kconfig option depend on something that's exposed in a _completely different menu_ is just made of fail. Fine, but why pick on virtio? This is extremely common in kconfig. For example, a ton of network drivers depend on PCI, it's exactly the same thing. Never noticed where CONFIG_PCI is exposed in bus options? Nope, not the same thing. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Thu, Sep 06, 2012 at 03:31:44AM -0700, Kent Overstreet wrote: On Thu, Sep 06, 2012 at 01:18:43PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 03:02:48AM -0700, Kent Overstreet wrote: On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: On Thu, Sep 06, 2012 at 11:44:03AM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 12:41:13AM -0700, Kent Overstreet wrote: On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. So are you saying virtio-blk depends on virtio-pci? If so, the kconfig should have that. As is, VIRTIO_BLK just has: depends on EXPERIMENTAL VIRTIO which is flat out broken. I don't think anything is broken. Can you show an example of a broken configuration? Do you not understand the difference between depends an selects? Or did you not read my original mail? Flip off everything in drivers - virtio Now go to drivers - block and try to turn on virtio-blk. It's not listed! Yes. Because you disabled all virtio backends. It does not make sense to have any frontends. How's a user - or even another kernel developer who isn't familiar with virtio - supposed to know that? I still don't know what exactly a virtio backend is - the term isn't even mentioned anywhere that I've seen. Whatever it is though virtio-blk should be depending on _that_, not a config option that _isn't exposed in the menu_! Now go back to drivers - virtio and turn on (randomly) balloon. Go back to drivers - block, and now you can turn on virtio-blk! Do you see what's wrong with this picture? Yes. You got unlucky with your random guess. It's a bug in balloon kconfig: it should not select virtio. I sent a patch to fix that yesterday. Then it's also a bug in the comments at the top of drivers/virtio/Kconfig. And besides that, how the _hell_ is a user supposed to know to turn on VIRTIO_PCI before VIRTIO_BLK? It's not documented anywhere (if that is what's supposed to happen! I still don't know) Well, what kind of device do you have? Tell us :) If it's a virtio pci device, you need to enable virtio-pci and virtio-blk. I run qemu with -drive if=virtio. You tell me! -drive if= is a compatibility option. qemu makes an effort to guess what it is you want to do. Result is usually correct but it means people building their own kernels get confused. For x86 kvm the modern equivalent is: -device virtio-blk-pci,drive=foobar -drive if=no,... If you use this you get what you asked for :). Yes this usage is not documented anywhere, but this is not guest driver's problem. Better yet, tell me how the user is supposed to figure it out! As usual when you do not know which driver to select. Boot a distro kernel and look around. Where is your virtio device? On a pci bus? There you are. and even if it was documented, having one kconfig option depend on something that's exposed in a _completely different menu_ is just made of fail. Fine, but why pick on virtio? This is extremely common in kconfig. For example, a ton of network drivers depend on PCI, it's exactly the same thing. Never noticed where CONFIG_PCI is exposed in bus options? I see it: CONFIG_PCI: │ Find out whether you have a PCI motherboard. PCI is the name of a │ │ bus system, i.e. the way the CPU talks to the other stuff inside │ │ your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or │ │ VESA. If you have PCI, say Y, otherwise N. │ Nope, not the same thing. You just happen to know what PCI is but not what VIRTIO PCI is. This is fair enough, but not sure how to help in this case. Your patch won't help though. -- MST -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
Kent Overstreet koverstr...@google.com writes: On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: Do you not understand the difference between depends an selects? Or did you not read my original mail? Now you're getting insulting. It's normal for options to depend on other options. Sometimes they're directly nested (eg. E1000 depends on NETDEVICES, and it's nested under that option), sometimes they're not (eg. E1000 depends on PCI, which is selected elsewhere). The fact that you are only just realizing this is not Michael's problem. Flip off everything in drivers - virtio Now go to drivers - block and try to turn on virtio-blk. It's not listed! Yes. Because you disabled all virtio backends. It does not make sense to have any frontends. How's a user - or even another kernel developer who isn't familiar with virtio - supposed to know that? I get annoyed that menuconfig doesn't show options whose dependencies aren't possible, too. (I got bitten the other way: it doesn't show dependencies which can't be disabled, and I was trying to turn KALLSYMS off). But as I found out just last week, the '/' key allows you to find any option, and shows what dependencies it has, and their values. Hope that helps, Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Fri, Sep 07, 2012 at 09:10:25AM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: Do you not understand the difference between depends an selects? Or did you not read my original mail? Now you're getting insulting. Yes, but at least I'm not being intentionally obtuse. It's normal for options to depend on other options. Sometimes they're directly nested (eg. E1000 depends on NETDEVICES, and it's nested under that option), sometimes they're not (eg. E1000 depends on PCI, which is selected elsewhere). The fact that you are only just realizing this is not Michael's problem. Like I said, I'm well aware of that. The issue here isn't the dependency, it's that it depends on something that isn't exposed anywhere! Think about it from the user's pov. They check what VIRTIO_BLK depends on - just VIRTIO. So they try to figure out how to flip on VIRTIO, or what VIRTIO even is. See how that last step might be problematic? CONFIG_VIRTIO is not exposed! It doesn't even seem to control anything! Go back to your example. Checking the dependencies for E1000 would tell you the user needs to flip on CONFIG_PCI. Done. Easy. User checks the dependencies here and... what do _you_ expect people to do? Look, depending on a kconfig option that's supposed to be user controllable but isn't exposed anywhere is flat out broken. The fact that it's in a different submenu just makes it worse. The problem is that VIRTIO_BLK's dependencies are not actually specified in the kconfig. If it depends on VIRTIO_PCI, that's what the kconfig should say. If it depends on having any of multiple virtio backends enabled, then specify that! depends VIRTIO_PCI || VIRTIO_WHATEVER Or if you really want to have a fake config option that's enabled if you have any virtio backend enabled, fix the damn comments and naming! How is anyone supposed to know that CONFIG_VIRTIO really means any virtio backend? Call it VIRTIO_ANY_BACKEND if that's what it really is. And, if that is what you're doing with CONFIG_VIRTIO (I'm still not sure) the comment at the top of drivers/virtio/Kconfig is _wrong_: # Virtio always gets selected by whoever wants it. VIRTIO tristate How is _anyone_ supposed to know that really means VIRTIO gets selected by things that provide a virtio backend? C'mon, you've had to debug other people's code before. What would _you_ think if you were tripped up by something like that? Flip off everything in drivers - virtio Now go to drivers - block and try to turn on virtio-blk. It's not listed! Yes. Because you disabled all virtio backends. It does not make sense to have any frontends. How's a user - or even another kernel developer who isn't familiar with virtio - supposed to know that? I get annoyed that menuconfig doesn't show options whose dependencies aren't possible, too. (I got bitten the other way: it doesn't show dependencies which can't be disabled, and I was trying to turn KALLSYMS off). But as I found out just last week, the '/' key allows you to find any option, and shows what dependencies it has, and their values. Yep, use it all the time. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
Kent Overstreet koverstr...@google.com writes: On Fri, Sep 07, 2012 at 09:10:25AM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: On Thu, Sep 06, 2012 at 12:49:56PM +0300, Michael S. Tsirkin wrote: On Thu, Sep 06, 2012 at 02:25:12AM -0700, Kent Overstreet wrote: Do you not understand the difference between depends an selects? Or did you not read my original mail? Now you're getting insulting. Yes, but at least I'm not being intentionally obtuse. Insulting again. Wow. It took me this long to understand your complaint. Perhaps I'm stupid. Or perhaps you are terrible at explaining yourself, and it is only through our patient and heroic efforts that we can comprehend you at all? Think about it from the user's pov. They check what VIRTIO_BLK depends on - just VIRTIO. So they try to figure out how to flip on VIRTIO, or what VIRTIO even is. See how that last step might be problematic? CONFIG_VIRTIO is not exposed! It doesn't even seem to control anything! Go back to your example. Checking the dependencies for E1000 would tell you the user needs to flip on CONFIG_PCI. Done. Easy. Actually, it depends on NET_VENDOR_INTEL which depends on CONFIG_PCI, but yes, it's discoverable. So your actual complaint is that: 1) CONFIG_VIRTIO is misleadingly documented both in comment and name. 2) It's not discoverable, since it's only selected via other things. And, if that is what you're doing with CONFIG_VIRTIO (I'm still not sure) the comment at the top of drivers/virtio/Kconfig is _wrong_: As grep would show you, it's selected by LGUEST, S390_GUEST, RPMSG, VIRTIO_PCI and VIRTIO_MMIO (VIRTIO_BALLOON is a cut paste bug, already patched by MST). We could change every virtio device to depend on (CONFIG_LGUEST | CONFIG_S390_GUEST | CONFIG_RPMSG | CONFIG_VIRTIO_PCI | CONFIG_VIRTIO_MMIO), which is more discoverable but uglier. How's this workaround? From: Rusty Russell ru...@rustcorp.com.au Subject: virtio: add help to CONFIG_VIRTIO option. Trying to enable a virtio driver (eg CONFIG_VIRTIO_BLK) is painful because it depends on CONFIG_VIRTIO. CONFIG_VIRTIO doesn't tell you how to turn it on (it's selected from anything which provides a virtio bus). This patch at least adds some documentation, visible in menuconfig, as a hint. Reported-by: Kent Overstreet koverstr...@google.com Signed-off-by: Rusty Russell ru...@rustcorp.com.au diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -1,8 +1,10 @@ -# Virtio always gets selected by whoever wants it. config VIRTIO tristate + ---help--- + This option is selected by any driver which implements the virtio + bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_LGUEST, + CONFIG_RPMSG or CONFIG_S390_GUEST. -# Similarly the virtio ring implementation. config VIRTIO_RING tristate depends on VIRTIO -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
"Michael S. Tsirkin" writes: > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: >> Kent Overstreet writes: >> >> > CONFIG_VIRTIO isn't exposed, everything else is supposed to select it >> > instead. >> >> This is a slight mis-understanding. It's supposed to be selected by >> the particular driver, probably virtio_pci in your case. >> >> Cheers, >> Rusty. > > Actually balloon selects VIRTIO, I think it's a bug. > Also isn't it time we dropped the experimental tag? > Leaving it in for now. Thanks, I applied this, and killed the EXPERIMENTAL in a separate patch. Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > Kent Overstreet writes: > > > CONFIG_VIRTIO isn't exposed, everything else is supposed to select it > > instead. > > This is a slight mis-understanding. It's supposed to be selected by > the particular driver, probably virtio_pci in your case. > > Cheers, > Rusty. Actually balloon selects VIRTIO, I think it's a bug. Also isn't it time we dropped the experimental tag? Leaving it in for now. --> virtio-balloon: dependency fix Devices should depend on virtio, not select it. It's supposed to be selected by the particular driver, e.g. VIRTIO_PCI. Make balloon depend on VIRTIO and EXPERIMENTAL (to match description). Signed-off-by: Michael S. Tsirkin -- diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index f38b17a..564bb64 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -27,8 +27,7 @@ config VIRTIO_PCI config VIRTIO_BALLOON tristate "Virtio balloon driver (EXPERIMENTAL)" - select VIRTIO - select VIRTIO_RING + depends on EXPERIMENTAL && VIRTIO ---help--- This driver supports increasing and decreasing the amount of memory within a KVM guest. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Wed, Sep 05, 2012 at 01:54:07PM +0800, Asias He wrote: > On 09/05/2012 01:48 PM, Michael S. Tsirkin wrote: > > On Wed, Sep 05, 2012 at 12:22:58PM +0800, Asias He wrote: > >> On 09/04/2012 02:23 PM, Rusty Russell wrote: > >>> Kent Overstreet writes: > >>> > CONFIG_VIRTIO isn't exposed, everything else is supposed to select it > instead. > >>> > >>> This is a slight mis-understanding. It's supposed to be selected by > >>> the particular driver, probably virtio_pci in your case. > >> > >> virtio_mmio selects virtio as well. > >> > >> drivers/virtio/Kconfig > >> config VIRTIO_MMIO > >>select VIRTIO > >> config VIRTIO_PCI > >>select VIRTIO > >> > >> How about this: > >> config VIRTIO_BLK > >>depends on EXPERIMENTAL && (VIRTIO_PCI || VIRTIO_MMIO) > > > > Isn't this exactly what depends on VIRTIO achieves? > > Yes. But this gives the information that virtio_blk depends on one of > the virtio transport: virito_pci or virtio_mmio. It can work with any transport. > -- > Asias -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
Michael S. Tsirkin m...@redhat.com writes: On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. Cheers, Rusty. Actually balloon selects VIRTIO, I think it's a bug. Also isn't it time we dropped the experimental tag? Leaving it in for now. Thanks, I applied this, and killed the EXPERIMENTAL in a separate patch. Cheers, Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Wed, Sep 05, 2012 at 01:54:07PM +0800, Asias He wrote: On 09/05/2012 01:48 PM, Michael S. Tsirkin wrote: On Wed, Sep 05, 2012 at 12:22:58PM +0800, Asias He wrote: On 09/04/2012 02:23 PM, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. virtio_mmio selects virtio as well. drivers/virtio/Kconfig config VIRTIO_MMIO select VIRTIO config VIRTIO_PCI select VIRTIO How about this: config VIRTIO_BLK depends on EXPERIMENTAL (VIRTIO_PCI || VIRTIO_MMIO) Isn't this exactly what depends on VIRTIO achieves? Yes. But this gives the information that virtio_blk depends on one of the virtio transport: virito_pci or virtio_mmio. It can work with any transport. -- Asias -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. Cheers, Rusty. Actually balloon selects VIRTIO, I think it's a bug. Also isn't it time we dropped the experimental tag? Leaving it in for now. -- virtio-balloon: dependency fix Devices should depend on virtio, not select it. It's supposed to be selected by the particular driver, e.g. VIRTIO_PCI. Make balloon depend on VIRTIO and EXPERIMENTAL (to match description). Signed-off-by: Michael S. Tsirkin m...@redhat.com -- diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index f38b17a..564bb64 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -27,8 +27,7 @@ config VIRTIO_PCI config VIRTIO_BALLOON tristate Virtio balloon driver (EXPERIMENTAL) - select VIRTIO - select VIRTIO_RING + depends on EXPERIMENTAL VIRTIO ---help--- This driver supports increasing and decreasing the amount of memory within a KVM guest. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On 09/05/2012 01:48 PM, Michael S. Tsirkin wrote: > On Wed, Sep 05, 2012 at 12:22:58PM +0800, Asias He wrote: >> On 09/04/2012 02:23 PM, Rusty Russell wrote: >>> Kent Overstreet writes: >>> CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. >>> >>> This is a slight mis-understanding. It's supposed to be selected by >>> the particular driver, probably virtio_pci in your case. >> >> virtio_mmio selects virtio as well. >> >> drivers/virtio/Kconfig >> config VIRTIO_MMIO >>select VIRTIO >> config VIRTIO_PCI >>select VIRTIO >> >> How about this: >> config VIRTIO_BLK >>depends on EXPERIMENTAL && (VIRTIO_PCI || VIRTIO_MMIO) > > Isn't this exactly what depends on VIRTIO achieves? Yes. But this gives the information that virtio_blk depends on one of the virtio transport: virito_pci or virtio_mmio. -- Asias -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Wed, Sep 05, 2012 at 12:22:58PM +0800, Asias He wrote: > On 09/04/2012 02:23 PM, Rusty Russell wrote: > > Kent Overstreet writes: > > > >> CONFIG_VIRTIO isn't exposed, everything else is supposed to select it > >> instead. > > > > This is a slight mis-understanding. It's supposed to be selected by > > the particular driver, probably virtio_pci in your case. > > virtio_mmio selects virtio as well. > > drivers/virtio/Kconfig > config VIRTIO_MMIO >select VIRTIO > config VIRTIO_PCI >select VIRTIO > > How about this: > config VIRTIO_BLK >depends on EXPERIMENTAL && (VIRTIO_PCI || VIRTIO_MMIO) Isn't this exactly what depends on VIRTIO achieves? > -- > Asias -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On 09/04/2012 02:23 PM, Rusty Russell wrote: > Kent Overstreet writes: > >> CONFIG_VIRTIO isn't exposed, everything else is supposed to select it >> instead. > > This is a slight mis-understanding. It's supposed to be selected by > the particular driver, probably virtio_pci in your case. virtio_mmio selects virtio as well. drivers/virtio/Kconfig config VIRTIO_MMIO select VIRTIO config VIRTIO_PCI select VIRTIO How about this: config VIRTIO_BLK depends on EXPERIMENTAL && (VIRTIO_PCI || VIRTIO_MMIO) -- Asias -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Mon, Sep 3, 2012 at 11:23 PM, Rusty Russell wrote: > Kent Overstreet writes: > >> CONFIG_VIRTIO isn't exposed, everything else is supposed to select it >> instead. > > This is a slight mis-understanding. It's supposed to be selected by > the particular driver, probably virtio_pci in your case. Yes... VIRT_BLOCK isn't selecting it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
Kent Overstreet writes: > CONFIG_VIRTIO isn't exposed, everything else is supposed to select it > instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. Cheers, Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Mon, Sep 3, 2012 at 11:23 PM, Rusty Russell ru...@rustcorp.com.au wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. Yes... VIRT_BLOCK isn't selecting it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On 09/04/2012 02:23 PM, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. virtio_mmio selects virtio as well. drivers/virtio/Kconfig config VIRTIO_MMIO select VIRTIO config VIRTIO_PCI select VIRTIO How about this: config VIRTIO_BLK depends on EXPERIMENTAL (VIRTIO_PCI || VIRTIO_MMIO) -- Asias -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On Wed, Sep 05, 2012 at 12:22:58PM +0800, Asias He wrote: On 09/04/2012 02:23 PM, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. virtio_mmio selects virtio as well. drivers/virtio/Kconfig config VIRTIO_MMIO select VIRTIO config VIRTIO_PCI select VIRTIO How about this: config VIRTIO_BLK depends on EXPERIMENTAL (VIRTIO_PCI || VIRTIO_MMIO) Isn't this exactly what depends on VIRTIO achieves? -- Asias -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
On 09/05/2012 01:48 PM, Michael S. Tsirkin wrote: On Wed, Sep 05, 2012 at 12:22:58PM +0800, Asias He wrote: On 09/04/2012 02:23 PM, Rusty Russell wrote: Kent Overstreet koverstr...@google.com writes: CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. This is a slight mis-understanding. It's supposed to be selected by the particular driver, probably virtio_pci in your case. virtio_mmio selects virtio as well. drivers/virtio/Kconfig config VIRTIO_MMIO select VIRTIO config VIRTIO_PCI select VIRTIO How about this: config VIRTIO_BLK depends on EXPERIMENTAL (VIRTIO_PCI || VIRTIO_MMIO) Isn't this exactly what depends on VIRTIO achieves? Yes. But this gives the information that virtio_blk depends on one of the virtio transport: virito_pci or virtio_mmio. -- Asias -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
Whoops, need VIRTIO_RING too >From 79dc9ae40e40cefd6079e4197cac858a1d59e1d8 Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Sun, 2 Sep 2012 21:44:37 -0700 Subject: [PATCH] virtio-blk: Fix kconfig option CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. Signed-off-by: Kent Overstreet --- drivers/block/Kconfig |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index a796407..40532b8 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -521,7 +521,9 @@ config XEN_BLKDEV_BACKEND config VIRTIO_BLK tristate "Virtio block driver (EXPERIMENTAL)" - depends on EXPERIMENTAL && VIRTIO + select VIRTIO + select VIRTIO_RING + depends on EXPERIMENTAL ---help--- This is the virtual block driver for virtio. It can be used with lguest or QEMU based VMMs (like KVM or Xen). Say Y or M. -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio-blk: Fix kconfig option
Whoops, need VIRTIO_RING too From 79dc9ae40e40cefd6079e4197cac858a1d59e1d8 Mon Sep 17 00:00:00 2001 From: Kent Overstreet koverstr...@google.com Date: Sun, 2 Sep 2012 21:44:37 -0700 Subject: [PATCH] virtio-blk: Fix kconfig option CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. Signed-off-by: Kent Overstreet koverstr...@google.com --- drivers/block/Kconfig |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index a796407..40532b8 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -521,7 +521,9 @@ config XEN_BLKDEV_BACKEND config VIRTIO_BLK tristate Virtio block driver (EXPERIMENTAL) - depends on EXPERIMENTAL VIRTIO + select VIRTIO + select VIRTIO_RING + depends on EXPERIMENTAL ---help--- This is the virtual block driver for virtio. It can be used with lguest or QEMU based VMMs (like KVM or Xen). Say Y or M. -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
