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

[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