[PATCH 0 of 9 v2] libxl: add support for hotplug script calling from libxl

[PATCH 0 of 9 v2] libxl: add support for hotplug script calling from libxl

[Roger Pau Monne]

This patch series adds support for hotplug script calling directly
from libxl, instead of relying on xenbackendd. Also some patches
contain general bug fixes.

Currently Linux hotplug script call functions are empty, so Linux
continues to use udev rules to call hotplug scripts.

Patches 1, 7, 8, 9 are NetBSD specific, and basicaly pave the way for
the application of the bigger changes present in this series.

Patch 2 adds support for mounting raw image files using the vnd
device. Since NetBSD doesn't have qdisk or blktap support, the only
way to use raw images with guests is to mount the image and pass the
block device as a "PHY" backend. To check wheter an OS supports raw
images as "PHY" backends two new files are added to the project, to
avoid using #ifdefs, that contain a helper function. The file
to be included is decided during the compilation process.

Patch 3 adds a generic function to fork the current process and
execute a given file, which will be later used to execute hotplug
scripts synchronously.

Patches 4 and 5 add a new function to wait for a device to reach a
certain state, and replace wait_for_dev_destroy with this more generic
implementation. This function is also used to wait for device
initialization before calling hotplug scripts. The added function will
benefit from a rework after event support is added to libxl.

Finally patch 6 adds the calling of hotplug scripts when devices are
initializated and removed. Two new files are also added to support
hotplug scripts, since Linux and NetBSD hotplug scripts have different
call parameters. The path of the script to execute is retrieved
from xenstore. The file to include is also decided at compile
time.

Please review, Roger.

[PATCH 1 of 9 v2] xenbackendd: pass type of block device to hotplug script

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1317386335 -7200
# Node ID 23578c9942bcc8767adc4e435bb1fd1cd89f5e18
# Parent  fd3567cafe1c7ccd0ddba0ad7fb067d435e13529
xenbackendd: pass type of block device to hotplug script

Pass the type of block device to attach to the block script instead
of reading it from xenstore, since new Xen versions don't make a
difference between a block device or an image.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r fd3567cafe1c -r 23578c9942bc tools/hotplug/NetBSD/block
--- a/tools/hotplug/NetBSD/block	Tue Nov 15 14:50:18 2011 +0100
+++ b/tools/hotplug/NetBSD/block	Fri Sep 30 14:38:55 2011 +0200
@@ -19,7 +19,7 @@ error() {
 
 xpath=$1
 xstatus=$2
-xtype=$(xenstore-read "$xpath/type")
+xtype=$3
 xparams=$(xenstore-read "$xpath/params")
 
 case $xstatus in
diff -r fd3567cafe1c -r 23578c9942bc tools/xenbackendd/xenbackendd.c
--- a/tools/xenbackendd/xenbackendd.c	Tue Nov 15 14:50:18 2011 +0100
+++ b/tools/xenbackendd/xenbackendd.c	Fri Sep 30 14:38:55 2011 +0200
@@ -89,15 +89,15 @@ dodebug(const char *fmt, ...)
 }
 
 static void
-doexec(const char *cmd, const char *arg1, const char *arg2)
+doexec(const char *cmd, const char *arg1, const char *arg2, const char *arg3)
 {
-	dodebug("exec %s %s %s", cmd, arg1, arg2);
+	dodebug("exec %s %s %s %s", cmd, arg1, arg2, arg3);
 	switch(vfork()) {
 	case -1:
 		dolog(LOG_ERR, "can't vfork: %s", strerror(errno));
 		break;
 	case 0:
-		execl(cmd, cmd, arg1, arg2, NULL);
+		execl(cmd, cmd, arg1, arg2, arg3, NULL);
 		dolog(LOG_ERR, "can't exec %s: %s", cmd, strerror(errno));
 		exit(EXIT_FAILURE);
 		/* NOTREACHED */
@@ -145,11 +145,14 @@ xen_setup(void)
 int
 main(int argc, char * const argv[])
 {
+	struct stat stab;
 	char **vec;
 	unsigned int num;
 	char *s;
 	int state;
 	char *sstate;
+	char *stype;
+	char *params;
 	char *p;
 	char buf[80];
 	int type;
@@ -297,11 +300,38 @@ main(int argc, char * const argv[])
 				    strerror(errno));
 				goto next2;
 			}
-			doexec(s, vec[XS_WATCH_PATH], sstate);
+			doexec(s, vec[XS_WATCH_PATH], sstate, NULL);
 			break;
 
 		case DEVTYPE_VBD:
-			doexec(vbd_script, vec[XS_WATCH_PATH], sstate);
+			/* check if given file is a block device or a raw image */
+			snprintf(buf, sizeof(buf), "%s/params", vec[XS_WATCH_PATH]);
+			params = xs_read(xs, XBT_NULL, buf, 0);
+			if(params == NULL) {
+				dolog(LOG_ERR,
+					"Failed to read %s (%s)", buf, strerror(errno));
+				goto next2;
+			}
+			if (stat(params, &stab) < 0) {
+				dolog(LOG_ERR,
+					"Failed to get info about %s (%s)", params,
+					strerror(errno));
+				goto next3;
+			}
+			stype = NULL;
+			if (S_ISBLK(stab.st_mode))
+				stype = "phy";
+			if (S_ISREG(stab.st_mode))
+				stype = "file";
+			if (stype == NULL) {
+				dolog(LOG_ERR,
+					"Failed to attach %s (not a block device or raw image)",
+					params, strerror(errno));
+				goto next3;
+			}
+			doexec(vbd_script, vec[XS_WATCH_PATH], sstate, stype);
+next3:
+			free(params);
 			break;
 
 		default:

[PATCH 2 of 9 v2] libxl: add support for image files for NetBSD

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1317386335 -7200
# Node ID 9e8abd626484f82a95d0edc07834ae287bc9467a
# Parent  23578c9942bcc8767adc4e435bb1fd1cd89f5e18
libxl: add support for image files for NetBSD

Created a helper function to detect if the OS is capable of using
image files as phy backends. Create two OS specific files, and
changed the Makefile to choose the correct one at compile time.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/Makefile
--- a/tools/libxl/Makefile	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/Makefile	Fri Sep 30 14:38:55 2011 +0200
@@ -32,6 +32,12 @@ endif
 LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o
 LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o
 
+ifeq ($(CONFIG_NetBSD),y)
+LIBXL_OBJS-y += libxl_phybackend.o
+else
+LIBXL_OBJS-y += libxl_nophybackend.o
+endif
+
 LIBXL_LIBS += -lyajl
 
 LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_device.c	Fri Sep 30 14:38:55 2011 +0200
@@ -138,15 +138,14 @@ static int disk_try_backend(disk_try_bac
               a->disk->format == LIBXL_DISK_FORMAT_EMPTY)) {
             goto bad_format;
         }
-        if (a->disk->format != LIBXL_DISK_FORMAT_EMPTY &&
-            !S_ISBLK(a->stab.st_mode)) {
-            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
-                       " unsuitable as phys path not a block device",
-                       a->disk->vdev);
-            return 0;
-        }
 
-        return backend;
+        if (libxl__try_phy_backend(a->stab.st_mode))
+            return backend;
+
+        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
+                   " unsuitable as phys path not a block device",
+                   a->disk->vdev);
+        return 0;
 
     case LIBXL_DISK_BACKEND_TAP:
         if (!libxl__blktap_enabled(a->gc)) {
diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_internal.h	Fri Sep 30 14:38:55 2011 +0200
@@ -252,6 +252,15 @@ _hidden int libxl__device_destroy(libxl_
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
 
+/*
+ * libxl__try_phy_backend - Check if there's support for the passed
+ * type of file using the PHY backend
+ * st_mode: mode_t of the file, as returned by stat function
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+_hidden int libxl__try_phy_backend(mode_t st_mode);
+
 /* from libxl_pci */
 
 _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/libxl_nophybackend.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxl/libxl_nophybackend.c	Fri Sep 30 14:38:55 2011 +0200
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2011
+ * Author Roger Pau Monne <roger.pau@entel.upc.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+ 
+#include <sys/stat.h>
+
+#include "libxl_internal.h"
+ 
+int libxl__try_phy_backend(mode_t st_mode)
+{
+    if (!S_ISBLK(st_mode)) {
+        return 0;
+    }
+
+    return 1;
+}
diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/libxl_phybackend.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxl/libxl_phybackend.c	Fri Sep 30 14:38:55 2011 +0200
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2011
+ * Author Roger Pau Monne <roger.pau@entel.upc.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+ 
+#include <sys/stat.h>
+
+#include "libxl_internal.h"
+ 
+int libxl__try_phy_backend(mode_t st_mode)
+{
+    if (S_ISREG(st_mode) || S_ISBLK(st_mode))
+        return 1;
+
+    return 0;
+}

Re: [PATCH 2 of 9 v2] libxl: add support for image files for NetBSD

[Ian Campbell]

On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1317386335 -7200
> # Node ID 9e8abd626484f82a95d0edc07834ae287bc9467a
> # Parent  23578c9942bcc8767adc4e435bb1fd1cd89f5e18
> libxl: add support for image files for NetBSD
> 
> Created a helper function to detect if the OS is capable of using
> image files as phy backends. Create two OS specific files, and
> changed the Makefile to choose the correct one at compile time.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
> 
> diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/Makefile
> --- a/tools/libxl/Makefile	Fri Sep 30 14:38:55 2011 +0200
> +++ b/tools/libxl/Makefile	Fri Sep 30 14:38:55 2011 +0200
> @@ -32,6 +32,12 @@ endif
>  LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o
>  LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o
>  
> +ifeq ($(CONFIG_NetBSD),y)
> +LIBXL_OBJS-y += libxl_phybackend.o
> +else
> +LIBXL_OBJS-y += libxl_nophybackend.o

phy vs nophy don't really make sense to me here, since in both cases the
content relates to the phy backend.

Perhaps we need libxl_$(OS).c to contain os specific stuff?

> +endif
> +
>  LIBXL_LIBS += -lyajl
>  
>  LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
> diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c	Fri Sep 30 14:38:55 2011 +0200
> +++ b/tools/libxl/libxl_device.c	Fri Sep 30 14:38:55 2011 +0200
> @@ -138,15 +138,14 @@ static int disk_try_backend(disk_try_bac
>                a->disk->format == LIBXL_DISK_FORMAT_EMPTY)) {
>              goto bad_format;
>          }
> -        if (a->disk->format != LIBXL_DISK_FORMAT_EMPTY &&
> -            !S_ISBLK(a->stab.st_mode)) {
> -            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
> -                       " unsuitable as phys path not a block device",
> -                       a->disk->vdev);
> -            return 0;
> -        }
>  
> -        return backend;
> +        if (libxl__try_phy_backend(a->stab.st_mode))
> +            return backend;
> +
> +        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
> +                   " unsuitable as phys path not a block device",
> +                   a->disk->vdev);
> +        return 0;
>  
>      case LIBXL_DISK_BACKEND_TAP:
>          if (!libxl__blktap_enabled(a->gc)) {
> diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h	Fri Sep 30 14:38:55 2011 +0200
> +++ b/tools/libxl/libxl_internal.h	Fri Sep 30 14:38:55 2011 +0200
> @@ -252,6 +252,15 @@ _hidden int libxl__device_destroy(libxl_
>  _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
>  _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
>  
> +/*
> + * libxl__try_phy_backend - Check if there's support for the passed
> + * type of file using the PHY backend
> + * st_mode: mode_t of the file, as returned by stat function
> + *
> + * Returns 0 on success, and < 0 on error.
> + */
> +_hidden int libxl__try_phy_backend(mode_t st_mode);
> +
>  /* from libxl_pci */
>  
>  _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
> diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/libxl_nophybackend.c
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tools/libxl/libxl_nophybackend.c	Fri Sep 30 14:38:55 2011 +0200
> @@ -0,0 +1,27 @@
> +/*
> + * Copyright (C) 2011
> + * Author Roger Pau Monne <roger.pau@entel.upc.edu>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> + 
> +#include <sys/stat.h>
> +
> +#include "libxl_internal.h"
> + 
> +int libxl__try_phy_backend(mode_t st_mode)
> +{
> +    if (!S_ISBLK(st_mode)) {
> +        return 0;
> +    }
> +
> +    return 1;
> +}
> diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/libxl_phybackend.c
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tools/libxl/libxl_phybackend.c	Fri Sep 30 14:38:55 2011 +0200
> @@ -0,0 +1,26 @@
> +/*
> + * Copyright (C) 2011
> + * Author Roger Pau Monne <roger.pau@entel.upc.edu>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> + 
> +#include <sys/stat.h>
> +
> +#include "libxl_internal.h"
> + 
> +int libxl__try_phy_backend(mode_t st_mode)
> +{
> +    if (S_ISREG(st_mode) || S_ISBLK(st_mode))
> +        return 1;
> +
> +    return 0;
> +}
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 9 v2] libxl: add support for image files for NetBSD

[Roger Pau Monné]

2011/11/18 Ian Campbell <Ian.Campbell@citrix.com>:
> On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
>> # HG changeset patch
>> # User Roger Pau Monne <roger.pau@entel.upc.edu>
>> # Date 1317386335 -7200
>> # Node ID 9e8abd626484f82a95d0edc07834ae287bc9467a
>> # Parent  23578c9942bcc8767adc4e435bb1fd1cd89f5e18
>> libxl: add support for image files for NetBSD
>>
>> Created a helper function to detect if the OS is capable of using
>> image files as phy backends. Create two OS specific files, and
>> changed the Makefile to choose the correct one at compile time.
>>
>> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
>>
>> diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/Makefile
>> --- a/tools/libxl/Makefile    Fri Sep 30 14:38:55 2011 +0200
>> +++ b/tools/libxl/Makefile    Fri Sep 30 14:38:55 2011 +0200
>> @@ -32,6 +32,12 @@ endif
>>  LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o
>>  LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o
>>
>> +ifeq ($(CONFIG_NetBSD),y)
>> +LIBXL_OBJS-y += libxl_phybackend.o
>> +else
>> +LIBXL_OBJS-y += libxl_nophybackend.o
>
> phy vs nophy don't really make sense to me here, since in both cases the
> content relates to the phy backend.
>
> Perhaps we need libxl_$(OS).c to contain os specific stuff?

A libxl_$(OS).c sounds interesting, I could put hotplug and backend OS
specific code there, but I'm afraid it might get crowded and become
difficult to understand. If I don't receive any other suggestions, I
will create a libxl_netbsd.c and libxl_linux.c (and libxl_solaris.c?)
and place the hotplug and backend helper functions there.

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

Re: [PATCH 2 of 9 v2] libxl: add support for image files for NetBSD

[Ian Campbell]

On Fri, 2011-11-18 at 14:49 +0000, Roger Pau Monné wrote:
> 2011/11/18 Ian Campbell <Ian.Campbell@citrix.com>:
> > On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
> >> # HG changeset patch
> >> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> >> # Date 1317386335 -7200
> >> # Node ID 9e8abd626484f82a95d0edc07834ae287bc9467a
> >> # Parent  23578c9942bcc8767adc4e435bb1fd1cd89f5e18
> >> libxl: add support for image files for NetBSD
> >>
> >> Created a helper function to detect if the OS is capable of using
> >> image files as phy backends. Create two OS specific files, and
> >> changed the Makefile to choose the correct one at compile time.
> >>
> >> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
> >>
> >> diff -r 23578c9942bc -r 9e8abd626484 tools/libxl/Makefile
> >> --- a/tools/libxl/Makefile    Fri Sep 30 14:38:55 2011 +0200
> >> +++ b/tools/libxl/Makefile    Fri Sep 30 14:38:55 2011 +0200
> >> @@ -32,6 +32,12 @@ endif
> >>  LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o
> >>  LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o
> >>
> >> +ifeq ($(CONFIG_NetBSD),y)
> >> +LIBXL_OBJS-y += libxl_phybackend.o
> >> +else
> >> +LIBXL_OBJS-y += libxl_nophybackend.o
> >
> > phy vs nophy don't really make sense to me here, since in both cases the
> > content relates to the phy backend.
> >
> > Perhaps we need libxl_$(OS).c to contain os specific stuff?
> 
> A libxl_$(OS).c sounds interesting, I could put hotplug and backend OS
> specific code there, but I'm afraid it might get crowded and become
> difficult to understand. If I don't receive any other suggestions, I
> will create a libxl_netbsd.c and libxl_linux.c (and libxl_solaris.c?)
> and place the hotplug and backend helper functions there.

I'm really not sure what the best answer is here.

FWIW the Solaris dom0 stuff has been unmaintained for a fair while, I
don't think you need to burden yourself with it.

Ian.


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

[PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1321609614 -3600
# Node ID ec94a7e4983ad6338db20fa0777a85507b582607
# Parent  9e8abd626484f82a95d0edc07834ae287bc9467a
libxl: add libxl__forkexec function to libxl_exec

Add a new function to libxl_exec that performs a fork and executes
the passed arguments using libxl__exec.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r 9e8abd626484 -r ec94a7e4983a tools/libxl/libxl_exec.c
--- a/tools/libxl/libxl_exec.c	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_exec.c	Fri Nov 18 10:46:54 2011 +0100
@@ -450,6 +450,40 @@ int libxl__spawn_check(libxl__gc *gc, li
     return ERROR_FAIL;
 }
 
+int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
+                    int stderrfd, char **args)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int status;
+    int rc = 0;
+    pid_t pid = fork();
+
+    switch (pid) {
+    case -1:
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "fork failed\n");
+        rc = -1;
+        break;
+    case 0:
+        libxl__exec(stdinfd, stdoutfd, stderrfd, args[0], args);
+        /* libxl__exec never returns */
+    default:
+        while (waitpid(pid, &status, 0) < 0) {
+            if (errno != EINTR) {
+                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "waitpid failed\n");
+                rc = -1;
+                break;
+            }
+        }
+        if (WIFEXITED(status)) {
+            rc = WEXITSTATUS(status);
+        } else {
+            rc = -1;
+        }
+        break;
+    }
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff -r 9e8abd626484 -r ec94a7e4983a tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_internal.h	Fri Nov 18 10:46:54 2011 +0100
@@ -400,6 +400,20 @@ _hidden int libxl__spawn_check(libxl__gc
 _hidden void libxl__exec(int stdinfd, int stdoutfd, int stderrfd,
                const char *arg0, char **args); // logs errors, never returns
 
+/*
+ * libxl__forkexec - Executes a file synchronously
+ * gc: allocation pool
+ * stdinfd, stdoutfd, stderrfd: fds to pass to libxl__exec
+ * args: file to execute and arguments to pass in the following format
+ *      args[0]: file to execute
+ *      args[1]: first argument to pass to the called program
+ *      args[n]: nth argument to pass to the called program
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+_hidden int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
+                            int stderrfd, char **args);
+
 /* from xl_create */
 _hidden int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info, uint32_t *domid);
 _hidden int libxl__domain_build(libxl__gc *gc,

Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec

[Ian Campbell]

On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1321609614 -3600
> # Node ID ec94a7e4983ad6338db20fa0777a85507b582607
> # Parent  9e8abd626484f82a95d0edc07834ae287bc9467a
> libxl: add libxl__forkexec function to libxl_exec
> 
> Add a new function to libxl_exec that performs a fork and executes
> the passed arguments using libxl__exec.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
> 
> diff -r 9e8abd626484 -r ec94a7e4983a tools/libxl/libxl_exec.c
> --- a/tools/libxl/libxl_exec.c	Fri Sep 30 14:38:55 2011 +0200
> +++ b/tools/libxl/libxl_exec.c	Fri Nov 18 10:46:54 2011 +0100
> @@ -450,6 +450,40 @@ int libxl__spawn_check(libxl__gc *gc, li
>      return ERROR_FAIL;
>  }
>  
> +int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
> +                    int stderrfd, char **args)
> +{
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    int status;
> +    int rc = 0;
> +    pid_t pid = fork();
> +
> +    switch (pid) {
> +    case -1:
> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "fork failed\n");
> +        rc = -1;
> +        break;
> +    case 0:
> +        libxl__exec(stdinfd, stdoutfd, stderrfd, args[0], args);
> +        /* libxl__exec never returns */
> +    default:
> +        while (waitpid(pid, &status, 0) < 0) {
> +            if (errno != EINTR) {
> +                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "waitpid failed\n");
> +                rc = -1;
> +                break;
> +            }
> +        }
> +        if (WIFEXITED(status)) {
> +            rc = WEXITSTATUS(status);
> +        } else {
> +            rc = -1;
> +        }
> +        break;
> +    }
> +    return rc;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff -r 9e8abd626484 -r ec94a7e4983a tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h	Fri Sep 30 14:38:55 2011 +0200
> +++ b/tools/libxl/libxl_internal.h	Fri Nov 18 10:46:54 2011 +0100
> @@ -400,6 +400,20 @@ _hidden int libxl__spawn_check(libxl__gc
>  _hidden void libxl__exec(int stdinfd, int stdoutfd, int stderrfd,
>                 const char *arg0, char **args); // logs errors, never returns
>  
> +/*
> + * libxl__forkexec - Executes a file synchronously
> + * gc: allocation pool
> + * stdinfd, stdoutfd, stderrfd: fds to pass to libxl__exec
> + * args: file to execute and arguments to pass in the following format
> + *      args[0]: file to execute
> + *      args[1]: first argument to pass to the called program
> + *      args[n]: nth argument to pass to the called program

args[n] must be NULL, right?

> + *
> + * Returns 0 on success, and < 0 on error.
> + */
> +_hidden int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
> +                            int stderrfd, char **args);
> +
>  /* from xl_create */
>  _hidden int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info, uint32_t *domid);
>  _hidden int libxl__domain_build(libxl__gc *gc,
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec

[Roger Pau Monné]

2011/11/18 Ian Campbell <Ian.Campbell@citrix.com>:
> args[n] must be NULL, right?

My bad, fixed it already.

Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec

[Ian Jackson]

Roger Pau Monne writes ("[Xen-devel] [PATCH 3 of 9 v2] libxl: add libxl__forkexe> +        while (waitpid(pid, &status, 0) < 0) {
> +            if (errno != EINTR) {
> +                LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "waitpid failed\n");
> +                rc = -1;
> +                break;
> +            }
> +        }

It is good to see the exit status actually being retrieved and
checked.  But:

> +        if (WIFEXITED(status)) {
> +            rc = WEXITSTATUS(status);
> +        } else {
> +            rc = -1;
> +        }

I don't think this is the right thing to do with it.

Full and proper checking of a subprocess exit status involves:
  * checking for WIFSIGNALED too and perhaps calling strsignal
  * printing a message somewhere

I have found that the best way to deal with this is to split this up
into two parts:
   - Functions which pass back the wait status without interpreting it,
     and leaves checking it up to the caller
   - A generic function for reporting the meaning of an exit status
     to the log

It's also OK to have convenience functions which bundle these two
together, but the actual checking of an exit status involves logging.

Conveniently, I have already written the second function for you:
libxl_report_child_exitstatus :-).

Ian.

Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec

[Roger Pau Monné]

2011/11/24 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> Conveniently, I have already written the second function for you:
> libxl_report_child_exitstatus :-).

Why does the libxl_report_child_exitstatus function print
"unexpectedly exited status zero", shouldn't it print something like
"child exited successfully"? Or I'm not supposed to call it if
WIFEXITED(status) && WEXITSTATUS(status) == 0?

Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec

[Ian Jackson]

Roger Pau Monné writes ("Re: [Xen-devel] [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec"):
> 2011/11/24 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> > Conveniently, I have already written the second function for you:
> > libxl_report_child_exitstatus :-).
> 
> Why does the libxl_report_child_exitstatus function print
> "unexpectedly exited status zero", shouldn't it print something like
> "child exited successfully"? Or I'm not supposed to call it if
> WIFEXITED(status) && WEXITSTATUS(status) == 0?

You're not supposed to call it in that case, unless you weren't
expecting the process to exit.

And you can write
   WIFEXITED(status) && WEXITSTATUS(status) == 0
more simply as
   status==0
so this is quite easy to do.

Ian.

[PATCH 4 of 9 v2] libxl: introduce libxl__wait_for_device_state

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1321609740 -3600
# Node ID a77bba3717402fdf24a72d028ae0f3d482e6f427
# Parent  ec94a7e4983ad6338db20fa0777a85507b582607
libxl: introduce libxl__wait_for_device_state

This is a generic function, that waits for xs watches to reach a
certain state and then executes the passed helper function. Removed
wait_for_dev_destroy and used this new function instead. This
function will also be used by future patches that need to wait for
the initialization of devices before executing hotplug scripts.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r ec94a7e4983a -r a77bba371740 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Fri Nov 18 10:46:54 2011 +0100
+++ b/tools/libxl/libxl_device.c	Fri Nov 18 10:49:00 2011 +0100
@@ -370,7 +370,9 @@ int libxl__device_disk_dev_number(const 
  * Returns 0 if a device is removed, ERROR_* if an error
  * or timeout occurred.
  */
-static int wait_for_dev_destroy(libxl__gc *gc, struct timeval *tv)
+int libxl__wait_for_device_state(libxl__gc *gc, struct timeval *tv,
+                                 int state,
+                                 libxl__device_state_handler handler)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int nfds, rc;
@@ -395,17 +397,14 @@ start:
         default:
             l1 = xs_read_watch(ctx->xsh, &n);
             if (l1 != NULL) {
-                char *state = libxl__xs_read(gc, XBT_NULL,
+                char *sstate = libxl__xs_read(gc, XBT_NULL,
                                              l1[XS_WATCH_PATH]);
-                if (!state || atoi(state) == 6) {
-                    xs_unwatch(ctx->xsh, l1[0], l1[1]);
-                    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
-                    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
-                               "Destroyed device backend at %s",
-                               l1[XS_WATCH_TOKEN]);
-                    rc = 0;
+                if (!sstate || atoi(sstate) == state) {
+                    /* Call handler function if present */
+                    if (handler)
+                        rc = handler(gc, l1, sstate);
                 } else {
-                    /* State is not "disconnected", continue waiting... */
+                    /* State is different than expected, continue waiting... */
                     goto start;
                 }
                 free(l1);
@@ -418,6 +417,23 @@ start:
 }
 
 /*
+ * Handler function for device destruction to be passed to
+ * libxl__wait_for_device_state
+ */
+static int destroy_device(libxl__gc *gc, char **l1, char *state)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+
+    xs_unwatch(ctx->xsh, l1[0], l1[1]);
+    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
+               "Destroyed device backend at %s",
+               l1[XS_WATCH_TOKEN]);
+
+    return 0;
+}
+
+/*
  * Returns 0 (device already destroyed) or 1 (caller must
  * wait_for_dev_destroy) on success, ERROR_* on fail.
  */
@@ -458,7 +474,7 @@ retry_transaction:
         struct timeval tv;
         tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
         tv.tv_usec = 0;
-        rc = wait_for_dev_destroy(gc, &tv);
+        rc = libxl__wait_for_device_state(gc, &tv, 6, destroy_device);
         if (rc < 0) /* an error or timeout occurred, clear watches */
             xs_unwatch(ctx->xsh, state_path, be_path);
         xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
@@ -566,7 +582,7 @@ int libxl__devices_destroy(libxl__gc *gc
         tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
         tv.tv_usec = 0;
         while (n_watches > 0) {
-            if (wait_for_dev_destroy(gc, &tv) < 0) {
+            if (libxl__wait_for_device_state(gc, &tv, 6, destroy_device) < 0) {
                 /* function returned ERROR_* */
                 break;
             } else {
diff -r ec94a7e4983a -r a77bba371740 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Fri Nov 18 10:46:54 2011 +0100
+++ b/tools/libxl/libxl_internal.h	Fri Nov 18 10:49:00 2011 +0100
@@ -20,6 +20,7 @@
 #include <stdint.h>
 #include <stdarg.h>
 #include <stdlib.h>
+#include <sys/time.h>
 
 #include <xs.h>
 #include <xenctrl.h>
@@ -252,6 +253,31 @@ _hidden int libxl__device_destroy(libxl_
 _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
 _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
 
+/* Handler for the libxl__wait_for_device_state callback */
+/*
+ * libxl__device_state_handler - Handler for the libxl__wait_for_device_state
+ * gc: allocation pool
+ * l1: array containing the path and token
+ * state: string that contains the state of the device
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+typedef int libxl__device_state_handler(libxl__gc *gc, char **l1, char *state);
+
+/*
+ * libxl__wait_for_device_state - waits a given time for a device to
+ * reach a given state
+ * gc: allocation pool
+ * tv: timeval struct containing the maximum time to wait
+ * state: state to wait for
+ * handler: callback function to execute when state is reached
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+_hidden int libxl__wait_for_device_state(libxl__gc *gc, struct timeval *tv,
+                                         int state,
+                                         libxl__device_state_handler handler);
+
 /*
  * libxl__try_phy_backend - Check if there's support for the passed
  * type of file using the PHY backend

Re: [PATCH 4 of 9 v2] libxl: introduce libxl__wait_for_device_state

[Ian Campbell]

On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1321609740 -3600
> # Node ID a77bba3717402fdf24a72d028ae0f3d482e6f427
> # Parent  ec94a7e4983ad6338db20fa0777a85507b582607
> libxl: introduce libxl__wait_for_device_state
> 
> This is a generic function, that waits for xs watches to reach a
> certain state and then executes the passed helper function. Removed
> wait_for_dev_destroy and used this new function instead. This
> function will also be used by future patches that need to wait for
> the initialization of devices before executing hotplug scripts.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> diff -r ec94a7e4983a -r a77bba371740 tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c	Fri Nov 18 10:46:54 2011 +0100
> +++ b/tools/libxl/libxl_device.c	Fri Nov 18 10:49:00 2011 +0100
> @@ -370,7 +370,9 @@ int libxl__device_disk_dev_number(const 
>   * Returns 0 if a device is removed, ERROR_* if an error
>   * or timeout occurred.
>   */
> -static int wait_for_dev_destroy(libxl__gc *gc, struct timeval *tv)
> +int libxl__wait_for_device_state(libxl__gc *gc, struct timeval *tv,
> +                                 int state,
> +                                 libxl__device_state_handler handler)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
>      int nfds, rc;
> @@ -395,17 +397,14 @@ start:
>          default:
>              l1 = xs_read_watch(ctx->xsh, &n);
>              if (l1 != NULL) {
> -                char *state = libxl__xs_read(gc, XBT_NULL,
> +                char *sstate = libxl__xs_read(gc, XBT_NULL,
>                                               l1[XS_WATCH_PATH]);
> -                if (!state || atoi(state) == 6) {
> -                    xs_unwatch(ctx->xsh, l1[0], l1[1]);
> -                    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
> -                    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> -                               "Destroyed device backend at %s",
> -                               l1[XS_WATCH_TOKEN]);
> -                    rc = 0;
> +                if (!sstate || atoi(sstate) == state) {
> +                    /* Call handler function if present */
> +                    if (handler)
> +                        rc = handler(gc, l1, sstate);
>                  } else {
> -                    /* State is not "disconnected", continue waiting... */
> +                    /* State is different than expected, continue waiting... */
>                      goto start;
>                  }
>                  free(l1);
> @@ -418,6 +417,23 @@ start:
>  }
>  
>  /*
> + * Handler function for device destruction to be passed to
> + * libxl__wait_for_device_state
> + */
> +static int destroy_device(libxl__gc *gc, char **l1, char *state)
> +{
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +
> +    xs_unwatch(ctx->xsh, l1[0], l1[1]);
> +    xs_rm(ctx->xsh, XBT_NULL, l1[XS_WATCH_TOKEN]);
> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> +               "Destroyed device backend at %s",
> +               l1[XS_WATCH_TOKEN]);
> +
> +    return 0;
> +}
> +
> +/*
>   * Returns 0 (device already destroyed) or 1 (caller must
>   * wait_for_dev_destroy) on success, ERROR_* on fail.
>   */
> @@ -458,7 +474,7 @@ retry_transaction:
>          struct timeval tv;
>          tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
>          tv.tv_usec = 0;
> -        rc = wait_for_dev_destroy(gc, &tv);
> +        rc = libxl__wait_for_device_state(gc, &tv, 6, destroy_device);
>          if (rc < 0) /* an error or timeout occurred, clear watches */
>              xs_unwatch(ctx->xsh, state_path, be_path);
>          xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
> @@ -566,7 +582,7 @@ int libxl__devices_destroy(libxl__gc *gc
>          tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
>          tv.tv_usec = 0;
>          while (n_watches > 0) {
> -            if (wait_for_dev_destroy(gc, &tv) < 0) {
> +            if (libxl__wait_for_device_state(gc, &tv, 6, destroy_device) < 0) {
>                  /* function returned ERROR_* */
>                  break;
>              } else {
> diff -r ec94a7e4983a -r a77bba371740 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h	Fri Nov 18 10:46:54 2011 +0100
> +++ b/tools/libxl/libxl_internal.h	Fri Nov 18 10:49:00 2011 +0100
> @@ -20,6 +20,7 @@
>  #include <stdint.h>
>  #include <stdarg.h>
>  #include <stdlib.h>
> +#include <sys/time.h>
>  
>  #include <xs.h>
>  #include <xenctrl.h>
> @@ -252,6 +253,31 @@ _hidden int libxl__device_destroy(libxl_
>  _hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
>  _hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state);
>  
> +/* Handler for the libxl__wait_for_device_state callback */
> +/*
> + * libxl__device_state_handler - Handler for the libxl__wait_for_device_state
> + * gc: allocation pool
> + * l1: array containing the path and token
> + * state: string that contains the state of the device
> + *
> + * Returns 0 on success, and < 0 on error.
> + */
> +typedef int libxl__device_state_handler(libxl__gc *gc, char **l1, char *state);
> +
> +/*
> + * libxl__wait_for_device_state - waits a given time for a device to
> + * reach a given state
> + * gc: allocation pool
> + * tv: timeval struct containing the maximum time to wait
> + * state: state to wait for
> + * handler: callback function to execute when state is reached
> + *
> + * Returns 0 on success, and < 0 on error.
> + */
> +_hidden int libxl__wait_for_device_state(libxl__gc *gc, struct timeval *tv,
> +                                         int state,
> +                                         libxl__device_state_handler handler);
> +
>  /*
>   * libxl__try_phy_backend - Check if there's support for the passed
>   * type of file using the PHY backend
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH 5 of 9 v2] libxl: wait for devices to initialize upon addition to the domain

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1321612154 -3600
# Node ID 4ad998fddb16a299cb230ea23ba944d84adbd2bf
# Parent  a77bba3717402fdf24a72d028ae0f3d482e6f427
libxl: wait for devices to initialize upon addition to the domain.

Block waiting for devices to initialize (switch to state 2). This is
necessary because we need to call the hotplug scripts when state is
2, otherwise the execution fails. Make use of the newly introduced
function libxl__wait_for_device_state.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r a77bba371740 -r 4ad998fddb16 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Fri Nov 18 10:49:00 2011 +0100
+++ b/tools/libxl/libxl.c	Fri Nov 18 11:29:14 2011 +0100
@@ -971,6 +971,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_t *front;
     flexarray_t *back;
     char *dev;
+    char *be_path, *state_path, *state;
+    struct timeval tv;
     libxl__device device;
     int major, minor, rc;
 
@@ -1075,6 +1077,23 @@ int libxl_device_disk_add(libxl_ctx *ctx
                              libxl__xs_kvs_of_flexarray(&gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(&gc, front, front->count));
 
+    be_path = libxl__device_backend_path(&gc, &device);
+    state_path = libxl__sprintf(&gc, "%s/state", be_path);
+    state = libxl__xs_read(&gc, XBT_NULL, state_path);
+
+    if (atoi(state) != 2) {
+        xs_watch(ctx->xsh, state_path, be_path);
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        rc = libxl__wait_for_device_state(&gc, &tv, 2, NULL);
+        xs_unwatch(ctx->xsh, state_path, be_path);
+        if (rc < 0) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "unable to initialize disk device: %s\n",
+                       disk->pdev_path);
+            goto out_free;
+        }
+    }
     rc = 0;
 
 out_free:
@@ -1450,6 +1469,8 @@ int libxl_device_nic_add(libxl_ctx *ctx,
     flexarray_t *back;
     libxl__device device;
     char *dompath, **l;
+    char *be_path, *state_path, *state;
+    struct timeval tv;
     unsigned int nb, rc;
 
     front = flexarray_make(16, 1);
@@ -1518,8 +1539,25 @@ int libxl_device_nic_add(libxl_ctx *ctx,
                              libxl__xs_kvs_of_flexarray(&gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(&gc, front, front->count));
 
-    /* FIXME: wait for plug */
+    be_path = libxl__device_backend_path(&gc, &device);
+    state_path = libxl__sprintf(&gc, "%s/state", be_path);
+    state = libxl__xs_read(&gc, XBT_NULL, state_path);
+
+    if (atoi(state) != 2) {
+        xs_watch(ctx->xsh, state_path, be_path);
+        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
+        tv.tv_usec = 0;
+        rc = libxl__wait_for_device_state(&gc, &tv, 2, NULL);
+        xs_unwatch(ctx->xsh, state_path, be_path);
+        if (rc < 0) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "unable to initialize nic device: %s\n",
+                       nic->ifname);
+            goto out_free;
+        }
+    }
     rc = 0;
+
 out_free:
     flexarray_free(back);
     flexarray_free(front);

Re: [PATCH 5 of 9 v2] libxl: wait for devices to initialize upon addition to the domain

[Ian Campbell]

On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1321612154 -3600
> # Node ID 4ad998fddb16a299cb230ea23ba944d84adbd2bf
> # Parent  a77bba3717402fdf24a72d028ae0f3d482e6f427
> libxl: wait for devices to initialize upon addition to the domain.
> 
> Block waiting for devices to initialize (switch to state 2). This is
> necessary because we need to call the hotplug scripts when state is
> 2, otherwise the execution fails. Make use of the newly introduced
> function libxl__wait_for_device_state.

It would be better to use the XenbusStateFoo constants than the numbers.
This also applies to the prviosu patch, I just didn't think of it...

Otherwise this looks good.

I'm a little concerned about the synchronous wait here (since it will
serialise adding all devices). Eventually it should be possible to do
all the adds and then wait for them all but this is dependent on the
event infrastructure.

> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
> 
> diff -r a77bba371740 -r 4ad998fddb16 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Fri Nov 18 10:49:00 2011 +0100
> +++ b/tools/libxl/libxl.c	Fri Nov 18 11:29:14 2011 +0100
> @@ -971,6 +971,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
>      flexarray_t *front;
>      flexarray_t *back;
>      char *dev;
> +    char *be_path, *state_path, *state;
> +    struct timeval tv;
>      libxl__device device;
>      int major, minor, rc;
>  
> @@ -1075,6 +1077,23 @@ int libxl_device_disk_add(libxl_ctx *ctx
>                               libxl__xs_kvs_of_flexarray(&gc, back, back->count),
>                               libxl__xs_kvs_of_flexarray(&gc, front, front->count));
>  
> +    be_path = libxl__device_backend_path(&gc, &device);
> +    state_path = libxl__sprintf(&gc, "%s/state", be_path);
> +    state = libxl__xs_read(&gc, XBT_NULL, state_path);
> +
> +    if (atoi(state) != 2) {
> +        xs_watch(ctx->xsh, state_path, be_path);
> +        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
> +        tv.tv_usec = 0;
> +        rc = libxl__wait_for_device_state(&gc, &tv, 2, NULL);
> +        xs_unwatch(ctx->xsh, state_path, be_path);
> +        if (rc < 0) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                       "unable to initialize disk device: %s\n",
> +                       disk->pdev_path);
> +            goto out_free;
> +        }
> +    }
>      rc = 0;
>  
>  out_free:
> @@ -1450,6 +1469,8 @@ int libxl_device_nic_add(libxl_ctx *ctx,
>      flexarray_t *back;
>      libxl__device device;
>      char *dompath, **l;
> +    char *be_path, *state_path, *state;
> +    struct timeval tv;
>      unsigned int nb, rc;
>  
>      front = flexarray_make(16, 1);
> @@ -1518,8 +1539,25 @@ int libxl_device_nic_add(libxl_ctx *ctx,
>                               libxl__xs_kvs_of_flexarray(&gc, back, back->count),
>                               libxl__xs_kvs_of_flexarray(&gc, front, front->count));
>  
> -    /* FIXME: wait for plug */
> +    be_path = libxl__device_backend_path(&gc, &device);
> +    state_path = libxl__sprintf(&gc, "%s/state", be_path);
> +    state = libxl__xs_read(&gc, XBT_NULL, state_path);
> +
> +    if (atoi(state) != 2) {
> +        xs_watch(ctx->xsh, state_path, be_path);
> +        tv.tv_sec = LIBXL_DESTROY_TIMEOUT;
> +        tv.tv_usec = 0;
> +        rc = libxl__wait_for_device_state(&gc, &tv, 2, NULL);
> +        xs_unwatch(ctx->xsh, state_path, be_path);
> +        if (rc < 0) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                       "unable to initialize nic device: %s\n",
> +                       nic->ifname);
> +            goto out_free;
> +        }
> +    }
>      rc = 0;
> +
>  out_free:
>      flexarray_free(back);
>      flexarray_free(front);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 5 of 9 v2] libxl: wait for devices to initialize upon addition to the domain

[Roger Pau Monné]

2011/11/18 Ian Campbell <Ian.Campbell@citrix.com>:
> It would be better to use the XenbusStateFoo constants than the numbers.
> This also applies to the prviosu patch, I just didn't think of it...

Done, I had to include xen/io/xenbus.h to libxl_internal.h to be able
to use XenbusState*.

> Otherwise this looks good.
>
> I'm a little concerned about the synchronous wait here (since it will
> serialise adding all devices). Eventually it should be possible to do
> all the adds and then wait for them all but this is dependent on the
> event infrastructure.

When the OS event patch is added to libxl we will be able to call the
hotplug scripts from the event handler, so we should no longer need to
wait for devices to initialize synchronously.

[PATCH 6 of 9 v2] libxl: execute hotplug scripts directly from libxl

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1317386335 -7200
# Node ID 1d81d1c4c851c0b07696373304801df56a221e90
# Parent  4ad998fddb16a299cb230ea23ba944d84adbd2bf
libxl: execute hotplug scripts directly from libxl.

Added the necessary handlers to execute hotplug scripts when necessary
from libxl. Split NetBSD and Linux hotplug calls into two separate
files, because parameters for hotplug scripts are different. Linux
has empty functions, since the calling of hotplug scripts is still
done using udev.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/Makefile
--- a/tools/libxl/Makefile	Fri Nov 18 11:29:14 2011 +0100
+++ b/tools/libxl/Makefile	Fri Sep 30 14:38:55 2011 +0200
@@ -34,8 +34,10 @@ LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpu
 
 ifeq ($(CONFIG_NetBSD),y)
 LIBXL_OBJS-y += libxl_phybackend.o
+LIBXL_OBJS-y += hotplug_netbsd.o
 else
 LIBXL_OBJS-y += libxl_nophybackend.o
+LIBXL_OBJS-y += hotplug_linux.o
 endif
 
 LIBXL_LIBS += -lyajl
diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/hotplug_linux.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxl/hotplug_linux.c	Fri Sep 30 14:38:55 2011 +0200
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2011
+ * Author Roger Pau Monne <roger.pau@entel.upc.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_internal.h"
+
+int libxl_disk_hotplug(libxl__gc *gc, libxl__device *dev)
+{
+    return 0;
+}
+
+int libxl_nic_hotplug_connect(libxl__gc *gc, libxl__device *dev)
+{
+    return 0;
+}
diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/hotplug_netbsd.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxl/hotplug_netbsd.c	Fri Sep 30 14:38:55 2011 +0200
@@ -0,0 +1,129 @@
+/*
+ * Copyright (C) 2011
+ * Author Roger Pau Monne <roger.pau@entel.upc.edu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include <sys/stat.h>
+
+#include "libxl_internal.h"
+
+int libxl__disk_hotplug(libxl__gc *gc, libxl__device *dev)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *be_path = libxl__device_backend_path(gc, dev);
+    struct stat stab;
+    char *stype, *sstate, *script, *params;
+    char **args;
+    int nr = 0;
+    flexarray_t *f_args;
+
+    script = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", be_path, "script"));
+    if (!script) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to read script from %s",
+                   be_path);
+        return -1;
+    }
+
+    params = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", be_path, "params"));
+    if (!params)
+        return -1;
+
+    sstate = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", be_path, "state"));
+    if (!sstate) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to read state from %s",
+                   be_path);
+        return -1;
+    }
+
+    if (stat(params, &stab) < 0) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to get stat info\n");
+        return -1;
+    }
+    if (S_ISBLK(stab.st_mode))
+        stype = "phy";
+    if (S_ISREG(stab.st_mode))
+        stype = "file";
+    if (stype == NULL) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Not block or regular file");
+        return -1;
+    }
+
+    f_args = flexarray_make(5, 1);
+    if (!f_args)
+        return -1;
+
+    flexarray_set(f_args, nr++, script);
+    flexarray_set(f_args, nr++, be_path);
+    flexarray_set(f_args, nr++, sstate);
+    flexarray_set(f_args, nr++, stype);
+    flexarray_set(f_args, nr++, NULL);
+
+    args = (char **) flexarray_contents(f_args);
+
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Calling disk hotplug script: %s %s %s %s",
+               args[0], args[1], args[2], args[3]);
+    if (libxl__forkexec(gc, -1, -1, -1, args) < 0) {
+        free(args);
+        return -1;
+    }
+
+    free(args);
+    return 0;
+}
+
+int libxl__nic_hotplug(libxl__gc *gc, libxl__device *dev)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *sstate, *script;
+    char **args;
+    int nr = 0;
+    int rc = -1;
+    flexarray_t *f_args;
+
+    script = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", be_path, "script"));
+    if (!script) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to read script from %s",
+                   be_path);
+        return -1;
+    }
+
+    sstate = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s", be_path, "state"));
+    if (!sstate) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to read state from %s",
+                   be_path);
+        return -1;
+    }
+
+    f_args = flexarray_make(4, 1);
+    if (!f_args)
+        return -1;
+
+    flexarray_set(f_args, nr++, script);
+    flexarray_set(f_args, nr++, be_path);
+    flexarray_set(f_args, nr++, sstate);
+    flexarray_set(f_args, nr++, NULL);
+
+    args = (char **) flexarray_contents(f_args);
+
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Calling nic hotplug script: %s %s %s",
+               args[0], args[1], args[2]);
+    if (libxl__forkexec(gc, -1, -1, -1, args) < 0) {
+        goto out;
+    }
+    rc = 0;
+
+out:
+    free(args);
+    return rc;
+}
diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Fri Nov 18 11:29:14 2011 +0100
+++ b/tools/libxl/libxl.c	Fri Sep 30 14:38:55 2011 +0200
@@ -1018,6 +1018,11 @@ int libxl_device_disk_add(libxl_ctx *ctx
             flexarray_append(back, "params");
             flexarray_append(back, dev);
 
+            flexarray_append(back, "script");
+            flexarray_append(back, libxl__sprintf(&gc, "%s/%s",
+                                                  libxl_xen_script_dir_path(),
+                                                  "block"));
+
             assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
             break;
         case LIBXL_DISK_BACKEND_TAP:
@@ -1094,6 +1099,15 @@ int libxl_device_disk_add(libxl_ctx *ctx
             goto out_free;
         }
     }
+
+    /* Call hotplug scripts to attach device */
+    if (libxl__device_execute_hotplug(&gc, &device) < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute hotplug script for disk: %s\n",
+                   disk->pdev_path);
+        rc = -1;
+        goto out_free;
+    }
+
     rc = 0;
 
 out_free:
@@ -1556,6 +1570,15 @@ int libxl_device_nic_add(libxl_ctx *ctx,
             goto out_free;
         }
     }
+
+    /* Call hotplug scripts to attach device */
+    if (libxl__device_execute_hotplug(&gc, &device) < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unable to execute hotplug script for nic: %s\n",
+                   nic->ifname);
+        rc = -1;
+        goto out_free;
+    }
+
     rc = 0;
 
 out_free:
diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Fri Nov 18 11:29:14 2011 +0100
+++ b/tools/libxl/libxl_device.c	Fri Sep 30 14:38:55 2011 +0200
@@ -68,6 +68,24 @@ int libxl__parse_backend_path(libxl__gc 
     return libxl__device_kind_from_string(strkind, &dev->backend_kind);
 }
 
+int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev)
+{
+    int rc = 0;
+
+    switch(dev->kind) {
+    case LIBXL__DEVICE_KIND_VIF:
+        rc = libxl__nic_hotplug(gc, dev);
+        break;
+    case LIBXL__DEVICE_KIND_VBD:
+        rc = libxl__disk_hotplug(gc, dev);
+        break;
+    default:
+        break;
+    }
+
+    return rc;
+}
+
 int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
                              char **bents, char **fents)
 {
@@ -449,6 +467,7 @@ int libxl__device_remove(libxl__gc *gc, 
     if (!state)
         goto out;
     if (atoi(state) != 4) {
+        libxl__device_execute_hotplug(gc, dev);
         libxl__device_destroy_tapdisk(gc, be_path);
         xs_rm(ctx->xsh, XBT_NULL, be_path);
         goto out;
@@ -492,6 +511,12 @@ int libxl__device_destroy(libxl__gc *gc,
     char *be_path = libxl__device_backend_path(gc, dev);
     char *fe_path = libxl__device_frontend_path(gc, dev);
 
+    /* 
+     * Run hotplug scripts, which will probably not be able to
+     * execute successfully since the device may still be plugged
+     */
+    libxl__device_execute_hotplug(gc, dev);
+
     xs_rm(ctx->xsh, XBT_NULL, be_path);
     xs_rm(ctx->xsh, XBT_NULL, fe_path);
 
diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Fri Nov 18 11:29:14 2011 +0100
+++ b/tools/libxl/libxl_internal.h	Fri Sep 30 14:38:55 2011 +0200
@@ -287,6 +287,34 @@ _hidden int libxl__wait_for_device_state
  */
 _hidden int libxl__try_phy_backend(mode_t st_mode);
 
+/* hotplug functions */
+/*
+ * libxl__device_execute_hotplug - generic function to execute hotplug scripts
+ * gc: allocation pool
+ * dev: reference to the device that executes the hotplug scripts
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+_hidden int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev);
+
+/*
+ * libxl__disk_hotplug - execute hotplug script for a disk type device
+ * gc: allocation pool
+ * dev: reference to the disk device
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+_hidden int libxl__disk_hotplug(libxl__gc *gc, libxl__device *dev);
+
+/*
+ * libxl__nic_hotplug - execute hotplug script for a nic type device
+ * gc: allocation pool
+ * dev: reference to the nic device
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+_hidden int libxl__nic_hotplug(libxl__gc *gc, libxl__device *dev);
+
 /* from libxl_pci */
 
 _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);

Re: [PATCH 6 of 9 v2] libxl: execute hotplug scripts directly from libxl

[Ian Campbell]

On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1317386335 -7200
> # Node ID 1d81d1c4c851c0b07696373304801df56a221e90
> # Parent  4ad998fddb16a299cb230ea23ba944d84adbd2bf
> libxl: execute hotplug scripts directly from libxl.
> 
> Added the necessary handlers to execute hotplug scripts when necessary
> from libxl. Split NetBSD and Linux hotplug calls into two separate
> files, because parameters for hotplug scripts are different. Linux
> has empty functions, since the calling of hotplug scripts is still
> done using udev.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
> 
> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/Makefile
> --- a/tools/libxl/Makefile      Fri Nov 18 11:29:14 2011 +0100
> +++ b/tools/libxl/Makefile      Fri Sep 30 14:38:55 2011 +0200
> @@ -34,8 +34,10 @@ LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpu

Should be:

>  ifeq ($(CONFIG_NetBSD),y)
>  LIBXL_OBJS-y += libxl_phybackend.o
> +LIBXL_OBJS-y += hotplug_netbsd.o
   elsif ($(CONFIG_Linux),y)
>  LIBXL_OBJS-y += libxl_nophybackend.o
> +LIBXL_OBJS-y += hotplug_linux.o
   else
   $(error A message of some sort)
>  endif



> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c        Fri Nov 18 11:29:14 2011 +0100
> +++ b/tools/libxl/libxl_device.c        Fri Sep 30 14:38:55 2011 +0200
> @@ -68,6 +68,24 @@ int libxl__parse_backend_path(libxl__gc
>      return libxl__device_kind_from_string(strkind, &dev->backend_kind);
>  }
> 
> +int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev)

No need for a add/remove type parameter?

> +{
> +    int rc = 0;
> +
> +    switch(dev->kind) {
> +    case LIBXL__DEVICE_KIND_VIF:
> +        rc = libxl__nic_hotplug(gc, dev);
> +        break;
> +    case LIBXL__DEVICE_KIND_VBD:
> +        rc = libxl__disk_hotplug(gc, dev);
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    return rc;
> +}
> +
>  int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
>                               char **bents, char **fents)
>  {
> @@ -449,6 +467,7 @@ int libxl__device_remove(libxl__gc *gc,
>      if (!state)
>          goto out;
>      if (atoi(state) != 4) {
> +        libxl__device_execute_hotplug(gc, dev);
>          libxl__device_destroy_tapdisk(gc, be_path);
>          xs_rm(ctx->xsh, XBT_NULL, be_path);
>          goto out;
> @@ -492,6 +511,12 @@ int libxl__device_destroy(libxl__gc *gc,
>      char *be_path = libxl__device_backend_path(gc, dev);
>      char *fe_path = libxl__device_frontend_path(gc, dev);
> 
> +    /*
> +     * Run hotplug scripts, which will probably not be able to
> +     * execute successfully since the device may still be plugged

What does this mean? If we don't expect it to work why are we calling
them?

> +     */
> +    libxl__device_execute_hotplug(gc, dev);
> +
>      xs_rm(ctx->xsh, XBT_NULL, be_path);
>      xs_rm(ctx->xsh, XBT_NULL, fe_path);
> 
> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h      Fri Nov 18 11:29:14 2011 +0100
> +++ b/tools/libxl/libxl_internal.h      Fri Sep 30 14:38:55 2011 +0200
> @@ -287,6 +287,34 @@ _hidden int libxl__wait_for_device_state
>   */
>  _hidden int libxl__try_phy_backend(mode_t st_mode);
> 
> +/* hotplug functions */
> +/*
> + * libxl__device_execute_hotplug - generic function to execute hotplug scripts
> + * gc: allocation pool
> + * dev: reference to the device that executes the hotplug scripts
> + *
> + * Returns 0 on success, and < 0 on error.
> + */
> +_hidden int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev);
> +
> +/*
> + * libxl__disk_hotplug - execute hotplug script for a disk type device
> + * gc: allocation pool
> + * dev: reference to the disk device
> + *
> + * Returns 0 on success, and < 0 on error.
> + */
> +_hidden int libxl__disk_hotplug(libxl__gc *gc, libxl__device *dev);
> +
> +/*
> + * libxl__nic_hotplug - execute hotplug script for a nic type device
> + * gc: allocation pool
> + * dev: reference to the nic device
> + *
> + * Returns 0 on success, and < 0 on error.
> + */
> +_hidden int libxl__nic_hotplug(libxl__gc *gc, libxl__device *dev);
> +
>  /* from libxl_pci */
> 
>  _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 6 of 9 v2] libxl: execute hotplug scripts directly from libxl

[Roger Pau Monné]

2011/11/18 Ian Campbell <Ian.Campbell@citrix.com>:
> On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
>> # HG changeset patch
>> # User Roger Pau Monne <roger.pau@entel.upc.edu>
>> # Date 1317386335 -7200
>> # Node ID 1d81d1c4c851c0b07696373304801df56a221e90
>> # Parent  4ad998fddb16a299cb230ea23ba944d84adbd2bf
>> libxl: execute hotplug scripts directly from libxl.
>>
>> Added the necessary handlers to execute hotplug scripts when necessary
>> from libxl. Split NetBSD and Linux hotplug calls into two separate
>> files, because parameters for hotplug scripts are different. Linux
>> has empty functions, since the calling of hotplug scripts is still
>> done using udev.
>>
>> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
>>
>> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/Makefile
>> --- a/tools/libxl/Makefile      Fri Nov 18 11:29:14 2011 +0100
>> +++ b/tools/libxl/Makefile      Fri Sep 30 14:38:55 2011 +0200
>> @@ -34,8 +34,10 @@ LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpu
>
> Should be:
>
>>  ifeq ($(CONFIG_NetBSD),y)
>>  LIBXL_OBJS-y += libxl_phybackend.o
>> +LIBXL_OBJS-y += hotplug_netbsd.o
>   elsif ($(CONFIG_Linux),y)
>>  LIBXL_OBJS-y += libxl_nophybackend.o
>> +LIBXL_OBJS-y += hotplug_linux.o
>   else
>   $(error A message of some sort)
>>  endif

Done, I've moved PHY backend selection and hotplug specific functions
to libxl_netbsd.c and libxl_linux.c, and added an error message in
case someone tries to use a different OS.

>> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_device.c
>> --- a/tools/libxl/libxl_device.c        Fri Nov 18 11:29:14 2011 +0100
>> +++ b/tools/libxl/libxl_device.c        Fri Sep 30 14:38:55 2011 +0200
>> @@ -68,6 +68,24 @@ int libxl__parse_backend_path(libxl__gc
>>      return libxl__device_kind_from_string(strkind, &dev->backend_kind);
>>  }
>>
>> +int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev)
>
> No need for a add/remove type parameter?

Although you could pass this kind of parameter, the action to perform
is based on the state of the device in the corresponding xenstore
entry. State 2 means that the device should be connected, state 6
means the device should be disconnected.

>> +{
>> +    int rc = 0;
>> +
>> +    switch(dev->kind) {
>> +    case LIBXL__DEVICE_KIND_VIF:
>> +        rc = libxl__nic_hotplug(gc, dev);
>> +        break;
>> +    case LIBXL__DEVICE_KIND_VBD:
>> +        rc = libxl__disk_hotplug(gc, dev);
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>>  int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
>>                               char **bents, char **fents)
>>  {
>> @@ -449,6 +467,7 @@ int libxl__device_remove(libxl__gc *gc,
>>      if (!state)
>>          goto out;
>>      if (atoi(state) != 4) {
>> +        libxl__device_execute_hotplug(gc, dev);
>>          libxl__device_destroy_tapdisk(gc, be_path);
>>          xs_rm(ctx->xsh, XBT_NULL, be_path);
>>          goto out;
>> @@ -492,6 +511,12 @@ int libxl__device_destroy(libxl__gc *gc,
>>      char *be_path = libxl__device_backend_path(gc, dev);
>>      char *fe_path = libxl__device_frontend_path(gc, dev);
>>
>> +    /*
>> +     * Run hotplug scripts, which will probably not be able to
>> +     * execute successfully since the device may still be plugged
>
> What does this mean? If we don't expect it to work why are we calling
> them?

If you call the libxl_domain_destroy function with force == 1, the
destroy process doesn't wait for devices to be disconnected, but we
might as well try to execute hotplug scripts, since we don't know the
actual state of the device. If we are lucky, the device might be
disconnected (state == 6), and we should be able to execute hotplug
scripts successfully.

>> +     */
>> +    libxl__device_execute_hotplug(gc, dev);
>> +
>>      xs_rm(ctx->xsh, XBT_NULL, be_path);
>>      xs_rm(ctx->xsh, XBT_NULL, fe_path);
>>
>> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_internal.h
>> --- a/tools/libxl/libxl_internal.h      Fri Nov 18 11:29:14 2011 +0100
>> +++ b/tools/libxl/libxl_internal.h      Fri Sep 30 14:38:55 2011 +0200
>> @@ -287,6 +287,34 @@ _hidden int libxl__wait_for_device_state
>>   */
>>  _hidden int libxl__try_phy_backend(mode_t st_mode);
>>
>> +/* hotplug functions */
>> +/*
>> + * libxl__device_execute_hotplug - generic function to execute hotplug scripts
>> + * gc: allocation pool
>> + * dev: reference to the device that executes the hotplug scripts
>> + *
>> + * Returns 0 on success, and < 0 on error.
>> + */
>> +_hidden int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev);
>> +
>> +/*
>> + * libxl__disk_hotplug - execute hotplug script for a disk type device
>> + * gc: allocation pool
>> + * dev: reference to the disk device
>> + *
>> + * Returns 0 on success, and < 0 on error.
>> + */
>> +_hidden int libxl__disk_hotplug(libxl__gc *gc, libxl__device *dev);
>> +
>> +/*
>> + * libxl__nic_hotplug - execute hotplug script for a nic type device
>> + * gc: allocation pool
>> + * dev: reference to the nic device
>> + *
>> + * Returns 0 on success, and < 0 on error.
>> + */
>> +_hidden int libxl__nic_hotplug(libxl__gc *gc, libxl__device *dev);
>> +
>>  /* from libxl_pci */
>>
>>  _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>
>

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

Re: [PATCH 6 of 9 v2] libxl: execute hotplug scripts directly from libxl

[Ian Campbell]

On Mon, 2011-11-21 at 11:42 +0000, Roger Pau Monné wrote:
> 2011/11/18 Ian Campbell <Ian.Campbell@citrix.com>:
> > On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
> >> # HG changeset patch
> >> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> >> # Date 1317386335 -7200
> >> # Node ID 1d81d1c4c851c0b07696373304801df56a221e90
> >> # Parent  4ad998fddb16a299cb230ea23ba944d84adbd2bf
> >> libxl: execute hotplug scripts directly from libxl.
> >>
> >> Added the necessary handlers to execute hotplug scripts when necessary
> >> from libxl. Split NetBSD and Linux hotplug calls into two separate
> >> files, because parameters for hotplug scripts are different. Linux
> >> has empty functions, since the calling of hotplug scripts is still
> >> done using udev.
> >>
> >> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
> >>
> >> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/Makefile
> >> --- a/tools/libxl/Makefile      Fri Nov 18 11:29:14 2011 +0100
> >> +++ b/tools/libxl/Makefile      Fri Sep 30 14:38:55 2011 +0200
> >> @@ -34,8 +34,10 @@ LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpu
> >
> > Should be:
> >
> >>  ifeq ($(CONFIG_NetBSD),y)
> >>  LIBXL_OBJS-y += libxl_phybackend.o
> >> +LIBXL_OBJS-y += hotplug_netbsd.o
> >   elsif ($(CONFIG_Linux),y)
> >>  LIBXL_OBJS-y += libxl_nophybackend.o
> >> +LIBXL_OBJS-y += hotplug_linux.o
> >   else
> >   $(error A message of some sort)
> >>  endif
> 
> Done, I've moved PHY backend selection and hotplug specific functions
> to libxl_netbsd.c and libxl_linux.c, and added an error message in
> case someone tries to use a different OS.
> 
> >> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_device.c
> >> --- a/tools/libxl/libxl_device.c        Fri Nov 18 11:29:14 2011 +0100
> >> +++ b/tools/libxl/libxl_device.c        Fri Sep 30 14:38:55 2011 +0200
> >> @@ -68,6 +68,24 @@ int libxl__parse_backend_path(libxl__gc
> >>      return libxl__device_kind_from_string(strkind, &dev->backend_kind);
> >>  }
> >>
> >> +int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev)
> >
> > No need for a add/remove type parameter?
> 
> Although you could pass this kind of parameter, the action to perform
> is based on the state of the device in the corresponding xenstore
> entry. State 2 means that the device should be connected, state 6
> means the device should be disconnected.

This seems a little fragile in the face of weird errors happening
asynchronously and could also be racy in the normal case. I think you
should pass in the action to be performed and, if necessary, confirm
that the state is correct. If possible you should just perform the
requested action irrespective of the backend state.

> 
> >> +{
> >> +    int rc = 0;
> >> +
> >> +    switch(dev->kind) {
> >> +    case LIBXL__DEVICE_KIND_VIF:
> >> +        rc = libxl__nic_hotplug(gc, dev);
> >> +        break;
> >> +    case LIBXL__DEVICE_KIND_VBD:
> >> +        rc = libxl__disk_hotplug(gc, dev);
> >> +        break;
> >> +    default:
> >> +        break;
> >> +    }
> >> +
> >> +    return rc;
> >> +}
> >> +
> >>  int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
> >>                               char **bents, char **fents)
> >>  {
> >> @@ -449,6 +467,7 @@ int libxl__device_remove(libxl__gc *gc,
> >>      if (!state)
> >>          goto out;
> >>      if (atoi(state) != 4) {
> >> +        libxl__device_execute_hotplug(gc, dev);
> >>          libxl__device_destroy_tapdisk(gc, be_path);
> >>          xs_rm(ctx->xsh, XBT_NULL, be_path);
> >>          goto out;
> >> @@ -492,6 +511,12 @@ int libxl__device_destroy(libxl__gc *gc,
> >>      char *be_path = libxl__device_backend_path(gc, dev);
> >>      char *fe_path = libxl__device_frontend_path(gc, dev);
> >>
> >> +    /*
> >> +     * Run hotplug scripts, which will probably not be able to
> >> +     * execute successfully since the device may still be plugged
> >
> > What does this mean? If we don't expect it to work why are we calling
> > them?
> 
> If you call the libxl_domain_destroy function with force == 1, the
> destroy process doesn't wait for devices to be disconnected, but we
> might as well try to execute hotplug scripts, since we don't know the
> actual state of the device. If we are lucky, the device might be
> disconnected (state == 6), and we should be able to execute hotplug
> scripts successfully.

If you pass the action to be performed rather than relying on the device
state then perhaps the scripts have a better chance of working.

However if the script cannot run reliably for a forced shutdown
(presumably you cannot teardown the loop device if it is still in use by
the backend?) then we need to solve that somehow rather than leaking the
resource.

Ian.

> 
> >> +     */
> >> +    libxl__device_execute_hotplug(gc, dev);
> >> +
> >>      xs_rm(ctx->xsh, XBT_NULL, be_path);
> >>      xs_rm(ctx->xsh, XBT_NULL, fe_path);
> >>
> >> diff -r 4ad998fddb16 -r 1d81d1c4c851 tools/libxl/libxl_internal.h
> >> --- a/tools/libxl/libxl_internal.h      Fri Nov 18 11:29:14 2011 +0100
> >> +++ b/tools/libxl/libxl_internal.h      Fri Sep 30 14:38:55 2011 +0200
> >> @@ -287,6 +287,34 @@ _hidden int libxl__wait_for_device_state
> >>   */
> >>  _hidden int libxl__try_phy_backend(mode_t st_mode);
> >>
> >> +/* hotplug functions */
> >> +/*
> >> + * libxl__device_execute_hotplug - generic function to execute hotplug scripts
> >> + * gc: allocation pool
> >> + * dev: reference to the device that executes the hotplug scripts
> >> + *
> >> + * Returns 0 on success, and < 0 on error.
> >> + */
> >> +_hidden int libxl__device_execute_hotplug(libxl__gc *gc, libxl__device *dev);
> >> +
> >> +/*
> >> + * libxl__disk_hotplug - execute hotplug script for a disk type device
> >> + * gc: allocation pool
> >> + * dev: reference to the disk device
> >> + *
> >> + * Returns 0 on success, and < 0 on error.
> >> + */
> >> +_hidden int libxl__disk_hotplug(libxl__gc *gc, libxl__device *dev);
> >> +
> >> +/*
> >> + * libxl__nic_hotplug - execute hotplug script for a nic type device
> >> + * gc: allocation pool
> >> + * dev: reference to the nic device
> >> + *
> >> + * Returns 0 on success, and < 0 on error.
> >> + */
> >> +_hidden int libxl__nic_hotplug(libxl__gc *gc, libxl__device *dev);
> >> +
> >>  /* from libxl_pci */
> >>
> >>  _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xensource.com
> >> http://lists.xensource.com/xen-devel
> >
> >
> >



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

Re: [PATCH 6 of 9 v2] libxl: execute hotplug scripts directly from libxl

[Ian Campbell]

I was just looking at this again because someone was talking about
hotplug issues on Linux and something new occurred to me.

On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
> [...]
> +++ b/tools/libxl/hotplug_linux.c       Fri Sep 30 14:38:55 2011 +0200
> [...]
> +int libxl_disk_hotplug(libxl__gc *gc, libxl__device *dev)
> [...]
> +int libxl_nic_hotplug_connect(libxl__gc *gc, libxl__device *dev)
> [...]
> +++ b/tools/libxl/hotplug_netbsd.c      Fri Sep 30 14:38:55 2011 +0200
> [...]
> +int libxl__disk_hotplug(libxl__gc *gc, libxl__device *dev)
> [...]
> +int libxl__nic_hotplug(libxl__gc *gc, libxl__device *dev)

Although Linux doesn't do hotplug this way now I think it will/should in
the future and the code will look very much like the netbsd one. So I
think that the code which actually handles calling the script should
probably be a generic function, only the selector on whether to do so
needs to be platform specific.

Ian.

Re: [PATCH 6 of 9 v2] libxl: execute hotplug scripts directly from libxl

[Roger Pau Monné]

2011/11/24 Ian Campbell <Ian.Campbell@citrix.com>:
> I was just looking at this again because someone was talking about
> hotplug issues on Linux and something new occurred to me.
>
> On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
>> [...]
>> +++ b/tools/libxl/hotplug_linux.c       Fri Sep 30 14:38:55 2011 +0200
>> [...]
>> +int libxl_disk_hotplug(libxl__gc *gc, libxl__device *dev)
>> [...]
>> +int libxl_nic_hotplug_connect(libxl__gc *gc, libxl__device *dev)
>> [...]
>> +++ b/tools/libxl/hotplug_netbsd.c      Fri Sep 30 14:38:55 2011 +0200
>> [...]
>> +int libxl__disk_hotplug(libxl__gc *gc, libxl__device *dev)
>> [...]
>> +int libxl__nic_hotplug(libxl__gc *gc, libxl__device *dev)
>
> Although Linux doesn't do hotplug this way now I think it will/should in
> the future and the code will look very much like the netbsd one. So I
> think that the code which actually handles calling the script should
> probably be a generic function, only the selector on whether to do so
> needs to be platform specific.

Sorry for the delay, I've been ill most of this week. From what I saw,
the problem is that NetBSD and Linux hotplug scripts have different
parameters, that's why I've split the calling functions into two
different files, and made them OS-dependent. I think I said that
before, but I would be good to agree on the calling parameters passed
to hotplug scripts. Currently NetBSD has the following parameters:

 * block: <backend path> <status> <type>
          where <type> can be "file" or "phy"

 * vif-bridge: <backend path> <status>

 * vif-ip: <backend path> <status>

Also Linux hotplug scripts need a good clean up, hard tabs and spaces
are all mixed, and makes the code really hard to understand.

Regards, Roger.

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

[PATCH 7 of 9 v2] hotplug NetBSD: detach devices when state is 5 or 6

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1317386335 -7200
# Node ID f46cb13d1ee576ed2c98d392356f380f205b587d
# Parent  1d81d1c4c851c0b07696373304801df56a221e90
hotplug NetBSD: detach devices when state is 5 or 6

With the move of hotplug calls from xenbackendd to libxl, we can
detach devices when the state is 5 or 6.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r 1d81d1c4c851 -r f46cb13d1ee5 tools/hotplug/NetBSD/block
--- a/tools/hotplug/NetBSD/block	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/hotplug/NetBSD/block	Fri Sep 30 14:38:55 2011 +0200
@@ -23,7 +23,7 @@ xtype=$3
 xparams=$(xenstore-read "$xpath/params")
 
 case $xstatus in
-6)
+5|6)
 	# device removed
 	case $xtype in
 	file)
diff -r 1d81d1c4c851 -r f46cb13d1ee5 tools/hotplug/NetBSD/vif-bridge
--- a/tools/hotplug/NetBSD/vif-bridge	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/hotplug/NetBSD/vif-bridge	Fri Sep 30 14:38:55 2011 +0200
@@ -14,7 +14,7 @@ xpath=$1
 xstatus=$2
 
 case $xstatus in
-6)
+5|6)
 	# device removed
 	xenstore-rm $xpath
 	exit 0
diff -r 1d81d1c4c851 -r f46cb13d1ee5 tools/hotplug/NetBSD/vif-ip
--- a/tools/hotplug/NetBSD/vif-ip	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/hotplug/NetBSD/vif-ip	Fri Sep 30 14:38:55 2011 +0200
@@ -14,7 +14,7 @@ xpath=$1
 xstatus=$2
 
 case $xstatus in
-6)
+5|6)
 	# device removed
 	xenstore-rm $xpath
 	exit 0

[PATCH 8 of 9 v2] hotplug: remove debug messages from NetBSD hotplug scripts

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1321612158 -3600
# Node ID d1591841fe963f054cd6b27d6d87ac005cfff29a
# Parent  f46cb13d1ee576ed2c98d392356f380f205b587d
hotplug: remove debug messages from NetBSD hotplug scripts

Remove unecessary debug messages from NetBSD hotplug scripts, left
error messages only.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r f46cb13d1ee5 -r d1591841fe96 tools/hotplug/NetBSD/block
--- a/tools/hotplug/NetBSD/block	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/hotplug/NetBSD/block	Fri Nov 18 11:29:18 2011 +0100
@@ -64,14 +64,12 @@ 2)
 			if [ "$status" = "free" ] && \
 			    vnconfig /dev/${disk}d $xparams >/dev/null; then
 				device=/dev/${disk}d
-				echo vnconfig /dev/${disk}d $xparams
 				break	
 			fi
 		done
 		if [ x$device = x ] ; then
 			error "no available vnd device"
 		fi
-		echo xenstore-write $xpath/vnd $device
 		xenstore-write $xpath/vnd $device
 		;;
 	phy)
@@ -79,9 +77,7 @@ 2)
 		;;
 	esac
 	physical_device=$(stat -f '%r' "$device")
-	echo xenstore-write $xpath/physical-device $physical_device
 	xenstore-write $xpath/physical-device $physical_device
-	echo xenstore-write $xpath/hotplug-status connected
 	xenstore-write $xpath/hotplug-status connected
 	exit 0
 	;;
diff -r f46cb13d1ee5 -r d1591841fe96 tools/hotplug/NetBSD/vif-bridge
--- a/tools/hotplug/NetBSD/vif-bridge	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/hotplug/NetBSD/vif-bridge	Fri Nov 18 11:29:18 2011 +0100
@@ -24,12 +24,9 @@ 2)
 	xfid=$(xenstore-read "$xpath/frontend-id")
 	xhandle=$(xenstore-read "$xpath/handle")
 	iface=$(xenstore-read "$xpath/vifname")
-	echo ifconfig $iface up
 	ifconfig $iface up
 	brconfig $xbridge add $iface
-	echo brconfig $xbridge add $iface
 	xenstore-write $xpath/hotplug-status connected
-	echo xenstore-write $xpath/hotplug-status connected
 	exit 0
 	;;
 *)
diff -r f46cb13d1ee5 -r d1591841fe96 tools/hotplug/NetBSD/vif-ip
--- a/tools/hotplug/NetBSD/vif-ip	Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/hotplug/NetBSD/vif-ip	Fri Nov 18 11:29:18 2011 +0100
@@ -24,10 +24,8 @@ 2)
 	xfid=$(xenstore-read "$xpath/frontend-id")
 	xhandle=$(xenstore-read "$xpath/handle")
 	iface=$(xenstore-read "$xpath/vifname")
-	echo ifconfig $iface $xip up
 	ifconfig $iface $xip up
 	xenstore-write $xpath/hotplug-status connected
-	echo xenstore-write $xpath/hotplug-status connected
 	exit 0
 	;;
 *)

[PATCH 9 of 9 v2] rc.d NetBSD: don't start xenbackendd by default, only when xend needs it

[Roger Pau Monne]

# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1317386335 -7200
# Node ID 8c77fe3008a486de7062d37053dcd0dbdc92ee0a
# Parent  d1591841fe963f054cd6b27d6d87ac005cfff29a
rc.d NetBSD: don't start xenbackendd by default, only when xend needs it.

With the move of hotplug scripts from xenbackendd to libxl
xenbackendd is no longer needed by libxl, only start it when xend is
started.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r d1591841fe96 -r 8c77fe3008a4 tools/hotplug/NetBSD/rc.d/xenbackendd
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/hotplug/NetBSD/rc.d/xenbackendd	Fri Sep 30 14:38:55 2011 +0200
@@ -0,0 +1,27 @@
+#!/bin/sh
+#
+# PROVIDE: xenbackendd
+# REQUIRE: xencommons
+
+. /etc/rc.subr
+
+DIR=$(dirname "$0")
+. "${DIR}/xen-hotplugpath.sh"
+
+LD_LIBRARY_PATH="${LIBDIR}"
+export LD_LIBRARY_PATH PYTHONPATH
+PATH="${PATH}:${SBINDIR}"
+export PATH
+
+name="xenbackendd"
+rcvar=$name
+command="${SBINDIR}/xenbackendd"
+if [ -n "${XENBACKENDD_DEBUG}" ]; then
+	command_args="${XENBACKENDD_ARGS} -d"
+else
+	command_args="${XENBACKENDD_ARGS}"
+fi
+
+load_rc_config $name
+run_rc_command "$1"
+
diff -r d1591841fe96 -r 8c77fe3008a4 tools/hotplug/NetBSD/rc.d/xencommons
--- a/tools/hotplug/NetBSD/rc.d/xencommons	Fri Nov 18 11:29:18 2011 +0100
+++ b/tools/hotplug/NetBSD/rc.d/xencommons	Fri Sep 30 14:38:55 2011 +0200
@@ -22,8 +22,6 @@ required_files="/kern/xen/privcmd"
 
 XENSTORED_PIDFILE="/var/run/xenstored.pid"
 XENCONSOLED_PIDFILE="/var/run/xenconsoled.pid"
-XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid"
-#XENBACKENDD_DEBUG=1
 #XENCONSOLED_TRACE="/var/log/xen/xenconsole-trace.log"
 #XENSTORED_TRACE="/var/log/xen/xenstore-trace.log"
 
@@ -46,7 +44,7 @@ xen_startcmd()
 			XENSTORED_ROOTDIR="/var/lib/xenstored"
 		fi
 		rm -f ${XENSTORED_ROOTDIR}/tdb* >/dev/null 2>&1
-		printf "Starting xenservices: xenstored, xenconsoled, xenbackendd."
+		printf "Starting xenservices: xenstored, xenconsoled."
 		XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}"
 		if [ -n "${XENSTORED_TRACE}" ]; then
 			XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log"
@@ -58,7 +56,7 @@ xen_startcmd()
 			sleep 1
 		done
 	else
-		printf "Starting xenservices: xenconsoled, xenbackendd."
+		printf "Starting xenservices: xenconsoled."
 	fi
 
 	XENCONSOLED_ARGS=""
@@ -68,13 +66,6 @@ xen_startcmd()
 
 	${SBINDIR}/xenconsoled ${XENCONSOLED_ARGS}
 
-	XENBACKENDD_ARGS=""
-	if [ -n "${XENBACKENDD_DEBUG}" ]; then
-		XENBACKENDD_ARGS="${XENBACKENDD_ARGS} -d"
-	fi
-
-	${SBINDIR}/xenbackendd ${XENBACKENDD_ARGS}
-
 	printf "\n"
 
 	printf "Setting domain 0 name.\n"
@@ -87,8 +78,6 @@ xen_stop()
 	printf "Stopping xencommons.\n"
 	printf "WARNING: Not stopping xenstored, as it cannot be restarted.\n"
 
-	rc_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
-	pids="$pids $rc_pid"
 	rc_pid=$(check_pidfile ${XENCONSOLED_PIDFILE} ${SBINDIR}/xenconsoled)
 	pids="$pids $rc_pid"
 
@@ -108,17 +97,12 @@ xen_status()
 		pids="$pids $xenconsoled_pid"
 	fi
 
-	xenbackend_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd)
-	if test -n ${xenbackend_pid}; then
-		pids="$pids $xenbackend_pid"
-	fi
-
-	if test -n "$xenbackend_pid" -a -n "$xenconsoled_pid" -a -n "$xenstored_pid";
+	if test -n "$xenconsoled_pid" -a -n "$xenstored_pid";
 	then
 		echo "xencommons are running as pids $pids."
 		return 0
 	fi
-	if test -z "$xenbackend_pid" -a -z "$xenconsoled_pid" -a -z "$xenstored_pid";
+	if test -z "$xenconsoled_pid" -a -z "$xenstored_pid";
 	then
 		echo "xencommons are not running."
 		return 0
@@ -134,11 +118,6 @@ xen_status()
 	else
 		echo "xenconsoled is not running."
 	fi
-	if test -n $xenbackend_pid; then
-		echo "xenbackendd is running as pid $xenbackend_pid."
-	else
-		echo "xenbackendd is not running."
-	fi
 }
 
 load_rc_config $name
diff -r d1591841fe96 -r 8c77fe3008a4 tools/hotplug/NetBSD/rc.d/xend
--- a/tools/hotplug/NetBSD/rc.d/xend	Fri Nov 18 11:29:18 2011 +0100
+++ b/tools/hotplug/NetBSD/rc.d/xend	Fri Sep 30 14:38:55 2011 +0200
@@ -1,7 +1,7 @@
 #!/bin/sh
 #
 # PROVIDE: xend
-# REQUIRE: xencommons
+# REQUIRE: xencommons xenbackendd
 
 . /etc/rc.subr