There are couple of callbacks we pass to virStreamSendAll(),
virStreamRecvAll() or its sparse variants. However, none of these
callbacks reports error if one occurs and neither do the
virStream* functions leaving user with very unhelpful error
message:
error: cannot receive data from volume fedora.img
error: An error occurred, but the cause is unknown
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
tools/virsh-util.c | 38 +++++++++++++++++++++++++++++---------
tools/virsh-util.h | 2 +-
tools/virsh-volume.c | 8 ++++++--
3 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index 44be3ad64..183f4c8bb 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -147,9 +147,15 @@ virshStreamSink(virStreamPtr st ATTRIBUTE_UNUSED,
size_t nbytes,
void *opaque)
{
- int *fd = opaque;
+ virshStreamCallbackDataPtr cbData = opaque;
+ int fd = cbData->fd;
+ const char *filename = cbData->filename;
+ int ret;
- return safewrite(*fd, bytes, nbytes);
+ if ((ret = safewrite(fd, bytes, nbytes)) < 0)
+ virReportSystemError(errno, _("unable to write to %s"), filename);
+
+ return ret;
}
@@ -161,8 +167,13 @@ virshStreamSource(virStreamPtr st ATTRIBUTE_UNUSED,
{
virshStreamCallbackDataPtr cbData = opaque;
int fd = cbData->fd;
+ const char *filename = cbData->filename;
+ int ret;
- return saferead(fd, bytes, nbytes);
+ if ((ret = saferead(fd, bytes, nbytes)) < 0)
+ virReportSystemError(errno, _("unable to read from %s"), filename);
+
+ return ret;
}
@@ -173,10 +184,13 @@ virshStreamSourceSkip(virStreamPtr st ATTRIBUTE_UNUSED,
{
virshStreamCallbackDataPtr cbData = opaque;
int fd = cbData->fd;
+ const char *filename = cbData->filename;
off_t cur;
- if ((cur = lseek(fd, offset, SEEK_CUR)) == (off_t) -1)
+ if ((cur = lseek(fd, offset, SEEK_CUR)) == (off_t) -1) {
+ virReportSystemError(errno, _("unable to seek in %s"), filename);
return -1;
+ }
return 0;
}
@@ -187,14 +201,20 @@ virshStreamSkip(virStreamPtr st ATTRIBUTE_UNUSED,
long long offset,
void *opaque)
{
- int *fd = opaque;
+ virshStreamCallbackDataPtr cbData = opaque;
+ int fd = cbData->fd;
+ const char *filename = cbData->filename;
off_t cur;
- if ((cur = lseek(*fd, offset, SEEK_CUR)) == (off_t) -1)
+ if ((cur = lseek(fd, offset, SEEK_CUR)) == (off_t) -1) {
+ virReportSystemError(errno, _("unable to seek in %s"), filename);
return -1;
+ }
- if (ftruncate(*fd, cur) < 0)
+ if (ftruncate(fd, cur) < 0) {
+ virReportSystemError(errno, _("unable to truncate %s"), filename);
return -1;
+ }
return 0;
}
@@ -207,12 +227,12 @@ virshStreamInData(virStreamPtr st ATTRIBUTE_UNUSED,
void *opaque)
{
virshStreamCallbackDataPtr cbData = opaque;
- vshControl *ctl = cbData->ctl;
int fd = cbData->fd;
+ const char *filename = cbData->filename;
int ret;
if ((ret = virFileInData(fd, inData, offset)) < 0)
- vshError(ctl, "%s", _("Unable to get current position in stream"));
+ virReportSystemError(errno, _("unable to seek in %s"), filename);
return ret;
}
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index 9a0af3513..0babb311b 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -60,8 +60,8 @@ virshStreamSink(virStreamPtr st,
typedef struct _virshStreamCallbackData virshStreamCallbackData;
typedef virshStreamCallbackData *virshStreamCallbackDataPtr;
struct _virshStreamCallbackData {
- vshControl *ctl;
int fd;
+ const char *filename;
};
int
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 0736bdcdb..ea4660fee 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -698,8 +698,8 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- cbData.ctl = ctl;
cbData.fd = fd;
+ cbData.filename = file;
if (vshCommandOptBool(cmd, "sparse"))
flags |= VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM;
@@ -795,6 +795,7 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
bool created = false;
virshControlPtr priv = ctl->privData;
unsigned int flags = 0;
+ virshStreamCallbackData cbData;
if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0)
return false;
@@ -821,6 +822,9 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
created = true;
}
+ cbData.fd = fd;
+ cbData.filename = file;
+
if (!(st = virStreamNew(priv->conn, 0))) {
vshError(ctl, _("cannot create a new stream"));
goto cleanup;
@@ -831,7 +835,7 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (virStreamSparseRecvAll(st, virshStreamSink, virshStreamSkip, &fd) < 0) {
+ if (virStreamSparseRecvAll(st, virshStreamSink, virshStreamSkip, &cbData) < 0) {
vshError(ctl, _("cannot receive data from volume %s"), name);
goto cleanup;
}
--
2.13.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 01, 2017 at 02:02:13PM +0200, Michal Privoznik wrote: > There are couple of callbacks we pass to virStreamSendAll(), > virStreamRecvAll() or its sparse variants. However, none of these > callbacks reports error if one occurs and neither do the > virStream* functions leaving user with very unhelpful error > message: > > error: cannot receive data from volume fedora.img > error: An error occurred, but the cause is unknown If we change the code to honour errno set in the callbacks, we only need to deal with ensuring virFileInData sets the errno correctly. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 06/01/2017 03:39 PM, Daniel P. Berrange wrote: > On Thu, Jun 01, 2017 at 02:02:13PM +0200, Michal Privoznik wrote: >> There are couple of callbacks we pass to virStreamSendAll(), >> virStreamRecvAll() or its sparse variants. However, none of these >> callbacks reports error if one occurs and neither do the >> virStream* functions leaving user with very unhelpful error >> message: >> >> error: cannot receive data from volume fedora.img >> error: An error occurred, but the cause is unknown > > If we change the code to honour errno set in the callbacks, > we only need to deal with ensuring virFileInData sets > the errno correctly. Okay, makes sense. Will post v3. Meanwhile, do you have any clever idea on this? https://www.redhat.com/archives/libvir-list/2017-June/msg00015.html Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 01, 2017 at 03:43:06PM +0200, Michal Privoznik wrote: > On 06/01/2017 03:39 PM, Daniel P. Berrange wrote: > > On Thu, Jun 01, 2017 at 02:02:13PM +0200, Michal Privoznik wrote: > >> There are couple of callbacks we pass to virStreamSendAll(), > >> virStreamRecvAll() or its sparse variants. However, none of these > >> callbacks reports error if one occurs and neither do the > >> virStream* functions leaving user with very unhelpful error > >> message: > >> > >> error: cannot receive data from volume fedora.img > >> error: An error occurred, but the cause is unknown > > > > If we change the code to honour errno set in the callbacks, > > we only need to deal with ensuring virFileInData sets > > the errno correctly. > > Okay, makes sense. Will post v3. > > Meanwhile, do you have any clever idea on this? > > https://www.redhat.com/archives/libvir-list/2017-June/msg00015.html Hmm, no bright ideas, without spending some time peering into the code. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.