Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

2017-08-18 Thread Stefan Hajnoczi
On Tue, Aug 15, 2017 at 05:06:46PM -0300, Eduardo Habkost wrote:
> On Tue, Aug 15, 2017 at 10:56:07AM +0100, Daniel P. Berrange wrote:
> > On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote:
> > > Eduardo Habkost  writes:
> > > 
> > > > It makes command-line parsing and generation of help text much
> > > > simpler.
> > > 
> > > There's really no excuse for parsing command line arguments by hand in
> > > Python.
> > > 
> > > > The optparse module is deprecated since Python 2.7, but argparse
> > > > is not available in Python 2.6 (the minimum Python version
> > > > required for building QEMU).
> > > 
> > > We have a few uses of argparse in the tree.  Are they okay?
> > > 
> > > We also use getopt in places.  Perhaps we should pick one way to parse
> > > command lines and stick to it.
> > 
> > I just came up against this problem with keycodemapdb which I'm adding as
> > a submodule to qemu.  I used argparse there because it is the modern
> > recommended API for python >= 2.7.   I examined possibilty of using
> > optparse instead, but it lacks key features - in particular the idea
> > of 'subcommands' is entirely missing from optparse.
> > 
> > The 'argparse' module is part of python core, but is *also* available
> > as a standalone module that is implemented in terms of optparse.
> > 
> > So, I would suggest that we just copy the 'argparse' module into the
> > QEMU 'scripts/thirdparty' directory.
> > 
> > Then in any files which need argparse, you can do this
> > 
> >   try:
> >   import argparse
> >   except:
> >   import os, sys
> >   sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty"))
> >   import argparse
> 
> What about:
> 
>   try:
>   import argparse
>   except:
>   from thirdparty import argparse
> 
> (I think we could move all our Python modules [qemu.py, qmp.py]
> to ./scripts/qemu/, so this would become "qemu.thirdparty").
> 
> > 
> > so that it tries to use the argparse provided by python, and falls back
> > to pulling in the one in our scripts/thirdparty directory.
> > 
> > When we finally bump our min python to 2.7, we can simply drop this
> > compat code for the import statement.  This avoids need for us to
> > re-write code to use a deprecated API, with a worse feature set.
> 
> Sounds good to me.

Sounds good.

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

2017-08-15 Thread Eduardo Habkost
On Tue, Aug 15, 2017 at 10:56:07AM +0100, Daniel P. Berrange wrote:
> On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote:
> > Eduardo Habkost  writes:
> > 
> > > It makes command-line parsing and generation of help text much
> > > simpler.
> > 
> > There's really no excuse for parsing command line arguments by hand in
> > Python.
> > 
> > > The optparse module is deprecated since Python 2.7, but argparse
> > > is not available in Python 2.6 (the minimum Python version
> > > required for building QEMU).
> > 
> > We have a few uses of argparse in the tree.  Are they okay?
> > 
> > We also use getopt in places.  Perhaps we should pick one way to parse
> > command lines and stick to it.
> 
> I just came up against this problem with keycodemapdb which I'm adding as
> a submodule to qemu.  I used argparse there because it is the modern
> recommended API for python >= 2.7.   I examined possibilty of using
> optparse instead, but it lacks key features - in particular the idea
> of 'subcommands' is entirely missing from optparse.
> 
> The 'argparse' module is part of python core, but is *also* available
> as a standalone module that is implemented in terms of optparse.
> 
> So, I would suggest that we just copy the 'argparse' module into the
> QEMU 'scripts/thirdparty' directory.
> 
> Then in any files which need argparse, you can do this
> 
>   try:
>   import argparse
>   except:
>   import os, sys
>   sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty"))
>   import argparse

What about:

  try:
  import argparse
  except:
  from thirdparty import argparse

(I think we could move all our Python modules [qemu.py, qmp.py]
to ./scripts/qemu/, so this would become "qemu.thirdparty").

> 
> so that it tries to use the argparse provided by python, and falls back
> to pulling in the one in our scripts/thirdparty directory.
> 
> When we finally bump our min python to 2.7, we can simply drop this
> compat code for the import statement.  This avoids need for us to
> re-write code to use a deprecated API, with a worse feature set.

Sounds good to me.

-- 
Eduardo



Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

2017-08-15 Thread Daniel P. Berrange
On Tue, Aug 08, 2017 at 05:39:31PM -0300, Eduardo Habkost wrote:
> It makes command-line parsing and generation of help text much
> simpler.
> 
> The optparse module is deprecated since Python 2.7, but argparse
> is not available in Python 2.6 (the minimum Python version
> required for building QEMU).
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Use optparse module, as the minimum Python version for building
>   QEMU is 2.6
>   * Reported-by: Stefan Hajnoczi 
>   * Suggested-by: "Daniel P. Berrange" 

I no longer suggest this approach :-)

I'd prefer if we just bundled a copy of 'argparse.py' in the
local scripts/thirdparty directory, and add that to the
python import path, if the system python lacks argparse.

eg if we had $QEMU-GIT/scripts/thirdparty/argparse.py you can
use:

try:
import argparse
except:
import os, sys
sys.path.append(os.path.join(os.path.dirname(__file__), "../thirdparty"))
import argparse

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

2017-08-15 Thread Daniel P. Berrange
On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote:
> Eduardo Habkost  writes:
> 
> > It makes command-line parsing and generation of help text much
> > simpler.
> 
> There's really no excuse for parsing command line arguments by hand in
> Python.
> 
> > The optparse module is deprecated since Python 2.7, but argparse
> > is not available in Python 2.6 (the minimum Python version
> > required for building QEMU).
> 
> We have a few uses of argparse in the tree.  Are they okay?
> 
> We also use getopt in places.  Perhaps we should pick one way to parse
> command lines and stick to it.

I just came up against this problem with keycodemapdb which I'm adding as
a submodule to qemu.  I used argparse there because it is the modern
recommended API for python >= 2.7.   I examined possibilty of using
optparse instead, but it lacks key features - in particular the idea
of 'subcommands' is entirely missing from optparse.

The 'argparse' module is part of python core, but is *also* available
as a standalone module that is implemented in terms of optparse.

So, I would suggest that we just copy the 'argparse' module into the
QEMU 'scripts/thirdparty' directory.

Then in any files which need argparse, you can do this

  try:
  import argparse
  except:
  import os, sys
  sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty"))
  import argparse

so that it tries to use the argparse provided by python, and falls back
to pulling in the one in our scripts/thirdparty directory.

When we finally bump our min python to 2.7, we can simply drop this
compat code for the import statement.  This avoids need for us to
re-write code to use a deprecated API, with a worse feature set.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

2017-08-15 Thread Markus Armbruster
Eduardo Habkost  writes:

> It makes command-line parsing and generation of help text much
> simpler.

There's really no excuse for parsing command line arguments by hand in
Python.

> The optparse module is deprecated since Python 2.7, but argparse
> is not available in Python 2.6 (the minimum Python version
> required for building QEMU).

We have a few uses of argparse in the tree.  Are they okay?

We also use getopt in places.  Perhaps we should pick one way to parse
command lines and stick to it.

>
> Signed-off-by: Eduardo Habkost 



Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

2017-08-09 Thread Stefan Hajnoczi
On Tue, Aug 08, 2017 at 05:39:31PM -0300, Eduardo Habkost wrote:
> It makes command-line parsing and generation of help text much
> simpler.
> 
> The optparse module is deprecated since Python 2.7, but argparse
> is not available in Python 2.6 (the minimum Python version
> required for building QEMU).
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Use optparse module, as the minimum Python version for building
>   QEMU is 2.6
>   * Reported-by: Stefan Hajnoczi 
>   * Suggested-by: "Daniel P. Berrange" 
> ---
>  scripts/qmp/qmp-shell | 63 
> +++
>  1 file changed, 23 insertions(+), 40 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


[Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

2017-08-08 Thread Eduardo Habkost
It makes command-line parsing and generation of help text much
simpler.

The optparse module is deprecated since Python 2.7, but argparse
is not available in Python 2.6 (the minimum Python version
required for building QEMU).

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Use optparse module, as the minimum Python version for building
  QEMU is 2.6
  * Reported-by: Stefan Hajnoczi 
  * Suggested-by: "Daniel P. Berrange" 
---
 scripts/qmp/qmp-shell | 63 +++
 1 file changed, 23 insertions(+), 40 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 860ffb2..ad72ef9 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -73,6 +73,7 @@ import sys
 import os
 import errno
 import atexit
+import optparse
 
 class QMPCompleter(list):
 def complete(self, text, state):
@@ -393,52 +394,34 @@ def die(msg):
 sys.stderr.write('ERROR: %s\n' % msg)
 sys.exit(1)
 
-def fail_cmdline(option=None):
-if option:
-sys.stderr.write('ERROR: bad command-line option \'%s\'\n' % option)
-sys.stderr.write('qmp-shell [ -v ] [ -p ] [ -H ] [ -N ] < UNIX socket 
path> | < TCP address:port >\n')
-sys.stderr.write('-v Verbose (echo command sent and received)\n')
-sys.stderr.write('-p Pretty-print JSON\n')
-sys.stderr.write('-H Use HMP interface\n')
-sys.stderr.write('-N Skip negotiate (for qemu-ga)\n')
-sys.exit(1)
-
 def main():
-addr = ''
-qemu = None
-hmp = False
-pretty = False
-verbose = False
-negotiate = True
+parser = optparse.OptionParser(description='QMP shell utility')
+parser.set_usage("%prog [options]  | ")
+parser.add_option('-v', action='store_true', dest='verbose',
+help='Verbose (echo command sent and received)')
+parser.add_option('-p', action='store_true', dest='pretty',
+help='Pretty-print JSON')
+parser.add_option('-H', action='store_true', dest='hmp',
+help='Use HMP interface')
+parser.add_option('-N', action='store_false', dest='negotiate',
+default=True, help='Skip negotiate (for qemu-ga)')
+opts,args = parser.parse_args()
+
+if len(args) != 1:
+parser.print_help(sys.stderr)
+sys.exit(1)
+addr = args[0]
 
 try:
-for arg in sys.argv[1:]:
-if arg == "-H":
-if qemu is not None:
-fail_cmdline(arg)
-hmp = True
-elif arg == "-p":
-pretty = True
-elif arg == "-N":
-negotiate = False
-elif arg == "-v":
-verbose = True
-else:
-if qemu is not None:
-fail_cmdline(arg)
-if hmp:
-qemu = HMPShell(arg)
-else:
-qemu = QMPShell(arg, pretty)
-addr = arg
-
-if qemu is None:
-fail_cmdline()
+if opts.hmp:
+qemu = HMPShell(addr)
+else:
+qemu = QMPShell(addr, opts.pretty)
 except QMPShellBadPort:
 die('bad port number in command-line')
 
 try:
-qemu.connect(negotiate)
+qemu.connect(opts.negotiate)
 except qmp.QMPConnectError:
 die('Didn\'t get QMP greeting message')
 except qmp.QMPCapabilitiesError:
@@ -447,7 +430,7 @@ def main():
 die('Could not connect to %s' % addr)
 
 qemu.show_banner()
-qemu.set_verbosity(verbose)
+qemu.set_verbosity(opts.verbose)
 while qemu.read_exec_command(qemu.get_prompt()):
 pass
 qemu.close()
-- 
2.9.4