[et-mgmt-tools] [PATCH] virtinst - virt-convert vmware output

[Joey Boggs]

Got it all figured out now, its set to raise an exception rather than fail()

Also changed the bus to ide rather than scsi.




Cole Robinson wrote:
> Joey Boggs wrote:
>   
>> John Levon wrote:
>>     
>>> On Mon, Sep 29, 2008 at 03:28:04PM -0400, Joey Boggs wrote:
>>>
>>>   
>>>       
>>>>> +import virtinst.ImageParser as ImageParser
>>>>> +from virtinst.cli import fail
>>>>>
>>>>> Surely this isn't right - this code is "library" code and shouldn't be
>>>>> using fail() ? It should be re-raising the exception...
>>>>>       
>>>>>           
>>>> Not sure about this, all the other apps in virtinst use fail() now in 
>>>> exceptions even virt-convert, or am I misunderstanding something?
>>>>     
>>>>         
>>> I just grepped and didn't see that. The only fail() usages are in
>>> virtinst/cli.py. That's right and proper: library code like that in
>>> virtconv/ (or most of virtinst/) should raise exceptions to allow the
>>> caller to decide the correct behaviour (if I'm a daemon, I'd better keep
>>> running; a GUI, I'd better bring up a dialog box, etc.).
>>>   
>>>       
>> Cole updated it only a few days ago, here's what I'm seeing at least:
>>
>>  grep -r "fail(" virt-convert
>>
>>         fail("Couldn't clean up output directory \"%s\": %s" %
>>         fail("Couldn't import file \"%s\": %s" %
>>         fail("Couldn't import file \"%s\": %s" % (options.input_file, e))
>>             fail("Could not create directory %s: %s" %
>>
>>     
>
> 'fail' should only be called from command line apps, hence you will
> see it in the virt-* utils and virtinst/cli.py. The reason it
> shouldn't be used anywhere else is because it forces the app to
> exit. You don't want to do that from a library (aka most of
> virtinst/* and all of virtconv/*) because it leaves the higher
> level apps no way to deal with the error.
>
>   
>>> It's a fine restriction, but it seems inconsistent: why are we assuming
>>> that virt-image import is using SCSI? Wouldn't a better default be IDE?
>>>
>>> regards
>>> john
>>>   
>>>       
>> Never caught that, I always assumed the disks were scsi, are they not? 
>> virt image isn't specific so I'm guessing we will need someone else to 
>> chime in and clarify this.
>>
>>     
>
> If there isn't any specific mention of IDE or SCSI in the
> virt-image file, then it will end up using virtinst's
> default, which is IDE for fullvirt guests and xvda for
> paravirt.
>
> Thanks,
> Cole
>
>   

-------------- next part --------------
A non-text attachment was scrubbed...
Name: virt-convert-vmware-conversion-module-9-29-1722.patch
Type: text/x-patch
Size: 8387 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/et-mgmt-tools/attachments/20080929/4db008ab/attachment.bin>