On Wed, May 22, 2013 at 3:58 PM, Guido Trotter <[email protected]> wrote:
> > > > On Wed, May 22, 2013 at 3:55 PM, Michele Tartara <[email protected]>wrote: > >> On Wed, May 22, 2013 at 3:29 PM, Guido Trotter <[email protected]>wrote: >> >>> >>> >>> >>> On Wed, May 22, 2013 at 3:00 PM, Michele Tartara <[email protected]>wrote: >>> >>>> On Wed, May 22, 2013 at 2:53 PM, Guido Trotter <[email protected]>wrote: >>>> >>>>> This has been reported by users, so we should have the extra debugging >>>>> available. >>>>> >>>>> Signed-off-by: Guido Trotter <[email protected]> >>>>> --- >>>>> lib/cmdlib.py | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/lib/cmdlib.py b/lib/cmdlib.py >>>>> index 0adf0f1..e600199 100644 >>>>> --- a/lib/cmdlib.py >>>>> +++ b/lib/cmdlib.py >>>>> @@ -7056,7 +7056,8 @@ def _ExpandCheckDisks(instance, disks): >>>>> else: >>>>> if not set(disks).issubset(instance.disks): >>>>> raise errors.ProgrammerError("Can only act on disks belonging >>>>> to the" >>>>> - " target instance") >>>>> + " target instance: %r versus %r" % >>>>> + (disks, instance.disks)) >>>>> >>>> >>>> I think it's not really clear which one is correct and which one is >>>> not, in that message. What about something like this? >>>> >>>> "Can only act on disks belonging to the target instance. Expected a >>>> subset of %s, but received %s" % (instance.disks, disks) >>>> >>>> >>> I thought so, but then I thought it didn't matter (you would be looking >>> at the code anyway when debugging it). >>> But thanks for the suggestion, I will apply it. >>> >>> diff --git a/lib/cmdlib.py b/lib/cmdlib.py >>> index e600199..5795d4d 100644 >>> --- a/lib/cmdlib.py >>> +++ b/lib/cmdlib.py >>> @@ -7056,8 +7056,8 @@ def _ExpandCheckDisks(instance, disks): >>> else: >>> if not set(disks).issubset(instance.disks): >>> raise errors.ProgrammerError("Can only act on disks belonging to >>> the" >>> - " target instance: %r versus %r" % >>> - (disks, instance.disks)) >>> + " target instance: expected a subset >>> of %r," >>> + " got %r" % (disks, instance.disks)) >>> >>> >> disks and instance.disks are inverted. >> >> > Right, thanks for catching it: > diff --git a/lib/cmdlib.py b/lib/cmdlib.py > index 5795d4d..5c4628e 100644 > --- a/lib/cmdlib.py > +++ b/lib/cmdlib.py > @@ -7057,7 +7057,7 @@ def _ExpandCheckDisks(instance, disks): > if not set(disks).issubset(instance.disks): > raise errors.ProgrammerError("Can only act on disks belonging to > the" > " target instance: expected a subset > of %r," > - " got %r" % (disks, instance.disks)) > + " got %r" % (instance.disks, disks)) > return disks > LGTM, thanks. Michele
