Re: [PATCH v7 6/8] block: make BlockConf size props 32bit and accept size suffixes

From: Eric Blake <eblake@redhat.com>
To: Roman Kagan <rvkagan@yandex-team.ru>, qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>, "Fam Zheng" <fam@euphon.net>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	qemu-block@nongnu.org, "Paul Durrant" <paul@xen.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"Max Reitz" <mreitz@redhat.com>,
	"Anthony Perard" <anthony.perard@citrix.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Keith Busch" <kbusch@kernel.org>,
	xen-devel@lists.xenproject.org, "John Snow" <jsnow@redhat.com>
Subject: Re: [PATCH v7 6/8] block: make BlockConf size props 32bit and accept size suffixes
Date: Thu, 28 May 2020 16:53:14 -0500	[thread overview]
Message-ID: <0439aa5e-413c-cf7e-83b7-1e942a882f5a@redhat.com> (raw)
In-Reply-To: <20200528213946.1636444-7-rvkagan@yandex-team.ru>

On 5/28/20 4:39 PM, Roman Kagan wrote:
> Convert all size-related properties in BlockConf to 32bit.  This will
> allow to accomodate bigger block sizes (in a followup patch).

s/allow to accomodate/accommodate/

> This also allows to make them all accept size suffixes, either via
> DEFINE_PROP_BLOCKSIZE or via DEFINE_PROP_SIZE32.
> 
> Also, since min_io_size is exposed to the guest by scsi and virtio-blk
> devices as an uint16_t in units of logical blocks, introduce an
> additional check in blkconf_blocksizes to prevent its silent truncation.
> 
> Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
> ---

> +    if (conf->min_io_size / conf->logical_block_size > UINT16_MAX) {
> +        error_setg(errp,
> +                   "min_io_size must not exceed " stringify(UINT16_MAX)
> +                   " logical blocks");

On my libc, this results in "must not exceed (65535) logical blocks".

Worse, I could envision a platform where it prints something funky like:

"exceed (2 * (32768) + 1) logical", based on however complex the 
definition of UINT16_MAX is.  You're better off printing this one with 
%d than with stringify().

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org