[PATCH 0/2] Warn before Power Off non-persistent VM
Adding a new field for VM properties, and a new error message. Note: The error message needs Portuguese and Chinese translation. Christy Perez (2): Add persistent flag to VM info Display appropriate warning for Power Off of non-persistent VM docs/API.md | 2 ++ po/en_US.po | 3 +++ po/pt_BR.po | 3 +++ po/zh_CN.po | 3 +++ src/kimchi/model/vms.py | 3 ++- ui/js/src/kimchi.guest_main.js | 6 +++++- ui/pages/i18n.json.tmpl | 1 + 7 files changed, 19 insertions(+), 2 deletions(-) -- 1.9.3
Kimchi can manage guests not created by Kimchi. If a user creates a non-persistent domain and uses the Power Off option, it will destroy the user's domain. In order to warn users with non-persistent guests on Power Off, this patch adds a 'persistent' field (like the one for networks and storage pools) to a vm's JSON representation. Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- docs/API.md | 2 ++ src/kimchi/model/vms.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/API.md b/docs/API.md index d75c55f..ebb6e61 100644 --- a/docs/API.md +++ b/docs/API.md @@ -45,6 +45,8 @@ the following general conventions: * **POST**: Create a new Virtual Machine * name *(optional)*: The name of the VM. Used to identify the VM in this API. If omitted, a name will be chosen based on the template used. + * persistent: If 'true', vm will persist after a Power Off or host reboot. + All virtual machines created by Kimchi are persistent. * template: The URI of a Template to use when building the VM * storagepool *(optional)*: Assign a specific Storage Pool to the new VM * graphics *(optional)*: Specify the graphics paramenter for this vm diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 476e4ac..5721b48 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -471,7 +471,8 @@ def lookup(self, name): 'ticket': self._get_ticket(dom), 'users': users, 'groups': groups, - 'access': 'full' + 'access': 'full', + 'persistent': True if dom.isPersistent() else False } def _vm_get_disk_paths(self, dom): -- 1.9.3
On 20-08-2014 19:43, Christy Perez wrote:
Kimchi can manage guests not created by Kimchi. If a user creates a non-persistent domain and uses the Power Off option, it will destroy the user's domain. In order to warn users with non-persistent guests on Power Off, this patch adds a 'persistent' field (like the one for networks and storage pools) to a vm's JSON representation.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com>
This patch breaks the tests. Please run "sudo make check" and fix the errors. Other than fixing the test errors, please add a few specific tests for this property as well.
- 'access': 'full' + 'access': 'full', + 'persistent': True if dom.isPersistent() else False "'persistent': dom.isPersistent()" should do the same thing in a clearer way.
On 08/21/2014 12:15 PM, Crístian Viana wrote:
On 20-08-2014 19:43, Christy Perez wrote:
Kimchi can manage guests not created by Kimchi. If a user creates a non-persistent domain and uses the Power Off option, it will destroy the user's domain. In order to warn users with non-persistent guests on Power Off, this patch adds a 'persistent' field (like the one for networks and storage pools) to a vm's JSON representation.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com>
This patch breaks the tests. Please run "sudo make check" and fix the errors. Other than fixing the test errors, please add a few specific tests for this property as well. ACK. Thanks.
- 'access': 'full' + 'access': 'full', + 'persistent': True if dom.isPersistent() else False "'persistent': dom.isPersistent()" should do the same thing in a clearer way. I'm pretty sure I had that first, and just got back '1'. I'll double-check, though.
On 08/21/2014 12:34 PM, Christy Perez wrote:
On 08/21/2014 12:15 PM, Crístian Viana wrote:
On 20-08-2014 19:43, Christy Perez wrote:
Kimchi can manage guests not created by Kimchi. If a user creates a non-persistent domain and uses the Power Off option, it will destroy the user's domain. In order to warn users with non-persistent guests on Power Off, this patch adds a 'persistent' field (like the one for networks and storage pools) to a vm's JSON representation.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com>
This patch breaks the tests. Please run "sudo make check" and fix the errors. Other than fixing the test errors, please add a few specific tests for this property as well. ACK. Thanks.
- 'access': 'full' + 'access': 'full', + 'persistent': True if dom.isPersistent() else False "'persistent': dom.isPersistent()" should do the same thing in a clearer way. I'm pretty sure I had that first, and just got back '1'. I'll double-check, though.
Just to follow up here, changing it did result in '1' being output instead of 'true.' Since everything else uses 'true' or 'false', I'm going to leave this as-is. Here's a snip with just the result from dom.isPersistent(): { "users":[], "screenshot":"/data/screenshots/79b8fe8b-4697-445c-aee5-3a781f27b61d-f113f739-0249-4913-afdd-5d7e662660f1.png", "cpus":1, "persistent":1, "groups":[], "graphics":{ "type":"vnc", "port":5900, "listen":"0.0.0.0" }, Regards, - Christy
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 08/21/2014 04:32 PM, Christy Perez wrote:
On 08/21/2014 12:34 PM, Christy Perez wrote:
On 08/21/2014 12:15 PM, Crístian Viana wrote:
Kimchi can manage guests not created by Kimchi. If a user creates a non-persistent domain and uses the Power Off option, it will destroy the user's domain. In order to warn users with non-persistent guests on Power Off, this patch adds a 'persistent' field (like the one for networks and storage pools) to a vm's JSON representation.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> This patch breaks the tests. Please run "sudo make check" and fix the errors. Other than fixing the test errors, please add a few specific tests for
On 20-08-2014 19:43, Christy Perez wrote: this property as well. ACK. Thanks.
- 'access': 'full' + 'access': 'full', + 'persistent': True if dom.isPersistent() else False "'persistent': dom.isPersistent()" should do the same thing in a clearer way. I'm pretty sure I had that first, and just got back '1'. I'll double-check, though.
Just to follow up here, changing it did result in '1' being output instead of 'true.' Since everything else uses 'true' or 'false', I'm going to leave this as-is.
You can use bool(dom.isPersistent())
Here's a snip with just the result from dom.isPersistent(): { "users":[],
"screenshot":"/data/screenshots/79b8fe8b-4697-445c-aee5-3a781f27b61d-f113f739-0249-4913-afdd-5d7e662660f1.png", "cpus":1, "persistent":1, "groups":[], "graphics":{ "type":"vnc", "port":5900, "listen":"0.0.0.0" },
Regards,
- Christy
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 21-08-2014 16:32, Christy Perez wrote:
Just to follow up here, changing it did result in '1' being output instead of 'true.' Since everything else uses 'true' or 'false', I'm going to leave this as-is.
Here's a snip with just the result from dom.isPersistent(): { "users":[],
"screenshot":"/data/screenshots/79b8fe8b-4697-445c-aee5-3a781f27b61d-f113f739-0249-4913-afdd-5d7e662660f1.png", "cpus":1, "persistent":1, "groups":[], "graphics":{ "type":"vnc", "port":5900, "listen":"0.0.0.0" },
You're right, it seems that this method doesn't return boolean natively, it returns int (http://libvirt.org/html/libvirt-libvirt.html#virDomainIsPersistent). I thought it returned boolean (it's an "isXXX" method...), so we could just use its return value.
Add a check in the UI to check the new 'persistent' flag for a VM. If the VM is non-persistent, report the danger of using Power Off. If the VM is not, warn that data may be lost (the current warning). Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- po/en_US.po | 3 +++ po/pt_BR.po | 3 +++ po/zh_CN.po | 3 +++ ui/js/src/kimchi.guest_main.js | 6 +++++- ui/pages/i18n.json.tmpl | 1 + 5 files changed, 15 insertions(+), 1 deletion(-) diff --git a/po/en_US.po b/po/en_US.po index a34da3a..387c748 100644 --- a/po/en_US.po +++ b/po/en_US.po @@ -1371,6 +1371,9 @@ msgstr "" "This action may produce undesirable results, for example unflushed disk " "cache in the guest. Would you like to continue?" +msgid "This VM is not persistent. Power Off will destroy it. Continue?" +msgstr "This VM is not persistent. Power Off will destroy it. Continue?" + msgid "Reset Confirmation" msgstr "Reset Confirmation" diff --git a/po/pt_BR.po b/po/pt_BR.po index 452e778..f5b7958 100644 --- a/po/pt_BR.po +++ b/po/pt_BR.po @@ -1434,6 +1434,9 @@ msgstr "" "Essa ação pode produzir resultados não desejáveis, como por exemplo cache de " "disco não atualizado no guest. Deseja continuar?" +msgid "This VM is not persistent. Power Off will destroy it. Continue?" +msgstr "This VM is not persistent. Power Off will destroy it. Continue?" + msgid "Reset Confirmation" msgstr "Confirmação de reinicialização" diff --git a/po/zh_CN.po b/po/zh_CN.po index 83c7018..5b6ed6e 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -1318,6 +1318,9 @@ msgid "" "cache in the guest. Would you like to continue?" msgstr "这样做可能导致不良后果，比如客户机磁盘缓存未刷新，确认要继续吗？" +msgid "This VM is not persistent. Power Off will destroy it. Continue?" +msgstr "This VM is not persistent. Power Off will destroy it. Continue?" + msgid "Reset Confirmation" msgstr "重置确认" diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index ff6f2e1..ff66744 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -43,9 +43,13 @@ kimchi.vmpoweroff = function(event) { button.addClass('loading'); var vm=button.closest('li[name=guest]'); var vm_id=vm.attr("id"); + var vmObject=vm.data(); + var vm_persistent=vmObject.persistent == true; + var content_msg = vm_persistent ? i18n['KCHVM6003M'] : + i18n['KCHVM6009M']; var settings = { title : i18n['KCHVM6002M'], - content : i18n['KCHVM6003M'], + content : content_msg, confirm : i18n['KCHAPI6002M'], cancel : i18n['KCHAPI6003M'] }; diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl index ccfb081..aa41bf0 100644 --- a/ui/pages/i18n.json.tmpl +++ b/ui/pages/i18n.json.tmpl @@ -126,6 +126,7 @@ "KCHVM6006M": "$_("Shut Down Confirmation")", "KCHVM6007M": "$_("Note the guest OS may ignore this request. Would you like to continue?")", "KCHVM6008M": "$_("VM Delete Confirmation")", + "KCHVM6009M": "$_("This VM is not persistent. Power Off will destroy it. Continue?")", "KCHVMCD6001M": "$_("This CDROM will be detached permanently and you can re-attach it. Continue to detach it?")", "KCHVMCD6002M": "$_("Attach")", -- 1.9.3
On 08/20/2014 07:43 PM, Christy Perez wrote:
Add a check in the UI to check the new 'persistent' flag for a VM. If the VM is non-persistent, report the danger of using Power Off. If the VM is not, warn that data may be lost (the current warning).
We provide to user 2 options to shutdown the VM: shutdown and power off. From my understating, in both cases the VM will be deleted if it is not persistent. So we need to handle both.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- po/en_US.po | 3 +++ po/pt_BR.po | 3 +++ po/zh_CN.po | 3 +++ ui/js/src/kimchi.guest_main.js | 6 +++++- ui/pages/i18n.json.tmpl | 1 + 5 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/po/en_US.po b/po/en_US.po index a34da3a..387c748 100644 --- a/po/en_US.po +++ b/po/en_US.po @@ -1371,6 +1371,9 @@ msgstr "" "This action may produce undesirable results, for example unflushed disk " "cache in the guest. Would you like to continue?"
+msgid "This VM is not persistent. Power Off will destroy it. Continue?" +msgstr "This VM is not persistent. Power Off will destroy it. Continue?" +
... Power Off will delete it, Do you want to continue? (I think "delete" is more human being than "destroy" hehe)
msgid "Reset Confirmation" msgstr "Reset Confirmation"
diff --git a/po/pt_BR.po b/po/pt_BR.po index 452e778..f5b7958 100644 --- a/po/pt_BR.po +++ b/po/pt_BR.po @@ -1434,6 +1434,9 @@ msgstr "" "Essa ação pode produzir resultados não desejáveis, como por exemplo cache de " "disco não atualizado no guest. Deseja continuar?"
+msgid "This VM is not persistent. Power Off will destroy it. Continue?" +msgstr "This VM is not persistent. Power Off will destroy it. Continue?"
A máquina virtual não é persistente. Desligá-la irá removê-la. Deseja continuar?
+ msgid "Reset Confirmation" msgstr "Confirmação de reinicialização"
diff --git a/po/zh_CN.po b/po/zh_CN.po index 83c7018..5b6ed6e 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -1318,6 +1318,9 @@ msgid "" "cache in the guest. Would you like to continue?" msgstr "这样做可能导致不良后果，比如客户机磁盘缓存未刷新，确认要继续吗？"
+msgid "This VM is not persistent. Power Off will destroy it. Continue?" +msgstr "This VM is not persistent. Power Off will destroy it. Continue?" + msgid "Reset Confirmation" msgstr "重置确认"
diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index ff6f2e1..ff66744 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -43,9 +43,13 @@ kimchi.vmpoweroff = function(event) { button.addClass('loading'); var vm=button.closest('li[name=guest]'); var vm_id=vm.attr("id"); + var vmObject=vm.data(); + var vm_persistent=vmObject.persistent == true; + var content_msg = vm_persistent ? i18n['KCHVM6003M'] : + i18n['KCHVM6009M']; var settings = { title : i18n['KCHVM6002M'], - content : i18n['KCHVM6003M'], + content : content_msg, confirm : i18n['KCHAPI6002M'], cancel : i18n['KCHAPI6003M'] }; diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl index ccfb081..aa41bf0 100644 --- a/ui/pages/i18n.json.tmpl +++ b/ui/pages/i18n.json.tmpl @@ -126,6 +126,7 @@ "KCHVM6006M": "$_("Shut Down Confirmation")", "KCHVM6007M": "$_("Note the guest OS may ignore this request. Would you like to continue?")", "KCHVM6008M": "$_("VM Delete Confirmation")", + "KCHVM6009M": "$_("This VM is not persistent. Power Off will destroy it. Continue?")",
"KCHVMCD6001M": "$_("This CDROM will be detached permanently and you can re-attach it. Continue to detach it?")", "KCHVMCD6002M": "$_("Attach")",
On 08/21/2014 09:44 AM, Aline Manera wrote:
On 08/20/2014 07:43 PM, Christy Perez wrote:
Add a check in the UI to check the new 'persistent' flag for a VM. If the VM is non-persistent, report the danger of using Power Off. If the VM is not, warn that data may be lost (the current warning).
We provide to user 2 options to shutdown the VM: shutdown and power off. From my understating, in both cases the VM will be deleted if it is not persistent. So we need to handle both.
Shut Down will not call destroy. It will call shutdown, which won't delete the guest.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> --- po/en_US.po | 3 +++ po/pt_BR.po | 3 +++ po/zh_CN.po | 3 +++ ui/js/src/kimchi.guest_main.js | 6 +++++- ui/pages/i18n.json.tmpl | 1 + 5 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/po/en_US.po b/po/en_US.po index a34da3a..387c748 100644 --- a/po/en_US.po +++ b/po/en_US.po @@ -1371,6 +1371,9 @@ msgstr "" "This action may produce undesirable results, for example unflushed disk " "cache in the guest. Would you like to continue?" +msgid "This VM is not persistent. Power Off will destroy it. Continue?" +msgstr "This VM is not persistent. Power Off will destroy it. Continue?" +
... Power Off will delete it, Do you want to continue?
(I think "delete" is more human being than "destroy" hehe)
I was just using the virsh command/terminology. But destroy does sound pretty dramatic. ;)
msgid "Reset Confirmation" msgstr "Reset Confirmation" diff --git a/po/pt_BR.po b/po/pt_BR.po index 452e778..f5b7958 100644 --- a/po/pt_BR.po +++ b/po/pt_BR.po @@ -1434,6 +1434,9 @@ msgstr "" "Essa ação pode produzir resultados não desejáveis, como por exemplo cache de " "disco não atualizado no guest. Deseja continuar?" +msgid "This VM is not persistent. Power Off will destroy it. Continue?" +msgstr "This VM is not persistent. Power Off will destroy it. Continue?"
A máquina virtual não é persistente. Desligá-la irá removê-la. Deseja continuar?
Thanks! I still need the Chinese translation, so when I get that I'll send v2.
+ msgid "Reset Confirmation" msgstr "Confirmação de reinicialização" diff --git a/po/zh_CN.po b/po/zh_CN.po index 83c7018..5b6ed6e 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -1318,6 +1318,9 @@ msgid "" "cache in the guest. Would you like to continue?" msgstr "这样做可能导致不良后果，比如客户机磁盘缓存未刷新，确认要继续 吗？" +msgid "This VM is not persistent. Power Off will destroy it. Continue?" +msgstr "This VM is not persistent. Power Off will destroy it. Continue?" + msgid "Reset Confirmation" msgstr "重置确认" diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index ff6f2e1..ff66744 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -43,9 +43,13 @@ kimchi.vmpoweroff = function(event) { button.addClass('loading'); var vm=button.closest('li[name=guest]'); var vm_id=vm.attr("id"); + var vmObject=vm.data(); + var vm_persistent=vmObject.persistent == true; + var content_msg = vm_persistent ? i18n['KCHVM6003M'] : + i18n['KCHVM6009M']; var settings = { title : i18n['KCHVM6002M'], - content : i18n['KCHVM6003M'], + content : content_msg, confirm : i18n['KCHAPI6002M'], cancel : i18n['KCHAPI6003M'] }; diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl index ccfb081..aa41bf0 100644 --- a/ui/pages/i18n.json.tmpl +++ b/ui/pages/i18n.json.tmpl @@ -126,6 +126,7 @@ "KCHVM6006M": "$_("Shut Down Confirmation")", "KCHVM6007M": "$_("Note the guest OS may ignore this request. Would you like to continue?")", "KCHVM6008M": "$_("VM Delete Confirmation")", + "KCHVM6009M": "$_("This VM is not persistent. Power Off will destroy it. Continue?")", "KCHVMCD6001M": "$_("This CDROM will be detached permanently and you can re-attach it. Continue to detach it?")", "KCHVMCD6002M": "$_("Attach")",
Regards, - Christy
participants (3)
-
Aline Manera -
Christy Perez -
Crístian Viana