xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] xl: empty disk devices
@ 2012-07-25 16:08 Ian Jackson
  2012-07-25 16:08 ` [PATCH 1/3] xl: support empty CDROM devices Ian Jackson
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Ian Jackson @ 2012-07-25 16:08 UTC (permalink / raw)
  To: xen-devel

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] xl: support empty CDROM devices
  2012-07-25 16:08 [PATCH v2 0/3] xl: empty disk devices Ian Jackson
@ 2012-07-25 16:08 ` Ian Jackson
  2012-07-25 16:08 ` [PATCH 2/3] xl: disk parsing preparation for empty cdrom devices Ian Jackson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Ian Jackson @ 2012-07-25 16:08 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] xl: disk parsing preparation for empty cdrom devices
  2012-07-25 16:08 [PATCH v2 0/3] xl: empty disk devices Ian Jackson
  2012-07-25 16:08 ` [PATCH 1/3] xl: support empty CDROM devices Ian Jackson
@ 2012-07-25 16:08 ` Ian Jackson
  2012-07-25 16:08 ` [PATCH 3/3] xl: support xend empty cdrom device syntax Ian Jackson
  2012-07-26  9:35 ` [PATCH v2 0/3] xl: empty disk devices Ian Campbell
  3 siblings, 0 replies; 5+ messages in thread
From: Ian Jackson @ 2012-07-25 16:08 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] xl: support xend empty cdrom device syntax
  2012-07-25 16:08 [PATCH v2 0/3] xl: empty disk devices Ian Jackson
  2012-07-25 16:08 ` [PATCH 1/3] xl: support empty CDROM devices Ian Jackson
  2012-07-25 16:08 ` [PATCH 2/3] xl: disk parsing preparation for empty cdrom devices Ian Jackson
@ 2012-07-25 16:08 ` Ian Jackson
  2012-07-26  9:35 ` [PATCH v2 0/3] xl: empty disk devices Ian Campbell
  3 siblings, 0 replies; 5+ messages in thread
From: Ian Jackson @ 2012-07-25 16:08 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 0/3] xl: empty disk devices
  2012-07-25 16:08 [PATCH v2 0/3] xl: empty disk devices Ian Jackson
                   ` (2 preceding siblings ...)
  2012-07-25 16:08 ` [PATCH 3/3] xl: support xend empty cdrom device syntax Ian Jackson
@ 2012-07-26  9:35 ` Ian Campbell
  3 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2012-07-26  9:35 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel@lists.xen.org

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-07-26  9:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-25 16:08 [PATCH v2 0/3] xl: empty disk devices Ian Jackson
2012-07-25 16:08 ` [PATCH 1/3] xl: support empty CDROM devices Ian Jackson
2012-07-25 16:08 ` [PATCH 2/3] xl: disk parsing preparation for empty cdrom devices Ian Jackson
2012-07-25 16:08 ` [PATCH 3/3] xl: support xend empty cdrom device syntax Ian Jackson
2012-07-26  9:35 ` [PATCH v2 0/3] xl: empty disk devices Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).