After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 702011 - ovirt: Set host-subject when needed
ovirt: Set host-subject when needed
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
unspecified
Other All
: Normal normal
: 3.22
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-06-11 13:53 UTC by Christophe Fergeau
Modified: 2016-03-31 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ovirt: Set host-subject when needed (4.64 KB, patch)
2013-06-11 13:53 UTC, Christophe Fergeau
accepted-commit_now Details | Review
coding-style: Add space before (int) (2.68 KB, patch)
2013-06-12 08:48 UTC, Christophe Fergeau
accepted-commit_now Details | Review
Update our govirt requirement to 0.1.0 (2.36 KB, patch)
2013-06-12 08:49 UTC, Christophe Fergeau
committed Details | Review
ovirt: Set host-subject when needed (3.08 KB, patch)
2013-06-12 08:49 UTC, Christophe Fergeau
committed Details | Review
coding-style: Add space before (cast) (5.46 KB, patch)
2013-06-12 14:27 UTC, Christophe Fergeau
committed Details | Review

Description Christophe Fergeau 2013-06-11 13:53:39 UTC
oVirt 3.2/libgovirt 0.1.0 can be used to know the host subject
to use to validate the SSL certificate of a given VM. This
host subject can be needed in some cases, for example when oVirt VMs
migrated from one host to another.

The BOXES_SPICE_HOST_SUBJECT environment variable hack is kept for now
as this is still needed on older oVirt instances which don't provide the
host subject in their REST API.
Comment 1 Christophe Fergeau 2013-06-11 13:53:41 UTC
Created attachment 246523 [details] [review]
ovirt: Set host-subject when needed
Comment 2 Zeeshan Ali 2013-06-11 20:33:37 UTC
Review of attachment 246523 [details] [review]:

Looks good otherwise.

::: configure.ac
@@ +51,3 @@
 AC_SUBST(GLIB_MIN_VERSION)
 GOBJECT_INTROSPECTION_MIN_VERSION=0.9.6
+GOVIRT_MIN_VERSION=0.1.0

If you could put that into a separate patch before pushing this change, that would be nice.

::: src/ovirt-machine.vala
@@ +82,3 @@
         switch (vm.display.type) {
         case Ovirt.VmDisplayType.SPICE:
+            var display = new SpiceDisplay (config, vm.display.address, (int)vm.display.port,

coding-style nitpicks:

* space after (int).
* each arg on separate line.

::: src/spice-display.vala
@@ +106,3 @@
         // FIXME: remove this once libgovirt has proper support for
         // getting the SPICE host subject, it's useful for testing purpose
         // in the mean time

This comment needs update.
Comment 3 Christophe Fergeau 2013-06-12 08:48:58 UTC
Created attachment 246592 [details] [review]
coding-style: Add space before (int)
Comment 4 Christophe Fergeau 2013-06-12 08:49:02 UTC
Created attachment 246593 [details] [review]
Update our govirt requirement to 0.1.0
Comment 5 Christophe Fergeau 2013-06-12 08:49:06 UTC
Created attachment 246594 [details] [review]
ovirt: Set host-subject when needed

oVirt 3.2/libgovirt 0.1.0 can be used to know the host subject
to use to validate the SSL certificate of a given VM. This
host subject can be needed in some cases, for example when oVirt VMs
migrated from one host to another.

The BOXES_SPICE_HOST_SUBJECT environment variable hack is kept for now
as this is still needed on older oVirt instances which don't provide the
host subject in their REST API.
Comment 6 Zeeshan Ali 2013-06-12 14:12:17 UTC
Review of attachment 246592 [details] [review]:

ACK.

::: src/display-page.vala
@@ +106,3 @@
                     window.get_window ().get_geometry (null, null, out width, null);
+                    window.begin_move_drag ((int) button_down_button,
+                                            root_x + (int) ((button_down_x / (double)old_width) * width),

missed one. :)
Comment 7 Zeeshan Ali 2013-06-12 14:13:22 UTC
Review of attachment 246593 [details] [review]:

ACK, assuming govirt 0.1.0 is either already released or will be before next 3.9.x release.
Comment 8 Zeeshan Ali 2013-06-12 14:14:05 UTC
Review of attachment 246594 [details] [review]:

ACK
Comment 9 Christophe Fergeau 2013-06-12 14:27:09 UTC
Created attachment 246646 [details] [review]
coding-style: Add space before (cast)
Comment 10 Christophe Fergeau 2013-06-12 14:28:21 UTC
Review of attachment 246592 [details] [review]:

::: src/display-page.vala
@@ +106,3 @@
                     window.get_window ().get_geometry (null, null, out width, null);
+                    window.begin_move_drag ((int) button_down_button,
+                                            root_x + (int) ((button_down_x / (double)old_width) * width),

if this is about the (double)..., note the commit log explicitly mentions (int).
Comment 11 Zeeshan Ali 2013-06-12 14:51:54 UTC
Review of attachment 246646 [details] [review]:

ACK
Comment 12 Christophe Fergeau 2013-06-13 07:18:31 UTC
Attachment 246593 [details] pushed as 3416239 - Update our govirt requirement to 0.1.0
Attachment 246594 [details] pushed as 9ba2b56 - ovirt: Set host-subject when needed
Attachment 246646 [details] pushed as 488f2fb - coding-style: Add space before (cast)