Re: [PATCH 4/6] conf: Add support for blkiotune group_name option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/02/2016 10:38 AM, Martin Kletzander wrote:
> On Mon, Oct 31, 2016 at 05:23:00PM -0400, John Ferlan wrote:
>> Modify _virDomainBlockIoTuneInfo and rng schema to support the group_name
>> option for iotune throttling. Document the new value.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> docs/formatdomain.html.in                          | 11 ++++
>> docs/schemas/domaincommon.rng                      |  5 ++
>> src/conf/domain_conf.c                             |  9 ++++
>> .../qemuxml2argv-blkdeviotune-group-num.xml        | 61
>> ++++++++++++++++++++++
>> .../qemuxml2xmlout-blkdeviotune-group-num.xml      |  1 +
>> tests/qemuxml2xmltest.c                            |  1 +
>> 6 files changed, 88 insertions(+)
>> create mode 100644
>> tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-group-num.xml
>> create mode 120000
>> tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-group-num.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index c70377b..8d30737 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -2622,6 +2622,17 @@
>>             <span class="since">Throughput limits since 1.2.11 and
>> QEMU 1.7</span>
>>           </p>
>>           </dd>
>> +          <dt><code>group_name</code></dt>
>> +          <dd>The optional <code>group_name</code> provides the cability
>> +            to share I/O throttling quota between multiple drives. This
>> +            prevents end-users from circumventing a hosting provider's
>> +            throttling policy by splitting 1 large drive in N small
>> drives
>> +            and getting N times the normal throttling quota. Any name
>> may
>> +            be used.
> 
> I'm really scared when I see that "any name may be used", I would rather
> put a sensible set of restrictions here that we can remove later, but
> let's go with "any name" for now.
> 
> [...]
> 

OK - I originally what just going to go with a number, but I recall
receiving negative reviews on some change where I fabricated a name, so
I figured I'd go with any name... Besides qemu would receive "any name"
and no sense trying guess/assume someone else's namespace.

>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index f41a783..8ad52d0 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -20416,6 +20420,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
>>                               def->blkdeviotune.size_iops_sec);
>>         }
>>
>> +        if (def->blkdeviotune.group_name) {
>> +            virBufferAsprintf(buf, "<group_name>%s</group_name>\n",
>> +                              def->blkdeviotune.group_name);
> 
> Because of that you need to escape it here.  Case that requires this
> would be nice to have in tests.

OK virBufferEscapeString it is...  Similar change for patch 5, plus I
made the requested patch 6 adjustment (email reduction initiative).

John

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]