Hi~

>> +def virsh_attach_detach_interface(name, command, type, extra = "", uri = 
>> ""):
> I don't suggest you use a function interface to deal with these 2 virsh 
> operations,
> it will be hard to maintain if attach-interface and detach-interface have many
> change in the future.

OK, you are right.
At first, I wrote 2 functions. But they are just too similar. So I put them 
together. 
I will separate them again, and send a V2 patch set. Thanks.:)

>> +    try:
>> +        virsh_cmd(cmd, uri)
>> +        return True
>> +    except error.CmdError:
>> +        logging.error("Attaching interface to VM %s failed." % name)
> It's incorrect, because you define a comment function, including detaching 
> interface,
> you had better to save virsh command output information then directly write 
> log file
> instead of repeatedly define info/error information yourself.

Hum, I missed it. I will also log the exception detail. Like the fallowing:
except error.CmdError, detail:
    logging.error("Attaching interface to %s failed.\n%s" % (name, detail))

Thanks. :)

-- 
Best Regards,
Tang chen
_______________________________________________
Autotest mailing list
Autotest@test.kernel.org
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to