Libguestfs - Aug 2018 - Re: [PATCH] v2v: rhv-upload-plugin: Use BrokenPipeError

With python 3, we have a nicer way to handle socket.error with errno set
to EPIPE (or ESHUTDOWN).

This is also more correct since in some cases (that I could not
reproduce yet with v2v), using e[0] with BrokenPipeError will fail with:
>>> OSError(errno.EPIPE, "Broken pipe")[0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'BrokenPipeError' object is not subscriptable

For python 2 e[0] seems to work, but is leftover from historic python
version that used to raise a tuple instead of socket.error instance.
In python 2.7 library code e.args[0] is used. If we ever port this to
python 2 this is the best form.
---
 v2v/rhv-upload-plugin.py | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index b5dd5521d..5cd6d5cab 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -17,7 +17,6 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
 import builtins
-import errno
 import json
 import logging
 import socket
@@ -361,9 +360,8 @@ def pwrite(h, buf, offset):
 
     try:
         http.send(buf)
-    except socket.error as e:
-        if e[0] != errno.EPIPE:
-            raise
+    except BrokenPipeError:
+        pass
 
     r = http.getresponse()
     if r.status != 200:
@@ -425,9 +423,8 @@ def emulate_zero(h, count, offset):
                 http.send(buf)
                 count -= len(buf)
             http.send(buffer(buf, 0, count))
-        except socket.error as e:
-            if e[0] != errno.EPIPE:
-                raise
+        except BrokenPipeError:
+            pass
 
         r = http.getresponse()
         if r.status != 200:
-- 
2.17.1

On Tue, Aug 28, 2018 at 11:50:56PM +0300, Nir Soffer
wrote:
> With python 3, we have a nicer way to handle socket.error with errno set
> to EPIPE (or ESHUTDOWN).
> 
> This is also more correct since in some cases (that I could not
> reproduce yet with v2v), using e[0] with BrokenPipeError will fail with:
> 
> >>> OSError(errno.EPIPE, "Broken pipe")[0]
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
> TypeError: 'BrokenPipeError' object is not subscriptable
> 
> For python 2 e[0] seems to work, but is leftover from historic python
> version that used to raise a tuple instead of socket.error instance.
> In python 2.7 library code e.args[0] is used. If we ever port this to
> python 2 this is the best form.
Unfortunately we do have to port this to Python 2, at least while RHEL 7
is a thing we have to deal with:

https://github.com/libguestfs/libguestfs/commit/866d303bce0fee88b118768d1b7b36c87da6200b

Do you know what exactly will work with Python 2?

As for upstream / Python 3, the patch looks fine so I'll push it shortly.

Rich.
>  v2v/rhv-upload-plugin.py | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> index b5dd5521d..5cd6d5cab 100644
> --- a/v2v/rhv-upload-plugin.py
> +++ b/v2v/rhv-upload-plugin.py
> @@ -17,7 +17,6 @@
>  # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>  
>  import builtins
> -import errno
>  import json
>  import logging
>  import socket
> @@ -361,9 +360,8 @@ def pwrite(h, buf, offset):
>  
>      try:
>          http.send(buf)
> -    except socket.error as e:
> -        if e[0] != errno.EPIPE:
> -            raise
> +    except BrokenPipeError:
> +        pass
>  
>      r = http.getresponse()
>      if r.status != 200:
> @@ -425,9 +423,8 @@ def emulate_zero(h, count, offset):
>                  http.send(buf)
>                  count -= len(buf)
>              http.send(buffer(buf, 0, count))
> -        except socket.error as e:
> -            if e[0] != errno.EPIPE:
> -                raise
> +        except BrokenPipeError:
> +            pass
>  
>          r = http.getresponse()
>          if r.status != 200:
> -- 
> 2.17.1
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Tue, Aug 28, 2018 at 11:55 PM Richard W.M. Jones <rjones@redhat.com>
wrote:
> On Tue, Aug 28, 2018 at 11:50:56PM +0300, Nir Soffer wrote:
> > With python 3, we have a nicer way to handle socket.error with errno
set
> > to EPIPE (or ESHUTDOWN).
> >
> > This is also more correct since in some cases (that I could not
> > reproduce yet with v2v), using e[0] with BrokenPipeError will fail
with:
> >
> > >>> OSError(errno.EPIPE, "Broken pipe")[0]
> > Traceback (most recent call last):
> >   File "<stdin>", line 1, in <module>
> > TypeError: 'BrokenPipeError' object is not subscriptable
> >
> > For python 2 e[0] seems to work, but is leftover from historic python
> > version that used to raise a tuple instead of socket.error instance.
> > In python 2.7 library code e.args[0] is used. If we ever port this to
> > python 2 this is the best form.
>
> Unfortunately we do have to port this to Python 2, at least while RHEL 7
> is a thing we have to deal with:
>
>
>
https://github.com/libguestfs/libguestfs/commit/866d303bce0fee88b118768d1b7b36c87da6200b
>
> Do you know what exactly will work with Python 2?
>
Yes - this should be equivalent of "except BrokenPipeError":

    expect socket.error as e:
        if e.args[0] not in (errno.EPIPE, errno.ESHUTDOWN):
            raise

See this for reference:
https://github.com/python/cpython/blob/491740f116755e220135e596ec802ea3a0f65596/Lib/asyncore.py#L115

This also works for hybrid code based that works on both python 2 and 3,
see:
https://gerrit.ovirt.org/c/93278/

As for upstream / Python 3, the patch looks fine so I'll push it
shortly.
>
> Rich.
>
> >  v2v/rhv-upload-plugin.py | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> > index b5dd5521d..5cd6d5cab 100644
> > --- a/v2v/rhv-upload-plugin.py
> > +++ b/v2v/rhv-upload-plugin.py
> > @@ -17,7 +17,6 @@
> >  # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> >
> >  import builtins
> > -import errno
> >  import json
> >  import logging
> >  import socket
> > @@ -361,9 +360,8 @@ def pwrite(h, buf, offset):
> >
> >      try:
> >          http.send(buf)
> > -    except socket.error as e:
> > -        if e[0] != errno.EPIPE:
> > -            raise
> > +    except BrokenPipeError:
> > +        pass
> >
> >      r = http.getresponse()
> >      if r.status != 200:
> > @@ -425,9 +423,8 @@ def emulate_zero(h, count, offset):
> >                  http.send(buf)
> >                  count -= len(buf)
> >              http.send(buffer(buf, 0, count))
> > -        except socket.error as e:
> > -            if e[0] != errno.EPIPE:
> > -                raise
> > +        except BrokenPipeError:
> > +            pass
> >
> >          r = http.getresponse()
> >          if r.status != 200:
> > --
> > 2.17.1
>
> --
> Richard Jones, Virtualization Group, Red Hat
> http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> libguestfs lets you edit virtual machines.  Supports shell scripting,
> bindings from many languages.  http://libguestfs.org
>