[PATCH] libxc portability fixes

[PATCH] libxc portability fixes

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]


Hi!

Attached patch mainly contains portability fixes:
- use MAP_ANON, that is what both (BSD-)Unix and Linux have
- change last_error handling to use pthreads
- cleanup: No need to include <xen/sys/privcmd.h>
  a second time in xg_private.h

The patch is mainly based on a diff for NetBSD.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>



-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

[-- Attachment #2: xen_libxc.diff --]
[-- Type: text/x-diff, Size: 5535 bytes --]

diff -r a00cc97b392a tools/libxc/xc_dom_core.c
--- a/tools/libxc/xc_dom_core.c	Wed Sep 12 09:43:33 2007 +0100
+++ b/tools/libxc/xc_dom_core.c	Tue Sep 18 10:52:18 2007 +0200
@@ -122,7 +122,7 @@ void *xc_dom_malloc_page_aligned(struct 
     memset(block, 0, sizeof(*block));
     block->mmap_len = size;
     block->mmap_ptr = mmap(NULL, block->mmap_len,
-                           PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS,
+                           PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON,
                            -1, 0);
     if ( block->mmap_ptr == MAP_FAILED )
     {
@@ -354,7 +354,7 @@ void *xc_dom_pfn_to_ptr(struct xc_dom_im
     {
         mode = "anonymous memory";
         phys->ptr = mmap(NULL, phys->count << page_shift,
-                         PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS,
+                         PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON,
                          -1, 0);
         if ( phys->ptr == MAP_FAILED )
         {
diff -r a00cc97b392a tools/libxc/xc_private.c
--- a/tools/libxc/xc_private.c	Wed Sep 12 09:43:33 2007 +0100
+++ b/tools/libxc/xc_private.c	Tue Sep 18 12:06:17 2007 +0200
@@ -10,7 +10,12 @@
 #include <stdarg.h>
 #include <pthread.h>
 
-static __thread xc_error last_error = { XC_ERROR_NONE, ""};
+static pthread_key_t last_error_pkey;
+static pthread_once_t last_error_pkey_once = PTHREAD_ONCE_INIT;
+
+static pthread_key_t errbuf_pkey;
+static pthread_once_t errbuf_pkey_once = PTHREAD_ONCE_INIT;
+
 #if DEBUG
 static xc_error_handler error_handler = xc_default_error_handler;
 #else
@@ -23,15 +28,46 @@ void xc_default_error_handler(const xc_e
     fprintf(stderr, "ERROR %s: %s\n", desc, err->message);
 }
 
+static void
+_xc_clean_last_error(void *m)
+{
+	if (m)
+		free(m);
+	pthread_setspecific(last_error_pkey, NULL);
+}
+
+static void
+_xc_init_last_error(void)
+{
+		pthread_key_create(&last_error_pkey, _xc_clean_last_error);
+}
+
+static xc_error *
+_xc_get_last_error(void)
+{
+	xc_error *last_error;
+
+	pthread_once(&last_error_pkey_once, _xc_init_last_error);
+
+	last_error = pthread_getspecific(last_error_pkey);
+	if (last_error == NULL) {
+		last_error = malloc(sizeof(xc_error));
+		pthread_setspecific(last_error_pkey, last_error);
+	}
+	return last_error;
+}
+
+
 const xc_error *xc_get_last_error(void)
 {
-    return &last_error;
+    return _xc_get_last_error();
 }
 
 void xc_clear_last_error(void)
 {
-    last_error.code = XC_ERROR_NONE;
-    last_error.message[0] = '\0';
+    xc_error *last_error = _xc_get_last_error();
+    last_error->code = XC_ERROR_NONE;
+    last_error->message[0] = '\0';
 }
 
 const char *xc_error_code_to_desc(int code)
@@ -64,9 +100,10 @@ xc_error_handler xc_set_error_handler(xc
 
 static void _xc_set_error(int code, const char *msg)
 {
-    last_error.code = code;
-    strncpy(last_error.message, msg, XC_MAX_ERROR_MSG_LEN - 1);
-    last_error.message[XC_MAX_ERROR_MSG_LEN-1] = '\0';
+    xc_error *last_error = _xc_get_last_error();
+    last_error->code = code;
+    strncpy(last_error->message, msg, XC_MAX_ERROR_MSG_LEN - 1);
+    last_error->message[XC_MAX_ERROR_MSG_LEN-1] = '\0';
 }
 
 void xc_set_error(int code, const char *fmt, ...)
@@ -84,23 +121,29 @@ void xc_set_error(int code, const char *
 
     errno = saved_errno;
 
-    if ( error_handler != NULL )
-        error_handler(&last_error);
+    if ( error_handler != NULL ) {
+        xc_error *last_error = _xc_get_last_error();
+        error_handler(last_error);
+    }
 }
 
 int lock_pages(void *addr, size_t len)
 {
       int e = 0;
+      void *laddr = (void *)((u_long)addr & ~0xfffUL);
+      size_t llen = (len + 0xfffUL) & ~0xfffUL;
 #ifndef __sun__
-      e = mlock(addr, len);
+      e = mlock(laddr, llen);
 #endif
       return (e);
 }
 
 void unlock_pages(void *addr, size_t len)
 {
+    void *laddr = (void *)((u_long)addr & ~0xfffUL);
+    size_t llen = (len + 0xfffUL) & ~0xfffUL;
 #ifndef __sun__
-    safe_munlock(addr, len);
+    safe_munlock(laddr, llen);
 #endif
 }
 
@@ -466,20 +509,43 @@ unsigned long xc_make_page_below_4G(
     return new_mfn;
 }
 
+static void
+_xc_clean_errbuf(void * m)
+{
+	if (m)
+		free(m);
+	pthread_setspecific(errbuf_pkey, NULL);
+}
+	
+static void
+_xc_init_errbuf(void)
+{
+    	pthread_key_create(&errbuf_pkey, _xc_clean_errbuf);
+}
+
 char *safe_strerror(int errcode)
 {
-    static __thread char errbuf[32];
+#define XS_BUFSIZE 32
+    char *errbuf;
     static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
     char *strerror_str;
 
+    pthread_once(&errbuf_pkey_once, _xc_init_errbuf);
+
+    errbuf = pthread_getspecific(errbuf_pkey);
+    if (errbuf == NULL) {
+    	errbuf = malloc(XS_BUFSIZE);
+    	pthread_setspecific(errbuf_pkey, errbuf);
+    }
+    
     /*
      * Thread-unsafe strerror() is protected by a local mutex. We copy
      * the string to a thread-private buffer before releasing the mutex.
      */
     pthread_mutex_lock(&mutex);
     strerror_str = strerror(errcode);
-    strncpy(errbuf, strerror_str, sizeof(errbuf));
-    errbuf[sizeof(errbuf)-1] = '\0';
+    strncpy(errbuf, strerror_str, XS_BUFSIZE);
+    errbuf[XS_BUFSIZE-1] = '\0';
     pthread_mutex_unlock(&mutex);
 
     return errbuf;
diff -r a00cc97b392a tools/libxc/xg_private.h
--- a/tools/libxc/xg_private.h	Wed Sep 12 09:43:33 2007 +0100
+++ b/tools/libxc/xg_private.h	Wed Sep 19 15:00:58 2007 +0200
@@ -15,7 +15,6 @@
 #include "xenguest.h"
 #include "xc_private.h"
 
-#include <xen/sys/privcmd.h>
 #include <xen/memory.h>
 #include <xen/elfnote.h>
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] libxc portability fixes

[Vincent Hanquez]

On Wed, Sep 19, 2007 at 03:19:37PM +0200, Christoph Egger wrote:
> Attached patch mainly contains portability fixes:
> - use MAP_ANON, that is what both (BSD-)Unix and Linux have

looks like that might bite us back at some point:

man mmap:
MAP_ANON
            Synonym for MAP_ANONYMOUS.  Deprecated.

what about:

#ifndef MAP_ANONYMOUS
#define MAP_ANONYMOUS MAP_ANON
#endif

Cheers,
-- 
Vincent Hanquez

Re: [PATCH] libxc portability fixes

[Christoph Egger]

On Wednesday 19 September 2007 16:25:15 Vincent Hanquez wrote:
> On Wed, Sep 19, 2007 at 03:19:37PM +0200, Christoph Egger wrote:
> > Attached patch mainly contains portability fixes:
> > - use MAP_ANON, that is what both (BSD-)Unix and Linux have
>
> looks like that might bite us back at some point:
>
> man mmap:
> MAP_ANON
>             Synonym for MAP_ANONYMOUS.  Deprecated.

MAP_ANONYMOUS is the proprietary thing. The glibc people
should know that and should deprecate MAP_ANONYMOUS instead.
They should also document that the fd argument must be
-1 when MAP_ANON is specified to match the Unix behaviour.


Christoph

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH] libxc portability fixes

[Vincent Hanquez]

On Wed, Sep 19, 2007 at 04:32:59PM +0200, Christoph Egger wrote:
> MAP_ANONYMOUS is the proprietary thing.

which "proprietary thing" ?

> The glibc people
> should know that and should deprecate MAP_ANONYMOUS instead.

solaris has deprecated MAP_ANON as well.

> They should also document that the fd argument must be
> -1 when MAP_ANON is specified to match the Unix behaviour.

----------
MAP_ANONYMOUS

The mapping is not backed by any file; its contents are initial‐
ized to zero.  The fd and offset arguments are ignored; however,
some implementations require fd to be -1  if MAP_ANONYMOUS  (or
MAP_ANON)  is specified, and portable applications should ensure
this.  The use of MAP_ANONYMOUS in conjunction  with MAP_SHARED
is only supported on Linux since kernel 2.4.
----------

what are you missing from this ?

Cheers,
-- 
Vincent Hanquez

Re: [PATCH] libxc portability fixes

[Christoph Egger]

On Wednesday 19 September 2007 16:49:49 Vincent Hanquez wrote:
> On Wed, Sep 19, 2007 at 04:32:59PM +0200, Christoph Egger wrote:
> > MAP_ANONYMOUS is the proprietary thing.
>
> which "proprietary thing" ?
>
> > The glibc people
> > should know that and should deprecate MAP_ANONYMOUS instead.
>
> solaris has deprecated MAP_ANON as well.

Oh, SystemV Unix has MAP_ANONYMOUS?
BSD-Unix only has MAP_ANON.

> > They should also document that the fd argument must be
> > -1 when MAP_ANON is specified to match the Unix behaviour.
>
> ----------
> MAP_ANONYMOUS
>
> The mapping is not backed by any file; its contents are initial‐
> ized to zero.  The fd and offset arguments are ignored; however,
> some implementations require fd to be -1  if MAP_ANONYMOUS  (or
> MAP_ANON)  is specified, and portable applications should ensure
> this.  The use of MAP_ANONYMOUS in conjunction  with MAP_SHARED
> is only supported on Linux since kernel 2.4.
> ----------
>
> what are you missing from this ?

My linux mmap manpage says:
---------------------------
       MAP_ANONYMOUS
              The  mapping  is not backed by any file; the fd and offset argu-
              ments are ignored.  The use of this  flag  in  conjunction  with
              MAP_SHARED is only supported on Linux since kernel 2.4.

       MAP_ANON
              Alias for MAP_ANONYMOUS.  Deprecated.
---------------------------

My bsd mmap manpage says:
-------------------------------
     MAP_ANON            Map anonymous memory not associated with any specific
                         file.  The file descriptor is not used for creating
                         MAP_ANON regions, and must be specified as -1.  The
                         mapped memory will be zero filled.
-------------------------------


Christoph


-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH] libxc portability fixes

[Vincent Hanquez]

On Wed, Sep 19, 2007 at 05:04:35PM +0200, Christoph Egger wrote:
> > solaris has deprecated MAP_ANON as well.
> 
> Oh, SystemV Unix has MAP_ANONYMOUS?
> BSD-Unix only has MAP_ANON.

I've no idea, but I imagine this was deprecated for a reason on 2
very different systems. and looks like HP-UX only has MAP_ANONYMOUS.

some random google searching:
http://www.winehq.org/pipermail/wine-devel/2004-December/031636.html

classic unix mess i'ld say :(

> My linux mmap manpage says:
> ---------------------------
>        MAP_ANONYMOUS
>               The  mapping  is not backed by any file; the fd and offset argu-
>               ments are ignored.  The use of this  flag  in  conjunction  with
>               MAP_SHARED is only supported on Linux since kernel 2.4.
> 
>        MAP_ANON
>               Alias for MAP_ANONYMOUS.  Deprecated.
> ---------------------------

Yes, not good indeed.
The text i've pasted come from my linux debian manpage.
you could file a bug to your distro to upgrade the manpages ;)

Cheers,
-- 
Vincent Hanquez

Re: [PATCH] libxc portability fixes

[Keir Fraser]

I checked in the patch anyhow. If it causes problems then we can introduce
some ifdef'ery.

 - Keir

On 19/9/07 16:41, "Vincent Hanquez" <vincent@xensource.com> wrote:

> On Wed, Sep 19, 2007 at 05:04:35PM +0200, Christoph Egger wrote:
>>> solaris has deprecated MAP_ANON as well.
>> 
>> Oh, SystemV Unix has MAP_ANONYMOUS?
>> BSD-Unix only has MAP_ANON.
> 
> I've no idea, but I imagine this was deprecated for a reason on 2
> very different systems. and looks like HP-UX only has MAP_ANONYMOUS.
> 
> some random google searching:
> http://www.winehq.org/pipermail/wine-devel/2004-December/031636.html
> 
> classic unix mess i'ld say :(
> 
>> My linux mmap manpage says:
>> ---------------------------
>>        MAP_ANONYMOUS
>>               The  mapping  is not backed by any file; the fd and offset
>> argu-
>>               ments are ignored.  The use of this  flag  in  conjunction
>> with
>>               MAP_SHARED is only supported on Linux since kernel 2.4.
>> 
>>        MAP_ANON
>>               Alias for MAP_ANONYMOUS.  Deprecated.
>> ---------------------------
> 
> Yes, not good indeed.
> The text i've pasted come from my linux debian manpage.
> you could file a bug to your distro to upgrade the manpages ;)
> 
> Cheers,

Re: [PATCH] libxc portability fixes

[John Levon]

On Wed, Sep 19, 2007 at 04:49:49PM +0200, Vincent Hanquez wrote:

> > The glibc people
> > should know that and should deprecate MAP_ANONYMOUS instead.
> 
> solaris has deprecated MAP_ANON as well.

We don't even *document* MAP_ANONYMOUS. Where did you get this
impression from?

regards
john

Re: [PATCH] libxc portability fixes

[Vincent Hanquez]

On Wed, Sep 19, 2007 at 11:21:54PM +0100, John Levon wrote:
> On Wed, Sep 19, 2007 at 04:49:49PM +0200, Vincent Hanquez wrote:
> 
> > > The glibc people
> > > should know that and should deprecate MAP_ANONYMOUS instead.
> > 
> > solaris has deprecated MAP_ANON as well.
> 
> We don't even *document* MAP_ANONYMOUS. Where did you get this
> impression from?

I actually found that on 2 differents MLs (blender and wine to name them)
by some random googling to find the reason it was deprecated on linux
(which i still don't know). 

Cheers,
-- 
Vincent Hanquez

Re: [PATCH] libxc portability fixes

[John Levon]

On Thu, Sep 20, 2007 at 02:30:15PM +0200, Vincent Hanquez wrote:

> > > > The glibc people
> > > > should know that and should deprecate MAP_ANONYMOUS instead.
> > > 
> > > solaris has deprecated MAP_ANON as well.
> > 
> > We don't even *document* MAP_ANONYMOUS. Where did you get this
> > impression from?
> 
> I actually found that on 2 differents MLs (blender and wine to name them)
> by some random googling to find the reason it was deprecated on linux
> (which i still don't know). 

Appears to be hearsay... as far as I can tell, GNU libc's attempt to
deprecate it is unilateral.

regards
john