[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

Since pretty much every cdrom drive use scsi today,  shouldnt the
readonly/writeable flag for MMC devices rather
be done down in the hw/scsi* code rather than the generic block code?

If/when/ever I get enough time I would like to port the writeable
dvd+r emulation I wrote for STGT to qemu.

In STGT, MMC/DVD devices can be either read-only or read-write.
If the filesize for the backing file is >0 bytes, it is assumed the
file is an iso image and that the file is a read-only iso image.
If filesize is ==0, then the file is opened read-write and is treated
as a "blank dvd+r disk that the initiator can write/burn to"

ronnie sahlberg

On Tue, Aug 7, 2012 at 6:47 PM, Markus Armbruster <armbru redhat com> wrote:
> Kevin Wolf <kwolf redhat com> writes:
>> 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.
> Correct.
>> 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.
> I can:
> $ qemu --enable-kvm -S -m 384 -vnc localhost:5500 -monitor stdio \
> -drive if=none,id=cd,readonly=on,format=raw \
> -device ide-cd,bus=ide.1,unit=0,drive=cd
> QEMU 1.1.50 monitor - type 'help' for more information
> (qemu) change cd r5.iso
> Could not open 'r5.iso'
> (qemu) q
> armbru blackfin:~/work/images$ ls -l r5.iso
> -r--r--r--. 1 armbru armbru 2872639488 Mar 31  2011 r5.iso
> Looks like a QEMU bug to me.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]