Re: [PATCH]: An implementation of HyperV KVP functionality

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>>> 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.
> 
>> > 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  (Johnpol@xxxxxxxx). 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 <zbr@xxxxxxxxxxx>" instead.
Thanks I will do just that.

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

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux