>>> On 11/11/2010 at 3:49 PM, in message <20101111124904.24010ee5@nehalam>, Stephen Hemminger <shemminger@xxxxxxxxxx> wrote: > On Thu, 11 Nov 2010 13:03:10 -0700 > "Ky Srinivasan" <ksrinivasan@xxxxxxxxxx> wrote: > >> +static char *kvp_keys[KVP_MAX_KEY] = {"FullyQualifiedDomainName", >> + "IntegrationServicesVersion", >> + "NetworkAddressIPv4", >> + "NetworkAddressIPv6", >> + "OSBuildNumber", >> + "OSName", >> + "OSMajorVersion", >> + "OSMinorVersion", >> + "OSVersion", >> + "ProcessorArchitecture", >> + }; > > Minor nit: > static const char *kvp_keys[KVP_MAX_KEY] = { > "FullQualifiedDomainName", Will do. > ... > > +/* > + * Global state maintained for transaction that is being processed. > + * Note that only one transaction can be active at any point in time. > + * > + * This state is set when we receive a request from the host; we > + * cleanup this state when the transaction is completed - when we respond > + * to the host with the key value. > + */ > + > +static u8 *recv_buffer; /* the receive buffer that we allocated */ > +static int recv_len; /* number of bytes received. */ > +static struct vmbus_channel *recv_channel; /*chn on which we got the > request*/ > +static u64 recv_req_id; /* request ID. */ > +static int kvp_current_index; > + > > I would put all the state variables for the transaction in one > structure, Will do. > > +static void kvp_timer_func(unsigned long __data) > +{ > + u32 key = *((u32 *)__data); > + /* > + * If the timer fires, the user-mode component has not responded; > + * process the pending transaction. > + */ > + kvp_respond_to_host(key, "Guest timed out"); > +} > > delayed_work is sometimes better for things like this, since it > runs in user context and can sleep. Although I don't need to block (sleep) in this code path, I agree with you having a full context is preferable. I will make the appropriate changes. > > + case (KVP_MAX_KEY): > + /* > + * We don't support this key > + * and any key beyond this. > + */ > + icmsghdrp->status = HV_E_FAIL; > + goto callback_done; > + > > case labels do not need parens Thank you; my next version of this patch will incorporate your feedback. Regards, K. Y _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel