Virtualization - Apr 2020 - [PATCH V2] vhost: do not enable VHOST_MENU by default

On 2020/4/17 ??2:33, Michael S. Tsirkin wrote:
> On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote:
>> On 2020/4/17 ??6:55, Michael S. Tsirkin wrote:
>>> On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote:
>>>> We try to keep the defconfig untouched after decoupling
CONFIG_VHOST
>>>> out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a
>>>> ("vhost: refine vhost and vringh kconfig") by
enabling VHOST_MENU by
>>>> default. Then the defconfigs can keep enabling CONFIG_VHOST_NET
>>>> without the caring of CONFIG_VHOST.
>>>>
>>>> But this will leave a "CONFIG_VHOST_MENU=y" in all
defconfigs and even
>>>> for the ones that doesn't want vhost. So it actually shifts
the
>>>> burdens to the maintainers of all other to add
"CONFIG_VHOST_MENU is
>>>> not set". So this patch tries to enable CONFIG_VHOST
explicitly in
>>>> defconfigs that enables CONFIG_VHOST_NET and
CONFIG_VHOST_VSOCK.
>>>>
>>>> Acked-by: Christian Borntraeger<borntraeger at
de.ibm.com>  (s390)
>>>> Acked-by: Michael Ellerman<mpe at ellerman.id.au> 
(powerpc)
>>>> Cc: Thomas Bogendoerfer<tsbogend at alpha.franken.de>
>>>> Cc: Benjamin Herrenschmidt<benh at kernel.crashing.org>
>>>> Cc: Paul Mackerras<paulus at samba.org>
>>>> Cc: Michael Ellerman<mpe at ellerman.id.au>
>>>> Cc: Heiko Carstens<heiko.carstens at de.ibm.com>
>>>> Cc: Vasily Gorbik<gor at linux.ibm.com>
>>>> Cc: Christian Borntraeger<borntraeger at de.ibm.com>
>>>> Reported-by: Geert Uytterhoeven<geert at linux-m68k.org>
>>>> Signed-off-by: Jason Wang<jasowang at redhat.com>
>>> I rebased this on top of OABI fix since that
>>> seems more orgent to fix.
>>> Pushed to my vhost branch pls take a look and
>>> if possible test.
>>> Thanks!
>>
>> I test this patch by generating the defconfigs that wants vhost_net or
>> vhost_vsock. All looks fine.
>>
>> But having CONFIG_VHOST_DPN=y may end up with the similar situation
that
>> this patch want to address.
>> Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another
>> menuconfig for VHOST_RING and do something similar?
>>
>> Thanks
> Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just
> an internal variable for the OABI fix. I kept it separate
> so it's easy to revert for 5.8. Yes we could squash it into
> VHOST directly but I don't see how that changes logic at all.

Sorry for being unclear.

I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be 
left in the defconfigs. This requires the arch maintainers to add 
"CONFIG_VHOST_VDPN is not set". (Geert complains about this)

Thanks

>

On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang
wrote:
> 
> On 2020/4/17 ??2:33, Michael S. Tsirkin wrote:
> > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote:
> > > On 2020/4/17 ??6:55, Michael S. Tsirkin wrote:
> > > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote:
> > > > > We try to keep the defconfig untouched after decoupling
CONFIG_VHOST
> > > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a
> > > > > ("vhost: refine vhost and vringh kconfig") by
enabling VHOST_MENU by
> > > > > default. Then the defconfigs can keep enabling
CONFIG_VHOST_NET
> > > > > without the caring of CONFIG_VHOST.
> > > > > 
> > > > > But this will leave a "CONFIG_VHOST_MENU=y"
in all defconfigs and even
> > > > > for the ones that doesn't want vhost. So it
actually shifts the
> > > > > burdens to the maintainers of all other to add
"CONFIG_VHOST_MENU is
> > > > > not set". So this patch tries to enable
CONFIG_VHOST explicitly in
> > > > > defconfigs that enables CONFIG_VHOST_NET and
CONFIG_VHOST_VSOCK.
> > > > > 
> > > > > Acked-by: Christian Borntraeger<borntraeger at
de.ibm.com>  (s390)
> > > > > Acked-by: Michael Ellerman<mpe at ellerman.id.au>
(powerpc)
> > > > > Cc: Thomas Bogendoerfer<tsbogend at
alpha.franken.de>
> > > > > Cc: Benjamin Herrenschmidt<benh at
kernel.crashing.org>
> > > > > Cc: Paul Mackerras<paulus at samba.org>
> > > > > Cc: Michael Ellerman<mpe at ellerman.id.au>
> > > > > Cc: Heiko Carstens<heiko.carstens at de.ibm.com>
> > > > > Cc: Vasily Gorbik<gor at linux.ibm.com>
> > > > > Cc: Christian Borntraeger<borntraeger at
de.ibm.com>
> > > > > Reported-by: Geert Uytterhoeven<geert at
linux-m68k.org>
> > > > > Signed-off-by: Jason Wang<jasowang at redhat.com>
> > > > I rebased this on top of OABI fix since that
> > > > seems more orgent to fix.
> > > > Pushed to my vhost branch pls take a look and
> > > > if possible test.
> > > > Thanks!
> > > 
> > > I test this patch by generating the defconfigs that wants
vhost_net or
> > > vhost_vsock. All looks fine.
> > > 
> > > But having CONFIG_VHOST_DPN=y may end up with the similar
situation that
> > > this patch want to address.
> > > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add
another
> > > menuconfig for VHOST_RING and do something similar?
> > > 
> > > Thanks
> > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is
just
> > an internal variable for the OABI fix. I kept it separate
> > so it's easy to revert for 5.8. Yes we could squash it into
> > VHOST directly but I don't see how that changes logic at all.
> 
> 
> Sorry for being unclear.
> 
> I meant since it was enabled by default, "CONFIG_VHOST_DPN=y"
will be left
> in the defconfigs.
But who cares? That does not add any code, does it?
> This requires the arch maintainers to add
> "CONFIG_VHOST_VDPN is not set". (Geert complains about this)
> 
> Thanks
> 
> 
> >

On 2020/4/17 ??4:29, Michael S. Tsirkin wrote:
> On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote:
>> On 2020/4/17 ??2:33, Michael S. Tsirkin wrote:
>>> On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote:
>>>> On 2020/4/17 ??6:55, Michael S. Tsirkin wrote:
>>>>> On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote:
>>>>>> We try to keep the defconfig untouched after decoupling
CONFIG_VHOST
>>>>>> out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a
>>>>>> ("vhost: refine vhost and vringh kconfig") by
enabling VHOST_MENU by
>>>>>> default. Then the defconfigs can keep enabling
CONFIG_VHOST_NET
>>>>>> without the caring of CONFIG_VHOST.
>>>>>>
>>>>>> But this will leave a "CONFIG_VHOST_MENU=y"
in all defconfigs and even
>>>>>> for the ones that doesn't want vhost. So it
actually shifts the
>>>>>> burdens to the maintainers of all other to add
"CONFIG_VHOST_MENU is
>>>>>> not set". So this patch tries to enable
CONFIG_VHOST explicitly in
>>>>>> defconfigs that enables CONFIG_VHOST_NET and
CONFIG_VHOST_VSOCK.
>>>>>>
>>>>>> Acked-by: Christian Borntraeger<borntraeger at
de.ibm.com>  (s390)
>>>>>> Acked-by: Michael Ellerman<mpe at ellerman.id.au>
(powerpc)
>>>>>> Cc: Thomas Bogendoerfer<tsbogend at
alpha.franken.de>
>>>>>> Cc: Benjamin Herrenschmidt<benh at
kernel.crashing.org>
>>>>>> Cc: Paul Mackerras<paulus at samba.org>
>>>>>> Cc: Michael Ellerman<mpe at ellerman.id.au>
>>>>>> Cc: Heiko Carstens<heiko.carstens at de.ibm.com>
>>>>>> Cc: Vasily Gorbik<gor at linux.ibm.com>
>>>>>> Cc: Christian Borntraeger<borntraeger at
de.ibm.com>
>>>>>> Reported-by: Geert Uytterhoeven<geert at
linux-m68k.org>
>>>>>> Signed-off-by: Jason Wang<jasowang at redhat.com>
>>>>> I rebased this on top of OABI fix since that
>>>>> seems more orgent to fix.
>>>>> Pushed to my vhost branch pls take a look and
>>>>> if possible test.
>>>>> Thanks!
>>>> I test this patch by generating the defconfigs that wants
vhost_net or
>>>> vhost_vsock. All looks fine.
>>>>
>>>> But having CONFIG_VHOST_DPN=y may end up with the similar
situation that
>>>> this patch want to address.
>>>> Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add
another
>>>> menuconfig for VHOST_RING and do something similar?
>>>>
>>>> Thanks
>>> Sorry I don't understand. After this patch CONFIG_VHOST_DPN is
just
>>> an internal variable for the OABI fix. I kept it separate
>>> so it's easy to revert for 5.8. Yes we could squash it into
>>> VHOST directly but I don't see how that changes logic at all.
>>
>> Sorry for being unclear.
>>
>> I meant since it was enabled by default, "CONFIG_VHOST_DPN=y"
will be left
>> in the defconfigs.
> But who cares?

FYI, please see https://www.spinics.net/lists/kvm/msg212685.html

> That does not add any code, does it?

It doesn't.

Thanks

>
>> This requires the arch maintainers to add
>> "CONFIG_VHOST_VDPN is not set". (Geert complains about this)
>>
>> Thanks
>>
>>