Re: [Xen-devel] [PATCH 2/2] xl: support xend empty cdrom device syntax

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-07-25 at 16:22 +0100, Ian Jackson wrote:
> xend accepts ',hdc:cdrom,r'[0] 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@xxxxxxxxxx>
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> ---
>  docs/misc/xl-disk-configuration.txt |   11 ++++++++++
>  tools/libxl/check-xl-disk-parse     |   36 
> +++++++++++++++++++++++++++++++++++
>  tools/libxl/libxlu_disk_l.c         |   16 +++++++++-----
>  tools/libxl/libxlu_disk_l.h         |    2 +-
>  tools/libxl/libxlu_disk_l.l         |    6 ++++-
>  5 files changed, 63 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".

Perhaps we should tighten the special case to specifically only
accept ,<vdev>:cdrom,<access> ? (and actually as it happens <access> has
to be "r" too)

Also perhaps it would be useful to always give the mapping to the
non-deprecated syntax?

> diff --git a/tools/libxl/check-xl-disk-parse b/tools/libxl/check-xl-disk-parse
> index 67805e5..ffe3613 100755
> --- a/tools/libxl/check-xl-disk-parse
> +++ b/tools/libxl/check-xl-disk-parse
> @@ -107,4 +107,40 @@ 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 ,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_l.c b/tools/libxl/libxlu_disk_l.c
> index aac816a..42b4c22 100644
> --- a/tools/libxl/libxlu_disk_l.c
> +++ b/tools/libxl/libxlu_disk_l.c
> @@ -1403,7 +1403,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);
> @@ -1418,7 +1422,7 @@ YY_RULE_SETUP
>       YY_BREAK
>  case 23:
>  YY_RULE_SETUP
> -#line 232 "libxlu_disk_l.l"
> +#line 236 "libxlu_disk_l.l"
>  {
>      BEGIN(LEXERR);
>      yymore();
> @@ -1426,17 +1430,17 @@ YY_RULE_SETUP
>       YY_BREAK
>  case 24:
>  YY_RULE_SETUP
> -#line 236 "libxlu_disk_l.l"
> +#line 240 "libxlu_disk_l.l"
>  {
>      xlu__disk_err(DPC,yytext,"bad disk syntax"); return 0;
>  }
>       YY_BREAK
>  case 25:
>  YY_RULE_SETUP
> -#line 239 "libxlu_disk_l.l"
> +#line 243 "libxlu_disk_l.l"
>  YY_FATAL_ERROR( "flex scanner jammed" );
>       YY_BREAK
> -#line 1440 "libxlu_disk_l.c"
> +#line 1444 "libxlu_disk_l.c"
>                       case YY_STATE_EOF(INITIAL):
>                       case YY_STATE_EOF(LEXERR):
>                               yyterminate();
> @@ -2528,4 +2532,4 @@ void xlu__disk_yyfree (void * ptr , yyscan_t yyscanner)
>  
>  #define YYTABLES_NAME "yytables"
>  
> -#line 239 "libxlu_disk_l.l"
> +#line 243 "libxlu_disk_l.l"
> diff --git a/tools/libxl/libxlu_disk_l.h b/tools/libxl/libxlu_disk_l.h
> index 72b82cb..b0e6aa3 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 239 "libxlu_disk_l.l"
> +#line 243 "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 9fbc1ad..508bb5a 100644
> --- a/tools/libxl/libxlu_disk_l.l
> +++ b/tools/libxl/libxlu_disk_l.l
> @@ -216,7 +216,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);



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