Skip to contentSkip to conversation

xl: Make blktap support optional

PATCH NAK Christoph Egger 4 participants 5 messages Aug 13, 2010 – Aug 16, 2010

lore
Share & tools
lore

Christoph Egger

author
#0 lore raw

PATCH 1/1 xl: Make blktap support optional

Make blktap support optional.
Enable it by default on Linux, disable it on non-Linux.

Signed-off-by Christoph Egger <[email protected]>
Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
tools/Rules.mk +11 -0
diff -r 2760576b0d7c -r 9996d4f06e70 tools/Rules.mk
@@ -26,8 +26,19 @@ LDLIBS_libxenguest = -L$(XEN_LIBXC) -lxe
 CFLAGS_libxenstore = -I$(XEN_XENSTORE) $(CFLAGS_include)
 LDLIBS_libxenstore = -L$(XEN_XENSTORE) -lxenstore
 
+ifeq ($(CONFIG_Linux),y)
+LIBXL_BLKTAP = y
+else
+LIBXL_BLKTAP = n
+endif
+
+ifeq ($(LIBXL_BLKTAP),y)
 CFLAGS_libblktapctl = -I$(XEN_BLKTAP2)/control -I$(XEN_BLKTAP2)/include $(CFLAGS_include)
 LDLIBS_libblktapctl = -L$(XEN_BLKTAP2)/control -lblktapctl
+else
+CFLAGS_libblktapctl =
+LDLIBS_libblktapctl =
+endif
 
 X11_LDPATH = -L/usr/X11R6/$(LIBLEAFDIR)
tools/libxl/Makefile +6 -0
diff -r 2760576b0d7c -r 9996d4f06e70 tools/libxl/Makefile
@@ -18,6 +18,12 @@ CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_
 LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(UTIL_LIBS)
 
 LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o
+ifeq ($(LIBXL_BLKTAP),y)
+LIBXL_OBJS-y += libxl_blktap2.o
+else
+LIBXL_OBJS-y += libxl_noblktap2.o
+endif
+
 LIBXL_OBJS = flexarray.o libxl.o libxl_pci.o libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o libxl_internal.o xenguest.o libxl_utils.o $(LIBXL_OBJS-y)
 
 AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h
tools/libxl/libxl.c +17 -41

95 lines not on this page · raw message

tools/libxl/libxl_blktap2.c new file +51 -0

51 lines not on this page · raw message

tools/libxl/libxl_internal.h +19 -0
diff -r 2760576b0d7c -r 9996d4f06e70 tools/libxl/libxl_internal.h
@@ -249,4 +249,23 @@ _hidden char *libxl_abs_path(libxl_gc *g
 _hidden char *_libxl_domid_to_name(libxl_gc *gc, uint32_t domid);
 _hidden char *_libxl_poolid_to_name(libxl_gc *gc, uint32_t poolid);
 
+/*
+ * blktap2 support
+ */
+
+/* libxl_blktap_enabled:
+ *    return true if blktap/blktap2 support is available.
+ */
+int libxl_blktap_enabled(libxl_ctx *ctx);
+
+/* libxl_blktap_devpath:
+ *    Argument: path and disk image as specified in config file.
+ *      The type specifies whether this is aio, qcow, qcow2, etc.
+ *    returns device path xenstore wants to have. returns NULL
+ *      if no device corresponds to the disk.
+ */
+const char *libxl_blktap_devpath(libxl_ctx *ctx,
+                                 const char *disk,
+                                 libxl_disk_phystype phystype);
+
 #endif
tools/libxl/libxl_noblktap2.c new file +29 -0
diff -r 2760576b0d7c -r 9996d4f06e70 tools/libxl/libxl_noblktap2.c
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2010      Advanced Micro Devices
+ * Author Christoph Egger <[email protected]>
+ *
+ * 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.
+ *
+ * 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.h"
+#include "libxl_osdeps.h"
+#include "libxl_internal.h"
+
+int libxl_blktap_enabled(libxl_ctx *ctx)
+{
+    return 0;
+}
+
+const char *libxl_blktap_devpath(libxl_ctx *ctx,
+                                 const char *disk,
+                                 libxl_disk_phystype phystype)
+{
+    return NULL;
+}

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Nacked-by on v1 1/1 NAK
#1 lore raw

Christoph Egger writes ("[Xen-devel] [PATCH] xl: Make blktap support optional"):

Quoted 2 lines

Make blktap support optional.
Enable it by default on Linux, disable it on non-Linux.

This patch still contains all the mixed up code motion, formatting and ifdefs that I criticised in previous instances.

Nacked-by Ian Jackson <[email protected]>

Ian.

#2 lore raw
In reply to Christoph Egger
Quoted 35 lines · Christoph Egger

diff -r 2760576b0d7c -r 9996d4f06e70 tools/libxl/libxl_blktap2.c

tools/libxl/libxl_blktap2.c new file +31 -0
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2010      Advanced Micro Devices
+ * Author Christoph Egger <[email protected]>
+ *
+ * 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.
+ *
+ * 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.h"
+#include "libxl_osdeps.h"
+#include "libxl_internal.h"
+
+#include "tap-ctl.h"
+
+int libxl_blktap_enabled(libxl_ctx *ctx)
+{
+    const char *msg;
+    return !tap_ctl_check(&msg);
+}
+
+const char *libxl_blktap_devpath(libxl_ctx *ctx,
+                                 const char *disk,
+                                 libxl_disk_phystype phystype)
+{
+    const char *type;

tools/libxl/libxl_blktap2.c line 41

Quoted 10 lines
tools/libxl/libxl_blktap2.c new file +10 -0
+    char *params, *devname;
+    int minor, err;
+
+    type = device_disk_string_of_phystype(phystype);
+    minor = tap_ctl_find_minor(type, disk);
+    if (minor >= 0) {
+        devname = libxl_sprintf(ctx, "/dev/xen/blktap-2/tapdev%d", minor);
+        if (devname)
+            return devname;
+    }

This actually breaks the build on linux. It's because this part of the patch needs a re-base since libxl_sprintf() etc. no longer takes a ctx argument. You will need to initialise a gc to do the libxl_sprintf() and then strdup() the result for the caller. The parts where you patched out of libxl.c had equivalent changes.

tools/libxl/libxl_blktap2.c line 48

Quoted 6 lines
tools/libxl/libxl_blktap2.c new file +6 -0
+    params = libxl_sprintf(ctx, "%s:%s", type, disk);
+    err = tap_ctl_create(params, &devname);
+    if (!err) {
+        libxl_ptr_add(ctx, devname);
+        return devname;
+    }

libxl_ptr_add will no longer be necessary when re-based as above.

tools/libxl/libxl_blktap2.c line 51

Quoted 3 lines
tools/libxl/libxl_blktap2.c new file +3 -0
+
+    return NULL;
+}

Other than that, which changed under your feet, I think the patch is fine.

#3 lore raw
Quoted 60 lines · Gianni Tedesco
Quoted 35 lines · Christoph Egger

diff -r 2760576b0d7c -r 9996d4f06e70 tools/libxl/libxl_blktap2.c

tools/libxl/libxl_blktap2.c new file +31 -0
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2010      Advanced Micro Devices
+ * Author Christoph Egger <[email protected]>
+ *
+ * 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.
+ *
+ * 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.h"
+#include "libxl_osdeps.h"
+#include "libxl_internal.h"
+
+#include "tap-ctl.h"
+
+int libxl_blktap_enabled(libxl_ctx *ctx)
+{
+    const char *msg;
+    return !tap_ctl_check(&msg);
+}
+
+const char *libxl_blktap_devpath(libxl_ctx *ctx,
+                                 const char *disk,
+                                 libxl_disk_phystype phystype)
+{
+    const char *type;

tools/libxl/libxl_blktap2.c line 41

Quoted 10 lines
tools/libxl/libxl_blktap2.c new file +10 -0
+    char *params, *devname;
+    int minor, err;
+
+    type = device_disk_string_of_phystype(phystype);
+    minor = tap_ctl_find_minor(type, disk);
+    if (minor >= 0) {
+        devname = libxl_sprintf(ctx, "/dev/xen/blktap-2/tapdev%d", minor);
+        if (devname)
+            return devname;
+    }

This actually breaks the build on linux. It's because this part of the patch needs a re-base since libxl_sprintf() etc. no longer takes a ctx argument. You will need to initialise a gc to do the libxl_sprintf() and then strdup() the result for the caller. The parts where you patched out of libxl.c had equivalent changes.

tools/libxl/libxl_blktap2.c line 48

Quoted 6 lines
tools/libxl/libxl_blktap2.c new file +6 -0
+    params = libxl_sprintf(ctx, "%s:%s", type, disk);
+    err = tap_ctl_create(params, &devname);
+    if (!err) {
+        libxl_ptr_add(ctx, devname);
+        return devname;
+    }
Quoted 8 lines

libxl_ptr_add will no longer be necessary when re-based as above.

tools/libxl/libxl_blktap2.c line 51

Quoted 3 lines
tools/libxl/libxl_blktap2.c new file +3 -0
+
+    return NULL;
+}

Other than that, which changed under your feet, I think the patch is fine.

I'll rebase, fix and apply this patch myself today.

#4 lore raw
In reply to Gianni Tedesco
Quoted 44 lines · Gianni Tedesco
Quoted 43 lines · Christoph Egger

diff -r 2760576b0d7c -r 9996d4f06e70 tools/libxl/libxl_blktap2.c

tools/libxl/libxl_blktap2.c new file +39 -0
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2010      Advanced Micro Devices
+ * Author Christoph Egger <[email protected]>
+ *
+ * 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.
+ *
+ * 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.h"
+#include "libxl_osdeps.h"
+#include "libxl_internal.h"
+
+#include "tap-ctl.h"
+
+int libxl_blktap_enabled(libxl_ctx *ctx)
+{
+    const char *msg;
+    return !tap_ctl_check(&msg);
+}
+
+const char *libxl_blktap_devpath(libxl_ctx *ctx,
+                                 const char *disk,
+                                 libxl_disk_phystype phystype)
+{
+    const char *type;
+    char *params, *devname;
+    int minor, err;
+
+    type = device_disk_string_of_phystype(phystype);
+    minor = tap_ctl_find_minor(type, disk);
+    if (minor >= 0) {
+        devname = libxl_sprintf(ctx, "/dev/xen/blktap-2/tapdev%d", minor);
+        if (devname)
Quoted 9 lines

tools/libxl/libxl_blktap2.c line 41

Quoted 2 lines
tools/libxl/libxl_blktap2.c new file +2 -0
+            return devname;
+    }

This actually breaks the build on linux. It's because this part of the patch needs a re-base since libxl_sprintf() etc. no longer takes a ctx argument. You will need to initialise a gc to do the libxl_sprintf() and then strdup() the result for the caller. The parts where you patched out of libxl.c had equivalent changes.

considering that these are internal function, I'll pass a libxl_gc instead of a libxl_ctx.