Re: [Xen-devel] [PATCH] xl: support empty CDROM devices

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-07-25 at 11:15 +0100, Ian Campbell wrote:
> 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' 

This can be fixed with the following hack to the parser. Effectively it
says that when target is given as a positional parameter and is blank
then format is implicitly "empty" and must be omitted.

I'm really not sure about the hunk in xlu_disk_parse. It doesn't seem to
be necessary (the tests work both with and without it) but perhaps it is
wise since when we parse the target positional param we don't yet know
if the device will have :cdrom specified.

I'm inclined towards doing this (and folding it into the original patch)
for compatibility with xm, thoughts?

8<-----------------------------
diff -r 576aeedaca77 docs/misc/xl-disk-configuration.txt
--- a/docs/misc/xl-disk-configuration.txt       Wed Jul 25 11:14:05 2012 +0100
+++ b/docs/misc/xl-disk-configuration.txt       Wed Jul 25 11:31:53 2012 +0100
@@ -91,6 +91,9 @@ Supported values:      raw, qcow, qcow2,
 Deprecated values:     None
 Default value:         raw
 
+When "target" has been provided as a positional parameter and is empty
+(the empty CDROM case referred to above) then this field is implicitly
+"empty" and must not be specified as a positional parameter.
 
 vdev
 ----
diff -r 576aeedaca77 tools/libxl/check-xl-disk-parse
--- a/tools/libxl/check-xl-disk-parse   Wed Jul 25 11:14:05 2012 +0100
+++ b/tools/libxl/check-xl-disk-parse   Wed Jul 25 11:31:53 2012 +0100
@@ -121,9 +121,12 @@ disk: {
 }
 
 EOF
-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
+
+expected <<EOF
+EOF
+one 255 ,,hdc:cdrom,r
 
 expected <<EOF
 disk: {
diff -r 576aeedaca77 tools/libxl/libxlu_disk.c
--- a/tools/libxl/libxlu_disk.c Wed Jul 25 11:14:05 2012 +0100
+++ b/tools/libxl/libxlu_disk.c Wed Jul 25 11:31:53 2012 +0100
@@ -78,6 +78,8 @@ int xlu_disk_parse(XLU_Config *cfg,
         disk->readwrite = 0;
         if (!disk->pdev_path || !strcmp(disk->pdev_path, ""))
             disk->format = LIBXL_DISK_FORMAT_EMPTY;
+    } else if (disk->format == LIBXL_DISK_FORMAT_EMPTY) {
+        disk->format = LIBXL_DISK_FORMAT_RAW;
     }
 
     if (!disk->vdev) {
diff -r 576aeedaca77 tools/libxl/libxlu_disk_l.c
diff -r 576aeedaca77 tools/libxl/libxlu_disk_l.h
diff -r 576aeedaca77 tools/libxl/libxlu_disk_l.l
--- a/tools/libxl/libxlu_disk_l.l       Wed Jul 25 11:14:05 2012 +0100
+++ b/tools/libxl/libxlu_disk_l.l       Wed Jul 25 11:31:53 2012 +0100
@@ -192,6 +192,8 @@ phy:/.*             { DPC->had_depr_prefix=1; DEPRE
         /* previous errors may just lead to subsequent ones */
     } else if (!DPC->disk->pdev_path) {
         SAVESTRING("target", pdev_path, yytext);
+        if (!strcmp(DPC->disk->pdev_path, ""))
+            DPC->disk->format = LIBXL_DISK_FORMAT_EMPTY;
     } else if (!DPC->had_depr_prefix &&
                DPC->disk->format == LIBXL_DISK_FORMAT_UNKNOWN) {
         setformat(DPC,yytext);




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel