Re: [PATCH V2] libxl: don't add cache mode for qdisk cdrom drives

[Wei Liu <wei.liu2@citrix.com>]

On Thu, Apr 28, 2016 at 03:20:46PM -0600, Jim Fehlig wrote:
> qemu commit 91a097e7 forbids specifying cache mode for empty
> drives. Attempting to create a domain with an empty qdisk cdrom
> drive results in
> 
> qemu-system-x86_64: -drive if=ide,index=1,readonly=on,media=cdrom,
>    cache=writeback,id=ide-832: Must specify either driver or file
> 

We need to fix this one way or another.

> libxl only allows an empty 'target=' for cdroms. By default, cdroms
> are readonly (see the 'access' parameter in xl-disk-configuration.txt)
> and forced to readonly by any tools (e.g. xl) using libxlutil's
> xlu_disk_parse() function. With cdroms always marked readonly,
> explicitly specifying the cache mode for cdrom drives can be dropped.
> The drive's 'readonly=on' option can also be set unconditionally.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>

CC Stefano who made the change to use writeback in
e9a327bbbcab127625b0917a2780cb3601a81d01 . Also CC Anthony.

Ian, Stefano and Anthony, do you have opinion on this patch?

> ---
> 
> V2:
> Drop explicitly setting cache mode since cdrom devices are
> readonly.
> Unconditionally add 'readonly=on' drive option for cdroms.
> 
>  tools/libxl/libxl_dm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index fd12844..959e292 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1368,8 +1368,8 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
>  
>              if (disks[i].is_cdrom) {
>                  drive = libxl__sprintf(gc,
> -                         "if=ide,index=%d,readonly=%s,media=cdrom,cache=writeback,id=ide-%i",
> -                         disk, disks[i].readwrite ? "off" : "on", dev_number);
> +                         "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
> +                         disk, dev_number);
>  
>                  if (target_path)
>                      drive = libxl__sprintf(gc, "%s,file=%s,format=%s",
> -- 
> 1.8.0.1
> 

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