Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
First, thanks for the patch. But please can you split the patch into smaller ones? Signed-off-by: Dan Hunsaker danhunsa...@gmail.com --- PVE/API2/OpenVZ.pm| 308 +++--- PVE/OpenVZ.pm | 92 - bin/pvectl| 16 ++- www/manager/Utils.js | 80 +-- www/manager/openvz/CmdMenu.js | 28 +++- www/manager/qemu/CmdMenu.js | 26 +++- www/mobile/OpenVzSummary.js | 30 ++-- www/mobile/QemuSummary.js | 34 +++-- 8 files changed, 401 insertions(+), 213 deletions(-) diff --git a/PVE/API2/OpenVZ.pm b/PVE/API2/OpenVZ.pm index 184ebdf..5d8c0c6 100644 --- a/PVE/API2/OpenVZ.pm +++ b/PVE/API2/OpenVZ.pm @@ -71,7 +71,7 @@ my $get_container_storage = sub { my $check_ct_modify_config_perm = sub { my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_; - + And there are tons of white-space changes. Please remove them first. - Dietmar ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
I noticed the whitespace changes after I sent this one, and resent it without them shortly after. Sent a reply that I was going to resend, but it seems not all my emails get through to the list? How would you recommend I split the changes? They're all related directly to providing suspend/resume support. On Oct 3, 2014 12:25 AM, Dietmar Maurer diet...@proxmox.com wrote: First, thanks for the patch. But please can you split the patch into smaller ones? Signed-off-by: Dan Hunsaker danhunsa...@gmail.com --- PVE/API2/OpenVZ.pm| 308 +++--- PVE/OpenVZ.pm | 92 - bin/pvectl| 16 ++- www/manager/Utils.js | 80 +-- www/manager/openvz/CmdMenu.js | 28 +++- www/manager/qemu/CmdMenu.js | 26 +++- www/mobile/OpenVzSummary.js | 30 ++-- www/mobile/QemuSummary.js | 34 +++-- 8 files changed, 401 insertions(+), 213 deletions(-) diff --git a/PVE/API2/OpenVZ.pm b/PVE/API2/OpenVZ.pm index 184ebdf..5d8c0c6 100644 --- a/PVE/API2/OpenVZ.pm +++ b/PVE/API2/OpenVZ.pm @@ -71,7 +71,7 @@ my $get_container_storage = sub { my $check_ct_modify_config_perm = sub { my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_; - + And there are tons of white-space changes. Please remove them first. - Dietmar ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
How would you recommend I split the changes? They're all related directly to providing suspend/resume support. 1.) Implement suspend/resume API 2.) add it to pvectl 3.) Implement suspend/resume GUI (extjs) 4.) Implement suspend/resume GUI (mobile) Alright, I'll make that happen tomorrow. Currently just after 02:00 here. :-) I also have some further ideas. Currently qemu suspend/resume does not save state to disk. It would be great to implement that also. I'll have to research that some, but I should be able to write a patch for that as well. Then implement an option in datacenter.cfg like: reboot: stop|suspend So that VMs are suspended while we reboot a host. What do you think? That would probably save a *lot* of time bringing servers back up after reboot. I'll look into that as well, probably next week. To go another step with that logic, I wonder if there might be a benefit to modifying QEMU migrations so they suspend with state, transfer the suspended VM, and resume on the destination node. I could see an advanced implementation where VM snapshots are taken periodically, and if the node experiences a power failure, the VM could resume from the snapshot. HA failover could take advantage of the same snapshots in the same way, thereby (hopefully) losing less data, and possibly resulting in less downtime. This would definitely need to be an option enabled on VMs that would benefit from such an approach, rather than enabled universally, and is advanced enough it might remain in the realm of third-party scripts or packages, but it still might be useful. Before I get too far into the QEMU suspend-with-state patch, I want to ask - does OpenVZ support suspend-with-state? Might be nice to support that in the patch, too, if it does. ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
1.) Implement suspend/resume API 2.) add it to pvectl 3.) Implement suspend/resume GUI (extjs) 4.) Implement suspend/resume GUI (mobile) Alright, I'll make that happen tomorrow. Currently just after 02:00 here. :-) Thanks! I also have some further ideas. Currently qemu suspend/resume does not save state to disk. It would be great to implement that also. I'll have to research that some, but I should be able to write a patch for that as well. Then implement an option in datacenter.cfg like: reboot: stop|suspend So that VMs are suspended while we reboot a host. What do you think? That would probably save a *lot* of time bringing servers back up after reboot. I'll look into that as well, probably next week. OK To go another step with that logic, I wonder if there might be a benefit to modifying QEMU migrations so they suspend with state, transfer the suspended VM, and resume on the destination node. This is how migrate works (basically). Or what is the difference? I could see an advanced implementation where VM snapshots are taken periodically, and if the node experiences a power failure, the VM could resume from the snapshot. HA failover could take advantage of the same snapshots in the same way, thereby (hopefully) losing less data, and possibly resulting in less downtime. This would definitely need to be an option enabled on VMs that would benefit from such an approach, rather than enabled universally, and is advanced enough it might remain in the realm of third-party scripts or packages, but it still might be useful. Before I get too far into the QEMU suspend-with-state patch, I want to ask - does OpenVZ support suspend-with-state? Might be nice to support that in the patch, too, if it does. You already implemented that! chkpnt CTID [--dumpfile name] This command saves a complete state of a running container to a dump file, and stops the container. If an option --dumpfile is not set, default dump file name /vz/dump/Dump.CTID is used. ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
Actually, that part wasn't me, but since the answer is yes, I'll look into getting QEMU to save state to disk so we can do the rest. :-) And now to sleep, finally... On Oct 3, 2014 2:53 AM, Dietmar Maurer diet...@proxmox.com wrote: 1.) Implement suspend/resume API 2.) add it to pvectl 3.) Implement suspend/resume GUI (extjs) 4.) Implement suspend/resume GUI (mobile) Alright, I'll make that happen tomorrow. Currently just after 02:00 here. :-) Thanks! I also have some further ideas. Currently qemu suspend/resume does not save state to disk. It would be great to implement that also. I'll have to research that some, but I should be able to write a patch for that as well. Then implement an option in datacenter.cfg like: reboot: stop|suspend So that VMs are suspended while we reboot a host. What do you think? That would probably save a *lot* of time bringing servers back up after reboot. I'll look into that as well, probably next week. OK To go another step with that logic, I wonder if there might be a benefit to modifying QEMU migrations so they suspend with state, transfer the suspended VM, and resume on the destination node. This is how migrate works (basically). Or what is the difference? I could see an advanced implementation where VM snapshots are taken periodically, and if the node experiences a power failure, the VM could resume from the snapshot. HA failover could take advantage of the same snapshots in the same way, thereby (hopefully) losing less data, and possibly resulting in less downtime. This would definitely need to be an option enabled on VMs that would benefit from such an approach, rather than enabled universally, and is advanced enough it might remain in the realm of third-party scripts or packages, but it still might be useful. Before I get too far into the QEMU suspend-with-state patch, I want to ask - does OpenVZ support suspend-with-state? Might be nice to support that in the patch, too, if it does. You already implemented that! chkpnt CTID [--dumpfile name] This command saves a complete state of a running container to a dump file, and stops the container. If an option --dumpfile is not set, default dump file name /vz/dump/Dump.CTID is used. ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
Oops. That's a lot of whitespace changes. Let me resubmit again without them... ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
Thanks. I will try to add that after the 3.2 release, which is planned next week. As discussed in a previous thread, following is a patch to support container suspend (via vzctl chkpnt) and resume (via vzctl restore). ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
Alright. If I need to rebase again by then, let me know. On Mar 4, 2014 1:02 AM, Dietmar Maurer diet...@proxmox.com wrote: Thanks. I will try to add that after the 3.2 release, which is planned next week. As discussed in a previous thread, following is a patch to support container suspend (via vzctl chkpnt) and resume (via vzctl restore). ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
Odd, I based it on the latest public master... Will do when I get home in a few hours. On Mar 2, 2014 11:57 PM, Dietmar Maurer diet...@proxmox.com wrote: Please can you rebase your patch? Applying: Add CT suspend/resume to PVE API error: patch failed: PVE/API2/OpenVZ.pm:1391 error: PVE/API2/OpenVZ.pm: patch does not apply error: patch failed: PVE/OpenVZ.pm:1205 ... -Original Message- From: pve-devel [mailto:pve-devel-boun...@pve.proxmox.com] On Behalf Of Daniel Hunsaker Sent: Samstag, 01. März 2014 17:37 To: pve-devel@pve.proxmox.com Subject: [pve-devel] [PATCH] Add CT suspend/resume to PVE API As discussed in a previous thread, following is a patch to support container suspend (via vzctl chkpnt) and resume (via vzctl restore). ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH] Add CT suspend/resume to PVE API
Rebased on latest public master. - Daniel Hunsaker Owner / Developer Lei's Genesis Experiment: Code For The Future! On Mon, Mar 3, 2014 at 2:16 PM, Daniel Hunsaker danhunsa...@gmail.comwrote: As discussed in a previous thread, following is a patch to support container suspend (via vzctl chkpnt) and resume (via vzctl restore). - Added /nodes/{node}/openvz/{vmid}/status/suspend to API - Added /nodes/{node}/openvz/{vmid}/status/resume to API - Adapted vm_suspend/vm_resume from PVE/QemuServer.pm into PVE/OpenVZ.pm - Removed locking since vzctl already does this for us, and the locks conflict with each other (container already locked) - Changed monitor commands to run_command(vzctl) calls - Refuse to suspend if CT is offline - Refuse to resume if CT is online - vzctl does these checks as well, but it doesn't really hurt to have them This was great, but there were artifacts in the web UI - specifically, the task descriptions were unformatted. So, I moved over to the web UI code... - Added descriptions for vzsuspend and vzresume tasks in web UI And while I was there anyway... - Added suspend/resume options to CmdMenu for both OpenVZ and QEMU guests - Confirm suspend before proceeding - No confirm on resume, since it's a startup action - Fixed OpenVZ CmdMenu shutdown and stop confirmation prompts to refer to CTs I considered adding these options to the toolbar, but there are enough options there already that it can get crowded quick in smaller browser windows (such as the ones I tend to use, for screen real estate purposes), so I opted against that. Signed-off-by: Daniel Hunsaker danhunsa...@gmail.com --- PVE/API2/OpenVZ.pm| 97 +++ PVE/OpenVZ.pm | 26 +++- www/manager/Utils.js | 2 + www/manager/openvz/CmdMenu.js | 25 ++- www/manager/qemu/CmdMenu.js | 21 ++ 5 files changed, 168 insertions(+), 3 deletions(-) diff --git a/PVE/API2/OpenVZ.pm b/PVE/API2/OpenVZ.pm index d9993fd..31f1b73 100644 --- a/PVE/API2/OpenVZ.pm +++ b/PVE/API2/OpenVZ.pm @@ -1391,6 +1391,103 @@ __PACKAGE__-register_method({ }}); __PACKAGE__-register_method({ + name = 'vm_suspend', + path = '{vmid}/status/suspend', + method = 'POST', + protected = 1, + proxyto = 'node', + description = Suspend the container., + permissions = { + check = ['perm', '/vms/{vmid}', [ 'VM.PowerMgmt' ]], + }, + parameters = { + additionalProperties = 0, + properties = { + node = get_standard_option('pve-node'), + vmid = get_standard_option('pve-vmid'), + }, + }, + returns = { + type = 'string', + }, + code = sub { + my ($param) = @_; + + my $rpcenv = PVE::RPCEnvironment::get(); + + my $authuser = $rpcenv-get_user(); + + my $node = extract_param($param, 'node'); + + my $vmid = extract_param($param, 'vmid'); + + die CT $vmid not running\n if !PVE::OpenVZ::check_running($vmid); + + my $realcmd = sub { + my $upid = shift; + + syslog('info', suspend CT $vmid: $upid\n); + + PVE::OpenVZ::vm_suspend($vmid); + + return; + }; + + my $upid = $rpcenv-fork_worker('vzsuspend', $vmid, $authuser, $realcmd); + + return $upid; + }}); + +__PACKAGE__-register_method({ + name = 'vm_resume', + path = '{vmid}/status/resume', + method = 'POST', + protected = 1, + proxyto = 'node', + description = Resume the container., + permissions = { + check = ['perm', '/vms/{vmid}', [ 'VM.PowerMgmt' ]], + }, + parameters = { + additionalProperties = 0, + properties = { + node = get_standard_option('pve-node'), + vmid = get_standard_option('pve-vmid'), + }, + }, + returns = { + type = 'string', + }, + code = sub { + my ($param) = @_; + + my $rpcenv = PVE::RPCEnvironment::get(); + + my $authuser = $rpcenv-get_user(); + + my $node = extract_param($param, 'node'); + + my $vmid = extract_param($param, 'vmid'); + + die CT $vmid already running\n if PVE::OpenVZ::check_running($vmid); + + my $realcmd = sub { + my $upid = shift; + + syslog('info', resume CT $vmid: $upid\n); + + PVE::OpenVZ::vm_resume($vmid); + + return; + }; + +