qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] Some fixes to qom scripts
@ 2015-05-13 12:14 Martin Cerveny
  2015-05-13 12:14 ` [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax Martin Cerveny
  2015-05-13 12:14 ` [Qemu-devel] [PATCH 2/2] scripts: qom-tree: add support of path as argument Martin Cerveny
  0 siblings, 2 replies; 13+ messages in thread
From: Martin Cerveny @ 2015-05-13 12:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial, Martin Cerveny

Hi all.

I found some minor problems with qom scripts.

Martin Cerveny

Martin Cerveny (2):
  scripts: qom-*: add network syntax
  scripts: qom-tree: add support of path as argument

 scripts/qmp/qom-fuse |   13 ++++++++++++-
 scripts/qmp/qom-get  |   12 +++++++++++-
 scripts/qmp/qom-list |   12 +++++++++++-
 scripts/qmp/qom-set  |   12 +++++++++++-
 scripts/qmp/qom-tree |   21 ++++++++++++++++++---
 5 files changed, 63 insertions(+), 7 deletions(-)

-- 
1.7.4.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax
  2015-05-13 12:14 [Qemu-devel] [PATCH 0/2] Some fixes to qom scripts Martin Cerveny
@ 2015-05-13 12:14 ` Martin Cerveny
  2015-05-19 12:51   ` Andreas Färber
  2015-05-19 14:16   ` Daniel P. Berrange
  2015-05-13 12:14 ` [Qemu-devel] [PATCH 2/2] scripts: qom-tree: add support of path as argument Martin Cerveny
  1 sibling, 2 replies; 13+ messages in thread
From: Martin Cerveny @ 2015-05-13 12:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial, Martin Cerveny

Add network syntax parsing (ip address, port) to qom-* scripts.

Signed-off-by: Martin Cerveny <M.Cerveny@computer.org>
---
 scripts/qmp/qom-fuse |   13 ++++++++++++-
 scripts/qmp/qom-get  |   12 +++++++++++-
 scripts/qmp/qom-list |   12 +++++++++++-
 scripts/qmp/qom-set  |   12 +++++++++++-
 scripts/qmp/qom-tree |   12 +++++++++++-
 5 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index 5c6754a..d49f36d 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -134,5 +134,16 @@ class QOMFS(Fuse):
 if __name__ == '__main__':
     import sys, os
 
-    fs = QOMFS(QEMUMonitorProtocol(os.environ['QMP_SOCKET']))
+    socket_path = os.environ['QMP_SOCKET']
+    connection = socket_path.split(':')
+    if len(connection) == 2:
+        try:
+            port = int(connection[1])
+        except ValueError:
+            raise QMPBadPort
+        connection = ( connection[0], port )
+    else:
+        connection = socket_path
+
+    fs = QOMFS(QEMUMonitorProtocol(connection))
     fs.main(sys.argv)
diff --git a/scripts/qmp/qom-get b/scripts/qmp/qom-get
index 0172c69..f12eb52 100755
--- a/scripts/qmp/qom-get
+++ b/scripts/qmp/qom-get
@@ -56,7 +56,17 @@ if len(args) > 0:
 else:
     usage_error("not enough arguments")
 
-srv = QEMUMonitorProtocol(socket_path)
+connection = socket_path.split(':')
+if len(connection) == 2:
+    try:
+         port = int(connection[1])
+    except ValueError:
+         raise QMPBadPort
+    connection = ( connection[0], port )
+else:
+    connection = socket_path
+
+srv = QEMUMonitorProtocol(connection)
 srv.connect()
 
 rsp = srv.command('qom-get', path=path, property=prop)
diff --git a/scripts/qmp/qom-list b/scripts/qmp/qom-list
index 1e7cc6c..fadcce4 100755
--- a/scripts/qmp/qom-list
+++ b/scripts/qmp/qom-list
@@ -48,7 +48,17 @@ if not socket_path:
     else:
         usage_error("no QMP socket path or address given");
 
-srv = QEMUMonitorProtocol(socket_path)
+connection = socket_path.split(':')
+if len(connection) == 2:
+    try:
+         port = int(connection[1])
+    except ValueError:
+         raise QMPBadPort
+    connection = ( connection[0], port )
+else:
+    connection = socket_path
+
+srv = QEMUMonitorProtocol(connection)
 srv.connect()
 
 if len(args) == 0:
diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set
index 54ecfec..b05fae3 100755
--- a/scripts/qmp/qom-set
+++ b/scripts/qmp/qom-set
@@ -58,7 +58,17 @@ if len(args) > 1:
 else:
     usage_error("not enough arguments")
 
-srv = QEMUMonitorProtocol(socket_path)
+connection = socket_path.split(':')
+if len(connection) == 2:
+    try:
+         port = int(connection[1])
+    except ValueError:
+         raise QMPBadPort
+    connection = ( connection[0], port )
+else:
+    connection = socket_path
+
+srv = QEMUMonitorProtocol(connection)
 srv.connect()
 
 print srv.command('qom-set', path=path, property=prop, value=sys.argv[2])
diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
index aea11d4..07f8a9d 100755
--- a/scripts/qmp/qom-tree
+++ b/scripts/qmp/qom-tree
@@ -50,7 +50,17 @@ if not socket_path:
     else:
         usage_error("no QMP socket path or address given");
 
-srv = QEMUMonitorProtocol(socket_path)
+connection = socket_path.split(':')
+if len(connection) == 2:
+    try:
+         port = int(connection[1])
+    except ValueError:
+         raise QMPBadPort
+    connection = ( connection[0], port )
+else:
+    connection = socket_path
+
+srv = QEMUMonitorProtocol(connection)
 srv.connect()
 
 def list_node(path):
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 2/2] scripts: qom-tree: add support of path as argument
  2015-05-13 12:14 [Qemu-devel] [PATCH 0/2] Some fixes to qom scripts Martin Cerveny
  2015-05-13 12:14 ` [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax Martin Cerveny
@ 2015-05-13 12:14 ` Martin Cerveny
  2015-05-14 10:00   ` Paolo Bonzini
  1 sibling, 1 reply; 13+ messages in thread
From: Martin Cerveny @ 2015-05-13 12:14 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial, Martin Cerveny

Add processing of optional argument path as "tree base".

Signed-off-by: Martin Cerveny <M.Cerveny@computer.org>
---
 scripts/qmp/qom-tree |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
index 07f8a9d..c3e3738 100755
--- a/scripts/qmp/qom-tree
+++ b/scripts/qmp/qom-tree
@@ -75,6 +75,11 @@ def list_node(path):
     print ''
     for item in items:
         if item['type'].startswith('child<'):
-            list_node(path + '/' + item['name'])
+            list_node((path if (path != '/') else '')  + '/' + item['name'])
 
-list_node('/machine')
+if len(args) == 0:
+    path = '/'
+else:
+    path = args[0]
+
+list_node(path)
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] scripts: qom-tree: add support of path as argument
  2015-05-13 12:14 ` [Qemu-devel] [PATCH 2/2] scripts: qom-tree: add support of path as argument Martin Cerveny
@ 2015-05-14 10:00   ` Paolo Bonzini
  2015-05-14 11:41     ` Martin Cerveny
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2015-05-14 10:00 UTC (permalink / raw)
  To: Martin Cerveny, qemu-devel; +Cc: qemu-trivial



On 13/05/2015 14:14, Martin Cerveny wrote:
>      for item in items:
>          if item['type'].startswith('child<'):
> -            list_node(path + '/' + item['name'])
> +            list_node((path if (path != '/') else '')  + '/' + item['name'])

I'm not sure which Python version introduced if...else.  The more
traditional idiom would be

	path != '/' and path or ''

Can you use it, and move the expression out of the 'for item in items'
loop into a variable?

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] scripts: qom-tree: add support of path as argument
  2015-05-14 10:00   ` Paolo Bonzini
@ 2015-05-14 11:41     ` Martin Cerveny
  2015-05-14 11:47       ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Cerveny @ 2015-05-14 11:41 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-trivial, qemu-devel

Hello.

Ternary/if/else >= python 2.5 (I use the same coding as in scripts/qmp/qemu-ga-client).
"import json" >= python 2.6 (scripts/qmp/qmp.py)
"import argparse" >= python 2.7 (scripts/analyze-migration.py, scripts/vmstate-static-checker.py)
"import optparse" < python 2.7 (deprecated, scripts/qmp/qemu-ga-client)
....

Of course there is no problem to use traditional syntax.
(My platform (centos5.10) has python2.4, I must also replace json imports:
try:
     import json
except ImportError:
     import simplejson as json
)

Which version of python is officialy minimum supported ?

Thanks for explanation of python status.

M.C>

On Thu, 14 May 2015, Paolo Bonzini wrote:

>
>
> On 13/05/2015 14:14, Martin Cerveny wrote:
>>      for item in items:
>>          if item['type'].startswith('child<'):
>> -            list_node(path + '/' + item['name'])
>> +            list_node((path if (path != '/') else '')  + '/' + item['name'])
>
> I'm not sure which Python version introduced if...else.  The more
> traditional idiom would be
>
> 	path != '/' and path or ''
>
> Can you use it, and move the expression out of the 'for item in items'
> loop into a variable?
>
> Paolo
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] scripts: qom-tree: add support of path as argument
  2015-05-14 11:41     ` Martin Cerveny
@ 2015-05-14 11:47       ` Paolo Bonzini
  2015-05-19 12:47         ` Andreas Färber
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2015-05-14 11:47 UTC (permalink / raw)
  To: Martin Cerveny; +Cc: qemu-trivial, qemu-devel



On 14/05/2015 13:41, Martin Cerveny wrote:
> Hello.
> 
> Ternary/if/else >= python 2.5 (I use the same coding as in
> scripts/qmp/qemu-ga-client).

Oh, that's old.  Shows my knowledge of Python.

Then your patch is okay---thanks for enduring with me. :)

Paolo

> "import json" >= python 2.6 (scripts/qmp/qmp.py)
> "import argparse" >= python 2.7 (scripts/analyze-migration.py,
> scripts/vmstate-static-checker.py)
> "import optparse" < python 2.7 (deprecated, scripts/qmp/qemu-ga-client)
> ....
> 
> Of course there is no problem to use traditional syntax.
> (My platform (centos5.10) has python2.4, I must also replace json imports:
> try:
>     import json
> except ImportError:
>     import simplejson as json
> )
> 
> Which version of python is officialy minimum supported ?
> 
> Thanks for explanation of python status.
> 
> M.C>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] scripts: qom-tree: add support of path as argument
  2015-05-14 11:47       ` Paolo Bonzini
@ 2015-05-19 12:47         ` Andreas Färber
  0 siblings, 0 replies; 13+ messages in thread
From: Andreas Färber @ 2015-05-19 12:47 UTC (permalink / raw)
  To: Paolo Bonzini, Martin Cerveny; +Cc: qemu-trivial, qemu-devel

Am 14.05.2015 um 13:47 schrieb Paolo Bonzini:
> On 14/05/2015 13:41, Martin Cerveny wrote:
>> Hello.
>>
>> Ternary/if/else >= python 2.5 (I use the same coding as in
>> scripts/qmp/qemu-ga-client).
> 
> Oh, that's old.  Shows my knowledge of Python.
> 
> Then your patch is okay---thanks for enduring with me. :)

Thanks, applied to qom-next then:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax
  2015-05-13 12:14 ` [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax Martin Cerveny
@ 2015-05-19 12:51   ` Andreas Färber
  2015-05-19 13:56     ` Eric Blake
  2015-05-19 14:16   ` Daniel P. Berrange
  1 sibling, 1 reply; 13+ messages in thread
From: Andreas Färber @ 2015-05-19 12:51 UTC (permalink / raw)
  To: Martin Cerveny, qemu-devel
  Cc: qemu-trivial, Paolo Bonzini, Markus Armbruster, Luiz Capitulino

Am 13.05.2015 um 14:14 schrieb Martin Cerveny:
> Add network syntax parsing (ip address, port) to qom-* scripts.
> 
> Signed-off-by: Martin Cerveny <M.Cerveny@computer.org>
> ---
>  scripts/qmp/qom-fuse |   13 ++++++++++++-
>  scripts/qmp/qom-get  |   12 +++++++++++-
>  scripts/qmp/qom-list |   12 +++++++++++-
>  scripts/qmp/qom-set  |   12 +++++++++++-
>  scripts/qmp/qom-tree |   12 +++++++++++-
>  5 files changed, 56 insertions(+), 5 deletions(-)

Could some Python guru please take a look at this?

Martin, could you clarify what this fixes or adds exactly so that we can
extend the commit message accordingly?

Thanks,
Andreas

> 
> diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
> index 5c6754a..d49f36d 100755
> --- a/scripts/qmp/qom-fuse
> +++ b/scripts/qmp/qom-fuse
> @@ -134,5 +134,16 @@ class QOMFS(Fuse):
>  if __name__ == '__main__':
>      import sys, os
>  
> -    fs = QOMFS(QEMUMonitorProtocol(os.environ['QMP_SOCKET']))
> +    socket_path = os.environ['QMP_SOCKET']
> +    connection = socket_path.split(':')
> +    if len(connection) == 2:
> +        try:
> +            port = int(connection[1])
> +        except ValueError:
> +            raise QMPBadPort
> +        connection = ( connection[0], port )
> +    else:
> +        connection = socket_path
> +
> +    fs = QOMFS(QEMUMonitorProtocol(connection))
>      fs.main(sys.argv)
> diff --git a/scripts/qmp/qom-get b/scripts/qmp/qom-get
> index 0172c69..f12eb52 100755
> --- a/scripts/qmp/qom-get
> +++ b/scripts/qmp/qom-get
> @@ -56,7 +56,17 @@ if len(args) > 0:
>  else:
>      usage_error("not enough arguments")
>  
> -srv = QEMUMonitorProtocol(socket_path)
> +connection = socket_path.split(':')
> +if len(connection) == 2:
> +    try:
> +         port = int(connection[1])
> +    except ValueError:
> +         raise QMPBadPort
> +    connection = ( connection[0], port )
> +else:
> +    connection = socket_path
> +
> +srv = QEMUMonitorProtocol(connection)
>  srv.connect()
>  
>  rsp = srv.command('qom-get', path=path, property=prop)
> diff --git a/scripts/qmp/qom-list b/scripts/qmp/qom-list
> index 1e7cc6c..fadcce4 100755
> --- a/scripts/qmp/qom-list
> +++ b/scripts/qmp/qom-list
> @@ -48,7 +48,17 @@ if not socket_path:
>      else:
>          usage_error("no QMP socket path or address given");
>  
> -srv = QEMUMonitorProtocol(socket_path)
> +connection = socket_path.split(':')
> +if len(connection) == 2:
> +    try:
> +         port = int(connection[1])
> +    except ValueError:
> +         raise QMPBadPort
> +    connection = ( connection[0], port )
> +else:
> +    connection = socket_path
> +
> +srv = QEMUMonitorProtocol(connection)
>  srv.connect()
>  
>  if len(args) == 0:
> diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set
> index 54ecfec..b05fae3 100755
> --- a/scripts/qmp/qom-set
> +++ b/scripts/qmp/qom-set
> @@ -58,7 +58,17 @@ if len(args) > 1:
>  else:
>      usage_error("not enough arguments")
>  
> -srv = QEMUMonitorProtocol(socket_path)
> +connection = socket_path.split(':')
> +if len(connection) == 2:
> +    try:
> +         port = int(connection[1])
> +    except ValueError:
> +         raise QMPBadPort
> +    connection = ( connection[0], port )
> +else:
> +    connection = socket_path
> +
> +srv = QEMUMonitorProtocol(connection)
>  srv.connect()
>  
>  print srv.command('qom-set', path=path, property=prop, value=sys.argv[2])
> diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
> index aea11d4..07f8a9d 100755
> --- a/scripts/qmp/qom-tree
> +++ b/scripts/qmp/qom-tree
> @@ -50,7 +50,17 @@ if not socket_path:
>      else:
>          usage_error("no QMP socket path or address given");
>  
> -srv = QEMUMonitorProtocol(socket_path)
> +connection = socket_path.split(':')
> +if len(connection) == 2:
> +    try:
> +         port = int(connection[1])
> +    except ValueError:
> +         raise QMPBadPort
> +    connection = ( connection[0], port )
> +else:
> +    connection = socket_path
> +
> +srv = QEMUMonitorProtocol(connection)
>  srv.connect()
>  
>  def list_node(path):
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax
  2015-05-19 12:51   ` Andreas Färber
@ 2015-05-19 13:56     ` Eric Blake
  2015-05-19 14:12       ` Martin Cerveny
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2015-05-19 13:56 UTC (permalink / raw)
  To: Andreas Färber, Martin Cerveny, qemu-devel
  Cc: qemu-trivial, Paolo Bonzini, Markus Armbruster, Luiz Capitulino

[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]

On 05/19/2015 06:51 AM, Andreas Färber wrote:
> Am 13.05.2015 um 14:14 schrieb Martin Cerveny:
>> Add network syntax parsing (ip address, port) to qom-* scripts.
>>
>> Signed-off-by: Martin Cerveny <M.Cerveny@computer.org>
>> ---
>>  scripts/qmp/qom-fuse |   13 ++++++++++++-
>>  scripts/qmp/qom-get  |   12 +++++++++++-
>>  scripts/qmp/qom-list |   12 +++++++++++-
>>  scripts/qmp/qom-set  |   12 +++++++++++-
>>  scripts/qmp/qom-tree |   12 +++++++++++-
>>  5 files changed, 56 insertions(+), 5 deletions(-)
> 
> Could some Python guru please take a look at this?

That disqualifies me (still a python newbie), but I still see something
questionable:

>>  
>> -srv = QEMUMonitorProtocol(socket_path)
>> +connection = socket_path.split(':')
>> +if len(connection) == 2:
>> +    try:
>> +         port = int(connection[1])
>> +    except ValueError:
>> +         raise QMPBadPort
>> +    connection = ( connection[0], port )

Won't that mishandle IPv6 connections, such as something like [::1]:8000
for connecting to port 8000 on localhost, since it splits into more than
2 pieces when splitting on :?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax
  2015-05-19 13:56     ` Eric Blake
@ 2015-05-19 14:12       ` Martin Cerveny
  2015-05-19 14:17         ` Eric Blake
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Cerveny @ 2015-05-19 14:12 UTC (permalink / raw)
  To: Eric Blake
  Cc: qemu-devel, qemu-trivial, Markus Armbruster, Luiz Capitulino,
	Martin Cerveny, Paolo Bonzini, Andreas Färber

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1402 bytes --]

Hello.

On Tue, 19 May 2015, Eric Blake wrote:
> On 05/19/2015 06:51 AM, Andreas Färber wrote:
>> Am 13.05.2015 um 14:14 schrieb Martin Cerveny:
>>> Add network syntax parsing (ip address, port) to qom-* scripts.
>>>
>>> Signed-off-by: Martin Cerveny <M.Cerveny@computer.org>
>>> ---
>>>  scripts/qmp/qom-fuse |   13 ++++++++++++-
>>>  scripts/qmp/qom-get  |   12 +++++++++++-
>>>  scripts/qmp/qom-list |   12 +++++++++++-
>>>  scripts/qmp/qom-set  |   12 +++++++++++-
>>>  scripts/qmp/qom-tree |   12 +++++++++++-
>>>  5 files changed, 56 insertions(+), 5 deletions(-)
>>
>> Could some Python guru please take a look at this?
>
> That disqualifies me (still a python newbie), but I still see something
> questionable:
>
>>>
>>> -srv = QEMUMonitorProtocol(socket_path)
>>> +connection = socket_path.split(':')
>>> +if len(connection) == 2:
>>> +    try:
>>> +         port = int(connection[1])
>>> +    except ValueError:
>>> +         raise QMPBadPort
>>> +    connection = ( connection[0], port )
>
> Won't that mishandle IPv6 connections, such as something like [::1]:8000
> for connecting to port 8000 on localhost, since it splits into more than
> 2 pieces when splitting on :?

Yes, this is problem, but I copy-paste the same construct from 
scripts/qmp/qmp-shell to be compatible.
Is the IPv6 support for utilities mandatory ?
If yes I can make V2.

M.C>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax
  2015-05-13 12:14 ` [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax Martin Cerveny
  2015-05-19 12:51   ` Andreas Färber
@ 2015-05-19 14:16   ` Daniel P. Berrange
  1 sibling, 0 replies; 13+ messages in thread
From: Daniel P. Berrange @ 2015-05-19 14:16 UTC (permalink / raw)
  To: Martin Cerveny; +Cc: qemu-trivial, qemu-devel

On Wed, May 13, 2015 at 02:14:53PM +0200, Martin Cerveny wrote:
> Add network syntax parsing (ip address, port) to qom-* scripts.
> 
> Signed-off-by: Martin Cerveny <M.Cerveny@computer.org>
> ---
>  scripts/qmp/qom-fuse |   13 ++++++++++++-
>  scripts/qmp/qom-get  |   12 +++++++++++-
>  scripts/qmp/qom-list |   12 +++++++++++-
>  scripts/qmp/qom-set  |   12 +++++++++++-
>  scripts/qmp/qom-tree |   12 +++++++++++-
>  5 files changed, 56 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
> index 5c6754a..d49f36d 100755
> --- a/scripts/qmp/qom-fuse
> +++ b/scripts/qmp/qom-fuse
> @@ -134,5 +134,16 @@ class QOMFS(Fuse):
>  if __name__ == '__main__':
>      import sys, os
>  
> -    fs = QOMFS(QEMUMonitorProtocol(os.environ['QMP_SOCKET']))
> +    socket_path = os.environ['QMP_SOCKET']
> +    connection = socket_path.split(':')
> +    if len(connection) == 2:
> +        try:
> +            port = int(connection[1])
> +        except ValueError:
> +            raise QMPBadPort
> +        connection = ( connection[0], port )
> +    else:
> +        connection = socket_path
> +
> +    fs = QOMFS(QEMUMonitorProtocol(connection))
>      fs.main(sys.argv)

Rather than duplicate this code in every single command line tool
I think it'd be better to add a static method to QEMUMonitorProtocol
eg

  @staticmethod
  def from_address_string(addr_string):
       connection = socket_path.split(':')
       if len(connection) == 2:
           try:
               port = int(connection[1])
           except ValueError:
               raise QMPBadPort
           connection = ( connection[0], port )
       else:
           connection = addr_string

       return QEMUMonitorProtocol(connection)

Then each script can just do

   srv = QEMUMonitorProtocol.from_address_string(
       os.environ['QMP_SOCKET'])


Really the from_address_string should check for None eg

   if addr_string is None:
      print >>sys.stderr "Address string is required"
      sys.exit(1)

And as Eric says, splitting on ':' doesn't work with
IPv6

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax
  2015-05-19 14:12       ` Martin Cerveny
@ 2015-05-19 14:17         ` Eric Blake
  2015-05-19 14:23           ` Andreas Färber
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2015-05-19 14:17 UTC (permalink / raw)
  To: Martin Cerveny
  Cc: qemu-devel, qemu-trivial, Markus Armbruster, Luiz Capitulino,
	Paolo Bonzini, Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 1683 bytes --]

On 05/19/2015 08:12 AM, Martin Cerveny wrote:
> Hello.
> 
> On Tue, 19 May 2015, Eric Blake wrote:
>> On 05/19/2015 06:51 AM, Andreas Färber wrote:
>>> Am 13.05.2015 um 14:14 schrieb Martin Cerveny:
>>>> Add network syntax parsing (ip address, port) to qom-* scripts.
>>>>
>>>> Signed-off-by: Martin Cerveny <M.Cerveny@computer.org>
>>>> ---
>>>>  scripts/qmp/qom-fuse |   13 ++++++++++++-
>>>>  scripts/qmp/qom-get  |   12 +++++++++++-
>>>>  scripts/qmp/qom-list |   12 +++++++++++-
>>>>  scripts/qmp/qom-set  |   12 +++++++++++-
>>>>  scripts/qmp/qom-tree |   12 +++++++++++-
>>>>  5 files changed, 56 insertions(+), 5 deletions(-)
>>>
>>> Could some Python guru please take a look at this?

>>>> +if len(connection) == 2:
>>>> +    try:
>>>> +         port = int(connection[1])
>>>> +    except ValueError:
>>>> +         raise QMPBadPort
>>>> +    connection = ( connection[0], port )
>>
>> Won't that mishandle IPv6 connections, such as something like [::1]:8000
>> for connecting to port 8000 on localhost, since it splits into more than
>> 2 pieces when splitting on :?
> 
> Yes, this is problem, but I copy-paste the same construct from
> scripts/qmp/qmp-shell to be compatible.

Might be worth mentioning that, as justification in the commit message.

> Is the IPv6 support for utilities mandatory ?

I don't have any strong feelings about it (I'm okay if you don't).  But
others might.

> If yes I can make V2.

If so, it would be good to fix qmp-shell, too - which makes it sound
like it would be a separate commit.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax
  2015-05-19 14:17         ` Eric Blake
@ 2015-05-19 14:23           ` Andreas Färber
  0 siblings, 0 replies; 13+ messages in thread
From: Andreas Färber @ 2015-05-19 14:23 UTC (permalink / raw)
  To: Eric Blake, Martin Cerveny
  Cc: qemu-trivial, Markus Armbruster, Paolo Bonzini, qemu-devel,
	Luiz Capitulino

[-- Attachment #1: Type: text/plain, Size: 1939 bytes --]

Am 19.05.2015 um 16:17 schrieb Eric Blake:
> On 05/19/2015 08:12 AM, Martin Cerveny wrote:
>> Hello.
>>
>> On Tue, 19 May 2015, Eric Blake wrote:
>>> On 05/19/2015 06:51 AM, Andreas Färber wrote:
>>>> Am 13.05.2015 um 14:14 schrieb Martin Cerveny:
>>>>> Add network syntax parsing (ip address, port) to qom-* scripts.
>>>>>
>>>>> Signed-off-by: Martin Cerveny <M.Cerveny@computer.org>
>>>>> ---
>>>>>  scripts/qmp/qom-fuse |   13 ++++++++++++-
>>>>>  scripts/qmp/qom-get  |   12 +++++++++++-
>>>>>  scripts/qmp/qom-list |   12 +++++++++++-
>>>>>  scripts/qmp/qom-set  |   12 +++++++++++-
>>>>>  scripts/qmp/qom-tree |   12 +++++++++++-
>>>>>  5 files changed, 56 insertions(+), 5 deletions(-)
>>>>
>>>> Could some Python guru please take a look at this?
> 
>>>>> +if len(connection) == 2:
>>>>> +    try:
>>>>> +         port = int(connection[1])
>>>>> +    except ValueError:
>>>>> +         raise QMPBadPort
>>>>> +    connection = ( connection[0], port )
>>>
>>> Won't that mishandle IPv6 connections, such as something like [::1]:8000
>>> for connecting to port 8000 on localhost, since it splits into more than
>>> 2 pieces when splitting on :?
>>
>> Yes, this is problem, but I copy-paste the same construct from
>> scripts/qmp/qmp-shell to be compatible.
> 
> Might be worth mentioning that, as justification in the commit message.
> 
>> Is the IPv6 support for utilities mandatory ?
> 
> I don't have any strong feelings about it (I'm okay if you don't).  But
> others might.
> 
>> If yes I can make V2.
> 
> If so, it would be good to fix qmp-shell, too - which makes it sound
> like it would be a separate commit.

Is there a chance this can be fixed in a way that we don't need to
replicate that code? :)

Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-05-19 14:23 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-13 12:14 [Qemu-devel] [PATCH 0/2] Some fixes to qom scripts Martin Cerveny
2015-05-13 12:14 ` [Qemu-devel] [PATCH 1/2] scripts: qom-*: add network syntax Martin Cerveny
2015-05-19 12:51   ` Andreas Färber
2015-05-19 13:56     ` Eric Blake
2015-05-19 14:12       ` Martin Cerveny
2015-05-19 14:17         ` Eric Blake
2015-05-19 14:23           ` Andreas Färber
2015-05-19 14:16   ` Daniel P. Berrange
2015-05-13 12:14 ` [Qemu-devel] [PATCH 2/2] scripts: qom-tree: add support of path as argument Martin Cerveny
2015-05-14 10:00   ` Paolo Bonzini
2015-05-14 11:41     ` Martin Cerveny
2015-05-14 11:47       ` Paolo Bonzini
2015-05-19 12:47         ` Andreas Färber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).