On Wed, Mar 05, 2014 at 06:12:41AM -0700, Eric Blake wrote: > 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. > I added the info into the commit message and pushed, thanks for the review. Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list