[pve-devel] [PATCH v3 container] fix: #3927: Error codes when starting/stopping

[pve-devel] [PATCH v3 container] fix: #3927: Error codes when starting/stopping

[Daniel Tschlatscher]

 containers Stopping and starting a container that is already in the desired
 state will not return an error value on the CLI anymore. The same as VMs do,
 when they are stopped but not running.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v2: return statements are now in the correct subroutines. 
 src/PVE/API2/LXC/Status.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/LXC/Status.pm b/src/PVE/API2/LXC/Status.pm
index f7e3128..364db10 100644
--- a/src/PVE/API2/LXC/Status.pm
+++ b/src/PVE/API2/LXC/Status.pm
@@ -154,7 +154,7 @@ __PACKAGE__->register_method({
 	raise_param_exc({ skiplock => "Only root may use this option." })
 	    if $skiplock && $authuser ne 'root@pam';
 
-	die "CT $vmid already running\n" if PVE::LXC::check_running($vmid);
+	my $pid = PVE::LXC::check_running($vmid);
 
 	PVE::Cluster::check_cfs_quorum();
 
@@ -183,6 +183,8 @@ __PACKAGE__->register_method({
 		die "you can't start a CT if it's a template\n"
 		    if PVE::LXC::Config->is_template($conf);
 
+		return if $pid;			# Container is already running
+
 		if (!$skiplock && !PVE::LXC::Config->has_lock($conf, 'mounted')) {
 		    PVE::LXC::Config->check_lock($conf);
 		}
@@ -238,7 +240,7 @@ __PACKAGE__->register_method({
 	raise_param_exc({ skiplock => "Only root may use this option." })
 	    if $skiplock && $authuser ne 'root@pam';
 
-	die "CT $vmid not running\n" if !PVE::LXC::check_running($vmid);
+	my $pid = PVE::LXC::check_running($vmid);
 
 	if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne 'ha') {
 
@@ -260,6 +262,8 @@ __PACKAGE__->register_method({
 
 		syslog('info', "stopping CT $vmid: $upid\n");
 
+		return  if !$pid;
+
 		my $conf = PVE::LXC::Config->load_config($vmid);
 		if (!$skiplock && !PVE::LXC::Config->has_lock($conf, 'mounted')) {
 		    PVE::LXC::Config->check_lock($conf);
-- 
2.30.2

[pve-devel] [PATCH qemu-server] Starting a running VM does not return an error

[Daniel Tschlatscher]

 code anymore This was adapted to match the behaviour of stopping a VM that's
 already stopped and the new container patches.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 PVE/QemuServer.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6a9f6b3..8d82b6f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5346,7 +5346,7 @@ sub vm_start {
 
 	$params->{resume} = $has_suspended_lock || defined($conf->{vmstate});
 
-	die "VM $vmid already running\n" if $running;
+	return if $running;
 
 	if (my $storagemap = $migrate_opts->{storagemap}) {
 	    my $replicated = $migrate_opts->{replicated_volumes};
-- 
2.30.2

Re: [pve-devel] [PATCH v3 container] fix: #3927: Error codes when starting/stopping

[Fabian Ebner]

Am 21.03.22 um 10:33 schrieb Daniel Tschlatscher:
>  containers Stopping and starting a container that is already in the desired
>  state will not return an error value on the CLI anymore. The same as VMs do,
>  when they are stopped but not running.
> 
> Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>

This also applies to the second patch: I'd feel more comfortable keeping
the error exit codes for the start commands. There might be users
already relying upon this behavior. If start succeeds, we know we have a
freshly started instance. We actually rely on it a bit ourselves, for
migration to detect a bogus instance already running on the target node.

For the stop command, it's not as relevant IMHO, and I'm fine with
either way, but we might want to wait until the next major release as it
can be seen as a breaking API change too.