linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: KY Srinivasan <kys@microsoft.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: "olaf@aepfle.de" <olaf@aepfle.de>,
	"thozza@redhat.com" <thozza@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dcbw@redhat.com" <dcbw@redhat.com>,
	"apw@canonical.com" <apw@canonical.com>,
	"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
	"ben@decadent.org.uk" <ben@decadent.org.uk>
Subject: RE: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state
Date: Wed, 5 Sep 2012 00:29:07 +0000	[thread overview]
Message-ID: <426367E2313C2449837CD2DE46E7EAF9309DFFBE@SN2PRD0310MB382.namprd03.prod.outlook.com> (raw)
In-Reply-To: <20120905000419.GC31997@kroah.com>



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, September 04, 2012 8:04 PM
> To: KY Srinivasan
> Cc: olaf@aepfle.de; thozza@redhat.com; linux-kernel@vger.kernel.org;
> dcbw@redhat.com; apw@canonical.com; devel@linuxdriverproject.org;
> ben@decadent.org.uk
> Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp
> state
> 
> On Tue, Sep 04, 2012 at 11:39:12PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Tuesday, September 04, 2012 6:59 PM
> > > To: KY Srinivasan
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de;
> > > apw@canonical.com; ben@decadent.org.uk; thozza@redhat.com;
> > > dcbw@redhat.com
> > > Subject: Re: [PATCH V4 05/10] Tools: hv: Add an example script to retrieve
> dhcp
> > > state
> > >
> > > On Tue, Sep 04, 2012 at 02:46:37PM -0700, K. Y. Srinivasan wrote:
> > > > To keep the KVP daemon code free of distro specific details, we invoke an
> > > > external script to retrieve the DHCP state. This is an example script that
> > > > was used to test the KVP code. This script has to be implemented in a Distro
> > > > specific fashion. For instance on distros that ship with Network Manager
> > > enabled,
> > > > this script can be based on NM APIs.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > ---
> > > >  tools/hv/hv_get_dhcp_info.sh |   25 +++++++++++++++++++++++++
> > > >  1 files changed, 25 insertions(+), 0 deletions(-)
> > > >  create mode 100755 tools/hv/hv_get_dhcp_info.sh
> > > >
> > > > diff --git a/tools/hv/hv_get_dhcp_info.sh b/tools/hv/hv_get_dhcp_info.sh
> > > > new file mode 100755
> > > > index 0000000..3de4587
> > > > --- /dev/null
> > > > +++ b/tools/hv/hv_get_dhcp_info.sh
> > > > @@ -0,0 +1,25 @@
> > > > +#!/bin/bash
> > > > +
> > > > +# This example script retrieves the DHCP state of a given interface.
> > > > +# In the interest of keeping the KVP daemon code free of distro specific
> > > > +# information; the kvp daemon code invokes this external script to gather
> > > > +# DHCP setting for the specific interface.
> > > > +#
> > > > +# Input: Name of the interface
> > > > +#
> > > > +# Output: The script prints the string "Enabled" to stdout to indicate
> > > > +#	that DHCP is enabled on the interface.
> > >
> > > What happens if DHCP is not enabled on the interface?  Shouldn't that
> > > also return something other than "success"?
> >
> > The script is expected to write "Enabled" to stdout to indicate if DHCP is
> enabled; if this
> > is not the case, implicitly we assume DHCP is not enabled (since this is a binary
> state).
> 
> It's not really "binary" given that you are expecting "Enabled" or
> nothing, right?  "Disabled" would make a bit more sense perhaps?

In the KVP daemon code, I currently only check for "Enabled" to see if
DHCP is enabled.

> 
> Having a script return "nothing at all" to show the failure of a state
> doesn't seem the wisest thing, does it?

I can certainly resubmit this patch with the change you are suggesting. This patch adds a new file
and does not impact any of the other patches that follows this. Would you want me to resubmit all of the
remaining patches starting with this one.

Regards,

K. Y 



  reply	other threads:[~2012-09-05  0:30 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-04 21:45 [PATCH V4 00/10] Tools: hv: kvp K. Y. Srinivasan
2012-09-04 21:46 ` [PATCH V4 01/10] Tools: hv: Get rid of some unused variables K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 02/10] Tools: hv: Correctly type string variables K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 03/10] Tools: hv: Add an example script to retrieve DNS entries K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 04/10] Tools: hv: Gather DNS information K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state K. Y. Srinivasan
2012-09-04 22:59     ` Greg KH
2012-09-04 23:39       ` KY Srinivasan
2012-09-05  0:04         ` Greg KH
2012-09-05  0:29           ` KY Srinivasan [this message]
2012-09-05  2:25             ` KY Srinivasan
2012-09-05 19:12               ` Greg KH
2012-09-04 21:46   ` [PATCH V4 06/10] Tools: hv: Gather DHCP information K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 07/10] Tools: hv: Add an example script to configure an interface K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 08/10] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 09/10] Tools: hv: Rename the function kvp_get_ip_address() K. Y. Srinivasan
2012-09-04 21:46   ` [PATCH V4 10/10] Tools: hv: Implement the KVP verb - KVP_OP_GET_IP_INFO K. Y. Srinivasan
2012-09-05  2:39 [PATCH V4 05/10] Tools: hv: Add an example script to retrieve dhcp state K. Y. Srinivasan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=426367E2313C2449837CD2DE46E7EAF9309DFFBE@SN2PRD0310MB382.namprd03.prod.outlook.com \
    --to=kys@microsoft.com \
    --cc=apw@canonical.com \
    --cc=ben@decadent.org.uk \
    --cc=dcbw@redhat.com \
    --cc=devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olaf@aepfle.de \
    --cc=thozza@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).