* [PATCH 3/5] xl disk configuration parsing changes
@ 2011-02-07 21:24 Kamala Narasimhan
2011-02-08 16:43 ` Stefano Stabellini
0 siblings, 1 reply; 3+ messages in thread
From: Kamala Narasimhan @ 2011-02-07 21:24 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
[-- Attachment #1: Type: text/plain, Size: 208 bytes --]
Attached is a patch to refactor xl disk configuration option parsing. Please let me know if there are further comments/issues to fix.
Signed-off-by: Kamala Narasimhan <kamala.narasimhan@citrix.com>
Kamala
[-- Attachment #2: xl-disk-parsing-changes.diff --]
[-- Type: text/x-diff, Size: 11077 bytes --]
diff -u -r libxl-interface-changes/xl_cmdimpl.c libxl/xl_cmdimpl.c
--- libxl-interface-changes/xl_cmdimpl.c 2011-02-07 12:03:37.000000000 -0500
+++ libxl/xl_cmdimpl.c 2011-02-07 14:58:39.000000000 -0500
@@ -438,143 +438,184 @@
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
+#define DSTATE_INITIAL 0
+#define DSTATE_ATTRIB_PARSED 1
+#define DSTATE_VDEV_PARSED 2
+static int parse_disk_attrib(libxl_device_disk *disk, char *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", attrib);
+ return ERROR_INVAL;
+ }
+
+ return 0;
+}
+
+static int parse_disk_vdev_info(libxl_device_disk *disk, char *vdev_info)
+{
+ char *vdev, *type;
+
+ 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 = strdup(vdev);
+ return 0;
+}
+
+static int parse_disk_pdev_info(libxl_device_disk *disk, char *pdev_info)
+{
+ char *pdev_type, *pdev_impl, *pdev_format, *pdev_path;
+
+ 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) ||
+ !strncmp(pdev_impl, "tapdisk", sizeof("tapdisk")-1) )
+ disk->backend = DISK_BACKEND_TAPDISK2;
+ else if ( !strncmp(pdev_impl, "ioemu", sizeof("ioemu")-1) )
+ disk->backend = DISK_BACKEND_QEMU;
+
+ if ( disk->backend == DISK_BACKEND_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->backend = DISK_BACKEND_BLKBACK;
+ else if ( !strncmp(pdev_type, "file", sizeof("file")-1) ||
+ !strncmp(pdev_type, "tap", sizeof("tap")-1) ||
+ !strncmp(pdev_type, "tap2", sizeof("tap2")-1) )
+ disk->backend = DISK_BACKEND_TAPDISK2;
+
+ 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 state = DSTATE_INITIAL, ret_val;
+ char *substr = buf2;
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->format = DISK_FORMAT_RAW;
- disk->backend = DISK_BACKEND_BLKBACK;
- }else if ( !strcmp(tok, "file") ) {
- state = DSTATE_PHYSPATH;
- disk->format = DISK_FORMAT_RAW;
- disk->backend = DISK_BACKEND_TAPDISK2;
- }else if ( !strcmp(tok, "tap") ) {
- state = DSTATE_TAP;
- }else{
- fprintf(stderr, "Unknown disk type: %s\n", tok);
- return 0;
+ for ( substr = strrchr(buf2, ',');
+ substr || (!substr && state == DSTATE_VDEV_PARSED);
+ substr = strrchr(buf2, ',') ) {
+ switch ( state ) {
+ case DSTATE_INITIAL: {
+ if ( !substr ) {
+ LOG("Invalid disk configuration option %s. Refer to the xl disk "
+ "configuration document for further information", buf2);
+ return ERROR_INVAL;
}
- tok = p + 1;
- } else if (*p == ',') {
- state = DSTATE_VIRTPATH;
- disk->backend = DISK_FORMAT_EMPTY;
- disk->pdev_path = strdup("");
- tok = p + 1;
- }
- break;
- case DSTATE_TAP:
- if ( *p == ':' ) {
- *p = '\0';
- if ( !strcmp(tok, "aio") ) {
- disk->format = DISK_FORMAT_RAW;
- disk->backend = DISK_BACKEND_TAPDISK2;
- }else if ( !strcmp(tok, "vhd") ) {
- disk->format = DISK_FORMAT_VHD;
- disk->backend = DISK_BACKEND_TAPDISK2;
- }else if ( !strcmp(tok, "qcow") ) {
- disk->format = DISK_FORMAT_QCOW;
- disk->backend = DISK_BACKEND_QEMU;
- }else if ( !strcmp(tok, "qcow2") ) {
- disk->format = DISK_FORMAT_QCOW2;
- disk->backend = DISK_BACKEND_QEMU;
- }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->pdev_path = (*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->vdev = (*tok) ? strdup(tok) : NULL;
- tok = p + 1;
+ *substr = '\0';
+ substr++;
+ ret_val = parse_disk_attrib(disk, substr);
+ if ( ret_val )
+ return ret_val;
+ state = DSTATE_ATTRIB_PARSED;
+ break;
}
- 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;
+ case DSTATE_ATTRIB_PARSED: {
+ if ( !substr ) {
+ LOG("Required vdev info missing in disk configuration option");
+ return ERROR_INVAL;
}
- tok = p + 1;
- state = (*p == ',') ? DSTATE_RW : DSTATE_TERMINAL;
+ *substr = '\0';
+ substr++;
+ ret_val = parse_disk_vdev_info(disk, substr);
+ if ( ret_val )
+ return ret_val;
+ state = DSTATE_VDEV_PARSED;
+ break;
}
- break;
- case DSTATE_RW:
- if ( *p == '\0' ) {
- disk->readwrite = (tok[0] == 'w');
- tok = p + 1;
- state = DSTATE_TERMINAL;
+ case DSTATE_VDEV_PARSED: {
+ ret_val = parse_disk_pdev_info(disk, buf2);
+ if ( ret_val )
+ return ret_val;
+ if ( disk->format == DISK_FORMAT_UNKNOWN )
+ disk->format = DISK_FORMAT_RAW;
+ if ( disk->backend == DISK_BACKEND_UNKNOWN )
+ disk->backend = DISK_BACKEND_TAPDISK2;
+ return 0;
}
- 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;
+ LOG("Disk configuration parsing failed");
+ return ERROR_INVAL;
}
static void parse_config_data(const char *configfile_filename_report,
@@ -768,7 +809,7 @@
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);
}
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 3/5] xl disk configuration parsing changes
2011-02-07 21:24 [PATCH 3/5] xl disk configuration parsing changes Kamala Narasimhan
@ 2011-02-08 16:43 ` Stefano Stabellini
2011-02-08 19:08 ` Kamala Narasimhan
0 siblings, 1 reply; 3+ messages in thread
From: Stefano Stabellini @ 2011-02-08 16:43 UTC (permalink / raw)
To: Kamala Narasimhan; +Cc: xen-devel@lists.xensource.com
On Mon, 7 Feb 2011, Kamala Narasimhan wrote:
> diff -u -r libxl-interface-changes/xl_cmdimpl.c libxl/xl_cmdimpl.c
> --- libxl-interface-changes/xl_cmdimpl.c 2011-02-07 12:03:37.000000000 -0500
> +++ libxl/xl_cmdimpl.c 2011-02-07 14:58:39.000000000 -0500
> @@ -438,143 +438,184 @@
> 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
> +#define DSTATE_INITIAL 0
> +#define DSTATE_ATTRIB_PARSED 1
> +#define DSTATE_VDEV_PARSED 2
Why did you need to remove 4 states?
Also it would be nice to split this patch in two: a patch that
introduces parse_disk_attrib, parse_disk_vdev_info, etc, and a second
patch that makes any necessary changes to the state machine.
Otherwise it is very difficult to understand the reason behind each
change to the state machine.
> +static int parse_disk_attrib(libxl_device_disk *disk, char *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", attrib);
> + return ERROR_INVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int parse_disk_vdev_info(libxl_device_disk *disk, char *vdev_info)
> +{
> + char *vdev, *type;
> +
> + 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 = strdup(vdev);
> + return 0;
> +}
> +
> +static int parse_disk_pdev_info(libxl_device_disk *disk, char *pdev_info)
> +{
> + char *pdev_type, *pdev_impl, *pdev_format, *pdev_path;
> +
> + 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) ||
> + !strncmp(pdev_impl, "tapdisk", sizeof("tapdisk")-1) )
> + disk->backend = DISK_BACKEND_TAPDISK2;
> + else if ( !strncmp(pdev_impl, "ioemu", sizeof("ioemu")-1) )
> + disk->backend = DISK_BACKEND_QEMU;
> +
> + if ( disk->backend == DISK_BACKEND_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->backend = DISK_BACKEND_BLKBACK;
> + else if ( !strncmp(pdev_type, "file", sizeof("file")-1) ||
> + !strncmp(pdev_type, "tap", sizeof("tap")-1) ||
> + !strncmp(pdev_type, "tap2", sizeof("tap2")-1) )
> + disk->backend = DISK_BACKEND_TAPDISK2;
> +
> + 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 state = DSTATE_INITIAL, ret_val;
> + char *substr = buf2;
>
> 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->format = DISK_FORMAT_RAW;
> - disk->backend = DISK_BACKEND_BLKBACK;
> - }else if ( !strcmp(tok, "file") ) {
> - state = DSTATE_PHYSPATH;
> - disk->format = DISK_FORMAT_RAW;
> - disk->backend = DISK_BACKEND_TAPDISK2;
> - }else if ( !strcmp(tok, "tap") ) {
> - state = DSTATE_TAP;
> - }else{
> - fprintf(stderr, "Unknown disk type: %s\n", tok);
> - return 0;
> + for ( substr = strrchr(buf2, ',');
> + substr || (!substr && state == DSTATE_VDEV_PARSED);
> + substr = strrchr(buf2, ',') ) {
> + switch ( state ) {
> + case DSTATE_INITIAL: {
> + if ( !substr ) {
> + LOG("Invalid disk configuration option %s. Refer to the xl disk "
> + "configuration document for further information", buf2);
> + return ERROR_INVAL;
> }
> - tok = p + 1;
> - } else if (*p == ',') {
> - state = DSTATE_VIRTPATH;
> - disk->backend = DISK_FORMAT_EMPTY;
> - disk->pdev_path = strdup("");
> - tok = p + 1;
> - }
> - break;
> - case DSTATE_TAP:
> - if ( *p == ':' ) {
> - *p = '\0';
> - if ( !strcmp(tok, "aio") ) {
> - disk->format = DISK_FORMAT_RAW;
> - disk->backend = DISK_BACKEND_TAPDISK2;
> - }else if ( !strcmp(tok, "vhd") ) {
> - disk->format = DISK_FORMAT_VHD;
> - disk->backend = DISK_BACKEND_TAPDISK2;
> - }else if ( !strcmp(tok, "qcow") ) {
> - disk->format = DISK_FORMAT_QCOW;
> - disk->backend = DISK_BACKEND_QEMU;
> - }else if ( !strcmp(tok, "qcow2") ) {
> - disk->format = DISK_FORMAT_QCOW2;
> - disk->backend = DISK_BACKEND_QEMU;
> - }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->pdev_path = (*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->vdev = (*tok) ? strdup(tok) : NULL;
> - tok = p + 1;
> + *substr = '\0';
> + substr++;
> + ret_val = parse_disk_attrib(disk, substr);
> + if ( ret_val )
> + return ret_val;
> + state = DSTATE_ATTRIB_PARSED;
> + break;
> }
> - 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;
> + case DSTATE_ATTRIB_PARSED: {
> + if ( !substr ) {
> + LOG("Required vdev info missing in disk configuration option");
> + return ERROR_INVAL;
> }
> - tok = p + 1;
> - state = (*p == ',') ? DSTATE_RW : DSTATE_TERMINAL;
> + *substr = '\0';
> + substr++;
> + ret_val = parse_disk_vdev_info(disk, substr);
> + if ( ret_val )
> + return ret_val;
> + state = DSTATE_VDEV_PARSED;
> + break;
> }
> - break;
> - case DSTATE_RW:
> - if ( *p == '\0' ) {
> - disk->readwrite = (tok[0] == 'w');
> - tok = p + 1;
> - state = DSTATE_TERMINAL;
> + case DSTATE_VDEV_PARSED: {
> + ret_val = parse_disk_pdev_info(disk, buf2);
> + if ( ret_val )
> + return ret_val;
> + if ( disk->format == DISK_FORMAT_UNKNOWN )
> + disk->format = DISK_FORMAT_RAW;
> + if ( disk->backend == DISK_BACKEND_UNKNOWN )
> + disk->backend = DISK_BACKEND_TAPDISK2;
> + return 0;
> }
> - 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;
> + LOG("Disk configuration parsing failed");
> + return ERROR_INVAL;
> }
There doesn't seem to be a way to set DISK_FORMAT_EMPTY anymore, in fact
a config line like ',hdc:cdrom,w' seems to be broken.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 3/5] xl disk configuration parsing changes
2011-02-08 16:43 ` Stefano Stabellini
@ 2011-02-08 19:08 ` Kamala Narasimhan
0 siblings, 0 replies; 3+ messages in thread
From: Kamala Narasimhan @ 2011-02-08 19:08 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel@lists.xensource.com
>> -#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
>> +#define DSTATE_INITIAL 0
>> +#define DSTATE_ATTRIB_PARSED 1
>> +#define DSTATE_VDEV_PARSED 2
>
> Why did you need to remove 4 states?
I didn't have a need for it.
> Also it would be nice to split this patch in two: a patch that
> introduces parse_disk_attrib, parse_disk_vdev_info, etc, and a second
> patch that makes any necessary changes to the state machine.
> Otherwise it is very difficult to understand the reason behind each
> change to the state machine.
>
That might be a little tricky. I will talk to you on this offline.
>> static int parse_disk_config(libxl_device_disk *disk, char *buf2)
>> {
>
> There doesn't seem to be a way to set DISK_FORMAT_EMPTY anymore, in fact
> a config line like ',hdc:cdrom,w' seems to be broken.
>
Patch 5/5, validation code should take care of it.
Kamala
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-02-08 19:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-07 21:24 [PATCH 3/5] xl disk configuration parsing changes Kamala Narasimhan
2011-02-08 16:43 ` Stefano Stabellini
2011-02-08 19:08 ` Kamala Narasimhan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.