On Sun, 30 Jan 2011, Kamala Narasimhan wrote:
> 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,
I agree that libxl_disk_phystype expresses both the format and the
backend type in a single confusing way, so there should be two enums:
one for the format (libxl_disk_format) and one for the backend type
(libxl_disk_pdev_type).
However I don't think libxl_disk_impl_type should be exposed at all, it
should be up to libxl to decide whether AIO should be enabled or not. It
might be useful to let the user disable the PV interface for a
particular disk (that is what ioemu stands for), but it is too late for
4.1, let's just ignore ioemu for the moment.
The backend types should be BLKBACK, TAPDISK2, and QEMU; TAPDISK refers
to blktap1 that is not supported by libxl. However libxl uses "tap:" as
backend string corresponding to TAPDISK2, I understand that might be
confusing but I wouldn't change it now.
Also it might be useful to retain the EMPTY format among the various
libxl_disk_format's, it should reduce the overall amount of changes.
> 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;
it would be nice if all the renaming was done in a separate patch so
that the real changes are easier to read
> @@ -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;
It looks like EMPTY, QCOW and QCOW2 disappeared.
> @@ -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;
> + }
> + }
I wouldn't take into account "aio" here.
> @@ -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),
we don't need impl_type
> 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;
> }
> }
I think we need to return the format string here, like "vhd", otherwise
tapdisk2 wouldn't work correctly. This function should take a
libxl_disk_format as an argument now.
>
> -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;
> }
Drop tap2.
> 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,
do we really need to change the parsing function that much? I
understand there are significant changes but this is a total rewrite.
I am concerned about all the bugs we might find later after the
release...
> @@ -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;
I would completely ignore "aio:" here.
I would also ignore "ioemu:" the same way.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home