| Message ID | 20170630132120.31578-4-stefanha@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote: > Format vsock hosts as "vsock:<cid>" so the addresses can be easily > distinguished from IPv4 and IPv6 addresses. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > utils/mount/network.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/utils/mount/network.c b/utils/mount/network.c > index 281e935..b5dcaa5 100644 > --- a/utils/mount/network.c > +++ b/utils/mount/network.c > @@ -45,6 +45,8 @@ > #include <rpc/pmap_prot.h> > #include <rpc/pmap_clnt.h> > > +#include <linux/vm_sockets.h> In the previous patch you had this surrounded by #ifdef AF_VSOCK I'm not keen on sprinkling a bunch ifdefs around since I think it makes the code harder to read. So my question is why is the ifdef need in the previous patch and not needed in this patch and are they needed in the previous patch? steved. > + > #include "sockaddr.h" > #include "xcommon.h" > #include "mount.h" > @@ -325,6 +327,12 @@ int nfs_string_to_sockaddr(const char *address, struct sockaddr *sap, > int nfs_present_sockaddr(const struct sockaddr *sap, const socklen_t salen, > char *buf, const size_t buflen) > { > + if (sap->sa_family == AF_VSOCK) { > + snprintf(buf, buflen, "vsock:%u", > + ((struct sockaddr_vm *)sap)->svm_cid); > + return 1; > + } > + > #ifdef HAVE_GETNAMEINFO > int result; > > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 30, 2017 at 10:40:49AM -0400, Steve Dickson wrote: > On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote: > > Format vsock hosts as "vsock:<cid>" so the addresses can be easily > > distinguished from IPv4 and IPv6 addresses. > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > utils/mount/network.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/utils/mount/network.c b/utils/mount/network.c > > index 281e935..b5dcaa5 100644 > > --- a/utils/mount/network.c > > +++ b/utils/mount/network.c > > @@ -45,6 +45,8 @@ > > #include <rpc/pmap_prot.h> > > #include <rpc/pmap_clnt.h> > > > > +#include <linux/vm_sockets.h> > In the previous patch you had this surrounded by #ifdef AF_VSOCK > I'm not keen on sprinkling a bunch ifdefs around since > I think it makes the code harder to read. So my question > is why is the ifdef need in the previous patch and > not needed in this patch and are they needed in the > previous patch? The lack of #ifdef is my mistake. My impression of nfs-utils is that the code is written to work in a variety of configurations and still support older kernels. So I am wrapping AF_VSOCK logic with an #ifdef. AF_VSOCK has been in Linux since v3.9 in commit d021c344051af91f42c5ba9fdedc176740cbd238 ("VSOCK: Introduce VM Sockets"). I'd love to eliminate the #ifdefs, but would it be acceptable to simply drop them?
On 07/03/2017 05:00 AM, Stefan Hajnoczi wrote: > On Fri, Jun 30, 2017 at 10:40:49AM -0400, Steve Dickson wrote: >> On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote: >>> Format vsock hosts as "vsock:<cid>" so the addresses can be easily >>> distinguished from IPv4 and IPv6 addresses. >>> >>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >>> --- >>> utils/mount/network.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/utils/mount/network.c b/utils/mount/network.c >>> index 281e935..b5dcaa5 100644 >>> --- a/utils/mount/network.c >>> +++ b/utils/mount/network.c >>> @@ -45,6 +45,8 @@ >>> #include <rpc/pmap_prot.h> >>> #include <rpc/pmap_clnt.h> >>> >>> +#include <linux/vm_sockets.h> >> In the previous patch you had this surrounded by #ifdef AF_VSOCK >> I'm not keen on sprinkling a bunch ifdefs around since >> I think it makes the code harder to read. So my question >> is why is the ifdef need in the previous patch and >> not needed in this patch and are they needed in the >> previous patch? > > The lack of #ifdef is my mistake. Fair enough. > > My impression of nfs-utils is that the code is written to work in a > variety of configurations and still support older kernels. So I am > wrapping AF_VSOCK logic with an #ifdef. > > AF_VSOCK has been in Linux since v3.9 in commit > d021c344051af91f42c5ba9fdedc176740cbd238 ("VSOCK: Introduce VM > Sockets"). > > I'd love to eliminate the #ifdefs, but would it be acceptable to simply > drop them? Very good question... CC-ing Felix... Would not ifdef-ing AF_VSOCK break compiling with the musl libc? Are there other implementations out there that would cause breakage? I'm pretty sure nfs-utils is only used in Linux environments, right? steved. > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Steve Dickson wrote: > > > On 07/03/2017 05:00 AM, Stefan Hajnoczi wrote: > > On Fri, Jun 30, 2017 at 10:40:49AM -0400, Steve Dickson wrote: > >> On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote: > >>> Format vsock hosts as "vsock:<cid>" so the addresses can be easily > >>> distinguished from IPv4 and IPv6 addresses. > >>> > >>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > >>> --- > >>> utils/mount/network.c | 8 ++++++++ > >>> 1 file changed, 8 insertions(+) > >>> > >>> diff --git a/utils/mount/network.c b/utils/mount/network.c > >>> index 281e935..b5dcaa5 100644 > >>> --- a/utils/mount/network.c > >>> +++ b/utils/mount/network.c > >>> @@ -45,6 +45,8 @@ > >>> #include <rpc/pmap_prot.h> > >>> #include <rpc/pmap_clnt.h> > >>> > >>> +#include <linux/vm_sockets.h> > >> In the previous patch you had this surrounded by #ifdef AF_VSOCK > >> I'm not keen on sprinkling a bunch ifdefs around since > >> I think it makes the code harder to read. So my question > >> is why is the ifdef need in the previous patch and > >> not needed in this patch and are they needed in the > >> previous patch? > > > > The lack of #ifdef is my mistake. > Fair enough. > > > > > My impression of nfs-utils is that the code is written to work in a > > variety of configurations and still support older kernels. So I am > > wrapping AF_VSOCK logic with an #ifdef. > > > > AF_VSOCK has been in Linux since v3.9 in commit > > d021c344051af91f42c5ba9fdedc176740cbd238 ("VSOCK: Introduce VM > > Sockets"). > > > > I'd love to eliminate the #ifdefs, but would it be acceptable to simply > > drop them? > Very good question... > > CC-ing Felix... Would not ifdef-ing AF_VSOCK break compiling > with the musl libc? musl has AF_VSOCK since 2013 (v0.9.12): http://git.musl-libc.org/cgit/musl/commit/?id=3d4583c3fba8989a596506619277ecd68768d9ab I doubt that many people are using a version of musl older than that. Felix > Are there other implementations out there that would cause breakage? > I'm pretty sure nfs-utils is only used in Linux environments, right? > > steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 03, 2017 at 10:00:48AM +0100, Stefan Hajnoczi wrote: > On Fri, Jun 30, 2017 at 10:40:49AM -0400, Steve Dickson wrote: > > On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote: > > > Format vsock hosts as "vsock:<cid>" so the addresses can be easily > > > distinguished from IPv4 and IPv6 addresses. > > > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > > --- > > > utils/mount/network.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/utils/mount/network.c b/utils/mount/network.c > > > index 281e935..b5dcaa5 100644 > > > --- a/utils/mount/network.c > > > +++ b/utils/mount/network.c > > > @@ -45,6 +45,8 @@ > > > #include <rpc/pmap_prot.h> > > > #include <rpc/pmap_clnt.h> > > > > > > +#include <linux/vm_sockets.h> > > In the previous patch you had this surrounded by #ifdef AF_VSOCK > > I'm not keen on sprinkling a bunch ifdefs around since > > I think it makes the code harder to read. So my question > > is why is the ifdef need in the previous patch and > > not needed in this patch and are they needed in the > > previous patch? > > The lack of #ifdef is my mistake. > > My impression of nfs-utils is that the code is written to work in a > variety of configurations and still support older kernels. So I am > wrapping AF_VSOCK logic with an #ifdef. It needs to be able to support older kernels at run-time, and I don't understand how #ifdefs would help with that. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 06, 2017 at 01:16:26PM -0400, J. Bruce Fields wrote: > On Mon, Jul 03, 2017 at 10:00:48AM +0100, Stefan Hajnoczi wrote: > > On Fri, Jun 30, 2017 at 10:40:49AM -0400, Steve Dickson wrote: > > > On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote: > > > > Format vsock hosts as "vsock:<cid>" so the addresses can be easily > > > > distinguished from IPv4 and IPv6 addresses. > > > > > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > > > --- > > > > utils/mount/network.c | 8 ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/utils/mount/network.c b/utils/mount/network.c > > > > index 281e935..b5dcaa5 100644 > > > > --- a/utils/mount/network.c > > > > +++ b/utils/mount/network.c > > > > @@ -45,6 +45,8 @@ > > > > #include <rpc/pmap_prot.h> > > > > #include <rpc/pmap_clnt.h> > > > > > > > > +#include <linux/vm_sockets.h> > > > In the previous patch you had this surrounded by #ifdef AF_VSOCK > > > I'm not keen on sprinkling a bunch ifdefs around since > > > I think it makes the code harder to read. So my question > > > is why is the ifdef need in the previous patch and > > > not needed in this patch and are they needed in the > > > previous patch? > > > > The lack of #ifdef is my mistake. > > > > My impression of nfs-utils is that the code is written to work in a > > variety of configurations and still support older kernels. So I am > > wrapping AF_VSOCK logic with an #ifdef. > > It needs to be able to support older kernels at run-time, and I don't > understand how #ifdefs would help with that. I will test the nfs-utils binaries against a kernel without AF_VSOCK support. What should happen is that an error is returned when creating a socket, adding an export, etc fails. I'll make sure the errors are graceful/meaningful. Stefan
On Mon, Jul 03, 2017 at 12:51:07PM -0400, Steve Dickson wrote: > On 07/03/2017 05:00 AM, Stefan Hajnoczi wrote: > > On Fri, Jun 30, 2017 at 10:40:49AM -0400, Steve Dickson wrote: > >> On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote: > Are there other implementations out there that would cause breakage? > I'm pretty sure nfs-utils is only used in Linux environments, right? Does nfs-utils have a set of minimum distro versions that are supported? For example, "the latest nfs-utils release is supported on RHEL 6, SLES 11, and Debian 8 jessie". If yes, then I could go check that these distros ship AF_VSOCK and <linux/vm_sockets.h>. Stefan
On 07/10/2017 02:14 PM, Stefan Hajnoczi wrote: > On Mon, Jul 03, 2017 at 12:51:07PM -0400, Steve Dickson wrote: >> On 07/03/2017 05:00 AM, Stefan Hajnoczi wrote: >>> On Fri, Jun 30, 2017 at 10:40:49AM -0400, Steve Dickson wrote: >>>> On 06/30/2017 09:21 AM, Stefan Hajnoczi wrote: >> Are there other implementations out there that would cause breakage? >> I'm pretty sure nfs-utils is only used in Linux environments, right? > > Does nfs-utils have a set of minimum distro versions that are supported? Not that I'm aware of... Now there are certain package versions nfs-utils is dependent on but not particular distro. steved. > > For example, "the latest nfs-utils release is supported on RHEL 6, SLES > 11, and Debian 8 jessie". > > If yes, then I could go check that these distros ship AF_VSOCK and > <linux/vm_sockets.h>. > > Stefan > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/utils/mount/network.c b/utils/mount/network.c index 281e935..b5dcaa5 100644 --- a/utils/mount/network.c +++ b/utils/mount/network.c @@ -45,6 +45,8 @@ #include <rpc/pmap_prot.h> #include <rpc/pmap_clnt.h> +#include <linux/vm_sockets.h> + #include "sockaddr.h" #include "xcommon.h" #include "mount.h" @@ -325,6 +327,12 @@ int nfs_string_to_sockaddr(const char *address, struct sockaddr *sap, int nfs_present_sockaddr(const struct sockaddr *sap, const socklen_t salen, char *buf, const size_t buflen) { + if (sap->sa_family == AF_VSOCK) { + snprintf(buf, buflen, "vsock:%u", + ((struct sockaddr_vm *)sap)->svm_cid); + return 1; + } + #ifdef HAVE_GETNAMEINFO int result;
Format vsock hosts as "vsock:<cid>" so the addresses can be easily distinguished from IPv4 and IPv6 addresses. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- utils/mount/network.c | 8 ++++++++ 1 file changed, 8 insertions(+)