Re: [PATCH 3/3] virsh: Prohibit all clear cpumap

[Osier Yang <jyang@xxxxxxxxxx>]

On 02/04/13 18:53, John Ferlan wrote:
> On 04/02/2013 01:42 AM, Osier Yang wrote:
> > This prohibits all clear cpumap eariler in virsh, for both vcpupin
> > and emulatorpin.
> > ---
> >   tools/virsh-domain.c | 35 ++++++++++++++++++++++++++++++-----
> >   1 file changed, 30 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 965f92c..8d3d63f 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -5552,6 +5552,20 @@ cleanup:
> >   }
> >   
> >   static bool
> > +vshIsCpumapAllClear(const unsigned char *cpumap,
> > +                    int maxcpu)
> > +{
> > +    int i;
> > +
> > +    for (i = 0; i < maxcpu; i++) {
> > +        if (VIR_CPU_USED(cpumap, i))
> > +            return false;
> > +    }
> > +
> > +    return true;
> > +}
> > +
> > +static bool
> >   cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
> >   {
> >       virDomainInfo info;
> > @@ -5620,7 +5634,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
> >   
> >       cpumaplen = VIR_CPU_MAPLEN(maxcpu);
> >   
> > -    /* Query mode: show CPU affinity information then exit.*/
> > +    /* Query mode: show CPU affinity information then exit. */
> >       if (query) {
> >           /* When query mode and neither "live", "config" nor "current"
> >            * is specified, set VIR_DOMAIN_AFFECT_CURRENT as flags */
> > @@ -5649,10 +5663,15 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
> >           goto cleanup;
> >       }
> >   
> > -    /* Pin mode: pinning specified vcpu to specified physical cpus*/
> > +    /* Pin mode: pinning specified vcpu to specified physical cpus. */
> >       if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen)))
> >           goto cleanup;
> >   
> > +    if (vshIsCpumapAllClear(cpumap, maxcpu)) {
> > +        vshError(ctl, "%s", _("No CPU specified"));
> > +        goto cleanup;
> > +    }
> > +
> >       if (flags == -1) {
> >           if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) != 0)
> >               goto cleanup;
> > @@ -5755,10 +5774,11 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
> >   
> >       cpumaplen = VIR_CPU_MAPLEN(maxcpu);
> >   
> > -    /* Query mode: show CPU affinity information then exit.*/
> > +    /* Query mode: show CPU affinity information then exit. */
> >       if (query) {
> >           /* When query mode and neither "live", "config" nor "current"
> > -         * is specified, set VIR_DOMAIN_AFFECT_CURRENT as flags */
> > +         * is specified, set VIR_DOMAIN_AFFECT_CURRENT as flags.
> > +         */
> >           if (flags == -1)
> >               flags = VIR_DOMAIN_AFFECT_CURRENT;
> >   
> > @@ -5775,10 +5795,15 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
> >           goto cleanup;
> >       }
> >   
> > -    /* Pin mode: pinning emulator threads to specified physical cpus*/
> > +    /* Pin mode: pinning emulator threads to specified physical cpus. */
> >       if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen)))
> >           goto cleanup;
> >   
> > +    if (vshIsCpumapAllClear(cpumap, maxcpu)) {
> > +        vshError(ctl, "%s", _("No CPU is specified"));
> > +        goto cleanup;
> > +    }
> > +
> >       if (flags == -1)
> >           flags = VIR_DOMAIN_AFFECT_LIVE;
> >   
> Unlike the virReportError() which provides the function trace - how
> would one know which of the two functions had the error here?  OK other
> than looking at the code and noting one error has an "is" verb and the
> other doesn't...

Something like "vcpupin: No CPU is specified" should be clear
enough to tell which function throws the error. But what I was
thinking is the prefix "vcpupin:" is redundant.  As one must known
he is executing a "vcpupin" command. E.g.

% virsh vcpupin toy 0 0,^0
error: No CPU specified

> One message could indicate "No domain vCPU affinity" while the other
> could indicate "No domain emulator affinity"...
>
> John
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

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