>>> On 11/12/2010 at  1:47 PM, in message <20101112184753.ga20...@kroah.com>, 
>>> Greg
KH <g...@kroah.com> 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.
> 
>> > 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. 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).

> 
>> >> --- linux.trees.git.orig/include/linux/connector.h        2010-11-09 
>> >> 17:22:15.000000000 
>> > -0500
>> >> +++ linux.trees.git/include/linux/connector.h     2010-11-11 
>> >> 13:14:52.000000000 
>> > -0500
>> >> @@ -42,8 +42,9 @@
>> >>  #define CN_VAL_DM_USERSPACE_LOG          0x1
>> >>  #define CN_IDX_DRBD                      0x8
>> >>  #define CN_VAL_DRBD                      0x1
>> >> +#define CN_KVP_IDX                       0x9     /* MSFT KVP 
>> >> functionality */
>> > 
>> > Did you reserve this number with anyone?  Who?
>> 
>> I sent an email to   Evgeniy Polyakov  (john...@2ka.mipt). The mail
>> bounced back. I was hoping you would help me register this index.
>> Would it make sense to have a separate patch to deal with registering
>> this index?
> 
> Yes, as I can not modify non-staging code without an ack from that
> maintainer.  And I'm pretty sure that Evgeniy's address has just
> changed, use "Evgeniy Polyakov <z...@ioremap.net>" instead.
Thanks I will do just that.

K. Y
> 
> thanks,
> 
> greg k-h

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Reply via email to