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

Re: [libvirt] [PATCHv4 3/4] blkiotune: add interface for blkiotune.device_weight



On Mon, Nov 14, 2011 at 09:30:01PM -0700, Eric Blake wrote:
> From: Hu Tao <hutao cn fujitsu com>
> 
> This adds per-device weights to <blkiotune>.  Note that the
> cgroups implementation only supports weights per block device,
> and not per-file within the device; hence this option must be
> global to the domain definition rather than tied to individual
> <devices>/<disk> entries:
> 
> <domain ...>
>   <blkiotune>
>     <device>
>       <path>/path/to/block</path>
>       <weight>1000</weight>
>     </device>
>   </blkiotune>
> ..
> 
> This patch also adds a parameter --device-weights to virsh command
> blkiotune for setting/getting blkiotune.weight_device for any
> hypervisor that supports it.  All <device> entries under
> <blkiotune> are concatenated into a single string attribute under
> virDomain{Get,Set}BlkioParameters, named "device_weight".
> 
> Signed-off-by: Hu Tao <hutao cn fujitsu com>
> Signed-off-by: Eric Blake <eblake redhat com>
> ---
>  docs/formatdomain.html.in                          |   29 +++++-
>  docs/schemas/domaincommon.rng                      |   26 ++++-
>  include/libvirt/libvirt.h.in                       |   10 ++
>  src/conf/domain_conf.c                             |   99 +++++++++++++++++++-
>  src/conf/domain_conf.h                             |   14 +++
>  src/libvirt_private.syms                           |    1 +
>  .../qemuxml2argv-blkiotune-device.xml              |   36 +++++++
>  tools/virsh.c                                      |   43 +++++++--
>  tools/virsh.pod                                    |    8 ++-
>  9 files changed, 245 insertions(+), 21 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index cbad196..99c5add 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -505,6 +505,14 @@
>    ...
>    &lt;blkiotune&gt;
>      &lt;weight&gt;800&lt;/weight&gt;
> +    &lt;device&gt;
> +      &lt;path&gt;/dev/sda&lt;/path&gt;
> +      &lt;weight&gt;1000&lt;/weight&gt;
> +    &lt;/device&gt;
> +    &lt;device&gt;
> +      &lt;path&gt;/dev/sdb&lt;/path&gt;
> +      &lt;weight&gt;500&lt;/weight&gt;
> +    &lt;/device&gt;
>    &lt;/blkiotune&gt;
>    ...
>  &lt;/domain&gt;
> @@ -514,10 +522,25 @@
>        <dt><code>blkiotune</code></dt>
>        <dd> The optional <code>blkiotune</code> element provides the ability
>          to tune Blkio cgroup tunable parameters for the domain. If this is
> -        omitted, it defaults to the OS provided defaults.</dd>
> +        omitted, it defaults to the OS provided
> +        defaults. <span class="since">Since 0.8.8</span></dd>
>        <dt><code>weight</code></dt>
> -      <dd> The optional <code>weight</code> element is the I/O weight of the
> -        guest. The value should be in range [100, 1000].</dd>
> +      <dd> The optional <code>weight</code> element is the overall I/O
> +        weight of the guest. The value should be in the range [100,
> +        1000].</dd>
> +      <dt><code>device</code></dt>
> +      <dd>The domain may have multiple <code>device</code> elements
> +        that further tune the weights for each host block device in
> +        use by the domain.  Note that
> +        multiple <a href="#elementsDisks">guest disks</a> can share a
> +        single host block device, if they are backed by files within
> +        the same host file system, which is why this tuning parameter
> +        is at the global domain level rather than associated with each
> +        guest disk device.  Each <code>device</code> element has two
> +        mandatory sub-elements, <code>path</code> describing the
> +        absolute path of the device, and <code>weight</code> giving
> +        the relative weight of that device, in the range [100,
> +        1000].  <span class="since">Since 0.9.8</span></dd>
>      </dl>
> 
> 
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index b6f858e..f776a51 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -322,12 +322,26 @@
>        <!-- The Blkio cgroup related tunables would go in the blkiotune -->
>        <optional>
>          <element name="blkiotune">
> -          <!-- I/O weight the VM can use -->
> -          <optional>
> -            <element name="weight">
> -              <ref name="weight"/>
> -            </element>
> -          </optional>
> +          <interleave>
> +            <!-- I/O weight the VM can use -->
> +            <optional>
> +              <element name="weight">
> +                <ref name="weight"/>
> +              </element>
> +            </optional>
> +            <zeroOrMore>
> +              <element name="device">
> +                <interleave>
> +                  <element name="path">
> +                    <ref name="absFilePath"/>
> +                  </element>
> +                  <element name="weight">
> +                    <ref name="weight"/>
> +                  </element>
> +                </interleave>
> +              </element>
> +            </zeroOrMore>
> +          </interleave>
>          </element>
>        </optional>
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 2ab89f5..ff4f51b 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1236,6 +1236,16 @@ char *                  virDomainGetSchedulerType(virDomainPtr domain,
> 
>  #define VIR_DOMAIN_BLKIO_WEIGHT "weight"
> 
> +/**
> + * VIR_DOMAIN_BLKIO_DEVICE_WEIGHT:
> + *
> + * Macro for the blkio tunable weight_device: it represents the
> + * per-device weight, as a string.  The string is parsed as a
> + * series of /path/to/device,weight elements, separated by ','.
> + */
> +
> +#define VIR_DOMAIN_BLKIO_DEVICE_WEIGHT "device_weight"
> +
>  /* Set Blkio tunables for the domain*/
>  int     virDomainSetBlkioParameters(virDomainPtr domain,
>                                      virTypedParameterPtr params,
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 58f4d0f..b35c83c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -603,6 +603,67 @@ VIR_ENUM_IMPL(virDomainStartupPolicy, VIR_DOMAIN_STARTUP_POLICY_LAST,
>  #define VIR_DOMAIN_XML_WRITE_FLAGS  VIR_DOMAIN_XML_SECURE
>  #define VIR_DOMAIN_XML_READ_FLAGS   VIR_DOMAIN_XML_INACTIVE
> 
> +
> +void
> +virBlkioDeviceWeightArrayClear(virBlkioDeviceWeightPtr deviceWeights,
> +                               int ndevices)
> +{
> +    int i;
> +
> +    for (i = 0; i < ndevices; i++)
> +        VIR_FREE(deviceWeights[i].path);
> +}
> +
> +/**
> + * virDomainBlkioDeviceWeightParseXML
> + *
> + * this function parses a XML node:
> + *
> + *   <device>
> + *     <path>/fully/qualified/device/path</path>
> + *     <weight>weight</weight>
> + *   </device>
> + *
> + * and fills a virBlkioDeviceWeight struct.
> + */
> +static int
> +virDomainBlkioDeviceWeightParseXML(xmlNodePtr root,
> +                                   virBlkioDeviceWeightPtr dw)
> +{
> +    char *c;
> +    xmlNodePtr node;
> +
> +    node = root->children;
> +    while (node) {
> +        if (node->type == XML_ELEMENT_NODE) {
> +            if (xmlStrEqual(node->name, BAD_CAST "path") && !dw->path) {
> +                dw->path = (char *)xmlNodeGetContent(node);
> +            } else if (xmlStrEqual(node->name, BAD_CAST "weight")) {
> +                c = (char *)xmlNodeGetContent(node);
> +                if (virStrToLong_ui(c, NULL, 10, &dw->weight) < 0) {
> +                    virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                         _("could not parse weight %s"),
> +                                         c);
> +                    VIR_FREE(c);
> +                    VIR_FREE(dw->path);
> +                    return -1;
> +                }
> +                VIR_FREE(c);
> +            }
> +        }
> +        node = node->next;
> +    }
> +    if (!dw->path) {
> +        virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                             _("missing per-device path"));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +
>  static void
>  virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
>  {
> @@ -1272,6 +1333,10 @@ void virDomainDefFree(virDomainDefPtr def)
>      VIR_FREE(def->emulator);
>      VIR_FREE(def->description);
> 
> +    virBlkioDeviceWeightArrayClear(def->blkio.devices,
> +                                   def->blkio.ndevices);
> +    VIR_FREE(def->blkio.devices);
> +
>      virDomainWatchdogDefFree(def->watchdog);
> 
>      virDomainMemballoonDefFree(def->memballoon);
> @@ -6711,6 +6776,22 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>                       &def->blkio.weight) < 0)
>          def->blkio.weight = 0;
> 
> +    if ((n = virXPathNodeSet("./blkiotune/device", ctxt, &nodes)) < 0) {
> +        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                             "%s", _("cannot extract blkiotune nodes"));
> +        goto error;
> +    }
> +    if (n && VIR_ALLOC_N(def->blkio.devices, n) < 0)
> +        goto no_memory;
> +
> +    for (i = 0; i < n; i++) {
> +        if (virDomainBlkioDeviceWeightParseXML(nodes[i],
> +                                               &def->blkio.devices[i]) < 0)
> +            goto error;
> +        def->blkio.ndevices++;
> +    }
> +    VIR_FREE(nodes);
> +
>      /* Extract other memory tunables */
>      if (virXPathULong("string(./memtune/hard_limit)", ctxt,
>                        &def->mem.hard_limit) < 0)
> @@ -10849,10 +10930,22 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>                        def->mem.cur_balloon);
> 
>      /* add blkiotune only if there are any */
> -    if (def->blkio.weight) {
> +    if (def->blkio.weight || def->blkio.devices) {
>          virBufferAddLit(buf, "  <blkiotune>\n");
> -        virBufferAsprintf(buf, "    <weight>%u</weight>\n",
> -                          def->blkio.weight);
> +
> +        if (def->blkio.weight)
> +            virBufferAsprintf(buf, "    <weight>%u</weight>\n",
> +                              def->blkio.weight);
> +
> +        for (n = 0; n < def->blkio.ndevices; n++) {
> +            virBufferAddLit(buf, "    <device>\n");
> +            virBufferEscapeString(buf, "      <path>%s</path>\n",
> +                                  def->blkio.devices[n].path);
> +            virBufferAsprintf(buf, "      <weight>%u</weight>\n",
> +                              def->blkio.devices[n].weight);
> +            virBufferAddLit(buf, "    </device>\n");
> +        }
> +
>          virBufferAddLit(buf, "  </blkiotune>\n");
>      }

We can filter out 0-weight here:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b35c83c..5160003 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10893,6 +10893,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     const char *type = NULL;
     int n, allones = 1;
+    int blkio = 0;
 
     virCheckFlags(DUMPXML_FLAGS |
                   VIR_DOMAIN_XML_INTERNAL_STATUS |
@@ -10930,7 +10931,15 @@ virDomainDefFormatInternal(virDomainDefPtr def,
                       def->mem.cur_balloon);
 
     /* add blkiotune only if there are any */
-    if (def->blkio.weight || def->blkio.devices) {
+
+    if (def->blkio.weight)
+        blkio = 1;
+    for (n = 0; n < def->blkio.ndevices; n++) {
+        if (def->blkio.devices[n].weight)
+            blkio = 1;
+    }
+
+    if (blkio) {
         virBufferAddLit(buf, "  <blkiotune>\n");
 
         if (def->blkio.weight)
@@ -10938,6 +10947,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
                               def->blkio.weight);
 
         for (n = 0; n < def->blkio.ndevices; n++) {
+            if (def->blkio.devices[n].weight == 0)
+                continue;
             virBufferAddLit(buf, "    <device>\n");
             virBufferEscapeString(buf, "      <path>%s</path>\n",
                                   def->blkio.devices[n].path);

> 
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index a3cb834..d3c6381 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1361,6 +1361,17 @@ struct _virDomainNumatuneDef {
>      /* Future NUMA tuning related stuff should go here. */
>  };
> 
> +typedef struct _virBlkioDeviceWeight virBlkioDeviceWeight;
> +typedef virBlkioDeviceWeight *virBlkioDeviceWeightPtr;
> +struct _virBlkioDeviceWeight {
> +    char *path;
> +    unsigned int weight;
> +};
> +
> +void virBlkioDeviceWeightArrayClear(virBlkioDeviceWeightPtr deviceWeights,
> +                                    int ndevices);
> +
> +
>  /*
>   * Guest VM main configuration
>   *
> @@ -1378,6 +1389,9 @@ struct _virDomainDef {
> 
>      struct {
>          unsigned int weight;
> +
> +        size_t ndevices;
> +        virBlkioDeviceWeightPtr devices;
>      } blkio;
> 
>      struct {
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index b15a8db..d78fd53 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -231,6 +231,7 @@ virDomainAuditVcpu;
> 
> 
>  # domain_conf.h
> +virBlkioDeviceWeightArrayClear;
>  virDiskNameToBusDeviceIndex;
>  virDiskNameToIndex;
>  virDomainActualNetDefFree;
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml
> new file mode 100644
> index 0000000..3412753
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml
> @@ -0,0 +1,36 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory>219136</memory>
> +  <currentMemory>219136</currentMemory>
> +  <blkiotune>
> +    <weight>800</weight>
> +    <device>
> +      <path>/dev/sda</path>
> +      <weight>400</weight>
> +    </device>
> +    <device>
> +      <path>/dev/sdb</path>
> +      <weight>900</weight>
> +    </device>
> +  </blkiotune>
> +  <vcpu>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu</emulator>
> +    <disk type='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' unit='0'/>
> +    </disk>
> +    <controller type='ide' index='0'/>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 83dc3c7..a1d2afd 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -4648,6 +4648,8 @@ static const vshCmdOptDef opts_blkiotune[] = {
>      {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>      {"weight", VSH_OT_INT, VSH_OFLAG_NONE,
>       N_("IO Weight in range [100, 1000]")},
> +    {"device-weights", VSH_OT_STRING, VSH_OFLAG_NONE,
> +     N_("per-device IO Weights, in the form of /path/to/device,weight,...")},
>      {"config", VSH_OT_BOOL, 0, N_("affect next boot")},
>      {"live", VSH_OT_BOOL, 0, N_("affect running domain")},
>      {"current", VSH_OT_BOOL, 0, N_("affect current domain")},
> @@ -4658,6 +4660,7 @@ static bool
>  cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
>  {
>      virDomainPtr dom;
> +    const char *device_weight = NULL;
>      int weight = 0;
>      int nparams = 0;
>      int rv = 0;
> @@ -4702,6 +4705,16 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
>          }
>      }
> 
> +    rv = vshCommandOptString(cmd, "device-weights", &device_weight);
> +    if (rv < 0) {
> +        vshError(ctl, "%s",
> +                 _("Unable to parse string parameter"));
> +        goto cleanup;
> +    }
> +    if (rv > 0) {
> +        nparams++;
> +    }
> +
>      if (nparams == 0) {
>          /* get the number of blkio parameters */
>          if (virDomainGetBlkioParameters(dom, NULL, &nparams, flags) != 0) {
> @@ -4749,12 +4762,14 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
>                      vshPrint(ctl, "%-15s: %d\n", params[i].field,
>                               params[i].value.b);
>                      break;
> +                case VIR_TYPED_PARAM_STRING:
> +                    vshPrint(ctl, "%-15s: %s\n", params[i].field,
> +                             params[i].value.s);
> +                    break;
>                  default:
>                      vshPrint(ctl, "unimplemented blkio parameter type\n");
>              }
>          }
> -
> -        ret = true;
>      } else {
>          /* set the blkio parameters */
>          params = vshCalloc(ctl, nparams, sizeof(*params));
> @@ -4765,18 +4780,30 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
> 
>              if (weight) {
>                  temp->value.ui = weight;
> -                strncpy(temp->field, VIR_DOMAIN_BLKIO_WEIGHT,
> -                        sizeof(temp->field));
> -                weight = 0;
> +                if (!virStrcpy(temp->field, VIR_DOMAIN_BLKIO_WEIGHT,
> +                               sizeof(temp->field)))
> +                    goto cleanup;
> +            }
> +
> +            if (device_weight) {
> +                temp->value.s = vshStrdup(ctl, device_weight);
> +                temp->type = VIR_TYPED_PARAM_STRING;
> +                if (!virStrcpy(temp->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT,
> +                               sizeof(temp->field)))
> +                    goto cleanup;
>              }
>          }
> -        if (virDomainSetBlkioParameters(dom, params, nparams, flags) != 0)
> +
> +        if (virDomainSetBlkioParameters(dom, params, nparams, flags) < 0) {
>              vshError(ctl, "%s", _("Unable to change blkio parameters"));
> -        else
> -            ret = true;
> +            goto cleanup;
> +        }
>      }
> 
> +    ret = true;
> +
>    cleanup:
> +    virTypedParameterArrayClear(params, nparams);
>      VIR_FREE(params);
>      virDomainFree(dom);
>      return ret;
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 775d302..ef024c5 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1032,12 +1032,18 @@ value are kilobytes (i.e. blocks of 1024 bytes).
> 
>  Specifying -1 as a value for these limits is interpreted as unlimited.
> 
> -=item B<blkiotune> I<domain-id> [I<--weight> B<weight>] [[I<--config>]
> +=item B<blkiotune> I<domain-id> [I<--weight> B<weight>]
> +[I<--device-weights> B<device-weights>] [[I<--config>]
>  [I<--live>] | [I<--current>]]
> 
>  Display or set the blkio parameters. QEMU/KVM supports I<--weight>.
>  I<--weight> is in range [100, 1000].
> 
> +B<device-weights> is a single string listing one or more device/weight
> +pairs, in the format of /path/to/device,weight,/path/to/device,weight.
> +Each weight is in the range [100, 1000], or the value 0 to remove that
> +device from per-device listings.
> +
>  If I<--live> is specified, affect a running guest.
>  If I<--config> is specified, affect the next boot of a persistent guest.
>  If I<--current> is specified, affect the current guest state.
> -- 
> 1.7.7.1

-- 
Thanks,
Hu Tao


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