Patchwork block: Set cdrom device read only flag

login
register
mail settings
Submitter Kevin Shanahan
Date Aug. 2, 2012, 1:32 a.m.
Message ID <20120802013242.GA18298@tuon.disenchant.local>
Download mbox | patch
Permalink /patch/174635/
State New
Headers show

Comments

Kevin Shanahan - Aug. 2, 2012, 1:32 a.m.
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.

Signed-off-by: Kevin Shanahan <kmshanah@disenchant.net>
---
Proposed fix for https://bugs.launchpad.net/qemu/+bug/1027525
Kevin Shanahan - Aug. 2, 2012, 2:16 a.m.
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.

Cheers,
Kevin.
Kevin Shanahan - Aug. 2, 2012, 5:19 a.m.
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>

> Cheers,
> Kevin.

Patch

diff -urN qemu-kvm-1.1.1.orig/blockdev.c qemu-kvm-1.1.1/blockdev.c
--- qemu-kvm-1.1.1.orig/blockdev.c	2012-07-16 17:22:03.000000000 +0930
+++ qemu-kvm-1.1.1/blockdev.c	2012-08-02 10:28:40.000000000 +0930
@@ -565,6 +565,7 @@ 
 	    break;
 	case MEDIA_CDROM:
             dinfo->media_cd = 1;
+            dinfo->bdrv->read_only = 1;
 	    break;
 	}
         break;