| Message ID | 1435261196-8068-1-git-send-email-rjones@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 25, 2015 at 08:39:56PM +0100, Richard W.M. Jones wrote: > Currently if qemu is connected to a curl source (eg. web server), and > the web server fails / times out / dies, you always see a bogus EIO > "Input/output error". > > For example, choose a large file located on any local webserver which > you control: > > $ qemu-img convert -p http://example.com/large.iso /tmp/test > > Once it starts copying the file, stop the webserver and you will see > qemu-img fail with: > > qemu-img: error while reading sector 61440: Input/output error > > This patch does two things: Firstly print the actual error from curl > so it doesn't get lost. Secondly, change EIO to EPROTO. EPROTO is a > POSIX.1 compatible errno which more accurately reflects that there was > a protocol error, rather than some kind of hardware failure. > > After this patch is applied, the error changes to: > > $ qemu-img convert -p http://example.com/large.iso /tmp/test > qemu-img: curl: transfer closed with 469989 bytes remaining to read > qemu-img: error while reading sector 16384: Protocol error > > Signed-off-by: Richard W.M. Jones <rjones@redhat.com> > --- > block/curl.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) According to scripts/get_maintainer.pl this patch goes via Jeff Cody. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On Thu, Jun 25, 2015 at 08:39:56PM +0100, Richard W.M. Jones wrote: > Currently if qemu is connected to a curl source (eg. web server), and > the web server fails / times out / dies, you always see a bogus EIO > "Input/output error". > > For example, choose a large file located on any local webserver which > you control: > > $ qemu-img convert -p http://example.com/large.iso /tmp/test > > Once it starts copying the file, stop the webserver and you will see > qemu-img fail with: > > qemu-img: error while reading sector 61440: Input/output error > > This patch does two things: Firstly print the actual error from curl > so it doesn't get lost. Secondly, change EIO to EPROTO. EPROTO is a > POSIX.1 compatible errno which more accurately reflects that there was > a protocol error, rather than some kind of hardware failure. > > After this patch is applied, the error changes to: > > $ qemu-img convert -p http://example.com/large.iso /tmp/test > qemu-img: curl: transfer closed with 469989 bytes remaining to read > qemu-img: error while reading sector 16384: Protocol error > > Signed-off-by: Richard W.M. Jones <rjones@redhat.com> > --- > block/curl.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/block/curl.c b/block/curl.c > index 3a2b63e..2fd7c06 100644 > --- a/block/curl.c > +++ b/block/curl.c > @@ -22,6 +22,7 @@ > * THE SOFTWARE. > */ > #include "qemu-common.h" > +#include "qemu/error-report.h" > #include "block/block_int.h" > #include "qapi/qmp/qbool.h" > #include "qapi/qmp/qstring.h" > @@ -298,6 +299,12 @@ static void curl_multi_check_completion(BDRVCURLState *s) > /* ACBs for successful messages get completed in curl_read_cb */ > if (msg->data.result != CURLE_OK) { > int i; > + > + /* Don't lose the original error message from curl, since > + * it contains extra data. > + */ > + error_report("curl: %s", state->errmsg); > + > for (i = 0; i < CURL_NUM_ACB; i++) { > CURLAIOCB *acb = state->acb[i]; > > @@ -305,7 +312,7 @@ static void curl_multi_check_completion(BDRVCURLState *s) > continue; > } > > - acb->common.cb(acb->common.opaque, -EIO); > + acb->common.cb(acb->common.opaque, -EPROTO); > qemu_aio_unref(acb); > state->acb[i] = NULL; > } > -- > 2.3.1 > > Thanks, applied to my block tree: https://github.com/codyprime/qemu-kvm-jtc/commits/block Jeff
On 06/25/2015 03:39 PM, Richard W.M. Jones wrote: > Currently if qemu is connected to a curl source (eg. web server), and > the web server fails / times out / dies, you always see a bogus EIO > "Input/output error". > > For example, choose a large file located on any local webserver which > you control: > > $ qemu-img convert -p http://example.com/large.iso /tmp/test > > Once it starts copying the file, stop the webserver and you will see > qemu-img fail with: > > qemu-img: error while reading sector 61440: Input/output error > > This patch does two things: Firstly print the actual error from curl > so it doesn't get lost. Secondly, change EIO to EPROTO. EPROTO is a > POSIX.1 compatible errno which more accurately reflects that there was > a protocol error, rather than some kind of hardware failure. > > After this patch is applied, the error changes to: > > $ qemu-img convert -p http://example.com/large.iso /tmp/test > qemu-img: curl: transfer closed with 469989 bytes remaining to read > qemu-img: error while reading sector 16384: Protocol error > > Signed-off-by: Richard W.M. Jones <rjones@redhat.com> > --- > block/curl.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/block/curl.c b/block/curl.c > index 3a2b63e..2fd7c06 100644 > --- a/block/curl.c > +++ b/block/curl.c > @@ -22,6 +22,7 @@ > * THE SOFTWARE. > */ > #include "qemu-common.h" > +#include "qemu/error-report.h" > #include "block/block_int.h" > #include "qapi/qmp/qbool.h" > #include "qapi/qmp/qstring.h" > @@ -298,6 +299,12 @@ static void curl_multi_check_completion(BDRVCURLState *s) > /* ACBs for successful messages get completed in curl_read_cb */ > if (msg->data.result != CURLE_OK) { > int i; > + > + /* Don't lose the original error message from curl, since > + * it contains extra data. > + */ > + error_report("curl: %s", state->errmsg); > + > for (i = 0; i < CURL_NUM_ACB; i++) { > CURLAIOCB *acb = state->acb[i]; > > @@ -305,7 +312,7 @@ static void curl_multi_check_completion(BDRVCURLState *s) > continue; > } > > - acb->common.cb(acb->common.opaque, -EIO); > + acb->common.cb(acb->common.opaque, -EPROTO); > qemu_aio_unref(acb); > state->acb[i] = NULL; > } > We'll see what Markus says! ;) I think it's a strict improvement. Reviewed-by: John Snow <jsnow@redhat.com>
diff --git a/block/curl.c b/block/curl.c index 3a2b63e..2fd7c06 100644 --- a/block/curl.c +++ b/block/curl.c @@ -22,6 +22,7 @@ * THE SOFTWARE. */ #include "qemu-common.h" +#include "qemu/error-report.h" #include "block/block_int.h" #include "qapi/qmp/qbool.h" #include "qapi/qmp/qstring.h" @@ -298,6 +299,12 @@ static void curl_multi_check_completion(BDRVCURLState *s) /* ACBs for successful messages get completed in curl_read_cb */ if (msg->data.result != CURLE_OK) { int i; + + /* Don't lose the original error message from curl, since + * it contains extra data. + */ + error_report("curl: %s", state->errmsg); + for (i = 0; i < CURL_NUM_ACB; i++) { CURLAIOCB *acb = state->acb[i]; @@ -305,7 +312,7 @@ static void curl_multi_check_completion(BDRVCURLState *s) continue; } - acb->common.cb(acb->common.opaque, -EIO); + acb->common.cb(acb->common.opaque, -EPROTO); qemu_aio_unref(acb); state->acb[i] = NULL; }
Currently if qemu is connected to a curl source (eg. web server), and the web server fails / times out / dies, you always see a bogus EIO "Input/output error". For example, choose a large file located on any local webserver which you control: $ qemu-img convert -p http://example.com/large.iso /tmp/test Once it starts copying the file, stop the webserver and you will see qemu-img fail with: qemu-img: error while reading sector 61440: Input/output error This patch does two things: Firstly print the actual error from curl so it doesn't get lost. Secondly, change EIO to EPROTO. EPROTO is a POSIX.1 compatible errno which more accurately reflects that there was a protocol error, rather than some kind of hardware failure. After this patch is applied, the error changes to: $ qemu-img convert -p http://example.com/large.iso /tmp/test qemu-img: curl: transfer closed with 469989 bytes remaining to read qemu-img: error while reading sector 16384: Protocol error Signed-off-by: Richard W.M. Jones <rjones@redhat.com> --- block/curl.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)