[virt-tools-list] [PATCH] Virt-manager CPU pinning support
Cole Robinson
crobinso at redhat.com
Thu Jul 23 01:32:00 UTC 2009
Michal Novotny wrote:
>
> On 07/21/2009 05:54 PM, Michal Novotny wrote:
>> Hi all,
>> since having virt-manager with CPU pinning support would be a nice new
>> feature I've added this support to my virt-manager (that has been
>> updated from hg repository today to the newest version so there should
>> be no problem with codebase). Basically it shows "CPU Affinity string"
>> entry on Processor tab in VM details. Apply button is not applicable
>> when you provide bad CPU pinning string so that you should
>> double-check the pinning string if you can't apply changes.
I would prefer to have the 'Apply' button made sensitive unconditionally once
the user starts typing. If the cpuset string they entered is invalid, throw up
an error box with the exception generated, so they can try and fix their
string. Not providing any feedback excepting debug logging won't help them
generate a correct string.
If you
>> want to disable CPU pinning, just delete the string to default value
>> (zero length string) and it will unset this entry in libvirt XML file
>> it won't be used for domain creation then.
>>
>> Michal
> # HG changeset patch
> # User Michal Novotny <minovotn at redhat.com>
> # Date 1248193428 -7200
> # Node ID 42231ab8d6d547ef9831f4e8cf9be451812b64d1
> # Parent e26f11f4234cfd4c6e802b92a570e30fef0b7291
> CPU Pinning support
>
> Virt-manager doesn't consider CPU pinning settings in the Processor tab yet so
> this is the patch to provide CPU pinning support for virt-manager. It's using
> only CPU definition for libvirt itself and it doesn't support CPU pinning for
> domains that are currently in running state since this feature will be rarely
> used for already running domains in my opinion.
>
> This is the second version with showing the current affinity on Processor tab
> fixed.
>
> diff -r e26f11f4234c -r 42231ab8d6d5 src/virtManager/details.py
> --- a/src/virtManager/details.py Mon Jul 20 15:09:32 2009 -0400
> +++ b/src/virtManager/details.py Tue Jul 21 18:23:48 2009 +0200
> @@ -288,6 +288,7 @@
> "on_details_pages_switch_page": self.switch_page,
>
> "on_config_vcpus_changed": self.config_vcpus_changed,
> + "on_config_vcpupin_changed": self.config_vcpupin_changed,
> "on_config_memory_changed": self.config_memory_changed,
> "on_config_maxmem_changed": self.config_maxmem_changed,
> "on_config_boot_device_changed": self.config_boot_options_changed,
> @@ -969,6 +970,7 @@
> if not(self.window.get_widget("config-apply").get_property("sensitive")):
> self.window.get_widget("config-vcpus").get_adjustment().value = self.vm.vcpu_count()
> self.window.get_widget("state-vm-vcpus").set_text("%d" % (self.vm.vcpu_count()))
> + self.window.get_widget("config-vcpupin").set_text("%s" % self.vm.vcpu_pinning() )
>
> def refresh_config_memory(self):
> self.window.get_widget("state-host-memory").set_text("%d MB" % (int(round(self.vm.get_connection().host_memory_size()/1024))))
> @@ -1592,10 +1594,29 @@
> def config_vcpus_changed(self, src):
> self.window.get_widget("config-apply").set_sensitive(True)
>
> + def config_vcpupin_changed(self, src):
> + val = src.get_text()
> +
> + # Since for cpuset we require None for define_vcpus, we'll change this now
> + if len(val) == 0:
> + val = None
> +
> + enable = False
> + if self.vm.check_cpuset_syntax(val):
> + enable = True
> + self.window.get_widget("config-apply").set_sensitive(enable)
> +
> def config_vcpus_apply(self):
> vcpus = self.window.get_widget("config-vcpus").get_adjustment().value
> - logging.info("Setting vcpus for %s to %s" % (self.vm.get_name(),
> - str(vcpus)))
> + cpuset = self.window.get_widget("config-vcpupin").get_text()
> +
> + # Since for cpuset we require None for define_vcpus, we'll change this now
> + if len(cpuset) == 0:
> + cpuset = None
> +
> + logging.info("Setting vcpus for %s to %s, cpuset is %s" %
> + (self.vm.get_name(), str(vcpus), cpuset))
> +
> hotplug_err = False
>
> try:
> @@ -1607,7 +1628,7 @@
>
> # Change persistent config
> try:
> - self.vm.define_vcpus(vcpus)
> + self.vm.define_vcpus(vcpus, cpuset)
> except Exception, e:
> self.err.show_err(_("Error changing vcpu value: %s" % str(e)),
> "".join(traceback.format_exc()))
> diff -r e26f11f4234c -r 42231ab8d6d5 src/virtManager/domain.py
> --- a/src/virtManager/domain.py Mon Jul 20 15:09:32 2009 -0400
> +++ b/src/virtManager/domain.py Tue Jul 21 18:23:48 2009 +0200
> @@ -24,6 +24,7 @@
> import logging
> import time
> import difflib
> +import re
>
> from virtManager import util
> import virtinst.util as vutil
> @@ -550,6 +551,13 @@
> return 0
> return self.record[0]["vcpuCount"]
>
> + def vcpu_pinning(self):
> + cpuset = vutil.get_xml_path(self.get_xml(), "/domain/vcpu/@cpuset")
> + # We need to set it to empty string not to show None in the entry
> + if cpuset is None:
> + cpuset = ""
> + return cpuset
> +
> def vcpu_max_count(self):
> cpus = vutil.get_xml_path(self.get_xml(), "/domain/vcpu")
> return int(cpus)
> @@ -1304,19 +1312,65 @@
> if vcpus != self.vcpu_count():
> self.vm.setVcpus(vcpus)
>
> - def define_vcpus(self, vcpus):
> + # This function checks the cpuset mask syntax, None is allowed
> + def check_cpuset_syntax(self, val):
> + if val is None:
> + return True
> + if type(val) is not type("string") or len(val) == 0:
> + logging.debug("Cpuset mask error must be string")
> + return False
> +
> + if re.match("^[0-9,-]*$", val) is None:
> + logging.debug("Cpuset mask can only contain numeric, ',', or "
> + "'-' characters")
> + return False
> +
> + pcpus = vutil.get_phy_cpus(self.connection.vmm)
> + for c in val.split(','):
> + if c.find('-') != -1:
> + (x, y) = c.split('-')
> + if int(x) > int(y):
> + logging.debug("Cpuset mask contains invalid format.")
> + return False
> +
> + if int(x) >= pcpus or int(y) >= pcpus:
> + logging.debug("Cpuset mask pCPU numbers must be less "
> + "than pCPUs.")
> + return False
> + else:
> + if int(c) >= pcpus:
> + logging.debug("Cpuset mask pCPU numbers must be less "
> + "than pCPUs.")
> + return True
> +
Since this code is largely a duplicate of the code from virtinst, I would
prefer reusing it. Just do something like
guest = virtinst.Guest(conn = self.get_connection().vmm)
guest.cpuset = val
We also don't want to call this function every time the text box is changed:
it will flood the debug log with multiple error messages as the user types the
string.
Another thing, if the typed string is ends with a comma, the above code
generates an unhandled exception. We probably want to explicitly handle that
case, either throwing a proper warning or sanitizing the string. This should
be fixed at the virtinst level.
> + def define_vcpus(self, vcpus, cpuset=None):
> vcpus = int(vcpus)
>
> def set_node(doc, ctx, val, xpath):
> node = ctx.xpathEval(xpath)
> node = (node and node[0] or None)
>
> + # Val argument is a tuple of vcpu count and cpu mask
> + if type(val) == tuple and len(val) > 1:
> + vcpus = val[0]
> + cpumask = val[1]
> + else:
> + vcpus = val
> +
> if node:
> - node.setContent(str(val))
> + node.setContent(str(vcpus))
> +
> + # If cpuset mask is not valid, don't change it
> + # If cpuset mask is None, we don't want to use cpuset
> + if cpumask is None or (cpumask is not None
> + and len(cpumask) == 0):
> + node.unsetProp("cpuset")
> + elif self.check_cpuset_syntax(cpumask):
> + node.setProp("cpuset", cpumask)
> return doc.serialize()
>
> def change_vcpu_xml(xml, vcpus):
> - return util.xml_parse_wrapper(xml, set_node, vcpus,
> + return util.xml_parse_wrapper(xml, set_node, (vcpus, cpuset),
> "/domain/vcpu[1]")
No need to cram vcpus and cpuset into a tuple, just change the 'set_node'
definition and invoke xml_parse_wrapper with
util.xml_parse_wrapper(xml, set_node, vcpus, cpuset,
"/domain/vcpu[1]")
>
> self.redefine(change_vcpu_xml, vcpus)
> diff -r e26f11f4234c -r 42231ab8d6d5 src/vmm-details.glade
> --- a/src/vmm-details.glade Mon Jul 20 15:09:32 2009 -0400
> +++ b/src/vmm-details.glade Tue Jul 21 18:23:48 2009 +0200
> @@ -1567,14 +1567,27 @@
> </packing>
> </child>
> <child>
> + <widget class="GtkLabel" id="label336">
> + <property name="visible">True</property>
> + <property name="xalign">1</property>
> + <property name="label" translatable="yes">CPU Affinity string:</property>
> + </widget>
Hmm, maybe change the string to 'Use physical CPUs:', or
'Physical CPU pinning:'.
Thanks,
Cole
More information about the virt-tools-list
mailing list