| Message ID | 1461878446-7676-1-git-send-email-jfehlig@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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@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? 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@lists.xen.org > http://lists.xen.org/xen-devel >
On 05/10/2016 04:06 AM, Stefano Stabellini wrote: > 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@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? > 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. Wei, Will this patch make 4.7? It allows empty cdroms with freshly released QEMU 2.6 :-). Regards, Jim
On Fri, May 13, 2016 at 02:36:18PM -0600, Jim Fehlig wrote: > On 05/10/2016 04:06 AM, Stefano Stabellini wrote: > > 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@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? > > 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. > > Wei, > > Will this patch make 4.7? It allows empty cdroms with freshly released QEMU 2.6 :-). > Yes, it's on my radar. I wanted to give some time for other people to look at it. Now two weeks has passed, I've queued it up for committing. Wei. > Regards, > Jim >
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",
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 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> --- 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(-)