Re: [PATCH v14 02/11] s390x/cpu topology: add topology entries on CPU hotplug

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

 





On 1/17/23 17:48, Nina Schoetterl-Glausch wrote:
On Tue, 2023-01-17 at 14:55 +0100, Pierre Morel wrote:

On 1/13/23 19:15, Nina Schoetterl-Glausch wrote:

[...]

+/**
+ * s390_topology_set_entry:
+ * @entry: Topology entry to setup
+ * @id: topology id to use for the setup
+ *
+ * Set the core bit inside the topology mask and
+ * increments the number of cores for the socket.
+ */
+static void s390_topology_set_entry(S390TopologyEntry *entry,

Not sure if I like the name, what it does is to add a cpu to the entry.

s390_topology_add_cpu_to_entry() ?

Yeah, that's better.

[...]

+/**
+ * s390_topology_set_cpu:
+ * @ms: MachineState used to initialize the topology structure on
+ *      first call.
+ * @cpu: the new S390CPU to insert in the topology structure
+ * @errp: the error pointer
+ *
+ * Called from CPU Hotplug to check and setup the CPU attributes
+ * before to insert the CPU in the topology.
+ */
+void s390_topology_set_cpu(MachineState *ms, S390CPU *cpu, Error **errp)
+{
+    Error *local_error = NULL;

Can't you just use ERRP_GUARD ?

I do not think it is necessary and I find it obfuscating.
So, should I?

/*
  * Propagate error object (if any) from @local_err to @dst_errp.
[...]
  * Please use ERRP_GUARD() instead when possible.
  * Please don't error_propagate(&error_fatal, ...), use
  * error_report_err() and exit(), because that's more obvious.
  */
void error_propagate(Error **dst_errp, Error *local_err);

So I'd say yes.

OK, you are right it is better.

Regards,
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