Re: [Qemu-devel] [PATCH v1 1/1] block/gluster: fix port type in the QAPI options list

[Jeff Cody]

On Wed, Aug 10, 2016 at 09:42:04AM +0200, Markus Armbruster wrote:
> Prasanna Kumar Kalever <address@hidden> 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 <address@hidden>
> 
> Commonly written as
> Suggested-by: Markus Armbruster <address@hidden>
> 
> > Signed-off-by: Prasanna Kumar Kalever <address@hidden>
> > ---
> >  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?