WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [RFC PATCH 33/35] Add the Xenbus sysfs and virtual devic

To: Chris Wright <chrisw@xxxxxxxxxxxx>
Subject: [Xen-devel] Re: [RFC PATCH 33/35] Add the Xenbus sysfs and virtual device hotplug driver.
From: Arjan van de Ven <arjan@xxxxxxxxxxxxx>
Date: Wed, 22 Mar 2006 09:53:20 +0100
Cc: virtualization@xxxxxxxxxxxxxx, Christian Limpach <Christian.Limpach@xxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, Ian Pratt <ian.pratt@xxxxxxxxxxxxx>
Delivery-date: Wed, 22 Mar 2006 12:34:06 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20060322063807.597300000@xxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20060322063040.960068000@xxxxxxxxxxxxxxxxxx> <20060322063807.597300000@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
> +
> +extern struct semaphore xenwatch_mutex;

you know, there is this thing called "struct mutex", please use that
instead; don't add new deprecated semaphore's when using them as mutex


> +#define streq(a, b) (strcmp((a), (b)) == 0)

ehhh why?


> +EXPORT_SYMBOL(xenbus_register_frontend);

please consider _GPL


> +static void xenbus_dev_free(struct xenbus_device *xendev)
> +{
> +     kfree(xendev);
> +}

why this wrapper ?

> +
> +/* Simplified asprintf. */
> +char *kasprintf(const char *fmt, ...)
> +{
> +     va_list ap;
> +     unsigned int len;
> +     char *p, dummy[1];
> +
> +     va_start(ap, fmt);
> +     /* FIXME: vsnprintf has a bug, NULL should work */
> +     len = vsnprintf(dummy, 0, fmt, ap);
> +     va_end(ap);
> +
> +     p = kmalloc(len + 1, GFP_KERNEL);
> +     if (!p)
> +             return NULL;
> +     va_start(ap, fmt);
> +     vsprintf(p, fmt, ap);
> +     va_end(ap);
> +     return p;
> +}

this should take a gfp mask most likely



> +
> +             /* We don't refcnt properly, so set reserved on page.
> +              * (this allocation is permanent) */
> +             SetPageReserved(virt_to_page(page));

you may want to reconsider this given the "recent" vm changes


> +     down(&xs_state.request_mutex);

please use a real mutex instead




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