On Mon, Dec 9, 2013 at 9:15 AM, Daniel P. Berrange <berrange@redhat.com> wrote:

From: "Daniel P. Berrange" <berrange@redhat.com>

The 'print' method must be called as a function in python3, ie with brackets.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- examples/consolecallback.py | 6 ++-- examples/dominfo.py | 14 +++++----- examples/domrestore.py | 17 ++++++------ examples/domsave.py | 15 +++++----- examples/domstart.py | 19 ++++++------- examples/esxlist.py | 14 +++++----- examples/event-test.py | 68 ++++++++++++++++++++++----------------------- examples/topology.py | 14 +++++----- 8 files changed, 82 insertions(+), 85 deletions(-)

diff --git a/examples/consolecallback.py b/examples/consolecallback.py index d8e33a9..c539a92 100644 --- a/examples/consolecallback.py +++ b/examples/consolecallback.py @@ -62,14 +62,14 @@ def lifecycle_callback (connection, domain, event, detail, console):

# main if len(sys.argv) != 3: - print "Usage:", sys.argv[0], "URI UUID" - print "for example:", sys.argv[0], "'qemu:///system' '32ad945f-7e78-c33a-e96d-39f25e025d81'" + print("Usage:", sys.argv[0], "URI UUID") + print("for example:", sys.argv[0], "'qemu:///system' '32ad945f-7e78-c33a-e96d-39f25e025d81'") sys.exit(1)

uri = sys.argv[1] uuid = sys.argv[2]

-print "Escape character is ^]" +print("Escape character is ^]") logging.basicConfig(filename='msg.log', level=logging.DEBUG) logging.info("URI: %s", uri) logging.info("UUID: %s", uuid) diff --git a/examples/dominfo.py b/examples/dominfo.py index bfa3ca3..d3049cd 100755 --- a/examples/dominfo.py +++ b/examples/dominfo.py @@ -8,15 +8,15 @@ import libxml2 import pdb

def usage(): - print 'Usage: %s DOMAIN' % sys.argv[0] - print ' Print information about the domain DOMAIN' + print('Usage: %s DOMAIN' % sys.argv[0]) + print(' Print information about the domain DOMAIN')

def print_section(title): - print "\n%s" % title - print "=" * 60 + print("\n%s" % title) + print("=" * 60)

def print_entry(key, value): - print "%-10s %-10s" % (key, value) + print("%-10s %-10s" % (key, value))

def print_xml(key, ctx, path): res = ctx.xpathEval(path) @@ -36,14 +36,14 @@ name = sys.argv[1] # Connect to libvirt conn = libvirt.openReadOnly(None) if conn is None: - print 'Failed to open connection to the hypervisor' + print('Failed to open connection to the hypervisor') sys.exit(1)

try: dom = conn.lookupByName(name) # Annoyiingly, libvirt prints its own error message here except libvirt.libvirtError: - print "Domain %s is not running" % name + print("Domain %s is not running" % name) sys.exit(0)

info = dom.info() diff --git a/examples/domrestore.py b/examples/domrestore.py index fffc90f..06fdfbc 100755 --- a/examples/domrestore.py +++ b/examples/domrestore.py @@ -8,10 +8,10 @@ import libxml2 import pdb

def usage(): - print 'Usage: %s DIR' % sys.argv[0] - print ' Restore all the domains contained in DIR' - print ' It is assumed that all files in DIR are' - print ' images of domU\'s previously created with save' + print('Usage: %s DIR' % sys.argv[0]) + print(' Restore all the domains contained in DIR') + print(' It is assumed that all files in DIR are') + print(' images of domU\'s previously created with save')

if len(sys.argv) != 2: usage() @@ -22,15 +22,14 @@ imgs = os.listdir(dir)

conn = libvirt.open(None) if conn is None: - print 'Failed to open connection to the hypervisor' + print('Failed to open connection to the hypervisor') sys.exit(1)

for img in imgs: file = os.path.join(dir, img) - print "Restoring %s ... " % img, - sys.stdout.flush() + print("Restoring %s ... " % img) ret = conn.restore(file) if ret == 0: - print "done" + print("done") else: - print "error %d" % ret + print("error %d" % ret) diff --git a/examples/domsave.py b/examples/domsave.py index bac4536..727217c 100755 --- a/examples/domsave.py +++ b/examples/domsave.py @@ -8,9 +8,9 @@ import libxml2 import pdb

def usage(): - print 'Usage: %s DIR' % sys.argv[0] - print ' Save all currently running domU\'s into DIR' - print ' DIR must exist and be writable by this process' + print('Usage: %s DIR' % sys.argv[0]) + print(' Save all currently running domU\'s into DIR') + print(' DIR must exist and be writable by this process')

if len(sys.argv) != 2: usage() @@ -20,7 +20,7 @@ dir = sys.argv[1]

conn = libvirt.open(None) if conn is None: - print 'Failed to open connection to the hypervisor' + print('Failed to open connection to the hypervisor') sys.exit(1)

doms = conn.listDomainsID() @@ -28,13 +28,12 @@ for id in doms: if id == 0: continue dom = conn.lookupByID(id) - print "Saving %s[%d] ... " % (dom.name(), id), - sys.stdout.flush() + print("Saving %s[%d] ... " % (dom.name(), id)) path = os.path.join(dir, dom.name()) ret = dom.save(path) if ret == 0: - print "done" + print("done") else: - print "error %d" % ret + print("error %d" % ret)

#pdb.set_trace() diff --git a/examples/domstart.py b/examples/domstart.py index b14fad1..ce1b60c 100755 --- a/examples/domstart.py +++ b/examples/domstart.py @@ -20,11 +20,11 @@ def read_domain(fname): return (name, xmldesc)

def usage(): - print 'Usage: %s domain.xml' % sys.argv[0] - print ' Check that the domain described by DOMAIN.XML is running' - print ' If the domain is not running, create it' - print ' DOMAIN.XML must be a XML description of the domain' - print ' in libvirt\'s XML format' + print('Usage: %s domain.xml' % sys.argv[0]) + print(' Check that the domain described by DOMAIN.XML is running') + print(' If the domain is not running, create it') + print(' DOMAIN.XML must be a XML description of the domain') + print(' in libvirt\'s XML format')

if len(sys.argv) != 2: usage() @@ -34,17 +34,16 @@ if len(sys.argv) != 2:

conn = libvirt.open(None) if conn is None: - print 'Failed to open connection to the hypervisor' + print('Failed to open connection to the hypervisor') sys.exit(1)

try: dom = conn.lookupByName(name) except libvirt.libvirtError: - print "Starting domain %s ... " % name, - sys.stdout.flush() + print("Starting domain %s ... " % name) dom = conn.createLinux(xmldesc, 0) if dom is None: - print "failed" + print("failed") sys.exit(1) else: - print "done" + print("done") diff --git a/examples/esxlist.py b/examples/esxlist.py index c55424f..0d47b00 100755 --- a/examples/esxlist.py +++ b/examples/esxlist.py @@ -10,8 +10,8 @@ import getpass

def usage(): - print "Usage: %s HOSTNAME" % sys.argv[0] - print " List active domains of HOSTNAME and print some info" + print("Usage: %s HOSTNAME" % sys.argv[0]) + print(" List active domains of HOSTNAME and print some info")

# This is the callback method passed to libvirt.openAuth() (see below). @@ -51,12 +51,12 @@ def request_credentials(credentials, user_data):

def print_section(title): - print "\n%s" % title - print "=" * 60 + print("\n%s" % title) + print("=" * 60)

def print_entry(key, value): - print "%-10s %-10s" % (key, value) + print("%-10s %-10s" % (key, value))

def print_xml(key, ctx, path): @@ -100,7 +100,7 @@ auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_NOECHOPROMPT], conn = libvirt.openAuth(uri, auth, 0)

if conn is None: - print "Failed to open connection to %s" % hostname + print("Failed to open connection to %s" % hostname) sys.exit(1)

state_names = { libvirt.VIR_DOMAIN_RUNNING : "running", @@ -136,7 +136,7 @@ for id in conn.listDomainsID(): ctx.setContextNode(d)

if not first: - print "------------------------------------------------------------" + print("------------------------------------------------------------") else: first = False

diff --git a/examples/event-test.py b/examples/event-test.py index 84f5259..cf1a8b8 100644 --- a/examples/event-test.py +++ b/examples/event-test.py @@ -30,7 +30,7 @@ do_debug = False def debug(msg): global do_debug if do_debug: - print msg + print(msg)

# # This general purpose event loop will support waiting for file handle @@ -456,50 +456,50 @@ def detailToString(event, detail): return eventStrings[event][detail]

def myDomainEventCallback1 (conn, dom, event, detail, opaque): - print "myDomainEventCallback1 EVENT: Domain %s(%s) %s %s" % (dom.name(), dom.ID(), + print("myDomainEventCallback1 EVENT: Domain %s(%s) %s %s" % (dom.name(), dom.ID(), eventToString(event), - detailToString(event, detail)) + detailToString(event, detail)))

def myDomainEventCallback2 (conn, dom, event, detail, opaque): - print "myDomainEventCallback2 EVENT: Domain %s(%s) %s %s" % (dom.name(), dom.ID(), + print("myDomainEventCallback2 EVENT: Domain %s(%s) %s %s" % (dom.name(), dom.ID(), eventToString(event), - detailToString(event, detail)) + detailToString(event, detail)))

def myDomainEventRebootCallback(conn, dom, opaque): - print "myDomainEventRebootCallback: Domain %s(%s)" % (dom.name(), dom.ID()) + print("myDomainEventRebootCallback: Domain %s(%s)" % (dom.name(), dom.ID()))

def myDomainEventRTCChangeCallback(conn, dom, utcoffset, opaque): - print "myDomainEventRTCChangeCallback: Domain %s(%s) %d" % (dom.name(), dom.ID(), utcoffset) + print("myDomainEventRTCChangeCallback: Domain %s(%s) %d" % (dom.name(), dom.ID(), utcoffset))

def myDomainEventWatchdogCallback(conn, dom, action, opaque): - print "myDomainEventWatchdogCallback: Domain %s(%s) %d" % (dom.name(), dom.ID(), action) + print("myDomainEventWatchdogCallback: Domain %s(%s) %d" % (dom.name(), dom.ID(), action))

def myDomainEventIOErrorCallback(conn, dom, srcpath, devalias, action, opaque): - print "myDomainEventIOErrorCallback: Domain %s(%s) %s %s %d" % (dom.name(), dom.ID(), srcpath, devalias, action) + print("myDomainEventIOErrorCallback: Domain %s(%s) %s %s %d" % (dom.name(), dom.ID(), srcpath, devalias, action))

def myDomainEventGraphicsCallback(conn, dom, phase, localAddr, remoteAddr, authScheme, subject, opaque): - print "myDomainEventGraphicsCallback: Domain %s(%s) %d %s" % (dom.name(), dom.ID(), phase, authScheme) + print("myDomainEventGraphicsCallback: Domain %s(%s) %d %s" % (dom.name(), dom.ID(), phase, authScheme))

def myDomainEventDiskChangeCallback(conn, dom, oldSrcPath, newSrcPath, devAlias, reason, opaque): - print "myDomainEventDiskChangeCallback: Domain %s(%s) disk change oldSrcPath: %s newSrcPath: %s devAlias: %s reason: %s" % ( - dom.name(), dom.ID(), oldSrcPath, newSrcPath, devAlias, reason) + print("myDomainEventDiskChangeCallback: Domain %s(%s) disk change oldSrcPath: %s newSrcPath: %s devAlias: %s reason: %s" % ( + dom.name(), dom.ID(), oldSrcPath, newSrcPath, devAlias, reason)) def myDomainEventTrayChangeCallback(conn, dom, devAlias, reason, opaque): - print "myDomainEventTrayChangeCallback: Domain %s(%s) tray change devAlias: %s reason: %s" % ( - dom.name(), dom.ID(), devAlias, reason) + print("myDomainEventTrayChangeCallback: Domain %s(%s) tray change devAlias: %s reason: %s" % ( + dom.name(), dom.ID(), devAlias, reason)) def myDomainEventPMWakeupCallback(conn, dom, reason, opaque): - print "myDomainEventPMWakeupCallback: Domain %s(%s) system pmwakeup" % ( - dom.name(), dom.ID()) + print("myDomainEventPMWakeupCallback: Domain %s(%s) system pmwakeup" % ( + dom.name(), dom.ID())) def myDomainEventPMSuspendCallback(conn, dom, reason, opaque): - print "myDomainEventPMSuspendCallback: Domain %s(%s) system pmsuspend" % ( - dom.name(), dom.ID()) + print("myDomainEventPMSuspendCallback: Domain %s(%s) system pmsuspend" % ( + dom.name(), dom.ID())) def myDomainEventBalloonChangeCallback(conn, dom, actual, opaque): - print "myDomainEventBalloonChangeCallback: Domain %s(%s) %d" % (dom.name(), dom.ID(), actual) + print("myDomainEventBalloonChangeCallback: Domain %s(%s) %d" % (dom.name(), dom.ID(), actual)) def myDomainEventPMSuspendDiskCallback(conn, dom, reason, opaque): - print "myDomainEventPMSuspendDiskCallback: Domain %s(%s) system pmsuspend_disk" % ( - dom.name(), dom.ID()) + print("myDomainEventPMSuspendDiskCallback: Domain %s(%s) system pmsuspend_disk" % ( + dom.name(), dom.ID())) def myDomainEventDeviceRemovedCallback(conn, dom, dev, opaque): - print "myDomainEventDeviceRemovedCallback: Domain %s(%s) device removed: %s" % ( - dom.name(), dom.ID(), dev) + print("myDomainEventDeviceRemovedCallback: Domain %s(%s) device removed: %s" % ( + dom.name(), dom.ID(), dev))

run = True

@@ -507,27 +507,27 @@ def myConnectionCloseCallback(conn, reason, opaque): reasonStrings = ( "Error", "End-of-file", "Keepalive", "Client", ) - print "myConnectionCloseCallback: %s: %s" % (conn.getURI(), reasonStrings[reason]) + print("myConnectionCloseCallback: %s: %s" % (conn.getURI(), reasonStrings[reason])) run = False

-def usage(out=sys.stderr): - print >>out, "usage: "+os.path.basename(sys.argv[0])+" [-hdl] [uri]" - print >>out, " uri will default to qemu:///system" - print >>out, " --help, -h Print this help message" - print >>out, " --debug, -d Print debug output" - print >>out, " --loop, -l Toggle event-loop-implementation" +def usage(): + print("usage: "+os.path.basename(sys.argv[0])+" [-hdl] [uri]") + print(" uri will default to qemu:///system") + print(" --help, -h Print(this help message") + print(" --debug, -d Print(debug output") + print(" --loop, -l Toggle event-loop-implementation")

def main(): try: opts, args = getopt.getopt(sys.argv[1:], "hdl", ["help", "debug", "loop"]) except getopt.GetoptError, err: # print help information and exit: - print str(err) # will print something like "option -a not recognized" + print(str(err)) # will print something like "option -a not recognized" usage() sys.exit(2) for o, a in opts: if o in ("-h", "--help"): - usage(sys.stdout) + usage() sys.exit() if o in ("-d", "--debug"): global do_debug @@ -541,7 +541,7 @@ def main(): else: uri = "qemu:///system"

- print "Using uri:" + uri + print("Using uri:" + uri)

# Run a background thread with the event loop if use_pure_python_event_loop: @@ -554,7 +554,7 @@ def main(): # Close connection on exit (to test cleanup paths) old_exitfunc = getattr(sys, 'exitfunc', None) def exit(): - print "Closing " + str(vc) + print("Closing " + vc.getURI()) vc.close() if (old_exitfunc): old_exitfunc() sys.exitfunc = exit diff --git a/examples/topology.py b/examples/topology.py index 62effe3..191669c 100755 --- a/examples/topology.py +++ b/examples/topology.py @@ -13,13 +13,13 @@ from xml.dom import minidom try: conn = libvirt.openReadOnly(None) except libvirt.libvirtError: - print 'Failed to connect to the hypervisor' + print('Failed to connect to the hypervisor') sys.exit(1)

try: capsXML = conn.getCapabilities() except libvirt.libvirtError: - print 'Failed to request capabilities' + print('Failed to request capabilities') sys.exit(1)

caps = minidom.parseString(capsXML) @@ -38,8 +38,8 @@ siblingsIds = [ proc.getAttribute('siblings') for proc in cells.getElementsByTagName('cpu') if proc.getAttribute('siblings') not in siblingsIds ]

-print "Host topology" -print "NUMA nodes:", cells.getAttribute('num') -print " Sockets:", len(set(socketIds)) -print " Cores:", len(set(siblingsIds)) -print " Threads:", total_cpus +print("Host topology") +print("NUMA nodes:", cells.getAttribute('num')) +print(" Sockets:", len(set(socketIds))) +print(" Cores:", len(set(siblingsIds))) +print(" Threads:", total_cpus) -- 1.8.3.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

ACK. -- Doug Goldstein

On Mon, Dec 9, 2013 at 9:15 AM, Daniel P. Berrange <berrange@redhat.com> wrote:

From: "Daniel P. Berrange" <berrange@redhat.com>

Strings in python3 default to unicode, so when writing to the self-pipe we must be sure to use bytes by calling the encode() method.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- examples/event-test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/event-test.py b/examples/event-test.py index cf1a8b8..1402c04 100644 --- a/examples/event-test.py +++ b/examples/event-test.py @@ -236,7 +236,7 @@ class virEventLoopPure: def interrupt(self): if self.runningPoll and not self.pendingWakeup: self.pendingWakeup = True - os.write(self.pipetrick[1], 'c') + os.write(self.pipetrick[1], 'c'.encode("UTF-8"))

# Registers a new file handle 'fd', monitoring for 'events' (libvirt -- 1.8.3.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

ACK. -- Doug Goldstein

On Mon, Dec 9, 2013 at 9:15 AM, Daniel P. Berrange <berrange@redhat.com> wrote:

From: "Daniel P. Berrange" <berrange@redhat.com>

The way native modules are registered has completely changed, so the code must be #ifdef'd for Python2 & 3

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-lxc-override.c | 73 +++++++++++++++++++++++++++++++++++------------ libvirt-override.c | 75 ++++++++++++++++++++++++++++++++++++------------- libvirt-qemu-override.c | 73 +++++++++++++++++++++++++++++++++++------------ 3 files changed, 166 insertions(+), 55 deletions(-)

diff --git a/libvirt-lxc-override.c b/libvirt-lxc-override.c index 03b00b0..60b41d8 100644 --- a/libvirt-lxc-override.c +++ b/libvirt-lxc-override.c @@ -21,10 +21,18 @@ #include "libvirt-utils.h" #include "build/libvirt-lxc.h"

-#ifndef __CYGWIN__ -extern void initlibvirtmod_lxc(void); +#if PY_MAJOR_VERSION > 2 +# ifndef __CYGWIN__ +extern PyObject *PyInit_libvirtmod_lxc(void); +# else +extern PyObject *PyInit_cygvirtmod_lxc(void); +# endif #else +# ifndef __CYGWIN__ +extern void initlibvirtmod_lxc(void); +# else extern void initcygvirtmod_lxc(void); +# endif #endif

#if 0 @@ -110,30 +118,59 @@ static PyMethodDef libvirtLxcMethods[] = { {NULL, NULL, 0, NULL} };

+#if PY_MAJOR_VERSION > 2 +static struct PyModuleDef moduledef = { + PyModuleDef_HEAD_INIT, +# ifndef __CYGWIN__ + "libvirtmod_lxc", +# else + "cygvirtmod_lxc", +# endif + NULL, + -1, + libvirtLxcMethods, + NULL, + NULL, + NULL, + NULL +}; + +PyObject * +# ifndef __CYGWIN__ +PyInit_libvirtmod_lxc +# else +PyInit_cygvirtmod_lxc +# endif + (void) +{ + PyObject *module; + + if (virInitialize() < 0) + return NULL; + + module = PyModule_Create(&moduledef); + + return module; +} +#else /* ! PY_MAJOR_VERSION > 2 */ void -#ifndef __CYGWIN__ +# ifndef __CYGWIN__ initlibvirtmod_lxc -#else +# else initcygvirtmod_lxc -#endif +# endif (void) { - static int initialized = 0; - - if (initialized != 0) - return; - if (virInitialize() < 0) return;

/* initialize the python extension module */ Py_InitModule((char *) -#ifndef __CYGWIN__ - "libvirtmod_lxc" -#else - "cygvirtmod_lxc" -#endif - , libvirtLxcMethods); - - initialized = 1; +# ifndef __CYGWIN__ + "libvirtmod_lxc", +# else + "cygvirtmod_lxc", +# endif + libvirtLxcMethods); } +#endif /* ! PY_MAJOR_VERSION > 2 */ diff --git a/libvirt-override.c b/libvirt-override.c index 5deb414..03aab89 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -25,10 +25,18 @@ #include "build/libvirt.h" #include "libvirt-utils.h"

-#ifndef __CYGWIN__ -extern void initlibvirtmod(void); +#if PY_MAJOR_VERSION > 2 +# ifndef __CYGWIN__ +extern PyObject *PyInit_libvirtmod(void); +# else +extern PyObject *PyInit_cygvirtmod(void); +# endif #else +# ifndef __CYGWIN__ +extern void initlibvirtmod(void); +# else extern void initcygvirtmod(void); +# endif #endif

#if 0 @@ -7466,30 +7474,59 @@ static PyMethodDef libvirtMethods[] = { {NULL, NULL, 0, NULL} };

+#if PY_MAJOR_VERSION > 2 +static struct PyModuleDef moduledef = { + PyModuleDef_HEAD_INIT, +# ifndef __CYGWIN__ + "libvirtmod", +# else + "cygvirtmod", +# endif + NULL, + -1, + libvirtMethods, + NULL, + NULL, + NULL, + NULL +}; + +PyObject * +# ifndef __CYGWIN__ +PyInit_libvirtmod +# else +PyInit_cygvirtmod +# endif + (void) +{ + PyObject *module; + + if (virInitialize() < 0) + return NULL; + + module = PyModule_Create(&moduledef); + + return module; +} +#else /* ! PY_MAJOR_VERSION > 2 */ void -#ifndef __CYGWIN__ +# ifndef __CYGWIN__ initlibvirtmod -#else +# else initcygvirtmod -#endif +# endif (void) { - static int initialized = 0; - - if (initialized != 0) - return; - if (virInitialize() < 0) return;

/* initialize the python extension module */ Py_InitModule((char *) -#ifndef __CYGWIN__ - "libvirtmod" -#else - "cygvirtmod" -#endif - , libvirtMethods); - - initialized = 1; -} +# ifndef __CYGWIN__ + "libvirtmod", +# else + "cygvirtmod", +# endif + libvirtMethods); +} +#endif /* ! PY_MAJOR_VERSION > 2 */ diff --git a/libvirt-qemu-override.c b/libvirt-qemu-override.c index a8e8c09..72257ac 100644 --- a/libvirt-qemu-override.c +++ b/libvirt-qemu-override.c @@ -21,10 +21,18 @@ #include "libvirt-utils.h" #include "build/libvirt-qemu.h"

-#ifndef __CYGWIN__ -extern void initlibvirtmod_qemu(void); +#if PY_MAJOR_VERSION > 2 +# ifndef __CYGWIN__ +extern PyObject *PyInit_libvirtmod_qemu(void); +# else +extern PyObject *PyInit_cygvirtmod_qemu(void); +# endif #else +# ifndef __CYGWIN__ +extern void initlibvirtmod_qemu(void); +# else extern void initcygvirtmod_qemu(void); +# endif #endif

#if 0 @@ -128,30 +136,59 @@ static PyMethodDef libvirtQemuMethods[] = { {NULL, NULL, 0, NULL} };

+#if PY_MAJOR_VERSION > 2 +static struct PyModuleDef moduledef = { + PyModuleDef_HEAD_INIT, +# ifndef __CYGWIN__ + "libvirtmod_qemu", +# else + "cygvirtmod_qemu", +# endif + NULL, + -1, + libvirtQemuMethods, + NULL, + NULL, + NULL, + NULL +}; + +PyObject * +# ifndef __CYGWIN__ +PyInit_libvirtmod_qemu +# else +PyInit_cygvirtmod_qemu +# endif + (void) +{ + PyObject *module; + + if (virInitialize() < 0) + return NULL; + + module = PyModule_Create(&moduledef); + + return module; +} +#else /* ! PY_MAJOR_VERSION > 2 */ void -#ifndef __CYGWIN__ +# ifndef __CYGWIN__ initlibvirtmod_qemu -#else +# else initcygvirtmod_qemu -#endif +# endif (void) { - static int initialized = 0; - - if (initialized != 0) - return; - if (virInitialize() < 0) return;

/* initialize the python extension module */ Py_InitModule((char *) -#ifndef __CYGWIN__ - "libvirtmod_qemu" -#else - "cygvirtmod_qemu" -#endif - , libvirtQemuMethods); - - initialized = 1; +# ifndef __CYGWIN__ + "libvirtmod_qemu", +# else + "cygvirtmod_qemu", +# endif + libvirtQemuMethods); } +#endif /* ! PY_MAJOR_VERSION > 2 */ -- 1.8.3.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

ACK. -- Doug Goldstein

On Mon, Dec 9, 2013 at 9:15 AM, Daniel P. Berrange <berrange@redhat.com> wrote:

From: "Daniel P. Berrange" <berrange@redhat.com>

The Exception class hiearchy in Python 2.4 reports different data types than in later Python versions. As a result the type(libvirt.libvirtError) does not return 'type'. We just special case handling of this class.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- sanitytest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sanitytest.py b/sanitytest.py index bd93fe6..eb4caee 100644 --- a/sanitytest.py +++ b/sanitytest.py @@ -39,9 +39,11 @@ for name in dir(libvirt): if name[0] == '_': continue thing = getattr(libvirt, name) + # Special-case libvirtError to deal with python 2.4 difference + # in Exception class type reporting. if type(thing) == int: gotenums.append(name) - elif type(thing) == type: + elif type(thing) == type or name == "libvirtError": gottypes.append(name) gotfunctions[name] = [] elif callable(thing): -- 1.8.3.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Don't have a setup to test, but in principle the code makes sense. -- Doug Goldstein

On Mon, Dec 9, 2013 at 9:15 AM, Daniel P. Berrange <berrange@redhat.com> wrote:

From: "Daniel P. Berrange" <berrange@redhat.com>

Make use of libvirt_constcharPtrWrap in all override code, to match generated code. This will isolate Python3 specific changes in one place.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-override.c | 36 ++++++++++++++++++------------------ libvirt-qemu-override.c | 4 ++-- 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/libvirt-override.c b/libvirt-override.c index 03aab89..84a5436 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -2127,15 +2127,15 @@ static int virConnectCredCallbackWrapper(virConnectCredentialPtr cred, Py_INCREF(Py_None); PyTuple_SetItem(pycred, i, pycreditem); PyList_SetItem(pycreditem, 0, PyInt_FromLong((long) cred[i].type)); - PyList_SetItem(pycreditem, 1, PyString_FromString(cred[i].prompt)); + PyList_SetItem(pycreditem, 1, libvirt_constcharPtrWrap(cred[i].prompt)); if (cred[i].challenge) { - PyList_SetItem(pycreditem, 2, PyString_FromString(cred[i].challenge)); + PyList_SetItem(pycreditem, 2, libvirt_constcharPtrWrap(cred[i].challenge)); } else { Py_INCREF(Py_None); PyList_SetItem(pycreditem, 2, Py_None); } if (cred[i].defresult) { - PyList_SetItem(pycreditem, 3, PyString_FromString(cred[i].defresult)); + PyList_SetItem(pycreditem, 3, libvirt_constcharPtrWrap(cred[i].defresult)); } else { Py_INCREF(Py_None); PyList_SetItem(pycreditem, 3, Py_None); @@ -2319,7 +2319,7 @@ libvirt_virConnectGetCPUModelNames(PyObject *self ATTRIBUTE_UNUSED,

for (i = 0; i < c_retval; i++) { PyObject *str; - if ((str = PyString_FromString(models[i])) == NULL) + if ((str = libvirt_constcharPtrWrap(models[i])) == NULL) goto error;

PyList_SET_ITEM(rv, i, str); @@ -2969,7 +2969,7 @@ libvirt_virDomainGetUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {

if (c_retval < 0) return VIR_PY_NONE; - py_retval = PyString_FromStringAndSize((char *) &uuid[0], VIR_UUID_BUFLEN); + py_retval = libvirt_charPtrSizeWrap((char *) &uuid[0], VIR_UUID_BUFLEN);

return py_retval; } @@ -2997,7 +2997,7 @@ libvirt_virDomainGetUUIDString(PyObject *self ATTRIBUTE_UNUSED, if (c_retval < 0) return VIR_PY_NONE;

- py_retval = PyString_FromString((char *) &uuidstr[0]); + py_retval = libvirt_constcharPtrWrap((char *) &uuidstr[0]); return py_retval; }

@@ -3186,7 +3186,7 @@ libvirt_virNetworkGetUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {

if (c_retval < 0) return VIR_PY_NONE; - py_retval = PyString_FromStringAndSize((char *) &uuid[0], VIR_UUID_BUFLEN); + py_retval = libvirt_charPtrSizeWrap((char *) &uuid[0], VIR_UUID_BUFLEN);

return py_retval; } @@ -3214,7 +3214,7 @@ libvirt_virNetworkGetUUIDString(PyObject *self ATTRIBUTE_UNUSED, if (c_retval < 0) return VIR_PY_NONE;

- py_retval = PyString_FromString((char *) &uuidstr[0]); + py_retval = libvirt_constcharPtrWrap((char *) &uuidstr[0]); return py_retval; }

@@ -3816,7 +3816,7 @@ libvirt_virStoragePoolGetUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { if (c_retval < 0) return VIR_PY_NONE;

- py_retval = PyString_FromStringAndSize((char *) &uuid[0], VIR_UUID_BUFLEN); + py_retval = libvirt_charPtrSizeWrap((char *) &uuid[0], VIR_UUID_BUFLEN);

return py_retval; } @@ -3843,7 +3843,7 @@ libvirt_virStoragePoolGetUUIDString(PyObject *self ATTRIBUTE_UNUSED, if (c_retval < 0) return VIR_PY_NONE;

- py_retval = PyString_FromString((char *) &uuidstr[0]); + py_retval = libvirt_constcharPtrWrap((char *) &uuidstr[0]); return py_retval; }

@@ -4030,7 +4030,7 @@ libvirt_virSecretGetUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {

if (c_retval < 0) return VIR_PY_NONE; - py_retval = PyString_FromStringAndSize((char *) &uuid[0], VIR_UUID_BUFLEN); + py_retval = libvirt_charPtrSizeWrap((char *) &uuid[0], VIR_UUID_BUFLEN);

return py_retval; } @@ -4058,7 +4058,7 @@ libvirt_virSecretGetUUIDString(PyObject *self ATTRIBUTE_UNUSED, if (c_retval < 0) return VIR_PY_NONE;

- py_retval = PyString_FromString((char *) &uuidstr[0]); + py_retval = libvirt_constcharPtrWrap((char *) &uuidstr[0]); return py_retval; }

@@ -4201,7 +4201,7 @@ libvirt_virSecretGetValue(PyObject *self ATTRIBUTE_UNUSED, if (c_retval == NULL) return VIR_PY_NONE;

- py_retval = PyString_FromStringAndSize((const char *)c_retval, size); + py_retval = libvirt_charPtrSizeWrap((char*)c_retval, size); VIR_FREE(c_retval);

return py_retval; @@ -4252,7 +4252,7 @@ libvirt_virNWFilterGetUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {

if (c_retval < 0) return VIR_PY_NONE; - py_retval = PyString_FromStringAndSize((char *) &uuid[0], VIR_UUID_BUFLEN); + py_retval = libvirt_charPtrSizeWrap((char *) &uuid[0], VIR_UUID_BUFLEN);

return py_retval; } @@ -4280,7 +4280,7 @@ libvirt_virNWFilterGetUUIDString(PyObject *self ATTRIBUTE_UNUSED, if (c_retval < 0) return VIR_PY_NONE;

- py_retval = PyString_FromString((char *) &uuidstr[0]); + py_retval = libvirt_constcharPtrWrap((char *) &uuidstr[0]); return py_retval; }

@@ -4601,7 +4601,7 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED, if (base_cpu == NULL) return VIR_PY_NONE;

- pybase_cpu = PyString_FromString(base_cpu); + pybase_cpu = libvirt_constcharPtrWrap(base_cpu); VIR_FREE(base_cpu);

if (pybase_cpu == NULL) @@ -6969,7 +6969,7 @@ libvirt_virDomainBlockPeek(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; }

- py_retval = PyString_FromStringAndSize(buf, size); + py_retval = libvirt_charPtrSizeWrap(buf, size);

cleanup: VIR_FREE(buf); @@ -7006,7 +7006,7 @@ libvirt_virDomainMemoryPeek(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; }

- py_retval = PyString_FromStringAndSize(buf, size); + py_retval = libvirt_charPtrSizeWrap(buf, size);

cleanup: VIR_FREE(buf); diff --git a/libvirt-qemu-override.c b/libvirt-qemu-override.c index 72257ac..480a7d3 100644 --- a/libvirt-qemu-override.c +++ b/libvirt-qemu-override.c @@ -85,7 +85,7 @@ libvirt_qemu_virDomainQemuMonitorCommand(PyObject *self ATTRIBUTE_UNUSED, if (c_retval < 0) return VIR_PY_NONE;

- py_retval = PyString_FromString(result); + py_retval = libvirt_constcharPtrWrap(result); VIR_FREE(result); return py_retval; } @@ -116,7 +116,7 @@ libvirt_qemu_virDomainQemuAgentCommand(PyObject *self ATTRIBUTE_UNUSED, PyObject if (!result) return VIR_PY_NONE;

- py_retval = PyString_FromString(result); + py_retval = libvirt_constcharPtrWrap(result); VIR_FREE(result); return py_retval; } -- 1.8.3.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

ACK. -- Doug Goldstein

On Mon, Dec 9, 2013 at 9:15 AM, Daniel P. Berrange <berrange@redhat.com> wrote:

From: "Daniel P. Berrange" <berrange@redhat.com>

Replace calls to PyString_AsString with the helper method libvirt_charPtrUnwrap. This isolates the code that will change in Python3.

In making this change, all callers now have responsibility for free'ing the string.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt-override.c | 106 +++++++++++++++++++++++++++++++---------------------- typewrappers.c | 18 +++++++++ typewrappers.h | 1 + 3 files changed, 81 insertions(+), 44 deletions(-)

diff --git a/libvirt-override.c b/libvirt-override.c index 84a5436..579ea43 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -58,18 +58,17 @@ extern void initcygvirtmod(void); #define VIR_PY_INT_FAIL (libvirt_intWrap(-1)) #define VIR_PY_INT_SUCCESS (libvirt_intWrap(0))

-/* We don't want to free() returned value. As written in doc: - * PyString_AsString returns pointer to 'internal buffer of string, - * not a copy' and 'It must not be deallocated'. */ static char *py_str(PyObject *obj) { PyObject *str = PyObject_Str(obj); + char *ret; if (!str) { PyErr_Print(); PyErr_Clear(); return NULL; }; - return PyString_AsString(str); + libvirt_charPtrUnwrap(str, &ret); + return ret; }

/* Helper function to convert a virTypedParameter output array into a @@ -147,9 +146,8 @@ cleanup: /* Allocate a new typed parameter array with the same contents and * length as info, and using the array params of length nparams as * hints on what types to use when creating the new array. The caller - * must NOT clear the array before freeing it, as it points into info - * rather than allocated strings. Return NULL on failure, after - * raising a python exception. */ + * must clear the array before freeing it. Return NULL on failure, + * after raising a python exception. */ static virTypedParameterPtr ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) setPyVirTypedParameter(PyObject *info, const virTypedParameter *params, int nparams) @@ -183,7 +181,8 @@ setPyVirTypedParameter(PyObject *info, while (PyDict_Next(info, &pos, &key, &value)) { char *keystr = NULL;

- if ((keystr = PyString_AsString(key)) == NULL) + if (libvirt_charPtrUnwrap(key, &keystr) < 0 || + keystr == NULL) goto cleanup;

for (i = 0; i < nparams; i++) { @@ -194,12 +193,14 @@ setPyVirTypedParameter(PyObject *info, PyErr_Format(PyExc_LookupError, "Attribute name \"%s\" could not be recognized", keystr); + free(keystr);

Here you use free() but....

goto cleanup; }

strncpy(temp->field, keystr, sizeof(*temp->field) - 1); temp->field[sizeof(*temp->field) - 1] = '\0'; temp->type = params[i].type; + free(keystr);

switch (params[i].type) { case VIR_TYPED_PARAM_INT: @@ -237,8 +238,9 @@ setPyVirTypedParameter(PyObject *info, } case VIR_TYPED_PARAM_STRING: { - char *string_val = PyString_AsString(value); - if (!string_val) + char *string_val; + if (libvirt_charPtrUnwrap(value, &string_val) < 0 || + !string_val) goto cleanup; temp->value.s = string_val; break; @@ -297,6 +299,7 @@ virPyDictToTypedParams(PyObject *dict, int n = 0; int max = 0; int ret = -1; + char *keystr = NULL;

*ret_params = NULL; *ret_nparams = 0; @@ -305,10 +308,10 @@ virPyDictToTypedParams(PyObject *dict, return -1;

while (PyDict_Next(dict, &pos, &key, &value)) { - char *keystr; int type = -1;

- if (!(keystr = PyString_AsString(key))) + if (libvirt_charPtrUnwrap(key, &keystr) < 0 || + !keystr) goto cleanup;

for (i = 0; i < nhints; i++) { @@ -396,15 +399,20 @@ virPyDictToTypedParams(PyObject *dict, } case VIR_TYPED_PARAM_STRING: { - char *val = PyString_AsString(value); - if (!val || - virTypedParamsAddString(¶ms, &n, &max, keystr, val) < 0) + char *val;; + if (libvirt_charPtrUnwrap(value, &val) < 0 || + !val || + virTypedParamsAddString(¶ms, &n, &max, keystr, val) < 0) { + VIR_FREE(val);

Here you use VIR_FREE(). We should probably be consistent.

goto cleanup; + } + VIR_FREE(val); break; } case VIR_TYPED_PARAM_LAST: break; /* unreachable */ } + VIR_FREE(keystr); }

*ret_params = params; @@ -413,6 +421,7 @@ virPyDictToTypedParams(PyObject *dict, ret = 0;

cleanup: + VIR_FREE(keystr); virTypedParamsFree(params, n); return ret; } @@ -957,7 +966,7 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED,

cleanup: virTypedParamsFree(params, nparams); - VIR_FREE(new_params); + virTypedParamsFree(new_params, nparams); return ret; }

@@ -1033,7 +1042,7 @@ libvirt_virDomainSetSchedulerParametersFlags(PyObject *self ATTRIBUTE_UNUSED,

cleanup: virTypedParamsFree(params, nparams); - VIR_FREE(new_params); + virTypedParamsFree(new_params, nparams); return ret; }

@@ -1107,7 +1116,7 @@ libvirt_virDomainSetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED,

cleanup: virTypedParamsFree(params, nparams); - VIR_FREE(new_params); + virTypedParamsFree(new_params, nparams); return ret; }

@@ -1227,7 +1236,7 @@ libvirt_virDomainSetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED,

cleanup: virTypedParamsFree(params, nparams); - VIR_FREE(new_params); + virTypedParamsFree(new_params, nparams); return ret; }

@@ -1347,7 +1356,7 @@ libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED,

cleanup: virTypedParamsFree(params, nparams); - VIR_FREE(new_params); + virTypedParamsFree(new_params, nparams); return ret; }

@@ -1468,7 +1477,7 @@ libvirt_virDomainSetInterfaceParameters(PyObject *self ATTRIBUTE_UNUSED,

cleanup: virTypedParamsFree(params, nparams); - VIR_FREE(new_params); + virTypedParamsFree(new_params, nparams); return ret; }

@@ -2163,9 +2172,9 @@ static int virConnectCredCallbackWrapper(virConnectCredentialPtr cred, pycreditem = PyTuple_GetItem(pycred, i); pyresult = PyList_GetItem(pycreditem, 4); if (pyresult != Py_None) - result = PyString_AsString(pyresult); + libvirt_charPtrUnwrap(pyresult, &result); if (result != NULL) { - cred[i].result = strdup(result); + cred[i].result = result; cred[i].resultlen = strlen(result); } else { cred[i].result = NULL; @@ -2942,7 +2951,7 @@ libvirt_virDomainGetSecurityLabelList(PyObject *self ATTRIBUTE_UNUSED, PyObject PyObject *entry = PyList_New(2); PyList_SetItem(entry, 0, libvirt_constcharPtrWrap(&labels[i].label[0])); PyList_SetItem(entry, 1, libvirt_boolWrap(labels[i].enforcing)); - PyList_Append(py_retval, entry); + PyList_Append(py_retval, entry); } free(labels); return py_retval; @@ -4566,10 +4575,11 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED, PyObject *list; virConnectPtr conn; unsigned int flags; - const char **xmlcpus = NULL; + char **xmlcpus = NULL; int ncpus = 0; char *base_cpu; PyObject *pybase_cpu; + size_t i, j;

if (!PyArg_ParseTuple(args, (char *)"OOi:virConnectBaselineCPU", &pyobj_conn, &list, &flags)) @@ -4577,15 +4587,15 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED, conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);

if (PyList_Check(list)) { - size_t i; - ncpus = PyList_Size(list); if (VIR_ALLOC_N(xmlcpus, ncpus) < 0) return VIR_PY_NONE;

for (i = 0; i < ncpus; i++) { - xmlcpus[i] = PyString_AsString(PyList_GetItem(list, i)); - if (xmlcpus[i] == NULL) { + if (libvirt_charPtrUnwrap(PyList_GetItem(list, i), &(xmlcpus[i])) < 0 || + xmlcpus[i] == NULL) { + for (j = 0 ; j < i ; j++) + VIR_FREE(xmlcpus[j]); VIR_FREE(xmlcpus); return VIR_PY_NONE; } @@ -4593,9 +4603,11 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED, }

LIBVIRT_BEGIN_ALLOW_THREADS; - base_cpu = virConnectBaselineCPU(conn, xmlcpus, ncpus, flags); + base_cpu = virConnectBaselineCPU(conn, (const char **)xmlcpus, ncpus, flags); LIBVIRT_END_ALLOW_THREADS;

+ for (i = 0 ; i < ncpus ; i++) + VIR_FREE(xmlcpus[i]); VIR_FREE(xmlcpus);

if (base_cpu == NULL) @@ -4821,7 +4833,7 @@ libvirt_virDomainSetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED,

cleanup: virTypedParamsFree(params, nparams); - VIR_FREE(new_params); + virTypedParamsFree(new_params, nparams); return ret; }

@@ -5105,18 +5117,18 @@ libvirt_virConnectDomainEventDeregister(ATTRIBUTE_UNUSED PyObject * self, /******************************************* * Event Impl *******************************************/ -static PyObject *addHandleObj = NULL; -static char *addHandleName = NULL; -static PyObject *updateHandleObj = NULL; -static char *updateHandleName = NULL; -static PyObject *removeHandleObj = NULL; -static char *removeHandleName = NULL; -static PyObject *addTimeoutObj = NULL; -static char *addTimeoutName = NULL; -static PyObject *updateTimeoutObj = NULL; -static char *updateTimeoutName = NULL; -static PyObject *removeTimeoutObj = NULL; -static char *removeTimeoutName = NULL; +static PyObject *addHandleObj; +static char *addHandleName; +static PyObject *updateHandleObj; +static char *updateHandleName; +static PyObject *removeHandleObj; +static char *removeHandleName; +static PyObject *addTimeoutObj; +static char *addTimeoutName; +static PyObject *updateTimeoutObj; +static char *updateTimeoutName; +static PyObject *removeTimeoutObj; +static char *removeTimeoutName;

Not sure the advantage of this change.

#define NAME(fn) ( fn ## Name ? fn ## Name : # fn )

@@ -5381,6 +5393,12 @@ libvirt_virEventRegisterImpl(ATTRIBUTE_UNUSED PyObject * self, Py_XDECREF(addTimeoutObj); Py_XDECREF(updateTimeoutObj); Py_XDECREF(removeTimeoutObj); + free(addHandleName); + free(updateHandleName); + free(removeHandleName); + free(addTimeoutName); + free(updateTimeoutName); + free(removeTimeoutName);

More free() vs VIR_FREE().

/* Parse and check arguments */ if (!PyArg_ParseTuple(args, (char *) "OOOOOO:virEventRegisterImpl", @@ -7084,7 +7102,7 @@ libvirt_virNodeSetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED,

cleanup: virTypedParamsFree(params, nparams); - VIR_FREE(new_params); + virTypedParamsFree(new_params, nparams); return ret; }

diff --git a/typewrappers.c b/typewrappers.c index 1622986..1e99554 100644 --- a/typewrappers.c +++ b/typewrappers.c @@ -317,6 +317,24 @@ libvirt_boolUnwrap(PyObject *obj, bool *val) return 0; }

+int +libvirt_charPtrUnwrap(PyObject *obj, char **str) +{ + const char *ret; + *str = NULL; + if (!obj) { + PyErr_SetString(PyExc_TypeError, "unexpected type"); + return -1; + } + + ret = PyString_AsString(obj); + if (ret && + !(*str = strdup(ret))) + return -1; + + return 0; +} + PyObject * libvirt_virDomainPtrWrap(virDomainPtr node) { diff --git a/typewrappers.h b/typewrappers.h index 04e364f..7068426 100644 --- a/typewrappers.h +++ b/typewrappers.h @@ -173,6 +173,7 @@ int libvirt_longlongUnwrap(PyObject *obj, long long *val); int libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val); int libvirt_doubleUnwrap(PyObject *obj, double *val); int libvirt_boolUnwrap(PyObject *obj, bool *val); +int libvirt_charPtrUnwrap(PyObject *obj, char **str); PyObject * libvirt_virConnectPtrWrap(virConnectPtr node); PyObject * libvirt_virDomainPtrWrap(virDomainPtr node); PyObject * libvirt_virNetworkPtrWrap(virNetworkPtr node); -- 1.8.3.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Some minor nits but overall looks ok. I'll double check it for any leaks with more context after I review the rest of the series. -- Doug Goldstein