Jeff Cody <jc...@redhat.com> writes:
> On Wed, Aug 10, 2016 at 09:42:04AM +0200, Markus Armbruster wrote:
>> 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.
>
> Are these two patches intended for 2.7?
Back when I suggested this work, I had hoped it could still make 2.7 as
a followup fix, but 1. the bug has turned out to be merely latent , and
2. it's -rc3. I guess we need to punt them to 2.8.
