Re: [Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command

2017-06-23 Thread Roger Pau Monné 
On Thu, Jun 22, 2017 at 11:29:44AM -0700, Stefano Stabellini wrote:
> On Thu, 22 Jun 2017, Roger Pau Monné wrote:
> > On Wed, Jun 21, 2017 at 01:16:56PM -0700, Stefano Stabellini wrote:
> > > On Tue, 20 Jun 2017, Roger Pau Monné wrote:
> > > > On Thu, Jun 15, 2017 at 12:09:36PM -0700, Stefano Stabellini wrote:
> > > > > Just reply with success to the other end for now. Delay the allocation
> > > > > of the actual socket to bind and/or connect.
> > > > > 
> > > > > Signed-off-by: Stefano Stabellini 
> > > > > CC: boris.ostrov...@oracle.com
> > > > > CC: jgr...@suse.com
> > > > > ---
> > > > >  drivers/xen/pvcalls-back.c | 27 +++
> > > > >  1 file changed, 27 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> > > > > index 437c2ad..953458b 100644
> > > > > --- a/drivers/xen/pvcalls-back.c
> > > > > +++ b/drivers/xen/pvcalls-back.c
> > > > > @@ -12,12 +12,17 @@
> > > > >   * GNU General Public License for more details.
> > > > >   */
> > > > >  
> > > > > +#include 
> > > > >  #include 
> > > > >  #include 
> > > > >  #include 
> > > > >  #include 
> > > > >  #include 
> > > > >  #include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > >  
> > > > >  #include 
> > > > >  #include 
> > > > > @@ -54,6 +59,28 @@ struct pvcalls_fedata {
> > > > >  static int pvcalls_back_socket(struct xenbus_device *dev,
> > > > >   struct xen_pvcalls_request *req)
> > > > >  {
> > > > > + struct pvcalls_fedata *fedata;
> > > > > + int ret;
> > > > > + struct xen_pvcalls_response *rsp;
> > > > > +
> > > > > + fedata = dev_get_drvdata(>dev);
> > > > > +
> > > > > + if (req->u.socket.domain != AF_INET ||
> > > > > + req->u.socket.type != SOCK_STREAM ||
> > > > > + (req->u.socket.protocol != IPPROTO_IP &&
> > > > > +  req->u.socket.protocol != AF_INET))
> > > > > + ret = -EAFNOSUPPORT;
> > > > 
> > > > Sorry for jumping into this out of the blue, but shouldn't all the
> > > > constants used above be part of the protocol? AF_INET/SOCK_STREAM/...
> > > > are all part of POSIX, but their specific value is not defined in the
> > > > standard, hence we should have XEN_AF_INET/XEN_SOCK_STREAM/... Or am I
> > > > just missing something?
> > > 
> > > The values of these constants for the pvcalls protocol are defined by
> > > docs/misc/pvcalls.markdown under "Socket families and address format".
> > > 
> > > They happen to be the same as the ones defined by Linux as AF_INET,
> > > SOCK_STREAM, etc, so in Linux I am just using those, but that is just an
> > > implementation detail internal to the Linux kernel driver. What is
> > > important from the protocol ABI perspective are the values defined by
> > > docs/misc/pvcalls.markdown.
> > 
> > Oh I see. I still think this should be part of the public pvcalls.h
> > header, and that the error codes should be the ones defined in
> > public/errno.h (or else also added to the pvcalls header).
> 
> This was done differently in the past, but now that we have a formal
> process, a person in charge of new PV drivers reviews, and design
> documents with clearly spelled out ABIs, I consider the design docs
> under docs/misc as the official specification. We don't need headers
> anymore, they are redundant. In fact, we cannot have two specifications,
> and the design docs are certainly the official ones (we don't want the
> specs to be written as header files in C). To me, the headers under
> xen/include/public/io/ are optional helpers. It doesn't matter what's in
> there, or if frontends and backends use them or not.
> 
> There is really an argument for removing those headers, because they
> might get out of sync with the spec by mistake, and in those cases, then
> we really end up with two specifications for the same protocol. I would
> be in favor of `git rm'ing all files under xen/include/public/io/ for
> which we have a complete design doc under docs/misc.

Thanks for the clarification. I agree that it would be good to remove
those headers, it's confusing and it's likely that they will get out
of sync.

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command

2017-06-22 Thread Stefano Stabellini 
On Thu, 22 Jun 2017, Andrew Cooper wrote:
> On 22/06/17 19:29, Stefano Stabellini wrote:
> > On Thu, 22 Jun 2017, Roger Pau Monné wrote:
> >> On Wed, Jun 21, 2017 at 01:16:56PM -0700, Stefano Stabellini wrote:
> >>> On Tue, 20 Jun 2017, Roger Pau Monné wrote:
>  On Thu, Jun 15, 2017 at 12:09:36PM -0700, Stefano Stabellini wrote:
> > Just reply with success to the other end for now. Delay the allocation
> > of the actual socket to bind and/or connect.
> >
> > Signed-off-by: Stefano Stabellini 
> > CC: boris.ostrov...@oracle.com
> > CC: jgr...@suse.com
> > ---
> >  drivers/xen/pvcalls-back.c | 27 +++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> > index 437c2ad..953458b 100644
> > --- a/drivers/xen/pvcalls-back.c
> > +++ b/drivers/xen/pvcalls-back.c
> > @@ -12,12 +12,17 @@
> >   * GNU General Public License for more details.
> >   */
> >  
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > @@ -54,6 +59,28 @@ struct pvcalls_fedata {
> >  static int pvcalls_back_socket(struct xenbus_device *dev,
> > struct xen_pvcalls_request *req)
> >  {
> > +   struct pvcalls_fedata *fedata;
> > +   int ret;
> > +   struct xen_pvcalls_response *rsp;
> > +
> > +   fedata = dev_get_drvdata(>dev);
> > +
> > +   if (req->u.socket.domain != AF_INET ||
> > +   req->u.socket.type != SOCK_STREAM ||
> > +   (req->u.socket.protocol != IPPROTO_IP &&
> > +req->u.socket.protocol != AF_INET))
> > +   ret = -EAFNOSUPPORT;
>  Sorry for jumping into this out of the blue, but shouldn't all the
>  constants used above be part of the protocol? AF_INET/SOCK_STREAM/...
>  are all part of POSIX, but their specific value is not defined in the
>  standard, hence we should have XEN_AF_INET/XEN_SOCK_STREAM/... Or am I
>  just missing something?
> >>> The values of these constants for the pvcalls protocol are defined by
> >>> docs/misc/pvcalls.markdown under "Socket families and address format".
> >>>
> >>> They happen to be the same as the ones defined by Linux as AF_INET,
> >>> SOCK_STREAM, etc, so in Linux I am just using those, but that is just an
> >>> implementation detail internal to the Linux kernel driver. What is
> >>> important from the protocol ABI perspective are the values defined by
> >>> docs/misc/pvcalls.markdown.
> >> Oh I see. I still think this should be part of the public pvcalls.h
> >> header, and that the error codes should be the ones defined in
> >> public/errno.h (or else also added to the pvcalls header).
> > This was done differently in the past, but now that we have a formal
> > process, a person in charge of new PV drivers reviews, and design
> > documents with clearly spelled out ABIs, I consider the design docs
> > under docs/misc as the official specification. We don't need headers
> > anymore, they are redundant. In fact, we cannot have two specifications,
> > and the design docs are certainly the official ones (we don't want the
> > specs to be written as header files in C). To me, the headers under
> > xen/include/public/io/ are optional helpers. It doesn't matter what's in
> > there, or if frontends and backends use them or not.
> >
> > There is really an argument for removing those headers, because they
> > might get out of sync with the spec by mistake, and in those cases, then
> > we really end up with two specifications for the same protocol. I would
> > be in favor of `git rm'ing all files under xen/include/public/io/ for
> > which we have a complete design doc under docs/misc.
> 
> +1.
> 
> Specifications should not be written in C.  The mess that is the net and
> block protocol ABIs are perfect examples of why.
> 
> Its fine (and indeed recommended) to provide a header file which
> describes the specified protocol, but the authoritative spec should be
> in text from.
> 
> I would really prefer if more people started using ../docs/specs/.  The
> migration v2 documents are currently lonely there...

I didn't realize we had a docs/specs. Feel free to move pvcalls and 9pfs
under there.___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command

2017-06-22 Thread Andrew Cooper 
On 22/06/17 19:29, Stefano Stabellini wrote:
> On Thu, 22 Jun 2017, Roger Pau Monné wrote:
>> On Wed, Jun 21, 2017 at 01:16:56PM -0700, Stefano Stabellini wrote:
>>> On Tue, 20 Jun 2017, Roger Pau Monné wrote:
 On Thu, Jun 15, 2017 at 12:09:36PM -0700, Stefano Stabellini wrote:
> Just reply with success to the other end for now. Delay the allocation
> of the actual socket to bind and/or connect.
>
> Signed-off-by: Stefano Stabellini 
> CC: boris.ostrov...@oracle.com
> CC: jgr...@suse.com
> ---
>  drivers/xen/pvcalls-back.c | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> index 437c2ad..953458b 100644
> --- a/drivers/xen/pvcalls-back.c
> +++ b/drivers/xen/pvcalls-back.c
> @@ -12,12 +12,17 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
>  
>  #include 
>  #include 
> @@ -54,6 +59,28 @@ struct pvcalls_fedata {
>  static int pvcalls_back_socket(struct xenbus_device *dev,
>   struct xen_pvcalls_request *req)
>  {
> + struct pvcalls_fedata *fedata;
> + int ret;
> + struct xen_pvcalls_response *rsp;
> +
> + fedata = dev_get_drvdata(>dev);
> +
> + if (req->u.socket.domain != AF_INET ||
> + req->u.socket.type != SOCK_STREAM ||
> + (req->u.socket.protocol != IPPROTO_IP &&
> +  req->u.socket.protocol != AF_INET))
> + ret = -EAFNOSUPPORT;
 Sorry for jumping into this out of the blue, but shouldn't all the
 constants used above be part of the protocol? AF_INET/SOCK_STREAM/...
 are all part of POSIX, but their specific value is not defined in the
 standard, hence we should have XEN_AF_INET/XEN_SOCK_STREAM/... Or am I
 just missing something?
>>> The values of these constants for the pvcalls protocol are defined by
>>> docs/misc/pvcalls.markdown under "Socket families and address format".
>>>
>>> They happen to be the same as the ones defined by Linux as AF_INET,
>>> SOCK_STREAM, etc, so in Linux I am just using those, but that is just an
>>> implementation detail internal to the Linux kernel driver. What is
>>> important from the protocol ABI perspective are the values defined by
>>> docs/misc/pvcalls.markdown.
>> Oh I see. I still think this should be part of the public pvcalls.h
>> header, and that the error codes should be the ones defined in
>> public/errno.h (or else also added to the pvcalls header).
> This was done differently in the past, but now that we have a formal
> process, a person in charge of new PV drivers reviews, and design
> documents with clearly spelled out ABIs, I consider the design docs
> under docs/misc as the official specification. We don't need headers
> anymore, they are redundant. In fact, we cannot have two specifications,
> and the design docs are certainly the official ones (we don't want the
> specs to be written as header files in C). To me, the headers under
> xen/include/public/io/ are optional helpers. It doesn't matter what's in
> there, or if frontends and backends use them or not.
>
> There is really an argument for removing those headers, because they
> might get out of sync with the spec by mistake, and in those cases, then
> we really end up with two specifications for the same protocol. I would
> be in favor of `git rm'ing all files under xen/include/public/io/ for
> which we have a complete design doc under docs/misc.

+1.

Specifications should not be written in C.  The mess that is the net and
block protocol ABIs are perfect examples of why.

Its fine (and indeed recommended) to provide a header file which
describes the specified protocol, but the authoritative spec should be
in text from.

I would really prefer if more people started using ../docs/specs/.  The
migration v2 documents are currently lonely there...

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command

2017-06-22 Thread Stefano Stabellini 
On Thu, 22 Jun 2017, Roger Pau Monné wrote:
> On Wed, Jun 21, 2017 at 01:16:56PM -0700, Stefano Stabellini wrote:
> > On Tue, 20 Jun 2017, Roger Pau Monné wrote:
> > > On Thu, Jun 15, 2017 at 12:09:36PM -0700, Stefano Stabellini wrote:
> > > > Just reply with success to the other end for now. Delay the allocation
> > > > of the actual socket to bind and/or connect.
> > > > 
> > > > Signed-off-by: Stefano Stabellini 
> > > > CC: boris.ostrov...@oracle.com
> > > > CC: jgr...@suse.com
> > > > ---
> > > >  drivers/xen/pvcalls-back.c | 27 +++
> > > >  1 file changed, 27 insertions(+)
> > > > 
> > > > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> > > > index 437c2ad..953458b 100644
> > > > --- a/drivers/xen/pvcalls-back.c
> > > > +++ b/drivers/xen/pvcalls-back.c
> > > > @@ -12,12 +12,17 @@
> > > >   * GNU General Public License for more details.
> > > >   */
> > > >  
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > >  
> > > >  #include 
> > > >  #include 
> > > > @@ -54,6 +59,28 @@ struct pvcalls_fedata {
> > > >  static int pvcalls_back_socket(struct xenbus_device *dev,
> > > > struct xen_pvcalls_request *req)
> > > >  {
> > > > +   struct pvcalls_fedata *fedata;
> > > > +   int ret;
> > > > +   struct xen_pvcalls_response *rsp;
> > > > +
> > > > +   fedata = dev_get_drvdata(>dev);
> > > > +
> > > > +   if (req->u.socket.domain != AF_INET ||
> > > > +   req->u.socket.type != SOCK_STREAM ||
> > > > +   (req->u.socket.protocol != IPPROTO_IP &&
> > > > +req->u.socket.protocol != AF_INET))
> > > > +   ret = -EAFNOSUPPORT;
> > > 
> > > Sorry for jumping into this out of the blue, but shouldn't all the
> > > constants used above be part of the protocol? AF_INET/SOCK_STREAM/...
> > > are all part of POSIX, but their specific value is not defined in the
> > > standard, hence we should have XEN_AF_INET/XEN_SOCK_STREAM/... Or am I
> > > just missing something?
> > 
> > The values of these constants for the pvcalls protocol are defined by
> > docs/misc/pvcalls.markdown under "Socket families and address format".
> > 
> > They happen to be the same as the ones defined by Linux as AF_INET,
> > SOCK_STREAM, etc, so in Linux I am just using those, but that is just an
> > implementation detail internal to the Linux kernel driver. What is
> > important from the protocol ABI perspective are the values defined by
> > docs/misc/pvcalls.markdown.
> 
> Oh I see. I still think this should be part of the public pvcalls.h
> header, and that the error codes should be the ones defined in
> public/errno.h (or else also added to the pvcalls header).

This was done differently in the past, but now that we have a formal
process, a person in charge of new PV drivers reviews, and design
documents with clearly spelled out ABIs, I consider the design docs
under docs/misc as the official specification. We don't need headers
anymore, they are redundant. In fact, we cannot have two specifications,
and the design docs are certainly the official ones (we don't want the
specs to be written as header files in C). To me, the headers under
xen/include/public/io/ are optional helpers. It doesn't matter what's in
there, or if frontends and backends use them or not.

There is really an argument for removing those headers, because they
might get out of sync with the spec by mistake, and in those cases, then
we really end up with two specifications for the same protocol. I would
be in favor of `git rm'ing all files under xen/include/public/io/ for
which we have a complete design doc under docs/misc.___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command

2017-06-22 Thread Roger Pau Monné 
On Wed, Jun 21, 2017 at 01:16:56PM -0700, Stefano Stabellini wrote:
> On Tue, 20 Jun 2017, Roger Pau Monné wrote:
> > On Thu, Jun 15, 2017 at 12:09:36PM -0700, Stefano Stabellini wrote:
> > > Just reply with success to the other end for now. Delay the allocation
> > > of the actual socket to bind and/or connect.
> > > 
> > > Signed-off-by: Stefano Stabellini 
> > > CC: boris.ostrov...@oracle.com
> > > CC: jgr...@suse.com
> > > ---
> > >  drivers/xen/pvcalls-back.c | 27 +++
> > >  1 file changed, 27 insertions(+)
> > > 
> > > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> > > index 437c2ad..953458b 100644
> > > --- a/drivers/xen/pvcalls-back.c
> > > +++ b/drivers/xen/pvcalls-back.c
> > > @@ -12,12 +12,17 @@
> > >   * GNU General Public License for more details.
> > >   */
> > >  
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > >  
> > >  #include 
> > >  #include 
> > > @@ -54,6 +59,28 @@ struct pvcalls_fedata {
> > >  static int pvcalls_back_socket(struct xenbus_device *dev,
> > >   struct xen_pvcalls_request *req)
> > >  {
> > > + struct pvcalls_fedata *fedata;
> > > + int ret;
> > > + struct xen_pvcalls_response *rsp;
> > > +
> > > + fedata = dev_get_drvdata(>dev);
> > > +
> > > + if (req->u.socket.domain != AF_INET ||
> > > + req->u.socket.type != SOCK_STREAM ||
> > > + (req->u.socket.protocol != IPPROTO_IP &&
> > > +  req->u.socket.protocol != AF_INET))
> > > + ret = -EAFNOSUPPORT;
> > 
> > Sorry for jumping into this out of the blue, but shouldn't all the
> > constants used above be part of the protocol? AF_INET/SOCK_STREAM/...
> > are all part of POSIX, but their specific value is not defined in the
> > standard, hence we should have XEN_AF_INET/XEN_SOCK_STREAM/... Or am I
> > just missing something?
> 
> The values of these constants for the pvcalls protocol are defined by
> docs/misc/pvcalls.markdown under "Socket families and address format".
> 
> They happen to be the same as the ones defined by Linux as AF_INET,
> SOCK_STREAM, etc, so in Linux I am just using those, but that is just an
> implementation detail internal to the Linux kernel driver. What is
> important from the protocol ABI perspective are the values defined by
> docs/misc/pvcalls.markdown.

Oh I see. I still think this should be part of the public pvcalls.h
header, and that the error codes should be the ones defined in
public/errno.h (or else also added to the pvcalls header).

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command

2017-06-21 Thread Stefano Stabellini 
On Tue, 20 Jun 2017, Roger Pau Monné wrote:
> On Thu, Jun 15, 2017 at 12:09:36PM -0700, Stefano Stabellini wrote:
> > Just reply with success to the other end for now. Delay the allocation
> > of the actual socket to bind and/or connect.
> > 
> > Signed-off-by: Stefano Stabellini 
> > CC: boris.ostrov...@oracle.com
> > CC: jgr...@suse.com
> > ---
> >  drivers/xen/pvcalls-back.c | 27 +++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> > index 437c2ad..953458b 100644
> > --- a/drivers/xen/pvcalls-back.c
> > +++ b/drivers/xen/pvcalls-back.c
> > @@ -12,12 +12,17 @@
> >   * GNU General Public License for more details.
> >   */
> >  
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > @@ -54,6 +59,28 @@ struct pvcalls_fedata {
> >  static int pvcalls_back_socket(struct xenbus_device *dev,
> > struct xen_pvcalls_request *req)
> >  {
> > +   struct pvcalls_fedata *fedata;
> > +   int ret;
> > +   struct xen_pvcalls_response *rsp;
> > +
> > +   fedata = dev_get_drvdata(>dev);
> > +
> > +   if (req->u.socket.domain != AF_INET ||
> > +   req->u.socket.type != SOCK_STREAM ||
> > +   (req->u.socket.protocol != IPPROTO_IP &&
> > +req->u.socket.protocol != AF_INET))
> > +   ret = -EAFNOSUPPORT;
> 
> Sorry for jumping into this out of the blue, but shouldn't all the
> constants used above be part of the protocol? AF_INET/SOCK_STREAM/...
> are all part of POSIX, but their specific value is not defined in the
> standard, hence we should have XEN_AF_INET/XEN_SOCK_STREAM/... Or am I
> just missing something?

The values of these constants for the pvcalls protocol are defined by
docs/misc/pvcalls.markdown under "Socket families and address format".

They happen to be the same as the ones defined by Linux as AF_INET,
SOCK_STREAM, etc, so in Linux I am just using those, but that is just an
implementation detail internal to the Linux kernel driver. What is
important from the protocol ABI perspective are the values defined by
docs/misc/pvcalls.markdown.___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command

2017-06-20 Thread Roger Pau Monné 
On Thu, Jun 15, 2017 at 12:09:36PM -0700, Stefano Stabellini wrote:
> Just reply with success to the other end for now. Delay the allocation
> of the actual socket to bind and/or connect.
> 
> Signed-off-by: Stefano Stabellini 
> CC: boris.ostrov...@oracle.com
> CC: jgr...@suse.com
> ---
>  drivers/xen/pvcalls-back.c | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> index 437c2ad..953458b 100644
> --- a/drivers/xen/pvcalls-back.c
> +++ b/drivers/xen/pvcalls-back.c
> @@ -12,12 +12,17 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
>  
>  #include 
>  #include 
> @@ -54,6 +59,28 @@ struct pvcalls_fedata {
>  static int pvcalls_back_socket(struct xenbus_device *dev,
>   struct xen_pvcalls_request *req)
>  {
> + struct pvcalls_fedata *fedata;
> + int ret;
> + struct xen_pvcalls_response *rsp;
> +
> + fedata = dev_get_drvdata(>dev);
> +
> + if (req->u.socket.domain != AF_INET ||
> + req->u.socket.type != SOCK_STREAM ||
> + (req->u.socket.protocol != IPPROTO_IP &&
> +  req->u.socket.protocol != AF_INET))
> + ret = -EAFNOSUPPORT;

Sorry for jumping into this out of the blue, but shouldn't all the
constants used above be part of the protocol? AF_INET/SOCK_STREAM/...
are all part of POSIX, but their specific value is not defined in the
standard, hence we should have XEN_AF_INET/XEN_SOCK_STREAM/... Or am I
just missing something?

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command

2017-06-20 Thread Boris Ostrovsky 
On 06/15/2017 03:09 PM, Stefano Stabellini wrote:
> Just reply with success to the other end for now. Delay the allocation
> of the actual socket to bind and/or connect.
>
> Signed-off-by: Stefano Stabellini 
> CC: boris.ostrov...@oracle.com
> CC: jgr...@suse.com

Reviewed-by: Boris Ostrovsky 



___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command

2017-06-15 Thread Stefano Stabellini 
Just reply with success to the other end for now. Delay the allocation
of the actual socket to bind and/or connect.

Signed-off-by: Stefano Stabellini 
CC: boris.ostrov...@oracle.com
CC: jgr...@suse.com
---
 drivers/xen/pvcalls-back.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index 437c2ad..953458b 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -12,12 +12,17 @@
  * GNU General Public License for more details.
  */
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
 
 #include 
 #include 
@@ -54,6 +59,28 @@ struct pvcalls_fedata {
 static int pvcalls_back_socket(struct xenbus_device *dev,
struct xen_pvcalls_request *req)
 {
+   struct pvcalls_fedata *fedata;
+   int ret;
+   struct xen_pvcalls_response *rsp;
+
+   fedata = dev_get_drvdata(>dev);
+
+   if (req->u.socket.domain != AF_INET ||
+   req->u.socket.type != SOCK_STREAM ||
+   (req->u.socket.protocol != IPPROTO_IP &&
+req->u.socket.protocol != AF_INET))
+   ret = -EAFNOSUPPORT;
+   else
+   ret = 0;
+
+   /* leave the actual socket allocation for later */
+
+   rsp = RING_GET_RESPONSE(>ring, fedata->ring.rsp_prod_pvt++);
+   rsp->req_id = req->req_id;
+   rsp->cmd = req->cmd;
+   rsp->u.socket.id = req->u.socket.id;
+   rsp->ret = ret;
+
return 0;
 }
 
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel