On Mon, 28 Mar 2011, David Scott wrote:
> # HG changeset patch
> # User David Scott <dave.scott@xxxxxxxxxxxxx>
> # Date 1301314652 -3600
> # Node ID 3aab79c907a2c78f4e81362944ee65ddf6f2cc6f
> # Parent 45326ad6a0d396bfcd3c83d209ab7a19d6499896
> libxl: add NIC QoS parameters
>
> The parameters are:
> qos_kib_per_sec: maximum rate in KiB/sec
> qos_timeslice_usec: time period over which the average rate is enforced in
> usec
>
> One can now execute commands like
> xl network-attach ... rate=1024,50000
> which should impose an average limit of 1MiB/sec, over intervals of 50ms
>
> The "rate" key in the network backend is interpreted by netback. It wants:
> bytes_per_interval, interval_length
>
> Signed-off-by: David Scott <dave.scott@xxxxxxxxxxxxx>
>
Thanks for the patch! Next time could you please generate the diff with
function names (diff -p)? It would make it much easier to read...
> diff -r 45326ad6a0d3 -r 3aab79c907a2 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c Mon Mar 28 13:17:32 2011 +0100
> +++ b/tools/libxl/libxl.c Mon Mar 28 13:17:32 2011 +0100
> @@ -1194,6 +1194,8 @@
> libxl_xen_script_dir_path()) < 0 )
> return ERROR_FAIL;
> nic_info->nictype = NICTYPE_IOEMU;
> + nic_info->qos_kib_per_sec = 0;
> + nic_info->qos_timeslice_usec = 0;
> return 0;
> }
>
> @@ -1205,6 +1207,7 @@
> libxl__device device;
> char *dompath, **l;
> unsigned int nb, rc;
> + uint32_t bytes_per_interval;
>
> front = flexarray_make(16, 1);
> if (!front) {
> @@ -1263,6 +1266,11 @@
> flexarray_append(back, libxl__strdup(&gc, nic->bridge));
> flexarray_append(back, "handle");
> flexarray_append(back, libxl__sprintf(&gc, "%d", nic->devid));
> + if (nic->qos_timeslice_usec > 0) {
> + bytes_per_interval = (uint32_t) (((uint64_t)nic->qos_kib_per_sec *
> 1024L * (uint64_t)nic->qos_timeslice_usec) / 1000000L);
> + flexarray_append(back, "rate");
> + flexarray_append(back, libxl__sprintf(&gc, "%u,%u",
> bytes_per_interval, nic->qos_timeslice_usec));
> + }
Could you please use 80 characters columns? This line is very very long...
>
> flexarray_append(front, "backend-id");
> flexarray_append(front, libxl__sprintf(&gc, "%d", nic->backend_domid));
> diff -r 45326ad6a0d3 -r 3aab79c907a2 tools/libxl/libxl.idl
> --- a/tools/libxl/libxl.idl Mon Mar 28 13:17:32 2011 +0100
> +++ b/tools/libxl/libxl.idl Mon Mar 28 13:17:32 2011 +0100
> @@ -225,6 +225,8 @@
> ("ifname", string),
> ("script", string),
> ("nictype", libxl_nic_type),
> + ("qos_kib_per_sec", uint32),
> + ("qos_timeslice_usec", uint32),
> ])
it is probably worth adding a comment here to explain what these
parameters are, like you did in the commit message
>
> libxl_device_net2 = Struct("device_net2", [
> diff -r 45326ad6a0d3 -r 3aab79c907a2 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Mon Mar 28 13:17:32 2011 +0100
> +++ b/tools/libxl/xl_cmdimpl.c Mon Mar 28 13:17:32 2011 +0100
> @@ -880,7 +880,10 @@
> nic->backend_domid = 0;
> }
> } else if (!strcmp(p, "rate")) {
> - fprintf(stderr, "the rate parameter for vifs is
> currently not supported\n");
> + if (sscanf(p2 + 1, "%u,%u", &(nic->qos_kib_per_sec),
> &(nic->qos_timeslice_usec)) != 2) {
> + fprintf(stderr, "Specified rate parameter needs to
> take the form: kib_per_sec,timeslice_usec\n");
> + break;
> + }
> } else if (!strcmp(p, "accel")) {
> fprintf(stderr, "the accel parameter for vifs is
> currently not supported\n");
> }
> @@ -4298,6 +4301,10 @@
> free(nic.model);
> nic.model = strdup((*argv) + 6);
> } else if (!strncmp("rate=", *argv, 5)) {
> + if (sscanf((*argv) + 5, "%u,%u", &(nic.qos_kib_per_sec),
> &(nic.qos_timeslice_usec)) != 2) {
> + fprintf(stderr, "Specified rate parameter needs to take the
> form: kib_per_sec,timeslice_usec\n");
> + return 1;
> + }
> } else if (!strncmp("accel=", *argv, 6)) {
> } else {
> fprintf(stderr, "unrecognized argument `%s'\n", *argv);
considering that qos_kib_per_sec and qos_timeslice_usec are uint32_t,
sscanf should be called using PRIu32:
sscanf(p2 + 1, "%"PRIu32",%"PRIu32,
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|