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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] [PATCH] xl: support empty CDROM devices"):
> On Wed, 2012-07-25 at 11:15 +0100, Ian Campbell wrote:
> > xl: support empty CDROM devices
> 
> It seems like I forgot to qrefresh before sending this.
> 
> I supect that given your updates these differences are now irrelevant
> but FTR the incremental bit which was omitted because of this was:

Thanks.

> diff -r 097bf63027e0 docs/misc/xl-disk-configuration.txt
> --- a/docs/misc/xl-disk-configuration.txt     Wed Jul 25 11:21:55 2012 +0100
> +++ b/docs/misc/xl-disk-configuration.txt     Wed Jul 25 17:00:00 2012 +0100
> @@ -91,8 +91,9 @@ Supported values:      raw, qcow, qcow2,
>  Deprecated values:     None
>  Default value:         raw
>  
> -Format cannot be specified as a positional parameter when target is
> -not provided (the empty CDROM case)
> +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.

This is no longer true in my patch.

>  expected <<EOF
> +EOF
> +one 255 ,,hdc:cdrom,r
> +
> +expected <<EOF

I have this already.

> diff -r 097bf63027e0 tools/libxl/libxlu_disk.c
> --- a/tools/libxl/libxlu_disk.c       Wed Jul 25 11:21:55 2012 +0100
> +++ b/tools/libxl/libxlu_disk.c       Wed Jul 25 17:00:00 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;
>      }

This is rather odd.  It appears to turn empty non-cdroms into RAW.  Is
that actually correct ?  It doesn't seem likely to me that it is.  I
think my new arrangements don't generate empty non-cdroms unless the
user explicitly specifies `empty' as the format or uses the xend
compatibility syntax and explicitly specifies `:disk'.

Ian.

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