* [PATCH] virtio-blk: Fix kconfig option
@ 2012-09-03 4:41 Kent Overstreet
2012-09-03 4:46 ` Kent Overstreet
` (2 more replies)
0 siblings, 3 replies; 42+ messages in thread
From: Kent Overstreet @ 2012-09-03 4:41 UTC (permalink / raw)
To: rusty, mst; +Cc: virtualization, linux-kernel
CONFIG_VIRTIO isn't exposed, everything else is supposed to select it
instead.
Signed-off-by: Kent Overstreet <koverstreet@google.com>
---
drivers/block/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index a796407..d4e1d12 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -521,7 +521,8 @@ config XEN_BLKDEV_BACKEND
config VIRTIO_BLK
tristate "Virtio block driver (EXPERIMENTAL)"
- depends on EXPERIMENTAL && VIRTIO
+ select VIRTIO
+ 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
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-03 4:41 [PATCH] virtio-blk: Fix kconfig option Kent Overstreet @ 2012-09-03 4:46 ` Kent Overstreet 2012-09-03 4:46 ` Kent Overstreet 2012-09-04 6:23 ` Rusty Russell 2 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-03 4:46 UTC (permalink / raw) To: rusty, mst; +Cc: linux-kernel, virtualization Whoops, need VIRTIO_RING too From 79dc9ae40e40cefd6079e4197cac858a1d59e1d8 Mon Sep 17 00:00:00 2001 From: Kent Overstreet <koverstreet@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 <koverstreet@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 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-03 4:41 [PATCH] virtio-blk: Fix kconfig option Kent Overstreet 2012-09-03 4:46 ` Kent Overstreet @ 2012-09-03 4:46 ` Kent Overstreet 2012-09-04 6:23 ` Rusty Russell 2 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-03 4:46 UTC (permalink / raw) To: rusty, mst; +Cc: virtualization, linux-kernel Whoops, need VIRTIO_RING too >From 79dc9ae40e40cefd6079e4197cac858a1d59e1d8 Mon Sep 17 00:00:00 2001 From: Kent Overstreet <koverstreet@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 <koverstreet@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 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-03 4:41 [PATCH] virtio-blk: Fix kconfig option Kent Overstreet @ 2012-09-04 6:23 ` Rusty Russell 2012-09-03 4:46 ` Kent Overstreet 2012-09-04 6:23 ` Rusty Russell 2 siblings, 0 replies; 42+ messages in thread From: Rusty Russell @ 2012-09-04 6:23 UTC (permalink / raw) To: Kent Overstreet, mst; +Cc: virtualization, linux-kernel Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-04 6:23 ` Rusty Russell 0 siblings, 0 replies; 42+ messages in thread From: Rusty Russell @ 2012-09-04 6:23 UTC (permalink / raw) To: Kent Overstreet, mst; +Cc: linux-kernel, virtualization Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-04 6:23 ` Rusty Russell @ 2012-09-04 23:12 ` Kent Overstreet -1 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-04 23:12 UTC (permalink / raw) To: Rusty Russell; +Cc: mst, virtualization, linux-kernel On Mon, Sep 3, 2012 at 11:23 PM, Rusty Russell <rusty@rustcorp.com.au> wrote: > Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-04 23:12 ` Kent Overstreet 0 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-04 23:12 UTC (permalink / raw) To: Rusty Russell; +Cc: virtualization, linux-kernel, mst On Mon, Sep 3, 2012 at 11:23 PM, Rusty Russell <rusty@rustcorp.com.au> wrote: > Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-04 6:23 ` Rusty Russell @ 2012-09-05 4:22 ` Asias He -1 siblings, 0 replies; 42+ messages in thread From: Asias He @ 2012-09-05 4:22 UTC (permalink / raw) To: Rusty Russell; +Cc: Kent Overstreet, mst, linux-kernel, virtualization On 09/04/2012 02:23 PM, Rusty Russell wrote: > Kent Overstreet <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-05 4:22 ` Asias He 0 siblings, 0 replies; 42+ messages in thread From: Asias He @ 2012-09-05 4:22 UTC (permalink / raw) To: Rusty Russell; +Cc: Kent Overstreet, virtualization, linux-kernel, mst On 09/04/2012 02:23 PM, Rusty Russell wrote: > Kent Overstreet <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-05 4:22 ` Asias He @ 2012-09-05 5:48 ` Michael S. Tsirkin -1 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-05 5:48 UTC (permalink / raw) To: Asias He; +Cc: Rusty Russell, Kent Overstreet, linux-kernel, virtualization 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-05 5:48 ` Michael S. Tsirkin 0 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-05 5:48 UTC (permalink / raw) To: Asias He; +Cc: Kent Overstreet, linux-kernel, virtualization 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-05 5:48 ` Michael S. Tsirkin @ 2012-09-05 5:54 ` Asias He -1 siblings, 0 replies; 42+ messages in thread From: Asias He @ 2012-09-05 5:54 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Rusty Russell, Kent Overstreet, linux-kernel, virtualization 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-05 5:54 ` Asias He 0 siblings, 0 replies; 42+ messages in thread From: Asias He @ 2012-09-05 5:54 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Kent Overstreet, linux-kernel, virtualization 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-05 5:54 ` Asias He @ 2012-09-05 6:04 ` Michael S. Tsirkin -1 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-05 6:04 UTC (permalink / raw) To: Asias He; +Cc: Rusty Russell, Kent Overstreet, linux-kernel, virtualization 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-05 6:04 ` Michael S. Tsirkin 0 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-05 6:04 UTC (permalink / raw) To: Asias He; +Cc: Kent Overstreet, linux-kernel, virtualization 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-04 6:23 ` Rusty Russell @ 2012-09-05 6:12 ` Michael S. Tsirkin -1 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-05 6:12 UTC (permalink / raw) To: Rusty Russell; +Cc: Kent Overstreet, virtualization, linux-kernel On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > Kent Overstreet <koverstreet@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 <mst@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. ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-05 6:12 ` Michael S. Tsirkin 0 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-05 6:12 UTC (permalink / raw) To: Rusty Russell; +Cc: Kent Overstreet, linux-kernel, virtualization On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > Kent Overstreet <koverstreet@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 <mst@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. ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-05 6:12 ` Michael S. Tsirkin @ 2012-09-06 1:46 ` Rusty Russell -1 siblings, 0 replies; 42+ messages in thread From: Rusty Russell @ 2012-09-06 1:46 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Kent Overstreet, virtualization, linux-kernel "Michael S. Tsirkin" <mst@redhat.com> writes: > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: >> Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 1:46 ` Rusty Russell 0 siblings, 0 replies; 42+ messages in thread From: Rusty Russell @ 2012-09-06 1:46 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Kent Overstreet, linux-kernel, virtualization "Michael S. Tsirkin" <mst@redhat.com> writes: > On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: >> Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-04 6:23 ` Rusty Russell @ 2012-09-06 7:41 ` Kent Overstreet -1 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-06 7:41 UTC (permalink / raw) To: Rusty Russell; +Cc: mst, virtualization, linux-kernel On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 7:41 ` Kent Overstreet 0 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-06 7:41 UTC (permalink / raw) To: Rusty Russell; +Cc: virtualization, linux-kernel, mst On Tue, Sep 04, 2012 at 03:53:53PM +0930, Rusty Russell wrote: > Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-06 7:41 ` Kent Overstreet @ 2012-09-06 8:44 ` Michael S. Tsirkin -1 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-06 8:44 UTC (permalink / raw) To: Kent Overstreet; +Cc: Rusty Russell, virtualization, linux-kernel 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 8:44 ` Michael S. Tsirkin 0 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-06 8:44 UTC (permalink / raw) To: Kent Overstreet; +Cc: linux-kernel, virtualization 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-06 8:44 ` Michael S. Tsirkin @ 2012-09-06 9:25 ` Kent Overstreet -1 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-06 9:25 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Rusty Russell, virtualization, linux-kernel 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 <koverstreet@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? ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 9:25 ` Kent Overstreet 0 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-06 9:25 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: linux-kernel, virtualization 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 <koverstreet@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? ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-06 9:25 ` Kent Overstreet @ 2012-09-06 9:49 ` Michael S. Tsirkin -1 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-06 9:49 UTC (permalink / raw) To: Kent Overstreet; +Cc: Rusty Russell, virtualization, linux-kernel 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 9:49 ` Michael S. Tsirkin 0 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-06 9:49 UTC (permalink / raw) To: Kent Overstreet; +Cc: linux-kernel, virtualization 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-06 9:49 ` Michael S. Tsirkin @ 2012-09-06 10:02 ` Kent Overstreet -1 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-06 10:02 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Rusty Russell, virtualization, linux-kernel 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 <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 10:02 ` Kent Overstreet 0 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-06 10:02 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: linux-kernel, virtualization 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 <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-06 10:02 ` Kent Overstreet @ 2012-09-06 10:18 ` Michael S. Tsirkin -1 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-06 10:18 UTC (permalink / raw) To: Kent Overstreet; +Cc: Rusty Russell, virtualization, linux-kernel 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 10:18 ` Michael S. Tsirkin 0 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-06 10:18 UTC (permalink / raw) To: Kent Overstreet; +Cc: linux-kernel, virtualization 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-06 10:18 ` Michael S. Tsirkin @ 2012-09-06 10:31 ` Kent Overstreet -1 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-06 10:31 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Rusty Russell, virtualization, linux-kernel 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 <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 10:31 ` Kent Overstreet 0 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-06 10:31 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: linux-kernel, virtualization 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 <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-06 10:31 ` Kent Overstreet @ 2012-09-06 11:09 ` Michael S. Tsirkin -1 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-06 11:09 UTC (permalink / raw) To: Kent Overstreet; +Cc: Rusty Russell, virtualization, linux-kernel 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 <koverstreet@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 11:09 ` Michael S. Tsirkin 0 siblings, 0 replies; 42+ messages in thread From: Michael S. Tsirkin @ 2012-09-06 11:09 UTC (permalink / raw) To: Kent Overstreet; +Cc: linux-kernel, virtualization 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 <koverstreet@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 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-06 10:02 ` Kent Overstreet @ 2012-09-06 23:40 ` Rusty Russell -1 siblings, 0 replies; 42+ messages in thread From: Rusty Russell @ 2012-09-06 23:40 UTC (permalink / raw) To: Kent Overstreet, Michael S. Tsirkin; +Cc: virtualization, linux-kernel Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-06 23:40 ` Rusty Russell 0 siblings, 0 replies; 42+ messages in thread From: Rusty Russell @ 2012-09-06 23:40 UTC (permalink / raw) To: Kent Overstreet, Michael S. Tsirkin; +Cc: linux-kernel, virtualization Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-06 23:40 ` Rusty Russell @ 2012-09-07 0:25 ` Kent Overstreet -1 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-07 0:25 UTC (permalink / raw) To: Rusty Russell; +Cc: Michael S. Tsirkin, virtualization, linux-kernel On Fri, Sep 07, 2012 at 09:10:25AM +0930, Rusty Russell wrote: > Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-07 0:25 ` Kent Overstreet 0 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-07 0:25 UTC (permalink / raw) To: Rusty Russell; +Cc: virtualization, linux-kernel, Michael S. Tsirkin On Fri, Sep 07, 2012 at 09:10:25AM +0930, Rusty Russell wrote: > Kent Overstreet <koverstreet@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. ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option 2012-09-07 0:25 ` Kent Overstreet @ 2012-09-07 2:57 ` Rusty Russell -1 siblings, 0 replies; 42+ messages in thread From: Rusty Russell @ 2012-09-07 2:57 UTC (permalink / raw) To: Kent Overstreet; +Cc: Michael S. Tsirkin, virtualization, linux-kernel Kent Overstreet <koverstreet@google.com> writes: > On Fri, Sep 07, 2012 at 09:10:25AM +0930, Rusty Russell wrote: >> Kent Overstreet <koverstreet@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 <rusty@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 <koverstreet@google.com> Signed-off-by: Rusty Russell <rusty@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] virtio-blk: Fix kconfig option @ 2012-09-07 2:57 ` Rusty Russell 0 siblings, 0 replies; 42+ messages in thread From: Rusty Russell @ 2012-09-07 2:57 UTC (permalink / raw) To: Kent Overstreet; +Cc: virtualization, linux-kernel, Michael S. Tsirkin Kent Overstreet <koverstreet@google.com> writes: > On Fri, Sep 07, 2012 at 09:10:25AM +0930, Rusty Russell wrote: >> Kent Overstreet <koverstreet@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 <rusty@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 <koverstreet@google.com> Signed-off-by: Rusty Russell <rusty@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 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] virtio-blk: Fix kconfig option @ 2012-09-03 4:41 Kent Overstreet 0 siblings, 0 replies; 42+ messages in thread From: Kent Overstreet @ 2012-09-03 4:41 UTC (permalink / raw) To: rusty, mst; +Cc: linux-kernel, virtualization CONFIG_VIRTIO isn't exposed, everything else is supposed to select it instead. Signed-off-by: Kent Overstreet <koverstreet@google.com> --- drivers/block/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index a796407..d4e1d12 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -521,7 +521,8 @@ config XEN_BLKDEV_BACKEND config VIRTIO_BLK tristate "Virtio block driver (EXPERIMENTAL)" - depends on EXPERIMENTAL && VIRTIO + select VIRTIO + 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 ^ permalink raw reply related [flat|nested] 42+ messages in thread
end of thread, other threads:[~2012-09-07 3:30 UTC | newest] Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-03 4:41 [PATCH] virtio-blk: Fix kconfig option Kent Overstreet 2012-09-03 4:46 ` Kent Overstreet 2012-09-03 4:46 ` Kent Overstreet 2012-09-04 6:23 ` Rusty Russell 2012-09-04 6:23 ` Rusty Russell 2012-09-04 23:12 ` Kent Overstreet 2012-09-04 23:12 ` Kent Overstreet 2012-09-05 4:22 ` Asias He 2012-09-05 4:22 ` Asias He 2012-09-05 5:48 ` Michael S. Tsirkin 2012-09-05 5:48 ` Michael S. Tsirkin 2012-09-05 5:54 ` Asias He 2012-09-05 5:54 ` Asias He 2012-09-05 6:04 ` Michael S. Tsirkin 2012-09-05 6:04 ` Michael S. Tsirkin 2012-09-05 6:12 ` Michael S. Tsirkin 2012-09-05 6:12 ` Michael S. Tsirkin 2012-09-06 1:46 ` Rusty Russell 2012-09-06 1:46 ` Rusty Russell 2012-09-06 7:41 ` Kent Overstreet 2012-09-06 7:41 ` Kent Overstreet 2012-09-06 8:44 ` Michael S. Tsirkin 2012-09-06 8:44 ` Michael S. Tsirkin 2012-09-06 9:25 ` Kent Overstreet 2012-09-06 9:25 ` Kent Overstreet 2012-09-06 9:49 ` Michael S. Tsirkin 2012-09-06 9:49 ` Michael S. Tsirkin 2012-09-06 10:02 ` Kent Overstreet 2012-09-06 10:02 ` Kent Overstreet 2012-09-06 10:18 ` Michael S. Tsirkin 2012-09-06 10:18 ` Michael S. Tsirkin 2012-09-06 10:31 ` Kent Overstreet 2012-09-06 10:31 ` Kent Overstreet 2012-09-06 11:09 ` Michael S. Tsirkin 2012-09-06 11:09 ` Michael S. Tsirkin 2012-09-06 23:40 ` Rusty Russell 2012-09-06 23:40 ` Rusty Russell 2012-09-07 0:25 ` Kent Overstreet 2012-09-07 0:25 ` Kent Overstreet 2012-09-07 2:57 ` Rusty Russell 2012-09-07 2:57 ` Rusty Russell 2012-09-03 4:41 Kent Overstreet
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.