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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 9 May 2016, Wei Liu wrote:
> 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@xxxxxxxx>
> 
> 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?

It looks good to me. We could consider to backport it, given that we
only have a loose relationship with QEMU and older versions of Xen could
end up running with newer versions of QEMU.


> > 
> > 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@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
> 

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