src/remote/remote_daemon_stream.c | 24 ++++++++++++------------ src/util/virfdstream.c | 5 +---- 2 files changed, 13 insertions(+), 16 deletions(-)
When a length of a file is defined, the responsible thread to send
stream is finishing inappropriately. It is happening because there is
wrong conditional which compares an offset with the length and because
of that the code throws ENOSPC error.
To test it:
virsh# vol-upload ... --length N
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
src/remote/remote_daemon_stream.c | 24 ++++++++++++------------
src/util/virfdstream.c | 5 +----
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c
index 4dd3af9e0..998e82a83 100644
--- a/src/remote/remote_daemon_stream.c
+++ b/src/remote/remote_daemon_stream.c
@@ -549,21 +549,21 @@ daemonStreamHandleWriteData(virNetServerClientPtr client,
} else if (ret == -2) {
/* Blocking, so indicate we have more todo later */
return 1;
- } else {
- virNetMessageError rerr;
+ } else if (ret) {
+ virNetMessageError rerr;
- memset(&rerr, 0, sizeof(rerr));
+ memset(&rerr, 0, sizeof(rerr));
- VIR_INFO("Stream send failed");
- stream->closed = true;
- virStreamEventRemoveCallback(stream->st);
- virStreamAbort(stream->st);
+ VIR_INFO("Stream send failed");
+ stream->closed = true;
+ virStreamEventRemoveCallback(stream->st);
+ virStreamAbort(stream->st);
- return virNetServerProgramSendReplyError(stream->prog,
- client,
- msg,
- &rerr,
- &msg->header);
+ return virNetServerProgramSendReplyError(stream->prog,
+ client,
+ msg,
+ &rerr,
+ &msg->header);
}
return 0;
diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index be40379a9..c3198c768 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -627,9 +627,6 @@ virFDStreamThread(void *opaque)
if (got < 0)
goto error;
- if (got == 0)
- break;
-
total += got;
}
@@ -783,7 +780,7 @@ static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes)
virObjectLock(fdst);
if (fdst->length) {
- if (fdst->length == fdst->offset) {
+ if (fdst->offset > fdst->length) {
virReportSystemError(ENOSPC, "%s",
_("cannot write to stream"));
virObjectUnlock(fdst);
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I'm not sure about this part. When offset is equal of length, nbytes is 0 and the function will return 0. Do you see any possible problems to remove this part? I checked all ret = 0 and I'm not seeing problems until now. > - if (got == 0) > - break; > - -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Sun, Mar 04, 2018 at 16:24:21 -0300, Julio Faracco wrote:
> When a length of a file is defined, the responsible thread to send
> stream is finishing inappropriately. It is happening because there is
> wrong conditional which compares an offset with the length and because
> of that the code throws ENOSPC error.
>
> To test it:
> virsh# vol-upload ... --length N
>
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
> src/remote/remote_daemon_stream.c | 24 ++++++++++++------------
> src/util/virfdstream.c | 5 +----
> 2 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c
> index 4dd3af9e0..998e82a83 100644
> --- a/src/remote/remote_daemon_stream.c
> +++ b/src/remote/remote_daemon_stream.c
> @@ -549,21 +549,21 @@ daemonStreamHandleWriteData(virNetServerClientPtr client,
> } else if (ret == -2) {
> /* Blocking, so indicate we have more todo later */
> return 1;
> - } else {
> - virNetMessageError rerr;
> + } else if (ret) {
> + virNetMessageError rerr;
>
> - memset(&rerr, 0, sizeof(rerr));
> + memset(&rerr, 0, sizeof(rerr));
>
> - VIR_INFO("Stream send failed");
> - stream->closed = true;
> - virStreamEventRemoveCallback(stream->st);
> - virStreamAbort(stream->st);
> + VIR_INFO("Stream send failed");
> + stream->closed = true;
> + virStreamEventRemoveCallback(stream->st);
> + virStreamAbort(stream->st);
It does not seem to be necessary to change indentation because of your
change here.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/04/2018 08:24 PM, Julio Faracco wrote: > When a length of a file is defined, the responsible thread to send > stream is finishing inappropriately. It is happening because there is > wrong conditional which compares an offset with the length and because > of that the code throws ENOSPC error. > > To test it: > virsh# vol-upload ... --length N > > Signed-off-by: Julio Faracco <jcfaracco@gmail.com> > --- > src/remote/remote_daemon_stream.c | 24 ++++++++++++------------ > src/util/virfdstream.c | 5 +---- > 2 files changed, 13 insertions(+), 16 deletions(-) So there are couple of problems here. The first one: This patch as is (apart from points raised by Peter) makes fdstreamtest loop endlessly. Secondly, the ENOSPC error is not transferred to the client (although it is reported in the logs): # virsh vol-upload --length 1000 --pool default --vol vol.img --file /dev/random error: cannot close volume fd.img error: Library function returned error but did not set virError For this I have a patch and I'll send it soon. But the root cause is that client indeed sends more data than announced in --length (thank God for the wireshark dissector plugin). So server replies with ENOSPC. However, this always used to be the case even before I touched that part of code. So I think the proper fix is to close the stream at daemon side once fdst->offset reaches fdst->length. My recollection is that back in days of iohelper we had a pipe where we just pushed incoming data to and iohelper pulled it and wrote it into the file. However, as soon as it reached fdst->length limit it died which caused stream to get closed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.