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



Thanks,

Guido

Reply via email to