On 2013-3-9 3:45, Chris Evich wrote:
Overall this LGTM, I made one optional suggestions and one observation
(re: err == "") below. Nice and simple test, I like it.
On 02/28/2013 03:02 AM, liyang wrote:
diff --git a/libvirt/tests/virsh_net_dumpxml.py
> ...
+ net_status_current = "active"
+ if virsh.net_list("--inactive"):
+ net_status_current = "inactive"
+
+ if net_status == "inactive" and net_status_current == "active":
+ virsh.net_destroy(net_name, ignore_status=True)
Not sure if it's a problem (I didn't try running code yet), but if the
network being tested is transient (i.e. came from net-create instead
of net-define), then stopping it won't make it "inactive" it will make
it *poof* dissapear (i.e. b/c it's not permanent).
Since no exit_status is verified (above) if this happens, then next
command is guaranteed to fail. If status_error="yes" then test will
pass under false-conditions (i.e. net_dumpxml failed for uncontrolled
reason).
Option/suggestion: Consider check & verify the net_destroy() was
successful, and to be extra-safe, maybe also check:
virsh.net_state_dict()[net_name]['persistent'] == True
Not critical though because this is probably a corner-case given a
controlled testing environment.
Thanks for reminding me, I will fix it...
+
+ result = virsh.net_dumpxml(vm_ref, extra, ignore_status=True)
+ status = result.exit_status
+ output = result.stdout.strip()
+ err = result.stderr.strip()
IIRC, virsh never uses stderr, always stdout.
+ if extra.find("--") != -1:
+ options = extra.split("--")
+ for option in options:
+ if option.strip() == "":
+ continue
+ if not virsh.has_command_help_match("net-dumpxml",
option.strip()):
+ status_error = "yes"
+ break
+
+ if net_status == "inactive" and net_status_current == "active":
+ virsh.net_start(net_name, ignore_status=True)
+
+ #check status_error
+ if status_error == "yes":
+ if status == 0 or err == "":
I think err will almost always == "" (see comment above). See this in
action: virsh.VirshSession.ERROR_REGEX_LIST and cmd_status_output().
Maybe we should make this a generally available function in virsh
module so tests can use it to check stdout for error messages? I'll
take a look at this more closely after the freeze and fix it as
necessary.
Hmm..in virsh.py, virsh command uses command() function,
session_id = dargs.get('session_id', None)
if session_id:
# Retrieve existing session
session = VirshSession(a_id=session_id)
else:
session = None
if session:
ret = session.cmd_result(cmd, ignore_status)
else:
ret = utils.run(cmd, verbose=debug, ignore_status=ignore_status)
I didn't pass any "session_id" in virsh.net_dumpxml(vm_ref, extra,
ignore_status=True),
so the ret which command() function returns will be utils.run()
function's result, and
it contains stdout and stderr..Is that right?
+ raise error.TestFail("Run successfully with wrong
command!")
+ elif status_error == "no":
+ if status != 0 or output == "":
+ raise error.TestFail("Run failed with right command")
--
Regards,
--------------------------------------------------
Li Yang
No. 6 Wenzhu Road, Nanjing, 210012, China
TEL：+86+25-86630566-8526
FUJITSU INTERNAL：7998-8526
FAX：+86+25-83317685
EMail：[email protected]
--------------------------------------------------
This communication is for use by the intended recipient(s) only and may contain
information that is privileged, confidential and exempt from disclosure under
applicable law. If you are not an intended recipient of this communication, you
are hereby notified that any dissemination, distribution or copying hereof is
strictly prohibited. If you have received this communication in error, please
notify me by reply e-mail, permanently delete this communication from your
system, and destroy any hard copies you may have printed
_______________________________________________
Virt-test-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/virt-test-devel
