Re: [virt-tools-list] [PATCH v2] Virtuozzo hypervisor basic support

2017-02-16 Thread Cole Robinson 
Thanks for the patch, especially taking a stab at the dogtail tests!

Please split this patch up, at least into 3 parts: virtinst/cli test bits,
virt-manager misc bits, virt-manager create wizard bits.

I just did a quick pass, a couple comments inline below

On 02/15/2017 07:01 AM, Mikhail Feoktistov wrote:
> This patch introduces virtuozzo hypervisor support.
> Here we implemented only basic functionality.
> User can create/start/stop/delete containers.
> Also we allow to change hardware configuration (basic devices)
> and connect via VNC. We are very intrested in the development of
> virt-manager to support all virtuozzo features.
> 
> GUI changes:
> Add virtuozzo hypervisor to connection list.
> Add radio buttons for choosing VM or container virtualization type.
> New wizzard window for setting template name for containers.
> 
> Creating guest:
> We don't call createXML() for virtuozzo guests, because
> virtuozzo driver in libvirt doesn't support transient domains.
> Instead of this we call defineXML() and createDomain().
> 
> If we create container from template we generate XML which
> contains only one storage device with type "template".
> Virtuozzo hypervisor will create new container and filesystem for it.
> After container was created we should not call the second
> defineXML(final_xml) because it rewrites "devices" section in XML
> and deletes container filesytem.
> ---
>  .../compare/virt-install-vz-ct-template.xml|  26 
>  tests/clitest.py   |  15 +++
>  tests/uitests/newvm.py |  23 
>  tests/utils.py |   1 +
>  ui/create.ui   | 149 
> -
>  virtManager/addhardware.py |   2 +-
>  virtManager/connect.py |   8 +-
>  virtManager/connection.py  |   1 +
>  virtManager/create.py  |  58 +++-
>  virtinst/connection.py |   3 +
>  virtinst/guest.py  |  22 ++-
>  virtinst/uri.py|   4 +-
>  12 files changed, 296 insertions(+), 16 deletions(-)
>  create mode 100644 tests/cli-test-xml/compare/virt-install-vz-ct-template.xml
> diff --git a/virtinst/guest.py b/virtinst/guest.py
> index 7d3fb9d..9f6cbcf 100644
> --- a/virtinst/guest.py
> +++ b/virtinst/guest.py
> @@ -358,6 +358,9 @@ class Guest(XMLBuilder):
>  # so use preserve
>  self.on_crash = "preserve"
>  
> +if self.type == "vz":
> +self.on_crash = "destroy"
> +

What's the justification for this? Is there an issue with the libvirt default
of on_crash=reboot ?

(that said I've thought about changing the default to on_crash=destroy
anyways, since it just seems like the sensible default anyways, and for common
qemu usage it's not going to have any functional difference anyways)

>  self._set_osxml_defaults()
>  
>  self.bootloader = None
> @@ -394,11 +397,16 @@ class Guest(XMLBuilder):
>  meter = util.ensure_meter(meter)
>  meter.start(size=None, text=meter_label)
>  
> -if doboot or transient or self.installer.has_install_phase():
> -self.domain = self.conn.createXML(install_xml or final_xml, 0)
> -
> -if not transient:
> +if self.type == "vz":
>  self.domain = self.conn.defineXML(final_xml)
> +if doboot:
> +self.domain.create()
> +else:
> +if doboot or transient or self.installer.has_install_phase():
> +self.domain = self.conn.createXML(install_xml or final_xml, 
> 0)
> +
> +if not transient:
> +self.domain = self.conn.defineXML(final_xml)

This has an issue, in that if define() works but create() fails, the guest
will be left defined on the host, which is a semantic change from the existing
behavior. That's one of the nice things about createXML at least, is that it
handles that for us.

That said if virtuozzo doesn't support createXML, I'm fine with just changing
this default entirely so we don't need to maintain different code paths. But
that can come later.

Thanks,
Cole

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer 00/10] Make dialogs use GtkHeaderBar

2017-02-16 Thread Daniel P. Berrange 
On Thu, Feb 16, 2017 at 11:05:13AM -0200, Eduardo Lima (Etrunko) wrote:
> On 16/02/17 10:06, Pavel Grunt wrote:
> > Hi,
> > 
> > On Thu, 2017-02-09 at 17:22 -0200, Eduardo Lima (Etrunko) wrote:
> >> This whole series is the result of the initial idea of having the
> >> new
> >> iso-dialog to use this widget. Having it done only for the iso-
> >> dialog
> >> would not make much sense, so I went on and ported all other
> >> dialogs.
> >> The widget requires Gtk version 3.12, which should not be a problem
> >> for
> >> any recent distro.
> >>
> >> Using GtkHeaderBar makes dialogs look cleaner and more 'modern',
> >> also
> >> follow the style used by GNOME applications. In the near future,
> >> virt-viewer main window could also make use of it, as we recently
> >> saw in
> >> a series sent to this mailing list.
> > 
> > so are we going for the GNOME style for all desktop envs ? In my
> > opinion it does not look nice in Windows and Xfce - simply does not
> > fit it in. iirc it was mentioned that there is a way to have both the
> > classic and the GNOME look in the discussion about Sagar's patches few
> > weeks ago..
> 
> I have no idea of what it looks like in Windows, but it indeed looks
> good in Linux environments. I tested in both GNOME and Enlightenment,
> not sure about other DEs out there. As far as I remember, the discussion
> on that other series was about the classic menu versus the buttons on
> the header bar.

Please post some screenshots showing the before & after state of this
on GNOME, Linux non-GNOME and Windows, so we can accurately evaluate
what the change is.

Also, I tried to apply this series to test it myself, but it fails to
apply to git master for some reason

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer 00/10] Make dialogs use GtkHeaderBar

2017-02-16 Thread Fabiano Fidêncio 
On Thu, Feb 16, 2017 at 1:06 PM, Pavel Grunt  wrote:
> Hi,
>
> On Thu, 2017-02-09 at 17:22 -0200, Eduardo Lima (Etrunko) wrote:
>> This whole series is the result of the initial idea of having the
>> new
>> iso-dialog to use this widget. Having it done only for the iso-
>> dialog
>> would not make much sense, so I went on and ported all other
>> dialogs.
>> The widget requires Gtk version 3.12, which should not be a problem
>> for
>> any recent distro.
>>
>> Using GtkHeaderBar makes dialogs look cleaner and more 'modern',
>> also
>> follow the style used by GNOME applications. In the near future,
>> virt-viewer main window could also make use of it, as we recently
>> saw in
>> a series sent to this mailing list.
>
> so are we going for the GNOME style for all desktop envs ? In my
> opinion it does not look nice in Windows and Xfce - simply does not
> fit it in. iirc it was mentioned that there is a way to have both the
> classic and the GNOME look in the discussion about Sagar's patches few
> weeks ago..

Pavel,

May I ask you some screenshots on how it looks on XFCE?
If possible, would be possible to also take some screenshots of
Sagar's work as well?

Best Regards,

[snip]

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer 00/10] Make dialogs use GtkHeaderBar

2017-02-16 Thread Eduardo Lima (Etrunko) 
On 16/02/17 10:06, Pavel Grunt wrote:
> Hi,
> 
> On Thu, 2017-02-09 at 17:22 -0200, Eduardo Lima (Etrunko) wrote:
>> This whole series is the result of the initial idea of having the
>> new
>> iso-dialog to use this widget. Having it done only for the iso-
>> dialog
>> would not make much sense, so I went on and ported all other
>> dialogs.
>> The widget requires Gtk version 3.12, which should not be a problem
>> for
>> any recent distro.
>>
>> Using GtkHeaderBar makes dialogs look cleaner and more 'modern',
>> also
>> follow the style used by GNOME applications. In the near future,
>> virt-viewer main window could also make use of it, as we recently
>> saw in
>> a series sent to this mailing list.
> 
> so are we going for the GNOME style for all desktop envs ? In my
> opinion it does not look nice in Windows and Xfce - simply does not
> fit it in. iirc it was mentioned that there is a way to have both the
> classic and the GNOME look in the discussion about Sagar's patches few
> weeks ago..

I have no idea of what it looks like in Windows, but it indeed looks
good in Linux environments. I tested in both GNOME and Enlightenment,
not sure about other DEs out there. As far as I remember, the discussion
on that other series was about the classic menu versus the buttons on
the header bar.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer 00/10] Make dialogs use GtkHeaderBar

2017-02-16 Thread Pavel Grunt 
Hi,

On Thu, 2017-02-09 at 17:22 -0200, Eduardo Lima (Etrunko) wrote:
> This whole series is the result of the initial idea of having the
> new
> iso-dialog to use this widget. Having it done only for the iso-
> dialog
> would not make much sense, so I went on and ported all other
> dialogs.
> The widget requires Gtk version 3.12, which should not be a problem
> for
> any recent distro.
> 
> Using GtkHeaderBar makes dialogs look cleaner and more 'modern',
> also
> follow the style used by GNOME applications. In the near future,
> virt-viewer main window could also make use of it, as we recently
> saw in
> a series sent to this mailing list.

so are we going for the GNOME style for all desktop envs ? In my
opinion it does not look nice in Windows and Xfce - simply does not
fit it in. iirc it was mentioned that there is a way to have both the
classic and the GNOME look in the discussion about Sagar's patches few
weeks ago..

Pavel

> 
> Eduardo Lima (Etrunko) (10):
>   Update Gtk+ requirements to 3.12
>   iso-dialog: Use GtkHeaderBar
>   guest-details-dialog: Use GtkHeaderBar
>   virt-viewer-vm-connection: Use GtkHeaderBar
>   remote-viewer-connect: Use GtkHeaderBar
>   virt-viewer-preferences: Use GtkHeaderBar
>   about-dialog: Use GtkHeaderBar
>   virt-viewer-auth: Use GtkHeaderBar
>   file-transfer-dialog: Use GtkHeaderBar
>   usb_device_selection: Use GtkHeaderBar
> 
>  configure.ac   |   4 +-
>  src/remote-viewer-connect.c|  20 +--
>  src/remote-viewer-iso-list-dialog.c|  32 +++-
>  src/resources/ui/remote-viewer-connect.ui  | 173 ++--
> -
>  src/resources/ui/remote-viewer-iso-list.ui |  87 
> ---
>  src/resources/ui/virt-viewer-about.ui  |  20 ++-
>  src/resources/ui/virt-viewer-auth.ui   | 148 
> --
>  .../ui/virt-viewer-file-transfer-dialog.ui |  72 +++--
>  src/resources/ui/virt-viewer-guest-details.ui  |  64 
>  src/resources/ui/virt-viewer-preferences.ui| 103 ++--
> 
>  src/resources/ui/virt-viewer-vm-connection.ui  |  83 --
>  src/virt-viewer-app.c  |  38 +++--
>  src/virt-viewer-auth.c |  35 +++--
>  src/virt-viewer-file-transfer-dialog.c |  59 +++
>  src/virt-viewer-session-spice.c|  20 ++-
>  src/virt-viewer-vm-connection.c|  15 +-
>  src/virt-viewer-window.c   |  21 ++-
>  17 files changed, 437 insertions(+), 557 deletions(-)
> 

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list