| Message ID | 1349103144-6827-3-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/01/2012 08:52 AM, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > qapi-schema.json | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file modificato, 53 inserzioni(+) > > diff --git a/qapi-schema.json b/qapi-schema.json > index 191d921..b443a99 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -2367,6 +2367,59 @@ > 'opts': 'NetClientOptions' } } > > ## > +# @IPSocketAddress > +# > +# Captures a range of possible destination addresses for an IP socket > +# > +# @host: host part of the address > +# > +# @port: port part of the address, or lowest port if @to is present > +# > +# @to: highest port to try > +# > +# @ipv4: whether to accept IPv4 addresses, default try both IPv4 and IPv6 > +# #optional > +# > +# @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6 > +# #optional > +# > +# Since 1.3 > +## > +{ 'type': 'IPSocketAddress', > + 'data': { > + 'host': 'str', > + 'port': 'str', > + '*to': 'uint16', I still think it's a bit weird to have: 'host':'localhost', 'port':'1000', 'to':1001 for a port range, all because of the possibility of named ports; should '*to' be a 'str' if only for symmetry in the output? But it's bike-shedding, so I'll live with whatever works (that is, I'm not requesting a v3 on this patch). > +## > +# @SocketAddress > +# > +# Captures the address of a socket, which could also be a named file descriptor > +# > +# Since 1.3 > +## > +{ 'union': 'SocketAddress', > + 'data': { > + 'inet': 'IPSocketAddress', > + 'unix': 'UnixSocketAddress', > + 'fd': 'String' } } I guess you had to use the String wrapper instead of 'str'; but a bit of reading in the file showed that this part is at least correct. Reviewed-by: Eric Blake <eblake@redhat.com>
Il 02/10/2012 01:56, Eric Blake ha scritto: > On 10/01/2012 08:52 AM, Paolo Bonzini wrote: >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> qapi-schema.json | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file modificato, 53 inserzioni(+) >> >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 191d921..b443a99 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -2367,6 +2367,59 @@ >> 'opts': 'NetClientOptions' } } >> >> ## >> +# @IPSocketAddress >> +# >> +# Captures a range of possible destination addresses for an IP socket >> +# >> +# @host: host part of the address >> +# >> +# @port: port part of the address, or lowest port if @to is present >> +# >> +# @to: highest port to try >> +# >> +# @ipv4: whether to accept IPv4 addresses, default try both IPv4 and IPv6 >> +# #optional >> +# >> +# @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6 >> +# #optional >> +# >> +# Since 1.3 >> +## >> +{ 'type': 'IPSocketAddress', >> + 'data': { >> + 'host': 'str', >> + 'port': 'str', >> + '*to': 'uint16', > > I still think it's a bit weird to have: > > 'host':'localhost', 'port':'1000', 'to':1001 > > for a port range, all because of the possibility of named ports; should > '*to' be a 'str' if only for symmetry in the output? But it's > bike-shedding, so I'll live with whatever works (that is, I'm not > requesting a v3 on this patch). Would it be better if I changed 'to' to 'count'? >> +## >> +# @SocketAddress >> +# >> +# Captures the address of a socket, which could also be a named file descriptor >> +# >> +# Since 1.3 >> +## >> +{ 'union': 'SocketAddress', >> + 'data': { >> + 'inet': 'IPSocketAddress', >> + 'unix': 'UnixSocketAddress', >> + 'fd': 'String' } } > > I guess you had to use the String wrapper instead of 'str'; but a bit of > reading in the file showed that this part is at least correct. It's not absolutely necessary, but I preferred it that way. Paolo
On 10/02/2012 03:00 AM, Paolo Bonzini wrote: > Il 02/10/2012 01:56, Eric Blake ha scritto: >> On 10/01/2012 08:52 AM, Paolo Bonzini wrote: >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> --- >>> +# >>> +# @port: port part of the address, or lowest port if @to is present >>> +# >>> +# @to: highest port to try >>> +# >>> +{ 'type': 'IPSocketAddress', >>> + 'data': { >>> + 'host': 'str', >>> + 'port': 'str', >>> + '*to': 'uint16', >> >> I still think it's a bit weird to have: >> >> 'host':'localhost', 'port':'1000', 'to':1001 >> >> for a port range, all because of the possibility of named ports; should >> '*to' be a 'str' if only for symmetry in the output? But it's >> bike-shedding, so I'll live with whatever works (that is, I'm not >> requesting a v3 on this patch). > > Would it be better if I changed 'to' to 'count'? That does look a little better: 'host':'localhost', 'port':'1000', 'count':2 for the 2-port range 1000-1001. But it's all the same information, so I'm not strongly tied to any particular representation, as long as libvirt can parse it when querying and produce it when starting NBD.
On Tue, 02 Oct 2012 05:39:52 -0600 Eric Blake <eblake@redhat.com> wrote: > On 10/02/2012 03:00 AM, Paolo Bonzini wrote: > > Il 02/10/2012 01:56, Eric Blake ha scritto: > >> On 10/01/2012 08:52 AM, Paolo Bonzini wrote: > >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > >>> --- > > >>> +# > >>> +# @port: port part of the address, or lowest port if @to is present > >>> +# > >>> +# @to: highest port to try > >>> +# > > >>> +{ 'type': 'IPSocketAddress', > >>> + 'data': { > >>> + 'host': 'str', > >>> + 'port': 'str', > >>> + '*to': 'uint16', > >> > >> I still think it's a bit weird to have: > >> > >> 'host':'localhost', 'port':'1000', 'to':1001 > >> > >> for a port range, all because of the possibility of named ports; should > >> '*to' be a 'str' if only for symmetry in the output? But it's > >> bike-shedding, so I'll live with whatever works (that is, I'm not > >> requesting a v3 on this patch). > > > > Would it be better if I changed 'to' to 'count'? > > That does look a little better: > > 'host':'localhost', 'port':'1000', 'count':2 > > for the 2-port range 1000-1001. But it's all the same information, so > I'm not strongly tied to any particular representation, as long as > libvirt can parse it when querying and produce it when starting NBD. Wouldn't it be cleaner to pass a list of port numbers? We could have: *port-list: [ 'int' ] *service: 'str' Or, we could make this an union.
On Mon, 1 Oct 2012 16:52:17 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Made a suggestion for this one that I think it's worth doing, but as Eric says I'll avoid bike shed, so: Acked-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > qapi-schema.json | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file modificato, 53 inserzioni(+) > > diff --git a/qapi-schema.json b/qapi-schema.json > index 191d921..b443a99 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -2367,6 +2367,59 @@ > 'opts': 'NetClientOptions' } } > > ## > +# @IPSocketAddress > +# > +# Captures a range of possible destination addresses for an IP socket > +# > +# @host: host part of the address > +# > +# @port: port part of the address, or lowest port if @to is present > +# > +# @to: highest port to try > +# > +# @ipv4: whether to accept IPv4 addresses, default try both IPv4 and IPv6 > +# #optional > +# > +# @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6 > +# #optional > +# > +# Since 1.3 > +## > +{ 'type': 'IPSocketAddress', > + 'data': { > + 'host': 'str', > + 'port': 'str', > + '*to': 'uint16', > + '*ipv4': 'bool', > + '*ipv6': 'bool' } } > + > +## > +# @UnixSocketAddress > +# > +# Captures the destination address of a Unix socket > +# > +# @path: filesystem path to use > +# > +# Since 1.3 > +## > +{ 'type': 'UnixSocketAddress', > + 'data': { > + 'path': 'str' } } > + > +## > +# @SocketAddress > +# > +# Captures the address of a socket, which could also be a named file descriptor > +# > +# Since 1.3 > +## > +{ 'union': 'SocketAddress', > + 'data': { > + 'inet': 'IPSocketAddress', > + 'unix': 'UnixSocketAddress', > + 'fd': 'String' } } > + > +## > # @getfd: > # > # Receive a file descriptor via SCM rights and assign it a name
Il 02/10/2012 14:27, Luiz Capitulino ha scritto: >>>> > >> for a port range, all because of the possibility of named ports; should >>>> > >> '*to' be a 'str' if only for symmetry in the output? But it's >>>> > >> bike-shedding, so I'll live with whatever works (that is, I'm not >>>> > >> requesting a v3 on this patch). >>> > > >>> > > Would it be better if I changed 'to' to 'count'? >> > >> > That does look a little better: >> > >> > 'host':'localhost', 'port':'1000', 'count':2 >> > >> > for the 2-port range 1000-1001. But it's all the same information, so >> > I'm not strongly tied to any particular representation, as long as >> > libvirt can parse it when querying and produce it when starting NBD. > Wouldn't it be cleaner to pass a list of port numbers? We could have: > > *port-list: [ 'int' ] > *service: 'str' A list of ports doesn't work too well for say 5900-5999. I think the port + count is the simplest. Paolo
On Tue, 02 Oct 2012 16:24:48 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 02/10/2012 14:27, Luiz Capitulino ha scritto: > >>>> > >> for a port range, all because of the possibility of named ports; should > >>>> > >> '*to' be a 'str' if only for symmetry in the output? But it's > >>>> > >> bike-shedding, so I'll live with whatever works (that is, I'm not > >>>> > >> requesting a v3 on this patch). > >>> > > > >>> > > Would it be better if I changed 'to' to 'count'? > >> > > >> > That does look a little better: > >> > > >> > 'host':'localhost', 'port':'1000', 'count':2 > >> > > >> > for the 2-port range 1000-1001. But it's all the same information, so > >> > I'm not strongly tied to any particular representation, as long as > >> > libvirt can parse it when querying and produce it when starting NBD. > > Wouldn't it be cleaner to pass a list of port numbers? We could have: > > > > *port-list: [ 'int' ] > > *service: 'str' > > A list of ports doesn't work too well for say 5900-5999. I think the > port + count is the simplest. True, but consider making it a union then, so that we can have service as a string and port as an integer.
Il 02/10/2012 17:27, Luiz Capitulino ha scritto: >>> > > Wouldn't it be cleaner to pass a list of port numbers? We could have: >>> > > >>> > > *port-list: [ 'int' ] >>> > > *service: 'str' >> > >> > A list of ports doesn't work too well for say 5900-5999. I think the >> > port + count is the simplest. > True, but consider making it a union then, so that we can have service > as a string and port as an integer. But then how would you name the containing union? In the end this is a hack for VNC or little more... any change will propagate all over the place due to QemuOpts and qemu-char using "to". It is an optional argument, I don't think it's worth much time. :) Paolo
diff --git a/qapi-schema.json b/qapi-schema.json index 191d921..b443a99 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2367,6 +2367,59 @@ 'opts': 'NetClientOptions' } } ## +# @IPSocketAddress +# +# Captures a range of possible destination addresses for an IP socket +# +# @host: host part of the address +# +# @port: port part of the address, or lowest port if @to is present +# +# @to: highest port to try +# +# @ipv4: whether to accept IPv4 addresses, default try both IPv4 and IPv6 +# #optional +# +# @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6 +# #optional +# +# Since 1.3 +## +{ 'type': 'IPSocketAddress', + 'data': { + 'host': 'str', + 'port': 'str', + '*to': 'uint16', + '*ipv4': 'bool', + '*ipv6': 'bool' } } + +## +# @UnixSocketAddress +# +# Captures the destination address of a Unix socket +# +# @path: filesystem path to use +# +# Since 1.3 +## +{ 'type': 'UnixSocketAddress', + 'data': { + 'path': 'str' } } + +## +# @SocketAddress +# +# Captures the address of a socket, which could also be a named file descriptor +# +# Since 1.3 +## +{ 'union': 'SocketAddress', + 'data': { + 'inet': 'IPSocketAddress', + 'unix': 'UnixSocketAddress', + 'fd': 'String' } } + +## # @getfd: # # Receive a file descriptor via SCM rights and assign it a name
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- qapi-schema.json | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file modificato, 53 inserzioni(+)