[et-mgmt-tools] [PATCH] virt-manager validation and error reporting improvements

[Hugh Brock]

Cole Robinson wrote:
> Cole Robinson wrote:
>> Hugh Brock wrote:
>>>
>>> Needed to change "guest.os_type" to "self._guest.os_type" here and 
>>> below, works fine with that change.
>>>
>>
>> Your comment was actually at the removed portion of the code so I was 
>> confused for a minute, but yes, definitely change that.
>>
>>>
>>> NIT: Seems silly to copy the entire self._guest object here merely so 
>>> you can hang onto the name. Why not just "name = 
>>> self._guest.get_name()" followed later by "self._guest.name = name"?
>>>
>>
>> Whoops, good call.
>>
>>>
>>> Otherwise looks pretty good. If you agree to the changes above I will 
>>> apply it.
>>>
>>
>> I agree, apply at will. :)
>>
>> Thanks,
>> Cole
>>
> 
> Since we found some other errors, here's the (hopefully) final 
> incarnation of this patch.
> 
> Signed-off-by: Cole Robinson <crobinso at redhat.com>
> 
> Thanks,
> Cole
> 
> 

I have applied this, thanks!

However I think we could stand another small patch that tightens up the 
validation on the paravirt download URL. Right now it allows spaces and 
various other characters that are illegal in URLs...

Next step: localize the validation messages...

--Hugh

-- 
Red Hat Virtualization Group http://redhat.com/virtualization
Hugh Brock           | virt-manager http://virt-manager.org
hbrock at redhat.com    | virtualization library http://libvirt.org