Re: [PATCH v2] qemu: Reject unsupported tuning in session mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/05/2014 03:37 AM, Martin Kletzander wrote:
> On Tue, Mar 04, 2014 at 02:04:05PM -0700, Eric Blake wrote:
>> On 03/04/2014 07:15 AM, Martin Kletzander wrote:
>>> When domain is started with setting that cannot be done, i.e. those
>>> that require cgroups, there is no error reported and it succeeds
>>> without any message whatsoever.
>>>
>>> When setting with API, virsh, an error is reported, but only due to
>>> the fact that no cgroups are mounted (priv->cgroup == NULL).
>>>
>>> Given the above it seems reasonable to reject such unsupported
>>> settings.
>>>

>>
>> I can understand failing the Set commands if we can't change things; but
>> since we have a fallback for the Get command even for an offline domain,
>> shouldn't we stick to returning the defaults rather than erroring out?
>>
> 

> 
> If that's really needed, I'll rework it that way (even though it'll
> probably need to get changed back when user cgroups will be
> user-editable), but until then, could we make the error reported now:
> 
> $ virsh -c qemu:///session schedinfo dummy
> Scheduler      : Unknown
> error: Requested operation is not valid: cgroup CPU controller is not mounted
> 
> changed into:
> 
> $ virsh -c qemu:///session schedinfo dummy
> Scheduler      : Unknown
> error: Operation not supported: CPU tuning is not available in session mode

> // I should've added that info into the commit message, I just forgot.

Indeed, trading one error for another nicer one is always safe - and
mentioning it in the commit message highlights that it is an improvement
(we aren't causing an error on any situation where we used to pass).
ACK to this patch, then.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]