[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen-devel][RFC] xl disk configuration handling


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxx>
  • Date: Sun, 30 Jan 2011 12:46:56 -0500
  • Delivery-date: Sun, 30 Jan 2011 09:47:38 -0800
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:subject :content-type; b=MtBj3ABmKTOUkhDSB/e4nNGE+Lti9feR3F1XFmhn0pi01a3SpNrNuC2LovHC3Oap4k +s8ML6GaaBPa3tUpwqHow9Jb9y06/VvoL1R0336by1BJFmtEObN6Qe1dEtpQfYS8fR7e eqUQM3w27gqY0qnDCzA0UqpIWC/OjDnqjWEQ4=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>

Based on the discussion we have had so far on xl disk configuration handling, I 
have made a fair amount of implementation changes.  As most of you might have 
suspected the changes are vast and has ripple effect all over the place due to 
interface changes.  But I think I got the go ahead from the maintainers to make 
these changes for 4.1 and I also believe we are better off making these changes 
for 4.1.  The plan is to send the patches in following order -

1) Patch 1/3 - Interface changes, parsing changes, frontend/endback specific 
disk parameters setup and other ripple effects caused by interface changes.  
Ideally, I would like to keep this patch to just the interface and parsing 
changes but by doing so I might break the rest of the code from even compiling. 
 So, this change is going to be huge.
2) Patch 2/3 - Disk validation changes
3) Patch 3/3 - "xl Disk Configuration option" documentation

Let me know if you would prefer a different order for convenience or for some 
other reason.

I have attached the changes I have made so far for you to get an idea of the 
level of changes coming.  Note that this is NOT a patch submission.

Here are the parts of the patch I would like to get reviewed -

1) Interface changes - libxl.idl, libxl.h
2) Parsing changes - xl_cmdimpl.c.  I have basically divided each disk 
configuration information into three chunks - pdev, vdev and attribute and 
parsed it based on our discussion/documentation.  In specific, 
parse_disk_config, parse_disk_attrib, parse_disk_vdev_info and 
parse_disk_pdev_info implementation is the core of parsing code and that is 
what I would like to get reviewed to be sure I am not way off from what you 
would be willing to accept.

What not to review in this patch -

Anything other than interface and parsing changes, I would request that you 
ignore for now.  I am sending it only for the following reasons -

1) Since we are very close to 4.1, I would like you to get an idea of all the 
places this change touches.
2) I would want the patch to build!

Other than that, there are parts of the patch outside interface/parsing changes 
that might be incomplete or even incorrect!  I will be sending out follow up 
emails to better understand certain parts of disk hotplug code and disk 
frontend/backend parameters setup code and will make changes based on the input 
I get and the patch as a whole should be ready for review after that.  If you 
would rather I resend just the interface/parsing changes for review in that 
case, let me know.  Thanks.

Kamala




diff -r 52e928af3637 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/libxl.c       Sun Jan 30 11:47:22 2011 -0500
@@ -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_path)) < 0)
             goto out;
         if (asprintf(&(waiter[i].token), "%d", LIBXL_EVENT_DISK_EJECT) < 0)
             goto out;
@@ -668,10 +668,9 @@ 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("");
     /* 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_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, 
"%s/dev", backend));
     disk->unpluggable = 1;
     disk->readwrite = 0;
     disk->is_cdrom = 1;
@@ -855,18 +854,19 @@ skip_autopass:
 
 
/******************************************************************************/
 
-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_pdev_type pdev_type, int is_cdrom)
 {
     struct stat stat_buf;
 
-    if ( (file_name[0] == '\0') && (disk_type == PHYSTYPE_EMPTY) )
+    if ( (file_name[0] == '\0') && (is_cdrom == 1) )
         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 ( pdev_type == DISK_PDEV_TYPE_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);
@@ -890,7 +890,7 @@ 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->pdev_type, 
disk->is_cdrom);
     if (rc)
         return rc;
 
@@ -905,11 +905,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_backend_type_of_phystype(disk->pdev_type);
+    devid = libxl__device_disk_dev_number(disk->vdev_path);
     if (devid==-1) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->virtpath);
+               " virtual disk identifier %s", disk->vdev_path);
         rc = ERROR_INVAL;
         goto out_free;
     }
@@ -920,37 +920,31 @@ int libxl_device_disk_add(libxl_ctx *ctx
     device.domid = disk->domid;
     device.kind = DEVICE_VBD;
 
-    switch (disk->phystype) {
-        case PHYSTYPE_PHY: {
+    switch (disk->pdev_type) {
+        case DISK_PDEV_TYPE_PHY: {
 
-            libxl__device_physdisk_major_minor(disk->physpath, &major, &minor);
+            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_PDEV_TYPE_FILE:
+        case DISK_PDEV_TYPE_TAP:
+        case DISK_PDEV_TYPE_TAP2:
             if (libxl__blktap_enabled(&gc)) {
                 const char *dev = libxl__blktap_devpath(&gc,
-                                               disk->physpath, disk->phystype);
+                                               disk->pdev_path, 
disk->pdev_type);
                 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_phystype(disk->pdev_type), disk->pdev_path));
                 flexarray_append(back, "params");
                 flexarray_append(back, libxl__strdup(&gc, dev));
                 backend_type = "phy";
@@ -963,7 +957,7 @@ 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_phystype(disk->pdev_type), disk->pdev_path));
 
             if (libxl__blktap_enabled(&gc))
                 device.backend_kind = DEVICE_TAP;
@@ -972,7 +966,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
             break;
 
         default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical 
type: %d\n", disk->phystype);
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical 
type: %d\n", disk->pdev_type);
             rc = ERROR_INVAL;
             goto out_free;
     }
@@ -988,7 +982,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_path);
     flexarray_append(back, "type");
     flexarray_append(back, backend_type);
     flexarray_append(back, "mode");
@@ -1028,11 +1022,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_path);
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
     device.backend_kind     = 
-        (disk->phystype == PHYSTYPE_PHY) ? DEVICE_VBD : DEVICE_TAP;
+        (disk->pdev_type == DISK_PDEV_TYPE_PHY) ? DEVICE_VBD : DEVICE_TAP;
     device.domid            = disk->domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;
@@ -1044,32 +1038,33 @@ 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;
+    int phystype = disk->pdev_type;
     switch (phystype) {
-        case PHYSTYPE_PHY: {
-            fprintf(stderr, "attaching PHY disk %s to domain 0\n", 
disk->physpath);
-            dev = disk->physpath;
+        case DISK_PDEV_TYPE_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_PDEV_TYPE_FILE:
+        case DISK_PDEV_TYPE_TAP:
+        case DISK_PDEV_TYPE_TAP2:
+            if ( disk->impl_type == DISK_IMPL_TYPE_AIO ) {
+                if (!libxl__blktap_enabled(&gc)) {
+                    dev = disk->pdev_path;
+                    break;
+                }
+            }
+            if ( disk->format == DISK_FORMAT_VHD ) {
+                if (libxl__blktap_enabled(&gc))
+                    dev = libxl__blktap_devpath(&gc, disk->pdev_path, 
phystype);
+                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;
             }
-        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");
-            break;
-
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk physical 
type: %d\n", phystype);
             break;
@@ -1082,7 +1077,7 @@ char * libxl_device_disk_local_attach(li
 
 int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk)
 {
-    /* Nothing to do for PHYSTYPE_PHY. */
+    /* Nothing to do for DISK_PDEV_TYPE_PHY. */
 
     /*
      * For other device types assume that the blktap2 process is
@@ -1669,13 +1664,13 @@ 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_phystype(ctx, libxl__xs_read(&gc, XBT_NULL, 
libxl__sprintf(&gc, "%s/%s/type", be_path, *dir)), &(pdisk->pdev_type));
+            pdisk->vdev_path = 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;
@@ -1710,7 +1705,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_path);
 
     /* tap devices entries in xenstore are written as vbd devices. */
     diskpath = libxl__sprintf(&gc, "%s/device/vbd/%d", dompath, 
diskinfo->devid);
@@ -1744,13 +1739,11 @@ 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("");
     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_path, disks[i].vdev_path))
             /* found */
             break;
     }
diff -r 52e928af3637 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h       Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/libxl.h       Sun Jan 30 11:47:22 2011 -0500
@@ -172,14 +172,27 @@ 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,
+} libxl_disk_format;
+
+typedef enum {
+    DISK_IMPL_TYPE_UNKNOWN = 0,
+    DISK_IMPL_TYPE_AIO,
+    DISK_IMPL_TYPE_TAPDISK,
+    DISK_IMPL_TYPE_IOEMU,
+} libxl_disk_impl_type;
+
+typedef enum {
+    DISK_PDEV_TYPE_UNKNOWN = 0,
+    DISK_PDEV_TYPE_PHY,
+    DISK_PDEV_TYPE_FILE,
+    DISK_PDEV_TYPE_TAP,
+    DISK_PDEV_TYPE_TAP2,
+} libxl_disk_pdev_type;
 
 typedef enum {
     NICTYPE_IOEMU = 1,
diff -r 52e928af3637 tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl     Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/libxl.idl     Sun Jan 30 11:47:22 2011 -0500
@@ -11,7 +11,9 @@ 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_impl_type = Number("disk_impl_type", namespace="libxl_")
+libxl_disk_pdev_type = Number("disk_pdev_type", 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 +205,11 @@ 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_path", string),
+    ("pdev_type", libxl_disk_pdev_type),
+    ("impl_type", libxl_disk_impl_type),
+    ("format", libxl_disk_format),
     ("unpluggable", integer),
     ("readwrite", integer),
     ("is_cdrom", integer),
diff -r 52e928af3637 tools/libxl/libxl_blktap2.c
--- a/tools/libxl/libxl_blktap2.c       Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/libxl_blktap2.c       Sun Jan 30 11:47:22 2011 -0500
@@ -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_pdev_type pdev_type)
 {
     const char *type;
     char *params, *devname = NULL;
     int minor, err;
 
-    type = libxl__device_disk_string_of_phystype(phystype);
+    type = libxl__device_disk_string_of_phystype(pdev_type);
     minor = tap_ctl_find_minor(type, disk);
     if (minor >= 0) {
         devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", minor);
diff -r 52e928af3637 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c        Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/libxl_device.c        Sun Jan 30 11:47:22 2011 -0500
@@ -121,31 +121,23 @@ out:
     return rc;
 }
 
-char *libxl__device_disk_string_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_string_of_phystype(libxl_disk_pdev_type pdev_type)
 {
-    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";
+    switch (pdev_type) {
+        case DISK_PDEV_TYPE_FILE: return "file";
+        case DISK_PDEV_TYPE_PHY: return "phy";
+        case DISK_PDEV_TYPE_TAP: return "tap";
+        case DISK_PDEV_TYPE_TAP2: return "tap2";
         default: return NULL;
     }
 }
 
-char *libxl__device_disk_backend_type_of_phystype(libxl_disk_phystype phystype)
+char *libxl__device_disk_backend_type_of_phystype(libxl_disk_pdev_type 
pdev_type)
 {
-    switch (phystype) {
-        case PHYSTYPE_QCOW: return "tap";
-        case PHYSTYPE_QCOW2: return "tap";
-        case PHYSTYPE_VHD: return "tap";
-        case PHYSTYPE_AIO: return "tap";
+    switch (pdev_type) {
         /* let's pretend file is tap:aio */
-        case PHYSTYPE_FILE: return "tap";
-        case PHYSTYPE_EMPTY: return "tap";
-        case PHYSTYPE_PHY: return "phy";
+        case DISK_PDEV_TYPE_FILE: return "tap";
+        case DISK_PDEV_TYPE_PHY: return "phy";
         default: return NULL;
     }
 }
diff -r 52e928af3637 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c    Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/libxl_dm.c    Sun Jan 30 11:47:22 2011 -0500
@@ -303,10 +303,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_path));
+                flexarray_append(dm_args, libxl__strdup(gc, 
disks[i].pdev_path));
             }
             libxl_device_disk_destroy(&disks[i]);
         }
diff -r 52e928af3637 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h      Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/libxl_internal.h      Sun Jan 30 11:47:22 2011 -0500
@@ -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_backend_type_of_phystype(libxl_disk_pdev_type 
pdev_type);
+_hidden char *libxl__device_disk_string_of_phystype(libxl_disk_pdev_type 
pdev_type);
 
 _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_pdev_type pdev_type);
 
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);
 
diff -r 52e928af3637 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/libxl_utils.c Sun Jan 30 11:47:22 2011 -0500
@@ -275,34 +275,20 @@ out:
     return rc;
 }
 
-int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_phystype 
*phystype)
+int libxl_string_to_phystype(libxl_ctx *ctx, char *s, libxl_disk_pdev_type 
*pdev_type)
 {
-    char *p;
-    int rc = 0;
+    *pdev_type = DISK_PDEV_TYPE_UNKNOWN;
 
-    if (!strcmp(s, "phy")) {
-        *phystype = PHYSTYPE_PHY;
-    } else if (!strcmp(s, "file")) {
-        *phystype = PHYSTYPE_FILE;
-    } else if (!strcmp(s, "tap")) {
-        p = strchr(s, ':');
-        if (!p) {
-            rc = ERROR_INVAL;
-            goto out;
-        }
-        p++;
-        if (!strcmp(p, "aio")) {
-            *phystype = PHYSTYPE_AIO;
-        } else if (!strcmp(p, "vhd")) {
-            *phystype = PHYSTYPE_VHD;
-        } else if (!strcmp(p, "qcow")) {
-            *phystype = PHYSTYPE_QCOW;
-        } else if (!strcmp(p, "qcow2")) {
-            *phystype = PHYSTYPE_QCOW2;
-        }
-    }
-out:
-    return rc;
+    if ( !strncmp(s, "phy", sizeof("phy")-1) )
+        *pdev_type = DISK_PDEV_TYPE_PHY;
+    else if ( !strncmp(s, "file", sizeof("file")-1) )
+        *pdev_type = DISK_PDEV_TYPE_FILE;
+    else if ( !strncmp(s, "tap", sizeof("tap")-1) )
+        *pdev_type = DISK_PDEV_TYPE_TAP;
+    else if ( !strncmp(s, "tap2", sizeof("tap2")-1) )
+        *pdev_type = DISK_PDEV_TYPE_TAP2;
+
+    return 0;
 }
 
 int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
@@ -553,10 +539,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_phystype(ctx, val, &(disk->pdev_type));
+    disk->vdev_path = 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));
diff -r 52e928af3637 tools/libxl/libxl_utils.h
--- a/tools/libxl/libxl_utils.h Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/libxl_utils.h Sun Jan 30 11:47:22 2011 -0500
@@ -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_phystype(libxl_ctx *ctx, char *s, libxl_disk_pdev_type 
*pdev_type);
 
 int libxl_read_file_contents(libxl_ctx *ctx, const char *filename,
                              void **data_r, int *datalen_r);
diff -r 52e928af3637 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/xl_cmdimpl.c  Sun Jan 30 11:47:22 2011 -0500
@@ -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].pdev_type);
+        printf("\t\t\t(virtpath %s)\n", d_config->disks[i].vdev_path);
         printf("\t\t\t(unpluggable %d)\n", d_config->disks[i].unpluggable);
         printf("\t\t\t(readwrite %d)\n", d_config->disks[i].readwrite);
         printf("\t\t\t(is_cdrom %d)\n", d_config->disks[i].is_cdrom);
@@ -438,137 +438,164 @@ static int parse_action_on_shutdown(cons
     return 0;
 }
 
-#define DSTATE_INITIAL   0
-#define DSTATE_TAP       1
-#define DSTATE_PHYSPATH  2
-#define DSTATE_VIRTPATH  3
-#define DSTATE_VIRTTYPE  4
-#define DSTATE_RW        5
-#define DSTATE_TERMINAL  6
-
+static int parse_disk_attrib(libxl_device_disk *disk, char *buf2)
+{
+    char *attrib = strrchr(buf2, ',');
+    if ( !attrib ) {
+        LOG("Invalid disk configuration option %s.  Refer to the xl disk "
+            "configuration document for further information", buf2);
+        return ERROR_INVAL;
+    }
+
+    *attrib = '\0';
+    attrib++;
+    if ( attrib[0] == 'w' )
+        disk->readwrite = 1;
+    else if ( attrib[0] != 'r' ) {
+        LOG("Required access rights attribute is missing or incorrect in "
+            "disk configuration option %s", buf2);
+        return ERROR_INVAL;
+    }
+
+    return 0;
+}
+
+static int parse_disk_vdev_info(libxl_device_disk *disk, char *buf2)
+{
+    char *vdev_info, *vdev, *type;
+
+    vdev_info = strrchr(buf2, ',');
+    if ( !vdev_info ) {
+        LOG("Required vdev info missing in disk configuration option");
+        return ERROR_INVAL;
+    }
+
+    *vdev_info = '\0';
+    vdev_info++;
+
+    type = strchr(vdev_info, ':');
+    if ( type ) {
+        *type = '\0';
+        type++;
+        if ( !strncmp(type, "cdrom", sizeof("cdrom")-1) ) {
+            disk->is_cdrom = 1;
+            disk->unpluggable = 1;
+        }
+        else {
+            LOG("Invalid vdev type %s specified in disk configuration option",
+                type);
+            return ERROR_INVAL;
+        }
+    }
+
+    vdev = vdev_info;
+    if ( vdev[0] == '\0' ) {
+        LOG("Mandatory attribute vdev not specified");
+        return ERROR_INVAL;
+    }
+
+    disk->vdev_path = strdup(vdev);
+    return 0;    
+}
+
+static int parse_disk_pdev_info(libxl_device_disk *disk, char *buf2)
+{
+    char *pdev_info, *pdev_type, *pdev_impl, *pdev_format, *pdev_path;
+
+    pdev_info = buf2;
+    if ( pdev_info[0] == '\0' && !disk->is_cdrom ) {
+        /* pdev can be empty string only for cdrom drive with no media 
inserted */
+        LOG("Required vdev info missing in disk configuration option");
+        return ERROR_INVAL;
+    }
+
+    pdev_path = strrchr(pdev_info, ':');
+    if ( !pdev_path ) {
+        disk->pdev_path = strdup(pdev_info);
+        return 0;
+    }
+
+    *pdev_path = '\0';
+    disk->pdev_path = strdup(++pdev_path);
+
+    pdev_format = strrchr(pdev_info, ':');
+    if ( !pdev_format )
+        pdev_format = pdev_info;
+    else {
+        pdev_format = '\0';
+        pdev_format++;
+    }
+
+    if ( !strncmp(pdev_format, "raw", sizeof("raw")-1) ) 
+        disk->format = DISK_FORMAT_RAW;
+    else if ( !strncmp(pdev_format, "qcow", sizeof("qcow")-1) )
+        disk->format = DISK_FORMAT_QCOW;
+    else if ( !strncmp(pdev_format, "qcow2", sizeof("qcow2")-1) )
+        disk->format = DISK_FORMAT_QCOW2;
+    else if ( !strncmp(pdev_format, "vhd", sizeof("vhd")-1) )
+        disk->format = DISK_FORMAT_VHD;
+
+    if ( disk->format == DISK_FORMAT_UNKNOWN ) 
+        pdev_impl = pdev_format;
+    else { 
+        pdev_impl = strrchr(pdev_info, ':');
+        if ( !pdev_impl )
+            return 0;
+        *pdev_impl = '\0';
+        pdev_impl++;
+    }
+ 
+    if ( !strncmp(pdev_impl, "aio", sizeof("aio")-1) )
+        disk->impl_type = DISK_IMPL_TYPE_AIO;
+    else if ( !strncmp(pdev_impl, "tapdisk", sizeof("tapdisk")-1) )
+        disk->impl_type = DISK_IMPL_TYPE_TAPDISK;
+    else if ( !strncmp(pdev_impl, "ioemu", sizeof("ioemu")-1) )
+        disk->impl_type = DISK_IMPL_TYPE_IOEMU;
+
+    if ( disk->impl_type == DISK_IMPL_TYPE_UNKNOWN )
+        pdev_type = pdev_impl;
+    else {
+        pdev_type = pdev_info;
+        if ( pdev_type[0] == '\0' )
+            return 0;
+    }
+
+    if ( !strncmp(pdev_type, "phy", sizeof("phy")-1) )
+        disk->pdev_type = DISK_PDEV_TYPE_PHY;
+    else if ( !strncmp(pdev_type, "file", sizeof("file")-1) )
+        disk->pdev_type = DISK_PDEV_TYPE_FILE;
+    else if ( !strncmp(pdev_type, "tap", sizeof("tap")-1) )
+        disk->pdev_type = DISK_PDEV_TYPE_TAP;
+    else if ( !strncmp(pdev_type, "tap2", sizeof("tap2")-1) )
+        disk->pdev_type = DISK_PDEV_TYPE_TAP2;
+
+    return 0; 
+}
+
+/*
+ * Note:  Following code calls methods each of which will parse
+ *        specific chunks of the disk configuration option, the specific
+ *        chunks being attribute, vdev and pdev. As with any parsing code
+ *        it makes assumption about the order in which specific chunks appear
+ *        in the disk configuration option string.  So, please use
+ *        care while modifying the code below, esp. when it comes to 
+ *        the order of calls.
+ */  
 static int parse_disk_config(libxl_device_disk *disk, char *buf2)
 {
-    int state = DSTATE_INITIAL;
-    char *p, *end, *tok;
+    int ret_val;
 
     memset(disk, 0, sizeof(*disk));
 
-    for(tok = p = buf2, end = buf2 + strlen(buf2) + 1; p < end; p++) {
-        switch(state){
-        case DSTATE_INITIAL:
-            if ( *p == ':' ) {
-                *p = '\0';
-                if ( !strcmp(tok, "phy") ) {
-                    state = DSTATE_PHYSPATH;
-                    disk->phystype = PHYSTYPE_PHY;
-                }else if ( !strcmp(tok, "file") ) {
-                    state = DSTATE_PHYSPATH;
-                    disk->phystype = PHYSTYPE_FILE;
-                }else if ( !strcmp(tok, "tap") ) {
-                    state = DSTATE_TAP;
-                }else{
-                    fprintf(stderr, "Unknown disk type: %s\n", tok);
-                    return 0;
-                }
-                tok = p + 1;
-            } else if (*p == ',') {
-                state = DSTATE_VIRTPATH;
-                disk->phystype = PHYSTYPE_EMPTY;
-                disk->physpath = strdup("");
-                tok = p + 1;
-            }
-            break;
-        case DSTATE_TAP:
-            if ( *p == ':' ) {
-                *p = '\0';
-                if ( !strcmp(tok, "aio") ) {
-                    disk->phystype = PHYSTYPE_AIO;
-                }else if ( !strcmp(tok, "vhd") ) {
-                    disk->phystype = PHYSTYPE_VHD;
-                }else if ( !strcmp(tok, "qcow") ) {
-                    disk->phystype = PHYSTYPE_QCOW;
-                }else if ( !strcmp(tok, "qcow2") ) {
-                    disk->phystype = PHYSTYPE_QCOW2;
-                }else {
-                    fprintf(stderr, "Unknown tapdisk type: %s\n", tok);
-                    return 0;
-                }
-
-                tok = p + 1;
-                state = DSTATE_PHYSPATH;
-            }
-            break;
-        case DSTATE_PHYSPATH:
-            if ( *p == ',' ) {
-                int ioemu_len;
-
-                *p = '\0';
-                disk->physpath = (*tok) ? strdup(tok) : NULL;
-                tok = p + 1;
-
-                /* hack for ioemu disk spec */
-                ioemu_len = strlen("ioemu:");
-                state = DSTATE_VIRTPATH;
-                if ( tok + ioemu_len < end &&
-                    !strncmp(tok, "ioemu:", ioemu_len)) {
-                    tok += ioemu_len;
-                    p += ioemu_len;
-                }
-            }
-            break;
-        case DSTATE_VIRTPATH:
-            if ( *p == ',' || *p == ':' || *p == '\0' ) {
-                switch(*p) {
-                case ':':
-                    state = DSTATE_VIRTTYPE;
-                    break;
-                case ',':
-                    state = DSTATE_RW;
-                    break;
-                case '\0':
-                    state = DSTATE_TERMINAL;
-                    break;
-                }
-                if ( tok == p )
-                    goto out;
-                *p = '\0';
-                disk->virtpath = (*tok) ? strdup(tok) : NULL;
-                tok = p + 1;
-            }
-            break;
-        case DSTATE_VIRTTYPE:
-            if ( *p == ',' || *p == '\0' ) {
-                *p = '\0';
-                if ( !strcmp(tok, "cdrom") ) {
-                    disk->is_cdrom = 1;
-                    disk->unpluggable = 1;
-                }else{
-                    fprintf(stderr, "Unknown virtual disk type: %s\n", tok);
-                    return 0;
-                }
-                tok = p + 1;
-                state = (*p == ',') ? DSTATE_RW : DSTATE_TERMINAL;
-            }
-            break;
-        case DSTATE_RW:
-            if ( *p == '\0' ) {
-                disk->readwrite = (tok[0] == 'w');
-                tok = p + 1;
-                state = DSTATE_TERMINAL;
-            }
-            break;
-        case DSTATE_TERMINAL:
-            goto out;
-        }
-    }
-
-out:
-    if ( tok != p || state != DSTATE_TERMINAL ) {
-        fprintf(stderr, "parse error in disk config near '%s'\n", tok);
-        return 0;
-    }
-
-    return 1;
+    ret_val = parse_disk_attrib(disk, buf2);
+    if ( ret_val )
+        return ret_val;
+
+    ret_val = parse_disk_vdev_info(disk, buf2);
+    if ( ret_val )
+        return ret_val; 
+
+    return parse_disk_pdev_info(disk, buf2);
 }
 
 static void parse_config_data(const char *configfile_filename_report,
@@ -762,7 +789,7 @@ static void parse_config_data(const char
 
             d_config->disks = (libxl_device_disk *) realloc(d_config->disks, 
sizeof (libxl_device_disk) * (d_config->num_disks + 1));
             disk = d_config->disks + d_config->num_disks;
-            if ( !parse_disk_config(disk, buf2) ) {
+            if ( parse_disk_config(disk, buf2) ) {
                 exit(1);
             }
 
@@ -1838,25 +1865,24 @@ static void cd_insert(const char *dom, c
         p = strchr(phys, ':');
         if (!p) {
             fprintf(stderr, "No type specified, ");
-            disk.physpath = phys;
+            disk.pdev_path = phys;
             if (!strncmp(phys, "/dev", 4)) {
                 fprintf(stderr, "assuming phy:\n");
-                disk.phystype = PHYSTYPE_PHY;
+                disk.pdev_type = DISK_PDEV_TYPE_PHY;
             } else {
                 fprintf(stderr, "assuming file:\n");
-                disk.phystype = PHYSTYPE_FILE;
+                disk.pdev_type = DISK_PDEV_TYPE_FILE;
             }
         } else {
             *p = '\0';
             p++;
-            disk.physpath = p;
-            libxl_string_to_phystype(&ctx, phys, &disk.phystype);
-        }
-    } else {
-            disk.physpath = strdup("");
-            disk.phystype = PHYSTYPE_EMPTY;
-    }
-    disk.virtpath = (char*)virtdev;
+            disk.pdev_path = p;
+            libxl_string_to_phystype(&ctx, phys, &disk.pdev_type);
+        }
+    } else 
+        disk.pdev_path = strdup("");
+
+    disk.vdev_path = (char*)virtdev;
     disk.unpluggable = 1;
     disk.readwrite = 0;
     disk.is_cdrom = 1;
@@ -4375,19 +4401,19 @@ int main_blockattach(int argc, char **ar
 
     tok = strtok(argv[optind+1], ":");
     if (!strcmp(tok, "phy")) {
-        disk.phystype = PHYSTYPE_PHY;
+        disk.pdev_type = DISK_PDEV_TYPE_PHY;
     } else if (!strcmp(tok, "file")) {
-        disk.phystype = PHYSTYPE_FILE;
+        disk.pdev_type = DISK_PDEV_TYPE_FILE;
     } else if (!strcmp(tok, "tap")) {
         tok = strtok(NULL, ":");
         if (!strcmp(tok, "aio")) {
-            disk.phystype = PHYSTYPE_AIO;
+            disk.impl_type = DISK_IMPL_TYPE_AIO;
         } else if (!strcmp(tok, "vhd")) {
-            disk.phystype = PHYSTYPE_VHD;
+            disk.format = DISK_FORMAT_VHD;
         } else if (!strcmp(tok, "qcow")) {
-            disk.phystype = PHYSTYPE_QCOW;
+            disk.format = DISK_FORMAT_QCOW;
         } else if (!strcmp(tok, "qcow2")) {
-            disk.phystype = PHYSTYPE_QCOW2;
+            disk.format = DISK_FORMAT_QCOW2;
         } else {
             fprintf(stderr, "Error: `%s' is not a valid disk image.\n", tok);
             return 1;
@@ -4396,12 +4422,12 @@ int main_blockattach(int argc, char **ar
         fprintf(stderr, "Error: `%s' is not a valid block device.\n", tok);
         return 1;
     }
-    disk.physpath = strtok(NULL, "\0");
-    if (!disk.physpath) {
+    disk.pdev_path = strtok(NULL, "\0");
+    if (!disk.pdev_path) {
         fprintf(stderr, "Error: missing path to disk image.\n");
         return 1;
     }
-    disk.virtpath = argv[optind+2];
+    disk.vdev_path = argv[optind+2];
     disk.unpluggable = 1;
     disk.readwrite = ((argc-optind <= 3) || (argv[optind+3][0] == 'w'));
 
diff -r 52e928af3637 tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c Sun Jan 30 11:47:22 2011 -0500
@@ -779,12 +779,12 @@ 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_QCOW);
+    _INT_CONST(m, DISK_FORMAT_QCOW2);
+    _INT_CONST(m, DISK_FORMAT_VHD);
+    _INT_CONST(m, DISK_IMPL_TYPE_AIO);
+    _INT_CONST(m, DISK_PDEV_TYPE_FILE);
+    _INT_CONST(m, DISK_PDEV_TYPE_PHY);
 
     _INT_CONST(m, NICTYPE_IOEMU);
     _INT_CONST(m, NICTYPE_VIF);
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.