[libvirt] [libvirt-python][PATCH 3/4] virStream: Introduce virStreamSparse{Recv, Send}All

[Martin Kletzander]

On Mon, May 22, 2017 at 12:57:14PM +0200, Michal Privoznik wrote:
>Yet again, our parser is not capable of generating proper
>wrapper. To be fair, this one wold be really tough anyway.
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> generator.py                  |   2 +
> libvirt-override-virStream.py | 117 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 119 insertions(+)
>
>diff --git a/generator.py b/generator.py
>index 0e07fc8..93d1dc3 100755
>--- a/generator.py
>+++ b/generator.py
>@@ -546,6 +546,8 @@ skip_function = (
>     'virStreamRecvHole', # overridden in libvirt-override-virStream.py
>     'virStreamSendHole', # overridden in libvirt-override-virStream.py
>     'virStreamRecvFlags', # overridden in libvirt-override-virStream.py
>+    'virStreamSparseRecvAll', # overridden in libvirt-override-virStream.py
>+    'virStreamSparseSendAll', # overridden in libvirt-override-virStream.py
>
>     'virConnectUnregisterCloseCallback', # overridden in virConnect.py
>     'virConnectRegisterCloseCallback', # overridden in virConnect.py
>diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py
>index 66d2bf6..568bd9f 100644
>--- a/libvirt-override-virStream.py
>+++ b/libvirt-override-virStream.py
>@@ -164,3 +164,120 @@
>         ret = libvirtmod.virStreamRecvFlags(self._o, nbytes, flags)
>         if ret is None: raise libvirtError ('virStreamRecvFlags() failed')
>         return ret
>+
>+    def sparseRecvAll(self, handler, holeHandler, opaque):
>+        """Receive the entire data stream, sending the data to
>+        the requested data sink handler and calling the skip
>+        holeHandler to generate holes for sparse stream targets.
>+        This is simply a convenient alternative to recvFlags, for
>+        apps that do blocking-I/O.
>+

I would add "And want to preserve sparseness."

>+        Hypothetical callbacks can look like this:
>+
>+            def handler(stream, # virStream instance
>+                        buf,    # string containing received data
>+                        opaque): # extra data passed to sparseRecvAll as opaque
>+                fd = opaque
>+                return os.write(fd, buf)
>+

So this is the same handler example as in recvAll(), but ... [1]

>+            def holeHandler(stream, # virStream instance
>+                            length, # number of bytes to skip
>+                            opaque): # extra data passed to sparseRecvAll as opaque
>+                fd = opaque
>+                cur = os.lseek(fd, length, os.SEEK_CUR)
>+                return os.ftruncate(fd, cur) # take this extra step to
>+                                             # actually allocate the hole
>+        """
>+        while True:
>+            want = 64 * 1024
>+            got = self.recvFlags(want, VIR_STREAM_RECV_STOP_AT_HOLE)
>+            if got == -2:
>+                raise libvirtError("cannot use sparseRecvAll with "
>+                                   "nonblocking stream")
>+            if got == -3:
>+                length = self.recvHole()
>+                if (length is None):

No need for the parentheses.

>+                    self.abort()
>+                    raise RuntimeError("recvHole handler failed")
>+                try:
>+                   ret = holeHandler(self, length, opaque)

[1] ... you are handling the return value (or exception in this case)
differently.  I think you should check for the return value and change
the example.  The reasoning behind that is that you can get so many
exceptions that you can't easily differentiate between those you want
and those you don't.  Moreover you are catching *all* exceptions, which
you should not do.  In some versions you can get InterruptedError, but
not in newer ones, you can get KeyboardInterrupt, and it should be the
handler's job to decide what to do.

>+                except:
>+                    self.abort()
>+                continue
>+
>+            if len(got) == 0:
>+                break
>+
>+            try:
>+                ret = handler(self, got, opaque)
>+                if type(ret) is int and ret < 0:
>+                    raise RuntimeError("recvAll handler returned %d" % ret)

See, here you check for the return value.

>+            except Exception:
>+                e = sys.exc_info()[1]

You can just do:
except Exception as e:

>+                try:
>+                    self.abort()
>+                except:
>+                    pass
>+                raise e

And now you're even wrapping the abort() in try/except, but you weren't
before.  I would say that if there's an exception from almost anywhere,
we should just not catch it because most of the decisions on what to
catch will be wrong in some situation.

same for SendAll.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170523/81516b0f/attachment-0001.sig>