xl disk configuration handling

PATCH In review Kamala Narasimhan 4 participants 19 messages Feb 7, 2011 – Feb 15, 2011 Arm/actions semi architecture

lore

Share & tools

lore

Kamala Narasimhan

author

2011-02-07 21:15 UTC #0 lore raw

cover xl disk configuration handling

Refactor xl disk configuration handling.

Patch 1/5 - "Xl Disk Configuration Option" documentation Patch 2/5 - Xl interface change plus changes to code it impacts Patch 3/5 - Xl disk configuration parsing changes Patch 4/5 - Xl block-attach refactoring to reuse xl parsing code Patch 5/5 - More disk validation checks

Note: I am nervous about this patch series as the changes are huge and I am worried about regressions especially this close to 4.1. I can try and fix the regressions fast but only after they are found and reported. So, I would encourage folks to run their standard config files that they knew worked before against the latest changes and report any/all regressions. Thanks!

Kamala

Kamala Narasimhan

author

2011-02-09 18:21 UTC #1 lore raw

PATCH 2/5 Xl interface change plus changes to code it impacts

Description:

This patch refactors xl disk specific interface to separate disk format and backend types, rename some variables, changes code that is impacted by this interface change.

Signed-off-by Kamala Narasimhan <[email protected]>

Kamala

PS: This patch incorporates all the comments made so far that are specific to this patch.

tools/libxl/libxl.c +77 ~~-68~~

diff -r 1967c7c290eb tools/libxl/libxl.c
@@ -588,7 +588,7 @@ int libxl_wait_for_disk_ejects(libxl_ctx
     for (i = 0; i < num_disks; i++) {
         if (asprintf(&(waiter[i].path), "%s/device/vbd/%d/eject",
                      libxl__xs_get_dompath(&gc, domid),
-                     libxl__device_disk_dev_number(disks[i].virtpath)) < 0)
+                     libxl__device_disk_dev_number(disks[i].vdev)) < 0)
             goto out;
         if (asprintf(&(waiter[i].token), "%d", LIBXL_EVENT_DISK_EJECT) < 0)
             goto out;

@@ -668,10 +668,10 @@ int libxl_event_get_disk_eject_info(libx
 
     disk->backend_domid = 0;
     disk->domid = domid;
-    disk->physpath = strdup("");
-    disk->phystype = PHYSTYPE_EMPTY;
+    disk->pdev_path = strdup("");
+    disk->format = DISK_FORMAT_EMPTY;
     /* this value is returned to the user: do not free right away */
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
     disk->unpluggable = 1;
     disk->readwrite = 0;
     disk->is_cdrom = 1;

@@ -863,24 +863,25 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, libxl_disk_phystype disk_type)
+static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
+    libxl_disk_backend backend_type, libxl_disk_format format)
 {
     struct stat stat_buf;
 
-    if ( (file_name[0] == '\0') && (disk_type == PHYSTYPE_EMPTY) )
+    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
         return 0;
 
-    if ( stat(file_name, &stat_buf) != 0 ) {
+    if (stat(file_name, &stat_buf) != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;
     }
-    if ( disk_type == PHYSTYPE_PHY ) {
+    if (backend_type == DISK_BACKEND_PHY) {
         if ( !(S_ISBLK(stat_buf.st_mode)) ) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n",
                 file_name);
             return ERROR_INVAL;
         }
-    } else if ( S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0 ) {
+    } else if (S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is 0!\n", file_name);
         return ERROR_INVAL;
     }

@@ -898,7 +899,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__device device;
     int major, minor, rc;
 
-    rc = validate_virtual_disk(ctx, disk->physpath, disk->phystype);
+    rc = validate_virtual_disk(ctx, disk->pdev_path, disk->backend, 
+             disk->format);
     if (rc)
         return rc;

@@ -913,11 +915,11 @@ int libxl_device_disk_add(libxl_ctx *ctx
         goto out_free;
     }
 
-    backend_type = libxl__device_disk_backend_type_of_phystype(disk->phystype);
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    backend_type = libxl__device_disk_string_of_backend(disk->backend);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     if (devid==-1) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->virtpath);
+               " virtual disk identifier %s", disk->vdev);
         rc = ERROR_INVAL;
         goto out_free;
     }

@@ -928,37 +930,33 @@ int libxl_device_disk_add(libxl_ctx *ctx
     device.domid = disk->domid;
     device.kind = DEVICE_VBD;
 
-    switch (disk->phystype) {
-        case PHYSTYPE_PHY: {
-
-            libxl__device_physdisk_major_minor(disk->physpath, &major, &minor);
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
             flexarray_append(back, "physical-device");
             flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
 
             flexarray_append(back, "params");
-            flexarray_append(back, disk->physpath);
+            flexarray_append(back, disk->pdev_path);
 
             device.backend_kind = DEVICE_VBD;
             break;
         }
-        case PHYSTYPE_EMPTY:
-            break;
-        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:
+        case DISK_BACKEND_TAP:
+        case DISK_BACKEND_QDISK: {
+            if (disk->format == DISK_FORMAT_EMPTY)
+                break;
             if (libxl__blktap_enabled(&gc)) {
                 const char *dev = libxl__blktap_devpath(&gc,
-                                               disk->physpath, disk->phystype);
+                                               disk->pdev_path, disk->format);
                 if (!dev) {
                     rc = ERROR_FAIL;
                     goto out_free;
                 }
                 flexarray_append(back, "tapdisk-params");
-                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", 
+                    libxl__device_disk_string_of_format(disk->format), 
+                    disk->pdev_path));
                 flexarray_append(back, "params");
                 flexarray_append(back, libxl__strdup(&gc, dev));
                 backend_type = "phy";

@@ -971,16 +969,17 @@ int libxl_device_disk_add(libxl_ctx *ctx
             }
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
-                          libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
 
-            if (libxl__blktap_enabled(&gc))
+            if (libxl__blktap_enabled(&gc) && 
+                 disk->format != DISK_BACKEND_QDISK)
                 device.backend_kind = DEVICE_TAP;
             else
                 device.backend_kind = DEVICE_QDISK;
             break;
-
+        }
         default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", disk->phystype);
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
             rc = ERROR_INVAL;
             goto out_free;
     }

@@ -996,7 +995,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_append(back, "state");
     flexarray_append(back, libxl__sprintf(&gc, "%d", 1));
     flexarray_append(back, "dev");
-    flexarray_append(back, disk->virtpath);
+    flexarray_append(back, disk->vdev);
     flexarray_append(back, "type");
     flexarray_append(back, backend_type);
     flexarray_append(back, "mode");

@@ -1036,11 +1035,11 @@ int libxl_device_disk_del(libxl_ctx *ctx
     libxl__device device;
     int devid;
 
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     = 
-        (disk->phystype == PHYSTYPE_PHY) ? DEVICE_VBD : DEVICE_TAP;
+        (disk->backend == DISK_BACKEND_PHY) ? DEVICE_VBD : DEVICE_TAP;
     device.domid            = disk->domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;

@@ -1052,36 +1051,44 @@ char * libxl_device_disk_local_attach(li
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     const char *dev = NULL;
     char *ret = NULL;
-    int phystype = disk->phystype;
-    switch (phystype) {
-        case PHYSTYPE_PHY: {
-            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->physpath);
-            dev = disk->physpath;
+
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->pdev_path);
+            dev = disk->pdev_path;
             break;
         }
-        case PHYSTYPE_FILE:
-            /* let's pretend is tap:aio for the moment */
-            phystype = PHYSTYPE_AIO;
-        case PHYSTYPE_AIO:
-            if (!libxl__blktap_enabled(&gc)) {
-                dev = disk->physpath;
+        case DISK_BACKEND_TAP: {
+            if (disk->format == DISK_FORMAT_VHD)
+            {
+                if (libxl__blktap_enabled(&gc))
+                    dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->format);
+                else
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
+                break;
+            } else if (disk->format == DISK_FORMAT_QCOW ||
+                       disk->format == DISK_FORMAT_QCOW2) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
+                break;
+            } else {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                    "type: %d\n", disk->backend);
                 break;
             }
-        case PHYSTYPE_VHD:
-            if (libxl__blktap_enabled(&gc))
-                dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
-            else
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
+        }
+        case DISK_BACKEND_QDISK: {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk "
+                "image\n");
             break;
-        case PHYSTYPE_QCOW:
-        case PHYSTYPE_QCOW2:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
+        }
+        case DISK_BACKEND_UNKNOWN:
+        default: {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                "type: %d\n", disk->backend);
             break;
+        }
+    }
 
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
-            break;
-    }
     if (dev != NULL)
         ret = strdup(dev);
     libxl__free_all(&gc);

@@ -1677,13 +1684,15 @@ static unsigned int libxl_append_disk_li
             pdisk->domid = domid;
             physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path, *dir), &len);
             if (physpath_tmp && strchr(physpath_tmp, ':')) {
-                pdisk->physpath = strdup(strchr(physpath_tmp, ':') + 1);
+                pdisk->pdev_path = strdup(strchr(physpath_tmp, ':') + 1);
                 free(physpath_tmp);
             } else {
-                pdisk->physpath = physpath_tmp;
+                pdisk->pdev_path = physpath_tmp;
             }
-            libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->phystype));
-            pdisk->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
+            libxl_string_to_backend(ctx, libxl__xs_read(&gc, XBT_NULL, 
+                libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), 
+                &(pdisk->backend));
+            pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
             pdisk->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir)));
             if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path, *dir)), "w"))
                 pdisk->readwrite = 1;

@@ -1718,7 +1727,7 @@ int libxl_device_disk_getinfo(libxl_ctx 
     char *val;
 
     dompath = libxl__xs_get_dompath(&gc, domid);
-    diskinfo->devid = libxl__device_disk_dev_number(disk->virtpath);
+    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev);
 
     /* tap devices entries in xenstore are written as vbd devices. */
     diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid);

@@ -1752,13 +1761,13 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
     libxl_device_disk *disks;
     int ret = ERROR_FAIL;
 
-    if (!disk->physpath) {
-        disk->physpath = strdup("");
-        disk->phystype = PHYSTYPE_EMPTY;
+    if (!disk->pdev_path) {
+        disk->pdev_path = strdup("");
+        disk->format = DISK_FORMAT_EMPTY;
     }
     disks = libxl_device_disk_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
-        if (disks[i].is_cdrom && !strcmp(disk->virtpath, disks[i].virtpath))
+        if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
             /* found */
             break;
     }

tools/libxl/libxl.h +14 -8

diff -r 1967c7c290eb tools/libxl/libxl.h
@@ -172,14 +172,20 @@ typedef enum {
 } libxl_console_consback;
 
 typedef enum {
-    PHYSTYPE_QCOW = 1,
-    PHYSTYPE_QCOW2,
-    PHYSTYPE_VHD,
-    PHYSTYPE_AIO,
-    PHYSTYPE_FILE,
-    PHYSTYPE_PHY,
-    PHYSTYPE_EMPTY,
-} libxl_disk_phystype;
+    DISK_FORMAT_UNKNOWN = 0,
+    DISK_FORMAT_QCOW,
+    DISK_FORMAT_QCOW2,
+    DISK_FORMAT_VHD,
+    DISK_FORMAT_RAW,
+    DISK_FORMAT_EMPTY,
+} libxl_disk_format;
+
+typedef enum {
+    DISK_BACKEND_UNKNOWN = 0,
+    DISK_BACKEND_PHY,
+    DISK_BACKEND_TAP,
+    DISK_BACKEND_QDISK,
+} libxl_disk_backend;
 
 typedef enum {
     NICTYPE_IOEMU = 1,

tools/libxl/libxl.idl +6 -4

diff -r 1967c7c290eb tools/libxl/libxl.idl
@@ -11,7 +11,8 @@ libxl_qemu_machine_type = Number("qemu_m
 libxl_qemu_machine_type = Number("qemu_machine_type", namespace="libxl_")
 libxl_console_consback = Number("console_consback", namespace="libxl_")
 libxl_console_constype = Number("console_constype", namespace="libxl_")
-libxl_disk_phystype = Number("disk_phystype", namespace="libxl_")
+libxl_disk_format = Number("disk_format", namespace="libxl_")
+libxl_disk_backend = Number("disk_backend", namespace="libxl_")
 libxl_nic_type = Number("nic_type", namespace="libxl_")
 libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE)

@@ -203,9 +204,10 @@ libxl_device_disk = Struct("device_disk"
 libxl_device_disk = Struct("device_disk", [
     ("backend_domid", uint32),
     ("domid", domid),
-    ("physpath", string),
-    ("phystype", libxl_disk_phystype),
-    ("virtpath", string),
+    ("pdev_path", string),
+    ("vdev", string),
+    ("backend", libxl_disk_backend),
+    ("format", libxl_disk_format),
     ("unpluggable", integer),
     ("readwrite", integer),
     ("is_cdrom", integer),

tools/libxl/libxl_blktap2.c +2 -2

diff -r 1967c7c290eb tools/libxl/libxl_blktap2.c
@@ -26,13 +26,13 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     const char *type;
     char *params, *devname = NULL;
     int minor, err;
 
-    type = libxl__device_disk_string_of_phystype(phystype);
+    type = libxl__device_disk_string_of_format(format);
     minor = tap_ctl_find_minor(type, disk);
     if (minor >= 0) {
         devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", minor);

tools/libxl/libxl_device.c +13 ~~-20~~

diff -r 1967c7c290eb tools/libxl/libxl_device.c
@@ -121,31 +121,24 @@ out:
     return rc;
 }
 
-char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_format(libxl_disk_format format)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "qcow";
-        case PHYSTYPE_QCOW2: return "qcow2";
-        case PHYSTYPE_VHD: return "vhd";
-        case PHYSTYPE_AIO: return "aio";
-        case PHYSTYPE_FILE: return "file";
-        case PHYSTYPE_PHY: return "phy";
-        case PHYSTYPE_EMPTY: return "file";
-        default: return NULL;
+    switch (format) {
+        case DISK_FORMAT_QCOW: return "qcow";
+        case DISK_FORMAT_QCOW2: return "qcow2"; 
+        case DISK_FORMAT_VHD: return "vhd"; 
+        case DISK_FORMAT_RAW:
+        case DISK_FORMAT_EMPTY: return "aio"; 
+        default: return NULL; 
     }
 }
 
-char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "tap";
-        case PHYSTYPE_QCOW2: return "tap";
-        case PHYSTYPE_VHD: return "tap";
-        case PHYSTYPE_AIO: return "tap";
-        /* let's pretend file is tap:aio */
-        case PHYSTYPE_FILE: return "tap";
-        case PHYSTYPE_EMPTY: return "tap";
-        case PHYSTYPE_PHY: return "phy";
+    switch (backend) {
+        case DISK_BACKEND_QDISK: return "qdisk";
+        case DISK_BACKEND_TAP: return "tap";
+        case DISK_BACKEND_PHY: return "phy";
         default: return NULL;
     }
 }

tools/libxl/libxl_dm.c +3 -3

diff -r 1967c7c290eb tools/libxl/libxl_dm.c
@@ -316,10 +316,10 @@ static char ** libxl_build_device_model_
         for (i; i < nb; i++) {
             if (disks[i].is_cdrom) {
                 flexarray_append(dm_args, "-cdrom");
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].virtpath));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             }
             libxl_device_disk_destroy(&disks[i]);
         }

tools/libxl/libxl_internal.h +3 -3

diff -r 1967c7c290eb tools/libxl/libxl_internal.h
@@ -178,8 +178,8 @@ _hidden void libxl__userdata_destroyall(
 _hidden void libxl__userdata_destroyall(libxl_ctx *ctx, uint32_t domid);
 
 /* from xl_device */
-_hidden char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
-_hidden char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype);
+_hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
+_hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
 _hidden int libxl__device_disk_dev_number(char *virtpath);

@@ -306,7 +306,7 @@ _hidden int libxl__blktap_enabled(libxl_
  */
 _hidden const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype);
+                                 libxl_disk_format format);
 
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);

tools/libxl/libxl_noblktap2.c +1 -1

diff -r 1967c7c290eb tools/libxl/libxl_noblktap2.c
@@ -23,7 +23,7 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     return NULL;
 }

tools/libxl/libxl_utils.c +10 ~~-12~~

diff -r 1967c7c290eb tools/libxl/libxl_utils.c
@@ -275,15 +275,15 @@ out:
     return rc;
 }
 
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype)
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend)
 {
     char *p;
     int rc = 0;
 
     if (!strcmp(s, "phy")) {
-        *phystype = PHYSTYPE_PHY;
+        *backend = DISK_BACKEND_PHY;
     } else if (!strcmp(s, "file")) {
-        *phystype = PHYSTYPE_FILE;
+        *backend = DISK_BACKEND_TAP;
     } else if (!strcmp(s, "tap")) {
         p = strchr(s, ':');
         if (!p) {

@@ -291,14 +291,12 @@ int libxl_string_to_phystype(libxl_ctx *
             goto out;
         }
         p++;
-        if (!strcmp(p, "aio")) {
-            *phystype = PHYSTYPE_AIO;
-        } else if (!strcmp(p, "vhd")) {
-            *phystype = PHYSTYPE_VHD;
+        if (!strcmp(p, "vhd")) {
+            *backend = DISK_BACKEND_TAP;
         } else if (!strcmp(p, "qcow")) {
-            *phystype = PHYSTYPE_QCOW;
+            *backend = DISK_BACKEND_QDISK;
         } else if (!strcmp(p, "qcow2")) {
-            *phystype = PHYSTYPE_QCOW2;
+            *backend = DISK_BACKEND_QDISK;
         }
     }
 out:

@@ -553,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx
     disk->backend_domid = strtoul(val, NULL, 10);
     disk->domid = domid;
     be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath));
-    disk->physpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
+    disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
-    libxl_string_to_phystype(ctx, val, &(disk->phystype));
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
+    libxl_string_to_backend(ctx, val, &(disk->backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path));
     disk->unpluggable = !strcmp(val, "1");
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path));

tools/libxl/libxl_utils.h +1 -1

diff -r 1967c7c290eb tools/libxl/libxl_utils.h
@@ -29,7 +29,7 @@ int libxl_get_stubdom_id(libxl_ctx *ctx,
 int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid);
 int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid);
 int libxl_create_logfile(libxl_ctx *ctx, char *name, char **full_name);
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype);
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend);
 
 int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
                              void **data_r, int *datalen_r);

tools/libxl/xl_cmdimpl.c +58 ~~-49~~

215 lines not on this page · raw message

tools/python/xen/lowlevel/xl/xl.c +11 -6

diff -r 1967c7c290eb tools/python/xen/lowlevel/xl/xl.c
@@ -779,12 +779,17 @@ PyMODINIT_FUNC initxl(void)
     _INT_CONST_LIBXL(m, CONSBACK_XENCONSOLED);
     _INT_CONST_LIBXL(m, CONSBACK_IOEMU);
 
-    _INT_CONST(m, PHYSTYPE_QCOW);
-    _INT_CONST(m, PHYSTYPE_QCOW2);
-    _INT_CONST(m, PHYSTYPE_VHD);
-    _INT_CONST(m, PHYSTYPE_AIO);
-    _INT_CONST(m, PHYSTYPE_FILE);
-    _INT_CONST(m, PHYSTYPE_PHY);
+    _INT_CONST(m, DISK_FORMAT_UNKNOWN);
+    _INT_CONST(m, DISK_FORMAT_QCOW);
+    _INT_CONST(m, DISK_FORMAT_QCOW2);
+    _INT_CONST(m, DISK_FORMAT_VHD);
+    _INT_CONST(m, DISK_FORMAT_RAW);
+    _INT_CONST(m, DISK_FORMAT_EMPTY);
+
+    _INT_CONST(m, DISK_BACKEND_UNKNOWN);
+    _INT_CONST(m, DISK_BACKEND_PHY);
+    _INT_CONST(m, DISK_BACKEND_TAP);
+    _INT_CONST(m, DISK_BACKEND_QDISK);
 
     _INT_CONST(m, NICTYPE_IOEMU);
     _INT_CONST(m, NICTYPE_VIF);

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

Ian Campbell

2011-02-10 09:23 UTC #2 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

tools/libxl/libxl.c int libxl_vncviewer_exec(libxl_ctx *ctx, · line 872

Quoted 11 lines

-static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, libxl_disk_phystype disk_type)

tools/libxl/libxl.c +3 -1

+static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
+    libxl_disk_backend backend_type, libxl_disk_format format)
 {
     struct stat stat_buf;
 
-    if ( (file_name[0] == '\0') && (disk_type == PHYSTYPE_EMPTY) )
+    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
         return 0;

If format == DISK_FORMAT_EMPTY then does the content of file_name matter?

Alternatively, if format == DISK_FORMAT_EMPTY then is file_name != "" actually an error?

Ian.

Kamala Narasimhan

author

2011-02-10 14:44 UTC #3 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Quoted 7 lines · Ian Campbell

Quoted 7 lines
-static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, libxl_disk_phystype disk_type)
tools/libxl/libxl.c +2 -0
+static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
+    libxl_disk_backend backend_type, libxl_disk_format format)
 {
     struct stat stat_buf;

Quoted 7 lines

tools/libxl/libxl.c int libxl_vncviewer_exec(libxl_ctx *ctx, · line 872
Quoted 3 lines
tools/libxl/libxl.c +1 -1
-    if ( (file_name[0] == '\0') && (disk_type == PHYSTYPE_EMPTY) )
+    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
         return 0;
If format == DISK_FORMAT_EMPTY then does the content of file_name matter?

Good catch! Just checking for format should do. I will make that change.

Quoted 3 lines

Alternatively, if format == DISK_FORMAT_EMPTY then is file_name != "" actually an error?

Ideally we should never end up that way as our code checks the string before setting format to DISK_FORMAT_EMPTY.

Kamala

Stefano Stabellini

2011-02-10 11:50 UTC #4 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

In reply to Kamala Narasimhan

Quoted 9 lines · Kamala Narasimhan

Description:
This patch refactors xl disk specific interface to separate disk format and backend types, rename some variables, changes code that is impacted by this interface change.
Signed-off-by Kamala Narasimhan <[email protected]>
Kamala
PS: This patch incorporates all the comments made so far that are specific to this patch.

Better, we are almost there.
The only problems I have found are in the changes to libxl_device_disk_local_attach:

Quoted 59 lines

tools/libxl/libxl.c +29 ~~-19~~

@@ -1052,36 +1051,44 @@ char * libxl_device_disk_local_attach(li
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     const char *dev = NULL;
     char *ret = NULL;
-    int phystype = disk->phystype;
-    switch (phystype) {
-        case PHYSTYPE_PHY: {
-            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->physpath);
-            dev = disk->physpath;
+
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->pdev_path);
+            dev = disk->pdev_path;
             break;
         }
-        case PHYSTYPE_FILE:
-            /* let's pretend is tap:aio for the moment */
-            phystype = PHYSTYPE_AIO;
-        case PHYSTYPE_AIO:
-            if (!libxl__blktap_enabled(&gc)) {
-                dev = disk->physpath;
+        case DISK_BACKEND_TAP: {
+            if (disk->format == DISK_FORMAT_VHD)
+            {
+                if (libxl__blktap_enabled(&gc))
+                    dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->format);
+                else
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
+                break;
+            } else if (disk->format == DISK_FORMAT_QCOW ||
+                       disk->format == DISK_FORMAT_QCOW2) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
+                break;
+            } else {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                    "type: %d\n", disk->backend);
                 break;
             }
-        case PHYSTYPE_VHD:
-            if (libxl__blktap_enabled(&gc))
-                dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
-            else
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
+        }
+        case DISK_BACKEND_QDISK: {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk "
+                "image\n");
             break;
-        case PHYSTYPE_QCOW:
-        case PHYSTYPE_QCOW2:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
+        }
+        case DISK_BACKEND_UNKNOWN:
+        default: {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                "type: %d\n", disk->backend);
             break;

tools/libxl/libxl.c char * libxl_device_disk_local_attach(li · line 1094

Quoted 10 lines

tools/libxl/libxl.c +2 -4

+        }
+    }
 
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
-            break;
-    }
     if (dev != NULL)
         ret = strdup(dev);
     libxl__free_all(&gc);

The simple raw file case is not supported anywhere, besides it is possible to attach a qdisk if it is in raw format.
It should be more or less like this:

phy && raw -> OK
phy && anything but raw -> KO
qdisk && raw -> OK
qdisk && anything but raw -> KO
tap && (qcow || qcow2) -> KO
tap && blktap2 enabled && (vhd || raw) -> OK tap && blktap2 disabled && vhd -> KO
tap && blktap2 disabled && raw -> OK (same as qdisk)

Quoted 4 lines

diff -r 1967c7c290eb tools/libxl/xl_cmdimpl.c

tools/libxl/xl_cmdimpl.c static void printf_info(int domid, · line 367

Quoted 10 lines

tools/libxl/xl_cmdimpl.c +3 -3

@@ -361,9 +361,9 @@ static void printf_info(int domid,
         printf("\t\t(tap\n");
         printf("\t\t\t(backend_domid %d)\n", d_config->disks[i].backend_domid);
         printf("\t\t\t(domid %d)\n", d_config->disks[i].domid);
-        printf("\t\t\t(physpath %s)\n", d_config->disks[i].physpath);
-        printf("\t\t\t(phystype %d)\n", d_config->disks[i].phystype);
-        printf("\t\t\t(virtpath %s)\n", d_config->disks[i].virtpath);
+        printf("\t\t\t(physpath %s)\n", d_config->disks[i].pdev_path);
+        printf("\t\t\t(phystype %d)\n", d_config->disks[i].backend);
+        printf("\t\t\t(virtpath %s)\n", d_config->disks[i].vdev);

we should print pdev_path, backend and vdev here

Kamala Narasimhan

author

2011-02-10 17:05 UTC #5 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Stefano - Attached is patch 2/5 with the change you had asked. Please let me know if there are further ones you would suggest.

Description:

This patch refactors xl disk specific interface to separate disk format and backend types, rename some variables, changes code that is impacted by this interface change.

Signed-off-by Kamala Narasimhan <[email protected]>

Kamala

tools/libxl/libxl.c +99 ~~-67~~

diff -r 1967c7c290eb tools/libxl/libxl.c
@@ -588,7 +588,7 @@ int libxl_wait_for_disk_ejects(libxl_ctx
     for (i = 0; i < num_disks; i++) {
         if (asprintf(&(waiter[i].path), "%s/device/vbd/%d/eject",
                      libxl__xs_get_dompath(&gc, domid),
-                     libxl__device_disk_dev_number(disks[i].virtpath)) < 0)
+                     libxl__device_disk_dev_number(disks[i].vdev)) < 0)
             goto out;
         if (asprintf(&(waiter[i].token), "%d", LIBXL_EVENT_DISK_EJECT) < 0)
             goto out;

@@ -668,10 +668,10 @@ int libxl_event_get_disk_eject_info(libx
 
     disk->backend_domid = 0;
     disk->domid = domid;
-    disk->physpath = strdup("");
-    disk->phystype = PHYSTYPE_EMPTY;
+    disk->pdev_path = strdup("");
+    disk->format = DISK_FORMAT_EMPTY;
     /* this value is returned to the user: do not free right away */
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
     disk->unpluggable = 1;
     disk->readwrite = 0;
     disk->is_cdrom = 1;

@@ -863,24 +863,25 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, libxl_disk_phystype disk_type)
+static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
+    libxl_disk_backend backend_type, libxl_disk_format format)
 {
     struct stat stat_buf;
 
-    if ( (file_name[0] == '\0') && (disk_type == PHYSTYPE_EMPTY) )
+    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
         return 0;
 
-    if ( stat(file_name, &stat_buf) != 0 ) {
+    if (stat(file_name, &stat_buf) != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;
     }
-    if ( disk_type == PHYSTYPE_PHY ) {
+    if (backend_type == DISK_BACKEND_PHY) {
         if ( !(S_ISBLK(stat_buf.st_mode)) ) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n",
                 file_name);
             return ERROR_INVAL;
         }
-    } else if ( S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0 ) {
+    } else if (S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is 0!\n", file_name);
         return ERROR_INVAL;
     }

@@ -898,7 +899,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__device device;
     int major, minor, rc;
 
-    rc = validate_virtual_disk(ctx, disk->physpath, disk->phystype);
+    rc = validate_virtual_disk(ctx, disk->pdev_path, disk->backend, 
+             disk->format);
     if (rc)
         return rc;

@@ -913,11 +915,11 @@ int libxl_device_disk_add(libxl_ctx *ctx
         goto out_free;
     }
 
-    backend_type = libxl__device_disk_backend_type_of_phystype(disk->phystype);
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    backend_type = libxl__device_disk_string_of_backend(disk->backend);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     if (devid==-1) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->virtpath);
+               " virtual disk identifier %s", disk->vdev);
         rc = ERROR_INVAL;
         goto out_free;
     }

@@ -928,37 +930,33 @@ int libxl_device_disk_add(libxl_ctx *ctx
     device.domid = disk->domid;
     device.kind = DEVICE_VBD;
 
-    switch (disk->phystype) {
-        case PHYSTYPE_PHY: {
-
-            libxl__device_physdisk_major_minor(disk->physpath, &major, &minor);
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
             flexarray_append(back, "physical-device");
             flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
 
             flexarray_append(back, "params");
-            flexarray_append(back, disk->physpath);
+            flexarray_append(back, disk->pdev_path);
 
             device.backend_kind = DEVICE_VBD;
             break;
         }
-        case PHYSTYPE_EMPTY:
-            break;
-        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:
+        case DISK_BACKEND_TAP:
+        case DISK_BACKEND_QDISK: {
+            if (disk->format == DISK_FORMAT_EMPTY)
+                break;
             if (libxl__blktap_enabled(&gc)) {
                 const char *dev = libxl__blktap_devpath(&gc,
-                                               disk->physpath, disk->phystype);
+                                               disk->pdev_path, disk->format);
                 if (!dev) {
                     rc = ERROR_FAIL;
                     goto out_free;
                 }
                 flexarray_append(back, "tapdisk-params");
-                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", 
+                    libxl__device_disk_string_of_format(disk->format), 
+                    disk->pdev_path));
                 flexarray_append(back, "params");
                 flexarray_append(back, libxl__strdup(&gc, dev));
                 backend_type = "phy";

@@ -971,16 +969,17 @@ int libxl_device_disk_add(libxl_ctx *ctx
             }
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
-                          libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
 
-            if (libxl__blktap_enabled(&gc))
+            if (libxl__blktap_enabled(&gc) && 
+                 disk->format != DISK_BACKEND_QDISK)
                 device.backend_kind = DEVICE_TAP;
             else
                 device.backend_kind = DEVICE_QDISK;
             break;
-
+        }
         default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", disk->phystype);
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
             rc = ERROR_INVAL;
             goto out_free;
     }

@@ -996,7 +995,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_append(back, "state");
     flexarray_append(back, libxl__sprintf(&gc, "%d", 1));
     flexarray_append(back, "dev");
-    flexarray_append(back, disk->virtpath);
+    flexarray_append(back, disk->vdev);
     flexarray_append(back, "type");
     flexarray_append(back, backend_type);
     flexarray_append(back, "mode");

@@ -1036,11 +1035,11 @@ int libxl_device_disk_del(libxl_ctx *ctx
     libxl__device device;
     int devid;
 
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     = 
-        (disk->phystype == PHYSTYPE_PHY) ? DEVICE_VBD : DEVICE_TAP;
+        (disk->backend == DISK_BACKEND_PHY) ? DEVICE_VBD : DEVICE_TAP;
     device.domid            = disk->domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;

@@ -1052,36 +1051,67 @@ char * libxl_device_disk_local_attach(li
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     const char *dev = NULL;
     char *ret = NULL;
-    int phystype = disk->phystype;
-    switch (phystype) {
-        case PHYSTYPE_PHY: {
-            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->physpath);
-            dev = disk->physpath;
+
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "physical block device must"
+                    " be raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching PHY disk %s to domain 0",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
         }
-        case PHYSTYPE_FILE:
-            /* let's pretend is tap:aio for the moment */
-            phystype = PHYSTYPE_AIO;
-        case PHYSTYPE_AIO:
-            if (!libxl__blktap_enabled(&gc)) {
-                dev = disk->physpath;
+        case DISK_BACKEND_TAP: {
+            if (disk->format == DISK_FORMAT_VHD || disk->format == DISK_FORMAT_RAW)
+            {
+                if (libxl__blktap_enabled(&gc))
+                    dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->format);
+                else {
+                    if (disk->format != DISK_FORMAT_RAW) {
+                        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required"
+                            " to open a vhd disk");
+                        break;
+                    } else {
+                        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching tap disk %s to domain 0",
+                            disk->pdev_path);
+                        dev = disk->pdev_path;
+                        break;
+                    }
+                }
+                break;
+            } else if (disk->format == DISK_FORMAT_QCOW ||
+                       disk->format == DISK_FORMAT_QCOW2) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image");
+                break;
+            } else {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                    "type: %d", disk->backend);
                 break;
             }
-        case PHYSTYPE_VHD:
-            if (libxl__blktap_enabled(&gc))
-                dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
-            else
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
-            break;
-        case PHYSTYPE_QCOW:
-        case PHYSTYPE_QCOW2:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
+        }
+        case DISK_BACKEND_QDISK: {
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk "
+                    "image if the format is not raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching qdisk %s to domain 0\n",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
 
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
+        }
+        case DISK_BACKEND_UNKNOWN:
+        default: {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                "type: %d", disk->backend);
             break;
+        }
     }
+
     if (dev != NULL)
         ret = strdup(dev);
     libxl__free_all(&gc);

@@ -1677,13 +1707,15 @@ static unsigned int libxl_append_disk_li
             pdisk->domid = domid;
             physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path, *dir), &len);
             if (physpath_tmp && strchr(physpath_tmp, ':')) {
-                pdisk->physpath = strdup(strchr(physpath_tmp, ':') + 1);
+                pdisk->pdev_path = strdup(strchr(physpath_tmp, ':') + 1);
                 free(physpath_tmp);
             } else {
-                pdisk->physpath = physpath_tmp;
+                pdisk->pdev_path = physpath_tmp;
             }
-            libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->phystype));
-            pdisk->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
+            libxl_string_to_backend(ctx, libxl__xs_read(&gc, XBT_NULL, 
+                libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), 
+                &(pdisk->backend));
+            pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
             pdisk->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir)));
             if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path, *dir)), "w"))
                 pdisk->readwrite = 1;

@@ -1718,7 +1750,7 @@ int libxl_device_disk_getinfo(libxl_ctx 
     char *val;
 
     dompath = libxl__xs_get_dompath(&gc, domid);
-    diskinfo->devid = libxl__device_disk_dev_number(disk->virtpath);
+    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev);
 
     /* tap devices entries in xenstore are written as vbd devices. */
     diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid);

@@ -1752,13 +1784,13 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
     libxl_device_disk *disks;
     int ret = ERROR_FAIL;
 
-    if (!disk->physpath) {
-        disk->physpath = strdup("");
-        disk->phystype = PHYSTYPE_EMPTY;
+    if (!disk->pdev_path) {
+        disk->pdev_path = strdup("");
+        disk->format = DISK_FORMAT_EMPTY;
     }
     disks = libxl_device_disk_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
-        if (disks[i].is_cdrom && !strcmp(disk->virtpath, disks[i].virtpath))
+        if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
             /* found */
             break;
     }

tools/libxl/libxl.h +14 -8

diff -r 1967c7c290eb tools/libxl/libxl.h
@@ -172,14 +172,20 @@ typedef enum {
 } libxl_console_consback;
 
 typedef enum {
-    PHYSTYPE_QCOW = 1,
-    PHYSTYPE_QCOW2,
-    PHYSTYPE_VHD,
-    PHYSTYPE_AIO,
-    PHYSTYPE_FILE,
-    PHYSTYPE_PHY,
-    PHYSTYPE_EMPTY,
-} libxl_disk_phystype;
+    DISK_FORMAT_UNKNOWN = 0,
+    DISK_FORMAT_QCOW,
+    DISK_FORMAT_QCOW2,
+    DISK_FORMAT_VHD,
+    DISK_FORMAT_RAW,
+    DISK_FORMAT_EMPTY,
+} libxl_disk_format;
+
+typedef enum {
+    DISK_BACKEND_UNKNOWN = 0,
+    DISK_BACKEND_PHY,
+    DISK_BACKEND_TAP,
+    DISK_BACKEND_QDISK,
+} libxl_disk_backend;
 
 typedef enum {
     NICTYPE_IOEMU = 1,

tools/libxl/libxl.idl +6 -4

diff -r 1967c7c290eb tools/libxl/libxl.idl
@@ -11,7 +11,8 @@ libxl_qemu_machine_type = Number("qemu_m
 libxl_qemu_machine_type = Number("qemu_machine_type", namespace="libxl_")
 libxl_console_consback = Number("console_consback", namespace="libxl_")
 libxl_console_constype = Number("console_constype", namespace="libxl_")
-libxl_disk_phystype = Number("disk_phystype", namespace="libxl_")
+libxl_disk_format = Number("disk_format", namespace="libxl_")
+libxl_disk_backend = Number("disk_backend", namespace="libxl_")
 libxl_nic_type = Number("nic_type", namespace="libxl_")
 libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE)

@@ -203,9 +204,10 @@ libxl_device_disk = Struct("device_disk"
 libxl_device_disk = Struct("device_disk", [
     ("backend_domid", uint32),
     ("domid", domid),
-    ("physpath", string),
-    ("phystype", libxl_disk_phystype),
-    ("virtpath", string),
+    ("pdev_path", string),
+    ("vdev", string),
+    ("backend", libxl_disk_backend),
+    ("format", libxl_disk_format),
     ("unpluggable", integer),
     ("readwrite", integer),
     ("is_cdrom", integer),

tools/libxl/libxl_blktap2.c +2 -2

diff -r 1967c7c290eb tools/libxl/libxl_blktap2.c
@@ -26,13 +26,13 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     const char *type;
     char *params, *devname = NULL;
     int minor, err;
 
-    type = libxl__device_disk_string_of_phystype(phystype);
+    type = libxl__device_disk_string_of_format(format);
     minor = tap_ctl_find_minor(type, disk);
     if (minor >= 0) {
         devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", minor);

tools/libxl/libxl_device.c +13 ~~-20~~

diff -r 1967c7c290eb tools/libxl/libxl_device.c
@@ -121,31 +121,24 @@ out:
     return rc;
 }
 
-char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_format(libxl_disk_format format)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "qcow";
-        case PHYSTYPE_QCOW2: return "qcow2";
-        case PHYSTYPE_VHD: return "vhd";
-        case PHYSTYPE_AIO: return "aio";
-        case PHYSTYPE_FILE: return "file";
-        case PHYSTYPE_PHY: return "phy";
-        case PHYSTYPE_EMPTY: return "file";
-        default: return NULL;
+    switch (format) {
+        case DISK_FORMAT_QCOW: return "qcow";
+        case DISK_FORMAT_QCOW2: return "qcow2"; 
+        case DISK_FORMAT_VHD: return "vhd"; 
+        case DISK_FORMAT_RAW:
+        case DISK_FORMAT_EMPTY: return "aio"; 
+        default: return NULL; 
     }
 }
 
-char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "tap";
-        case PHYSTYPE_QCOW2: return "tap";
-        case PHYSTYPE_VHD: return "tap";
-        case PHYSTYPE_AIO: return "tap";
-        /* let's pretend file is tap:aio */
-        case PHYSTYPE_FILE: return "tap";
-        case PHYSTYPE_EMPTY: return "tap";
-        case PHYSTYPE_PHY: return "phy";
+    switch (backend) {
+        case DISK_BACKEND_QDISK: return "qdisk";
+        case DISK_BACKEND_TAP: return "tap";
+        case DISK_BACKEND_PHY: return "phy";
         default: return NULL;
     }
 }

tools/libxl/libxl_dm.c +3 -3

diff -r 1967c7c290eb tools/libxl/libxl_dm.c
@@ -316,10 +316,10 @@ static char ** libxl_build_device_model_
         for (i; i < nb; i++) {
             if (disks[i].is_cdrom) {
                 flexarray_append(dm_args, "-cdrom");
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].virtpath));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             }
             libxl_device_disk_destroy(&disks[i]);
         }

tools/libxl/libxl_internal.h +3 -3

diff -r 1967c7c290eb tools/libxl/libxl_internal.h
@@ -178,8 +178,8 @@ _hidden void libxl__userdata_destroyall(
 _hidden void libxl__userdata_destroyall(libxl_ctx *ctx, uint32_t domid);
 
 /* from xl_device */
-_hidden char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
-_hidden char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype);
+_hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
+_hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
 _hidden int libxl__device_disk_dev_number(char *virtpath);

@@ -306,7 +306,7 @@ _hidden int libxl__blktap_enabled(libxl_
  */
 _hidden const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype);
+                                 libxl_disk_format format);
 
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);

tools/libxl/libxl_noblktap2.c +1 -1

diff -r 1967c7c290eb tools/libxl/libxl_noblktap2.c
@@ -23,7 +23,7 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     return NULL;
 }

tools/libxl/libxl_utils.c +10 ~~-12~~

diff -r 1967c7c290eb tools/libxl/libxl_utils.c
@@ -275,15 +275,15 @@ out:
     return rc;
 }
 
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype)
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend)
 {
     char *p;
     int rc = 0;
 
     if (!strcmp(s, "phy")) {
-        *phystype = PHYSTYPE_PHY;
+        *backend = DISK_BACKEND_PHY;
     } else if (!strcmp(s, "file")) {
-        *phystype = PHYSTYPE_FILE;
+        *backend = DISK_BACKEND_TAP;
     } else if (!strcmp(s, "tap")) {
         p = strchr(s, ':');
         if (!p) {

@@ -291,14 +291,12 @@ int libxl_string_to_phystype(libxl_ctx *
             goto out;
         }
         p++;
-        if (!strcmp(p, "aio")) {
-            *phystype = PHYSTYPE_AIO;
-        } else if (!strcmp(p, "vhd")) {
-            *phystype = PHYSTYPE_VHD;
+        if (!strcmp(p, "vhd")) {
+            *backend = DISK_BACKEND_TAP;
         } else if (!strcmp(p, "qcow")) {
-            *phystype = PHYSTYPE_QCOW;
+            *backend = DISK_BACKEND_QDISK;
         } else if (!strcmp(p, "qcow2")) {
-            *phystype = PHYSTYPE_QCOW2;
+            *backend = DISK_BACKEND_QDISK;
         }
     }
 out:

@@ -553,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx
     disk->backend_domid = strtoul(val, NULL, 10);
     disk->domid = domid;
     be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath));
-    disk->physpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
+    disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
-    libxl_string_to_phystype(ctx, val, &(disk->phystype));
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
+    libxl_string_to_backend(ctx, val, &(disk->backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path));
     disk->unpluggable = !strcmp(val, "1");
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path));

tools/libxl/libxl_utils.h +1 -1

diff -r 1967c7c290eb tools/libxl/libxl_utils.h
@@ -29,7 +29,7 @@ int libxl_get_stubdom_id(libxl_ctx *ctx,
 int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid);
 int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid);
 int libxl_create_logfile(libxl_ctx *ctx, char *name, char **full_name);
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype);
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend);
 
 int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
                              void **data_r, int *datalen_r);

tools/libxl/xl_cmdimpl.c +58 ~~-49~~

215 lines not on this page · raw message

tools/python/xen/lowlevel/xl/xl.c +11 -6

diff -r 1967c7c290eb tools/python/xen/lowlevel/xl/xl.c
@@ -779,12 +779,17 @@ PyMODINIT_FUNC initxl(void)
     _INT_CONST_LIBXL(m, CONSBACK_XENCONSOLED);
     _INT_CONST_LIBXL(m, CONSBACK_IOEMU);
 
-    _INT_CONST(m, PHYSTYPE_QCOW);
-    _INT_CONST(m, PHYSTYPE_QCOW2);
-    _INT_CONST(m, PHYSTYPE_VHD);
-    _INT_CONST(m, PHYSTYPE_AIO);
-    _INT_CONST(m, PHYSTYPE_FILE);
-    _INT_CONST(m, PHYSTYPE_PHY);
+    _INT_CONST(m, DISK_FORMAT_UNKNOWN);
+    _INT_CONST(m, DISK_FORMAT_QCOW);
+    _INT_CONST(m, DISK_FORMAT_QCOW2);
+    _INT_CONST(m, DISK_FORMAT_VHD);
+    _INT_CONST(m, DISK_FORMAT_RAW);
+    _INT_CONST(m, DISK_FORMAT_EMPTY);
+
+    _INT_CONST(m, DISK_BACKEND_UNKNOWN);
+    _INT_CONST(m, DISK_BACKEND_PHY);
+    _INT_CONST(m, DISK_BACKEND_TAP);
+    _INT_CONST(m, DISK_BACKEND_QDISK);
 
     _INT_CONST(m, NICTYPE_IOEMU);
     _INT_CONST(m, NICTYPE_VIF);

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

Kamala Narasimhan

author

2011-02-10 20:00 UTC #6 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Stefano - This includes a fix for empty cdrom case issue you found while testing.

Description:

This patch refactors xl disk specific interface to separate disk format and backend types, rename some variables, changes code that is impacted by this interface change.

Signed-off-by Kamala Narasimhan <[email protected]>

Kamala

tools/libxl/libxl.c +99 ~~-67~~

diff -r 1967c7c290eb tools/libxl/libxl.c
@@ -588,7 +588,7 @@ int libxl_wait_for_disk_ejects(libxl_ctx
     for (i = 0; i < num_disks; i++) {
         if (asprintf(&(waiter[i].path), "%s/device/vbd/%d/eject",
                      libxl__xs_get_dompath(&gc, domid),
-                     libxl__device_disk_dev_number(disks[i].virtpath)) < 0)
+                     libxl__device_disk_dev_number(disks[i].vdev)) < 0)
             goto out;
         if (asprintf(&(waiter[i].token), "%d", LIBXL_EVENT_DISK_EJECT) < 0)
             goto out;

@@ -668,10 +668,10 @@ int libxl_event_get_disk_eject_info(libx
 
     disk->backend_domid = 0;
     disk->domid = domid;
-    disk->physpath = strdup("");
-    disk->phystype = PHYSTYPE_EMPTY;
+    disk->pdev_path = strdup("");
+    disk->format = DISK_FORMAT_EMPTY;
     /* this value is returned to the user: do not free right away */
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
     disk->unpluggable = 1;
     disk->readwrite = 0;
     disk->is_cdrom = 1;

@@ -863,24 +863,25 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, libxl_disk_phystype disk_type)
+static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
+    libxl_disk_backend backend_type, libxl_disk_format format)
 {
     struct stat stat_buf;
 
-    if ( (file_name[0] == '\0') && (disk_type == PHYSTYPE_EMPTY) )
+    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
         return 0;
 
-    if ( stat(file_name, &stat_buf) != 0 ) {
+    if (stat(file_name, &stat_buf) != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;
     }
-    if ( disk_type == PHYSTYPE_PHY ) {
+    if (backend_type == DISK_BACKEND_PHY) {
         if ( !(S_ISBLK(stat_buf.st_mode)) ) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n",
                 file_name);
             return ERROR_INVAL;
         }
-    } else if ( S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0 ) {
+    } else if (S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is 0!\n", file_name);
         return ERROR_INVAL;
     }

@@ -898,7 +899,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__device device;
     int major, minor, rc;
 
-    rc = validate_virtual_disk(ctx, disk->physpath, disk->phystype);
+    rc = validate_virtual_disk(ctx, disk->pdev_path, disk->backend, 
+             disk->format);
     if (rc)
         return rc;

@@ -913,11 +915,11 @@ int libxl_device_disk_add(libxl_ctx *ctx
         goto out_free;
     }
 
-    backend_type = libxl__device_disk_backend_type_of_phystype(disk->phystype);
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    backend_type = libxl__device_disk_string_of_backend(disk->backend);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     if (devid==-1) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->virtpath);
+               " virtual disk identifier %s", disk->vdev);
         rc = ERROR_INVAL;
         goto out_free;
     }

@@ -928,37 +930,33 @@ int libxl_device_disk_add(libxl_ctx *ctx
     device.domid = disk->domid;
     device.kind = DEVICE_VBD;
 
-    switch (disk->phystype) {
-        case PHYSTYPE_PHY: {
-
-            libxl__device_physdisk_major_minor(disk->physpath, &major, &minor);
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
             flexarray_append(back, "physical-device");
             flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
 
             flexarray_append(back, "params");
-            flexarray_append(back, disk->physpath);
+            flexarray_append(back, disk->pdev_path);
 
             device.backend_kind = DEVICE_VBD;
             break;
         }
-        case PHYSTYPE_EMPTY:
-            break;
-        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:
+        case DISK_BACKEND_TAP:
+        case DISK_BACKEND_QDISK: {
+            if (disk->format == DISK_FORMAT_EMPTY)
+                break;
             if (libxl__blktap_enabled(&gc)) {
                 const char *dev = libxl__blktap_devpath(&gc,
-                                               disk->physpath, disk->phystype);
+                                               disk->pdev_path, disk->format);
                 if (!dev) {
                     rc = ERROR_FAIL;
                     goto out_free;
                 }
                 flexarray_append(back, "tapdisk-params");
-                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", 
+                    libxl__device_disk_string_of_format(disk->format), 
+                    disk->pdev_path));
                 flexarray_append(back, "params");
                 flexarray_append(back, libxl__strdup(&gc, dev));
                 backend_type = "phy";

@@ -971,16 +969,17 @@ int libxl_device_disk_add(libxl_ctx *ctx
             }
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
-                          libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
 
-            if (libxl__blktap_enabled(&gc))
+            if (libxl__blktap_enabled(&gc) && 
+                 disk->format != DISK_BACKEND_QDISK)
                 device.backend_kind = DEVICE_TAP;
             else
                 device.backend_kind = DEVICE_QDISK;
             break;
-
+        }
         default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", disk->phystype);
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
             rc = ERROR_INVAL;
             goto out_free;
     }

@@ -996,7 +995,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_append(back, "state");
     flexarray_append(back, libxl__sprintf(&gc, "%d", 1));
     flexarray_append(back, "dev");
-    flexarray_append(back, disk->virtpath);
+    flexarray_append(back, disk->vdev);
     flexarray_append(back, "type");
     flexarray_append(back, backend_type);
     flexarray_append(back, "mode");

@@ -1036,11 +1035,11 @@ int libxl_device_disk_del(libxl_ctx *ctx
     libxl__device device;
     int devid;
 
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     = 
-        (disk->phystype == PHYSTYPE_PHY) ? DEVICE_VBD : DEVICE_TAP;
+        (disk->backend == DISK_BACKEND_PHY) ? DEVICE_VBD : DEVICE_TAP;
     device.domid            = disk->domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;

@@ -1052,36 +1051,67 @@ char * libxl_device_disk_local_attach(li
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     const char *dev = NULL;
     char *ret = NULL;
-    int phystype = disk->phystype;
-    switch (phystype) {
-        case PHYSTYPE_PHY: {
-            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->physpath);
-            dev = disk->physpath;
+
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "physical block device must"
+                    " be raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching PHY disk %s to domain 0",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
         }
-        case PHYSTYPE_FILE:
-            /* let's pretend is tap:aio for the moment */
-            phystype = PHYSTYPE_AIO;
-        case PHYSTYPE_AIO:
-            if (!libxl__blktap_enabled(&gc)) {
-                dev = disk->physpath;
+        case DISK_BACKEND_TAP: {
+            if (disk->format == DISK_FORMAT_VHD || disk->format == DISK_FORMAT_RAW)
+            {
+                if (libxl__blktap_enabled(&gc))
+                    dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->format);
+                else {
+                    if (disk->format != DISK_FORMAT_RAW) {
+                        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required"
+                            " to open a vhd disk");
+                        break;
+                    } else {
+                        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching tap disk %s to domain 0",
+                            disk->pdev_path);
+                        dev = disk->pdev_path;
+                        break;
+                    }
+                }
+                break;
+            } else if (disk->format == DISK_FORMAT_QCOW ||
+                       disk->format == DISK_FORMAT_QCOW2) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image");
+                break;
+            } else {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                    "type: %d", disk->backend);
                 break;
             }
-        case PHYSTYPE_VHD:
-            if (libxl__blktap_enabled(&gc))
-                dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
-            else
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
-            break;
-        case PHYSTYPE_QCOW:
-        case PHYSTYPE_QCOW2:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
+        }
+        case DISK_BACKEND_QDISK: {
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk "
+                    "image if the format is not raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching qdisk %s to domain 0\n",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
 
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
+        }
+        case DISK_BACKEND_UNKNOWN:
+        default: {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                "type: %d", disk->backend);
             break;
+        }
     }
+
     if (dev != NULL)
         ret = strdup(dev);
     libxl__free_all(&gc);

@@ -1677,13 +1707,15 @@ static unsigned int libxl_append_disk_li
             pdisk->domid = domid;
             physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path, *dir), &len);
             if (physpath_tmp && strchr(physpath_tmp, ':')) {
-                pdisk->physpath = strdup(strchr(physpath_tmp, ':') + 1);
+                pdisk->pdev_path = strdup(strchr(physpath_tmp, ':') + 1);
                 free(physpath_tmp);
             } else {
-                pdisk->physpath = physpath_tmp;
+                pdisk->pdev_path = physpath_tmp;
             }
-            libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->phystype));
-            pdisk->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
+            libxl_string_to_backend(ctx, libxl__xs_read(&gc, XBT_NULL, 
+                libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), 
+                &(pdisk->backend));
+            pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
             pdisk->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir)));
             if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path, *dir)), "w"))
                 pdisk->readwrite = 1;

@@ -1718,7 +1750,7 @@ int libxl_device_disk_getinfo(libxl_ctx 
     char *val;
 
     dompath = libxl__xs_get_dompath(&gc, domid);
-    diskinfo->devid = libxl__device_disk_dev_number(disk->virtpath);
+    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev);
 
     /* tap devices entries in xenstore are written as vbd devices. */
     diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid);

@@ -1752,13 +1784,13 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
     libxl_device_disk *disks;
     int ret = ERROR_FAIL;
 
-    if (!disk->physpath) {
-        disk->physpath = strdup("");
-        disk->phystype = PHYSTYPE_EMPTY;
+    if (!disk->pdev_path) {
+        disk->pdev_path = strdup("");
+        disk->format = DISK_FORMAT_EMPTY;
     }
     disks = libxl_device_disk_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
-        if (disks[i].is_cdrom && !strcmp(disk->virtpath, disks[i].virtpath))
+        if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
             /* found */
             break;
     }

tools/libxl/libxl.h +14 -8

diff -r 1967c7c290eb tools/libxl/libxl.h
@@ -172,14 +172,20 @@ typedef enum {
 } libxl_console_consback;
 
 typedef enum {
-    PHYSTYPE_QCOW = 1,
-    PHYSTYPE_QCOW2,
-    PHYSTYPE_VHD,
-    PHYSTYPE_AIO,
-    PHYSTYPE_FILE,
-    PHYSTYPE_PHY,
-    PHYSTYPE_EMPTY,
-} libxl_disk_phystype;
+    DISK_FORMAT_UNKNOWN = 0,
+    DISK_FORMAT_QCOW,
+    DISK_FORMAT_QCOW2,
+    DISK_FORMAT_VHD,
+    DISK_FORMAT_RAW,
+    DISK_FORMAT_EMPTY,
+} libxl_disk_format;
+
+typedef enum {
+    DISK_BACKEND_UNKNOWN = 0,
+    DISK_BACKEND_PHY,
+    DISK_BACKEND_TAP,
+    DISK_BACKEND_QDISK,
+} libxl_disk_backend;
 
 typedef enum {
     NICTYPE_IOEMU = 1,

tools/libxl/libxl.idl +6 -4

diff -r 1967c7c290eb tools/libxl/libxl.idl
@@ -11,7 +11,8 @@ libxl_qemu_machine_type = Number("qemu_m
 libxl_qemu_machine_type = Number("qemu_machine_type", namespace="libxl_")
 libxl_console_consback = Number("console_consback", namespace="libxl_")
 libxl_console_constype = Number("console_constype", namespace="libxl_")
-libxl_disk_phystype = Number("disk_phystype", namespace="libxl_")
+libxl_disk_format = Number("disk_format", namespace="libxl_")
+libxl_disk_backend = Number("disk_backend", namespace="libxl_")
 libxl_nic_type = Number("nic_type", namespace="libxl_")
 libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE)

@@ -203,9 +204,10 @@ libxl_device_disk = Struct("device_disk"
 libxl_device_disk = Struct("device_disk", [
     ("backend_domid", uint32),
     ("domid", domid),
-    ("physpath", string),
-    ("phystype", libxl_disk_phystype),
-    ("virtpath", string),
+    ("pdev_path", string),
+    ("vdev", string),
+    ("backend", libxl_disk_backend),
+    ("format", libxl_disk_format),
     ("unpluggable", integer),
     ("readwrite", integer),
     ("is_cdrom", integer),

tools/libxl/libxl_blktap2.c +2 -2

diff -r 1967c7c290eb tools/libxl/libxl_blktap2.c
@@ -26,13 +26,13 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     const char *type;
     char *params, *devname = NULL;
     int minor, err;
 
-    type = libxl__device_disk_string_of_phystype(phystype);
+    type = libxl__device_disk_string_of_format(format);
     minor = tap_ctl_find_minor(type, disk);
     if (minor >= 0) {
         devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", minor);

tools/libxl/libxl_device.c +13 ~~-20~~

diff -r 1967c7c290eb tools/libxl/libxl_device.c
@@ -121,31 +121,24 @@ out:
     return rc;
 }
 
-char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_format(libxl_disk_format format)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "qcow";
-        case PHYSTYPE_QCOW2: return "qcow2";
-        case PHYSTYPE_VHD: return "vhd";
-        case PHYSTYPE_AIO: return "aio";
-        case PHYSTYPE_FILE: return "file";
-        case PHYSTYPE_PHY: return "phy";
-        case PHYSTYPE_EMPTY: return "file";
-        default: return NULL;
+    switch (format) {
+        case DISK_FORMAT_QCOW: return "qcow";
+        case DISK_FORMAT_QCOW2: return "qcow2"; 
+        case DISK_FORMAT_VHD: return "vhd"; 
+        case DISK_FORMAT_RAW:
+        case DISK_FORMAT_EMPTY: return "aio"; 
+        default: return NULL; 
     }
 }
 
-char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "tap";
-        case PHYSTYPE_QCOW2: return "tap";
-        case PHYSTYPE_VHD: return "tap";
-        case PHYSTYPE_AIO: return "tap";
-        /* let's pretend file is tap:aio */
-        case PHYSTYPE_FILE: return "tap";
-        case PHYSTYPE_EMPTY: return "tap";
-        case PHYSTYPE_PHY: return "phy";
+    switch (backend) {
+        case DISK_BACKEND_QDISK: return "qdisk";
+        case DISK_BACKEND_TAP: return "tap";
+        case DISK_BACKEND_PHY: return "phy";
         default: return NULL;
     }
 }

tools/libxl/libxl_dm.c +3 -3

diff -r 1967c7c290eb tools/libxl/libxl_dm.c
@@ -316,10 +316,10 @@ static char ** libxl_build_device_model_
         for (i; i < nb; i++) {
             if (disks[i].is_cdrom) {
                 flexarray_append(dm_args, "-cdrom");
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].virtpath));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             }
             libxl_device_disk_destroy(&disks[i]);
         }

tools/libxl/libxl_internal.h +3 -3

diff -r 1967c7c290eb tools/libxl/libxl_internal.h
@@ -178,8 +178,8 @@ _hidden void libxl__userdata_destroyall(
 _hidden void libxl__userdata_destroyall(libxl_ctx *ctx, uint32_t domid);
 
 /* from xl_device */
-_hidden char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
-_hidden char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype);
+_hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
+_hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
 _hidden int libxl__device_disk_dev_number(char *virtpath);

@@ -306,7 +306,7 @@ _hidden int libxl__blktap_enabled(libxl_
  */
 _hidden const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype);
+                                 libxl_disk_format format);
 
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);

tools/libxl/libxl_noblktap2.c +1 -1

diff -r 1967c7c290eb tools/libxl/libxl_noblktap2.c
@@ -23,7 +23,7 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     return NULL;
 }

tools/libxl/libxl_utils.c +10 ~~-12~~

diff -r 1967c7c290eb tools/libxl/libxl_utils.c
@@ -275,15 +275,15 @@ out:
     return rc;
 }
 
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype)
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend)
 {
     char *p;
     int rc = 0;
 
     if (!strcmp(s, "phy")) {
-        *phystype = PHYSTYPE_PHY;
+        *backend = DISK_BACKEND_PHY;
     } else if (!strcmp(s, "file")) {
-        *phystype = PHYSTYPE_FILE;
+        *backend = DISK_BACKEND_TAP;
     } else if (!strcmp(s, "tap")) {
         p = strchr(s, ':');
         if (!p) {

@@ -291,14 +291,12 @@ int libxl_string_to_phystype(libxl_ctx *
             goto out;
         }
         p++;
-        if (!strcmp(p, "aio")) {
-            *phystype = PHYSTYPE_AIO;
-        } else if (!strcmp(p, "vhd")) {
-            *phystype = PHYSTYPE_VHD;
+        if (!strcmp(p, "vhd")) {
+            *backend = DISK_BACKEND_TAP;
         } else if (!strcmp(p, "qcow")) {
-            *phystype = PHYSTYPE_QCOW;
+            *backend = DISK_BACKEND_QDISK;
         } else if (!strcmp(p, "qcow2")) {
-            *phystype = PHYSTYPE_QCOW2;
+            *backend = DISK_BACKEND_QDISK;
         }
     }
 out:

@@ -553,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx
     disk->backend_domid = strtoul(val, NULL, 10);
     disk->domid = domid;
     be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath));
-    disk->physpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
+    disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
-    libxl_string_to_phystype(ctx, val, &(disk->phystype));
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
+    libxl_string_to_backend(ctx, val, &(disk->backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path));
     disk->unpluggable = !strcmp(val, "1");
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path));

tools/libxl/libxl_utils.h +1 -1

diff -r 1967c7c290eb tools/libxl/libxl_utils.h
@@ -29,7 +29,7 @@ int libxl_get_stubdom_id(libxl_ctx *ctx,
 int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid);
 int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid);
 int libxl_create_logfile(libxl_ctx *ctx, char *name, char **full_name);
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype);
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend);
 
 int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
                              void **data_r, int *datalen_r);

tools/libxl/xl_cmdimpl.c +59 ~~-49~~

216 lines not on this page · raw message

tools/python/xen/lowlevel/xl/xl.c +11 -6

diff -r 1967c7c290eb tools/python/xen/lowlevel/xl/xl.c
@@ -779,12 +779,17 @@ PyMODINIT_FUNC initxl(void)
     _INT_CONST_LIBXL(m, CONSBACK_XENCONSOLED);
     _INT_CONST_LIBXL(m, CONSBACK_IOEMU);
 
-    _INT_CONST(m, PHYSTYPE_QCOW);
-    _INT_CONST(m, PHYSTYPE_QCOW2);
-    _INT_CONST(m, PHYSTYPE_VHD);
-    _INT_CONST(m, PHYSTYPE_AIO);
-    _INT_CONST(m, PHYSTYPE_FILE);
-    _INT_CONST(m, PHYSTYPE_PHY);
+    _INT_CONST(m, DISK_FORMAT_UNKNOWN);
+    _INT_CONST(m, DISK_FORMAT_QCOW);
+    _INT_CONST(m, DISK_FORMAT_QCOW2);
+    _INT_CONST(m, DISK_FORMAT_VHD);
+    _INT_CONST(m, DISK_FORMAT_RAW);
+    _INT_CONST(m, DISK_FORMAT_EMPTY);
+
+    _INT_CONST(m, DISK_BACKEND_UNKNOWN);
+    _INT_CONST(m, DISK_BACKEND_PHY);
+    _INT_CONST(m, DISK_BACKEND_TAP);
+    _INT_CONST(m, DISK_BACKEND_QDISK);
 
     _INT_CONST(m, NICTYPE_IOEMU);
     _INT_CONST(m, NICTYPE_VIF);

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

Stefano Stabellini

2011-02-11 13:47 UTC #7 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Quoted 6 lines · Kamala Narasimhan

Stefano - This includes a fix for empty cdrom case issue you found while testing.
Description:
This patch refactors xl disk specific interface to separate disk format and backend types, rename some variables, changes code that is impacted by this interface change.

you need to update the ocaml bindings, otherwise the patch will cause build problems:

make[7]: Entering directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl'
CC xl_stubs.o
xl_stubs.c: In function 'device_disk_val':
xl_stubs.c:195: error: 'libxl_device_disk' has no member named 'physpath' xl_stubs.c:196: error: 'libxl_device_disk' has no member named 'phystype' xl_stubs.c:196: error: 'PHYSTYPE_QCOW' undeclared (first use in this function) xl_stubs.c:196: error: (Each undeclared identifier is reported only once xl_stubs.c:196: error: for each function it appears in.)
xl_stubs.c:197: error: 'libxl_device_disk' has no member named 'virtpath' make[7]: *** [xl_stubs.o] Error 1
make[7]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl' make[6]: *** [subdir-install-xl] Error 2
make[6]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[5]: *** [subdirs-install] Error 2
make[5]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[4]: *** [subdir-install-libs] Error 2
make[4]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[3]: *** [subdirs-install] Error 2
make[3]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[2]: *** [subdir-install-ocaml] Error 2
make[2]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools' make[1]: *** [subdirs-install] Error 2
make[1]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools'
make: *** [install-tools] Error 2

apart from this it looks OK to me.

Kamala Narasimhan

author

2011-02-11 14:45 UTC #8 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Quoted 37 lines · Stefano Stabellini

Quoted 6 lines · Kamala Narasimhan
Stefano - This includes a fix for empty cdrom case issue you found while testing.
Description:
This patch refactors xl disk specific interface to separate disk format and backend types, rename some variables, changes code that is impacted by this interface change.
you need to update the ocaml bindings, otherwise the patch will cause build problems:
make[7]: Entering directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl'
CC xl_stubs.o
xl_stubs.c: In function 'device_disk_val':
xl_stubs.c:195: error: 'libxl_device_disk' has no member named 'physpath' xl_stubs.c:196: error: 'libxl_device_disk' has no member named 'phystype' xl_stubs.c:196: error: 'PHYSTYPE_QCOW' undeclared (first use in this function) xl_stubs.c:196: error: (Each undeclared identifier is reported only once xl_stubs.c:196: error: for each function it appears in.)
xl_stubs.c:197: error: 'libxl_device_disk' has no member named 'virtpath' make[7]: *** [xl_stubs.o] Error 1
make[7]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl' make[6]: *** [subdir-install-xl] Error 2
make[6]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[5]: *** [subdirs-install] Error 2
make[5]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[4]: *** [subdir-install-libs] Error 2
make[4]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[3]: *** [subdirs-install] Error 2
make[3]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[2]: *** [subdir-install-ocaml] Error 2
make[2]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools' make[1]: *** [subdirs-install] Error 2
make[1]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools'
make: *** [install-tools] Error 2

Quoted 1 line

apart from this it looks OK to me.

Turns out I didn't have ocaml installed on my build system and skipped past ocaml builds and that is why I didn't even see this issue. I will fix it.

Kamala

Kamala Narasimhan

author

2011-02-11 15:19 UTC #9 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

In reply to Stefano Stabellini

Quoted 37 lines · Stefano Stabellini

Quoted 6 lines · Kamala Narasimhan
Stefano - This includes a fix for empty cdrom case issue you found while testing.
Description:
This patch refactors xl disk specific interface to separate disk format and backend types, rename some variables, changes code that is impacted by this interface change.
you need to update the ocaml bindings, otherwise the patch will cause build problems:
make[7]: Entering directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl'
CC xl_stubs.o
xl_stubs.c: In function 'device_disk_val':
xl_stubs.c:195: error: 'libxl_device_disk' has no member named 'physpath' xl_stubs.c:196: error: 'libxl_device_disk' has no member named 'phystype' xl_stubs.c:196: error: 'PHYSTYPE_QCOW' undeclared (first use in this function) xl_stubs.c:196: error: (Each undeclared identifier is reported only once xl_stubs.c:196: error: for each function it appears in.)
xl_stubs.c:197: error: 'libxl_device_disk' has no member named 'virtpath' make[7]: *** [xl_stubs.o] Error 1
make[7]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl' make[6]: *** [subdir-install-xl] Error 2
make[6]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[5]: *** [subdirs-install] Error 2
make[5]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[4]: *** [subdir-install-libs] Error 2
make[4]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[3]: *** [subdirs-install] Error 2
make[3]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[2]: *** [subdir-install-ocaml] Error 2
make[2]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools' make[1]: *** [subdirs-install] Error 2
make[1]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools'
make: *** [install-tools] Error 2

Quoted 1 line

apart from this it looks OK to me.

Stefano - Here is a revised patch that fixes the ocaml bindings build issues. While I confirmed that the build now works, I wouldn't even know where to start when it comes to testing it!

Signed-off-by Kamala Narasimhan <[email protected]>

Kamala

tools/libxl/libxl.c +99 ~~-67~~

diff -r 1967c7c290eb tools/libxl/libxl.c
@@ -588,7 +588,7 @@ int libxl_wait_for_disk_ejects(libxl_ctx
     for (i = 0; i < num_disks; i++) {
         if (asprintf(&(waiter[i].path), "%s/device/vbd/%d/eject",
                      libxl__xs_get_dompath(&gc, domid),
-                     libxl__device_disk_dev_number(disks[i].virtpath)) < 0)
+                     libxl__device_disk_dev_number(disks[i].vdev)) < 0)
             goto out;
         if (asprintf(&(waiter[i].token), "%d", LIBXL_EVENT_DISK_EJECT) < 0)
             goto out;

@@ -668,10 +668,10 @@ int libxl_event_get_disk_eject_info(libx
 
     disk->backend_domid = 0;
     disk->domid = domid;
-    disk->physpath = strdup("");
-    disk->phystype = PHYSTYPE_EMPTY;
+    disk->pdev_path = strdup("");
+    disk->format = DISK_FORMAT_EMPTY;
     /* this value is returned to the user: do not free right away */
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
     disk->unpluggable = 1;
     disk->readwrite = 0;
     disk->is_cdrom = 1;

@@ -863,24 +863,25 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, libxl_disk_phystype disk_type)
+static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
+    libxl_disk_backend backend_type, libxl_disk_format format)
 {
     struct stat stat_buf;
 
-    if ( (file_name[0] == '\0') && (disk_type == PHYSTYPE_EMPTY) )
+    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
         return 0;
 
-    if ( stat(file_name, &stat_buf) != 0 ) {
+    if (stat(file_name, &stat_buf) != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;
     }
-    if ( disk_type == PHYSTYPE_PHY ) {
+    if (backend_type == DISK_BACKEND_PHY) {
         if ( !(S_ISBLK(stat_buf.st_mode)) ) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n",
                 file_name);
             return ERROR_INVAL;
         }
-    } else if ( S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0 ) {
+    } else if (S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is 0!\n", file_name);
         return ERROR_INVAL;
     }

@@ -898,7 +899,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__device device;
     int major, minor, rc;
 
-    rc = validate_virtual_disk(ctx, disk->physpath, disk->phystype);
+    rc = validate_virtual_disk(ctx, disk->pdev_path, disk->backend, 
+             disk->format);
     if (rc)
         return rc;

@@ -913,11 +915,11 @@ int libxl_device_disk_add(libxl_ctx *ctx
         goto out_free;
     }
 
-    backend_type = libxl__device_disk_backend_type_of_phystype(disk->phystype);
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    backend_type = libxl__device_disk_string_of_backend(disk->backend);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     if (devid==-1) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->virtpath);
+               " virtual disk identifier %s", disk->vdev);
         rc = ERROR_INVAL;
         goto out_free;
     }

@@ -928,37 +930,33 @@ int libxl_device_disk_add(libxl_ctx *ctx
     device.domid = disk->domid;
     device.kind = DEVICE_VBD;
 
-    switch (disk->phystype) {
-        case PHYSTYPE_PHY: {
-
-            libxl__device_physdisk_major_minor(disk->physpath, &major, &minor);
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
             flexarray_append(back, "physical-device");
             flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
 
             flexarray_append(back, "params");
-            flexarray_append(back, disk->physpath);
+            flexarray_append(back, disk->pdev_path);
 
             device.backend_kind = DEVICE_VBD;
             break;
         }
-        case PHYSTYPE_EMPTY:
-            break;
-        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:
+        case DISK_BACKEND_TAP:
+        case DISK_BACKEND_QDISK: {
+            if (disk->format == DISK_FORMAT_EMPTY)
+                break;
             if (libxl__blktap_enabled(&gc)) {
                 const char *dev = libxl__blktap_devpath(&gc,
-                                               disk->physpath, disk->phystype);
+                                               disk->pdev_path, disk->format);
                 if (!dev) {
                     rc = ERROR_FAIL;
                     goto out_free;
                 }
                 flexarray_append(back, "tapdisk-params");
-                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", 
+                    libxl__device_disk_string_of_format(disk->format), 
+                    disk->pdev_path));
                 flexarray_append(back, "params");
                 flexarray_append(back, libxl__strdup(&gc, dev));
                 backend_type = "phy";

@@ -971,16 +969,17 @@ int libxl_device_disk_add(libxl_ctx *ctx
             }
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
-                          libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
 
-            if (libxl__blktap_enabled(&gc))
+            if (libxl__blktap_enabled(&gc) && 
+                 disk->format != DISK_BACKEND_QDISK)
                 device.backend_kind = DEVICE_TAP;
             else
                 device.backend_kind = DEVICE_QDISK;
             break;
-
+        }
         default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", disk->phystype);
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
             rc = ERROR_INVAL;
             goto out_free;
     }

@@ -996,7 +995,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_append(back, "state");
     flexarray_append(back, libxl__sprintf(&gc, "%d", 1));
     flexarray_append(back, "dev");
-    flexarray_append(back, disk->virtpath);
+    flexarray_append(back, disk->vdev);
     flexarray_append(back, "type");
     flexarray_append(back, backend_type);
     flexarray_append(back, "mode");

@@ -1036,11 +1035,11 @@ int libxl_device_disk_del(libxl_ctx *ctx
     libxl__device device;
     int devid;
 
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     = 
-        (disk->phystype == PHYSTYPE_PHY) ? DEVICE_VBD : DEVICE_TAP;
+        (disk->backend == DISK_BACKEND_PHY) ? DEVICE_VBD : DEVICE_TAP;
     device.domid            = disk->domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;

@@ -1052,36 +1051,67 @@ char * libxl_device_disk_local_attach(li
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     const char *dev = NULL;
     char *ret = NULL;
-    int phystype = disk->phystype;
-    switch (phystype) {
-        case PHYSTYPE_PHY: {
-            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->physpath);
-            dev = disk->physpath;
+
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "physical block device must"
+                    " be raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching PHY disk %s to domain 0",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
         }
-        case PHYSTYPE_FILE:
-            /* let's pretend is tap:aio for the moment */
-            phystype = PHYSTYPE_AIO;
-        case PHYSTYPE_AIO:
-            if (!libxl__blktap_enabled(&gc)) {
-                dev = disk->physpath;
+        case DISK_BACKEND_TAP: {
+            if (disk->format == DISK_FORMAT_VHD || disk->format == DISK_FORMAT_RAW)
+            {
+                if (libxl__blktap_enabled(&gc))
+                    dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->format);
+                else {
+                    if (disk->format != DISK_FORMAT_RAW) {
+                        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required"
+                            " to open a vhd disk");
+                        break;
+                    } else {
+                        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching tap disk %s to domain 0",
+                            disk->pdev_path);
+                        dev = disk->pdev_path;
+                        break;
+                    }
+                }
+                break;
+            } else if (disk->format == DISK_FORMAT_QCOW ||
+                       disk->format == DISK_FORMAT_QCOW2) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image");
+                break;
+            } else {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                    "type: %d", disk->backend);
                 break;
             }
-        case PHYSTYPE_VHD:
-            if (libxl__blktap_enabled(&gc))
-                dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
-            else
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
-            break;
-        case PHYSTYPE_QCOW:
-        case PHYSTYPE_QCOW2:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
+        }
+        case DISK_BACKEND_QDISK: {
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk "
+                    "image if the format is not raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching qdisk %s to domain 0\n",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
 
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
+        }
+        case DISK_BACKEND_UNKNOWN:
+        default: {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                "type: %d", disk->backend);
             break;
+        }
     }
+
     if (dev != NULL)
         ret = strdup(dev);
     libxl__free_all(&gc);

@@ -1677,13 +1707,15 @@ static unsigned int libxl_append_disk_li
             pdisk->domid = domid;
             physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path, *dir), &len);
             if (physpath_tmp && strchr(physpath_tmp, ':')) {
-                pdisk->physpath = strdup(strchr(physpath_tmp, ':') + 1);
+                pdisk->pdev_path = strdup(strchr(physpath_tmp, ':') + 1);
                 free(physpath_tmp);
             } else {
-                pdisk->physpath = physpath_tmp;
+                pdisk->pdev_path = physpath_tmp;
             }
-            libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->phystype));
-            pdisk->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
+            libxl_string_to_backend(ctx, libxl__xs_read(&gc, XBT_NULL, 
+                libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), 
+                &(pdisk->backend));
+            pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
             pdisk->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir)));
             if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path, *dir)), "w"))
                 pdisk->readwrite = 1;

@@ -1718,7 +1750,7 @@ int libxl_device_disk_getinfo(libxl_ctx 
     char *val;
 
     dompath = libxl__xs_get_dompath(&gc, domid);
-    diskinfo->devid = libxl__device_disk_dev_number(disk->virtpath);
+    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev);
 
     /* tap devices entries in xenstore are written as vbd devices. */
     diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid);

@@ -1752,13 +1784,13 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
     libxl_device_disk *disks;
     int ret = ERROR_FAIL;
 
-    if (!disk->physpath) {
-        disk->physpath = strdup("");
-        disk->phystype = PHYSTYPE_EMPTY;
+    if (!disk->pdev_path) {
+        disk->pdev_path = strdup("");
+        disk->format = DISK_FORMAT_EMPTY;
     }
     disks = libxl_device_disk_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
-        if (disks[i].is_cdrom && !strcmp(disk->virtpath, disks[i].virtpath))
+        if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
             /* found */
             break;
     }

tools/libxl/libxl.h +14 -8

diff -r 1967c7c290eb tools/libxl/libxl.h
@@ -172,14 +172,20 @@ typedef enum {
 } libxl_console_consback;
 
 typedef enum {
-    PHYSTYPE_QCOW = 1,
-    PHYSTYPE_QCOW2,
-    PHYSTYPE_VHD,
-    PHYSTYPE_AIO,
-    PHYSTYPE_FILE,
-    PHYSTYPE_PHY,
-    PHYSTYPE_EMPTY,
-} libxl_disk_phystype;
+    DISK_FORMAT_UNKNOWN = 0,
+    DISK_FORMAT_QCOW,
+    DISK_FORMAT_QCOW2,
+    DISK_FORMAT_VHD,
+    DISK_FORMAT_RAW,
+    DISK_FORMAT_EMPTY,
+} libxl_disk_format;
+
+typedef enum {
+    DISK_BACKEND_UNKNOWN = 0,
+    DISK_BACKEND_PHY,
+    DISK_BACKEND_TAP,
+    DISK_BACKEND_QDISK,
+} libxl_disk_backend;
 
 typedef enum {
     NICTYPE_IOEMU = 1,

tools/libxl/libxl.idl +6 -4

diff -r 1967c7c290eb tools/libxl/libxl.idl
@@ -11,7 +11,8 @@ libxl_qemu_machine_type = Number("qemu_m
 libxl_qemu_machine_type = Number("qemu_machine_type", namespace="libxl_")
 libxl_console_consback = Number("console_consback", namespace="libxl_")
 libxl_console_constype = Number("console_constype", namespace="libxl_")
-libxl_disk_phystype = Number("disk_phystype", namespace="libxl_")
+libxl_disk_format = Number("disk_format", namespace="libxl_")
+libxl_disk_backend = Number("disk_backend", namespace="libxl_")
 libxl_nic_type = Number("nic_type", namespace="libxl_")
 libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE)

@@ -203,9 +204,10 @@ libxl_device_disk = Struct("device_disk"
 libxl_device_disk = Struct("device_disk", [
     ("backend_domid", uint32),
     ("domid", domid),
-    ("physpath", string),
-    ("phystype", libxl_disk_phystype),
-    ("virtpath", string),
+    ("pdev_path", string),
+    ("vdev", string),
+    ("backend", libxl_disk_backend),
+    ("format", libxl_disk_format),
     ("unpluggable", integer),
     ("readwrite", integer),
     ("is_cdrom", integer),

tools/libxl/libxl_blktap2.c +2 -2

diff -r 1967c7c290eb tools/libxl/libxl_blktap2.c
@@ -26,13 +26,13 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     const char *type;
     char *params, *devname = NULL;
     int minor, err;
 
-    type = libxl__device_disk_string_of_phystype(phystype);
+    type = libxl__device_disk_string_of_format(format);
     minor = tap_ctl_find_minor(type, disk);
     if (minor >= 0) {
         devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", minor);

tools/libxl/libxl_device.c +13 ~~-20~~

diff -r 1967c7c290eb tools/libxl/libxl_device.c
@@ -121,31 +121,24 @@ out:
     return rc;
 }
 
-char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_format(libxl_disk_format format)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "qcow";
-        case PHYSTYPE_QCOW2: return "qcow2";
-        case PHYSTYPE_VHD: return "vhd";
-        case PHYSTYPE_AIO: return "aio";
-        case PHYSTYPE_FILE: return "file";
-        case PHYSTYPE_PHY: return "phy";
-        case PHYSTYPE_EMPTY: return "file";
-        default: return NULL;
+    switch (format) {
+        case DISK_FORMAT_QCOW: return "qcow";
+        case DISK_FORMAT_QCOW2: return "qcow2"; 
+        case DISK_FORMAT_VHD: return "vhd"; 
+        case DISK_FORMAT_RAW:
+        case DISK_FORMAT_EMPTY: return "aio"; 
+        default: return NULL; 
     }
 }
 
-char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "tap";
-        case PHYSTYPE_QCOW2: return "tap";
-        case PHYSTYPE_VHD: return "tap";
-        case PHYSTYPE_AIO: return "tap";
-        /* let's pretend file is tap:aio */
-        case PHYSTYPE_FILE: return "tap";
-        case PHYSTYPE_EMPTY: return "tap";
-        case PHYSTYPE_PHY: return "phy";
+    switch (backend) {
+        case DISK_BACKEND_QDISK: return "qdisk";
+        case DISK_BACKEND_TAP: return "tap";
+        case DISK_BACKEND_PHY: return "phy";
         default: return NULL;
     }
 }

tools/libxl/libxl_dm.c +3 -3

diff -r 1967c7c290eb tools/libxl/libxl_dm.c
@@ -316,10 +316,10 @@ static char ** libxl_build_device_model_
         for (i; i < nb; i++) {
             if (disks[i].is_cdrom) {
                 flexarray_append(dm_args, "-cdrom");
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].virtpath));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             }
             libxl_device_disk_destroy(&disks[i]);
         }

tools/libxl/libxl_internal.h +3 -3

diff -r 1967c7c290eb tools/libxl/libxl_internal.h
@@ -178,8 +178,8 @@ _hidden void libxl__userdata_destroyall(
 _hidden void libxl__userdata_destroyall(libxl_ctx *ctx, uint32_t domid);
 
 /* from xl_device */
-_hidden char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
-_hidden char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype);
+_hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
+_hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
 _hidden int libxl__device_disk_dev_number(char *virtpath);

@@ -306,7 +306,7 @@ _hidden int libxl__blktap_enabled(libxl_
  */
 _hidden const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype);
+                                 libxl_disk_format format);
 
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);

tools/libxl/libxl_noblktap2.c +1 -1

diff -r 1967c7c290eb tools/libxl/libxl_noblktap2.c
@@ -23,7 +23,7 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     return NULL;
 }

tools/libxl/libxl_utils.c +10 ~~-12~~

diff -r 1967c7c290eb tools/libxl/libxl_utils.c
@@ -275,15 +275,15 @@ out:
     return rc;
 }
 
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype)
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend)
 {
     char *p;
     int rc = 0;
 
     if (!strcmp(s, "phy")) {
-        *phystype = PHYSTYPE_PHY;
+        *backend = DISK_BACKEND_PHY;
     } else if (!strcmp(s, "file")) {
-        *phystype = PHYSTYPE_FILE;
+        *backend = DISK_BACKEND_TAP;
     } else if (!strcmp(s, "tap")) {
         p = strchr(s, ':');
         if (!p) {

@@ -291,14 +291,12 @@ int libxl_string_to_phystype(libxl_ctx *
             goto out;
         }
         p++;
-        if (!strcmp(p, "aio")) {
-            *phystype = PHYSTYPE_AIO;
-        } else if (!strcmp(p, "vhd")) {
-            *phystype = PHYSTYPE_VHD;
+        if (!strcmp(p, "vhd")) {
+            *backend = DISK_BACKEND_TAP;
         } else if (!strcmp(p, "qcow")) {
-            *phystype = PHYSTYPE_QCOW;
+            *backend = DISK_BACKEND_QDISK;
         } else if (!strcmp(p, "qcow2")) {
-            *phystype = PHYSTYPE_QCOW2;
+            *backend = DISK_BACKEND_QDISK;
         }
     }
 out:

@@ -553,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx
     disk->backend_domid = strtoul(val, NULL, 10);
     disk->domid = domid;
     be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath));
-    disk->physpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
+    disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
-    libxl_string_to_phystype(ctx, val, &(disk->phystype));
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
+    libxl_string_to_backend(ctx, val, &(disk->backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path));
     disk->unpluggable = !strcmp(val, "1");
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path));

tools/libxl/libxl_utils.h +1 -1

diff -r 1967c7c290eb tools/libxl/libxl_utils.h
@@ -29,7 +29,7 @@ int libxl_get_stubdom_id(libxl_ctx *ctx,
 int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid);
 int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid);
 int libxl_create_logfile(libxl_ctx *ctx, char *name, char **full_name);
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype);
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend);
 
 int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
                              void **data_r, int *datalen_r);

tools/libxl/xl_cmdimpl.c +59 ~~-49~~

216 lines not on this page · raw message

tools/ocaml/libs/xl/xl_stubs.c +7 -6

diff -r 1967c7c290eb tools/ocaml/libs/xl/xl_stubs.c
@@ -192,12 +192,13 @@ static int device_disk_val(caml_gc *gc, 
 	CAMLparam1(v);
 
 	c_val->backend_domid = Int_val(Field(v, 0));
-	c_val->physpath = dup_String_val(gc, Field(v, 1));
-	c_val->phystype = (Int_val(Field(v, 2))) + PHYSTYPE_QCOW;
-	c_val->virtpath = dup_String_val(gc, Field(v, 3));
-	c_val->unpluggable = Bool_val(Field(v, 4));
-	c_val->readwrite = Bool_val(Field(v, 5));
-	c_val->is_cdrom = Bool_val(Field(v, 6));
+	c_val->pdev_path = dup_String_val(gc, Field(v, 1));
+	c_val->vdev = dup_String_val(gc, Field(v, 2));
+        c_val->backend = (Int_val(Field(v, 3)));
+        c_val->format = (Int_val(Field(v, 4)));
+	c_val->unpluggable = Bool_val(Field(v, 5));
+	c_val->readwrite = Bool_val(Field(v, 6));
+	c_val->is_cdrom = Bool_val(Field(v, 7));
 
 	CAMLreturn(0);
 }

tools/python/xen/lowlevel/xl/xl.c +11 -6

diff -r 1967c7c290eb tools/python/xen/lowlevel/xl/xl.c
@@ -779,12 +779,17 @@ PyMODINIT_FUNC initxl(void)
     _INT_CONST_LIBXL(m, CONSBACK_XENCONSOLED);
     _INT_CONST_LIBXL(m, CONSBACK_IOEMU);
 
-    _INT_CONST(m, PHYSTYPE_QCOW);
-    _INT_CONST(m, PHYSTYPE_QCOW2);
-    _INT_CONST(m, PHYSTYPE_VHD);
-    _INT_CONST(m, PHYSTYPE_AIO);
-    _INT_CONST(m, PHYSTYPE_FILE);
-    _INT_CONST(m, PHYSTYPE_PHY);
+    _INT_CONST(m, DISK_FORMAT_UNKNOWN);
+    _INT_CONST(m, DISK_FORMAT_QCOW);
+    _INT_CONST(m, DISK_FORMAT_QCOW2);
+    _INT_CONST(m, DISK_FORMAT_VHD);
+    _INT_CONST(m, DISK_FORMAT_RAW);
+    _INT_CONST(m, DISK_FORMAT_EMPTY);
+
+    _INT_CONST(m, DISK_BACKEND_UNKNOWN);
+    _INT_CONST(m, DISK_BACKEND_PHY);
+    _INT_CONST(m, DISK_BACKEND_TAP);
+    _INT_CONST(m, DISK_BACKEND_QDISK);
 
     _INT_CONST(m, NICTYPE_IOEMU);
     _INT_CONST(m, NICTYPE_VIF);

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

Stefano Stabellini

2011-02-11 15:26 UTC #10 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Quoted 38 lines · Kamala Narasimhan

Quoted 37 lines · Stefano Stabellini
Quoted 6 lines · Kamala Narasimhan
Stefano - This includes a fix for empty cdrom case issue you found while testing.
Description:
This patch refactors xl disk specific interface to separate disk format and backend types, rename some variables, changes code that is impacted by this interface change.
you need to update the ocaml bindings, otherwise the patch will cause build problems:
make[7]: Entering directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl'
CC xl_stubs.o
xl_stubs.c: In function 'device_disk_val':
xl_stubs.c:195: error: 'libxl_device_disk' has no member named 'physpath' xl_stubs.c:196: error: 'libxl_device_disk' has no member named 'phystype' xl_stubs.c:196: error: 'PHYSTYPE_QCOW' undeclared (first use in this function) xl_stubs.c:196: error: (Each undeclared identifier is reported only once xl_stubs.c:196: error: for each function it appears in.)
xl_stubs.c:197: error: 'libxl_device_disk' has no member named 'virtpath' make[7]: *** [xl_stubs.o] Error 1
make[7]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl' make[6]: *** [subdir-install-xl] Error 2
make[6]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[5]: *** [subdirs-install] Error 2
make[5]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[4]: *** [subdir-install-libs] Error 2
make[4]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[3]: *** [subdirs-install] Error 2
make[3]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[2]: *** [subdir-install-ocaml] Error 2
make[2]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools' make[1]: *** [subdirs-install] Error 2
make[1]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools'
make: *** [install-tools] Error 2

Quoted 5 lines

Quoted 1 line
apart from this it looks OK to me.
Stefano - Here is a revised patch that fixes the ocaml bindings build issues. While I confirmed that the build now works, I wouldn't even know where to start when it comes to testing it!
Signed-off-by Kamala Narasimhan <[email protected]>

The patch is OK for me.

Kamala Narasimhan

author

2011-02-11 19:12 UTC #11 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

In reply to Kamala Narasimhan

Quoted 37 lines · Kamala Narasimhan

Quoted 36 lines · Stefano Stabellini
Quoted 6 lines · Kamala Narasimhan
Stefano - This includes a fix for empty cdrom case issue you found while testing.
Description:
This patch refactors xl disk specific interface to separate disk format and backend types, rename some variables, changes code that is impacted by this interface change.
you need to update the ocaml bindings, otherwise the patch will cause build problems:
make[7]: Entering directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl'
CC xl_stubs.o
xl_stubs.c: In function 'device_disk_val':
xl_stubs.c:195: error: 'libxl_device_disk' has no member named 'physpath' xl_stubs.c:196: error: 'libxl_device_disk' has no member named 'phystype' xl_stubs.c:196: error: 'PHYSTYPE_QCOW' undeclared (first use in this function) xl_stubs.c:196: error: (Each undeclared identifier is reported only once xl_stubs.c:196: error: for each function it appears in.)
xl_stubs.c:197: error: 'libxl_device_disk' has no member named 'virtpath' make[7]: *** [xl_stubs.o] Error 1
make[7]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs/xl' make[6]: *** [subdir-install-xl] Error 2
make[6]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[5]: *** [subdirs-install] Error 2
make[5]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml/libs' make[4]: *** [subdir-install-libs] Error 2
make[4]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[3]: *** [subdirs-install] Error 2
make[3]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools/ocaml' make[2]: *** [subdir-install-ocaml] Error 2
make[2]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools' make[1]: *** [subdirs-install] Error 2
make[1]: Leaving directory `/local/scratch/sstabellini/xen-unstable.hg/tools'
make: *** [install-tools] Error 2

Quoted 6 lines

Quoted 1 line
apart from this it looks OK to me.
Stefano - Here is a revised patch that fixes the ocaml bindings build issues. While I confirmed that the build now works, I wouldn't even know where to start when it comes to testing it!
Signed-off-by Kamala Narasimhan <[email protected]>

Per suggestion, along with the latest patch is a more detailed description to add to the check-in message -

Currently we pile all the backend and format information pertaining to disk option in a single enum. This check-in separates the two and uses two enums, one for disk format and another for disk backend. This helps clearly differentiate between disk format and backend within the implementation and also helps cleanup the code in this area in preparation for the impending parser revamping to be done post 4.1. Along with separating format and backend, this check-in also removes unwanted types and renames variables in the disk interface and fixes the code affected by the interface changes.

In specific, here are the disk interface changes made - In libxl_device_disk structure physpath was renamed to pdev_path, virtpath was renamed to vdev, phystype was removed and replaced with backend and format enums. Also previously a single enum libxl_disk_phystype held the values for qcow, qcow2, vhd, aio, file, phy, empty and that got refactored into two enums, libxl_disk_format to hold unknown, qcow, qcow2, vhd, raw, empty and libxl_disk_backend to hold unknown, phy, tap and qdisk.

Signed-off-by Kamala Narasimhan <[email protected]>

Kamala

tools/libxl/libxl.c +99 ~~-67~~

diff -r 6c22ae0f6540 tools/libxl/libxl.c
@@ -588,7 +588,7 @@ int libxl_wait_for_disk_ejects(libxl_ctx
     for (i = 0; i < num_disks; i++) {
         if (asprintf(&(waiter[i].path), "%s/device/vbd/%d/eject",
                      libxl__xs_get_dompath(&gc, domid),
-                     libxl__device_disk_dev_number(disks[i].virtpath)) < 0)
+                     libxl__device_disk_dev_number(disks[i].vdev)) < 0)
             goto out;
         if (asprintf(&(waiter[i].token), "%d", LIBXL_EVENT_DISK_EJECT) < 0)
             goto out;

@@ -668,10 +668,10 @@ int libxl_event_get_disk_eject_info(libx
 
     disk->backend_domid = 0;
     disk->domid = domid;
-    disk->physpath = strdup("");
-    disk->phystype = PHYSTYPE_EMPTY;
+    disk->pdev_path = strdup("");
+    disk->format = DISK_FORMAT_EMPTY;
     /* this value is returned to the user: do not free right away */
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
     disk->unpluggable = 1;
     disk->readwrite = 0;
     disk->is_cdrom = 1;

@@ -863,24 +863,25 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, libxl_disk_phystype disk_type)
+static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
+    libxl_disk_backend backend_type, libxl_disk_format format)
 {
     struct stat stat_buf;
 
-    if ( (file_name[0] == '\0') && (disk_type == PHYSTYPE_EMPTY) )
+    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
         return 0;
 
-    if ( stat(file_name, &stat_buf) != 0 ) {
+    if (stat(file_name, &stat_buf) != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;
     }
-    if ( disk_type == PHYSTYPE_PHY ) {
+    if (backend_type == DISK_BACKEND_PHY) {
         if ( !(S_ISBLK(stat_buf.st_mode)) ) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n",
                 file_name);
             return ERROR_INVAL;
         }
-    } else if ( S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0 ) {
+    } else if (S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is 0!\n", file_name);
         return ERROR_INVAL;
     }

@@ -898,7 +899,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__device device;
     int major, minor, rc;
 
-    rc = validate_virtual_disk(ctx, disk->physpath, disk->phystype);
+    rc = validate_virtual_disk(ctx, disk->pdev_path, disk->backend, 
+             disk->format);
     if (rc)
         return rc;

@@ -913,11 +915,11 @@ int libxl_device_disk_add(libxl_ctx *ctx
         goto out_free;
     }
 
-    backend_type = libxl__device_disk_backend_type_of_phystype(disk->phystype);
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    backend_type = libxl__device_disk_string_of_backend(disk->backend);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     if (devid==-1) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->virtpath);
+               " virtual disk identifier %s", disk->vdev);
         rc = ERROR_INVAL;
         goto out_free;
     }

@@ -928,37 +930,33 @@ int libxl_device_disk_add(libxl_ctx *ctx
     device.domid = disk->domid;
     device.kind = DEVICE_VBD;
 
-    switch (disk->phystype) {
-        case PHYSTYPE_PHY: {
-
-            libxl__device_physdisk_major_minor(disk->physpath, &major, &minor);
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
             flexarray_append(back, "physical-device");
             flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
 
             flexarray_append(back, "params");
-            flexarray_append(back, disk->physpath);
+            flexarray_append(back, disk->pdev_path);
 
             device.backend_kind = DEVICE_VBD;
             break;
         }
-        case PHYSTYPE_EMPTY:
-            break;
-        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:
+        case DISK_BACKEND_TAP:
+        case DISK_BACKEND_QDISK: {
+            if (disk->format == DISK_FORMAT_EMPTY)
+                break;
             if (libxl__blktap_enabled(&gc)) {
                 const char *dev = libxl__blktap_devpath(&gc,
-                                               disk->physpath, disk->phystype);
+                                               disk->pdev_path, disk->format);
                 if (!dev) {
                     rc = ERROR_FAIL;
                     goto out_free;
                 }
                 flexarray_append(back, "tapdisk-params");
-                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", 
+                    libxl__device_disk_string_of_format(disk->format), 
+                    disk->pdev_path));
                 flexarray_append(back, "params");
                 flexarray_append(back, libxl__strdup(&gc, dev));
                 backend_type = "phy";

@@ -971,16 +969,17 @@ int libxl_device_disk_add(libxl_ctx *ctx
             }
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
-                          libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
 
-            if (libxl__blktap_enabled(&gc))
+            if (libxl__blktap_enabled(&gc) && 
+                 disk->format != DISK_BACKEND_QDISK)
                 device.backend_kind = DEVICE_TAP;
             else
                 device.backend_kind = DEVICE_QDISK;
             break;
-
+        }
         default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", disk->phystype);
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
             rc = ERROR_INVAL;
             goto out_free;
     }

@@ -996,7 +995,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_append(back, "state");
     flexarray_append(back, libxl__sprintf(&gc, "%d", 1));
     flexarray_append(back, "dev");
-    flexarray_append(back, disk->virtpath);
+    flexarray_append(back, disk->vdev);
     flexarray_append(back, "type");
     flexarray_append(back, backend_type);
     flexarray_append(back, "mode");

@@ -1036,11 +1035,11 @@ int libxl_device_disk_del(libxl_ctx *ctx
     libxl__device device;
     int devid;
 
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     = 
-        (disk->phystype == PHYSTYPE_PHY) ? DEVICE_VBD : DEVICE_TAP;
+        (disk->backend == DISK_BACKEND_PHY) ? DEVICE_VBD : DEVICE_TAP;
     device.domid            = disk->domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;

@@ -1052,36 +1051,67 @@ char * libxl_device_disk_local_attach(li
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     const char *dev = NULL;
     char *ret = NULL;
-    int phystype = disk->phystype;
-    switch (phystype) {
-        case PHYSTYPE_PHY: {
-            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->physpath);
-            dev = disk->physpath;
+
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: {
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "physical block device must"
+                    " be raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching PHY disk %s to domain 0",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
         }
-        case PHYSTYPE_FILE:
-            /* let's pretend is tap:aio for the moment */
-            phystype = PHYSTYPE_AIO;
-        case PHYSTYPE_AIO:
-            if (!libxl__blktap_enabled(&gc)) {
-                dev = disk->physpath;
+        case DISK_BACKEND_TAP: {
+            if (disk->format == DISK_FORMAT_VHD || disk->format == DISK_FORMAT_RAW)
+            {
+                if (libxl__blktap_enabled(&gc))
+                    dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->format);
+                else {
+                    if (disk->format != DISK_FORMAT_RAW) {
+                        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required"
+                            " to open a vhd disk");
+                        break;
+                    } else {
+                        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching tap disk %s to domain 0",
+                            disk->pdev_path);
+                        dev = disk->pdev_path;
+                        break;
+                    }
+                }
+                break;
+            } else if (disk->format == DISK_FORMAT_QCOW ||
+                       disk->format == DISK_FORMAT_QCOW2) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image");
+                break;
+            } else {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                    "type: %d", disk->backend);
                 break;
             }
-        case PHYSTYPE_VHD:
-            if (libxl__blktap_enabled(&gc))
-                dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
-            else
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
-            break;
-        case PHYSTYPE_QCOW:
-        case PHYSTYPE_QCOW2:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
+        }
+        case DISK_BACKEND_QDISK: {
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk "
+                    "image if the format is not raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching qdisk %s to domain 0\n",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
 
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
+        }
+        case DISK_BACKEND_UNKNOWN:
+        default: {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                "type: %d", disk->backend);
             break;
+        }
     }
+
     if (dev != NULL)
         ret = strdup(dev);
     libxl__free_all(&gc);

@@ -1677,13 +1707,15 @@ static unsigned int libxl_append_disk_li
             pdisk->domid = domid;
             physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path, *dir), &len);
             if (physpath_tmp && strchr(physpath_tmp, ':')) {
-                pdisk->physpath = strdup(strchr(physpath_tmp, ':') + 1);
+                pdisk->pdev_path = strdup(strchr(physpath_tmp, ':') + 1);
                 free(physpath_tmp);
             } else {
-                pdisk->physpath = physpath_tmp;
+                pdisk->pdev_path = physpath_tmp;
             }
-            libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->phystype));
-            pdisk->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
+            libxl_string_to_backend(ctx, libxl__xs_read(&gc, XBT_NULL, 
+                libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), 
+                &(pdisk->backend));
+            pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
             pdisk->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir)));
             if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path, *dir)), "w"))
                 pdisk->readwrite = 1;

@@ -1718,7 +1750,7 @@ int libxl_device_disk_getinfo(libxl_ctx 
     char *val;
 
     dompath = libxl__xs_get_dompath(&gc, domid);
-    diskinfo->devid = libxl__device_disk_dev_number(disk->virtpath);
+    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev);
 
     /* tap devices entries in xenstore are written as vbd devices. */
     diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid);

@@ -1752,13 +1784,13 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
     libxl_device_disk *disks;
     int ret = ERROR_FAIL;
 
-    if (!disk->physpath) {
-        disk->physpath = strdup("");
-        disk->phystype = PHYSTYPE_EMPTY;
+    if (!disk->pdev_path) {
+        disk->pdev_path = strdup("");
+        disk->format = DISK_FORMAT_EMPTY;
     }
     disks = libxl_device_disk_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
-        if (disks[i].is_cdrom && !strcmp(disk->virtpath, disks[i].virtpath))
+        if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
             /* found */
             break;
     }

tools/libxl/libxl.h +14 -8

diff -r 6c22ae0f6540 tools/libxl/libxl.h
@@ -172,14 +172,20 @@ typedef enum {
 } libxl_console_consback;
 
 typedef enum {
-    PHYSTYPE_QCOW = 1,
-    PHYSTYPE_QCOW2,
-    PHYSTYPE_VHD,
-    PHYSTYPE_AIO,
-    PHYSTYPE_FILE,
-    PHYSTYPE_PHY,
-    PHYSTYPE_EMPTY,
-} libxl_disk_phystype;
+    DISK_FORMAT_UNKNOWN = 0,
+    DISK_FORMAT_QCOW,
+    DISK_FORMAT_QCOW2,
+    DISK_FORMAT_VHD,
+    DISK_FORMAT_RAW,
+    DISK_FORMAT_EMPTY,
+} libxl_disk_format;
+
+typedef enum {
+    DISK_BACKEND_UNKNOWN = 0,
+    DISK_BACKEND_PHY,
+    DISK_BACKEND_TAP,
+    DISK_BACKEND_QDISK,
+} libxl_disk_backend;
 
 typedef enum {
     NICTYPE_IOEMU = 1,

tools/libxl/libxl.idl +6 -4

diff -r 6c22ae0f6540 tools/libxl/libxl.idl
@@ -11,7 +11,8 @@ libxl_qemu_machine_type = Number("qemu_m
 libxl_qemu_machine_type = Number("qemu_machine_type", namespace="libxl_")
 libxl_console_consback = Number("console_consback", namespace="libxl_")
 libxl_console_constype = Number("console_constype", namespace="libxl_")
-libxl_disk_phystype = Number("disk_phystype", namespace="libxl_")
+libxl_disk_format = Number("disk_format", namespace="libxl_")
+libxl_disk_backend = Number("disk_backend", namespace="libxl_")
 libxl_nic_type = Number("nic_type", namespace="libxl_")
 libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE)

@@ -203,9 +204,10 @@ libxl_device_disk = Struct("device_disk"
 libxl_device_disk = Struct("device_disk", [
     ("backend_domid", uint32),
     ("domid", domid),
-    ("physpath", string),
-    ("phystype", libxl_disk_phystype),
-    ("virtpath", string),
+    ("pdev_path", string),
+    ("vdev", string),
+    ("backend", libxl_disk_backend),
+    ("format", libxl_disk_format),
     ("unpluggable", integer),
     ("readwrite", integer),
     ("is_cdrom", integer),

tools/libxl/libxl_blktap2.c +2 -2

diff -r 6c22ae0f6540 tools/libxl/libxl_blktap2.c
@@ -26,13 +26,13 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     const char *type;
     char *params, *devname = NULL;
     int minor, err;
 
-    type = libxl__device_disk_string_of_phystype(phystype);
+    type = libxl__device_disk_string_of_format(format);
     minor = tap_ctl_find_minor(type, disk);
     if (minor >= 0) {
         devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", minor);

tools/libxl/libxl_device.c +13 ~~-20~~

diff -r 6c22ae0f6540 tools/libxl/libxl_device.c
@@ -121,31 +121,24 @@ out:
     return rc;
 }
 
-char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_format(libxl_disk_format format)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "qcow";
-        case PHYSTYPE_QCOW2: return "qcow2";
-        case PHYSTYPE_VHD: return "vhd";
-        case PHYSTYPE_AIO: return "aio";
-        case PHYSTYPE_FILE: return "file";
-        case PHYSTYPE_PHY: return "phy";
-        case PHYSTYPE_EMPTY: return "file";
-        default: return NULL;
+    switch (format) {
+        case DISK_FORMAT_QCOW: return "qcow";
+        case DISK_FORMAT_QCOW2: return "qcow2"; 
+        case DISK_FORMAT_VHD: return "vhd"; 
+        case DISK_FORMAT_RAW:
+        case DISK_FORMAT_EMPTY: return "aio"; 
+        default: return NULL; 
     }
 }
 
-char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "tap";
-        case PHYSTYPE_QCOW2: return "tap";
-        case PHYSTYPE_VHD: return "tap";
-        case PHYSTYPE_AIO: return "tap";
-        /* let's pretend file is tap:aio */
-        case PHYSTYPE_FILE: return "tap";
-        case PHYSTYPE_EMPTY: return "tap";
-        case PHYSTYPE_PHY: return "phy";
+    switch (backend) {
+        case DISK_BACKEND_QDISK: return "qdisk";
+        case DISK_BACKEND_TAP: return "tap";
+        case DISK_BACKEND_PHY: return "phy";
         default: return NULL;
     }
 }

tools/libxl/libxl_dm.c +3 -3

diff -r 6c22ae0f6540 tools/libxl/libxl_dm.c
@@ -316,10 +316,10 @@ static char ** libxl_build_device_model_
         for (i; i < nb; i++) {
             if (disks[i].is_cdrom) {
                 flexarray_append(dm_args, "-cdrom");
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].virtpath));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             }
             libxl_device_disk_destroy(&disks[i]);
         }

tools/libxl/libxl_internal.h +3 -3

diff -r 6c22ae0f6540 tools/libxl/libxl_internal.h
@@ -178,8 +178,8 @@ _hidden void libxl__userdata_destroyall(
 _hidden void libxl__userdata_destroyall(libxl_ctx *ctx, uint32_t domid);
 
 /* from xl_device */
-_hidden char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
-_hidden char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype);
+_hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
+_hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
 _hidden int libxl__device_disk_dev_number(char *virtpath);

@@ -306,7 +306,7 @@ _hidden int libxl__blktap_enabled(libxl_
  */
 _hidden const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype);
+                                 libxl_disk_format format);
 
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);

tools/libxl/libxl_noblktap2.c +1 -1

diff -r 6c22ae0f6540 tools/libxl/libxl_noblktap2.c
@@ -23,7 +23,7 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     return NULL;
 }

tools/libxl/libxl_utils.c +10 ~~-12~~

diff -r 6c22ae0f6540 tools/libxl/libxl_utils.c
@@ -275,15 +275,15 @@ out:
     return rc;
 }
 
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype)
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend)
 {
     char *p;
     int rc = 0;
 
     if (!strcmp(s, "phy")) {
-        *phystype = PHYSTYPE_PHY;
+        *backend = DISK_BACKEND_PHY;
     } else if (!strcmp(s, "file")) {
-        *phystype = PHYSTYPE_FILE;
+        *backend = DISK_BACKEND_TAP;
     } else if (!strcmp(s, "tap")) {
         p = strchr(s, ':');
         if (!p) {

@@ -291,14 +291,12 @@ int libxl_string_to_phystype(libxl_ctx *
             goto out;
         }
         p++;
-        if (!strcmp(p, "aio")) {
-            *phystype = PHYSTYPE_AIO;
-        } else if (!strcmp(p, "vhd")) {
-            *phystype = PHYSTYPE_VHD;
+        if (!strcmp(p, "vhd")) {
+            *backend = DISK_BACKEND_TAP;
         } else if (!strcmp(p, "qcow")) {
-            *phystype = PHYSTYPE_QCOW;
+            *backend = DISK_BACKEND_QDISK;
         } else if (!strcmp(p, "qcow2")) {
-            *phystype = PHYSTYPE_QCOW2;
+            *backend = DISK_BACKEND_QDISK;
         }
     }
 out:

@@ -553,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx
     disk->backend_domid = strtoul(val, NULL, 10);
     disk->domid = domid;
     be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath));
-    disk->physpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
+    disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
-    libxl_string_to_phystype(ctx, val, &(disk->phystype));
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
+    libxl_string_to_backend(ctx, val, &(disk->backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path));
     disk->unpluggable = !strcmp(val, "1");
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path));

tools/libxl/libxl_utils.h +1 -1

diff -r 6c22ae0f6540 tools/libxl/libxl_utils.h
@@ -29,7 +29,7 @@ int libxl_get_stubdom_id(libxl_ctx *ctx,
 int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid);
 int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid);
 int libxl_create_logfile(libxl_ctx *ctx, char *name, char **full_name);
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype);
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend);
 
 int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
                              void **data_r, int *datalen_r);

tools/libxl/xl_cmdimpl.c +68 ~~-50~~

230 lines not on this page · raw message

tools/ocaml/libs/xl/xl_stubs.c +7 -6

diff -r 6c22ae0f6540 tools/ocaml/libs/xl/xl_stubs.c
@@ -192,12 +192,13 @@ static int device_disk_val(caml_gc *gc, 
 	CAMLparam1(v);
 
 	c_val->backend_domid = Int_val(Field(v, 0));
-	c_val->physpath = dup_String_val(gc, Field(v, 1));
-	c_val->phystype = (Int_val(Field(v, 2))) + PHYSTYPE_QCOW;
-	c_val->virtpath = dup_String_val(gc, Field(v, 3));
-	c_val->unpluggable = Bool_val(Field(v, 4));
-	c_val->readwrite = Bool_val(Field(v, 5));
-	c_val->is_cdrom = Bool_val(Field(v, 6));
+	c_val->pdev_path = dup_String_val(gc, Field(v, 1));
+	c_val->vdev = dup_String_val(gc, Field(v, 2));
+        c_val->backend = (Int_val(Field(v, 3)));
+        c_val->format = (Int_val(Field(v, 4)));
+	c_val->unpluggable = Bool_val(Field(v, 5));
+	c_val->readwrite = Bool_val(Field(v, 6));
+	c_val->is_cdrom = Bool_val(Field(v, 7));
 
 	CAMLreturn(0);
 }

tools/python/xen/lowlevel/xl/xl.c +11 -6

diff -r 6c22ae0f6540 tools/python/xen/lowlevel/xl/xl.c
@@ -779,12 +779,17 @@ PyMODINIT_FUNC initxl(void)
     _INT_CONST_LIBXL(m, CONSBACK_XENCONSOLED);
     _INT_CONST_LIBXL(m, CONSBACK_IOEMU);
 
-    _INT_CONST(m, PHYSTYPE_QCOW);
-    _INT_CONST(m, PHYSTYPE_QCOW2);
-    _INT_CONST(m, PHYSTYPE_VHD);
-    _INT_CONST(m, PHYSTYPE_AIO);
-    _INT_CONST(m, PHYSTYPE_FILE);
-    _INT_CONST(m, PHYSTYPE_PHY);
+    _INT_CONST(m, DISK_FORMAT_UNKNOWN);
+    _INT_CONST(m, DISK_FORMAT_QCOW);
+    _INT_CONST(m, DISK_FORMAT_QCOW2);
+    _INT_CONST(m, DISK_FORMAT_VHD);
+    _INT_CONST(m, DISK_FORMAT_RAW);
+    _INT_CONST(m, DISK_FORMAT_EMPTY);
+
+    _INT_CONST(m, DISK_BACKEND_UNKNOWN);
+    _INT_CONST(m, DISK_BACKEND_PHY);
+    _INT_CONST(m, DISK_BACKEND_TAP);
+    _INT_CONST(m, DISK_BACKEND_QDISK);
 
     _INT_CONST(m, NICTYPE_IOEMU);
     _INT_CONST(m, NICTYPE_VIF);

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

Ian Jackson

2011-02-14 17:45 UTC #12 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Kamala Narasimhan writes ("Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts"):

Quoted 2 lines

Per suggestion, along with the latest patch is a more detailed description to add to the check-in message -

It looks good to me, thanks, apart from some nits. If you could address these I'll apply it :-).

tools/libxl/libxl.c int libxl_device_disk_add(libxl_ctx *ctx · line 975

Quoted 3 lines

tools/libxl/libxl.c +2 -1

-            if (libxl__blktap_enabled(&gc))
+            if (libxl__blktap_enabled(&gc) && 
+                 disk->format != DISK_BACKEND_QDISK)

Some mistake here surely ?

Quoted 3 lines

diff -r 6c22ae0f6540 tools/libxl/libxl.c
tools/libxl/libxl.c +0 -0

...

Quoted 1 line

- switch (disk->phystype) {

...

Quoted 1 line

+ switch (disk->backend) {

You have introduced these braces { }, but that seems to me to be just a stylistic change and they are not really needed ?

Quoted 1 line

+ libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);

This function can fail. It has two call sites neither of which check the return value. Perhaps that should be addressed in a separate patch, as your change here isn't making it worse. So no need to do anything about this right now if you don't want to.

tools/libxl/libxl.c int libxl_device_disk_add(libxl_ctx *ctx · line 946

Quoted 4 lines

tools/libxl/libxl.c +2 -2

-        case PHYSTYPE_QCOW2:
-        case PHYSTYPE_VHD:
+        case DISK_BACKEND_TAP:
+        case DISK_BACKEND_QDISK: {

Once again, this is a stylistic change. These { } are not used elsewhere in libxl so you should not introduce them. (It would be a different matter if there were some reason why they are required in a particular case, eg to introduce a relevant block scope.)

tools/libxl/libxl.c

Quoted 4 lines

case DSTATE_PHYSPATH:

tools/libxl/libxl.c +1 -1

-            if ( *p == ',' ) {
+            if (*p == ',') {
                 int ioemu_len;

It is good that you fixed up the formatting in code you changed, but please do not make formatting changes to unchanged lines.

We will do a style cleanup after 4.1 is released.

tools/libxl/libxl.c

Quoted 2 lines

tools/libxl/libxl.c +1 -1

-                if ( tok + ioemu_len < end &&
+                if (tok + ioemu_len < end &&

Once again. Can you make sure your patch doesn't have /any/ unrelated formatting changes to lines you don't touch, please ?

Ian.

Kamala Narasimhan

author

2011-02-14 18:30 UTC #13 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Quoted 4 lines · Ian Jackson

Kamala Narasimhan writes ("Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts"):
Quoted 2 lines
Per suggestion, along with the latest patch is a more detailed description to add to the check-in message -

Quoted 9 lines

It looks good to me, thanks, apart from some nits. If you could address these I'll apply it :-).
tools/libxl/libxl.c int libxl_device_disk_add(libxl_ctx *ctx · line 975
Quoted 3 lines
tools/libxl/libxl.c +2 -1
-            if (libxl__blktap_enabled(&gc))
+            if (libxl__blktap_enabled(&gc) && 
+                 disk->format != DISK_BACKEND_QDISK)
Some mistake here surely ?

Of course!

Quoted 11 lines

Quoted 3 lines
diff -r 6c22ae0f6540 tools/libxl/libxl.c
tools/libxl/libxl.c +0 -0
...
Quoted 1 line
- switch (disk->phystype) {
...
Quoted 1 line
+ switch (disk->backend) {
You have introduced these braces { }, but that seems to me to be just a stylistic change and they are not really needed ?

No, I have switched disk->phystype to disk->backend.

Quoted 7 lines

Quoted 1 line
+ libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
This function can fail. It has two call sites neither of which check the return value. Perhaps that should be addressed in a separate patch, as your change here isn't making it worse. So no need to do anything about this right now if you don't want to.

Yes, it isn't part of what we intended to change. There are other things too with respect to disk configuration option code that requires revisiting.

Quoted 10 lines

tools/libxl/libxl.c int libxl_device_disk_add(libxl_ctx *ctx · line 946
Quoted 4 lines
tools/libxl/libxl.c +2 -2
-        case PHYSTYPE_QCOW2:
-        case PHYSTYPE_VHD:
+        case DISK_BACKEND_TAP:
+        case DISK_BACKEND_QDISK: {
Once again, this is a stylistic change. These { } are not used elsewhere in libxl so you should not introduce them. (It would be a different matter if there were some reason why they are required in a particular case, eg to introduce a relevant block scope.)

I agree, will change.

Quoted 8 lines

tools/libxl/libxl.c
Quoted 4 lines
case DSTATE_PHYSPATH:
tools/libxl/libxl.c +1 -1
-            if ( *p == ',' ) {
+            if (*p == ',') {
                 int ioemu_len;
It is good that you fixed up the formatting in code you changed, but please do not make formatting changes to unchanged lines.

Argg... I was worried you might ask why I didn't fix the style around the code I changed. I have reverted the changes.

Quoted 8 lines

We will do a style cleanup after 4.1 is released.
tools/libxl/libxl.c
Quoted 2 lines
tools/libxl/libxl.c +1 -1
-                if ( tok + ioemu_len < end &&
+                if (tok + ioemu_len < end &&
Once again. Can you make sure your patch doesn't have /any/ unrelated formatting changes to lines you don't touch, please ?

Yes.

I will send a patch momentarily.

Kamala

Kamala Narasimhan

author

2011-02-14 19:51 UTC #14 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

In reply to Ian Jackson

Attached patch should address your concerns.

Posting the patch description again for convenience -

Per suggestion, along with the latest patch is a more detailed description to add to the check-in message -

Signed-off-by Kamala Narasimhan <[email protected]>

Kamala

tools/libxl/libxl.c +93 ~~-68~~

diff -r 6c22ae0f6540 tools/libxl/libxl.c
@@ -588,7 +588,7 @@ int libxl_wait_for_disk_ejects(libxl_ctx
     for (i = 0; i < num_disks; i++) {
         if (asprintf(&(waiter[i].path), "%s/device/vbd/%d/eject",
                      libxl__xs_get_dompath(&gc, domid),
-                     libxl__device_disk_dev_number(disks[i].virtpath)) < 0)
+                     libxl__device_disk_dev_number(disks[i].vdev)) < 0)
             goto out;
         if (asprintf(&(waiter[i].token), "%d", LIBXL_EVENT_DISK_EJECT) < 0)
             goto out;

@@ -668,10 +668,10 @@ int libxl_event_get_disk_eject_info(libx
 
     disk->backend_domid = 0;
     disk->domid = domid;
-    disk->physpath = strdup("");
-    disk->phystype = PHYSTYPE_EMPTY;
+    disk->pdev_path = strdup("");
+    disk->format = DISK_FORMAT_EMPTY;
     /* this value is returned to the user: do not free right away */
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", backend));
     disk->unpluggable = 1;
     disk->readwrite = 0;
     disk->is_cdrom = 1;

@@ -863,18 +863,19 @@ int libxl_vncviewer_exec(libxl_ctx *ctx,
 
 /******************************************************************************/
 
-static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, libxl_disk_phystype disk_type)
+static int validate_virtual_disk(libxl_ctx *ctx, char *file_name, 
+    libxl_disk_backend backend_type, libxl_disk_format format)
 {
     struct stat stat_buf;
 
-    if ( (file_name[0] == '\0') && (disk_type == PHYSTYPE_EMPTY) )
+    if ((file_name[0] == '\0') && (format == DISK_FORMAT_EMPTY))
         return 0;
 
     if ( stat(file_name, &stat_buf) != 0 ) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "failed to stat %s", file_name);
         return ERROR_INVAL;
     }
-    if ( disk_type == PHYSTYPE_PHY ) {
+    if (backend_type == DISK_BACKEND_PHY) {
         if ( !(S_ISBLK(stat_buf.st_mode)) ) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n",
                 file_name);

@@ -898,7 +899,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__device device;
     int major, minor, rc;
 
-    rc = validate_virtual_disk(ctx, disk->physpath, disk->phystype);
+    rc = validate_virtual_disk(ctx, disk->pdev_path, disk->backend, 
+             disk->format);
     if (rc)
         return rc;

@@ -913,11 +915,11 @@ int libxl_device_disk_add(libxl_ctx *ctx
         goto out_free;
     }
 
-    backend_type = libxl__device_disk_backend_type_of_phystype(disk->phystype);
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    backend_type = libxl__device_disk_string_of_backend(disk->backend);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     if (devid==-1) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->virtpath);
+               " virtual disk identifier %s", disk->vdev);
         rc = ERROR_INVAL;
         goto out_free;
     }

@@ -928,37 +930,32 @@ int libxl_device_disk_add(libxl_ctx *ctx
     device.domid = disk->domid;
     device.kind = DEVICE_VBD;
 
-    switch (disk->phystype) {
-        case PHYSTYPE_PHY: {
-
-            libxl__device_physdisk_major_minor(disk->physpath, &major, &minor);
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: 
+            libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
             flexarray_append(back, "physical-device");
             flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
 
             flexarray_append(back, "params");
-            flexarray_append(back, disk->physpath);
+            flexarray_append(back, disk->pdev_path);
 
             device.backend_kind = DEVICE_VBD;
             break;
-        }
-        case PHYSTYPE_EMPTY:
-            break;
-        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:
+        case DISK_BACKEND_TAP:
+        case DISK_BACKEND_QDISK: 
+            if (disk->format == DISK_FORMAT_EMPTY)
+                break;
             if (libxl__blktap_enabled(&gc)) {
                 const char *dev = libxl__blktap_devpath(&gc,
-                                               disk->physpath, disk->phystype);
+                                               disk->pdev_path, disk->format);
                 if (!dev) {
                     rc = ERROR_FAIL;
                     goto out_free;
                 }
                 flexarray_append(back, "tapdisk-params");
-                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", 
+                    libxl__device_disk_string_of_format(disk->format), 
+                    disk->pdev_path));
                 flexarray_append(back, "params");
                 flexarray_append(back, libxl__strdup(&gc, dev));
                 backend_type = "phy";

@@ -971,16 +968,16 @@ int libxl_device_disk_add(libxl_ctx *ctx
             }
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
-                          libxl__device_disk_string_of_phystype(disk->phystype), disk->physpath));
+                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
 
-            if (libxl__blktap_enabled(&gc))
+            if (libxl__blktap_enabled(&gc) && 
+                 disk->backend != DISK_BACKEND_QDISK)
                 device.backend_kind = DEVICE_TAP;
             else
                 device.backend_kind = DEVICE_QDISK;
             break;
-
         default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", disk->phystype);
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
             rc = ERROR_INVAL;
             goto out_free;
     }

@@ -996,7 +993,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_append(back, "state");
     flexarray_append(back, libxl__sprintf(&gc, "%d", 1));
     flexarray_append(back, "dev");
-    flexarray_append(back, disk->virtpath);
+    flexarray_append(back, disk->vdev);
     flexarray_append(back, "type");
     flexarray_append(back, backend_type);
     flexarray_append(back, "mode");

@@ -1036,11 +1033,11 @@ int libxl_device_disk_del(libxl_ctx *ctx
     libxl__device device;
     int devid;
 
-    devid = libxl__device_disk_dev_number(disk->virtpath);
+    devid = libxl__device_disk_dev_number(disk->vdev);
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     = 
-        (disk->phystype == PHYSTYPE_PHY) ? DEVICE_VBD : DEVICE_TAP;
+        (disk->backend == DISK_BACKEND_PHY) ? DEVICE_VBD : DEVICE_TAP;
     device.domid            = disk->domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;

@@ -1052,36 +1049,62 @@ char * libxl_device_disk_local_attach(li
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     const char *dev = NULL;
     char *ret = NULL;
-    int phystype = disk->phystype;
-    switch (phystype) {
-        case PHYSTYPE_PHY: {
-            fprintf(stderr, "attaching PHY disk %s to domain 0\n", disk->physpath);
-            dev = disk->physpath;
-            break;
-        }
-        case PHYSTYPE_FILE:
-            /* let's pretend is tap:aio for the moment */
-            phystype = PHYSTYPE_AIO;
-        case PHYSTYPE_AIO:
-            if (!libxl__blktap_enabled(&gc)) {
-                dev = disk->physpath;
+
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY: 
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "physical block device must"
+                    " be raw");
                 break;
             }
-        case PHYSTYPE_VHD:
-            if (libxl__blktap_enabled(&gc))
-                dev = libxl__blktap_devpath(&gc, disk->physpath, phystype);
-            else
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required to open a vhd disk\n");
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching PHY disk %s to domain 0",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
-        case PHYSTYPE_QCOW:
-        case PHYSTYPE_QCOW2:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image\n");
+        case DISK_BACKEND_TAP: 
+            if (disk->format == DISK_FORMAT_VHD || disk->format == DISK_FORMAT_RAW)
+            {
+                if (libxl__blktap_enabled(&gc))
+                    dev = libxl__blktap_devpath(&gc, disk->pdev_path, disk->format);
+                else {
+                    if (disk->format != DISK_FORMAT_RAW) {
+                        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "tapdisk2 is required"
+                            " to open a vhd disk");
+                        break;
+                    } else {
+                        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching tap disk %s to domain 0",
+                            disk->pdev_path);
+                        dev = disk->pdev_path;
+                        break;
+                    }
+                }
+                break;
+            } else if (disk->format == DISK_FORMAT_QCOW ||
+                       disk->format == DISK_FORMAT_QCOW2) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qcow or qcow2 disk image");
+                break;
+            } else {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                    "type: %d", disk->backend);
+                break;
+            }
+        case DISK_BACKEND_QDISK: 
+            if (disk->format != DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk "
+                    "image if the format is not raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "attaching qdisk %s to domain 0\n",
+                disk->pdev_path);
+            dev = disk->pdev_path;
             break;
-
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical type: %d\n", phystype);
+        case DISK_BACKEND_UNKNOWN:
+        default: 
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                "type: %d", disk->backend);
             break;
     }
+
     if (dev != NULL)
         ret = strdup(dev);
     libxl__free_all(&gc);

@@ -1677,13 +1700,15 @@ static unsigned int libxl_append_disk_li
             pdisk->domid = domid;
             physpath_tmp = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/params", be_path, *dir), &len);
             if (physpath_tmp && strchr(physpath_tmp, ':')) {
-                pdisk->physpath = strdup(strchr(physpath_tmp, ':') + 1);
+                pdisk->pdev_path = strdup(strchr(physpath_tmp, ':') + 1);
                 free(physpath_tmp);
             } else {
-                pdisk->physpath = physpath_tmp;
+                pdisk->pdev_path = physpath_tmp;
             }
-            libxl_string_to_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->phystype));
-            pdisk->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
+            libxl_string_to_backend(ctx, libxl__xs_read(&gc, XBT_NULL, 
+                libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), 
+                &(pdisk->backend));
+            pdisk->vdev = xs_read(ctx->xsh, XBT_NULL, libxl__sprintf(&gc, "%s/%s/dev", be_path, *dir), &len);
             pdisk->unpluggable = atoi(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/removable", be_path, *dir)));
             if (!strcmp(libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/%s/mode", be_path, *dir)), "w"))
                 pdisk->readwrite = 1;

@@ -1718,7 +1743,7 @@ int libxl_device_disk_getinfo(libxl_ctx 
     char *val;
 
     dompath = libxl__xs_get_dompath(&gc, domid);
-    diskinfo->devid = libxl__device_disk_dev_number(disk->virtpath);
+    diskinfo->devid = libxl__device_disk_dev_number(disk->vdev);
 
     /* tap devices entries in xenstore are written as vbd devices. */
     diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, diskinfo->devid);

@@ -1752,13 +1777,13 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
     libxl_device_disk *disks;
     int ret = ERROR_FAIL;
 
-    if (!disk->physpath) {
-        disk->physpath = strdup("");
-        disk->phystype = PHYSTYPE_EMPTY;
+    if (!disk->pdev_path) {
+        disk->pdev_path = strdup("");
+        disk->format = DISK_FORMAT_EMPTY;
     }
     disks = libxl_device_disk_list(ctx, domid, &num);
     for (i = 0; i < num; i++) {
-        if (disks[i].is_cdrom && !strcmp(disk->virtpath, disks[i].virtpath))
+        if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
             /* found */
             break;
     }

tools/libxl/libxl.h +14 -8

diff -r 6c22ae0f6540 tools/libxl/libxl.h
@@ -172,14 +172,20 @@ typedef enum {
 } libxl_console_consback;
 
 typedef enum {
-    PHYSTYPE_QCOW = 1,
-    PHYSTYPE_QCOW2,
-    PHYSTYPE_VHD,
-    PHYSTYPE_AIO,
-    PHYSTYPE_FILE,
-    PHYSTYPE_PHY,
-    PHYSTYPE_EMPTY,
-} libxl_disk_phystype;
+    DISK_FORMAT_UNKNOWN = 0,
+    DISK_FORMAT_QCOW,
+    DISK_FORMAT_QCOW2,
+    DISK_FORMAT_VHD,
+    DISK_FORMAT_RAW,
+    DISK_FORMAT_EMPTY,
+} libxl_disk_format;
+
+typedef enum {
+    DISK_BACKEND_UNKNOWN = 0,
+    DISK_BACKEND_PHY,
+    DISK_BACKEND_TAP,
+    DISK_BACKEND_QDISK,
+} libxl_disk_backend;
 
 typedef enum {
     NICTYPE_IOEMU = 1,

tools/libxl/libxl.idl +6 -4

diff -r 6c22ae0f6540 tools/libxl/libxl.idl
@@ -11,7 +11,8 @@ libxl_qemu_machine_type = Number("qemu_m
 libxl_qemu_machine_type = Number("qemu_machine_type", namespace="libxl_")
 libxl_console_consback = Number("console_consback", namespace="libxl_")
 libxl_console_constype = Number("console_constype", namespace="libxl_")
-libxl_disk_phystype = Number("disk_phystype", namespace="libxl_")
+libxl_disk_format = Number("disk_format", namespace="libxl_")
+libxl_disk_backend = Number("disk_backend", namespace="libxl_")
 libxl_nic_type = Number("nic_type", namespace="libxl_")
 libxl_cpuid_policy_list = Builtin("cpuid_policy_list", destructor_fn="libxl_cpuid_destroy", passby=PASS_BY_REFERENCE)

@@ -203,9 +204,10 @@ libxl_device_disk = Struct("device_disk"
 libxl_device_disk = Struct("device_disk", [
     ("backend_domid", uint32),
     ("domid", domid),
-    ("physpath", string),
-    ("phystype", libxl_disk_phystype),
-    ("virtpath", string),
+    ("pdev_path", string),
+    ("vdev", string),
+    ("backend", libxl_disk_backend),
+    ("format", libxl_disk_format),
     ("unpluggable", integer),
     ("readwrite", integer),
     ("is_cdrom", integer),

tools/libxl/libxl_blktap2.c +2 -2

diff -r 6c22ae0f6540 tools/libxl/libxl_blktap2.c
@@ -26,13 +26,13 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     const char *type;
     char *params, *devname = NULL;
     int minor, err;
 
-    type = libxl__device_disk_string_of_phystype(phystype);
+    type = libxl__device_disk_string_of_format(format);
     minor = tap_ctl_find_minor(type, disk);
     if (minor >= 0) {
         devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", minor);

tools/libxl/libxl_device.c +13 ~~-20~~

diff -r 6c22ae0f6540 tools/libxl/libxl_device.c
@@ -121,31 +121,24 @@ out:
     return rc;
 }
 
-char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_format(libxl_disk_format format)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "qcow";
-        case PHYSTYPE_QCOW2: return "qcow2";
-        case PHYSTYPE_VHD: return "vhd";
-        case PHYSTYPE_AIO: return "aio";
-        case PHYSTYPE_FILE: return "file";
-        case PHYSTYPE_PHY: return "phy";
-        case PHYSTYPE_EMPTY: return "file";
-        default: return NULL;
+    switch (format) {
+        case DISK_FORMAT_QCOW: return "qcow";
+        case DISK_FORMAT_QCOW2: return "qcow2"; 
+        case DISK_FORMAT_VHD: return "vhd"; 
+        case DISK_FORMAT_RAW:
+        case DISK_FORMAT_EMPTY: return "aio"; 
+        default: return NULL; 
     }
 }
 
-char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_backend(libxl_disk_backend backend)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "tap";
-        case PHYSTYPE_QCOW2: return "tap";
-        case PHYSTYPE_VHD: return "tap";
-        case PHYSTYPE_AIO: return "tap";
-        /* let's pretend file is tap:aio */
-        case PHYSTYPE_FILE: return "tap";
-        case PHYSTYPE_EMPTY: return "tap";
-        case PHYSTYPE_PHY: return "phy";
+    switch (backend) {
+        case DISK_BACKEND_QDISK: return "qdisk";
+        case DISK_BACKEND_TAP: return "tap";
+        case DISK_BACKEND_PHY: return "phy";
         default: return NULL;
     }
 }

tools/libxl/libxl_dm.c +3 -3

diff -r 6c22ae0f6540 tools/libxl/libxl_dm.c
@@ -316,10 +316,10 @@ static char ** libxl_build_device_model_
         for (i; i < nb; i++) {
             if (disks[i].is_cdrom) {
                 flexarray_append(dm_args, "-cdrom");
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             } else {
-                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].virtpath));
-                flexarray_append(dm_args, libxl__strdup(gc, disks[i].physpath));
+                flexarray_append(dm_args, libxl__sprintf(gc, "-%s", disks[i].vdev));
+                flexarray_append(dm_args, libxl__strdup(gc, disks[i].pdev_path));
             }
             libxl_device_disk_destroy(&disks[i]);
         }

tools/libxl/libxl_internal.h +3 -3

diff -r 6c22ae0f6540 tools/libxl/libxl_internal.h
@@ -178,8 +178,8 @@ _hidden void libxl__userdata_destroyall(
 _hidden void libxl__userdata_destroyall(libxl_ctx *ctx, uint32_t domid);
 
 /* from xl_device */
-_hidden char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
-_hidden char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype);
+_hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
+_hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
 _hidden int libxl__device_disk_dev_number(char *virtpath);

@@ -306,7 +306,7 @@ _hidden int libxl__blktap_enabled(libxl_
  */
 _hidden const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype);
+                                 libxl_disk_format format);
 
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);

tools/libxl/libxl_noblktap2.c +1 -1

diff -r 6c22ae0f6540 tools/libxl/libxl_noblktap2.c
@@ -23,7 +23,7 @@ int libxl__blktap_enabled(libxl__gc *gc)
 
 const char *libxl__blktap_devpath(libxl__gc *gc,
                                  const char *disk,
-                                 libxl_disk_phystype phystype)
+                                 libxl_disk_format format)
 {
     return NULL;
 }

tools/libxl/libxl_utils.c +10 ~~-12~~

diff -r 6c22ae0f6540 tools/libxl/libxl_utils.c
@@ -275,15 +275,15 @@ out:
     return rc;
 }
 
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype)
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend)
 {
     char *p;
     int rc = 0;
 
     if (!strcmp(s, "phy")) {
-        *phystype = PHYSTYPE_PHY;
+        *backend = DISK_BACKEND_PHY;
     } else if (!strcmp(s, "file")) {
-        *phystype = PHYSTYPE_FILE;
+        *backend = DISK_BACKEND_TAP;
     } else if (!strcmp(s, "tap")) {
         p = strchr(s, ':');
         if (!p) {

@@ -291,14 +291,12 @@ int libxl_string_to_phystype(libxl_ctx *
             goto out;
         }
         p++;
-        if (!strcmp(p, "aio")) {
-            *phystype = PHYSTYPE_AIO;
-        } else if (!strcmp(p, "vhd")) {
-            *phystype = PHYSTYPE_VHD;
+        if (!strcmp(p, "vhd")) {
+            *backend = DISK_BACKEND_TAP;
         } else if (!strcmp(p, "qcow")) {
-            *phystype = PHYSTYPE_QCOW;
+            *backend = DISK_BACKEND_QDISK;
         } else if (!strcmp(p, "qcow2")) {
-            *phystype = PHYSTYPE_QCOW2;
+            *backend = DISK_BACKEND_QDISK;
         }
     }
 out:

@@ -553,10 +551,10 @@ int libxl_devid_to_device_disk(libxl_ctx
     disk->backend_domid = strtoul(val, NULL, 10);
     disk->domid = domid;
     be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/backend", diskpath));
-    disk->physpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
+    disk->pdev_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/params", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/type", be_path));
-    libxl_string_to_phystype(ctx, val, &(disk->phystype));
-    disk->virtpath = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
+    libxl_string_to_backend(ctx, val, &(disk->backend));
+    disk->vdev = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/dev", be_path));
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/removable", be_path));
     disk->unpluggable = !strcmp(val, "1");
     val = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/mode", be_path));

tools/libxl/libxl_utils.h +1 -1

diff -r 6c22ae0f6540 tools/libxl/libxl_utils.h
@@ -29,7 +29,7 @@ int libxl_get_stubdom_id(libxl_ctx *ctx,
 int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid);
 int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid);
 int libxl_create_logfile(libxl_ctx *ctx, char *name, char **full_name);
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype);
+int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend);
 
 int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
                              void **data_r, int *datalen_r);

tools/libxl/xl_cmdimpl.c +55 ~~-37~~

174 lines not on this page · raw message

tools/ocaml/libs/xl/xl_stubs.c +7 -6

diff -r 6c22ae0f6540 tools/ocaml/libs/xl/xl_stubs.c
@@ -192,12 +192,13 @@ static int device_disk_val(caml_gc *gc, 
 	CAMLparam1(v);
 
 	c_val->backend_domid = Int_val(Field(v, 0));
-	c_val->physpath = dup_String_val(gc, Field(v, 1));
-	c_val->phystype = (Int_val(Field(v, 2))) + PHYSTYPE_QCOW;
-	c_val->virtpath = dup_String_val(gc, Field(v, 3));
-	c_val->unpluggable = Bool_val(Field(v, 4));
-	c_val->readwrite = Bool_val(Field(v, 5));
-	c_val->is_cdrom = Bool_val(Field(v, 6));
+	c_val->pdev_path = dup_String_val(gc, Field(v, 1));
+	c_val->vdev = dup_String_val(gc, Field(v, 2));
+        c_val->backend = (Int_val(Field(v, 3)));
+        c_val->format = (Int_val(Field(v, 4)));
+	c_val->unpluggable = Bool_val(Field(v, 5));
+	c_val->readwrite = Bool_val(Field(v, 6));
+	c_val->is_cdrom = Bool_val(Field(v, 7));
 
 	CAMLreturn(0);
 }

tools/python/xen/lowlevel/xl/xl.c +11 -6

diff -r 6c22ae0f6540 tools/python/xen/lowlevel/xl/xl.c
@@ -779,12 +779,17 @@ PyMODINIT_FUNC initxl(void)
     _INT_CONST_LIBXL(m, CONSBACK_XENCONSOLED);
     _INT_CONST_LIBXL(m, CONSBACK_IOEMU);
 
-    _INT_CONST(m, PHYSTYPE_QCOW);
-    _INT_CONST(m, PHYSTYPE_QCOW2);
-    _INT_CONST(m, PHYSTYPE_VHD);
-    _INT_CONST(m, PHYSTYPE_AIO);
-    _INT_CONST(m, PHYSTYPE_FILE);
-    _INT_CONST(m, PHYSTYPE_PHY);
+    _INT_CONST(m, DISK_FORMAT_UNKNOWN);
+    _INT_CONST(m, DISK_FORMAT_QCOW);
+    _INT_CONST(m, DISK_FORMAT_QCOW2);
+    _INT_CONST(m, DISK_FORMAT_VHD);
+    _INT_CONST(m, DISK_FORMAT_RAW);
+    _INT_CONST(m, DISK_FORMAT_EMPTY);
+
+    _INT_CONST(m, DISK_BACKEND_UNKNOWN);
+    _INT_CONST(m, DISK_BACKEND_PHY);
+    _INT_CONST(m, DISK_BACKEND_TAP);
+    _INT_CONST(m, DISK_BACKEND_QDISK);
 
     _INT_CONST(m, NICTYPE_IOEMU);
     _INT_CONST(m, NICTYPE_VIF);

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

Ian Jackson

2011-02-15 19:22 UTC #15 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Kamala Narasimhan writes ("Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts"):

Quoted 1 line

Attached patch should address your concerns.

Thanks for this. However, there were other changes made between this most recent version and the previous version, besides the ones I mentioned in my comments and which you said you'd address.

When you post an updated version of a patch you should state separately
- what the patch does to the tree, including intended changes, motivation, etc., as you have done (for the commit message)
- how the patch differs from the previous version, if applicable

Can you explain what the changes you made and why you made them ? Ideally in general you should use revision control system tools to make sure that you know what they all are.

But, for your info here is the output of
diff -w --exclude=\*{~,.o,.d,.opic} -ru A B |grep -v '^Only in'

Most of this is formatting noise, which addresses my comments.

However, you have also:
* Initialised disk->format and disk->backend somewhere you previously didn't
* Recognised the "tap2" prefix in a place you previously didn't
* Changed the handling of the "aio" and "raw" prefixes

Normally patch such as this one, which is presented as being mature, ought not to need unexplained semantic changes at this late stage.

Ian.

u/iwj/work/xen-unstable-tools.hg/tools/libxl/libxl.c renamed from libxl/libxl.c +7 ~~-14~~

diff -w --exclude='*~' --exclude='*.o' --exclude='*.d' --exclude='*.opic' -ru tools/libxl/libxl.c /u/iwj/work/xen-unstable-tools.hg/tools/libxl/libxl.c
@@ -931,7 +931,7 @@
     device.kind = DEVICE_VBD;
 
     switch (disk->backend) {
-        case DISK_BACKEND_PHY: {
+        case DISK_BACKEND_PHY: 
             libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
             flexarray_append(back, "physical-device");
             flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));

@@ -941,9 +941,8 @@
 
             device.backend_kind = DEVICE_VBD;
             break;
-        }
         case DISK_BACKEND_TAP:
-        case DISK_BACKEND_QDISK: {
+        case DISK_BACKEND_QDISK: 
             if (disk->format == DISK_FORMAT_EMPTY)
                 break;
             if (libxl__blktap_enabled(&gc)) {

@@ -972,12 +971,11 @@
                           libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
 
             if (libxl__blktap_enabled(&gc) && 
-                 disk->format != DISK_BACKEND_QDISK)
+                 disk->backend != DISK_BACKEND_QDISK)
                 device.backend_kind = DEVICE_TAP;
             else
                 device.backend_kind = DEVICE_QDISK;
             break;
-        }
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
             rc = ERROR_INVAL;

@@ -1053,7 +1051,7 @@
     char *ret = NULL;
 
     switch (disk->backend) {
-        case DISK_BACKEND_PHY: {
+        case DISK_BACKEND_PHY: 
             if (disk->format != DISK_FORMAT_RAW) {
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "physical block device must"
                     " be raw");

@@ -1063,8 +1061,7 @@
                 disk->pdev_path);
             dev = disk->pdev_path;
             break;
-        }
-        case DISK_BACKEND_TAP: {
+        case DISK_BACKEND_TAP: 
             if (disk->format == DISK_FORMAT_VHD || disk->format == DISK_FORMAT_RAW)
             {
                 if (libxl__blktap_enabled(&gc))

@@ -1091,8 +1088,7 @@
                     "type: %d", disk->backend);
                 break;
             }
-        }
-        case DISK_BACKEND_QDISK: {
+        case DISK_BACKEND_QDISK: 
             if (disk->format != DISK_FORMAT_RAW) {
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally attach a qdisk "
                     "image if the format is not raw");

@@ -1102,15 +1098,12 @@
                 disk->pdev_path);
             dev = disk->pdev_path;
             break;
-
-        }
         case DISK_BACKEND_UNKNOWN:
-        default: {
+        default: 
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
                 "type: %d", disk->backend);
             break;
         }
-    }
 
     if (dev != NULL)
         ret = strdup(dev);

u/iwj/work/xen-unstable-tools.hg/tools/libxl/xl_cmdimpl.c renamed from libxl/xl_cmdimpl.c +13 -5

diff -w --exclude='*~' --exclude='*.o' --exclude='*.d' --exclude='*.opic' -ru tools/libxl/xl_cmdimpl.c /u/iwj/work/xen-unstable-tools.hg/tools/libxl/xl_cmdimpl.c
@@ -452,6 +452,8 @@
     char *p, *end, *tok;
 
     memset(disk, 0, sizeof(*disk));
+    disk->format = DISK_FORMAT_RAW;
+    disk->backend = DISK_BACKEND_TAP;
 
     for(tok = p = buf2, end = buf2 + strlen(buf2) + 1; p < end; p++) {
         switch(state){

@@ -466,7 +468,8 @@
                     state = DSTATE_PHYSPATH;
                     disk->format = DISK_FORMAT_RAW;
                     disk->backend = DISK_BACKEND_TAP;
-                }else if (!strcmp(tok, "tap")) {
+                }else if ((!strcmp(tok, "tap")) ||
+                          (!strcmp(tok, "tap2"))) {
                     state = DSTATE_TAP;
                 }else{
                     fprintf(stderr, "Unknown disk type: %s\n", tok);

@@ -485,9 +488,10 @@
             if ( *p == ':' ) {
                 *p = '\0';
                 if (!strcmp(tok, "aio")) {
-                    disk->format = DISK_FORMAT_RAW;
-                    disk->backend = DISK_BACKEND_TAP;
-                }else if (!strcmp(tok, "vhd")) {
+                    tok = p + 1;
+                    break;
+                }
+                if (!strcmp(tok, "vhd")) {
                     disk->format = DISK_FORMAT_VHD;
                     disk->backend = DISK_BACKEND_TAP;
                 }else if (!strcmp(tok, "qcow")) {

@@ -496,7 +500,11 @@
                 }else if (!strcmp(tok, "qcow2")) {
                     disk->format = DISK_FORMAT_QCOW2;
                     disk->backend = DISK_BACKEND_QDISK;
-                }else {
+                }else if (!strcmp(tok, "raw")) {
+                    disk->format = DISK_FORMAT_RAW;
+                    disk->backend = DISK_BACKEND_TAP;
+                }
+                else {
                     fprintf(stderr, "Unknown tapdisk type: %s\n", tok);
                     return 0;
                 }

Kamala Narasimhan

author

2011-02-15 19:38 UTC #16 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Quoted 20 lines · Ian Jackson

Kamala Narasimhan writes ("Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts"):
Quoted 1 line
Attached patch should address your concerns.
Thanks for this. However, there were other changes made between this most recent version and the previous version, besides the ones I mentioned in my comments and which you said you'd address.
When you post an updated version of a patch you should state separately
- what the patch does to the tree, including intended changes, motivation, etc., as you have done (for the commit message)
- how the patch differs from the previous version, if applicable
Can you explain what the changes you made and why you made them ? Ideally in general you should use revision control system tools to make sure that you know what they all are.
But, for your info here is the output of
diff -w --exclude=\*{~,.o,.d,.opic} -ru A B |grep -v '^Only in'

Quoted 8 lines

Most of this is formatting noise, which addresses my comments.
However, you have also:
* Initialised disk->format and disk->backend somewhere you previously didn't
* Recognised the "tap2" prefix in a place you previously didn't
* Changed the handling of the "aio" and "raw" prefixes

Yes, they are all changes made based on input from Stefano and in an attempt to be in sync with the documentation. Ideally we wouldn't need these changes if we were to go with the rest of the patches but since we made a decision not to go with the rest before 4.2, we decided to do some minimal changes to sync with the doc.

Quoted 2 lines

Normally patch such as this one, which is presented as being mature, ought not to need unexplained semantic changes at this late stage.

I think we are beating to death an already dull set of changes which makes it all the more uninteresting to me :) But I digress...

Just to be clear - Would you accept the patch I sent earlier if I extended the description to include the fact that I initialize disk format, backend to default values and changed how we handle "aio" prefix plus recognize "raw" and "tap2" prefixes now?

Kamala

Ian Jackson

2011-02-15 19:41 UTC #17 lore raw

Re: [PATCH 2/5] Xl interface change plus changes to code it impacts

Kamala Narasimhan writes ("Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts"):

Quoted 5 lines

Yes, they are all changes made based on input from Stefano and in an attempt to be in sync with the documentation. Ideally we wouldn't need these changes if we were to go with the rest of the patches but since we made a decision not to go with the rest before 4.2, we decided to do some minimal changes to sync with the doc.

Right, that makes sense.

Quoted 4 lines

Just to be clear - Would you accept the patch I sent earlier if I extended the description to include the fact that I initialize disk format, backend to default values and changed how we handle "aio" prefix plus recognize "raw" and "tap2" prefixes now?

Thanks, I just wanted an explanation. I will apply the series now.

Ian.

Kamala Narasimhan

author

2011-02-09 18:26 UTC #18 lore raw

In reply to Kamala Narasimhan

Quoted 8 lines · Kamala Narasimhan

Refactor xl disk configuration handling.
Patch 1/5 - "Xl Disk Configuration Option" documentation Patch 2/5 - Xl interface change plus changes to code it impacts Patch 3/5 - Xl disk configuration parsing changes Patch 4/5 - Xl block-attach refactoring to reuse xl parsing code Patch 5/5 - More disk validation checks

Quoted 6 lines

Note: I am nervous about this patch series as the changes are huge and I am worried about regressions especially this close to 4.1. I can try and fix the regressions fast but only after they are found and reported. So, I would encourage folks to run their standard config files that they knew worked before against the latest changes and report any/all regressions. Thanks!

Note: Upon further discussion a decision was made to go with patches 1 & 2 for 4.1 and the rest will be reposted for 4.2 and would include relevant review comments.

Kamala