[PATCH 1/4] libxc: prefer using privcmd character device

[PATCH 1/4] libxc: prefer using privcmd character device

[Doug Goldstein]

Prefer using the character device over the proc file if the character
device exists. This follows similar conversions of xenbus to avoid
issues with FMODE_ATOMIC_POS added in Linux 3.14 and newer.

CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
 tools/libxc/xc_linux_osdep.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/xc_linux_osdep.c b/tools/libxc/xc_linux_osdep.c
index 76c55ff..fb69770 100644
--- a/tools/libxc/xc_linux_osdep.c
+++ b/tools/libxc/xc_linux_osdep.c
@@ -46,7 +46,13 @@
 static xc_osdep_handle linux_privcmd_open(xc_interface *xch)
 {
     int flags, saved_errno;
-    int fd = open("/proc/xen/privcmd", O_RDWR);
+    int fd = open("/dev/xen/privcmd", O_RDWR); /* prefer this newer interface */
+
+    if ( fd == -1 )
+    {
+        /* Fallback to /proc/xen/privcmd */
+        fd = open("/proc/xen/privcmd", O_RDWR);
+    }
 
     if ( fd == -1 )
     {
-- 
2.4.10

[PATCH 2/4] update outdated header comment on privcmd.h

[Doug Goldstein]

The BSDs have always accessed privcmd via /dev/xen/privcmd while Linux
has used /proc/xen/privcmd but things are shifting to /dev/xen/privcmd
as well.

CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
 tools/include/xen-sys/FreeBSD/privcmd.h    | 2 +-
 tools/include/xen-sys/Linux/privcmd.h      | 2 +-
 tools/include/xen-sys/NetBSD/privcmd.h     | 2 +-
 tools/include/xen-sys/NetBSDRump/privcmd.h | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/include/xen-sys/FreeBSD/privcmd.h b/tools/include/xen-sys/FreeBSD/privcmd.h
index 0434d4d..cf1241f 100644
--- a/tools/include/xen-sys/FreeBSD/privcmd.h
+++ b/tools/include/xen-sys/FreeBSD/privcmd.h
@@ -1,7 +1,7 @@
 /******************************************************************************
  * privcmd.h
  *
- * Interface to /proc/xen/privcmd.
+ * Interface to /dev/xen/privcmd.
  *
  * Copyright (c) 2003-2005, K A Fraser
  *
diff --git a/tools/include/xen-sys/Linux/privcmd.h b/tools/include/xen-sys/Linux/privcmd.h
index 5be860a..e4e666a 100644
--- a/tools/include/xen-sys/Linux/privcmd.h
+++ b/tools/include/xen-sys/Linux/privcmd.h
@@ -1,7 +1,7 @@
 /******************************************************************************
  * privcmd.h
  * 
- * Interface to /proc/xen/privcmd.
+ * Interface to /dev/xen/privcmd.
  * 
  * Copyright (c) 2003-2005, K A Fraser
  * 
diff --git a/tools/include/xen-sys/NetBSD/privcmd.h b/tools/include/xen-sys/NetBSD/privcmd.h
index 1296b30..555bad9 100644
--- a/tools/include/xen-sys/NetBSD/privcmd.h
+++ b/tools/include/xen-sys/NetBSD/privcmd.h
@@ -30,7 +30,7 @@
 #ifndef __NetBSD_PRIVCMD_H__
 #define __NetBSD_PRIVCMD_H__
 
-/* Interface to /proc/xen/privcmd */
+/* Interface to /dev/xen/privcmd */
 
 typedef struct privcmd_hypercall
 {
diff --git a/tools/include/xen-sys/NetBSDRump/privcmd.h b/tools/include/xen-sys/NetBSDRump/privcmd.h
index 1296b30..555bad9 100644
--- a/tools/include/xen-sys/NetBSDRump/privcmd.h
+++ b/tools/include/xen-sys/NetBSDRump/privcmd.h
@@ -30,7 +30,7 @@
 #ifndef __NetBSD_PRIVCMD_H__
 #define __NetBSD_PRIVCMD_H__
 
-/* Interface to /proc/xen/privcmd */
+/* Interface to /dev/xen/privcmd */
 
 typedef struct privcmd_hypercall
 {
-- 
2.4.10

Re: [PATCH 2/4] update outdated header comment on privcmd.h

[Ian Jackson]

Doug Goldstein writes ("[PATCH 2/4] update outdated header comment on privcmd.h"):
> The BSDs have always accessed privcmd via /dev/xen/privcmd while Linux
> has used /proc/xen/privcmd but things are shifting to /dev/xen/privcmd
> as well.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH 3/4] gdbsx: prefer privcmd character device

[Doug Goldstein]

Prefer using the character device over the proc file if the character
device exists.

CC: Mukesh Rathor <mukesh.rathor@oracle.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
 tools/debugger/gdbsx/xg/xg_main.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/debugger/gdbsx/xg/xg_main.c b/tools/debugger/gdbsx/xg/xg_main.c
index 8c8a402..9364aee 100644
--- a/tools/debugger/gdbsx/xg/xg_main.c
+++ b/tools/debugger/gdbsx/xg/xg_main.c
@@ -127,9 +127,11 @@ xg_init()
     int flags, saved_errno;
 
     XGTRC("E\n");
-    if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
-        perror("Failed to open /proc/xen/privcmd\n");
-        return -1;
+    if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1) {
+        if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
+            perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd\n");
+            return -1;
+        }
     }
     /* Although we return the file handle as the 'xc handle' the API
      * does not specify / guarentee that this integer is in fact
-- 
2.4.10

Re: [PATCH 3/4] gdbsx: prefer privcmd character device

[Ian Jackson]

Doug Goldstein writes ("[PATCH 3/4] gdbsx: prefer privcmd character device"):
> Prefer using the character device over the proc file if the character
> device exists.

I think this common logic (which also appears in your 1/4 patch)
should be factored out.

Could gdbsx not use a libxc handle ?

Ian.

Re: [PATCH 3/4] gdbsx: prefer privcmd character device

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 579 bytes --]

On 11/24/15 12:53 PM, Ian Jackson wrote:
> Doug Goldstein writes ("[PATCH 3/4] gdbsx: prefer privcmd character device"):
>> Prefer using the character device over the proc file if the character
>> device exists.
> 
> I think this common logic (which also appears in your 1/4 patch)
> should be factored out.
> 
> Could gdbsx not use a libxc handle ?
> 
> Ian.
> 

I agree. I actually emailed the maintainer about this but his address
bounced so I left it as the simpler patch instead. I can make that
chance and resubmit if you'd prefer.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 3/4] gdbsx: prefer privcmd character device

[Doug Goldstein]

[-- Attachment #1.1: Type: text/plain, Size: 726 bytes --]

On 11/24/15 1:19 PM, Doug Goldstein wrote:
> On 11/24/15 12:53 PM, Ian Jackson wrote:
>> Doug Goldstein writes ("[PATCH 3/4] gdbsx: prefer privcmd character device"):
>>> Prefer using the character device over the proc file if the character
>>> device exists.
>>
>> I think this common logic (which also appears in your 1/4 patch)
>> should be factored out.
>>
>> Could gdbsx not use a libxc handle ?
>>
>> Ian.
>>
> 
> I agree. I actually emailed the maintainer about this but his address
> bounced so I left it as the simpler patch instead. I can make that
> chance and resubmit if you'd prefer.
> 

I'll just drop this out of the v2 and look at improving it in another
series.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 3/4] gdbsx: prefer privcmd character device

[Ian Jackson]

Doug Goldstein writes ("Re: [PATCH 3/4] gdbsx: prefer privcmd character device"):
> I agree. I actually emailed the maintainer about this but his address
> bounced so I left it as the simpler patch instead. I can make that
> chance and resubmit if you'd prefer.

Yes, please, thanks!

Ian.

Re: [PATCH 3/4] gdbsx: prefer privcmd character device

[Ian Campbell]

On Tue, 2015-11-24 at 18:53 +0000, Ian Jackson wrote:
> Doug Goldstein writes ("[PATCH 3/4] gdbsx: prefer privcmd character
> device"):
> > Prefer using the character device over the proc file if the character
> > device exists.
> 
> I think this common logic (which also appears in your 1/4 patch)
> should be factored out.
> 

For reasons I don't really understand gdbsc appears to avoid the use of
libxenctrl completely and has it's own h/call wrapper and such.

(Perhaps a legacy of it being out of tree?)

[PATCH 4/4] xendomains initscript: test for privcmd char device

[Doug Goldstein]

Allow the init script to continue if either the character device or the
proc file is available.

CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
 tools/hotplug/Linux/xendomains.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/hotplug/Linux/xendomains.in b/tools/hotplug/Linux/xendomains.in
index 0603842..686f061 100644
--- a/tools/hotplug/Linux/xendomains.in
+++ b/tools/hotplug/Linux/xendomains.in
@@ -45,7 +45,7 @@ fi
 
 # Correct exit code would probably be 5, but it's enough
 # if xend complains if we're not running as privileged domain
-if ! [ -e /proc/xen/privcmd ]; then
+if ! [ -e /dev/xen/privcmd ] || [ -e /proc/xen/privcmd ]; then
 	exit 0
 fi
 
-- 
2.4.10

Re: [PATCH 4/4] xendomains initscript: test for privcmd char device

[Ian Jackson]

Doug Goldstein writes ("[PATCH 4/4] xendomains initscript: test for privcmd char device"):
> Allow the init script to continue if either the character device or the
> proc file is available.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH 1/4] libxc: prefer using privcmd character device

[Ian Jackson]

Doug Goldstein writes ("[PATCH 1/4] libxc: prefer using privcmd character device"):
> Prefer using the character device over the proc file if the character
> device exists. This follows similar conversions of xenbus to avoid
> issues with FMODE_ATOMIC_POS added in Linux 3.14 and newer.

This is a fine change, but

> -    int fd = open("/proc/xen/privcmd", O_RDWR);
> +    int fd = open("/dev/xen/privcmd", O_RDWR); /* prefer this newer interface */
> +
> +    if ( fd == -1 )
> +    {

I think this should check the errno value.  I expect that any of
ENOENT, ENXIO and ENODEV should be tolerated - but others should
probaby be a fatal error.

Ian.