Prasanna Kumar Kalever <prasanna.kale...@redhat.com> writes:
> After introduction of qapi schema in gluster block driver code, the port
> type is now string as per InetSocketAddress
>
> { 'struct': 'InetSocketAddress',
> 'data': {
> 'host': 'str',
> 'port': 'str',
> '*to': 'uint16',
> '*ipv4': 'bool',
> '*ipv6': 'bool' } }
>
> but the current code still treats it as QEMU_OPT_NUMBER, hence fixing port
> to accept QEMU_OPT_STRING.
>
> Credits: Markus Armbruster <arm...@redhat.com>
Commonly written as
Suggested-by: Markus Armbruster <arm...@redhat.com>
> Signed-off-by: Prasanna Kumar Kalever <prasanna.kale...@redhat.com>
> ---
> block/gluster.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index edde1ad..e6afa48 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -161,7 +161,7 @@ static QemuOptsList runtime_tcp_opts = {
> },
> {
> .name = GLUSTER_OPT_PORT,
> - .type = QEMU_OPT_NUMBER,
> + .type = QEMU_OPT_STRING,
> .help = "port number on which glusterd is listening (default
> 24007)",
> },
> {
The difference between QEMU_OPT_NUMBER and QEMU_OPT_STRING:
* The string value is stored for both. For QEMU_OPT_NUMBER, we
additionally parse the string as decimal number (this can fail,
obviously), and store the result as uint64_t. See qemu_opt_parse().
* qemu_opt_get() & friends return the stored string for both.
* qemu_opt_get_number() & friends require QEMU_OPT_NUMBER and return the
stored number.
* qemu_opts_print() prints the stored string (with comma doubled) for
QEMU_OPT_STRING, and the stored number for QEMU_OPT_NUMBER.
Your patch works, because:
* We get the value only with qemu_opt_get(). The only effect we get
from QEMU_OPT_NUMBER is qemu_opt_parse() failure.
* "[PATCH v2 1/1] block/gluster: improve defense over string to int
conversion" fixes the conversion port string to port number to detect
errors. With QEMU_OPT_NUMBER, this can't actually fail, because
qemu_opt_parse() fails first. With QEMU_OPT_STRING, it can.
The commit message should explain this.
I'd squash the two patches together, because a decent commit message for
the squash will probably be simpler than separate ones.
