Re: [PATCH] libxl: blktap2 portiblity fixes

[Christoph Egger <Christoph.Egger@amd.com>]

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

On Friday 23 July 2010 20:05:50 Ian Jackson wrote:
> Christoph Egger writes ("Re: [Xen-devel] [PATCH] libxl: blktap2 portiblity 
fixes"):
> > New corrected version is attached. It also reverts
> > c/s 21835 as this showed up due to this bug in my tree.
> >
> > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
>
> I applied this, but I found that it broke the build.  I have reverted
> these three patches of yours:
>
>   df9d8319bd37 Fix blktap2 NetBSD build and also revert broken change
>   e76befc7fe2d portability fixes from tools/console
>   24277e3237ca Fix linking error when creating the xl binary.

Can you use the c/s numbers, please?

It was not necessary to backout c/s 21834 as this wasn't the root cause.

> Please come back with a proper fix.

Attached.

Re-include c/s 21834 which takes over portability fixes from tools/console
to make libxl_bootloader.c build on NetBSD. Also use $(UTIL_LIBS) in the
Makefile as done in tools/console/Makefile.

blktapctl is build on Linux only. Introduce two API functions
(libxl_blktap_*) which I have discussed with Stefano
in end of June/beginning of July.
Implement the Linux API by moving the
Linux specific code from libxl.c into libxl_linux.c.

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

> that doesn't break the build 
   ... on Linux you mean ?

> Then I'll test and apply it.

Christoph


-- 
---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

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

diff -r 2b768d52bc7f tools/Rules.mk
--- a/tools/Rules.mk	Sun Jul 25 22:20:47 2010 +0100
+++ b/tools/Rules.mk	Mon Jul 26 12:41:02 2010 +0200
@@ -26,8 +26,13 @@ LDFLAGS_libxenguest = -L$(XEN_LIBXC) -lx
 CFLAGS_libxenstore = -I$(XEN_XENSTORE) $(CFLAGS_include)
 LDFLAGS_libxenstore = -L$(XEN_XENSTORE) -lxenstore
 
+ifeq ($(CONFIG_Linux),y)
 CFLAGS_libblktapctl = -I$(XEN_BLKTAP2)/control -I$(XEN_BLKTAP2)/include $(CFLAGS_include)
 LDFLAGS_libblktapctl = -L$(XEN_BLKTAP2)/control -lblktapctl
+else
+CFLAGS_libblktapctl =
+LDFLAGS_libblktapctl =
+endif
 
 X11_LDPATH = -L/usr/X11R6/$(LIBLEAFDIR)
 
diff -r 2b768d52bc7f tools/libxl/Makefile
--- a/tools/libxl/Makefile	Sun Jul 25 22:20:47 2010 +0100
+++ b/tools/libxl/Makefile	Mon Jul 26 12:41:02 2010 +0200
@@ -15,9 +15,11 @@ CFLAGS += -Werror -Wno-format-zero-lengt
 CFLAGS += -I. -fPIC
 CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl)
 
-LIBS = $(LDFLAGS_libxenctrl) $(LDFLAGS_libxenguest) $(LDFLAGS_libxenstore) $(LDFLAGS_libblktapctl) -lutil
+LIBS = $(LDFLAGS_libxenctrl) $(LDFLAGS_libxenguest) $(LDFLAGS_libxenstore) $(LDFLAGS_libblktapctl) $(UTIL_LIBS)
 
 LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o
+LIBXL_OBJS-$(CONFIG_Linux) += libxl_linux.o
+LIBXL_OBJS-$(CONFIG_NetBSD) += libxl_netbsd.o
 LIBXL_OBJS = flexarray.o libxl.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
diff -r 2b768d52bc7f tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Sun Jul 25 22:20:47 2010 +0100
+++ b/tools/libxl/libxl.c	Mon Jul 26 12:41:02 2010 +0200
@@ -34,7 +34,6 @@
 #include "libxl_utils.h"
 #include "libxl_internal.h"
 #include "flexarray.h"
-#include "tap-ctl.h"
 
 #define PAGE_TO_MEMKB(pages) ((pages) * 4)
 
@@ -1391,27 +1390,6 @@ int libxl_confirm_device_model_startup(s
 
 /******************************************************************************/
 
-static char *get_blktap2_device(struct libxl_ctx *ctx,
-				const char *name, const char *type)
-{
-    int minor = tap_ctl_find_minor(type, name);
-    if (minor < 0)
-        return NULL;
-    return libxl_sprintf(ctx, "/dev/xen/blktap-2/tapdev%d", minor);
-}
-
-static char *make_blktap2_device(struct libxl_ctx *ctx,
-				 const char *name, const char *type)
-{
-    char *params, *devname = NULL;
-    int err;
-    params = libxl_sprintf(ctx, "%s:%s", type, name);
-    err = tap_ctl_create(params, &devname);
-    if (!err)
-        libxl_ptr_add(ctx, devname);
-    return err ? NULL : devname;
-}
-
 int libxl_device_disk_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
 {
     flexarray_t *front;
@@ -1460,14 +1438,13 @@ int libxl_device_disk_add(struct libxl_c
         case PHYSTYPE_FILE:
             /* let's pretend is tap:aio for the moment */
             disk->phystype = PHYSTYPE_AIO;
-        case PHYSTYPE_AIO: case PHYSTYPE_QCOW: case PHYSTYPE_QCOW2: case PHYSTYPE_VHD: {
-            const char *msg;
-            if (!tap_ctl_check(&msg)) {
-                const char *type = device_disk_string_of_phystype(disk->phystype);
-                char *dev;
-                dev = get_blktap2_device(ctx, disk->physpath, type);
-                if (!dev)
-                    dev = make_blktap2_device(ctx, disk->physpath, type);
+        case PHYSTYPE_AIO:
+        case PHYSTYPE_QCOW:
+        case PHYSTYPE_QCOW2:
+        case PHYSTYPE_VHD:
+            if (libxl_blktap_enabled(ctx)) {
+                const char *dev = libxl_blktap_devpath(ctx,
+                                               disk->physpath, disk->phystype);
                 if (!dev)
                     return ERROR_FAIL;
                 flexarray_set(back, boffset++, "tapdisk-params");
@@ -1488,7 +1465,7 @@ int libxl_device_disk_add(struct libxl_c
 
             device.backend_kind = DEVICE_TAP;
             break;
-        }
+
         default:
             XL_LOG(ctx, XL_LOG_ERROR, "unrecognized disk physical type: %d\n", disk->phystype);
             return ERROR_INVAL;
@@ -1552,7 +1529,7 @@ int libxl_device_disk_del(struct libxl_c
 
 const char * libxl_device_disk_local_attach(struct libxl_ctx *ctx, libxl_device_disk *disk)
 {
-    char *dev = NULL;
+    const char *dev = NULL;
     int phystype = disk->phystype;
     switch (phystype) {
         case PHYSTYPE_PHY: {
@@ -1563,16 +1540,14 @@ const char * libxl_device_disk_local_att
         case PHYSTYPE_FILE:
             /* let's pretend is tap:aio for the moment */
             phystype = PHYSTYPE_AIO;
-        case PHYSTYPE_AIO: case PHYSTYPE_QCOW: case PHYSTYPE_QCOW2: case PHYSTYPE_VHD: {
-            const char *msg;
-            if (!tap_ctl_check(&msg)) {
-                const char *type = device_disk_string_of_phystype(phystype);
-                dev = get_blktap2_device(ctx, disk->physpath, type);
-                if (!dev)
-                    dev = make_blktap2_device(ctx, disk->physpath, type);
-            }
+        case PHYSTYPE_AIO:
+        case PHYSTYPE_QCOW:
+        case PHYSTYPE_QCOW2:
+        case PHYSTYPE_VHD:
+            if (libxl_blktap_enabled(ctx))
+                dev = libxl_blktap_devpath(ctx, disk->physpath, phystype);
             break;
-        }
+
         default:
             XL_LOG(ctx, XL_LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
             break;
diff -r 2b768d52bc7f tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c	Sun Jul 25 22:20:47 2010 +0100
+++ b/tools/libxl/libxl_bootloader.c	Mon Jul 26 12:41:02 2010 +0200
@@ -15,9 +15,16 @@
 #include "libxl_osdeps.h"
 
 #include <string.h>
-#include <pty.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <termios.h>
+#if defined(__NetBSD__) || defined(__OpenBSD__)
+#include <util.h>
+#elif defined(__linux__)
+#include <pty.h>
+#elif defined(__sun__)
+#include <stropts.h>
+#endif
 
 #include <sys/stat.h>
 #include <sys/types.h>
diff -r 2b768d52bc7f tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Sun Jul 25 22:20:47 2010 +0100
+++ b/tools/libxl/libxl_device.c	Mon Jul 26 12:41:02 2010 +0200
@@ -123,7 +123,7 @@ char *device_disk_backend_type_of_physty
     }
 }
 
-int device_physdisk_major_minor(char *physpath, int *major, int *minor)
+int device_physdisk_major_minor(const char *physpath, int *major, int *minor)
 {
     struct stat buf;
     if (stat(physpath, &buf) < 0)
diff -r 2b768d52bc7f tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Sun Jul 25 22:20:47 2010 +0100
+++ b/tools/libxl/libxl_internal.h	Mon Jul 26 12:41:02 2010 +0200
@@ -149,7 +149,7 @@ void libxl__userdata_destroyall(struct l
 char *device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
 char *device_disk_string_of_phystype(libxl_disk_phystype phystype);
 
-int device_physdisk_major_minor(char *physpath, int *major, int *minor);
+int device_physdisk_major_minor(const char *physpath, int *major, int *minor);
 int device_disk_dev_number(char *virtpath);
 
 int libxl_device_generic_add(struct libxl_ctx *ctx, libxl_device *device,
diff -r 2b768d52bc7f tools/libxl/libxl_linux.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxl/libxl_linux.c	Mon Jul 26 12:41:02 2010 +0200
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2010      Advanced Micro Devices
+ * Author Christoph Egger <Christoph.Egger@amd.com>
+ *
+ * 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 "tap-ctl.h"
+
+int libxl_blktap_enabled(struct libxl_ctx *ctx)
+{
+    const char *msg;
+    return !tap_ctl_check(&msg);
+}
+
+const char *libxl_blktap_devpath(struct 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)
+            return devname;
+    }
+
+    params = libxl_sprintf(ctx, "%s:%s", type, disk);
+    err = tap_ctl_create(params, &devname);
+    if (!err) {
+        libxl_ptr_add(ctx, devname);
+        return devname;
+    }
+
+    return NULL;
+}
diff -r 2b768d52bc7f tools/libxl/libxl_netbsd.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tools/libxl/libxl_netbsd.c	Mon Jul 26 12:41:02 2010 +0200
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2010      Advanced Micro Devices
+ * Author Christoph Egger <Christoph.Egger@amd.com>
+ *
+ * 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"
+
+int libxl_blktap_enabled(struct libxl_ctx *ctx)
+{
+    return 0;
+}
+
+const char *libxl_blktap_devpath(struct libxl_ctx *ctx,
+                                 const char *disk,
+                                 libxl_disk_phystype phystype)
+{
+    return NULL;
+}
diff -r 2b768d52bc7f tools/libxl/libxl_osdeps.h
--- a/tools/libxl/libxl_osdeps.h	Sun Jul 25 22:20:47 2010 +0100
+++ b/tools/libxl/libxl_osdeps.h	Mon Jul 26 12:41:02 2010 +0200
@@ -23,6 +23,8 @@
 
 #define _GNU_SOURCE
 
+#include <libxl_internal.h>
+
 #ifdef NEED_OWN_ASPRINTF
 #include <stdarg.h>
 
@@ -30,4 +32,23 @@ int asprintf(char **buffer, char *fmt, .
 int vasprintf(char **buffer, const char *fmt, va_list ap);
 #endif /*NEED_OWN_ASPRINTF*/
 
+/*
+ * blktap2 support
+ */
+
+/* libxl_blktap_enabled:
+ *    return true if blktap/blktap2 support is available.
+ */
+int libxl_blktap_enabled(struct 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(struct libxl_ctx *ctx,
+                                 const char *disk,
+                                 libxl_disk_phystype phystype);
+
 #endif

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

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