On 09/12/14 05:55, Eric Blake wrote:
> Treating -1 as the maximum bandwidth of block jobs is not very
> practical, given the restrictions on overflow between 32-bit
> vs. 64-bit long, as well as conversions between MiB/s and bytes/s.
> We already document that 0 means unlimited, so the only reason to
> keep negative support is for back-compat.
>
> Meanwhile, it is a good idea to allow users to input in scales
> other than MiB/s. This patch just rounds back up to MiB/s, but by
> using a common helper, a later patch can then determine when a
> particular input scale will be better for using flags with the
> corresponding API call.
>
> * tools/virsh-domain.c (blockJobBandwidth): New function.
> (blockJobImpl, cmdBlockCopy): Use it.
> * tools/virsh.pod (blockjob, blockcopy, blockpull, blockcommit):
> Document this.
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> tools/virsh-domain.c | 73 ++++++++++++++++++++++++++++++++++++++++++----------
> tools/virsh.pod | 42 ++++++++++++++++++------------
> 2 files changed, 84 insertions(+), 31 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index cc1e554..594647a 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1467,6 +1470,56 @@ typedef enum {
> VSH_CMD_BLOCK_JOB_COMMIT,
> } vshCmdBlockJobMode;
>
> +
> +/* Parse a block job bandwidth parameter. Returns -1 on error with
> + * message printed, 0 if no bandwidth needed, and 1 if *bandwidth is
> + * set to non-zero. */
> +static int
> +blockJobBandwidth(vshControl *ctl, const vshCmd *cmd,
vshBlockJobBandwidth perhaps?
> + unsigned long *bandwidth)
> +{
> + const char *value = NULL;
> + int ret = vshCommandOptString(cmd, "bandwidth", &value);
> + unsigned long long speed;
> +
> + *bandwidth = 0;
> + if (ret <= 0)
> + return ret;
> +
> + /* For back-compat, accept -1 as meaning unlimited, by converting
> + * negative values to 0 (and forbid wraparound back to positive).
> + * Alas, we have to parse the raw string ourselves, instead of
> + * using vshCommandOptUL. If only we had never allowed wraparound
> + * on bandwidth. */
> + if (strchr(value, '-')) {
> + unsigned long tmp;
> +
> + if (vshCommandOptULWrap(cmd, "bandwidth", &tmp) < 0 ||
> + (long) tmp >= 0) {
> + vshError(ctl, _("could not parse bandwidth '%s'"), value);
> + return -1;
> + }
> + return 0;
> + }
> +
> + /* Read a number in bytes/s, but with default unit of MiB/s if no
> + * unit was specified. */
> + if (vshCommandOptScaledInt(cmd, "bandwidth", &speed, 1024 * 1024,
> + (sizeof(*bandwidth) < sizeof(speed) ?
> + ULONG_MAX * 1024 * 1024 : ULLONG_MAX)) < 0) {
> + vshError(ctl, _("could not parse bandwidth '%s'"), value);
> + return -1;
> + }
> +
> + /* Now scale it to MiB/s for the caller. What a pain. */
> + speed = VIR_DIV_UP(speed, 1024 * 1024);
> +
> + /* We already checked that narrowing the type will fit. */
> + *bandwidth = speed;
> + return !!speed;
> +}
> +
> +
> static bool
> blockJobImpl(vshControl *ctl, const vshCmd *cmd,
> vshCmdBlockJobMode mode, virDomainPtr *pdom)
ACK,
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list