[virt-tools-list] [virt-manager PATCH] details: disable config disk with readonly and shareable at the same time

[Cole Robinson]

On 01/28/2014 10:16 AM, Chen Hanxiao wrote:
> At 2014-01-28 22:21:26,"Cole Robinson" <crobinso at redhat.com> wrote:
> 
>>On 01/28/2014 02:09 AM, Chen Hanxiao wrote:
>>> From: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
>>> 
>>> Currently we could config disk with readonly and
>>> shareable at the same time, which is meaningless.
>>> virsh had already discouraged users doing this.
>>> This patch will disable users to config both
>>> readonly and shareable at the same time by UI.
>>> 
>>
>>How does 'virsh' complain? If libvirt throws an error about this, I'd rather
>>just let it complain and show that error to the user, than reproduce their
>>error check, since this is a fairly minor corner case.
>>
> 
> virsh could not do this since commit:
> 
> f919cf691735535dedc66a2cae244350ebb6c5e5
> 
> virsh # attach-disk aa 1.img sdd --shareable --mode=readonly --print-xml
> error: option --mode already seen
> 
> If throwing errors is not acceptable, do we have some better metod to avoid this?
> 

That just looks like general virsh 'this option was specified twice on the
command line' warning.

The XML allows setting both shareable and readonly simultaneously, so I don't
see why virt-manager should disallow it. Even if the XML rejected that
combination, there's virtually no benefit to explicitly check for it in
virt-manager: we would just let libvirt throw the error.

Nowadays I try to avoid duplicating libvirt validation logic unless there's a
really good reason, because it just means more code in virt-manager for very
little gain.

Thanks,
Cole