search for: request_failed

...reate a VM with an empty or partially uploaded disk. Decorate all the NBD hander functions with a @failing decorator, ensuring that any error in the decorated function will mark the handle as failed. Since errors are handled now by @failing decorator, remove the code to mark a handle as failed in request_failed(). --- I tested 3 flows by injecting errors in imageio server: ## Error in flush() nbdkit: python[1]: debug: pwrite count=65536 offset=6442385408 fua=0     (100.00/100%) nbdkit: python[1]: debug: flush unexpected response from imageio server: could not flush 403: Forbidden b'no flush for you...

v1 was here: https://www.redhat.com/archives/libguestfs/2018-June/msg00015.html v2 fixes all the issues raised in Nir's review. I tested it against ovirt-engine-4.2.4.1-1.el7.noarch and it works, although I wasn't easily able to trigger the error path so that code is not really tested (except for syntax checking). Rich.

...+228,32 @@ def can_flush(h): def get_size(h): return params['disk_size'] +# Any unexpected HTTP response status from the server will end up +# calling this function which logs the full error, pauses the +# transfer, sets the failed state, and raises a RuntimeError +# exception. +def request_failed(h, r, msg): + # Setting the failed flag in the handle causes the disk to be + # cleaned up on close. + h['failed'] = True + h['transfer_service'].pause() + + status = r.status + reason = r.reason + try: + body = r.read() + except EnvironmentError as e:...

.... If the server does not support zero, the client + # emulates it by streaming zeroes to server. + return True @failing def get_size(h): - return params['disk_size'] + client = h['client'] + try: + return client.size() + except Exception as e: + request_failed("cannot get size", e) # Any unexpected HTTP response status from the server will end up calling this # function which logs the full error, and raises a RuntimeError exception. -def request_failed(r, msg): - status = r.status - reason = r.reason - try: - body = r.read...

On Tue, Aug 21, 2018 at 03:55:50PM +0300, Nir Soffer wrote: > On Tue, Aug 21, 2018 at 2:49 PM Richard W.M. Jones <rjones@redhat.com> > wrote: > > It seems more likely that whatever version of nbdkit being used does > > not include: > > > > > > https://github.com/libguestfs/nbdkit/commit/72c0d64a47db642cafa89884f2ee554bd0b8e822 > > > > This has

...worse. Detected by pylint. --- v2v/rhv-upload-plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index 3272c3ce3..e651bc686 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -321,41 +321,41 @@ def request_failed(h, r, msg): body = "(Unable to read response body: %s)" % e # Log the full error if we're verbose. debug("unexpected response from imageio server:") debug(msg) debug("%d: %s" % (status, reason)) debug(body) # Only a short...

...e" that are not used in current code. --- v2v/rhv-upload-plugin.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py index 10887c031..5c036c46a 100644 --- a/v2v/rhv-upload-plugin.py +++ b/v2v/rhv-upload-plugin.py @@ -263,7 +263,6 @@ def request_failed(h, r, msg): def pread(h, count, offset): http = h['http'] transfer = h['transfer'] - transfer_service = h['transfer_service'] headers = {"Range", "bytes=%d-%d" % (offset, offset+count-1)} # Authorization is only needed for old im...

...on.loads(data) h['can_zero'] = "zero" in j['features'] h['can_trim'] = "trim" in j['features'] h['can_flush'] = "flush" in j['features'] @@ -276,6 +278,7 @@ def pread(h, count, offset): request_failed(h, r, "could not read sector offset %d size %d" % (offset, count)) + return r.read() def pwrite(h, buf, offset): @@ -302,6 +305,8 @@ def pwrite(h, buf, offset): "could not write sector offset %d size %d&q...

Fix NameError introduced by inocomplete change to extract create_transfer() by extracting cancel_transfer() function. Finally extract finallize_transfer() function, dealing with the poorly documented and over complicated oVirt SDK. Tested with: libguestfs-1.40.2-4.fc30.x86_64 nbdkit-1.12.8-1.fc30.x86_64 Tested flows: - Successful import - Error in close() - by injecting long sleep in vdsm

On Tue, Aug 21, 2018 at 01:15:02AM +0300, Nir Soffer wrote: > The oVirt server may fail a PUT request before reading all request body. > However before closing the connection, it writes a detailed error > response, that will make debugging issues like expired tickets much > easier to handle. If we don't get the response and log the error, the > error may be lost when the daemon

...es the > -# transfer, sets the failed state, and raises a RuntimeError > -# exception. > +# Any unexpected HTTP response status from the server will end up calling this > +# function which logs the full error, sets the failed state, and raises a > +# RuntimeError exception. > def request_failed(h, r, msg): > - # Setting the failed flag in the handle causes the disk to be > - # cleaned up on close. > + # Setting the failed flag in the handle will cancel the transfer on close. > h['failed'] = True > - h['transfer_service'].pause() > >...

...; len(buf): http.send(buf) count -= len(buf) - http.send(buffer(buf, 0, count)) + http.send(memoryview(buf)[:count]) except BrokenPipeError: pass r = http.getresponse() if r.status != 200: request_failed(h, r, "could not write zeroes offset %d size %d" % (offset, count)) r.read() def trim(h, count, offset): http = h['http'] # Construct the JSON request for trimming. buf = json.dumps({'op'...

...the daemon log is rotated. > > > > This series looks fine, I'll push it in a minute. > > > > Couple of questions: > > > > (1) Are we using the nbdkit enhanced Python error reporting yet? > > What's stopping us if we are not? > > > > In request_failed() we raise RuntimeError, so it should log a traceback, but > I did not see a traceback, maybe because I did not test it in verbose mode. If you see anything at all, then you'd see the traceback. It seems more likely that whatever version of nbdkit being used does not include: https://gi...

...og the error, the > > error may be lost when the daemon log is rotated. > > This series looks fine, I'll push it in a minute. > > Couple of questions: > > (1) Are we using the nbdkit enhanced Python error reporting yet? > What's stopping us if we are not? > In request_failed() we raise RuntimeError, so it should log a traceback, but I did not see a traceback, maybe because I did not test it in verbose mode. (2) Is there a BZ associated with this change? If not, we'll need one > if you want to get these changes into RHEL. > Not yet, the only thing we have i...

...gt; > This series looks fine, I'll push it in a minute. > > > > > > Couple of questions: > > > > > > (1) Are we using the nbdkit enhanced Python error reporting yet? > > > What's stopping us if we are not? > > > > > > > In request_failed() we raise RuntimeError, so it should log a traceback, > but > > I did not see a traceback, maybe because I did not test it in verbose > mode. > > If you see anything at all, then you'd see the traceback. > > It seems more likely that whatever version of nbdkit being use...

...error: ['Traceback (most recent call last):\n', ' File "/home/nsoffer/src/libguestfs/tmp/v2v.ayEM8r/rhv-upload-plugin.py", line 373, in pwrite\n (offset, count))\n', ' File "/home/nsoffer/src/libguestfs/tmp/v2v.ayEM8r/rhv-upload-plugin.py", line 321, in request_failed\n raise RuntimeError("%s: %d %s: %r" % (msg, status, reason, body[:200]))\n', 'RuntimeError: could not write sector offset 1897857024 size 65536: 403 Forbidden: b\'{"explanation": "Access was denied to this resource.", "code": 403, "detail...

...gt; count -= len(buf) > - http.send(buffer(buf, 0, count)) > + http.send(memoryview(buf)[:count]) > except BrokenPipeError: > pass > > r = http.getresponse() > if r.status != 200: > request_failed(h, r, > "could not write zeroes offset %d size %d" % > (offset, count)) > > r.read() > > def trim(h, count, offset): > http = h['http'] > > # Construct the JSON request for...

...hich logs the full error, pauses the -# transfer, sets the failed state, and raises a RuntimeError -# exception. +# Any unexpected HTTP response status from the server will end up calling this +# function which logs the full error, sets the failed state, and raises a +# RuntimeError exception. def request_failed(h, r, msg): - # Setting the failed flag in the handle causes the disk to be - # cleaned up on close. + # Setting the failed flag in the handle will cancel the transfer on close. h['failed'] = True - h['transfer_service'].pause() status = r.status reason...

...ailed state, and raises a RuntimeError > > -# exception. > > +# Any unexpected HTTP response status from the server will end up > calling this > > +# function which logs the full error, sets the failed state, and raises > a > > +# RuntimeError exception. > > def request_failed(h, r, msg): > > - # Setting the failed flag in the handle causes the disk to be > > - # cleaned up on close. > > + # Setting the failed flag in the handle will cancel the transfer on > close. > > h['failed'] = True > > - h['transfer_serv...

..."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?