Re: [PATCH] libxl: don't add cache mode for empty drives

[Jim Fehlig <jfehlig@suse.com>]

Roger Pau Monné wrote:
> On Thu, Apr 28, 2016 at 09:27:30AM +0100, George Dunlap wrote:
>> On Wed, Apr 27, 2016 at 5:22 PM, Jim Fehlig <jfehlig@suse.com> wrote:
>>> On 04/27/2016 01:38 AM, Roger Pau Monné wrote:
>>>> On Tue, Apr 26, 2016 at 10:35:31PM -0600, Jim Fehlig wrote:
>>>>> qemu commit 91a097e7 forbids specifying the cache mode for empty
>>>>> drives. Attempting to create a domain with an empty qdisk cdrom
>>>>> 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
>>>>>
>>>>> Change libxl to only emit cache mode when a cdrom target is specified.
>>>> What happens then when a cdrom is inserted? I cannot seem to find the code
>>>> in libxl_cdrom_insert that sets the cache mode.
>>> I cannot find it either. I suppose it would need to be setup via xenstore,
>>> similar to other options like feature_discard. But looking at
>>> $qemu-src/hw/block/xen_disk.c, it seems the XenBlkDev struct has no field to
>>> specify cache mode. Would qemu's xen_disk need to be extended to support cache
>>> mode, followed by a libxl patch to set the cache mode in xenstore?
>>>
>>>>  Is the default one used
>>>> then?
>>> Yes, the default cache mode (which is already writeback AIUI) would be used if
>>> not explicitly specified. Which brings up the option of removing
>>> 'cache=writeback' for cdroms altogether. Any opinion on that option?
>> What's the effective difference between caching modes for read-only
>> media anyway?
> 
> That's right, cdroms should always be read-only in which case the cache mode 
> doesn't matter. But I'm not sure if this is enforced in libxl.

xl-disk-configuration.txt states the default 'access=' value for cdrom devices
is readonly. It doesn't mention readonly is enforced for cdrom devices, but
xlu_disk_parse() in libxlutil unconditionally sets the disk's readwrite field to
0 for cdroms.

> IMHO, we should make sure ro is enforced with cdrom devices and then we can 
> use the default cache mode.

I'll send a V2 based on the above findings.

Regards,
Jim

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