On Fri, Nov 12, 2010 at 01:59:42PM -0700, Ky Srinivasan wrote:
>
>
> >>> On 11/12/2010 at 1:47 PM, in message <20101112184753.GA20893@xxxxxxxxx>, Greg
> KH <greg@xxxxxxxxx> wrote:
> > On Fri, Nov 12, 2010 at 11:06:18AM -0700, Ky Srinivasan wrote:
> >> >> +typedef struct kvp_msg {
> >> >> + __u32 kvp_key; /* Key */
> >> >> + __u8 kvp_value[0]; /* Corresponding value */
> >> >> +} kvp_msg_t;
> >> >
> >> > I thought that kvp_value was really KVP_VALUE_SIZE?
> >>
> >> kvp_value is typed information and KVP_VALUE_SIZE specifies the
> >> maximum size of the supported value. For instance if kvp_value is a
> >> string (which is the case for all the values currently supported),
> >> KVP_VALUE_SIZE specifies the maximum size of the string that will be
> >> supported.
> >
> > So it's a variable length structure? How do you konw how long the
> > structure is, does that depend on the key?
>
> kvp_value is a null terminated string. In the current implementation;
> the kernel component sends an index (key) to the user mode and
> receives the corresponding value - a string.
Why does the kernel care about the string at all?
> >> > And no typedefs, you did run your code through checkpatch.pl, right?
> >> > Why ignore the stuff it spits back at you?
> >> I will fix this.
> >> >
> >> >
> >> >> +static char *kvp_keys[KVP_MAX_KEY] = {"FullyQualifiedDomainName",
> >> >> + "IntegrationServicesVersion",
> >> >> + "NetworkAddressIPv4",
> >> >> + "NetworkAddressIPv6",
> >> >> + "OSBuildNumber",
> >> >> + "OSName",
> >> >> + "OSMajorVersion",
> >> >> + "OSMinorVersion",
> >> >> + "OSVersion",
> >> >> + "ProcessorArchitecture",
> >> >> + };
> >> >
> >> > Why list these at all, as more might come in the future, and the kernel
> >> > really doesn't care about them, right?
> >> The core HyperV KVP protocol is such that the host iterates through an
> >> index (starting with 0); the guest is free to associate a key with the
> >> index and return the associated value. The host side iteration is
> >> stopped when the guest returns a failure on an index. MSFT has
> >> specified the keys and their ordinal value in their KVP specification.
> >> The array you see above is part of that specification.
> >
> > So you match on the string name of the key? I'm confused, as I didn't
> > think I saw you matching on the string name, only the key value.
> >
> > Also, as the kernel isn't handling the key type (with one exception),
> > why even list these in the kernel at all? I'm all for documenting
> > stuff, but don't use code memory for documentation :)
>
> When we respond back to the host, we need to specify the key for
> which the value is being returned. Note that the host only iterates
> over an integer key space while the guest is expected to return both
> the key name (guest is free to associate the key name with the integer
> index) and the corresponding value.
Ah, to verify that the guest really did know what the key value that was
being used was for? Odd way of verification, but I guess it works.
> I use, the array of strings kvp_keys[] to implement the mapping
> between the integer index and the key name. Look at the function
> kvp_respond_to_host() where we lookup the kvp_keys[] array prior to
> responding back to the host.
>
> In the next iteration of this patch, I am thinking of moving index to
> key mapping code into the user-level daemon, so the kernel side will
> not have to be modified as the key-space expands (in the future).
Yes, I would recommend that so the kernel would not have to be modified
for every time the hyperv kernel is also changed :)
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
