[libvirt] [PATCH v2] virsh: Checking the volume capacity before uploading a new file.
Julio Faracco
jcfaracco at gmail.com
Tue Jan 23 02:42:24 UTC 2018
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 at redhat.com>:
> 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 at gmail.com>
>> ---
>> 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;
>> + }
More information about the libvir-list
mailing list