[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 1/1] block/gluster: fix port type in the QAPI
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH v1 1/1] block/gluster: fix port type in the QAPI options list |
Date: |
Wed, 10 Aug 2016 09:04:09 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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?