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@xxxxxxxxxx> > --- > 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 _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
