Re: [Qemu-devel] [PATCH] block: Set cdrom device read only flag

[Kevin Wolf]

Am 02.08.2012 09:20, schrieb Kevin Shanahan:
> On Thu, Aug 02, 2012 at 02:49:52PM +0930, Kevin Shanahan wrote:
>> On Thu, Aug 02, 2012 at 11:46:13AM +0930, Kevin Shanahan wrote:
>>> On Thu, Aug 02, 2012 at 11:02:42AM +0930, Kevin Shanahan wrote:
>>>> Set the block driver read_only flag for cdrom devices so that
>>>> qmp_change_blockdev does not attempt to open cdrom files in read-write
>>>> mode when changing media.
>>>
>>> Hrm, this fixes my simple test case using the kvm monitor directly but
>>> changing media via libvirt still has the same issue (fails for RO
>>> files, succeeds for writable files).
>>>
>>> $ virsh attach-disk --type cdrom --mode readonly test1 
>>> /srv/kvm/iso/ubuntu-12.04-server-amd64.iso hdc
>>> error: Failed to attach disk
>>> error: internal error unable to execute QEMU command 'change': Could not 
>>> open '/srv/kvm/iso/ubuntu-12.04-server-amd64.iso'
>>>
>>> I'll keep looking into it.
>>
>> In the libvirt case, it seems libvirt is failing to add media=cdrom to
>> the commandline, so in this case qemu is defaulting to media=disk and
>> my proposed fix has no effect. Diving into libvirt now to see why no
>> media=disk is getting added...
>>
>> Common test case has this xml (generated by virt-install):
>>
>>     <disk type='block' device='cdrom'>
>>       <driver name='qemu' type='raw'/>
>>       <target dev='hdc' bus='ide'/>
>>       <readonly/>
>>       <alias name='ide0-1-0'/>
>>       <address type='drive' controller='0' bus='1' target='0' unit='0'/>
>>     </disk>
> 
> Ok, looks like libvirt is intentionally leaving media=cdrom off the
> command line in the case that "-device ide-cd,..." is
> supported. Presumably by specifying the device this way, qemu is
> supposed to work out that the media type is cdrom automatically (but
> it doesn't, it defaults to disk).
> 
> Libvirt wants to use:
> 
> qemu-kvm ... \
>      -drive if=none,id=drive-ide0-1-0,readonly=on,format=raw \
>      -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 ...
> 
> If I hack qemu/qemu_command.c::qemuBuildDriveStr() to ignore the check
> for QEMU_CAPS_IDE_CD and always add media=cdrom, then (with my qemu as
> well patch) qemu will open cdrom media files read-only.
> 
> There's probably a neater way to just get qemu to set the media type
> if "-device ide-cd,..." is used, but I haven't worked it out yet.
> 
> Anyway, apologies for the rambling conversation with myself on your
> lists. Hope this is helpful in some way.

Thanks, that's some good information.

However, I don't think you should start from media=cdrom. libvirt
already does specify readonly=on and that is the property you're really
interested in. Since commit 528f7663 (released with 0.13) the 'change'
command should keep the read-only flag for all kinds of media.

Now I'm not sure where you lost the read-only flag. At least on git
master it doesn't seem to reproduce for me.

Kevin