On 01/28/2014 10:16 AM, Chen Hanxiao wrote: > At 2014-01-28 22:21:26,"Cole Robinson" <crobinso@xxxxxxxxxx> wrote: > >>On 01/28/2014 02:09 AM, Chen Hanxiao wrote: >>> From: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx> >>> >>> 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 _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
