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
