Re: [kvm-unit-tests PATCH v1 2/5] s390x: css: simplifications of the tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/1/21 11:11 AM, Janosch Frank wrote:
On 1/29/21 3:34 PM, Pierre Morel wrote:

...snip...

diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
index fe05021..f300969 100644
--- a/lib/s390x/css_lib.c
+++ b/lib/s390x/css_lib.c
@@ -118,6 +118,31 @@ out:
  	return schid;
  }
+/*
+ * css_enable: enable the subchannel with the specified ISC

enabled or enable?

Forgot to change the cut and paste :(

/*
 * css_enabled: report if the sub channel is enabled


I.e. do you test if it is enabled or do you want to enable it.

+ * @schid: Subchannel Identifier
+ * Return value:
+ *   true if the subchannel is enabled
+ *   false otherwise
+ */
+bool css_enabled(int schid)
+{
+	struct pmcw *pmcw = &schib.pmcw;
+	int cc;
+
+	cc = stsch(schid, &schib);
+	if (cc) {
+		report_info("stsch: updating sch %08x failed with cc=%d",
+			    schid, cc);
+		return false;
+	}
+
+	if (!(pmcw->flags & PMCW_ENABLE)) {
+		report_info("stsch: sch %08x not enabled", schid);
+		return 0;

Please stay with true/false or change the return type to int and use ints.

yes


+	}
+	return true;
+}

...snip...

@@ -129,16 +120,21 @@ static void test_sense(void)
  	report_info("reserved 0x%02x cu_type 0x%04x cu_model 0x%02x dev_type 0x%04x dev_model 0x%02x",
  		    senseid->reserved, senseid->cu_type, senseid->cu_model,
  		    senseid->dev_type, senseid->dev_model);
+	report_info("cu_type expected 0x%04x got 0x%04x", (uint16_t)cu_type,
+		    senseid->cu_type);
- report(senseid->cu_type == cu_type, "cu_type expected 0x%04x got 0x%04x",
-	       (uint16_t)cu_type, senseid->cu_type);
+	retval = senseid->cu_type == cu_type;
error:
  	free_io_mem(ccw, sizeof(*ccw));
  error_ccw:
  	free_io_mem(senseid, sizeof(*senseid));
-error_senseid:
-	unregister_io_int_func(css_irq_io);
+	return retval;

Could you return senseid->cu_type == cu_type here?

It would work for the current code and DEFAULT_CU_TYPE.
But I do not think it is a good idea due to the goto we have in error case before I find that it makes the code less understandable.

Other opinion?

Thanks for the comments,
Pierre

--
Pierre Morel
IBM Lab Boeblingen



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux