Hello Nishith, Just a small suggestion on the indentation part of the patch. I think your indentation is off by 1 space. Thanks, Nitesh Konkar. On Wed, May 4, 2016 at 8:22 PM, Nitesh Konkar < niteshkonkar.libvirt(a)gmail.com&gt; wrote: > Hello Nishith, > > Just a small suggestion on the indentation part of the patch. I think > your indentation is off by 1 space. > > Thanks, > Nitesh Konkar. > > > On Wed, May 4, 2016 at 7:55 PM, Nishith Shah <nishithshah.2211(a)gmail.com&gt; > wrote: > >> Use vshCommandOptScaledInt instead of vshCommandOptULongLong so that >> values with suffixes can be passed when bytes are being passed along. >> Values for the iops parameters still need to be given in the absolute >> form as they are not bytes but numbers. Please refer to the bug link >> https://bugzilla.redhat.com/show_bug.cgi?id=885380 which can be closed. >> >> Signed-off-by: Nishith Shah <nishithshah.2211(a)gmail.com&gt; >> --- >> tools/virsh-domain.c | 24 ++++++++++++------------ >> tools/virsh.pod | 18 ++++++++++++------ >> 2 files changed, 24 insertions(+), 18 deletions(-) >> >> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c >> index 0a6caae..336a65f 100644 >> --- a/tools/virsh-domain.c >> +++ b/tools/virsh-domain.c >> @@ -1164,7 +1164,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { >> }, >> {.name = "total-bytes-sec", >> .type = VSH_OT_INT, >> - .help = N_("total throughput limit in bytes per second") >> + .help = N_("total throughput limit, as scaled integer (default >> bytes)") >> }, >> {.name = "read_bytes_sec", >> .type = VSH_OT_ALIAS, >> @@ -1172,7 +1172,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { >> }, >> {.name = "read-bytes-sec", >> .type = VSH_OT_INT, >> - .help = N_("read throughput limit in bytes per second") >> + .help = N_("read throughput limit, as scaled integer (default >> bytes)") >> }, >> {.name = "write_bytes_sec", >> .type = VSH_OT_ALIAS, >> @@ -1180,7 +1180,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { >> }, >> {.name = "write-bytes-sec", >> .type = VSH_OT_INT, >> - .help = N_("write throughput limit in bytes per second") >> + .help = N_("write throughput limit, as scaled integer (default >> bytes)") >> }, >> {.name = "total_iops_sec", >> .type = VSH_OT_ALIAS, >> @@ -1212,7 +1212,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { >> }, >> {.name = "total-bytes-sec-max", >> .type = VSH_OT_INT, >> - .help = N_("total max in bytes") >> + .help = N_("total max, as scaled integer (default bytes)") >> }, >> {.name = "read_bytes_sec_max", >> .type = VSH_OT_ALIAS, >> @@ -1220,7 +1220,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { >> }, >> {.name = "read-bytes-sec-max", >> .type = VSH_OT_INT, >> - .help = N_("read max in bytes") >> + .help = N_("read max, as scaled integer (default bytes)") >> }, >> {.name = "write_bytes_sec_max", >> .type = VSH_OT_ALIAS, >> @@ -1228,7 +1228,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { >> }, >> {.name = "write-bytes-sec-max", >> .type = VSH_OT_INT, >> - .help = N_("write max in bytes") >> + .help = N_("write max, as scaled integer (default bytes)") >> }, >> {.name = "total_iops_sec_max", >> .type = VSH_OT_ALIAS, >> @@ -1299,7 +1299,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) >> if (vshCommandOptStringReq(ctl, cmd, "device", &disk) < 0) >> goto cleanup; >> >> - if ((rv = vshCommandOptULongLong(ctl, cmd, "total-bytes-sec", >> &value)) < 0) { >> + if ((rv = vshCommandOptScaledInt(ctl, cmd, "total-bytes-sec", >> &value, 1, ULLONG_MAX)) < 0) { >> goto interror; >> } else if (rv > 0) { >> if (virTypedParamsAddULLong(&params, &nparams, &maxparams, >> @@ -1308,7 +1308,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) >> goto save_error; >> } >> >> - if ((rv = vshCommandOptULongLong(ctl, cmd, "read-bytes-sec", >> &value)) < 0) { >> + if ((rv = vshCommandOptScaledInt(ctl, cmd, "read-bytes-sec", >> &value, 1, ULLONG_MAX)) < 0) { >> goto interror; >> } else if (rv > 0) { >> if (virTypedParamsAddULLong(&params, &nparams, &maxparams, >> @@ -1317,7 +1317,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) >> goto save_error; >> } >> >> - if ((rv = vshCommandOptULongLong(ctl, cmd, "write-bytes-sec", >> &value)) < 0) { >> + if ((rv = vshCommandOptScaledInt(ctl, cmd, "write-bytes-sec", >> &value, 1, ULLONG_MAX)) < 0) { >> goto interror; >> } else if (rv > 0) { >> if (virTypedParamsAddULLong(&params, &nparams, &maxparams, >> @@ -1326,7 +1326,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) >> goto save_error; >> } >> >> - if ((rv = vshCommandOptULongLong(ctl, cmd, "total-bytes-sec-max", >> &value)) < 0) { >> + if ((rv = vshCommandOptScaledInt(ctl, cmd, "total-bytes-sec-max", >> &value, 1, ULLONG_MAX)) < 0) { >> goto interror; >> } else if (rv > 0) { >> if (virTypedParamsAddULLong(&params, &nparams, &maxparams, >> @@ -1335,7 +1335,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) >> goto save_error; >> } >> >> - if ((rv = vshCommandOptULongLong(ctl, cmd, "read-bytes-sec-max", >> &value)) < 0) { >> + if ((rv = vshCommandOptScaledInt(ctl, cmd, "read-bytes-sec-max", >> &value, 1, ULLONG_MAX)) < 0) { >> goto interror; >> } else if (rv > 0) { >> if (virTypedParamsAddULLong(&params, &nparams, &maxparams, >> @@ -1344,7 +1344,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) >> goto save_error; >> } >> >> - if ((rv = vshCommandOptULongLong(ctl, cmd, "write-bytes-sec-max", >> &value)) < 0) { >> + if ((rv = vshCommandOptScaledInt(ctl, cmd, "write-bytes-sec-max", >> &value, 1, ULLONG_MAX)) < 0) { >> goto interror; >> } else if (rv > 0) { >> if (virTypedParamsAddULLong(&params, &nparams, &maxparams, >> diff --git a/tools/virsh.pod b/tools/virsh.pod >> index 2a95df7..6844823 100644 >> --- a/tools/virsh.pod >> +++ b/tools/virsh.pod >> @@ -1097,15 +1097,21 @@ I<domain> (see also B<domblklist> for listing >> these names). >> >> If no limit is specified, it will query current I/O limits setting. >> Otherwise, alter the limits with these flags: >> -I<--total-bytes-sec> specifies total throughput limit in bytes per >> second. >> -I<--read-bytes-sec> specifies read throughput limit in bytes per second. >> -I<--write-bytes-sec> specifies write throughput limit in bytes per >> second. >> +I<--total-bytes-sec> specifies total throughput limit as a scaled >> integer, the >> +default being bytes per second if no suffix is specified. >> +I<--read-bytes-sec> specifies read throughput limit as a scaled >> integer, the >> +default being bytes per second if no suffix is specified. >> +I<--write-bytes-sec> specifies write throughput limit as a scaled >> integer, the >> +default being bytes per second if no suffix is specified. >> I<--total-iops-sec> specifies total I/O operations limit per second. >> I<--read-iops-sec> specifies read I/O operations limit per second. >> I<--write-iops-sec> specifies write I/O operations limit per second. >> -I<--total-bytes-sec-max> specifies maximum total throughput limit in >> bytes per second. >> -I<--read-bytes-sec-max> specifies maximum read throughput limit in >> bytes per second. >> -I<--write-bytes-sec-max> specifies maximum write throughput limit in >> bytes per second. >> +I<--total-bytes-sec-max> specifies maximum total throughput limit as a >> scaled >> +integer, the default being bytes per second if no suffix is specified >> +I<--read-bytes-sec-max> specifies maximum read throughput limit as a >> scaled >> +integer, the default being bytes per second if no suffix is specified. >> +I<--write-bytes-sec-max> specifies maximum write throughput limit as a >> scaled >> +integer, the default being bytes per second if no suffix is specified. >> I<--total-iops-sec-max> specifies maximum total I/O operations limit >> per second. >> I<--read-iops-sec-max> specifies maximum read I/O operations limit per >> second. >> I<--write-iops-sec-max> specifies maximum write I/O operations limit >> per second. >> -- >> 2.1.4 >> >> -- >> libvir-list mailing list >> libvir-list(a)redhat.com >> https://www.redhat.com/mailman/listinfo/libvir-list >> > >

On 05/04/2016 10:52 AM, Nitesh Konkar wrote: > Hello Nishith, > > Just a small suggestion on the indentation part of the patch. I think your > indentation is off by 1 space. > Where did you spot an indentation issue? I glanced quickly but didnt see one. Though inn your quoted email it does indeed look wrong, but that's because your response is in html format with a non-monospace font. - Cole > > On Wed, May 4, 2016 at 8:22 PM, Nitesh Konkar < niteshkonkar.libvirt(a)gmail.com > <mailto:niteshkonkar.libvirt@gmail.com>> wrote: > > Hello Nishith, > > Just a small suggestion on the indentation part of the patch. I think your > indentation is off by 1 space. > > Thanks, > Nitesh Konkar. > > > On Wed, May 4, 2016 at 7:55 PM, Nishith Shah < nishithshah.2211(a)gmail.com > <mailto:nishithshah.2211@gmail.com>> wrote: > > Use vshCommandOptScaledInt instead of vshCommandOptULongLong so that > values with suffixes can be passed when bytes are being passed along. > Values for the iops parameters still need to be given in the absolute > form as they are not bytes but numbers. Please refer to the bug link > https://bugzilla.redhat.com/show_bug.cgi?id=885380 which can be closed. > > Signed-off-by: Nishith Shah <nishithshah.2211(a)gmail.com > <mailto:nishithshah.2211@gmail.com>> > --- > tools/virsh-domain.c | 24 ++++++++++++------------ > tools/virsh.pod | 18 ++++++++++++------ > 2 files changed, 24 insertions(+), 18 deletions(-) > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index 0a6caae..336a65f 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -1164,7 +1164,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { > }, > {.name = "total-bytes-sec", > .type = VSH_OT_INT, > - .help = N_("total throughput limit in bytes per second") > + .help = N_("total throughput limit, as scaled integer (default > bytes)") > }, > {.name = "read_bytes_sec", > .type = VSH_OT_ALIAS, > @@ -1172,7 +1172,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { > }, > {.name = "read-bytes-sec", > .type = VSH_OT_INT, > - .help = N_("read throughput limit in bytes per second") > + .help = N_("read throughput limit, as scaled integer (default > bytes)") > }, > {.name = "write_bytes_sec", > .type = VSH_OT_ALIAS, > @@ -1180,7 +1180,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { > }, > {.name = "write-bytes-sec", > .type = VSH_OT_INT, > - .help = N_("write throughput limit in bytes per second") > + .help = N_("write throughput limit, as scaled integer (default > bytes)") > }, > {.name = "total_iops_sec", > .type = VSH_OT_ALIAS, > @@ -1212,7 +1212,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { > }, > {.name = "total-bytes-sec-max", > .type = VSH_OT_INT, > - .help = N_("total max in bytes") > + .help = N_("total max, as scaled integer (default bytes)") > }, > {.name = "read_bytes_sec_max", > .type = VSH_OT_ALIAS, > @@ -1220,7 +1220,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { > }, > {.name = "read-bytes-sec-max", > .type = VSH_OT_INT, > - .help = N_("read max in bytes") > + .help = N_("read max, as scaled integer (default bytes)") > }, > {.name = "write_bytes_sec_max", > .type = VSH_OT_ALIAS, > @@ -1228,7 +1228,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { > }, > {.name = "write-bytes-sec-max", > .type = VSH_OT_INT, > - .help = N_("write max in bytes") > + .help = N_("write max, as scaled integer (default bytes)") > }, > {.name = "total_iops_sec_max", > .type = VSH_OT_ALIAS, > @@ -1299,7 +1299,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) > if (vshCommandOptStringReq(ctl, cmd, "device", &disk) < 0) > goto cleanup; > > - if ((rv = vshCommandOptULongLong(ctl, cmd, "total-bytes-sec", > &value)) < 0) { > + if ((rv = vshCommandOptScaledInt(ctl, cmd, "total-bytes-sec", > &value, 1, ULLONG_MAX)) < 0) { > goto interror; > } else if (rv > 0) { > if (virTypedParamsAddULLong(&params, &nparams, &maxparams, > @@ -1308,7 +1308,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) > goto save_error; > } > > - if ((rv = vshCommandOptULongLong(ctl, cmd, "read-bytes-sec", > &value)) < 0) { > + if ((rv = vshCommandOptScaledInt(ctl, cmd, "read-bytes-sec", > &value, 1, ULLONG_MAX)) < 0) { > goto interror; > } else if (rv > 0) { > if (virTypedParamsAddULLong(&params, &nparams, &maxparams, > @@ -1317,7 +1317,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) > goto save_error; > } > > - if ((rv = vshCommandOptULongLong(ctl, cmd, "write-bytes-sec", > &value)) < 0) { > + if ((rv = vshCommandOptScaledInt(ctl, cmd, "write-bytes-sec", > &value, 1, ULLONG_MAX)) < 0) { > goto interror; > } else if (rv > 0) { > if (virTypedParamsAddULLong(&params, &nparams, &maxparams, > @@ -1326,7 +1326,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) > goto save_error; > } > > - if ((rv = vshCommandOptULongLong(ctl, cmd, "total-bytes-sec-max", > &value)) < 0) { > + if ((rv = vshCommandOptScaledInt(ctl, cmd, "total-bytes-sec-max", > &value, 1, ULLONG_MAX)) < 0) { > goto interror; > } else if (rv > 0) { > if (virTypedParamsAddULLong(&params, &nparams, &maxparams, > @@ -1335,7 +1335,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) > goto save_error; > } > > - if ((rv = vshCommandOptULongLong(ctl, cmd, "read-bytes-sec-max", > &value)) < 0) { > + if ((rv = vshCommandOptScaledInt(ctl, cmd, "read-bytes-sec-max", > &value, 1, ULLONG_MAX)) < 0) { > goto interror; > } else if (rv > 0) { > if (virTypedParamsAddULLong(&params, &nparams, &maxparams, > @@ -1344,7 +1344,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) > goto save_error; > } > > - if ((rv = vshCommandOptULongLong(ctl, cmd, "write-bytes-sec-max", > &value)) < 0) { > + if ((rv = vshCommandOptScaledInt(ctl, cmd, "write-bytes-sec-max", > &value, 1, ULLONG_MAX)) < 0) { > goto interror; > } else if (rv > 0) { > if (virTypedParamsAddULLong(&params, &nparams, &maxparams, > diff --git a/tools/virsh.pod b/tools/virsh.pod > index 2a95df7..6844823 100644 > --- a/tools/virsh.pod > +++ b/tools/virsh.pod > @@ -1097,15 +1097,21 @@ I<domain> (see also B<domblklist> for listing > these names). > > If no limit is specified, it will query current I/O limits setting. > Otherwise, alter the limits with these flags: > -I<--total-bytes-sec> specifies total throughput limit in bytes per > second. > -I<--read-bytes-sec> specifies read throughput limit in bytes per second. > -I<--write-bytes-sec> specifies write throughput limit in bytes per > second. > +I<--total-bytes-sec> specifies total throughput limit as a scaled > integer, the > +default being bytes per second if no suffix is specified. > +I<--read-bytes-sec> specifies read throughput limit as a scaled > integer, the > +default being bytes per second if no suffix is specified. > +I<--write-bytes-sec> specifies write throughput limit as a scaled > integer, the > +default being bytes per second if no suffix is specified. > I<--total-iops-sec> specifies total I/O operations limit per second. > I<--read-iops-sec> specifies read I/O operations limit per second. > I<--write-iops-sec> specifies write I/O operations limit per second. > -I<--total-bytes-sec-max> specifies maximum total throughput limit in > bytes per second. > -I<--read-bytes-sec-max> specifies maximum read throughput limit in > bytes per second. > -I<--write-bytes-sec-max> specifies maximum write throughput limit in > bytes per second. > +I<--total-bytes-sec-max> specifies maximum total throughput limit as > a scaled > +integer, the default being bytes per second if no suffix is specified > +I<--read-bytes-sec-max> specifies maximum read throughput limit as a > scaled > +integer, the default being bytes per second if no suffix is specified. > +I<--write-bytes-sec-max> specifies maximum write throughput limit as > a scaled > +integer, the default being bytes per second if no suffix is specified. > I<--total-iops-sec-max> specifies maximum total I/O operations limit > per second. > I<--read-iops-sec-max> specifies maximum read I/O operations limit > per second. > I<--write-iops-sec-max> specifies maximum write I/O operations limit > per second. > -- > 2.1.4 > > -- > libvir-list mailing list > libvir-list(a)redhat.com <mailto:libvir-list@redhat.com> > https://www.redhat.com/mailman/listinfo/libvir-list > > > > > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list >