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

[Mikhail Feoktistov]

On 17.02.2017 4:42, Cole Robinson wrote:
> 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)
>
Virtuozzo hypervisor doesn't support restart for on_crash case, it 
always destroys container.
And user can't configure on_crash action.
you can see these checks in libvirt
src/vz/vz_sdk.c line 2599
function prlsdkCheckUnsupportedParams()

     if (def->onReboot != VIR_DOMAIN_LIFECYCLE_RESTART ||
         def->onPoweroff != VIR_DOMAIN_LIFECYCLE_DESTROY ||
         def->onCrash != VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY) {

         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("The following parameters must be "
                          "on_reboot = restart, on_poweroff = destroy, "
                          "on_crash = destroy. "
                          "Different actions are not supported by vz 
driver"));
         return -1;
     }

Thanks for the suggestion to make onCrash as "destroy" by default in 
virt-manager.
I'll do it in the next patch series.
>>           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.
My suggestion is to use createXML for transient domains only.
In other cases use defineXML
I'll send another patch
>
> Thanks,
> Cole
>