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

[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.

Cheers,
Kevin.


--- libvirt-0.9.13.orig/src/qemu/qemu_command.c.orig    2012-08-02 
16:45:25.000000000 +0930
+++ libvirt-0.9.13.orig/src/qemu/qemu_command.c 2012-08-02 16:46:11.000000000 
+0930
@@ -2082,7 +2082,7 @@
             if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_CD))
                 virBufferAddLit(&opt, ",media=cdrom");
         } else if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE) {
-            if (!qemuCapsGet(qemuCaps, QEMU_CAPS_IDE_CD))
+            //if (!qemuCapsGet(qemuCaps, QEMU_CAPS_IDE_CD))
                 virBufferAddLit(&opt, ",media=cdrom");
         } else {
             virBufferAddLit(&opt, ",media=cdrom");