* [PATCH 0/2] libxl, Handle the command line options of qemu 0.12.
@ 2010-08-04 16:11 anthony.perard
2010-08-04 16:11 ` [PATCH 1/2] libxl, Introduce the command line handler for the new qemu anthony.perard
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: anthony.perard @ 2010-08-04 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
This patch gives the ability to the libxl to handle the command line argument
of the last qemu version. I begin by checking the version of the device model
with '-h' options, '--version' is not handled in qemu-xen. If I found a version
upper than 0.12, I use the new command line argument, or else I use the old
functions to build args.
The second patch introduces a udev rule to handle tap devices and add them to
the bridge instead of using a script called by qemu.
Anthony
Anthony PERARD (2):
libxl, Introduce the command line handler for the new qemu.
tools/hotplug, Use udev rules instead of qemu script to setup the
bridge.
tools/hotplug/Linux/vif-bridge | 42 +++++++--
tools/hotplug/Linux/vif-common.sh | 12 ++-
tools/hotplug/Linux/xen-backend.rules | 5 +-
tools/libxl/libxl.c | 163 ++++++++++++++++++++++++++++++++-
tools/libxl/libxl_utils.c | 73 +++++++++++++++
tools/libxl/libxl_utils.h | 5 +
6 files changed, 286 insertions(+), 14 deletions(-)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/2] libxl, Introduce the command line handler for the new qemu.
2010-08-04 16:11 [PATCH 0/2] libxl, Handle the command line options of qemu 0.12 anthony.perard
@ 2010-08-04 16:11 ` anthony.perard
2010-08-04 16:41 ` Gianni Tedesco
2010-08-04 17:24 ` Stefano Stabellini
2010-08-04 16:11 ` [PATCH 2/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
2010-08-06 17:23 ` [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12 anthony.perard
2 siblings, 2 replies; 22+ messages in thread
From: anthony.perard @ 2010-08-04 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
This patch adds a function to check the version of the device model.
Depending on the version of the DM, the command line arguments will be
built differently.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl.c | 163 ++++++++++++++++++++++++++++++++++++++++++++-
tools/libxl/libxl_utils.c | 73 ++++++++++++++++++++
tools/libxl/libxl_utils.h | 5 ++
3 files changed, 240 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 02faa0b..19dba55 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -964,7 +964,7 @@ skip_autopass:
return 0;
}
-static char ** libxl_build_device_model_args(libxl_ctx *ctx,
+static char ** libxl_build_device_model_args_old(libxl_ctx *ctx,
libxl_device_model_info *info,
libxl_device_nic *vifs,
int num_vifs)
@@ -1098,10 +1098,171 @@ static char ** libxl_build_device_model_args(libxl_ctx *ctx,
else
flexarray_set(dm_args, num++, "xenfv");
flexarray_set(dm_args, num++, NULL);
+ return (char **) flexarray_contents(dm_args);
+}
+
+static char ** libxl_build_device_model_args_new(libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_nic *vifs,
+ int num_vifs)
+{
+ int num = 0, i;
+ int new_qemu = 0;
+ flexarray_t *dm_args;
+ int nb;
+ libxl_device_disk *disks;
+
+ dm_args = flexarray_make(16, 1);
+ if (!dm_args)
+ return NULL;
+
+ new_qemu = libxl_check_device_model_version(ctx, info->device_model);
+
+ flexarray_set(dm_args, num++, "qemu-system-xen");
+ flexarray_set(dm_args, num++, "-xen-domid");
+
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", info->domid));
+
+ if (info->dom_name) {
+ flexarray_set(dm_args, num++, "-name");
+ flexarray_set(dm_args, num++, info->dom_name);
+ }
+ if (info->vnc || info->vncdisplay || info->vnclisten || info->vncunused) {
+ int display = 0;
+ const char *listen = "127.0.0.1";
+
+ flexarray_set(dm_args, num++, "-vnc");
+
+ if (info->vncdisplay) {
+ display = info->vncdisplay;
+ if (info->vnclisten && strchr(info->vnclisten, ':') == NULL) {
+ listen = info->vnclisten;
+ }
+ } else if (info->vnclisten) {
+ listen = info->vnclisten;
+ }
+
+ if (strchr(listen, ':') != NULL)
+ flexarray_set(dm_args, num++,
+ libxl_sprintf(ctx, "%s%s", listen,
+ info->vncunused ? ",to=99" : ""));
+ else
+ flexarray_set(dm_args, num++,
+ libxl_sprintf(ctx, "%s:%d%s", listen, display,
+ info->vncunused ? ",to=99" : ""));
+ }
+ if (info->sdl) {
+ flexarray_set(dm_args, num++, "-sdl");
+ }
+ if (info->keymap) {
+ flexarray_set(dm_args, num++, "-k");
+ flexarray_set(dm_args, num++, info->keymap);
+ }
+ if (info->nographic && (!info->sdl && !info->vnc)) {
+ flexarray_set(dm_args, num++, "-nographic");
+ }
+ if (info->serial) {
+ flexarray_set(dm_args, num++, "-serial");
+ flexarray_set(dm_args, num++, info->serial);
+ }
+ if (info->type == XENFV) {
+ int ioemu_vifs = 0;
+ if (info->stdvga) {
+ flexarray_set(dm_args, num++, "-vga");
+ flexarray_set(dm_args, num++, "std");
+ }
+
+ if (info->boot) {
+ flexarray_set(dm_args, num++, "-boot");
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "order=%s", info->boot));
+ }
+ if (info->usb || info->usbdevice) {
+ flexarray_set(dm_args, num++, "-usb");
+ if (info->usbdevice) {
+ flexarray_set(dm_args, num++, "-usbdevice");
+ flexarray_set(dm_args, num++, info->usbdevice);
+ }
+ }
+ if (info->soundhw) {
+ flexarray_set(dm_args, num++, "-soundhw");
+ flexarray_set(dm_args, num++, info->soundhw);
+ }
+ if (!info->apic) {
+ flexarray_set(dm_args, num++, "-no-acpi");
+ }
+ if (info->vcpus > 1) {
+ flexarray_set(dm_args, num++, "-smp");
+ if (info->vcpu_avail)
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d,maxcpus=%d", info->vcpus, info->vcpu_avail));
+ else
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", info->vcpus));
+ }
+ for (i = 0; i < num_vifs; i++) {
+ if (vifs[i].nictype == NICTYPE_IOEMU) {
+ char *smac = libxl_sprintf(ctx, "%02x:%02x:%02x:%02x:%02x:%02x",
+ vifs[i].mac[0], vifs[i].mac[1], vifs[i].mac[2],
+ vifs[i].mac[3], vifs[i].mac[4], vifs[i].mac[5]);
+ if (!vifs[i].ifname)
+ vifs[i].ifname = libxl_sprintf(ctx, "tap%d.%d", info->domid, vifs[i].devid - 1);
+ flexarray_set(dm_args, num++, "-net");
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "nic,vlan=%d,macaddr=%s,model=%s",
+ vifs[i].devid, smac, vifs[i].model));
+ flexarray_set(dm_args, num++, "-net");
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=%s",
+ vifs[i].devid, vifs[i].ifname, "/etc/xen/scripts/qemu-ifup"));
+ ioemu_vifs++;
+ }
+ }
+ /* If we have no emulated nics, tell qemu not to create any */
+ if ( ioemu_vifs == 0 ) {
+ flexarray_set(dm_args, num++, "-net");
+ flexarray_set(dm_args, num++, "none");
+ }
+ }
+ if (info->saved_state) {
+ flexarray_set(dm_args, num++, "-loadvm");
+ flexarray_set(dm_args, num++, info->saved_state);
+ }
+ for (i = 0; info->extra && info->extra[i] != NULL; i++)
+ flexarray_set(dm_args, num++, info->extra[i]);
+ flexarray_set(dm_args, num++, "-M");
+ if (info->type == XENPV)
+ flexarray_set(dm_args, num++, "xenpv");
+ else
+ flexarray_set(dm_args, num++, "xenfv");
+
+ disks = libxl_device_disk_list(ctx, info->domid, &nb);
+ for (; nb > 0; --nb, ++disks) {
+ if ( disks->is_cdrom ) {
+ flexarray_set(dm_args, num++, "-cdrom");
+ flexarray_set(dm_args, num++, disks->physpath);
+ }else{
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "-%s", disks->virtpath));
+ flexarray_set(dm_args, num++, disks->physpath);
+ }
+ }
+
+ flexarray_set(dm_args, num++, NULL);
return (char **) flexarray_contents(dm_args);
}
+static char ** libxl_build_device_model_args(libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_nic *vifs,
+ int num_vifs)
+{
+ int new_qemu;
+
+ new_qemu = libxl_check_device_model_version(ctx, info->device_model);
+
+ if (new_qemu) {
+ return libxl_build_device_model_args_new(ctx, info, vifs, num_vifs);
+ } else {
+ return libxl_build_device_model_args_old(ctx, info, vifs, num_vifs);
+ }
+}
+
void dm_xenstore_record_pid(void *for_spawn, pid_t innerchild)
{
libxl_device_model_starting *starting = for_spawn;
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 0b330b2..5fff2cc 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -532,3 +532,76 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac)
}
return 0;
}
+
+int libxl_check_device_model_version(libxl_ctx *ctx, char *path)
+{
+ pid_t pid = -1;
+ int pipefd[2];
+ char buf[100];
+ ssize_t i, count = 0;
+ int status;
+ char *abs_path = NULL;
+
+ abs_path = libxl_abs_path(ctx, path, libxl_private_bindir_path());
+
+ if (pipe(pipefd))
+ return 0;
+
+ pid = fork();
+ if (pid == -1) {
+ return 0;
+ }
+
+ if (!pid) {
+ close(pipefd[0]);
+ if (dup2(pipefd[1], STDOUT_FILENO) == -1)
+ exit(1);
+ execlp(abs_path, abs_path, "-h", NULL);
+
+ close(pipefd[1]);
+ exit(127);
+ }
+
+ close(pipefd[1]);
+ if (abs_path != path)
+ libxl_free(ctx, abs_path);
+
+ // attempt to get the first line of `qemu -h`
+ while ((i = read(pipefd[0], buf + count, 99 - count)) > 0) {
+ if (i + count > 90)
+ break;
+ for (int j = 0; j < i; j++) {
+ if (buf[j + count] == '\n')
+ break;
+ }
+ count += i;
+ }
+ count += i;
+ close(pipefd[0]);
+ waitpid(pid, &status, 0);
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+ return 0;
+ }
+
+ // Search for the new version or the old version:
+ // QEMU emulator version 0.12.50, ...
+ // QEMU PC emulator version 0.10.2, ...
+ if (strncmp("QEMU", buf, 4) == 0) {
+ char *v = strstr(buf, "version ");
+ if (v) {
+ int major, minor;
+ char *endptr = NULL;
+
+ v += strlen("version ");
+ major = strtol(v, &endptr, 10);
+ if (major == 0 && endptr && *endptr == '.') {
+ v = endptr + 1;
+ minor = strtol(v, &endptr, 10);
+ if (minor >= 12)
+ return 1;
+ }
+ return 0;
+ }
+ }
+ return 0;
+}
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 98a912c..bf8b361 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -68,5 +68,10 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac);
int libxl_devid_to_device_net2(libxl_ctx *ctx, uint32_t domid,
const char *devid, libxl_device_net2 *net2);
+/* check the version of qemu
+ * return 1 if is the new one
+ * return 0 if is the old one or if there are an error */
+int libxl_check_device_model_version(libxl_ctx *ctx, char *path);
+
#endif
--
1.6.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge.
2010-08-04 16:11 [PATCH 0/2] libxl, Handle the command line options of qemu 0.12 anthony.perard
2010-08-04 16:11 ` [PATCH 1/2] libxl, Introduce the command line handler for the new qemu anthony.perard
@ 2010-08-04 16:11 ` anthony.perard
2010-08-04 17:44 ` Stefano Stabellini
2010-08-06 17:23 ` [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12 anthony.perard
2 siblings, 1 reply; 22+ messages in thread
From: anthony.perard @ 2010-08-04 16:11 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD
From: Anthony PERARD <anthony.perard@citrix.com>
This patch adds a second argument to vif-bridge script. It can be "vif"
or "tap". "vif" give the default behavior and "tap" just add the
interface to the found bridge when the action is "add".
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/hotplug/Linux/vif-bridge | 42 +++++++++++++++++++++++++++-----
tools/hotplug/Linux/vif-common.sh | 12 ++++++---
tools/hotplug/Linux/xen-backend.rules | 5 ++-
tools/libxl/libxl.c | 4 +-
4 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
index d35144e..63223f3 100644
--- a/tools/hotplug/Linux/vif-bridge
+++ b/tools/hotplug/Linux/vif-bridge
@@ -29,6 +29,13 @@
# rules for its ip addresses (if any).
#============================================================================
+# Older versions of Xen do not pass in the type as an argument
+if [ $# -lt 2 ]; then
+ type_if=vif
+else
+ type_if=$2
+fi
+
dir=$(dirname "$0")
. "$dir/vif-common.sh"
@@ -79,22 +86,43 @@ then
fatal "Could not find bridge device $bridge"
fi
+case "$type_if" in
+ vif)
+ dev=$vif
+ ;;
+ tap)
+ dev=$INTERFACE
+ ;;
+esac
+
case "$command" in
online)
- setup_bridge_port "$vif"
- add_to_bridge "$bridge" "$vif"
+ if [ "$type_if" = vif ]; then
+ setup_bridge_port "$vif"
+ add_to_bridge "$bridge" "$vif"
+ fi
;;
offline)
- do_without_error brctl delif "$bridge" "$vif"
- do_without_error ifconfig "$vif" down
+ if [ "$type_if" = vif ]; then
+ do_without_error brctl delif "$bridge" "$vif"
+ do_without_error ifconfig "$vif" down
+ fi
+ ;;
+
+ add)
+ if [ "$type_if" = tap ]; then
+ add_to_bridge "$bridge" "$dev"
+ fi
;;
esac
-handle_iptable
+if [ "$type_if" = vif ]; then
+ handle_iptable
+fi
-log debug "Successful vif-bridge $command for $vif, bridge $bridge."
-if [ "$command" == "online" ]
+log debug "Successful vif-bridge $command for $dev, bridge $bridge."
+if [ "$type_if" = vif -a "$command" = "online" ]
then
success
fi
diff --git a/tools/hotplug/Linux/vif-common.sh b/tools/hotplug/Linux/vif-common.sh
index 44dd342..4cf00c0 100644
--- a/tools/hotplug/Linux/vif-common.sh
+++ b/tools/hotplug/Linux/vif-common.sh
@@ -33,7 +33,9 @@ fi
case "$command" in
add | remove)
- exit 0
+ if [ "$type_if" != tap ]; then
+ exit 0
+ fi
;;
esac
@@ -48,9 +50,11 @@ evalVariables "$@"
ip=${ip:-}
ip=$(xenstore_read_default "$XENBUS_PATH/ip" "$ip")
-# Check presence of compulsory args.
-XENBUS_PATH="${XENBUS_PATH:?}"
-vif="${vif:?}"
+if [ "$type_if" != tap ]; then
+ # Check presence of compulsory args.
+ XENBUS_PATH="${XENBUS_PATH:?}"
+ vif="${vif:?}"
+fi
vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
diff --git a/tools/hotplug/Linux/xen-backend.rules b/tools/hotplug/Linux/xen-backend.rules
index 2d844a1..8b749ac 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -2,10 +2,11 @@ SUBSYSTEM=="xen-backend", KERNEL=="tap*", RUN+="/etc/xen/scripts/blktap $env{ACT
SUBSYSTEM=="xen-backend", KERNEL=="vbd*", RUN+="/etc/xen/scripts/block $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vtpm*", RUN+="/etc/xen/scripts/vtpm $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online vif"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline vif"
SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
SUBSYSTEM=="xen-backend", ACTION=="remove", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
KERNEL=="evtchn", NAME="xen/%k"
KERNEL=="blktap[0-9]*", NAME="xen/%k"
KERNEL=="pci_iomul", NAME="xen/%k"
+SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} tap"
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 19dba55..4613c2f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1209,8 +1209,8 @@ static char ** libxl_build_device_model_args_new(libxl_ctx *ctx,
flexarray_set(dm_args, num++, libxl_sprintf(ctx, "nic,vlan=%d,macaddr=%s,model=%s",
vifs[i].devid, smac, vifs[i].model));
flexarray_set(dm_args, num++, "-net");
- flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=%s",
- vifs[i].devid, vifs[i].ifname, "/etc/xen/scripts/qemu-ifup"));
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=no",
+ vifs[i].devid, vifs[i].ifname));
ioemu_vifs++;
}
}
--
1.6.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] libxl, Introduce the command line handler for the new qemu.
2010-08-04 16:11 ` [PATCH 1/2] libxl, Introduce the command line handler for the new qemu anthony.perard
@ 2010-08-04 16:41 ` Gianni Tedesco
2010-08-04 17:24 ` Stefano Stabellini
1 sibling, 0 replies; 22+ messages in thread
From: Gianni Tedesco @ 2010-08-04 16:41 UTC (permalink / raw)
To: anthony.perard; +Cc: xen-devel
On Wed, 2010-08-04 at 17:11 +0100, anthony.perard@citrix.com wrote:
> + // Search for the new version or the old version:
> + // QEMU emulator version 0.12.50, ...
> + // QEMU PC emulator version 0.10.2, ...
> + if (strncmp("QEMU", buf, 4) == 0) {
> + char *v = strstr(buf, "version ");
> + if (v) {
> + int major, minor;
> + char *endptr = NULL;
> +
> + v += strlen("version ");
> + major = strtol(v, &endptr, 10);
> + if (major == 0 && endptr && *endptr == '.') {
> + v = endptr + 1;
> + minor = strtol(v, &endptr, 10);
> + if (minor >= 12)
> + return 1;
> + }
> + return 0;
> + }
> + }
> + return 0;
> +}
I don't like it. The version number is not a clear indication of
parameters especially of patched versions. If you are going to do
anything like this then qemu-dm ought to print out QEMU-DM and handle it
like that.
As for mainstream qemu, I hear that a future version will contain some
capability querying functionality.
Gianni
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] libxl, Introduce the command line handler for the new qemu.
2010-08-04 16:11 ` [PATCH 1/2] libxl, Introduce the command line handler for the new qemu anthony.perard
2010-08-04 16:41 ` Gianni Tedesco
@ 2010-08-04 17:24 ` Stefano Stabellini
2010-08-05 13:25 ` Anthony PERARD
1 sibling, 1 reply; 22+ messages in thread
From: Stefano Stabellini @ 2010-08-04 17:24 UTC (permalink / raw)
To: anthony.perard; +Cc: xen-devel
On Wed, 4 Aug 2010, anthony.perard@citrix.com wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
>
> This patch adds a function to check the version of the device model.
> Depending on the version of the DM, the command line arguments will be
> built differently.
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> tools/libxl/libxl.c | 163 ++++++++++++++++++++++++++++++++++++++++++++-
> tools/libxl/libxl_utils.c | 73 ++++++++++++++++++++
> tools/libxl/libxl_utils.h | 5 ++
> 3 files changed, 240 insertions(+), 1 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 02faa0b..19dba55 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -964,7 +964,7 @@ skip_autopass:
> return 0;
> }
>
> -static char ** libxl_build_device_model_args(libxl_ctx *ctx,
> +static char ** libxl_build_device_model_args_old(libxl_ctx *ctx,
> libxl_device_model_info *info,
> libxl_device_nic *vifs,
> int num_vifs)
> @@ -1098,10 +1098,171 @@ static char ** libxl_build_device_model_args(libxl_ctx *ctx,
> else
> flexarray_set(dm_args, num++, "xenfv");
> flexarray_set(dm_args, num++, NULL);
> + return (char **) flexarray_contents(dm_args);
> +}
> +
> +static char ** libxl_build_device_model_args_new(libxl_ctx *ctx,
> + libxl_device_model_info *info,
> + libxl_device_nic *vifs,
> + int num_vifs)
> +{
> + int num = 0, i;
> + int new_qemu = 0;
> + flexarray_t *dm_args;
> + int nb;
> + libxl_device_disk *disks;
> +
> + dm_args = flexarray_make(16, 1);
> + if (!dm_args)
> + return NULL;
> +
> + new_qemu = libxl_check_device_model_version(ctx, info->device_model);
> +
do we actually need to call libxl_check_device_model_version again here?
> + flexarray_set(dm_args, num++, "qemu-system-xen");
> + flexarray_set(dm_args, num++, "-xen-domid");
> +
> + flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", info->domid));
> +
> + if (info->dom_name) {
> + flexarray_set(dm_args, num++, "-name");
> + flexarray_set(dm_args, num++, info->dom_name);
> + }
> + if (info->vnc || info->vncdisplay || info->vnclisten || info->vncunused) {
> + int display = 0;
> + const char *listen = "127.0.0.1";
> +
> + flexarray_set(dm_args, num++, "-vnc");
> +
> + if (info->vncdisplay) {
> + display = info->vncdisplay;
> + if (info->vnclisten && strchr(info->vnclisten, ':') == NULL) {
> + listen = info->vnclisten;
> + }
> + } else if (info->vnclisten) {
> + listen = info->vnclisten;
> + }
> +
> + if (strchr(listen, ':') != NULL)
> + flexarray_set(dm_args, num++,
> + libxl_sprintf(ctx, "%s%s", listen,
> + info->vncunused ? ",to=99" : ""));
> + else
> + flexarray_set(dm_args, num++,
> + libxl_sprintf(ctx, "%s:%d%s", listen, display,
> + info->vncunused ? ",to=99" : ""));
> + }
> + if (info->sdl) {
> + flexarray_set(dm_args, num++, "-sdl");
> + }
> + if (info->keymap) {
> + flexarray_set(dm_args, num++, "-k");
> + flexarray_set(dm_args, num++, info->keymap);
> + }
> + if (info->nographic && (!info->sdl && !info->vnc)) {
> + flexarray_set(dm_args, num++, "-nographic");
> + }
> + if (info->serial) {
> + flexarray_set(dm_args, num++, "-serial");
> + flexarray_set(dm_args, num++, info->serial);
> + }
> + if (info->type == XENFV) {
> + int ioemu_vifs = 0;
>
> + if (info->stdvga) {
> + flexarray_set(dm_args, num++, "-vga");
> + flexarray_set(dm_args, num++, "std");
> + }
> +
> + if (info->boot) {
> + flexarray_set(dm_args, num++, "-boot");
> + flexarray_set(dm_args, num++, libxl_sprintf(ctx, "order=%s", info->boot));
> + }
> + if (info->usb || info->usbdevice) {
> + flexarray_set(dm_args, num++, "-usb");
> + if (info->usbdevice) {
> + flexarray_set(dm_args, num++, "-usbdevice");
> + flexarray_set(dm_args, num++, info->usbdevice);
> + }
> + }
> + if (info->soundhw) {
> + flexarray_set(dm_args, num++, "-soundhw");
> + flexarray_set(dm_args, num++, info->soundhw);
> + }
> + if (!info->apic) {
> + flexarray_set(dm_args, num++, "-no-acpi");
> + }
> + if (info->vcpus > 1) {
> + flexarray_set(dm_args, num++, "-smp");
> + if (info->vcpu_avail)
> + flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d,maxcpus=%d", info->vcpus, info->vcpu_avail));
> + else
> + flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", info->vcpus));
> + }
> + for (i = 0; i < num_vifs; i++) {
> + if (vifs[i].nictype == NICTYPE_IOEMU) {
> + char *smac = libxl_sprintf(ctx, "%02x:%02x:%02x:%02x:%02x:%02x",
> + vifs[i].mac[0], vifs[i].mac[1], vifs[i].mac[2],
> + vifs[i].mac[3], vifs[i].mac[4], vifs[i].mac[5]);
> + if (!vifs[i].ifname)
> + vifs[i].ifname = libxl_sprintf(ctx, "tap%d.%d", info->domid, vifs[i].devid - 1);
> + flexarray_set(dm_args, num++, "-net");
> + flexarray_set(dm_args, num++, libxl_sprintf(ctx, "nic,vlan=%d,macaddr=%s,model=%s",
> + vifs[i].devid, smac, vifs[i].model));
> + flexarray_set(dm_args, num++, "-net");
> + flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=%s",
> + vifs[i].devid, vifs[i].ifname, "/etc/xen/scripts/qemu-ifup"));
> + ioemu_vifs++;
> + }
> + }
> + /* If we have no emulated nics, tell qemu not to create any */
> + if ( ioemu_vifs == 0 ) {
> + flexarray_set(dm_args, num++, "-net");
> + flexarray_set(dm_args, num++, "none");
> + }
> + }
> + if (info->saved_state) {
> + flexarray_set(dm_args, num++, "-loadvm");
> + flexarray_set(dm_args, num++, info->saved_state);
> + }
> + for (i = 0; info->extra && info->extra[i] != NULL; i++)
> + flexarray_set(dm_args, num++, info->extra[i]);
> + flexarray_set(dm_args, num++, "-M");
> + if (info->type == XENPV)
> + flexarray_set(dm_args, num++, "xenpv");
> + else
> + flexarray_set(dm_args, num++, "xenfv");
> +
> + disks = libxl_device_disk_list(ctx, info->domid, &nb);
> + for (; nb > 0; --nb, ++disks) {
> + if ( disks->is_cdrom ) {
> + flexarray_set(dm_args, num++, "-cdrom");
> + flexarray_set(dm_args, num++, disks->physpath);
> + }else{
> + flexarray_set(dm_args, num++, libxl_sprintf(ctx, "-%s", disks->virtpath));
> + flexarray_set(dm_args, num++, disks->physpath);
> + }
> + }
> +
> + flexarray_set(dm_args, num++, NULL);
> return (char **) flexarray_contents(dm_args);
> }
>
> +static char ** libxl_build_device_model_args(libxl_ctx *ctx,
> + libxl_device_model_info *info,
> + libxl_device_nic *vifs,
> + int num_vifs)
> +{
> + int new_qemu;
> +
> + new_qemu = libxl_check_device_model_version(ctx, info->device_model);
> +
> + if (new_qemu) {
> + return libxl_build_device_model_args_new(ctx, info, vifs, num_vifs);
> + } else {
> + return libxl_build_device_model_args_old(ctx, info, vifs, num_vifs);
> + }
> +}
> +
> void dm_xenstore_record_pid(void *for_spawn, pid_t innerchild)
> {
> libxl_device_model_starting *starting = for_spawn;
> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index 0b330b2..5fff2cc 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -532,3 +532,76 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac)
> }
> return 0;
> }
> +
> +int libxl_check_device_model_version(libxl_ctx *ctx, char *path)
> +{
> + pid_t pid = -1;
> + int pipefd[2];
> + char buf[100];
> + ssize_t i, count = 0;
> + int status;
> + char *abs_path = NULL;
> +
> + abs_path = libxl_abs_path(ctx, path, libxl_private_bindir_path());
> +
> + if (pipe(pipefd))
> + return 0;
> +
> + pid = fork();
> + if (pid == -1) {
> + return 0;
> + }
> +
> + if (!pid) {
> + close(pipefd[0]);
> + if (dup2(pipefd[1], STDOUT_FILENO) == -1)
> + exit(1);
> + execlp(abs_path, abs_path, "-h", NULL);
> +
> + close(pipefd[1]);
> + exit(127);
> + }
> +
> + close(pipefd[1]);
> + if (abs_path != path)
> + libxl_free(ctx, abs_path);
> +
> + // attempt to get the first line of `qemu -h`
> + while ((i = read(pipefd[0], buf + count, 99 - count)) > 0) {
> + if (i + count > 90)
> + break;
> + for (int j = 0; j < i; j++) {
> + if (buf[j + count] == '\n')
> + break;
> + }
> + count += i;
> + }
> + count += i;
> + close(pipefd[0]);
> + waitpid(pid, &status, 0);
> + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
> + return 0;
> + }
> +
> + // Search for the new version or the old version:
> + // QEMU emulator version 0.12.50, ...
> + // QEMU PC emulator version 0.10.2, ...
> + if (strncmp("QEMU", buf, 4) == 0) {
> + char *v = strstr(buf, "version ");
> + if (v) {
> + int major, minor;
> + char *endptr = NULL;
> +
> + v += strlen("version ");
> + major = strtol(v, &endptr, 10);
> + if (major == 0 && endptr && *endptr == '.') {
> + v = endptr + 1;
> + minor = strtol(v, &endptr, 10);
> + if (minor >= 12)
> + return 1;
> + }
> + return 0;
> + }
> + }
> + return 0;
> +}
Please use C style comments.
I think Gianni is right about the string to search: it is probably
better to add a Xen specific string to qemu-dm, like "QEMU-DM", and rely
on that and the version (0.10.2 or older) to distinguish between the
two.
> diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
> index 98a912c..bf8b361 100644
> --- a/tools/libxl/libxl_utils.h
> +++ b/tools/libxl/libxl_utils.h
> @@ -68,5 +68,10 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac);
> int libxl_devid_to_device_net2(libxl_ctx *ctx, uint32_t domid,
> const char *devid, libxl_device_net2 *net2);
>
> +/* check the version of qemu
> + * return 1 if is the new one
> + * return 0 if is the old one or if there are an error */
> +int libxl_check_device_model_version(libxl_ctx *ctx, char *path);
> +
> #endif
>
> --
> 1.6.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge.
2010-08-04 16:11 ` [PATCH 2/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
@ 2010-08-04 17:44 ` Stefano Stabellini
2010-08-06 15:49 ` Anthony PERARD
0 siblings, 1 reply; 22+ messages in thread
From: Stefano Stabellini @ 2010-08-04 17:44 UTC (permalink / raw)
To: anthony.perard; +Cc: xen-devel
On Wed, 4 Aug 2010, anthony.perard@citrix.com wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
>
> This patch adds a second argument to vif-bridge script. It can be "vif"
> or "tap". "vif" give the default behavior and "tap" just add the
> interface to the found bridge when the action is "add".
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> tools/hotplug/Linux/vif-bridge | 42 +++++++++++++++++++++++++++-----
> tools/hotplug/Linux/vif-common.sh | 12 ++++++---
> tools/hotplug/Linux/xen-backend.rules | 5 ++-
> tools/libxl/libxl.c | 4 +-
> 4 files changed, 48 insertions(+), 15 deletions(-)
>
> diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
> index d35144e..63223f3 100644
> --- a/tools/hotplug/Linux/vif-bridge
> +++ b/tools/hotplug/Linux/vif-bridge
> @@ -29,6 +29,13 @@
> # rules for its ip addresses (if any).
> #============================================================================
>
> +# Older versions of Xen do not pass in the type as an argument
> +if [ $# -lt 2 ]; then
> + type_if=vif
> +else
> + type_if=$2
> +fi
> +
> dir=$(dirname "$0")
> . "$dir/vif-common.sh"
>
> @@ -79,22 +86,43 @@ then
> fatal "Could not find bridge device $bridge"
> fi
>
> +case "$type_if" in
> + vif)
> + dev=$vif
> + ;;
> + tap)
> + dev=$INTERFACE
> + ;;
> +esac
> +
> case "$command" in
> online)
> - setup_bridge_port "$vif"
> - add_to_bridge "$bridge" "$vif"
> + if [ "$type_if" = vif ]; then
> + setup_bridge_port "$vif"
> + add_to_bridge "$bridge" "$vif"
> + fi
> ;;
>
> offline)
> - do_without_error brctl delif "$bridge" "$vif"
> - do_without_error ifconfig "$vif" down
> + if [ "$type_if" = vif ]; then
> + do_without_error brctl delif "$bridge" "$vif"
> + do_without_error ifconfig "$vif" down
> + fi
> + ;;
> +
> + add)
> + if [ "$type_if" = tap ]; then
> + add_to_bridge "$bridge" "$dev"
> + fi
> ;;
> esac
Can we limit the amount of "if" introduced by this patch somehow?
If we use a different command (add instead of online), why do we need
the "if" under the online and offline cases?
If add is only used with tap devices, why do we need an if under add?
>
> -handle_iptable
> +if [ "$type_if" = vif ]; then
> + handle_iptable
> +fi
>
why is handle_iptable only called with vifs?
> -log debug "Successful vif-bridge $command for $vif, bridge $bridge."
> -if [ "$command" == "online" ]
> +log debug "Successful vif-bridge $command for $dev, bridge $bridge."
> +if [ "$type_if" = vif -a "$command" = "online" ]
> then
> success
> fi
> diff --git a/tools/hotplug/Linux/vif-common.sh b/tools/hotplug/Linux/vif-common.sh
> index 44dd342..4cf00c0 100644
> --- a/tools/hotplug/Linux/vif-common.sh
> +++ b/tools/hotplug/Linux/vif-common.sh
> @@ -33,7 +33,9 @@ fi
>
> case "$command" in
> add | remove)
> - exit 0
> + if [ "$type_if" != tap ]; then
> + exit 0
> + fi
> ;;
> esac
>
> @@ -48,9 +50,11 @@ evalVariables "$@"
> ip=${ip:-}
> ip=$(xenstore_read_default "$XENBUS_PATH/ip" "$ip")
>
> -# Check presence of compulsory args.
> -XENBUS_PATH="${XENBUS_PATH:?}"
> -vif="${vif:?}"
> +if [ "$type_if" != tap ]; then
> + # Check presence of compulsory args.
> + XENBUS_PATH="${XENBUS_PATH:?}"
> + vif="${vif:?}"
> +fi
vifs get the parameter from xenstore, how do tap devices get the
parameter?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] libxl, Introduce the command line handler for the new qemu.
2010-08-04 17:24 ` Stefano Stabellini
@ 2010-08-05 13:25 ` Anthony PERARD
2010-08-05 14:40 ` Gianni Tedesco
0 siblings, 1 reply; 22+ messages in thread
From: Anthony PERARD @ 2010-08-05 13:25 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, Gianni Tedesco
Stefano Stabellini wrote:
> On Wed, 4 Aug 2010, anthony.perard@citrix.com wrote:
>> From: Anthony PERARD <anthony.perard@citrix.com>
>>
>> This patch adds a function to check the version of the device model.
>> Depending on the version of the DM, the command line arguments will be
>> built differently.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>> tools/libxl/libxl.c | 163 ++++++++++++++++++++++++++++++++++++++++++++-
>> tools/libxl/libxl_utils.c | 73 ++++++++++++++++++++
>> tools/libxl/libxl_utils.h | 5 ++
>> 3 files changed, 240 insertions(+), 1 deletions(-)
>>
>> [...]
>>
>> + // Search for the new version or the old version:
>> + // QEMU emulator version 0.12.50, ...
>> + // QEMU PC emulator version 0.10.2, ...
>> + if (strncmp("QEMU", buf, 4) == 0) {
>> + char *v = strstr(buf, "version ");
>> + if (v) {
>> + int major, minor;
>> + char *endptr = NULL;
>> +
>> + v += strlen("version ");
>> + major = strtol(v, &endptr, 10);
>> + if (major == 0 && endptr && *endptr == '.') {
>> + v = endptr + 1;
>> + minor = strtol(v, &endptr, 10);
>> + if (minor >= 12)
>> + return 1;
>> + }
>> + return 0;
>> + }
>> + }
>> + return 0;
>> +}
>
> I think Gianni is right about the string to search: it is probably
> better to add a Xen specific string to qemu-dm, like "QEMU-DM", and rely
> on that and the version (0.10.2 or older) to distinguish between the
> two.
I can do something better by run "qemu -M ?" and check if "xenfv"
machine is supported. This work with both versions.
--
Anthony PERARD
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] libxl, Introduce the command line handler for the new qemu.
2010-08-05 13:25 ` Anthony PERARD
@ 2010-08-05 14:40 ` Gianni Tedesco
2010-08-05 16:45 ` Stefano Stabellini
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Gianni Tedesco @ 2010-08-05 14:40 UTC (permalink / raw)
To: Anthony Perard; +Cc: xen-devel, Stefano Stabellini
On Thu, 2010-08-05 at 14:25 +0100, Anthony Perard wrote:
> Stefano Stabellini wrote:
> > On Wed, 4 Aug 2010, anthony.perard@citrix.com wrote:
> >> From: Anthony PERARD <anthony.perard@citrix.com>
> >>
> >> This patch adds a function to check the version of the device model.
> >> Depending on the version of the DM, the command line arguments will be
> >> built differently.
> >>
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >> ---
> >> tools/libxl/libxl.c | 163 ++++++++++++++++++++++++++++++++++++++++++++-
> >> tools/libxl/libxl_utils.c | 73 ++++++++++++++++++++
> >> tools/libxl/libxl_utils.h | 5 ++
> >> 3 files changed, 240 insertions(+), 1 deletions(-)
> >>
> >> [...]
> >>
> >> + // Search for the new version or the old version:
> >> + // QEMU emulator version 0.12.50, ...
> >> + // QEMU PC emulator version 0.10.2, ...
> >> + if (strncmp("QEMU", buf, 4) == 0) {
> >> + char *v = strstr(buf, "version ");
> >> + if (v) {
> >> + int major, minor;
> >> + char *endptr = NULL;
> >> +
> >> + v += strlen("version ");
> >> + major = strtol(v, &endptr, 10);
> >> + if (major == 0 && endptr && *endptr == '.') {
> >> + v = endptr + 1;
> >> + minor = strtol(v, &endptr, 10);
> >> + if (minor >= 12)
> >> + return 1;
> >> + }
> >> + return 0;
> >> + }
> >> + }
> >> + return 0;
> >> +}
> >
> > I think Gianni is right about the string to search: it is probably
> > better to add a Xen specific string to qemu-dm, like "QEMU-DM", and rely
> > on that and the version (0.10.2 or older) to distinguish between the
> > two.
>
> I can do something better by run "qemu -M ?" and check if "xenfv"
> machine is supported. This work with both versions.
Yes but this is the problem exactly. The output of this does not
correlate to which set of command-line options are supported.
We cannot predict the future as to precisely what form any merge to qemu
upstream will take. Our only reliable choice is to add something new and
deliberately non-standardized to existing qemu-xen fork and detect that.
Gianni
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] libxl, Introduce the command line handler for the new qemu.
2010-08-05 14:40 ` Gianni Tedesco
@ 2010-08-05 16:45 ` Stefano Stabellini
2010-08-05 16:48 ` Gianni Tedesco
2010-08-05 18:03 ` [PATCH] Change the first line of help to add 'QEMU-DM' anthony.perard
` (2 subsequent siblings)
3 siblings, 1 reply; 22+ messages in thread
From: Stefano Stabellini @ 2010-08-05 16:45 UTC (permalink / raw)
To: Gianni Tedesco (3P); +Cc: Anthony Perard, xen-devel, Stefano Stabellini
On Thu, 5 Aug 2010, Gianni Tedesco (3P) wrote:
> Yes but this is the problem exactly. The output of this does not
> correlate to which set of command-line options are supported.
>
> We cannot predict the future as to precisely what form any merge to qemu
> upstream will take. Our only reliable choice is to add something new and
> deliberately non-standardized to existing qemu-xen fork and detect that.
>
That and also hope that if in the future qemu changes command line
options it will bump version number.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] libxl, Introduce the command line handler for the new qemu.
2010-08-05 16:45 ` Stefano Stabellini
@ 2010-08-05 16:48 ` Gianni Tedesco
0 siblings, 0 replies; 22+ messages in thread
From: Gianni Tedesco @ 2010-08-05 16:48 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Anthony Perard, xen-devel
On Thu, 2010-08-05 at 17:45 +0100, Stefano Stabellini wrote:
> On Thu, 5 Aug 2010, Gianni Tedesco (3P) wrote:
> > Yes but this is the problem exactly. The output of this does not
> > correlate to which set of command-line options are supported.
> >
> > We cannot predict the future as to precisely what form any merge to qemu
> > upstream will take. Our only reliable choice is to add something new and
> > deliberately non-standardized to existing qemu-xen fork and detect that.
> >
>
> That and also hope that if in the future qemu changes command line
> options it will bump version number.
Indeed, but this is apparently also a problem for libvirt which qemu
maintainers care deeply about so I think there will be a solution for
that more along the lines of capability querying returning JSON data
objects but that is just speculation at this point AFAIK.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] Change the first line of help to add 'QEMU-DM'.
2010-08-05 14:40 ` Gianni Tedesco
2010-08-05 16:45 ` Stefano Stabellini
@ 2010-08-05 18:03 ` anthony.perard
2010-08-05 18:03 ` [PATCH v2 1/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
2010-08-05 18:05 ` [PATCH v2 1/2] libxl, Introduce the command line handler for the new qemu anthony.perard
3 siblings, 0 replies; 22+ messages in thread
From: anthony.perard @ 2010-08-05 18:03 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, gianni.tedesco, Stefano.Stabellini
From: Anthony PERARD <anthony.perard@citrix.com>
Add QEMU-DM at the beginning of the help message. With that, xl can
detect the forked version of qemu and build the correct command line.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
vl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/vl.c b/vl.c
index ad8c0d0..67e0b1b 100644
--- a/vl.c
+++ b/vl.c
@@ -3990,7 +3990,7 @@ static void help(int exitcode)
{
/* Please keep in synch with QEMU_OPTION_ enums, qemu_options[]
and qemu-doc.texi */
- printf("QEMU PC emulator version " QEMU_VERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n"
+ printf("QEMU-DM emulator version " QEMU_VERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n"
"usage: %s [options] [disk_image]\n"
"\n"
"'disk_image' is a raw hard image image for IDE hard disk 0\n"
--
1.6.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge.
2010-08-05 14:40 ` Gianni Tedesco
2010-08-05 16:45 ` Stefano Stabellini
2010-08-05 18:03 ` [PATCH] Change the first line of help to add 'QEMU-DM' anthony.perard
@ 2010-08-05 18:03 ` anthony.perard
2010-08-05 18:06 ` Anthony PERARD
2010-08-05 18:05 ` [PATCH v2 1/2] libxl, Introduce the command line handler for the new qemu anthony.perard
3 siblings, 1 reply; 22+ messages in thread
From: anthony.perard @ 2010-08-05 18:03 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, gianni.tedesco, Stefano.Stabellini
From: Anthony PERARD <anthony.perard@citrix.com>
This patch adds a second argument to vif-bridge script. It can be "vif"
or "tap". "vif" give the default behavior and "tap" just add the
interface to the found bridge when the action is "add".
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/hotplug/Linux/vif-bridge | 42 +++++++++++++++++++++++++++-----
tools/hotplug/Linux/vif-common.sh | 12 ++++++---
tools/hotplug/Linux/xen-backend.rules | 5 ++-
tools/libxl/libxl.c | 4 +-
4 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
index d35144e..63223f3 100644
--- a/tools/hotplug/Linux/vif-bridge
+++ b/tools/hotplug/Linux/vif-bridge
@@ -29,6 +29,13 @@
# rules for its ip addresses (if any).
#============================================================================
+# Older versions of Xen do not pass in the type as an argument
+if [ $# -lt 2 ]; then
+ type_if=vif
+else
+ type_if=$2
+fi
+
dir=$(dirname "$0")
. "$dir/vif-common.sh"
@@ -79,22 +86,43 @@ then
fatal "Could not find bridge device $bridge"
fi
+case "$type_if" in
+ vif)
+ dev=$vif
+ ;;
+ tap)
+ dev=$INTERFACE
+ ;;
+esac
+
case "$command" in
online)
- setup_bridge_port "$vif"
- add_to_bridge "$bridge" "$vif"
+ if [ "$type_if" = vif ]; then
+ setup_bridge_port "$vif"
+ add_to_bridge "$bridge" "$vif"
+ fi
;;
offline)
- do_without_error brctl delif "$bridge" "$vif"
- do_without_error ifconfig "$vif" down
+ if [ "$type_if" = vif ]; then
+ do_without_error brctl delif "$bridge" "$vif"
+ do_without_error ifconfig "$vif" down
+ fi
+ ;;
+
+ add)
+ if [ "$type_if" = tap ]; then
+ add_to_bridge "$bridge" "$dev"
+ fi
;;
esac
-handle_iptable
+if [ "$type_if" = vif ]; then
+ handle_iptable
+fi
-log debug "Successful vif-bridge $command for $vif, bridge $bridge."
-if [ "$command" == "online" ]
+log debug "Successful vif-bridge $command for $dev, bridge $bridge."
+if [ "$type_if" = vif -a "$command" = "online" ]
then
success
fi
diff --git a/tools/hotplug/Linux/vif-common.sh b/tools/hotplug/Linux/vif-common.sh
index 44dd342..4cf00c0 100644
--- a/tools/hotplug/Linux/vif-common.sh
+++ b/tools/hotplug/Linux/vif-common.sh
@@ -33,7 +33,9 @@ fi
case "$command" in
add | remove)
- exit 0
+ if [ "$type_if" != tap ]; then
+ exit 0
+ fi
;;
esac
@@ -48,9 +50,11 @@ evalVariables "$@"
ip=${ip:-}
ip=$(xenstore_read_default "$XENBUS_PATH/ip" "$ip")
-# Check presence of compulsory args.
-XENBUS_PATH="${XENBUS_PATH:?}"
-vif="${vif:?}"
+if [ "$type_if" != tap ]; then
+ # Check presence of compulsory args.
+ XENBUS_PATH="${XENBUS_PATH:?}"
+ vif="${vif:?}"
+fi
vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
diff --git a/tools/hotplug/Linux/xen-backend.rules b/tools/hotplug/Linux/xen-backend.rules
index 2d844a1..8b749ac 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -2,10 +2,11 @@ SUBSYSTEM=="xen-backend", KERNEL=="tap*", RUN+="/etc/xen/scripts/blktap $env{ACT
SUBSYSTEM=="xen-backend", KERNEL=="vbd*", RUN+="/etc/xen/scripts/block $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vtpm*", RUN+="/etc/xen/scripts/vtpm $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online vif"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline vif"
SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
SUBSYSTEM=="xen-backend", ACTION=="remove", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
KERNEL=="evtchn", NAME="xen/%k"
KERNEL=="blktap[0-9]*", NAME="xen/%k"
KERNEL=="pci_iomul", NAME="xen/%k"
+SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} tap"
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 40c7255..db45af3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1206,8 +1206,8 @@ static char ** libxl_build_device_model_args_new(libxl_ctx *ctx,
flexarray_set(dm_args, num++, libxl_sprintf(ctx, "nic,vlan=%d,macaddr=%s,model=%s",
vifs[i].devid, smac, vifs[i].model));
flexarray_set(dm_args, num++, "-net");
- flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=%s",
- vifs[i].devid, vifs[i].ifname, "/etc/xen/scripts/qemu-ifup"));
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=no",
+ vifs[i].devid, vifs[i].ifname));
ioemu_vifs++;
}
}
--
1.6.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 1/2] libxl, Introduce the command line handler for the new qemu.
2010-08-05 14:40 ` Gianni Tedesco
` (2 preceding siblings ...)
2010-08-05 18:03 ` [PATCH v2 1/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
@ 2010-08-05 18:05 ` anthony.perard
3 siblings, 0 replies; 22+ messages in thread
From: anthony.perard @ 2010-08-05 18:05 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, gianni.tedesco, Stefano.Stabellini
From: Anthony PERARD <anthony.perard@citrix.com>
This patch adds a function to check the version of the device model.
Depending on the version of the DM, the command line arguments will be
built differently.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl.c | 160 ++++++++++++++++++++++++++++++++++++++++++++-
tools/libxl/libxl_utils.c | 78 ++++++++++++++++++++++
tools/libxl/libxl_utils.h | 6 ++
3 files changed, 243 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 732772c..40c7255 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -964,7 +964,7 @@ skip_autopass:
return 0;
}
-static char ** libxl_build_device_model_args(libxl_ctx *ctx,
+static char ** libxl_build_device_model_args_old(libxl_ctx *ctx,
libxl_device_model_info *info,
libxl_device_nic *vifs,
int num_vifs)
@@ -1098,10 +1098,168 @@ static char ** libxl_build_device_model_args(libxl_ctx *ctx,
else
flexarray_set(dm_args, num++, "xenfv");
flexarray_set(dm_args, num++, NULL);
+ return (char **) flexarray_contents(dm_args);
+}
+
+static char ** libxl_build_device_model_args_new(libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_nic *vifs,
+ int num_vifs)
+{
+ int num = 0, i;
+ flexarray_t *dm_args;
+ int nb;
+ libxl_device_disk *disks;
+
+ dm_args = flexarray_make(16, 1);
+ if (!dm_args)
+ return NULL;
+
+ flexarray_set(dm_args, num++, "qemu-system-xen");
+ flexarray_set(dm_args, num++, "-xen-domid");
+
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", info->domid));
+
+ if (info->dom_name) {
+ flexarray_set(dm_args, num++, "-name");
+ flexarray_set(dm_args, num++, info->dom_name);
+ }
+ if (info->vnc || info->vncdisplay || info->vnclisten || info->vncunused) {
+ int display = 0;
+ const char *listen = "127.0.0.1";
+
+ flexarray_set(dm_args, num++, "-vnc");
+
+ if (info->vncdisplay) {
+ display = info->vncdisplay;
+ if (info->vnclisten && strchr(info->vnclisten, ':') == NULL) {
+ listen = info->vnclisten;
+ }
+ } else if (info->vnclisten) {
+ listen = info->vnclisten;
+ }
+
+ if (strchr(listen, ':') != NULL)
+ flexarray_set(dm_args, num++,
+ libxl_sprintf(ctx, "%s%s", listen,
+ info->vncunused ? ",to=99" : ""));
+ else
+ flexarray_set(dm_args, num++,
+ libxl_sprintf(ctx, "%s:%d%s", listen, display,
+ info->vncunused ? ",to=99" : ""));
+ }
+ if (info->sdl) {
+ flexarray_set(dm_args, num++, "-sdl");
+ }
+ if (info->keymap) {
+ flexarray_set(dm_args, num++, "-k");
+ flexarray_set(dm_args, num++, info->keymap);
+ }
+ if (info->nographic && (!info->sdl && !info->vnc)) {
+ flexarray_set(dm_args, num++, "-nographic");
+ }
+ if (info->serial) {
+ flexarray_set(dm_args, num++, "-serial");
+ flexarray_set(dm_args, num++, info->serial);
+ }
+ if (info->type == XENFV) {
+ int ioemu_vifs = 0;
+
+ if (info->stdvga) {
+ flexarray_set(dm_args, num++, "-vga");
+ flexarray_set(dm_args, num++, "std");
+ }
+
+ if (info->boot) {
+ flexarray_set(dm_args, num++, "-boot");
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "order=%s", info->boot));
+ }
+ if (info->usb || info->usbdevice) {
+ flexarray_set(dm_args, num++, "-usb");
+ if (info->usbdevice) {
+ flexarray_set(dm_args, num++, "-usbdevice");
+ flexarray_set(dm_args, num++, info->usbdevice);
+ }
+ }
+ if (info->soundhw) {
+ flexarray_set(dm_args, num++, "-soundhw");
+ flexarray_set(dm_args, num++, info->soundhw);
+ }
+ if (!info->apic) {
+ flexarray_set(dm_args, num++, "-no-acpi");
+ }
+ if (info->vcpus > 1) {
+ flexarray_set(dm_args, num++, "-smp");
+ if (info->vcpu_avail)
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d,maxcpus=%d", info->vcpus, info->vcpu_avail));
+ else
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", info->vcpus));
+ }
+ for (i = 0; i < num_vifs; i++) {
+ if (vifs[i].nictype == NICTYPE_IOEMU) {
+ char *smac = libxl_sprintf(ctx, "%02x:%02x:%02x:%02x:%02x:%02x",
+ vifs[i].mac[0], vifs[i].mac[1], vifs[i].mac[2],
+ vifs[i].mac[3], vifs[i].mac[4], vifs[i].mac[5]);
+ if (!vifs[i].ifname)
+ vifs[i].ifname = libxl_sprintf(ctx, "tap%d.%d", info->domid, vifs[i].devid - 1);
+ flexarray_set(dm_args, num++, "-net");
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "nic,vlan=%d,macaddr=%s,model=%s",
+ vifs[i].devid, smac, vifs[i].model));
+ flexarray_set(dm_args, num++, "-net");
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=%s",
+ vifs[i].devid, vifs[i].ifname, "/etc/xen/scripts/qemu-ifup"));
+ ioemu_vifs++;
+ }
+ }
+ /* If we have no emulated nics, tell qemu not to create any */
+ if ( ioemu_vifs == 0 ) {
+ flexarray_set(dm_args, num++, "-net");
+ flexarray_set(dm_args, num++, "none");
+ }
+ }
+ if (info->saved_state) {
+ flexarray_set(dm_args, num++, "-loadvm");
+ flexarray_set(dm_args, num++, info->saved_state);
+ }
+ for (i = 0; info->extra && info->extra[i] != NULL; i++)
+ flexarray_set(dm_args, num++, info->extra[i]);
+ flexarray_set(dm_args, num++, "-M");
+ if (info->type == XENPV)
+ flexarray_set(dm_args, num++, "xenpv");
+ else
+ flexarray_set(dm_args, num++, "xenfv");
+ disks = libxl_device_disk_list(ctx, info->domid, &nb);
+ for (; nb > 0; --nb, ++disks) {
+ if ( disks->is_cdrom ) {
+ flexarray_set(dm_args, num++, "-cdrom");
+ flexarray_set(dm_args, num++, disks->physpath);
+ }else{
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "-%s", disks->virtpath));
+ flexarray_set(dm_args, num++, disks->physpath);
+ }
+ }
+
+ flexarray_set(dm_args, num++, NULL);
return (char **) flexarray_contents(dm_args);
}
+static char ** libxl_build_device_model_args(libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_nic *vifs,
+ int num_vifs)
+{
+ int new_qemu;
+
+ new_qemu = libxl_check_device_model_version(ctx, info->device_model);
+
+ if (new_qemu == 1) {
+ return libxl_build_device_model_args_new(ctx, info, vifs, num_vifs);
+ } else {
+ return libxl_build_device_model_args_old(ctx, info, vifs, num_vifs);
+ }
+}
+
void dm_xenstore_record_pid(void *for_spawn, pid_t innerchild)
{
libxl_device_model_starting *starting = for_spawn;
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 8a1fa5a..a6417bb 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -535,3 +535,81 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac)
}
return 0;
}
+
+#define QEMU_VERSION_STR "QEMU emulator version "
+
+
+int libxl_check_device_model_version(libxl_ctx *ctx, char *path)
+{
+ pid_t pid = -1;
+ int pipefd[2];
+ char buf[100];
+ ssize_t i, count = 0;
+ int status;
+ char *abs_path = NULL;
+
+ abs_path = libxl_abs_path(ctx, path, libxl_private_bindir_path());
+
+ if (pipe(pipefd))
+ return -1;
+
+ pid = fork();
+ if (pid == -1) {
+ return -1;
+ }
+
+ if (!pid) {
+ close(pipefd[0]);
+ if (dup2(pipefd[1], STDOUT_FILENO) == -1)
+ exit(1);
+ execlp(abs_path, abs_path, "-h", NULL);
+
+ close(pipefd[1]);
+ exit(127);
+ }
+
+ close(pipefd[1]);
+ if (abs_path != path)
+ libxl_free(ctx, abs_path);
+
+ /* attempt to get the first line of `qemu -h` */
+ while ((i = read(pipefd[0], buf + count, 99 - count)) > 0) {
+ if (i + count > 90)
+ break;
+ for (int j = 0; j < i; j++) {
+ if (buf[j + count] == '\n')
+ break;
+ }
+ count += i;
+ }
+ count += i;
+ close(pipefd[0]);
+ waitpid(pid, &status, 0);
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+ return -1;
+ }
+
+ /* Check if we have the forked qemu-xen. */
+ /* QEMU-DM emulator version 0.10.2, ... */
+ if (strncmp("QEMU-DM ", buf, 7) == 0) {
+ return 0;
+ }
+
+ /* Check if the version is above 12.0 */
+ /* The first line is : QEMU emulator version 0.12.50, ... */
+ if (strncmp(QEMU_VERSION_STR, buf, strlen(QEMU_VERSION_STR)) == 0) {
+ int major, minor;
+ char *endptr = NULL;
+ char *v = buf + strlen(QEMU_VERSION_STR);
+
+ major = strtol(v, &endptr, 10);
+ if (major == 0 && endptr && *endptr == '.') {
+ v = endptr + 1;
+ minor = strtol(v, &endptr, 10);
+ if (minor >= 12)
+ return 1;
+ }
+ }
+
+ return -1;
+}
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 98a912c..5de2137 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -68,5 +68,11 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac);
int libxl_devid_to_device_net2(libxl_ctx *ctx, uint32_t domid,
const char *devid, libxl_device_net2 *net2);
+/* check the version of qemu
+ * return 1 if is the new one
+ * return 0 if is the old one
+ * return -1 if there are an error */
+int libxl_check_device_model_version(libxl_ctx *ctx, char *path);
+
#endif
--
1.6.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge.
2010-08-05 18:03 ` [PATCH v2 1/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
@ 2010-08-05 18:06 ` Anthony PERARD
0 siblings, 0 replies; 22+ messages in thread
From: Anthony PERARD @ 2010-08-05 18:06 UTC (permalink / raw)
To: xen-devel; +Cc: Gianni Tedesco (3P), Stefano Stabellini
Anthony Perard wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
>
> This patch adds a second argument to vif-bridge script. It can be "vif"
> or "tap". "vif" give the default behavior and "tap" just add the
> interface to the found bridge when the action is "add".
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> tools/hotplug/Linux/vif-bridge | 42 +++++++++++++++++++++++++++-----
> tools/hotplug/Linux/vif-common.sh | 12 ++++++---
> tools/hotplug/Linux/xen-backend.rules | 5 ++-
> tools/libxl/libxl.c | 4 +-
> 4 files changed, 48 insertions(+), 15 deletions(-)
Sorry, wrong patch, this one is not updated...
--
Anthony PERARD
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge.
2010-08-04 17:44 ` Stefano Stabellini
@ 2010-08-06 15:49 ` Anthony PERARD
2010-08-10 10:52 ` Ian Campbell
0 siblings, 1 reply; 22+ messages in thread
From: Anthony PERARD @ 2010-08-06 15:49 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel
Stefano Stabellini wrote:
> On Wed, 4 Aug 2010, anthony.perard@citrix.com wrote:
>> From: Anthony PERARD <anthony.perard@citrix.com>
>>
>> This patch adds a second argument to vif-bridge script. It can be "vif"
>> or "tap". "vif" give the default behavior and "tap" just add the
>> interface to the found bridge when the action is "add".
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>> tools/hotplug/Linux/vif-bridge | 42 +++++++++++++++++++++++++++-----
>> tools/hotplug/Linux/vif-common.sh | 12 ++++++---
>> tools/hotplug/Linux/xen-backend.rules | 5 ++-
>> tools/libxl/libxl.c | 4 +-
>> 4 files changed, 48 insertions(+), 15 deletions(-)
>>
>> diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
>> index d35144e..63223f3 100644
>> --- a/tools/hotplug/Linux/vif-bridge
>> +++ b/tools/hotplug/Linux/vif-bridge
>> @@ -29,6 +29,13 @@
>> # rules for its ip addresses (if any).
>> #============================================================================
>>
>> +# Older versions of Xen do not pass in the type as an argument
>> +if [ $# -lt 2 ]; then
>> + type_if=vif
>> +else
>> + type_if=$2
>> +fi
>> +
>> dir=$(dirname "$0")
>> . "$dir/vif-common.sh"
>>
>> @@ -79,22 +86,43 @@ then
>> fatal "Could not find bridge device $bridge"
>> fi
>>
>> +case "$type_if" in
>> + vif)
>> + dev=$vif
>> + ;;
>> + tap)
>> + dev=$INTERFACE
>> + ;;
>> +esac
>> +
>> case "$command" in
>> online)
>> - setup_bridge_port "$vif"
>> - add_to_bridge "$bridge" "$vif"
>> + if [ "$type_if" = vif ]; then
>> + setup_bridge_port "$vif"
>> + add_to_bridge "$bridge" "$vif"
>> + fi
>> ;;
>>
>> offline)
>> - do_without_error brctl delif "$bridge" "$vif"
>> - do_without_error ifconfig "$vif" down
>> + if [ "$type_if" = vif ]; then
>> + do_without_error brctl delif "$bridge" "$vif"
>> + do_without_error ifconfig "$vif" down
>> + fi
>> + ;;
>> +
>> + add)
>> + if [ "$type_if" = tap ]; then
>> + add_to_bridge "$bridge" "$dev"
>> + fi
>> ;;
>> esac
>
> Can we limit the amount of "if" introduced by this patch somehow?
> If we use a different command (add instead of online), why do we need
> the "if" under the online and offline cases?
> If add is only used with tap devices, why do we need an if under add?
The command come directly from the $ACTION in udev, and for some reason,
this event is different in both cases. Here, I just check if we really
want to setup a vif or a tap. This is too much? I can check the argument
before that, in vif-common.sh, like is already done for a vif.
>>
>> -handle_iptable
>> +if [ "$type_if" = vif ]; then
>> + handle_iptable
>> +fi
>>
>
> why is handle_iptable only called with vifs?
Because iptable wasn't handled before with the qemu script and it
doesn't seem to be necessary to handle iptable for a tap device.
>> -log debug "Successful vif-bridge $command for $vif, bridge $bridge."
>> -if [ "$command" == "online" ]
>> +log debug "Successful vif-bridge $command for $dev, bridge $bridge."
>> +if [ "$type_if" = vif -a "$command" = "online" ]
>> then
>> success
>> fi
>> diff --git a/tools/hotplug/Linux/vif-common.sh b/tools/hotplug/Linux/vif-common.sh
>> index 44dd342..4cf00c0 100644
>> --- a/tools/hotplug/Linux/vif-common.sh
>> +++ b/tools/hotplug/Linux/vif-common.sh
>> @@ -33,7 +33,9 @@ fi
>>
>> case "$command" in
>> add | remove)
>> - exit 0
>> + if [ "$type_if" != tap ]; then
>> + exit 0
>> + fi
>> ;;
>> esac
>>
>> @@ -48,9 +50,11 @@ evalVariables "$@"
>> ip=${ip:-}
>> ip=$(xenstore_read_default "$XENBUS_PATH/ip" "$ip")
>>
>> -# Check presence of compulsory args.
>> -XENBUS_PATH="${XENBUS_PATH:?}"
>> -vif="${vif:?}"
>> +if [ "$type_if" != tap ]; then
>> + # Check presence of compulsory args.
>> + XENBUS_PATH="${XENBUS_PATH:?}"
>> + vif="${vif:?}"
>> +fi
>
> vifs get the parameter from xenstore, how do tap devices get the
> parameter?
The interface name is get from $INTERFACE, and the bridge is found
automatically by the script. But there must be a way to take the bridge
from xenstore, by using the name of the device (tap32.0) to have the path.
--
Anthony PERARD
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12.
2010-08-04 16:11 [PATCH 0/2] libxl, Handle the command line options of qemu 0.12 anthony.perard
2010-08-04 16:11 ` [PATCH 1/2] libxl, Introduce the command line handler for the new qemu anthony.perard
2010-08-04 16:11 ` [PATCH 2/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
@ 2010-08-06 17:23 ` anthony.perard
2010-08-06 17:23 ` [PATCH v2 1/3] libxl, Fix name of tap device anthony.perard
` (2 more replies)
2 siblings, 3 replies; 22+ messages in thread
From: anthony.perard @ 2010-08-06 17:23 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Stefano.Stabellini
From: Anthony PERARD <anthony.perard@citrix.com>
This patch gives the ability to the libxl to handle the command line argument
of the last qemu version. I begin by checking the version of the device model
with '-h' options, '--version' is not handled in qemu-xen. If I found a version
upper than 0.12, I use the new command line argument, or else I use the old
functions to build args.
The second patch introduces a udev rule to handle tap devices and add them to
the bridge instead of using a script called by qemu.
Anthony PERARD (3):
libxl, Fix name of tap device.
libxl, Introduce the command line handler for the new qemu.
tools/hotplug, Use udev rules instead of qemu script to setup the
bridge.
tools/hotplug/Linux/vif-bridge | 20 +++--
tools/hotplug/Linux/vif-common.sh | 70 ++++++++++----
tools/hotplug/Linux/xen-backend.rules | 5 +-
tools/libxl/libxl.c | 164 ++++++++++++++++++++++++++++++++-
tools/libxl/libxl_utils.c | 78 ++++++++++++++++
tools/libxl/libxl_utils.h | 6 +
6 files changed, 312 insertions(+), 31 deletions(-)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/3] libxl, Fix name of tap device.
2010-08-06 17:23 ` [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12 anthony.perard
@ 2010-08-06 17:23 ` anthony.perard
2010-08-06 17:23 ` [PATCH v2 2/3] libxl, Introduce the command line handler for the new qemu anthony.perard
2010-08-09 15:40 ` [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12 Stefano Stabellini
2010-08-10 15:20 ` Ian Jackson
2 siblings, 1 reply; 22+ messages in thread
From: anthony.perard @ 2010-08-06 17:23 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Stefano.Stabellini
From: Anthony PERARD <anthony.perard@citrix.com>
Currently, tap device names for a domain begin at tap42.-1, but it must
be tap42.0.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 732772c..97cca38 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1070,7 +1070,7 @@ static char ** libxl_build_device_model_args(libxl_ctx *ctx,
vifs[i].mac[0], vifs[i].mac[1], vifs[i].mac[2],
vifs[i].mac[3], vifs[i].mac[4], vifs[i].mac[5]);
if (!vifs[i].ifname)
- vifs[i].ifname = libxl_sprintf(ctx, "tap%d.%d", info->domid, vifs[i].devid - 1);
+ vifs[i].ifname = libxl_sprintf(ctx, "tap%d.%d", info->domid, vifs[i].devid);
flexarray_set(dm_args, num++, "-net");
flexarray_set(dm_args, num++, libxl_sprintf(ctx, "nic,vlan=%d,macaddr=%s,model=%s",
vifs[i].devid, smac, vifs[i].model));
--
1.6.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 2/3] libxl, Introduce the command line handler for the new qemu.
2010-08-06 17:23 ` [PATCH v2 1/3] libxl, Fix name of tap device anthony.perard
@ 2010-08-06 17:23 ` anthony.perard
2010-08-06 17:23 ` [PATCH v2 3/3] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
0 siblings, 1 reply; 22+ messages in thread
From: anthony.perard @ 2010-08-06 17:23 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Stefano.Stabellini
From: Anthony PERARD <anthony.perard@citrix.com>
This patch adds a function to check the version of the device model.
Depending on the version of the DM, the command line arguments will be
built differently.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl.c | 160 ++++++++++++++++++++++++++++++++++++++++++++-
tools/libxl/libxl_utils.c | 78 ++++++++++++++++++++++
tools/libxl/libxl_utils.h | 6 ++
3 files changed, 243 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 97cca38..53b5499 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -964,7 +964,7 @@ skip_autopass:
return 0;
}
-static char ** libxl_build_device_model_args(libxl_ctx *ctx,
+static char ** libxl_build_device_model_args_old(libxl_ctx *ctx,
libxl_device_model_info *info,
libxl_device_nic *vifs,
int num_vifs)
@@ -1098,10 +1098,168 @@ static char ** libxl_build_device_model_args(libxl_ctx *ctx,
else
flexarray_set(dm_args, num++, "xenfv");
flexarray_set(dm_args, num++, NULL);
+ return (char **) flexarray_contents(dm_args);
+}
+
+static char ** libxl_build_device_model_args_new(libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_nic *vifs,
+ int num_vifs)
+{
+ int num = 0, i;
+ flexarray_t *dm_args;
+ int nb;
+ libxl_device_disk *disks;
+
+ dm_args = flexarray_make(16, 1);
+ if (!dm_args)
+ return NULL;
+
+ flexarray_set(dm_args, num++, "qemu-system-xen");
+ flexarray_set(dm_args, num++, "-xen-domid");
+
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", info->domid));
+
+ if (info->dom_name) {
+ flexarray_set(dm_args, num++, "-name");
+ flexarray_set(dm_args, num++, info->dom_name);
+ }
+ if (info->vnc || info->vncdisplay || info->vnclisten || info->vncunused) {
+ int display = 0;
+ const char *listen = "127.0.0.1";
+
+ flexarray_set(dm_args, num++, "-vnc");
+
+ if (info->vncdisplay) {
+ display = info->vncdisplay;
+ if (info->vnclisten && strchr(info->vnclisten, ':') == NULL) {
+ listen = info->vnclisten;
+ }
+ } else if (info->vnclisten) {
+ listen = info->vnclisten;
+ }
+
+ if (strchr(listen, ':') != NULL)
+ flexarray_set(dm_args, num++,
+ libxl_sprintf(ctx, "%s%s", listen,
+ info->vncunused ? ",to=99" : ""));
+ else
+ flexarray_set(dm_args, num++,
+ libxl_sprintf(ctx, "%s:%d%s", listen, display,
+ info->vncunused ? ",to=99" : ""));
+ }
+ if (info->sdl) {
+ flexarray_set(dm_args, num++, "-sdl");
+ }
+ if (info->keymap) {
+ flexarray_set(dm_args, num++, "-k");
+ flexarray_set(dm_args, num++, info->keymap);
+ }
+ if (info->nographic && (!info->sdl && !info->vnc)) {
+ flexarray_set(dm_args, num++, "-nographic");
+ }
+ if (info->serial) {
+ flexarray_set(dm_args, num++, "-serial");
+ flexarray_set(dm_args, num++, info->serial);
+ }
+ if (info->type == XENFV) {
+ int ioemu_vifs = 0;
+
+ if (info->stdvga) {
+ flexarray_set(dm_args, num++, "-vga");
+ flexarray_set(dm_args, num++, "std");
+ }
+
+ if (info->boot) {
+ flexarray_set(dm_args, num++, "-boot");
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "order=%s", info->boot));
+ }
+ if (info->usb || info->usbdevice) {
+ flexarray_set(dm_args, num++, "-usb");
+ if (info->usbdevice) {
+ flexarray_set(dm_args, num++, "-usbdevice");
+ flexarray_set(dm_args, num++, info->usbdevice);
+ }
+ }
+ if (info->soundhw) {
+ flexarray_set(dm_args, num++, "-soundhw");
+ flexarray_set(dm_args, num++, info->soundhw);
+ }
+ if (!info->apic) {
+ flexarray_set(dm_args, num++, "-no-acpi");
+ }
+ if (info->vcpus > 1) {
+ flexarray_set(dm_args, num++, "-smp");
+ if (info->vcpu_avail)
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d,maxcpus=%d", info->vcpus, info->vcpu_avail));
+ else
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", info->vcpus));
+ }
+ for (i = 0; i < num_vifs; i++) {
+ if (vifs[i].nictype == NICTYPE_IOEMU) {
+ char *smac = libxl_sprintf(ctx, "%02x:%02x:%02x:%02x:%02x:%02x",
+ vifs[i].mac[0], vifs[i].mac[1], vifs[i].mac[2],
+ vifs[i].mac[3], vifs[i].mac[4], vifs[i].mac[5]);
+ if (!vifs[i].ifname)
+ vifs[i].ifname = libxl_sprintf(ctx, "tap%d.%d", info->domid, vifs[i].devid);
+ flexarray_set(dm_args, num++, "-net");
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "nic,vlan=%d,macaddr=%s,model=%s",
+ vifs[i].devid, smac, vifs[i].model));
+ flexarray_set(dm_args, num++, "-net");
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=%s",
+ vifs[i].devid, vifs[i].ifname, "/etc/xen/scripts/qemu-ifup"));
+ ioemu_vifs++;
+ }
+ }
+ /* If we have no emulated nics, tell qemu not to create any */
+ if ( ioemu_vifs == 0 ) {
+ flexarray_set(dm_args, num++, "-net");
+ flexarray_set(dm_args, num++, "none");
+ }
+ }
+ if (info->saved_state) {
+ flexarray_set(dm_args, num++, "-loadvm");
+ flexarray_set(dm_args, num++, info->saved_state);
+ }
+ for (i = 0; info->extra && info->extra[i] != NULL; i++)
+ flexarray_set(dm_args, num++, info->extra[i]);
+ flexarray_set(dm_args, num++, "-M");
+ if (info->type == XENPV)
+ flexarray_set(dm_args, num++, "xenpv");
+ else
+ flexarray_set(dm_args, num++, "xenfv");
+ disks = libxl_device_disk_list(ctx, info->domid, &nb);
+ for (; nb > 0; --nb, ++disks) {
+ if ( disks->is_cdrom ) {
+ flexarray_set(dm_args, num++, "-cdrom");
+ flexarray_set(dm_args, num++, disks->physpath);
+ }else{
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "-%s", disks->virtpath));
+ flexarray_set(dm_args, num++, disks->physpath);
+ }
+ }
+
+ flexarray_set(dm_args, num++, NULL);
return (char **) flexarray_contents(dm_args);
}
+static char ** libxl_build_device_model_args(libxl_ctx *ctx,
+ libxl_device_model_info *info,
+ libxl_device_nic *vifs,
+ int num_vifs)
+{
+ int new_qemu;
+
+ new_qemu = libxl_check_device_model_version(ctx, info->device_model);
+
+ if (new_qemu == 1) {
+ return libxl_build_device_model_args_new(ctx, info, vifs, num_vifs);
+ } else {
+ return libxl_build_device_model_args_old(ctx, info, vifs, num_vifs);
+ }
+}
+
void dm_xenstore_record_pid(void *for_spawn, pid_t innerchild)
{
libxl_device_model_starting *starting = for_spawn;
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 8a1fa5a..a6417bb 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -535,3 +535,81 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac)
}
return 0;
}
+
+#define QEMU_VERSION_STR "QEMU emulator version "
+
+
+int libxl_check_device_model_version(libxl_ctx *ctx, char *path)
+{
+ pid_t pid = -1;
+ int pipefd[2];
+ char buf[100];
+ ssize_t i, count = 0;
+ int status;
+ char *abs_path = NULL;
+
+ abs_path = libxl_abs_path(ctx, path, libxl_private_bindir_path());
+
+ if (pipe(pipefd))
+ return -1;
+
+ pid = fork();
+ if (pid == -1) {
+ return -1;
+ }
+
+ if (!pid) {
+ close(pipefd[0]);
+ if (dup2(pipefd[1], STDOUT_FILENO) == -1)
+ exit(1);
+ execlp(abs_path, abs_path, "-h", NULL);
+
+ close(pipefd[1]);
+ exit(127);
+ }
+
+ close(pipefd[1]);
+ if (abs_path != path)
+ libxl_free(ctx, abs_path);
+
+ /* attempt to get the first line of `qemu -h` */
+ while ((i = read(pipefd[0], buf + count, 99 - count)) > 0) {
+ if (i + count > 90)
+ break;
+ for (int j = 0; j < i; j++) {
+ if (buf[j + count] == '\n')
+ break;
+ }
+ count += i;
+ }
+ count += i;
+ close(pipefd[0]);
+ waitpid(pid, &status, 0);
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+ return -1;
+ }
+
+ /* Check if we have the forked qemu-xen. */
+ /* QEMU-DM emulator version 0.10.2, ... */
+ if (strncmp("QEMU-DM ", buf, 7) == 0) {
+ return 0;
+ }
+
+ /* Check if the version is above 12.0 */
+ /* The first line is : QEMU emulator version 0.12.50, ... */
+ if (strncmp(QEMU_VERSION_STR, buf, strlen(QEMU_VERSION_STR)) == 0) {
+ int major, minor;
+ char *endptr = NULL;
+ char *v = buf + strlen(QEMU_VERSION_STR);
+
+ major = strtol(v, &endptr, 10);
+ if (major == 0 && endptr && *endptr == '.') {
+ v = endptr + 1;
+ minor = strtol(v, &endptr, 10);
+ if (minor >= 12)
+ return 1;
+ }
+ }
+
+ return -1;
+}
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 98a912c..5de2137 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -68,5 +68,11 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac);
int libxl_devid_to_device_net2(libxl_ctx *ctx, uint32_t domid,
const char *devid, libxl_device_net2 *net2);
+/* check the version of qemu
+ * return 1 if is the new one
+ * return 0 if is the old one
+ * return -1 if there are an error */
+int libxl_check_device_model_version(libxl_ctx *ctx, char *path);
+
#endif
--
1.6.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2 3/3] tools/hotplug, Use udev rules instead of qemu script to setup the bridge.
2010-08-06 17:23 ` [PATCH v2 2/3] libxl, Introduce the command line handler for the new qemu anthony.perard
@ 2010-08-06 17:23 ` anthony.perard
0 siblings, 0 replies; 22+ messages in thread
From: anthony.perard @ 2010-08-06 17:23 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Stefano.Stabellini
From: Anthony PERARD <anthony.perard@citrix.com>
This patch adds a second argument to vif-bridge script. It can be "vif"
or "tap". "vif" give the default behavior and "tap" just add the
interface to the found bridge when the action is "add".
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/hotplug/Linux/vif-bridge | 20 ++++++---
tools/hotplug/Linux/vif-common.sh | 70 ++++++++++++++++++++++++---------
tools/hotplug/Linux/xen-backend.rules | 5 +-
tools/libxl/libxl.c | 6 +-
4 files changed, 70 insertions(+), 31 deletions(-)
diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
index d35144e..82ba676 100644
--- a/tools/hotplug/Linux/vif-bridge
+++ b/tools/hotplug/Linux/vif-bridge
@@ -81,20 +81,26 @@ fi
case "$command" in
online)
- setup_bridge_port "$vif"
- add_to_bridge "$bridge" "$vif"
+ setup_bridge_port "$dev"
+ add_to_bridge "$bridge" "$dev"
;;
offline)
- do_without_error brctl delif "$bridge" "$vif"
- do_without_error ifconfig "$vif" down
+ do_without_error brctl delif "$bridge" "$dev"
+ do_without_error ifconfig "$dev" down
+ ;;
+
+ add)
+ add_to_bridge "$bridge" "$dev"
;;
esac
-handle_iptable
+if [ "$type_if" = vif ]; then
+ handle_iptable
+fi
-log debug "Successful vif-bridge $command for $vif, bridge $bridge."
-if [ "$command" == "online" ]
+log debug "Successful vif-bridge $command for $dev, bridge $bridge."
+if [ "$type_if" = vif -a "$command" = "online" ]
then
success
fi
diff --git a/tools/hotplug/Linux/vif-common.sh b/tools/hotplug/Linux/vif-common.sh
index 44dd342..780cc71 100644
--- a/tools/hotplug/Linux/vif-common.sh
+++ b/tools/hotplug/Linux/vif-common.sh
@@ -31,12 +31,6 @@ then
exit 1
fi
-case "$command" in
- add | remove)
- exit 0
- ;;
-esac
-
# Parameters may be read from the environment, the command line arguments, and
# the store, with overriding in that order. The environment is given by the
@@ -45,24 +39,62 @@ esac
evalVariables "$@"
-ip=${ip:-}
-ip=$(xenstore_read_default "$XENBUS_PATH/ip" "$ip")
+# Older versions of Xen do not pass in the type as an argument,
+# so the default value is vif.
+: ${type_if:=vif}
+
+case "$type_if" in
+ vif)
+ dev=$vif
+ ;;
+ tap)
+ dev=$INTERFACE
+ ;;
+ *)
+ log err "unknown interface type $type_if"
+ exit 1
+ ;;
+esac
-# Check presence of compulsory args.
-XENBUS_PATH="${XENBUS_PATH:?}"
-vif="${vif:?}"
+case "$command" in
+ online | offline)
+ test "$type_if" != vif && exit 0
+ ;;
+ add | remove)
+ test "$type_if" != tap && exit 0
+ ;;
+esac
-vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
-if [ "$vifname" ]
-then
- if [ "$command" == "online" ] && ! ip link show "$vifname" >&/dev/null
- then
- do_or_die ip link set "$vif" name "$vifname"
- fi
- vif="$vifname"
+if [ "$type_if" = vif ]; then
+ # Check presence of compulsory args.
+ XENBUS_PATH="${XENBUS_PATH:?}"
+ vif="${vif:?}"
+
+ vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "")
+ if [ "$vifname" ]
+ then
+ if [ "$command" == "online" ] && ! ip link show "$vifname" >&/dev/null
+ then
+ do_or_die ip link set "$vif" name "$vifname"
+ fi
+ vif="$vifname"
+ fi
+elif [ "$type_if" = tap ]; then
+ # Check presence of compulsory args.
+ : ${INTERFACE:?}
+
+ # Get xenbus_path from device name.
+ # The name is built like that: "tap${domid}.${devid}".
+ dev_=${dev#tap}
+ domid=${dev_%.*}
+ devid=${dev_#*.}
+
+ XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
fi
+ip=${ip:-}
+ip=$(xenstore_read_default "$XENBUS_PATH/ip" "$ip")
frob_iptable()
{
diff --git a/tools/hotplug/Linux/xen-backend.rules b/tools/hotplug/Linux/xen-backend.rules
index 2d844a1..ccbe508 100644
--- a/tools/hotplug/Linux/xen-backend.rules
+++ b/tools/hotplug/Linux/xen-backend.rules
@@ -2,10 +2,11 @@ SUBSYSTEM=="xen-backend", KERNEL=="tap*", RUN+="/etc/xen/scripts/blktap $env{ACT
SUBSYSTEM=="xen-backend", KERNEL=="vbd*", RUN+="/etc/xen/scripts/block $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vtpm*", RUN+="/etc/xen/scripts/vtpm $env{ACTION}"
SUBSYSTEM=="xen-backend", KERNEL=="vif2-*", RUN+="/etc/xen/scripts/vif2 $env{ACTION}"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online"
-SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="online", RUN+="/etc/xen/scripts/vif-setup online type_if=vif"
+SUBSYSTEM=="xen-backend", KERNEL=="vif-*", ACTION=="offline", RUN+="/etc/xen/scripts/vif-setup offline type_if=vif"
SUBSYSTEM=="xen-backend", KERNEL=="vscsi*", RUN+="/etc/xen/scripts/vscsi $env{ACTION}"
SUBSYSTEM=="xen-backend", ACTION=="remove", RUN+="/etc/xen/scripts/xen-hotplug-cleanup"
KERNEL=="evtchn", NAME="xen/%k"
KERNEL=="blktap[0-9]*", NAME="xen/%k"
KERNEL=="pci_iomul", NAME="xen/%k"
+SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 53b5499..2f92732 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1075,7 +1075,7 @@ static char ** libxl_build_device_model_args_old(libxl_ctx *ctx,
flexarray_set(dm_args, num++, libxl_sprintf(ctx, "nic,vlan=%d,macaddr=%s,model=%s",
vifs[i].devid, smac, vifs[i].model));
flexarray_set(dm_args, num++, "-net");
- flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,bridge=%s",
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,bridge=%s,script=no",
vifs[i].devid, vifs[i].ifname, vifs[i].bridge));
ioemu_vifs++;
}
@@ -1206,8 +1206,8 @@ static char ** libxl_build_device_model_args_new(libxl_ctx *ctx,
flexarray_set(dm_args, num++, libxl_sprintf(ctx, "nic,vlan=%d,macaddr=%s,model=%s",
vifs[i].devid, smac, vifs[i].model));
flexarray_set(dm_args, num++, "-net");
- flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=%s",
- vifs[i].devid, vifs[i].ifname, "/etc/xen/scripts/qemu-ifup"));
+ flexarray_set(dm_args, num++, libxl_sprintf(ctx, "tap,vlan=%d,ifname=%s,script=no",
+ vifs[i].devid, vifs[i].ifname));
ioemu_vifs++;
}
}
--
1.6.5
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12.
2010-08-06 17:23 ` [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12 anthony.perard
2010-08-06 17:23 ` [PATCH v2 1/3] libxl, Fix name of tap device anthony.perard
@ 2010-08-09 15:40 ` Stefano Stabellini
2010-08-10 15:20 ` Ian Jackson
2 siblings, 0 replies; 22+ messages in thread
From: Stefano Stabellini @ 2010-08-09 15:40 UTC (permalink / raw)
To: Anthony Perard; +Cc: xen-devel, Stabellini
On Fri, 6 Aug 2010, Anthony Perard wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
>
> This patch gives the ability to the libxl to handle the command line argument
> of the last qemu version. I begin by checking the version of the device model
> with '-h' options, '--version' is not handled in qemu-xen. If I found a version
> upper than 0.12, I use the new command line argument, or else I use the old
> functions to build args.
>
> The second patch introduces a udev rule to handle tap devices and add them to
> the bridge instead of using a script called by qemu.
>
> Anthony PERARD (3):
> libxl, Fix name of tap device.
> libxl, Introduce the command line handler for the new qemu.
> tools/hotplug, Use udev rules instead of qemu script to setup the
> bridge.
>
applied, thanks
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge.
2010-08-06 15:49 ` Anthony PERARD
@ 2010-08-10 10:52 ` Ian Campbell
0 siblings, 0 replies; 22+ messages in thread
From: Ian Campbell @ 2010-08-10 10:52 UTC (permalink / raw)
To: Anthony PERARD; +Cc: xen-devel, Stefano Stabellini
On Fri, 2010-08-06 at 16:49 +0100, Anthony PERARD wrote:
>
> > Can we limit the amount of "if" introduced by this patch somehow?
> > If we use a different command (add instead of online), why do we
> need
> > the "if" under the online and offline cases?
> > If add is only used with tap devices, why do we need an if under
> add?
>
> The command come directly from the $ACTION in udev, and for some
> reason, this event is different in both cases. Here, I just check if
> we really want to setup a vif or a tap. This is too much?
It could be that just handling one of online or add would be sufficient.
I don't really remember but when I made the patch for XCP which you are
basing this patch on I may just have been trying to avoid a large change
in behaviour and/or other knockon effects. It's equally possible that
one or the other type of device actually generates both types of event
or that I was being overly/unnecessarily conservative.
I think I'd recommend confirming the actual behaviour WRT event names of
both vif and tap devices on a few "important/interesting" kernel
versions before removing the extra if statements.
I wonder if we should be looking to begin transitioning the hotplug
events generated by netback to be consistent with those generated by
other network devices? (assuming there is consistency in general and the
the tun/tap driver is the correct one in this case)
Ian
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12.
2010-08-06 17:23 ` [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12 anthony.perard
2010-08-06 17:23 ` [PATCH v2 1/3] libxl, Fix name of tap device anthony.perard
2010-08-09 15:40 ` [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12 Stefano Stabellini
@ 2010-08-10 15:20 ` Ian Jackson
2 siblings, 0 replies; 22+ messages in thread
From: Ian Jackson @ 2010-08-10 15:20 UTC (permalink / raw)
To: anthony.perard; +Cc: xen-devel, Stefano.Stabellini
anthony.perard@citrix.com writes ("[Xen-devel] [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12."):
> This patch gives the ability to the libxl to handle the command line
> argument of the last qemu version. I begin by checking the version
> of the device model with '-h' options, '--version' is not handled in
> qemu-xen. If I found a version upper than 0.12, I use the new
> command line argument, or else I use the old functions to build
> args.
I see I'm too late with this but I agree with the comments that
checking the version number is not the right approach.
The right approach, it seems to me, is to call strstr() on a copy of
the qemu-dm -h output to try to see whether it supports each
particular variant.
Ian.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2010-08-10 15:20 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-04 16:11 [PATCH 0/2] libxl, Handle the command line options of qemu 0.12 anthony.perard
2010-08-04 16:11 ` [PATCH 1/2] libxl, Introduce the command line handler for the new qemu anthony.perard
2010-08-04 16:41 ` Gianni Tedesco
2010-08-04 17:24 ` Stefano Stabellini
2010-08-05 13:25 ` Anthony PERARD
2010-08-05 14:40 ` Gianni Tedesco
2010-08-05 16:45 ` Stefano Stabellini
2010-08-05 16:48 ` Gianni Tedesco
2010-08-05 18:03 ` [PATCH] Change the first line of help to add 'QEMU-DM' anthony.perard
2010-08-05 18:03 ` [PATCH v2 1/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
2010-08-05 18:06 ` Anthony PERARD
2010-08-05 18:05 ` [PATCH v2 1/2] libxl, Introduce the command line handler for the new qemu anthony.perard
2010-08-04 16:11 ` [PATCH 2/2] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
2010-08-04 17:44 ` Stefano Stabellini
2010-08-06 15:49 ` Anthony PERARD
2010-08-10 10:52 ` Ian Campbell
2010-08-06 17:23 ` [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12 anthony.perard
2010-08-06 17:23 ` [PATCH v2 1/3] libxl, Fix name of tap device anthony.perard
2010-08-06 17:23 ` [PATCH v2 2/3] libxl, Introduce the command line handler for the new qemu anthony.perard
2010-08-06 17:23 ` [PATCH v2 3/3] tools/hotplug, Use udev rules instead of qemu script to setup the bridge anthony.perard
2010-08-09 15:40 ` [PATCH v2 0/3] libxl, Handle the command line options of qemu 0.12 Stefano Stabellini
2010-08-10 15:20 ` Ian Jackson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.