Hi Peter, I agree about "length" parameter. What I didn't understand is this error (check my test case): # virsh vol-create-as --pool disk-loop loop0 --capacity 20M # virsh vol-info loop0 disk-loop Name: loop0 Type: file Capacity: 20.00 MiB Allocation: 20.00 MiB # dd if=/dev/zero of=/tmp/test bs=10M count=1 # ls -l /tmp/test -rw-rw-r-- 1 julio julio 10485760 Jan 23 00:31 /tmp/test Now, let's upload 5M (partial): # virsh vol-upload loop0 /tmp/test --pool disk-loop --length 5242880 error: cannot send data to volume loop0 error: Library function returned error but did not set virError But, if I set a length with 15M, no errors occurred: # virsh vol-upload loop0 /tmp/test --pool disk-loop --length 15728640 2018-01-22 12:44 GMT-02:00 Peter Krempa <pkrempa@xxxxxxxxxx>: > On Sun, Jan 21, 2018 at 21:51:05 -0200, Julio Faracco wrote: >> The current command 'vol-upload' is not checking if the volume accepts >> a file bigger than its capacity. It can cause an interrupt of the >> upload stream. This commit adds a check that fails before starting to >> send new file to volume if the file is bigger. >> >> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1529059 >> >> Signed-off-by: Julio Faracco <jcfaracco@xxxxxxxxx> >> --- >> tools/virsh-volume.c | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c >> index 8265a39..04e480c 100644 >> --- a/tools/virsh-volume.c >> +++ b/tools/virsh-volume.c >> @@ -672,6 +672,7 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd) >> { >> const char *file = NULL; >> virStorageVolPtr vol = NULL; >> + virStorageVolInfo volumeInfo; >> bool ret = false; >> int fd = -1; >> virStreamPtr st = NULL; >> @@ -679,6 +680,7 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd) >> unsigned long long offset = 0, length = 0; >> virshControlPtr priv = ctl->privData; >> unsigned int flags = 0; >> + off_t fileLen = -1; >> virshStreamCallbackData cbData; >> >> if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0) >> @@ -701,6 +703,29 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd) >> cbData.ctl = ctl; >> cbData.fd = fd; >> >> + if (virStorageVolGetInfo(vol, &volumeInfo) < 0) >> + goto cleanup; >> + >> + if ((fileLen = virFileLength(file, fd)) < 0) { >> + vshError(ctl, _("cannot get the file %s length"), file); > > This is wrong since you might want to upload contents of a block device, > in which case you'd report this error. > >> + goto cleanup; >> + } >> + >> + if (length < fileLen) { > > I think this is wrong. The 'length' parameter was designed to allow > upload of a partial file. Adding this would break it. > >> + vshError(ctl, _("length parameter is smaller than file size")); >> + goto cleanup; >> + } -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
