[libvirt] [PATCH 3/3] qemu: Implement blkio tunable XML configuration and parsing.

[Gui Jianfeng]

Daniel P. Berrange wrote:
> On Sun, Jan 23, 2011 at 02:19:44PM +0800, Gui Jianfeng wrote:
>> Implement blkio tunable XML configuration and parsing.
>>
>> Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
>> ---
>>  src/conf/domain_conf.c |   17 ++++++++++++++++-
>>  src/conf/domain_conf.h |    4 ++++
>>  src/qemu/qemu_cgroup.c |   16 +++++++++++++++-
>>  src/qemu/qemu_conf.c   |    3 ++-
>>  4 files changed, 37 insertions(+), 3 deletions(-)
> 
> This needs to also update docs/schemas/domain.rng
> and ideally add a new test case + example datafile
> for tests/qemuxml2xmltest.c
> 
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 645767e..a05ada5 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -4810,7 +4810,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>>  {
>>      xmlNodePtr *nodes = NULL, node = NULL;
>>      char *tmp = NULL;
>> -    int i, n;
>> +    int i, n, w;
>>      long id = -1;
>>      virDomainDefPtr def;
>>      unsigned long count;
>> @@ -4887,6 +4887,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>>      if (node)
>>          def->mem.hugepage_backed = 1;
>>  
>> +    /* Extract blkio cgroup tunables */
>> +    w = virXPathULong("string(./blkiotune/weight)", ctxt,
>> +                      &def->blkio.weight);
>> +    if (w < 0 || def->blkio.weight > 1000 || def->blkio.weight < 100)
>> +        virDomainReportError(VIR_ERR_XML_ERROR, "%s",
>> +                             _("I/O weight should fall in [100, 1000]"));
>> +
> 
> The virDomainDefParseXML method should only do syntax validation.
> Semantic validation should be left to the hypervisor drivers.
> 
>>      /* Extract other memory tunables */
>>      if (virXPathULong("string(./memtune/hard_limit)", ctxt,
>>                        &def->mem.hard_limit) < 0)
>> @@ -7289,6 +7296,14 @@ char *virDomainDefFormat(virDomainDefPtr def,
>>      virBufferVSprintf(&buf, "  <currentMemory>%lu</currentMemory>\n",
>>                        def->mem.cur_balloon);
>>  
>> +    /* add blkiotune only if there are any */
>> +    if (def->blkio.weight) {
>> +        virBufferVSprintf(&buf, "  <blkiotune>\n");
>> +        virBufferVSprintf(&buf, "    <weight>%lu</weight>\n",
>> +                          def->blkio.weight);
>> +        virBufferVSprintf(&buf, "  </blkiotune>\n");
>> +    }
>> +
>>      /* add memtune only if there are any */
>>      if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee ||
>>          def->mem.swap_hard_limit)
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index cf7bdc0..b117869 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -969,6 +969,10 @@ struct _virDomainDef {
>>      char *description;
>>  
>>      struct {
>> +        unsigned long weight;
>> +    } blkio;
>> +
>> +    struct {
>>          unsigned long max_balloon;
>>          unsigned long cur_balloon;
>>          unsigned long hugepage_backed;
>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>> index e5536c0..a3e8e6a 100644
>> --- a/src/qemu/qemu_cgroup.c
>> +++ b/src/qemu/qemu_cgroup.c
>> @@ -54,7 +54,6 @@ int qemuCgroupControllerActive(struct qemud_driver *driver,
>>      return 0;
>>  }
>>  
>> -
>>  int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
>>                             const char *path,
>>                             size_t depth ATTRIBUTE_UNUSED,
>> @@ -270,6 +269,21 @@ int qemuSetupCgroup(struct qemud_driver *driver,
>>          }
>>      }
>>  
>> +    if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
>> +        if (vm->def->blkio.weight != 0) {
>> +            rc = virCgroupSetWeight(cgroup, vm->def->blkio.weight);
>> +            if(rc != 0) {
>> +                virReportSystemError(-rc,
>> +                                     _("Unable to set io weight for domain %s"),
>> +                                     vm->def->name);
>> +                goto cleanup;
>> +            }
>> +        }
>> +    } else {
>> +        VIR_WARN("Blkio cgroup is disabled in qemu configuration file: %s",
>> +                 vm->def->name);
>> +    }
> 
> I think it is probably worth raising a real error here
> 
>     qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                     _("Block I/O tuning is not available on this host"));

Ok, Will change.

Thanks,
Gui

> 
> Regards,
> Daniel
> 

-- 
Regards
Gui Jianfeng