RE: [PATCH 1/1] Drivers: hv: vmbus: Add additional distro specific Kconfig options

KY Srinivasan
2013-04-10T17:42:12Z

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Wednesday, April 10, 2013 12:34 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Add additional distro specific Kconfig
> options
>
> On Wed, Apr 10, 2013 at 04:12:24PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Wednesday, April 10, 2013 11:50 AM
> > > To: KY Srinivasan
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de;
> > > apw@canonical.com; jasowang@redhat.com
> > > Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Add additional distro specific
> Kconfig
> > > options
> > >
> > > On Wed, Apr 10, 2013 at 09:08:17AM -0700, K. Y. Srinivasan wrote:
> > > > The guest ID captures information about the guest and has room for
> > > > distributions to add distro specific information. Add Kconfig options
> > > > to support distro specific information to be managed easily.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > ---
> > > > drivers/hv/Kconfig | 14 ++++++++++++++
> > > > drivers/hv/hv.c | 4 +++-
> > > > drivers/hv/hyperv_vmbus.h | 2 +-
> > > > 3 files changed, 18 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> > > > index 0403b51..d2ca9c7 100644
> > > > --- a/drivers/hv/Kconfig
> > > > +++ b/drivers/hv/Kconfig
> > > > @@ -19,4 +19,18 @@ config HYPERV_BALLOON
> > > > help
> > > > Select this option to enable Hyper-V Balloon driver.
> > > >
> > > > +config HYPERV_GUEST_D1
> > > > + hex "Distro specific information"
> > > > + range 0x00 0xff
> > > > + default 0
> > > > + help
> > > > + This specifies the Distro vendor
> > >
> > > And just what is a distro supposed to set the value here to? For
> > > example, what would Arch Linux pick? Fedora? RHEL? SLES? openSUSE?
> > > You do know that there are more than 0xff different Linux distro
> > > "vendors"? Do they all just get to pick a number that they like?
> >
> > MSFT is planning to manage this space; we will assign distributions
> > unique numbers that they can use here.
>
> And how will that happen? A single line in a Kconfig file doesn't cut
> it, sorry.
>
> What's wrong with lanana.org managing it instead? That's who manages
> all of the rest of the kernel-limited resources.
>
> And what happens when you have more than 256? We already know that's
> not going to be enough numbers, why start out already with an obsolete
> range?

Greg,
You are reading more into this than there is. As it stands now (without this patch), the code
that constructs the guest OS ID has place holders for adding in some distro specific tags into
this ID. Look at the function generate_guest_id() in hyperv_vmbus.h. Currently these values are
hard-coded to 0. All I am doing here is to present a more maintainable way of customizing these.
The default values for these are currently 0 and even with this patch, the default values will continue
to be 0. However, if a distro chooses to modify these defaults, with this patch they can do so in a more
maintainable way - they could set this config parameter on a per-tree basis.

This information will potentially be used to automate the support infrastructure. We feel that with 24 bits
between the two distro specific tags, we have enough bits to differentiate the distros that may run on
our platform.

>
> > > > +config HYPERV_GUEST_D2
> > > > + hex "Additional Distro specific information"
> > > > + range 0x0000 0xffff
> > > > + default 0
> > > > + help
> > > > + Additional Distro specific kernel version information
> > >
> > > Again, what is a distro supposed to do here? What's wrong with the
> > > kernel version information that the kernel already has? Can't you just
> > > use that string?
> >
> > We already have the kernel version information. Distros can choose to
> > use this as they please. We are recommending that they use this field
> > to track some distro specific build number.
>
> That's in the kernel version already, why put it here again?
>
> Again, you need _way_ more than just a single Kconfig line for new
> configuration options where you expect distros to set a value to a
> unique thing,
>
> And lastly, you do know about /etc/os-release, the cross-distro,
> standardized way to specify distro version and build information,
> right? Why aren't you just using that instead of creating your own
> "unique" way of defining the exact same thing?
>
> Don't create a new standard just when we all finally picked one to
> follow, it makes it look like you don't think what the community decided
> on is valid, which isn't a wise decision.

As I said, we are not proposing any new standard here. KVP currently retrieves info
from /etc/os-release. The tooling on the Windows host uses the guest OS ID MSR to identify guest
operating systems running on the host . With what we currently have, we can detect that it is Linux based
on the information we use to construct the OS ID. This information also includes the kernel version number
as well. This patch merely makes it easy to add distro specific tags. It is entirely up to distros to decide if they want to
add these tags. The default values are 0 for both these fields and they continue to be 0.

Regards,

K. Y