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

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

 



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

[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]