Libguestfs - Nov 2019 - Re: Dealing with ImageIO problems

On Tue, Nov 19, 2019 at 04:14:45PM +0200, Nir Soffer
wrote:
>On Tue, Nov 19, 2019 at 3:36 PM Martin Kletzander
<mkletzan@redhat.com>
>wrote:
>
>> On Tue, Nov 19, 2019 at 01:48:31PM +0100, Martin Kletzander wrote:
>> >On Tue, Nov 19, 2019 at 02:24:11PM +0200, Nir Soffer wrote:
>> >>On Tue, Nov 19, 2019 at 2:16 PM Martin Kletzander
<mkletzan@redhat.com>
>> wrote:
>> >>> On Tue, Nov 19, 2019 at 01:14:23PM +0200, Nir Soffer
wrote:
>> >>> >On Tue, Nov 19, 2019 at 11:17 AM Martin Kletzander
<
>> mkletzan@redhat.com> wrote:
>> >>> >>
>> >>> >> Traceback (most recent call last):
>> >>> >>   File
"/var/tmp/rhvupload.Vw0CIU/rhv-upload-plugin.py", line 97, in
>> >>> open
>> >>> >>     transfer_service.cancel()
>> >>> >> NameError: name 'transfer_service' is not
defined
>> >>> >
>> >>> >Does this fix the error for you?
>> >>> >
>> >>>
>>
https://github.com/nirs/virt-v2v/commit/2f93dbffad81a26445831293ecac213eadffbd57
>> >>> >
>> >>> >I did not test it yet.
>> >>> >
>> >>>
>> >>> If I remove the stray `def finalize_transfer(...)` it
works, yes.
>> >>>
>> >>
>> >>Right, belongs to the next patch.
>> >>
>> >>
>> >>> In the meantime I managed to fix the original issue (which
I needed to
>> >>> revert to
>> >>> test this patch =) ), thankfully it was just caused by my
oVirt host
>> setup.
>> >>>
>> >>> Unfortunately I am getting another issue.  Now nbdkit
fails after all
>> the
>> >>> data
>> >>> is written and it is trying to write the disk id file. 
I'm getting "No
>> >>> such
>> >>> file or directory", so because it is writing to that
file I'm assuming
>> the
>> >>> directory does not exist.  On the other hand it *must*
exists because
>> it's
>> >>> the
>> >>> same one where the python scripts are saved.  So I need to
debug more.
>> >>>
>> >>
>> >>This works for me, are you running a modified plugin?
>> >
>> >No, it is a current master of virt-v2v.  But don't worry about
it now.
>> It is
>> >most probably something with my setup again.  Even though it worked
>> before and
>> >these issues started showing up after an update to virt-v2v. 
I'll try
>> with
>> >current master of nbdkit, libguestfs and virt-v2v first and if that
fails
>> I'll
>> >try different version of virt-v2v and then start debugging it from
the
>> script
>> >itself.  It is a bit tedious as I need to modify the script
in-tree,
>> upload it
>> >to the server, build it there, install it, then run
virt-v2v-wrapper
>> (that's how
>> >I'm testing it now) and then inspect the logs on that remote
machine.
>>
>> Actually looking at the whole log what I'm getting is:
>>
>> nbdkit: python[1]: error:
/var/tmp/rhvupload.jngN1W/rhv-upload-plugin.py:
>> flush: error: ['Traceback (most recent call last):\n', ' 
File
>> "/var/tmp/rhvupload.jngN1W/rhv-upload-plugin.py", line 298,
in flush\n
>> http.request("PATCH", h[\'path\'], body=buf,
headers=headers)\n', '  File
>> "/usr/lib64/python3.7/http/client.py", line 1252, in
request\n
>> self._send_request(method, url, body, headers, encode_chunked)\n',
'  File
>> "/usr/lib64/python3.7/http/client.py", line 1263, in
_send_request\n
>> self.putrequest(method, url, **skips)\n', '  File
>> "/usr/lib64/python3.7/http/client.py", line 1108, in
putrequest\n    raise
>> CannotSendRequest(self.__state)\n',
'http.client.CannotSendRequest:
>> Request-sent\n']
>>
>
>Looks like you cannot communicate with imageio daemon - this is fatal error
>that should fail the
>upload.
>...
>and then after couple of lines, when it is actually closing I get:
>
>>
>> ...
>>
>> finalized after 8.060538530349731 seconds
>>
>
>But we tried to finallize the transfer instead of cancel it.
>
>
>> nbdkit: python[1]: error:
/var/tmp/rhvupload.jngN1W/rhv-upload-plugin.py:
>> close: error: ['Traceback (most recent call last):\n', ' 
File
>> "/var/tmp/rhvupload.jngN1W/rhv-upload-plugin.py", line 362,
in close\n',
>> "FileNotFoundError: [Errno 2] No such file or directory:
>> '/var/tmp/rhvupload.jngN1W/diskid.0'\n"]
>> nbdkit: debug: python: unload plugin
>>
>> So it might be because virt-v2v already removed that directory and did
not
>> wait
>> for nbdkit to completely end.  I'm testing with older commit of
virt-v2v
>> now.
>>
>
>flush() failure probably caused qemu-img to fai, and virt-v2v failed and
>cleaned up
>which is expected.
>
>I think the issue is incorrect error handling in the flush() and other
>handlers:
>
>288 def flush(h):
>289     http = h['http']
>290
>291     # Construct the JSON request for flushing.
>292     buf = json.dumps({'op': "flush"}).encode()
>293
>294     headers = {"Content-Type": "application/json",
>295                "Content-Length": str(len(buf))}
>296
>297     http.request("PATCH", h['path'], body=buf,
headers=headers)
>
>If this raised (like in your traceback, we don't set h['failed']
= True
>
>299     r = http.getresponse()
>300     if r.status != 200:
>301         request_failed(h, r, "could not flush")
>302
>303     r.read()
>
>Same here.
>
>Same issue in all handlers. We should handle all errors in the handlers.
>
>But the root cause is something else - why you cannot communicate with
>imageio daemon?
That is something I would love to find out, but I don't know what to try
next.
I'm probably tired of spending a whole day on this, especially when I just
need
it to test something else =)

On Tue, Nov 19, 2019 at 4:34 PM Martin Kletzander <mkletzan@redhat.com>
wrote:
>
> On Tue, Nov 19, 2019 at 04:14:45PM +0200, Nir Soffer wrote:
> >On Tue, Nov 19, 2019 at 3:36 PM Martin Kletzander
<mkletzan@redhat.com>
> >wrote:
> >
> >> On Tue, Nov 19, 2019 at 01:48:31PM +0100, Martin Kletzander wrote:
> >> >On Tue, Nov 19, 2019 at 02:24:11PM +0200, Nir Soffer wrote:
> >> >>On Tue, Nov 19, 2019 at 2:16 PM Martin Kletzander
<mkletzan@redhat.com>
> >> wrote:
> >> >>> On Tue, Nov 19, 2019 at 01:14:23PM +0200, Nir Soffer
wrote:
> >> >>> >On Tue, Nov 19, 2019 at 11:17 AM Martin
Kletzander <
> >> mkletzan@redhat.com> wrote:
> >> >>> >>
> >> >>> >> Traceback (most recent call last):
> >> >>> >>   File
"/var/tmp/rhvupload.Vw0CIU/rhv-upload-plugin.py", line 97, in
> >> >>> open
> >> >>> >>     transfer_service.cancel()
> >> >>> >> NameError: name 'transfer_service'
is not defined
> >> >>> >
> >> >>> >Does this fix the error for you?
> >> >>> >
> >> >>>
> >>
https://github.com/nirs/virt-v2v/commit/2f93dbffad81a26445831293ecac213eadffbd57
> >> >>> >
> >> >>> >I did not test it yet.
> >> >>> >
> >> >>>
> >> >>> If I remove the stray `def finalize_transfer(...)` it
works, yes.
> >> >>>
> >> >>
> >> >>Right, belongs to the next patch.
> >> >>
> >> >>
> >> >>> In the meantime I managed to fix the original issue
(which I needed to
> >> >>> revert to
> >> >>> test this patch =) ), thankfully it was just caused
by my oVirt host
> >> setup.
> >> >>>
> >> >>> Unfortunately I am getting another issue.  Now nbdkit
fails after all
> >> the
> >> >>> data
> >> >>> is written and it is trying to write the disk id
file.  I'm getting "No
> >> >>> such
> >> >>> file or directory", so because it is writing to
that file I'm assuming
> >> the
> >> >>> directory does not exist.  On the other hand it
*must* exists because
> >> it's
> >> >>> the
> >> >>> same one where the python scripts are saved.  So I
need to debug more.
> >> >>>
> >> >>
> >> >>This works for me, are you running a modified plugin?
> >> >
> >> >No, it is a current master of virt-v2v.  But don't worry
about it now.
> >> It is
> >> >most probably something with my setup again.  Even though it
worked
> >> before and
> >> >these issues started showing up after an update to virt-v2v. 
I'll try
> >> with
> >> >current master of nbdkit, libguestfs and virt-v2v first and if
that fails
> >> I'll
> >> >try different version of virt-v2v and then start debugging it
from the
> >> script
> >> >itself.  It is a bit tedious as I need to modify the script
in-tree,
> >> upload it
> >> >to the server, build it there, install it, then run
virt-v2v-wrapper
> >> (that's how
> >> >I'm testing it now) and then inspect the logs on that
remote machine.
> >>
> >> Actually looking at the whole log what I'm getting is:
> >>
> >> nbdkit: python[1]: error:
/var/tmp/rhvupload.jngN1W/rhv-upload-plugin.py:
> >> flush: error: ['Traceback (most recent call last):\n',
'  File
> >> "/var/tmp/rhvupload.jngN1W/rhv-upload-plugin.py", line
298, in flush\n
> >> http.request("PATCH", h[\'path\'], body=buf,
headers=headers)\n', '  File
> >> "/usr/lib64/python3.7/http/client.py", line 1252, in
request\n
> >> self._send_request(method, url, body, headers,
encode_chunked)\n', '  File
> >> "/usr/lib64/python3.7/http/client.py", line 1263, in
_send_request\n
> >> self.putrequest(method, url, **skips)\n', '  File
> >> "/usr/lib64/python3.7/http/client.py", line 1108, in
putrequest\n    raise
> >> CannotSendRequest(self.__state)\n',
'http.client.CannotSendRequest:
> >> Request-sent\n']
> >>
> >
> >Looks like you cannot communicate with imageio daemon - this is fatal
error
> >that should fail the
> >upload.
> >...
> >and then after couple of lines, when it is actually closing I get:
> >
> >>
> >> ...
> >>
> >> finalized after 8.060538530349731 seconds
> >>
> >
> >But we tried to finallize the transfer instead of cancel it.
> >
> >
> >> nbdkit: python[1]: error:
/var/tmp/rhvupload.jngN1W/rhv-upload-plugin.py:
> >> close: error: ['Traceback (most recent call last):\n',
'  File
> >> "/var/tmp/rhvupload.jngN1W/rhv-upload-plugin.py", line
362, in close\n',
> >> "FileNotFoundError: [Errno 2] No such file or directory:
> >> '/var/tmp/rhvupload.jngN1W/diskid.0'\n"]
> >> nbdkit: debug: python: unload plugin
> >>
> >> So it might be because virt-v2v already removed that directory and
did not
> >> wait
> >> for nbdkit to completely end.  I'm testing with older commit
of virt-v2v
> >> now.
> >>
> >
> >flush() failure probably caused qemu-img to fai, and virt-v2v failed
and
> >cleaned up
> >which is expected.
> >
> >I think the issue is incorrect error handling in the flush() and other
> >handlers:
> >
> >288 def flush(h):
> >289     http = h['http']
> >290
> >291     # Construct the JSON request for flushing.
> >292     buf = json.dumps({'op': "flush"}).encode()
> >293
> >294     headers = {"Content-Type":
"application/json",
> >295                "Content-Length": str(len(buf))}
> >296
> >297     http.request("PATCH", h['path'], body=buf,
headers=headers)
> >
> >If this raised (like in your traceback, we don't set
h['failed'] = True
> >
> >299     r = http.getresponse()
> >300     if r.status != 200:
> >301         request_failed(h, r, "could not flush")
> >302
> >303     r.read()
> >
> >Same here.
> >
> >Same issue in all handlers. We should handle all errors in the
handlers.
> >
> >But the root cause is something else - why you cannot communicate with
> >imageio daemon?
>
> That is something I would love to find out, but I don't know what to
try next.
Can you share entire virt-v2v log, and /var/log/ovirt-imageio-daemon/daemon.log
from the host you are uploading to?

if you use the proxy (transfer.proxy_url), which you should not, also the proxy
log from the engine host (/var/log/ovirt-imageio-proxy/image-proxy.log).

On Tue, Nov 19, 2019 at 06:16:57PM +0200, Nir Soffer
wrote:
>On Tue, Nov 19, 2019 at 4:34 PM Martin Kletzander
<mkletzan@redhat.com> wrote:
>>On Tue, Nov 19, 2019 at 04:14:45PM +0200, Nir Soffer wrote:
>>>But the root cause is something else - why you cannot communicate
with
>>>imageio daemon?
>>
>>That is something I would love to find out, but I don't know what to
try next.
>
>Can you share entire virt-v2v log, and
/var/log/ovirt-imageio-daemon/daemon.log
>from the host you are uploading to?
>
>if you use the proxy (transfer.proxy_url), which you should not, also the
proxy
>log from the engine host (/var/log/ovirt-imageio-proxy/image-proxy.log).
>
I'll follow up personally as this is probably not related to anyone on this
list
and I feel like spamming everyone.

[Adding the mailing list address back]

On Thu, Nov 21, 2019 at 12:44:38PM +0200, Nir Soffer
wrote:
> On Thu, Nov 21, 2019 at 12:16 PM Richard W.M. Jones
<rjones@redhat.com> wrote:
> >
> > On Wed, Nov 20, 2019 at 06:00:52PM +0200, Nir Soffer wrote:
> > > 5. can_zero fails, not sure why (Maybe Richard knows more about
this)
> > >
> > > nbdkit: python[1]: debug: python: can_zero
> > > nbdkit: python[1]: debug: sending error reply: Operation not
supported
> > >
> > > rhv-upload-plugin always reports that it can zero, I cannot
explain this.
> >
> > I believe this is a bug in nbdkit’s python bindings.  Will take a
> > closer look and reply in a new thread.  I don't believe it affects
> > anything here however.
> >
> > > 6. writing the first sector fails
> > >
> > > python[1]: nbdkit: debug: vddk[3]python: pwrite count=65536
offset=0 fua=0:
> > > ...
> > > nbdkit: python[1]: error:
> > > /var/tmp/rhvupload.RShGz0/rhv-upload-plugin.py: pwrite: error:
> > > ['Traceback (most recent call last):\n', '  File
> > > "/var/tmp/rhvupload.RShGz0/rhv-upload-plugin.py", line
189, in
> > > pwrite\n    http.endheaders()\n', '  File
> > > "/usr/lib64/python3.7/http/client.py", line 1247, in
endheaders\n
> > > self._send_output(message_body,
encode_chunked=encode_chunked)\n', '
> > > File "/usr/lib64/python3.7/http/client.py", line 1026,
in
> > > _send_output\n    self.send(msg)\n', '  File
> > > "/usr/lib64/python3.7/http/client.py", line 966, in
send\n
> > > self.connect()\n', '  File
> > > "/var/tmp/rhvupload.RShGz0/rhv-upload-plugin.py", line
357, in
> > > connect\n    self.sock.connect(self.path)\n',
'ConnectionRefusedError:
> > > [Errno 111] Connection refused\n']
> > >
> > > But it fails in send(), and we don't handle errors proeply
around
> > > send(), only around getresponse().
> > > So the error is forgotten by the plugin.
> >
> > That sounds bad?
> 
> Indeed, but we never had this issue in real environment.
>
> Basically we should handle any exception in the plugin as failure,
> but I'm not sure where we should fix it.
>
> We can add error handler around all the entry points
> (e.g. pwrite()), setting h['failed'] = True. But this makes it
> harder to write correct plugins; everyone has to ensure correct
> error handling instead of getting it right by default.
In nbdkit-python-plugin, any exception which escapes will be caught in
the C code and turned into an NBD error on the wire.  The function
which does this is:

 
https://github.com/libguestfs/nbdkit/blob/a52ebb4e8d18a73b9ae009cd768bfab18505f59a/plugins/python/python.c#L189

Since each plugin method corresponds* (* = mostly) to a single NBD
command on the wire, returning an error from the method signals an
error back to the NBD client.  If the NBD client is ‘qemu-img convert’
then it will exit the first time it sees an error.

(The retry filter http://libguestfs.org/nbdkit-retry-filter.1.html
changes this behaviour, but we are not using that for output to
rhv-upload.)
> We can handle this in the python plugin (e.g. h['failed'] = True)
> but the handle is controlled by the plugin, so this does not sound
> like a good way.
The close() method is always called, whether or not there was an
error.  The same connection handle (‘h’) is passed back to the close
method.  It's quite safe and normal to store state in this handle.

nbdkit itself doesn't save the fact that there was an error during the
connection (it can't since it doesn't know if the error was
recoverable or not).  But rhv-upload-plugin needs to handle failed vs
successful transfers differently.  Thus we save the failure state in
the handle the first time we see an error.
> Maybe we can change close() to:
> 
>     def close(failed=True):
>         if failed:
>             # cleanup after some call failed
>             ...
> 
>         # Cleanup after successful run
> 
> The python plugin will remember unhandled errors and will call:
> 
>     close(failed=True)
> 
> To write correct plugin author must not handle fatal errors in the plugin.
This changes how the Python plugin would work versus other plugins,
and the C code can't know if the NBD connection failed, because some
NBD errors are recoverable.  Also the client does not send us any
error indication when it disconnects.

The answer is probably to wrap every method in a big try...except
which sets the failed flag in the handle.
> How do we handle errors in other plugins?
It's kind of easier to study this using the sh plugin.  Here's an
example showing the order in which callbacks happen on error:

----------------------------------------------------------------------
nbdkit sh -fv - --run 'qemu-img convert $nbd /tmp/out'
<<'EOF'
case "$1" in
get_size) echo 512 ;;
pread) echo 'EIO error' >&2; exit 1 ;;
*) exit 2 ;;
esac
EOF
----------------------------------------------------------------------

Running this gives the following output (filtered a bit to show the
key points):

  $ bash test.sh 
  nbdkit: debug: sh: load
  nbdkit: debug: sh: load: tmpdir: /tmp/nbdkitshB6KCs0
  nbdkit: debug: sh: config key=script, value=-
  nbdkit: debug: sh: config_complete
  nbdkit: sh[1]: debug: sh: open readonly=0
  nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh open false
""
  nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh open:
status 2
  nbdkit: sh[1]: debug: sh: open returned handle 0x7face4002070
  nbdkit: sh[1]: debug: sh: get_size
  nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh get_size
""
  nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh
get_size: status 0
  nbdkit: sh[1]: debug: sh: can_write
  nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh can_write
""
  nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh
can_write: status 2
...
  nbdkit: sh[1]: debug: sh: pread count=512 offset=0
  nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh pread
"" 512 0
  nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh pread:
status 1
  nbdkit: sh[1]: error: /tmp/nbdkitshB6KCs0/inline-script.sh: error
  nbdkit: sh[1]: debug: sending error reply: Input/output error
  nbdkit: sh[1]: debug: client sent NBD_CMD_DISC, closing connection
  qemu-img: Could not open 'nbd:localhost:10809': Could not read image
for determining its format: Input/output error
  nbdkit: sh[1]: debug: sh: close
  nbdkit: sh[1]: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh close
""
  nbdkit: sh[1]: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh close:
status 2
  nbdkit: debug: sh: unload plugin
  nbdkit: debug: calling: /tmp/nbdkitshB6KCs0/inline-script.sh unload
  nbdkit: debug: completed: /tmp/nbdkitshB6KCs0/inline-script.sh unload: status
2

Notice in particular how close() and unload() are both called in the
plugin even though qemu-img convert has already exited with an error.

Also the same handle (empty string above) is passed back to close().
> We probably should continue to the mailing list.
> 
> Nir
Thanks, Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Thu, Nov 21, 2019 at 1:39 PM Richard W.M. Jones <rjones@redhat.com>
wrote:
>
> [Adding the mailing list address back]
>
> On Thu, Nov 21, 2019 at 12:44:38PM +0200, Nir Soffer wrote:
> > On Thu, Nov 21, 2019 at 12:16 PM Richard W.M. Jones
<rjones@redhat.com> wrote:
> > >
> > > On Wed, Nov 20, 2019 at 06:00:52PM +0200, Nir Soffer wrote:
> > > > 5. can_zero fails, not sure why (Maybe Richard knows more
about this)
> > > >
> > > > nbdkit: python[1]: debug: python: can_zero
> > > > nbdkit: python[1]: debug: sending error reply: Operation not
supported
> > > >
> > > > rhv-upload-plugin always reports that it can zero, I cannot
explain this.
> > >
> > > I believe this is a bug in nbdkit’s python bindings.  Will take a
> > > closer look and reply in a new thread.  I don't believe it
affects
> > > anything here however.
> > >
> > > > 6. writing the first sector fails
> > > >
> > > > python[1]: nbdkit: debug: vddk[3]python: pwrite count=65536
offset=0 fua=0:
> > > > ...
> > > > nbdkit: python[1]: error:
> > > > /var/tmp/rhvupload.RShGz0/rhv-upload-plugin.py: pwrite:
error:
> > > > ['Traceback (most recent call last):\n', '  File
> > > > "/var/tmp/rhvupload.RShGz0/rhv-upload-plugin.py",
line 189, in
> > > > pwrite\n    http.endheaders()\n', '  File
> > > > "/usr/lib64/python3.7/http/client.py", line 1247,
in endheaders\n
> > > > self._send_output(message_body,
encode_chunked=encode_chunked)\n', '
> > > > File "/usr/lib64/python3.7/http/client.py", line
1026, in
> > > > _send_output\n    self.send(msg)\n', '  File
> > > > "/usr/lib64/python3.7/http/client.py", line 966,
in send\n
> > > > self.connect()\n', '  File
> > > > "/var/tmp/rhvupload.RShGz0/rhv-upload-plugin.py",
line 357, in
> > > > connect\n    self.sock.connect(self.path)\n',
'ConnectionRefusedError:
> > > > [Errno 111] Connection refused\n']
> > > >
> > > > But it fails in send(), and we don't handle errors
proeply around
> > > > send(), only around getresponse().
> > > > So the error is forgotten by the plugin.
> > >
> > > That sounds bad?
> >
> > Indeed, but we never had this issue in real environment.
> >
> > Basically we should handle any exception in the plugin as failure,
> > but I'm not sure where we should fix it.
> >
> > We can add error handler around all the entry points
> > (e.g. pwrite()), setting h['failed'] = True. But this makes it
> > harder to write correct plugins; everyone has to ensure correct
> > error handling instead of getting it right by default.
>
> In nbdkit-python-plugin, any exception which escapes will be caught in
> the C code and turned into an NBD error on the wire.  The function
> which does this is:
>
>  
https://github.com/libguestfs/nbdkit/blob/a52ebb4e8d18a73b9ae009cd768bfab18505f59a/plugins/python/python.c#L189
>
> Since each plugin method corresponds* (* = mostly) to a single NBD
> command on the wire, returning an error from the method signals an
> error back to the NBD client.  If the NBD client is ‘qemu-img convert’
> then it will exit the first time it sees an error.
>
> (The retry filter http://libguestfs.org/nbdkit-retry-filter.1.html
> changes this behaviour, but we are not using that for output to
> rhv-upload.)
>
> > We can handle this in the python plugin (e.g. h['failed'] =
True)
> > but the handle is controlled by the plugin, so this does not sound
> > like a good way.
>
> The close() method is always called, whether or not there was an
> error.  The same connection handle (‘h’) is passed back to the close
> method.  It's quite safe and normal to store state in this handle.
>
> nbdkit itself doesn't save the fact that there was an error during the
> connection (it can't since it doesn't know if the error was
> recoverable or not).  But rhv-upload-plugin needs to handle failed vs
> successful transfers differently.  Thus we save the failure state in
> the handle the first time we see an error.
>
> > Maybe we can change close() to:
> >
> >     def close(failed=True):
> >         if failed:
> >             # cleanup after some call failed
> >             ...
> >
> >         # Cleanup after successful run
> >
> > The python plugin will remember unhandled errors and will call:
> >
> >     close(failed=True)
> >
> > To write correct plugin author must not handle fatal errors in the
plugin.
>
> This changes how the Python plugin would work versus other plugins,
> and the C code can't know if the NBD connection failed, because some
> NBD errors are recoverable.  Also the client does not send us any
> error indication when it disconnects.
>
> The answer is probably to wrap every method in a big try...except
> which sets the failed flag in the handle.
Implemented in
https://www.redhat.com/archives/libguestfs/2019-November/msg00147.html

This works with current plugin, and can be backported to downstream if needed.

However it makes it harder to write plugins in python. The basic idea is that
not handling an error in python will crash with a traceback *without* having to
do anything. It would be best if the python plugin ensure this.

If we want to support recoverable errors, we can define a special
RecoverableError
exception that signal the python plugin that this request failed, but
the error is not
fatal.

We can also define more specific errors like UnsupportedOperation, for
example if
plugin's zero() fail and the author wants nbdkit to stop calling
zero() and emulate
it with write.

Nir