This now includes Ian Campbell''s original fix, and seems to work (well, pass the tests) for me. 1/3 xl: support empty CDROM devices 2/3 xl: disk parsing preparation for empty cdrom devices 3/3 xl: support xend empty cdrom device syntax
From: Ian Campbell <ian.campbell@citrix.com> The important change here is to xlu_disk_parse to correctly set format == EMPTY for CDROM devices which are empty. Test cases are added which check for correctness here. xend accepts '',hdc:cdrom,r''[0] as an empty CDROM drive however this is not consistent with the xl syntax in docs/misc/xl-disk-configuration.txt which requires '',,hdc:cdrom,r'' (the additional positional paramter is the format). I''m not sure if/how this can be fixed. Note that xend does not accept '',,hdc:cdrom,r'' There are several incidental cleanups included the the cdrom-{insert,eject} commands: - add a dry-run mode - use the non-deprecated disk specification syntax - check for and report errors from libxl_cdrom_insert [0] http://wiki.xen.org/wiki/CD_Rom_Support_in_Xen Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> - Changes in v2: * Add a test case for `devtype=cdrom,,,hdc'' --- docs/man/xl.pod.1 | 30 +++++++++++++++++++++--------- tools/libxl/check-xl-disk-parse | 35 +++++++++++++++++++++++++++++++++++ tools/libxl/libxlu_disk.c | 2 ++ tools/libxl/libxlu_disk_l.c | 35 ++++++++++++++--------------------- tools/libxl/libxlu_disk_l.h | 10 +--------- tools/libxl/libxlu_disk_l.l | 5 +++-- tools/libxl/xl_cmdimpl.c | 3 ++- tools/libxl/xl_cmdtable.c | 4 ++-- 8 files changed, 80 insertions(+), 44 deletions(-) diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1 index cbd7608..25ce777 100644 --- a/docs/man/xl.pod.1 +++ b/docs/man/xl.pod.1 @@ -1037,9 +1037,12 @@ in the domain. List virtual block devices for a domain. -=item B<cd-insert> I<domain-id> I<VirtualDevice> I<be-dev> +=item B<cd-insert> I<domain-id> I<VirtualDevice> I<target> -Insert a cdrom into a guest domain''s cd drive. Only works with HVM domains. +Insert a cdrom into a guest domain''s existing virtial cd drive. The +virtual drive must already exist but can be current empty. + +Only works with HVM domains. B<OPTIONS> @@ -1047,20 +1050,29 @@ B<OPTIONS> =item I<VirtualDevice> -How the device should be presented to the guest domain; for example /dev/hdc. +How the device should be presented to the guest domain; for example "hdc". -=item I<be-dev> +=item I<target> -the device in the backend domain (usually domain 0) to be exported; it -can be a path to a file (file://path/to/file.iso). See B<disk> in -L<xl.cfg(5)> for the details. +the target path in the backend domain (usually domain 0) to be +exported; Can be a block device or a file etc. See B<target> in +F<docs/misc/xl-disk-configuration.txt>. =back =item B<cd-eject> I<domain-id> I<VirtualDevice> -Eject a cdrom from a guest''s cd drive. Only works with HVM domains. -I<VirtualDevice> is the cdrom device in the guest to eject. +Eject a cdrom from a guest''s virtual cd drive. Only works with HVM domains. + +B<OPTIONS> + +=over 4 + +=item I<VirtualDevice> + +How the device should be presented to the guest domain; for example "hdc". + +=back =back diff --git a/tools/libxl/check-xl-disk-parse b/tools/libxl/check-xl-disk-parse index 67805e5..7a33780 100755 --- a/tools/libxl/check-xl-disk-parse +++ b/tools/libxl/check-xl-disk-parse @@ -107,4 +107,39 @@ disk: { EOF one 0 backendtype=phy,vdev=xvdb,access=w,target=/dev/vg/guest-volume +expected <<EOF +disk: { + "backend_domid": 0, + "pdev_path": "", + "vdev": "hdc", + "backend": "unknown", + "format": "empty", + "script": null, + "removable": 1, + "readwrite": 0, + "is_cdrom": 1 +} + +EOF +one 0 devtype=cdrom,,,hdc +one 0 ,,hdc:cdrom,r +one 0 vdev=hdc,access=r,devtype=cdrom,target+one 0 ,empty,hdc:cdrom,r + +expected <<EOF +disk: { + "backend_domid": 0, + "pdev_path": null, + "vdev": "hdc", + "backend": "unknown", + "format": "empty", + "script": null, + "removable": 1, + "readwrite": 0, + "is_cdrom": 1 +} + +EOF +one 0 vdev=hdc,access=r,devtype=cdrom,format=empty + complete diff --git a/tools/libxl/libxlu_disk.c b/tools/libxl/libxlu_disk.c index 3d51def..18fe386 100644 --- a/tools/libxl/libxlu_disk.c +++ b/tools/libxl/libxlu_disk.c @@ -76,6 +76,8 @@ int xlu_disk_parse(XLU_Config *cfg, if (disk->is_cdrom) { disk->removable = 1; disk->readwrite = 0; + if (!disk->pdev_path || !strcmp(disk->pdev_path, "")) + disk->format = LIBXL_DISK_FORMAT_EMPTY; } if (!disk->vdev) { diff --git a/tools/libxl/libxlu_disk_l.c b/tools/libxl/libxlu_disk_l.c index 2f0f283..8b9f44d 100644 --- a/tools/libxl/libxlu_disk_l.c +++ b/tools/libxl/libxlu_disk_l.c @@ -841,11 +841,12 @@ static void setaccess(DiskParseContext *dpc, const char *str) { /* Sets ->format from the string. IDL should provide something for this. */ static void setformat(DiskParseContext *dpc, const char *str) { - if (!strcmp(str,"") || - !strcmp(str,"raw")) DSET(dpc,format,FORMAT,str,RAW); + if (!strcmp(str,"")) DSET(dpc,format,FORMAT,str,RAW); + else if (!strcmp(str,"raw")) DSET(dpc,format,FORMAT,str,RAW); else if (!strcmp(str,"qcow")) DSET(dpc,format,FORMAT,str,QCOW); else if (!strcmp(str,"qcow2")) DSET(dpc,format,FORMAT,str,QCOW2); else if (!strcmp(str,"vhd")) DSET(dpc,format,FORMAT,str,VHD); + else if (!strcmp(str,"empty")) DSET(dpc,format,FORMAT,str,EMPTY); else xlu__disk_err(dpc,str,"unknown value for format"); } @@ -860,7 +861,7 @@ static void setbackendtype(DiskParseContext *dpc, const char *str) { #define DEPRECATE(usewhatinstead) /* not currently reported */ -#line 864 "libxlu_disk_l.c" +#line 865 "libxlu_disk_l.c" #define INITIAL 0 #define LEXERR 1 @@ -1096,12 +1097,12 @@ YY_DECL register int yy_act; struct yyguts_t * yyg = (struct yyguts_t*)yyscanner; -#line 126 "libxlu_disk_l.l" +#line 127 "libxlu_disk_l.l" /*----- the scanner rules which do the parsing -----*/ -#line 1105 "libxlu_disk_l.c" +#line 1106 "libxlu_disk_l.c" if ( !yyg->yy_init ) { @@ -1295,7 +1296,7 @@ YY_RULE_SETUP * matched the whole string, so these patterns take precedence */ case 13: YY_RULE_SETUP -#line 160 "libxlu_disk_l.l" +#line 161 "libxlu_disk_l.l" { STRIP('':''); DPC->had_depr_prefix=1; DEPRECATE("use `[format=]...,''"); @@ -1304,7 +1305,7 @@ YY_RULE_SETUP YY_BREAK case 14: YY_RULE_SETUP -#line 166 "libxlu_disk_l.l" +#line 167 "libxlu_disk_l.l" { STRIP('':''); DPC->had_depr_prefix=1; DEPRECATE("use `script=...''"); @@ -1329,7 +1330,7 @@ case 17: yyg->yy_c_buf_p = yy_cp = yy_bp + 4; YY_DO_BEFORE_ACTION; /* set up yytext again */ YY_RULE_SETUP -#line 174 "libxlu_disk_l.l" +#line 175 "libxlu_disk_l.l" { DPC->had_depr_prefix=1; DEPRECATE(0); } YY_BREAK case 18: @@ -1337,7 +1338,7 @@ case 18: yyg->yy_c_buf_p = yy_cp = yy_bp + 6; YY_DO_BEFORE_ACTION; /* set up yytext again */ YY_RULE_SETUP -#line 175 "libxlu_disk_l.l" +#line 176 "libxlu_disk_l.l" { DPC->had_depr_prefix=1; DEPRECATE(0); } YY_BREAK case 19: @@ -1345,7 +1346,7 @@ case 19: yyg->yy_c_buf_p = yy_cp = yy_bp + 5; YY_DO_BEFORE_ACTION; /* set up yytext again */ YY_RULE_SETUP -#line 176 "libxlu_disk_l.l" +#line 177 "libxlu_disk_l.l" { DPC->had_depr_prefix=1; DEPRECATE(0); } YY_BREAK case 20: @@ -1369,7 +1370,7 @@ YY_RULE_SETUP case 22: /* rule 22 can match eol */ YY_RULE_SETUP -#line 186 "libxlu_disk_l.l" +#line 187 "libxlu_disk_l.l" { char *colon; STRIP('',''); @@ -1406,7 +1407,7 @@ YY_RULE_SETUP YY_BREAK case 23: YY_RULE_SETUP -#line 220 "libxlu_disk_l.l" +#line 221 "libxlu_disk_l.l" { BEGIN(LEXERR); yymore(); @@ -2516,12 +2517,4 @@ void xlu__disk_yyfree (void * ptr , yyscan_t yyscanner) #define YYTABLES_NAME "yytables" -#line 227 "libxlu_disk_l.l" - -/* - * Local variables: - * mode: C - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ +#line 228 "libxlu_disk_l.l" diff --git a/tools/libxl/libxlu_disk_l.h b/tools/libxl/libxlu_disk_l.h index a3fbd72..87bf96c 100644 --- a/tools/libxl/libxlu_disk_l.h +++ b/tools/libxl/libxlu_disk_l.h @@ -340,16 +340,8 @@ extern int xlu__disk_yylex (yyscan_t yyscanner); #undef YY_DECL #endif -#line 227 "libxlu_disk_l.l" +#line 228 "libxlu_disk_l.l" #line 346 "libxlu_disk_l.h" #undef xlu__disk_yyIN_HEADER #endif /* xlu__disk_yyHEADER_H */ - -/* - * Local variables: - * mode: C - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l index a3e7180..6e53928 100644 --- a/tools/libxl/libxlu_disk_l.l +++ b/tools/libxl/libxlu_disk_l.l @@ -92,11 +92,12 @@ static void setaccess(DiskParseContext *dpc, const char *str) { /* Sets ->format from the string. IDL should provide something for this. */ static void setformat(DiskParseContext *dpc, const char *str) { - if (!strcmp(str,"") || - !strcmp(str,"raw")) DSET(dpc,format,FORMAT,str,RAW); + if (!strcmp(str,"")) DSET(dpc,format,FORMAT,str,RAW); + else if (!strcmp(str,"raw")) DSET(dpc,format,FORMAT,str,RAW); else if (!strcmp(str,"qcow")) DSET(dpc,format,FORMAT,str,QCOW); else if (!strcmp(str,"qcow2")) DSET(dpc,format,FORMAT,str,QCOW2); else if (!strcmp(str,"vhd")) DSET(dpc,format,FORMAT,str,VHD); + else if (!strcmp(str,"empty")) DSET(dpc,format,FORMAT,str,EMPTY); else xlu__disk_err(dpc,str,"unknown value for format"); } diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index e2aa8592..3fd152a 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -2211,7 +2211,8 @@ static void cd_insert(const char *dom, const char *virtdev, char *phys) find_domain(dom); - if (asprintf(&buf, "%s,%s:cdrom,r", phys ? phys : "", virtdev) < 0) { + if (asprintf(&buf, "vdev=%s,access=r,devtype=cdrom,target=%s", + virtdev, phys ? phys : "") < 0) { fprintf(stderr, "out of memory\n"); return; } diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c index f0a88c2..85ea768 100644 --- a/tools/libxl/xl_cmdtable.c +++ b/tools/libxl/xl_cmdtable.c @@ -174,12 +174,12 @@ struct cmd_spec cmd_table[] = { "- for internal use only", }, { "cd-insert", - &main_cd_insert, 0, 1, + &main_cd_insert, 1, 1, "Insert a cdrom into a guest''s cd drive", "<Domain> <VirtualDevice> <type:path>", }, { "cd-eject", - &main_cd_eject, 0, 1, + &main_cd_eject, 1, 1, "Eject a cdrom from a guest''s cd drive", "<Domain> <VirtualDevice>", }, -- 1.7.2.5
Ian Jackson
2012-Jul-25 16:08 UTC
[PATCH 2/3] xl: disk parsing preparation for empty cdrom devices
Prepare the ground for parsing the xend empty cdrom syntax, by separating out some non-functional changes as this pre-patch: * Clarify the disk syntax documentation wording to refer to deprecated syntaxes too. * Make DPC in libxlu_disk_l.l useable in the helper functions as well as in lexer rules, by providing two definitions, each in force in the appropriate parts of the file. * Break the <vdev>[:<devtype>] parsing out into a helper function, `vdev_and_devtype''. No functional change. Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> --- docs/misc/xl-disk-configuration.txt | 11 ++-- tools/libxl/libxlu_disk_l.c | 104 +++++++++++++++++++--------------- tools/libxl/libxlu_disk_l.h | 2 +- tools/libxl/libxlu_disk_l.l | 44 +++++++++----- 4 files changed, 93 insertions(+), 68 deletions(-) diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt index 3bf20bf..740b8f6 100644 --- a/docs/misc/xl-disk-configuration.txt +++ b/docs/misc/xl-disk-configuration.txt @@ -164,16 +164,17 @@ information to be interpreted by /etc/xen/scripts/block-<script>. -=================================-DEPRECATED PARAMETERS AND PREFIXES -=================================+===========================================+DEPRECATED PARAMETERS, PREFIXES AND SYNTAXES +=========================================== -Deprecated values are acceptable and are intended work compatibly with +Deprecated forms are acceptable and are intended work compatibly with xend and xl from xen 4.1. In future they may print a warning. Support for deprecated parameters and syntaxes are likely to be dropped in future versions of xl. -There is also support for a deprecated old syntax for <diskspec>: + +There is support for a deprecated old syntax for <diskspec>: [<format>:][<target>],<vdev>[:<devtype>],<access> (deprecated) diff --git a/tools/libxl/libxlu_disk_l.c b/tools/libxl/libxlu_disk_l.c index 8b9f44d..b4ad651 100644 --- a/tools/libxl/libxlu_disk_l.c +++ b/tools/libxl/libxlu_disk_l.c @@ -794,8 +794,6 @@ void xlu__disk_yyset_column(int column_no, yyscan_t yyscanner); * and particularly to avoid repeating boilerplate values such as * DPC->disk, yytext, etc. */ -#define DPC ((DiskParseContext*)yyextra) - /* Sets an enum, checking it hasn''t already been set to a different value */ #define DSET(dpc,member,enumname,str,valname) do{ \ if (dpc->disk->member != LIBXL_DISK_##enumname##_UNKNOWN && \ @@ -828,6 +826,8 @@ static void savestring(DiskParseContext *dpc, const char *what_respecified, *update = strdup(value); } +#define DPC dpc /* our convention in lexer helper functions */ + /* Sets ->readwrite from the string. This ought to be an enum, perhaps. */ static void setaccess(DiskParseContext *dpc, const char *str) { if (!strcmp(str, "r") || !strcmp(str, "ro")) { @@ -860,8 +860,32 @@ static void setbackendtype(DiskParseContext *dpc, const char *str) { #define DEPRECATE(usewhatinstead) /* not currently reported */ +/* Handles a vdev positional parameter which includes a devtype. */ +static int vdev_and_devtype(DiskParseContext *dpc, char *str) { + /* returns 1 if it was <vdev>:<devtype>, 0 (doing nothing) otherwise */ + char *colon = strrchr(str, '':''); + if (!colon) + return 0; + + DEPRECATE("use `devtype=...''"); + *colon++ = 0; + SAVESTRING("vdev", vdev, str); + + if (!strcmp(colon,"cdrom")) { + DPC->disk->is_cdrom = 1; + } else if (!strcmp(colon,"disk")) { + DPC->disk->is_cdrom = 0; + } else { + xlu__disk_err(DPC,colon,"unknown deprecated type"); + } + return 1; +} -#line 865 "libxlu_disk_l.c" +#undef DPC /* needs to be defined differently the actual lexer */ +#define DPC ((DiskParseContext*)yyextra) + + +#line 889 "libxlu_disk_l.c" #define INITIAL 0 #define LEXERR 1 @@ -1097,12 +1121,12 @@ YY_DECL register int yy_act; struct yyguts_t * yyg = (struct yyguts_t*)yyscanner; -#line 127 "libxlu_disk_l.l" +#line 151 "libxlu_disk_l.l" /*----- the scanner rules which do the parsing -----*/ -#line 1106 "libxlu_disk_l.c" +#line 1130 "libxlu_disk_l.c" if ( !yyg->yy_init ) { @@ -1223,72 +1247,72 @@ do_action: /* This label is used only to access EOF actions. */ case 1: /* rule 1 can match eol */ YY_RULE_SETUP -#line 130 "libxlu_disk_l.l" +#line 155 "libxlu_disk_l.l" { /* ignore whitespace before parameters */ } YY_BREAK /* ordinary parameters setting enums or strings */ case 2: /* rule 2 can match eol */ YY_RULE_SETUP -#line 134 "libxlu_disk_l.l" +#line 159 "libxlu_disk_l.l" { STRIP('',''); setformat(DPC, FROMEQUALS); } YY_BREAK case 3: YY_RULE_SETUP -#line 136 "libxlu_disk_l.l" +#line 161 "libxlu_disk_l.l" { DPC->disk->is_cdrom = 1; } YY_BREAK case 4: YY_RULE_SETUP -#line 137 "libxlu_disk_l.l" +#line 162 "libxlu_disk_l.l" { DPC->disk->is_cdrom = 1; } YY_BREAK case 5: YY_RULE_SETUP -#line 138 "libxlu_disk_l.l" +#line 163 "libxlu_disk_l.l" { DPC->disk->is_cdrom = 0; } YY_BREAK case 6: /* rule 6 can match eol */ YY_RULE_SETUP -#line 139 "libxlu_disk_l.l" +#line 164 "libxlu_disk_l.l" { xlu__disk_err(DPC,yytext,"unknown value for type"); } YY_BREAK case 7: /* rule 7 can match eol */ YY_RULE_SETUP -#line 141 "libxlu_disk_l.l" +#line 166 "libxlu_disk_l.l" { STRIP('',''); setaccess(DPC, FROMEQUALS); } YY_BREAK case 8: /* rule 8 can match eol */ YY_RULE_SETUP -#line 142 "libxlu_disk_l.l" +#line 167 "libxlu_disk_l.l" { STRIP('',''); setbackendtype(DPC,FROMEQUALS); } YY_BREAK case 9: /* rule 9 can match eol */ YY_RULE_SETUP -#line 144 "libxlu_disk_l.l" +#line 169 "libxlu_disk_l.l" { STRIP('',''); SAVESTRING("vdev", vdev, FROMEQUALS); } YY_BREAK case 10: /* rule 10 can match eol */ YY_RULE_SETUP -#line 145 "libxlu_disk_l.l" +#line 170 "libxlu_disk_l.l" { STRIP('',''); SAVESTRING("script", script, FROMEQUALS); } YY_BREAK /* the target magic parameter, eats the rest of the string */ case 11: YY_RULE_SETUP -#line 149 "libxlu_disk_l.l" +#line 174 "libxlu_disk_l.l" { STRIP('',''); SAVESTRING("target", pdev_path, FROMEQUALS); } YY_BREAK /* unknown parameters */ case 12: /* rule 12 can match eol */ YY_RULE_SETUP -#line 153 "libxlu_disk_l.l" +#line 178 "libxlu_disk_l.l" { xlu__disk_err(DPC,yytext,"unknown parameter"); } YY_BREAK /* deprecated prefixes */ @@ -1296,7 +1320,7 @@ YY_RULE_SETUP * matched the whole string, so these patterns take precedence */ case 13: YY_RULE_SETUP -#line 161 "libxlu_disk_l.l" +#line 185 "libxlu_disk_l.l" { STRIP('':''); DPC->had_depr_prefix=1; DEPRECATE("use `[format=]...,''"); @@ -1305,7 +1329,7 @@ YY_RULE_SETUP YY_BREAK case 14: YY_RULE_SETUP -#line 167 "libxlu_disk_l.l" +#line 191 "libxlu_disk_l.l" { STRIP('':''); DPC->had_depr_prefix=1; DEPRECATE("use `script=...''"); @@ -1317,12 +1341,12 @@ case 15: yyg->yy_c_buf_p = yy_cp = yy_bp + 8; YY_DO_BEFORE_ACTION; /* set up yytext again */ YY_RULE_SETUP -#line 172 "libxlu_disk_l.l" +#line 197 "libxlu_disk_l.l" { DPC->had_depr_prefix=1; DEPRECATE(0); } YY_BREAK case 16: YY_RULE_SETUP -#line 173 "libxlu_disk_l.l" +#line 198 "libxlu_disk_l.l" { DPC->had_depr_prefix=1; DEPRECATE(0); } YY_BREAK case 17: @@ -1330,7 +1354,7 @@ case 17: yyg->yy_c_buf_p = yy_cp = yy_bp + 4; YY_DO_BEFORE_ACTION; /* set up yytext again */ YY_RULE_SETUP -#line 175 "libxlu_disk_l.l" +#line 199 "libxlu_disk_l.l" { DPC->had_depr_prefix=1; DEPRECATE(0); } YY_BREAK case 18: @@ -1338,7 +1362,7 @@ case 18: yyg->yy_c_buf_p = yy_cp = yy_bp + 6; YY_DO_BEFORE_ACTION; /* set up yytext again */ YY_RULE_SETUP -#line 176 "libxlu_disk_l.l" +#line 200 "libxlu_disk_l.l" { DPC->had_depr_prefix=1; DEPRECATE(0); } YY_BREAK case 19: @@ -1346,7 +1370,7 @@ case 19: yyg->yy_c_buf_p = yy_cp = yy_bp + 5; YY_DO_BEFORE_ACTION; /* set up yytext again */ YY_RULE_SETUP -#line 177 "libxlu_disk_l.l" +#line 201 "libxlu_disk_l.l" { DPC->had_depr_prefix=1; DEPRECATE(0); } YY_BREAK case 20: @@ -1354,13 +1378,13 @@ case 20: yyg->yy_c_buf_p = yy_cp = yy_bp + 4; YY_DO_BEFORE_ACTION; /* set up yytext again */ YY_RULE_SETUP -#line 177 "libxlu_disk_l.l" +#line 202 "libxlu_disk_l.l" { DPC->had_depr_prefix=1; DEPRECATE(0); } YY_BREAK case 21: /* rule 21 can match eol */ YY_RULE_SETUP -#line 179 "libxlu_disk_l.l" +#line 204 "libxlu_disk_l.l" { xlu__disk_err(DPC,yytext,"unknown deprecated disk prefix"); return 0; @@ -1370,9 +1394,8 @@ YY_RULE_SETUP case 22: /* rule 22 can match eol */ YY_RULE_SETUP -#line 187 "libxlu_disk_l.l" +#line 211 "libxlu_disk_l.l" { - char *colon; STRIP('',''); if (DPC->err) { @@ -1383,19 +1406,8 @@ YY_RULE_SETUP DPC->disk->format == LIBXL_DISK_FORMAT_UNKNOWN) { setformat(DPC,yytext); } else if (!DPC->disk->vdev) { - colon = strrchr(yytext, '':''); - if (colon) { - DEPRECATE("use `devtype=...''"); - *colon++ = 0; - if (!strcmp(colon,"cdrom")) { - DPC->disk->is_cdrom = 1; - } else if (!strcmp(colon,"disk")) { - DPC->disk->is_cdrom = 0; - } else { - xlu__disk_err(DPC,colon,"unknown deprecated type"); - } - } - SAVESTRING("vdev", vdev, yytext); + if (!vdev_and_devtype(DPC,yytext)) + SAVESTRING("vdev", vdev, yytext); } else if (!DPC->access_set) { DPC->access_set = 1; setaccess(DPC,yytext); @@ -1407,7 +1419,7 @@ YY_RULE_SETUP YY_BREAK case 23: YY_RULE_SETUP -#line 221 "libxlu_disk_l.l" +#line 233 "libxlu_disk_l.l" { BEGIN(LEXERR); yymore(); @@ -1415,17 +1427,17 @@ YY_RULE_SETUP YY_BREAK case 24: YY_RULE_SETUP -#line 224 "libxlu_disk_l.l" +#line 237 "libxlu_disk_l.l" { xlu__disk_err(DPC,yytext,"bad disk syntax"); return 0; } YY_BREAK case 25: YY_RULE_SETUP -#line 227 "libxlu_disk_l.l" +#line 240 "libxlu_disk_l.l" YY_FATAL_ERROR( "flex scanner jammed" ); YY_BREAK -#line 1428 "libxlu_disk_l.c" +#line 1441 "libxlu_disk_l.c" case YY_STATE_EOF(INITIAL): case YY_STATE_EOF(LEXERR): yyterminate(); @@ -2517,4 +2529,4 @@ void xlu__disk_yyfree (void * ptr , yyscan_t yyscanner) #define YYTABLES_NAME "yytables" -#line 228 "libxlu_disk_l.l" +#line 240 "libxlu_disk_l.l" diff --git a/tools/libxl/libxlu_disk_l.h b/tools/libxl/libxlu_disk_l.h index 87bf96c..4d0328e 100644 --- a/tools/libxl/libxlu_disk_l.h +++ b/tools/libxl/libxlu_disk_l.h @@ -340,7 +340,7 @@ extern int xlu__disk_yylex (yyscan_t yyscanner); #undef YY_DECL #endif -#line 228 "libxlu_disk_l.l" +#line 240 "libxlu_disk_l.l" #line 346 "libxlu_disk_l.h" #undef xlu__disk_yyIN_HEADER diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l index 6e53928..7e0a635 100644 --- a/tools/libxl/libxlu_disk_l.l +++ b/tools/libxl/libxlu_disk_l.l @@ -45,8 +45,6 @@ void xlu__disk_yyset_column(int column_no, yyscan_t yyscanner); * and particularly to avoid repeating boilerplate values such as * DPC->disk, yytext, etc. */ -#define DPC ((DiskParseContext*)yyextra) - /* Sets an enum, checking it hasn''t already been set to a different value */ #define DSET(dpc,member,enumname,str,valname) do{ \ if (dpc->disk->member != LIBXL_DISK_##enumname##_UNKNOWN && \ @@ -79,6 +77,8 @@ static void savestring(DiskParseContext *dpc, const char *what_respecified, *update = strdup(value); } +#define DPC dpc /* our convention in lexer helper functions */ + /* Sets ->readwrite from the string. This ought to be an enum, perhaps. */ static void setaccess(DiskParseContext *dpc, const char *str) { if (!strcmp(str, "r") || !strcmp(str, "ro")) { @@ -111,6 +111,30 @@ static void setbackendtype(DiskParseContext *dpc, const char *str) { #define DEPRECATE(usewhatinstead) /* not currently reported */ +/* Handles a vdev positional parameter which includes a devtype. */ +static int vdev_and_devtype(DiskParseContext *dpc, char *str) { + /* returns 1 if it was <vdev>:<devtype>, 0 (doing nothing) otherwise */ + char *colon = strrchr(str, '':''); + if (!colon) + return 0; + + DEPRECATE("use `devtype=...''"); + *colon++ = 0; + SAVESTRING("vdev", vdev, str); + + if (!strcmp(colon,"cdrom")) { + DPC->disk->is_cdrom = 1; + } else if (!strcmp(colon,"disk")) { + DPC->disk->is_cdrom = 0; + } else { + xlu__disk_err(DPC,colon,"unknown deprecated type"); + } + return 1; +} + +#undef DPC /* needs to be defined differently the actual lexer */ +#define DPC ((DiskParseContext*)yyextra) + %} %option warn @@ -185,7 +209,6 @@ phy:/.* { DPC->had_depr_prefix=1; DEPRECATE(0); } /* positional parameters */ [^=,]*,|[^=,]+,? { - char *colon; STRIP('',''); if (DPC->err) { @@ -196,19 +219,8 @@ phy:/.* { DPC->had_depr_prefix=1; DEPRECATE(0); } DPC->disk->format == LIBXL_DISK_FORMAT_UNKNOWN) { setformat(DPC,yytext); } else if (!DPC->disk->vdev) { - colon = strrchr(yytext, '':''); - if (colon) { - DEPRECATE("use `devtype=...''"); - *colon++ = 0; - if (!strcmp(colon,"cdrom")) { - DPC->disk->is_cdrom = 1; - } else if (!strcmp(colon,"disk")) { - DPC->disk->is_cdrom = 0; - } else { - xlu__disk_err(DPC,colon,"unknown deprecated type"); - } - } - SAVESTRING("vdev", vdev, yytext); + if (!vdev_and_devtype(DPC,yytext)) + SAVESTRING("vdev", vdev, yytext); } else if (!DPC->access_set) { DPC->access_set = 1; setaccess(DPC,yytext); -- 1.7.2.5
xend accepts `,hdc:cdrom,r'' as an empty CDROM drive. However this is not consistent with the existing xl syntax in docs/misc/xl-disk-configuration.txt which requires `,,hdc:cdrom,r'' (the additional positional paramter is the format). We fix this by spotting the case specially: when the target is empty and the format contains a colon, reinterpret the format as <vdev>:<devtype>. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> - Changes in v2: * Rebased on top of `xl: support empty CDROM devices'' * Fix commit message. --- docs/misc/xl-disk-configuration.txt | 11 +++++++++++ tools/libxl/check-xl-disk-parse | 1 + tools/libxl/libxlu_disk_l.c | 16 ++++++++++------ tools/libxl/libxlu_disk_l.h | 2 +- tools/libxl/libxlu_disk_l.l | 6 +++++- 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/docs/misc/xl-disk-configuration.txt b/docs/misc/xl-disk-configuration.txt index 740b8f6..5da5e11 100644 --- a/docs/misc/xl-disk-configuration.txt +++ b/docs/misc/xl-disk-configuration.txt @@ -221,3 +221,14 @@ to specify several of these, for example: "tap:aio:/some/path..." All of these prefixes are now stripped and ignored. + + +Missing format and empty target +------------------------------- + +The following syntax is also supported: + + ,<vdev>:<devtype>,<access> (deprecated) + +This is soley for compatibility with xend''s syntax for empty cdroms, +which is (for example) ",hdc:cdrom,r". diff --git a/tools/libxl/check-xl-disk-parse b/tools/libxl/check-xl-disk-parse index 7a33780..ffe3613 100755 --- a/tools/libxl/check-xl-disk-parse +++ b/tools/libxl/check-xl-disk-parse @@ -123,6 +123,7 @@ disk: { EOF one 0 devtype=cdrom,,,hdc one 0 ,,hdc:cdrom,r +one 0 ,hdc:cdrom,r one 0 vdev=hdc,access=r,devtype=cdrom,target one 0 ,empty,hdc:cdrom,r diff --git a/tools/libxl/libxlu_disk_l.c b/tools/libxl/libxlu_disk_l.c index b4ad651..f690385 100644 --- a/tools/libxl/libxlu_disk_l.c +++ b/tools/libxl/libxlu_disk_l.c @@ -1404,7 +1404,11 @@ YY_RULE_SETUP SAVESTRING("target", pdev_path, yytext); } else if (!DPC->had_depr_prefix && DPC->disk->format == LIBXL_DISK_FORMAT_UNKNOWN) { - setformat(DPC,yytext); + if (!*DPC->disk->pdev_path && vdev_and_devtype(DPC,yytext)) { + DPC->disk->format = LIBXL_DISK_FORMAT_EMPTY; + } else { + setformat(DPC,yytext); + } } else if (!DPC->disk->vdev) { if (!vdev_and_devtype(DPC,yytext)) SAVESTRING("vdev", vdev, yytext); @@ -1419,7 +1423,7 @@ YY_RULE_SETUP YY_BREAK case 23: YY_RULE_SETUP -#line 233 "libxlu_disk_l.l" +#line 237 "libxlu_disk_l.l" { BEGIN(LEXERR); yymore(); @@ -1427,17 +1431,17 @@ YY_RULE_SETUP YY_BREAK case 24: YY_RULE_SETUP -#line 237 "libxlu_disk_l.l" +#line 241 "libxlu_disk_l.l" { xlu__disk_err(DPC,yytext,"bad disk syntax"); return 0; } YY_BREAK case 25: YY_RULE_SETUP -#line 240 "libxlu_disk_l.l" +#line 244 "libxlu_disk_l.l" YY_FATAL_ERROR( "flex scanner jammed" ); YY_BREAK -#line 1441 "libxlu_disk_l.c" +#line 1445 "libxlu_disk_l.c" case YY_STATE_EOF(INITIAL): case YY_STATE_EOF(LEXERR): yyterminate(); @@ -2529,4 +2533,4 @@ void xlu__disk_yyfree (void * ptr , yyscan_t yyscanner) #define YYTABLES_NAME "yytables" -#line 240 "libxlu_disk_l.l" +#line 244 "libxlu_disk_l.l" diff --git a/tools/libxl/libxlu_disk_l.h b/tools/libxl/libxlu_disk_l.h index 4d0328e..26c9a86 100644 --- a/tools/libxl/libxlu_disk_l.h +++ b/tools/libxl/libxlu_disk_l.h @@ -340,7 +340,7 @@ extern int xlu__disk_yylex (yyscan_t yyscanner); #undef YY_DECL #endif -#line 240 "libxlu_disk_l.l" +#line 244 "libxlu_disk_l.l" #line 346 "libxlu_disk_l.h" #undef xlu__disk_yyIN_HEADER diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l index 7e0a635..f4e6b1a 100644 --- a/tools/libxl/libxlu_disk_l.l +++ b/tools/libxl/libxlu_disk_l.l @@ -217,7 +217,11 @@ phy:/.* { DPC->had_depr_prefix=1; DEPRECATE(0); } SAVESTRING("target", pdev_path, yytext); } else if (!DPC->had_depr_prefix && DPC->disk->format == LIBXL_DISK_FORMAT_UNKNOWN) { - setformat(DPC,yytext); + if (!*DPC->disk->pdev_path && vdev_and_devtype(DPC,yytext)) { + DPC->disk->format = LIBXL_DISK_FORMAT_EMPTY; + } else { + setformat(DPC,yytext); + } } else if (!DPC->disk->vdev) { if (!vdev_and_devtype(DPC,yytext)) SAVESTRING("vdev", vdev, yytext); -- 1.7.2.5
On Wed, 2012-07-25 at 17:08 +0100, Ian Jackson wrote:> This now includes Ian Campbell''s original fix, and seems to work > (well, pass the tests) for me.I tried these too and they seem good. Applied, thanks.> > 1/3 xl: support empty CDROM devices > 2/3 xl: disk parsing preparation for empty cdrom devices > 3/3 xl: support xend empty cdrom device syntax > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel