On Fri, Jul 23 2021, "wangyanan (Y)" <wangyana...@huawei.com> wrote:
> On 2021/7/23 16:02, Markus Armbruster wrote: >> Daniel P. Berrangé <berra...@redhat.com> writes: >> >>> On Thu, Jul 22, 2021 at 11:43:26PM +0800, Yanan Wang wrote: >>>> + /* >>>> + * The topology parameters must be specified equal to or great than >>>> one >>>> + * or just omitted, explicit configuration like "cpus=0" is not >>>> allowed. >>>> + */ >>>> + if ((config->has_cpus && config->cpus == 0) || >>>> + (config->has_sockets && config->sockets == 0) || >>>> + (config->has_dies && config->dies == 0) || >>>> + (config->has_cores && config->cores == 0) || >>>> + (config->has_threads && config->threads == 0) || >>>> + (config->has_maxcpus && config->maxcpus == 0)) { >>>> + error_setg(errp, "parameters must be equal to or greater than one >>>> if provided"); >>> I'd suggest a slight tweak since when seen it lacks context: >>> >>> $ ./qemu-system-x86_64 -smp 4,cores=0,sockets=2 >>> qemu-system-x86_64: parameters must be equal to or greater than one if >>> provided >>> >>> >>> error_setg(errp, "CPU topology parameters must be equal to or greater >>> than one if provided"); >> Let's scratch "if provided". >> >> I'd replace "must be equal to or greater than one" by "must be >> positive", or maybe "must be greater than zero". > How about we use "must be greater than zero" ? > After a grep search of these two sentences in QEMU, they both show up > in several places. "must be positive" always reports an invalid value that > is "< 0". While the check in this patch actually reject an invalid zero > value. Of the two, I'd prefer "greater than zero".