[virt-tools-list] [PATCH 1/3] virt-manager: add support for domain title
Cole Robinson
crobinso at redhat.com
Fri Sep 20 21:09:41 UTC 2013
On 09/10/2013 10:07 AM, Giuseppe Scrivano wrote:
> Commit b79ba8382e2205c416d7c4836ac9ee08c72e2c56 in libvirt adds a
> <title> attribute to the domain XML, that is used by the user to
> identify more easily the VM. This feature is not supported by all
> HVs. This patch allows to see and modify the title from the details
> window.
>
> Something similar was requested here:
> https://bugzilla.redhat.com/show_bug.cgi?id=798949
>
> Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
> ---
> ui/vmm-details.ui | 84 +++++++++++++++++++++++++++++++++-----------------
> virtManager/details.py | 18 ++++++++++-
> virtManager/domain.py | 51 ++++++++++++++++++++++++++++--
> 3 files changed, 121 insertions(+), 32 deletions(-)
>
> diff --git a/ui/vmm-details.ui b/ui/vmm-details.ui
> index 9ce87f4..2b42408 100644
> --- a/ui/vmm-details.ui
> +++ b/ui/vmm-details.ui
> @@ -781,7 +781,7 @@
> <property name="visible">True</property>
> <property name="can_focus">False</property>
> <property name="border_width">3</property>
> - <property name="n_rows">5</property>
> + <property name="n_rows">6</property>
> <property name="n_columns">2</property>
> <property name="column_spacing">6</property>
> <property name="row_spacing">6</property>
> @@ -884,49 +884,39 @@
> </packing>
> </child>
> <child>
> - <object class="GtkLabel" id="label24">
> + <object class="GtkEntry" id="overview-name">
> <property name="visible">True</property>
> - <property name="can_focus">False</property>
> - <property name="xalign">0</property>
> - <property name="yalign">0</property>
> - <property name="label" translatable="yes">Description:</property>
> + <property name="can_focus">True</property>
> + <signal name="changed" handler="on_overview_name_changed" swapped="no"/>
> </object>
> <packing>
> - <property name="top_attach">3</property>
> - <property name="bottom_attach">4</property>
> - <property name="x_options">GTK_FILL</property>
> + <property name="left_attach">1</property>
> + <property name="right_attach">2</property>
> <property name="y_options">GTK_FILL</property>
> </packing>
> </child>
> <child>
> - <object class="GtkScrolledWindow" id="scrolledwindow2">
> + <object class="GtkAlignment" id="alignment14">
> <property name="visible">True</property>
> - <property name="can_focus">True</property>
> - <property name="shadow_type">in</property>
> - <property name="min_content_height">80</property>
> + <property name="can_focus">False</property>
> <child>
> - <object class="GtkTextView" id="overview-description">
> - <property name="visible">True</property>
> - <property name="can_focus">True</property>
> - <property name="wrap_mode">word</property>
> - </object>
> + <placeholder/>
> </child>
> </object>
> <packing>
> - <property name="left_attach">1</property>
> - <property name="right_attach">2</property>
> - <property name="top_attach">3</property>
> - <property name="bottom_attach">5</property>
> + <property name="top_attach">5</property>
> + <property name="bottom_attach">6</property>
> + <property name="x_options">GTK_FILL</property>
> <property name="y_options">GTK_FILL</property>
> </packing>
> </child>
> <child>
> - <object class="GtkAlignment" id="alignment14">
> + <object class="GtkLabel" id="label24">
> <property name="visible">True</property>
> <property name="can_focus">False</property>
> - <child>
> - <placeholder/>
> - </child>
> + <property name="xalign">0</property>
> + <property name="yalign">0</property>
> + <property name="label" translatable="yes">Description:</property>
> </object>
> <packing>
> <property name="top_attach">4</property>
> @@ -936,14 +926,52 @@
> </packing>
> </child>
> <child>
> - <object class="GtkEntry" id="overview-name">
> + <object class="GtkLabel" id="label81">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="xalign">0</property>
> + <property name="label" translatable="yes">Title:</property>
> + </object>
> + <packing>
> + <property name="top_attach">3</property>
> + <property name="bottom_attach">4</property>
> + <property name="x_options">GTK_FILL</property>
> + <property name="y_options"/>
> + </packing>
> + </child>
> + <child>
> + <object class="GtkEntry" id="overview-title">
> <property name="visible">True</property>
> <property name="can_focus">True</property>
> - <signal name="changed" handler="on_overview_name_changed" swapped="no"/>
> + <signal name="changed" handler="on_overview_title_changed" swapped="no"/>
> + </object>
> + <packing>
> + <property name="left_attach">1</property>
> + <property name="right_attach">2</property>
> + <property name="top_attach">3</property>
> + <property name="bottom_attach">4</property>
> + <property name="y_options">GTK_FILL</property>
> + </packing>
> + </child>
> + <child>
> + <object class="GtkScrolledWindow" id="scrolledwindow2">
> + <property name="visible">True</property>
> + <property name="can_focus">True</property>
> + <property name="shadow_type">in</property>
> + <property name="min_content_height">80</property>
> + <child>
> + <object class="GtkTextView" id="overview-description">
> + <property name="visible">True</property>
> + <property name="can_focus">True</property>
> + <property name="wrap_mode">word</property>
> + </object>
> + </child>
> </object>
> <packing>
> <property name="left_attach">1</property>
> <property name="right_attach">2</property>
> + <property name="top_attach">4</property>
> + <property name="bottom_attach">6</property>
> <property name="y_options">GTK_FILL</property>
> </packing>
> </child>
> diff --git a/virtManager/details.py b/virtManager/details.py
> index a4fddd4..21b93fa 100644
> --- a/virtManager/details.py
> +++ b/virtManager/details.py
> @@ -43,6 +43,7 @@ from virtinst import util
>
> # Parameters that can be editted in the details window
> (EDIT_NAME,
> +EDIT_TITLE,
> EDIT_ACPI,
> EDIT_APIC,
> EDIT_CLOCK,
> @@ -93,7 +94,7 @@ EDIT_WATCHDOG_ACTION,
> EDIT_CONTROLLER_MODEL,
>
> EDIT_TPM_TYPE,
> -) = range(1, 40)
> +) = range(1, 41)
>
>
> # Columns in hw list model
> @@ -434,6 +435,7 @@ class vmmDetails(vmmGObjectUI):
> "on_details_pages_switch_page": self.switch_page,
>
> "on_overview_name_changed": lambda *x: self.enable_apply(x, EDIT_NAME),
> + "on_overview_title_changed": lambda *x: self.enable_apply(x, EDIT_TITLE),
> "on_overview_acpi_changed": self.config_acpi_changed,
> "on_overview_apic_changed": self.config_apic_changed,
> "on_overview_clock_changed": lambda *x: self.enable_apply(x, EDIT_CLOCK),
> @@ -1993,6 +1995,11 @@ class vmmDetails(vmmGObjectUI):
> name = self.widget("overview-name").get_text()
> add_define(self.vm.define_name, name)
>
> + if self.edited(EDIT_TITLE):
> + title = self.widget("overview-title").get_text()
> + add_define(self.vm.define_title, title)
> + add_hotplug(self.vm.define_title, title)
> +
> if self.edited(EDIT_ACPI):
> enable_acpi = self.widget("overview-acpi").get_active()
> if self.widget("overview-acpi").get_inconsistent():
> @@ -2556,6 +2563,15 @@ class vmmDetails(vmmGObjectUI):
> desc = self.vm.get_description() or ""
> desc_widget = self.widget("overview-description")
> desc_widget.get_buffer().set_text(desc)
> + title = self.vm.get_title()
> + if title is not None:
> + self.widget("overview-title").set_sensitive(True)
> + self.widget("overview-title").set_text(self.vm.get_title() or "")
> + else:
> + # TITLE is None when the HV doesn't support the <title> metadata,
> + # so disable the possibility of changing it.
> + self.widget("overview-title").set_sensitive(False)
> + self.widget("overview-title").set_text("")
>
I like to do this in one swoop, like
title = self.vm.get_title()
self.widget("overview-title").set_sensitive(bool(title))
self.widget("overview-title").set_text(title or "")cool
> # Hypervisor Details
> self.widget("overview-hv").set_text(self.vm.get_pretty_hv_type())
> diff --git a/virtManager/domain.py b/virtManager/domain.py
> index cf2825d..ca000c3 100644
> --- a/virtManager/domain.py
> +++ b/virtManager/domain.py
> @@ -200,6 +200,7 @@ class vmmDomain(vmmLibvirtObject):
> self._is_management_domain = None
> self._id = None
> self._name = None
> + self._title = None
> self._snapshot_list = None
>
> self._inactive_xml_flags = 0
> @@ -326,9 +327,38 @@ class vmmDomain(vmmLibvirtObject):
> ###########################
>
> def get_name(self):
> - if self._name is None:
> - self._name = self._backend.name()
> - return self._name
> + return self._backend.name()
> +
Don't drop the name caching please. While the GetName() API doesn't do a
network round trip, it clutters up reports from ./virt-manager --trace-libvirt
when I'm trying to reduce the number of libvirt API calls virt-manager makes.
> + def get_descriptive_name(self):
> + # When available, include the title in the name
> + name = self.get_name()
> + title = self.get_title()
> + if title:
> + return "%s - %s" % (name, title)
> + return name
> +
> + def get_title(self):
> + # Return None when the hypervisor doesn't support a title for the
> + # domain
> + if self._title is None:
> + try:
> + self._title = self._backend.metadata(
> + libvirt.VIR_DOMAIN_METADATA_TITLE,
> + None,
> + 0)
> + except libvirt.libvirtError, e:
> + # For us there is no difference between no metadata and an
> + # empty title.
> + err_no_metadata = "VIR_ERR_NO_DOMAIN_METADATA"
> + err_no_support = "VIR_ERR_NO_SUPPORT"
> + if uihelpers.exception_is_libvirt_error(e, err_no_metadata):
> + return ""
> + elif uihelpers.exception_is_libvirt_error(e, err_no_support):
> + return None
> + else:
> + raise e
> +
> + return self._title or ""
>
This pattern is sub-optimal for a few reasons.
1) Every time we access title its a separate API call, which we try to do as
few as possible to cut down on remote connection traffic.
2) We don't cache the 'no support' result, so if a HV doesn't support the
metadata call, we will do an unneeded API call every time we try to access
title even though we know it fails.
3) It's using a separate data location rather than the XML.
>From what I can tell from the libvirt code, title is also reflected in the XML
at /domain/title. So here's what I'd do:
- Get title from the XML
- Have define_title change guest.title, like say changing the description.
Just have hotplug_title try the set_metadata command (a future HV could
theoretically allow changing the title but only for an offline VM).
- Use virtinst/support.py to check if getmetadata is even supported. Then
cache the result in domain.py like we do for self.managedsave_supported. (we
already globally cache support checks per connection in virtinst/connection.py
but don't do it for VMs yet, since it's likely a bit more difficult. Though in
general we can probably assume that if one VM in a connection supports an API
call then all VMs on that connection support the API, but I think there are
corner cases where that isn't true.) Briefly tested support check could look like:
diff --git a/virtinst/support.py b/virtinst/support.py
index b0a84f2..3edeb3a 100644
--- a/virtinst/support.py
+++ b/virtinst/support.py
@@ -344,7 +344,9 @@ SUPPORT_DOMAIN_SET_METADATA = _make(version=9010)
SUPPORT_DOMAIN_CPU_HOST_MODEL = _make(version=9010)
SUPPORT_DOMAIN_LIST_SNAPSHOTS = _make(function="virDomain.listAllSnapshots",
args=())
-
+SUPPORT_DOMAIN_GET_METADATA = _make(
+ function="virDomain.metadata",
+ args=(getattr(libvirt, "VIR_DOMAIN_METADATA_TITLE", 1), None, 0))
# Pool checks
# This can't ever require a pool object for back compat reasons
> def get_id(self):
> if self._id is None:
> @@ -387,6 +417,7 @@ class vmmDomain(vmmLibvirtObject):
> vmmLibvirtObject._invalidate_xml(self)
> self._guest_to_define = None
> self._name = None
> + self._title = None
> self._id = None
>
> def _get_guest_to_define(self):
> @@ -487,6 +518,20 @@ class vmmDomain(vmmLibvirtObject):
>
> self.emit("config-changed")
>
> + def define_title(self, title):
> + logging.debug("Changing guest title to '%s'", title)
> + try:
> + self._title = self._backend.setMetadata(
> + libvirt.VIR_DOMAIN_METADATA_TITLE,
> + title,
> + None,
> + None,
> + 0)
> + finally:
> + self._invalidate_xml()
> +
> + self.emit("config-changed")
> +
As mentioned above, I'd prefer to split this into define_title which goes the
XML route, and hotplug_title which does the setMetadata. You should be able to
follow the pattern we use for changing the guest description.
In fact, this impl may be buggy? Does it persist the title change if run on an
active VM?
Thanks,
Cole
More information about the virt-tools-list
mailing list