https://bugzilla.redhat.com/show_bug.cgi?id=1087104 Commit id 'c6212539' explicitly allowed a negative value to be used for offset and length as a shorthand for the largest value after commit id 'f18c02ec' modified virStrToLong_ui() to essentially disallow a negative value. However, allowing a negative value for offset ONLY worked if the negative value was -1 since the eventual lseek() does allow a -1 to mean the end of the file. Providing other negative values resulted in errors such as: $ virsh vol-download --pool default qcow3-vol2 /home/vm-images/raw \ --offset -2 --length -1000 error: cannot download from volume qcow3-vol2 error: Unable to seek /home/vm-images/qcow3-vol2 to 18446744073709551614: Invalid argument $ Thus, it seems unreasonable to expect or allow a negative value for offset since the only benefit is to lseek() to the end of the file and then only take advantage of how the OS would handle such a seek. For the purposes of upload or download of volume data, that seems to be a no-op. Therefore, disallow a negative value for offset. Additionally, modify the man page for vol-upload and vol-download to provide more details regarding the valid values for both offset and length. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- tools/virsh-volume.c | 6 +++--- tools/virsh.pod | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 724a86b..b528928 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -787,13 +787,13 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd) unsigned long long offset = 0, length = 0; bool created = false; - if (vshCommandOptULongLongWrap(cmd, "offset", &offset) < 0) { - vshError(ctl, _("Unable to parse integer")); + if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) { + vshError(ctl, _("Unable to parse offset value")); return false; } if (vshCommandOptULongLongWrap(cmd, "length", &length) < 0) { - vshError(ctl, _("Unable to parse integer")); + vshError(ctl, _("Unable to parse length value")); return false; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 1a2b01f..e02ff5d 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2961,7 +2961,10 @@ is in. I<vol-name-or-key-or-path> is the name or key or path of the volume where the file will be uploaded. I<--offset> is the position in the storage volume at which to start writing -the data. I<--length> is an upper bound of the amount of data to be uploaded. +the data. The value must be 0 or larger. I<--length> is an upper bound +of the amount of data to be uploaded. A negative value is interpreted +as an unsigned long long value to essentially include everything from +the offset to the end of the volume. An error will occur if the I<local-file> is greater than the specified length. =item B<vol-download> [I<--pool> I<pool-or-uuid>] [I<--offset> I<bytes>] @@ -2972,7 +2975,10 @@ I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool the volume is in. I<vol-name-or-key-or-path> is the name or key or path of the volume to download. I<--offset> is the position in the storage volume at which to start reading -the data. I<--length> is an upper bound of the amount of data to be downloaded. +the data. The value must be 0 or larger. I<--length> is an upper bound of +the amount of data to be downloaded. A negative value is interpreted as +an unsigned long long value to essentially include everything from the +offset to the end of the volume. =item B<vol-wipe> [I<--pool> I<pool-or-uuid>] [I<--algorithm> I<algorithm>] I<vol-name-or-key-or-path> -- 1.9.3 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list