[libvirt] [PATCH] qemu: raise an error when trying to use readonly ide disks

[Martin Kletzander]

On Tue, Jul 01, 2014 at 09:40:00AM +0200, Giuseppe Scrivano wrote:
>Martin Kletzander <mkletzan at redhat.com> writes:
>
>> If the user updates from QEMU without DRIVE_READONLY to newer one,
>> that supports that flag, than XML with readonly IDE HDD will stop
>> working even though it worked before the update *as requested*.  That
>> readonly flag does not reflect how the disk is exposed in the guest;
>> as you said IDE does not have any readonly concept, that is only how
>> the device reacts to writes.
>>
>> Changing the behaviour to:
>>
>> if (disk->readonly &&
>>    virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY) &&
>>    !(disk->bus == VIR_DOMAIN_DISK_BUS_IDE &&
>>      disk->device == VIR_DOMAIN_DISK_DEVICE_DISK))
>>    virBufferAddLit(&opt, ",readonly=on");
>>
>>
>> would keep the backward compatibility.  This behaviour makes more
>> sense to me.
>
>this behaves in a quite different way that my proposed patch but if
><readonly/> affects also the SELinux label and we allow the process to
>run anyway by skipping readonly=on for IDE disks, wouldn't qemu fail in
>weird ways when trying to write to the file?
>

My case was that it doesn't fail in a new way if you update from older
qemu that didn't support the DRIVE_READONLY capability.  However it
would probably fail differently when there is readonly=on because it
would not report an error in the guest.

Anyway, thinking about it, qemu doesn't start for some versions of
libvirt already with these setting, this just changes the error to a
more readable one.  So given the circumstances such backward
compatibility is not such huge issue (since we already broke it).  I,
therefore, agree with Dan's ACK.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140701/2277931a/attachment-0001.sig>